petra: adapt garrisoning to the attackers
ClosedPublic
Actions

Authored by mimo on May 7 2017, 11:34 AM.

Details

Reviewers

Commits

rP19547: petra: adapt garrisoning to the attackers

Summary

When a structure with arrow was attacked, it was garrisoned independently of the attacker, which was not the best answer when atacked by a ram for example. This patch tries to adapt the garrisoning to the attackers.

Test Plan

Check in game that the AI reacts as expected

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mimo created this revision.May 7 2017, 11:34 AM

remove some leftover debug changes

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1054/ for more details.

Harbormaster completed remote builds in B1512: Diff 1711.May 7 2017, 5:09 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1055/ for more details.

Harbormaster completed remote builds in B1513: Diff 1712.May 7 2017, 6:47 PM

Nice! These sorts of improvements to garrisoning attacked structures are great to have.

I tested it briefly and will test it more soon, I like the looks of these changes.

Sandarac accepted this revision.May 9 2017, 7:52 AM

Sandarac added inline comments.

binaries/data/mods/public/simulation/ai/petra/garrisonManager.js
112 ↗	(On Diff #1712)	It seems "inertStructure" is not really used, although I guess in the future it could.
285 ↗	(On Diff #1712)	Maybe it would make sense to also include rangeSiege? (it's not a big thing)

This revision is now accepted and ready to land.May 9 2017, 7:52 AM

Also, I think at some point it would be best to explicitly target melee siege units, as they do a lot of damage to structures.

Thanks for the review. As this patch is now well tested :) i'll commit it in its current state and adress your comments (emergency case and removal of inertStructures) in a following one where the health level of the units to be ungarrisoned is also taken into account.

In D446#18682, @Sandarac wrote:

Also, I think at some point it would be best to explicitly target melee siege units, as they do a lot of damage to structures.

yes, but should be a in another patch as it would mainly involve the defenseArmy.js file (function assignUnit) i guess.

Closed by commit rP19547: petra: adapt garrisoning to the attackers (authored by mimo). · Explain WhyMay 9 2017, 7:54 PM

This revision was automatically updated to reflect the committed changes.

mimo mentioned this in D468: follow-up and complement of D446 to adapt garrisoning to the structure attackers.May 9 2017, 8:22 PM

refs #4571