As suggested in D333, Petra should prioritize capturing gaia relics that are within its territory. While this does not usually happen at game start with the current waythe relics are spawned in the victory condition (the relics are placed in neutral territory unless there are no neutral spawn points), it happens often when the AI gains new territory.
Details
Details
- Reviewers
mimo - Commits
- rP19644: Petra: Improve relic capturing
Run some games to test that the AI will capture gaia relics on its territory.
Diff Detail
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
Comment Actions
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/1289/ for more details.
Comment Actions
Thanks for the new patch. I've made some comments inlined.
| binaries/data/mods/public/simulation/ai/petra/attackManager.js | ||
|---|---|---|
| 504 ↗ | (On Diff #2090) | This function is a copy of lines 250/260, so if the code stays like that, you should replace these lines by a call to that new function. But another possibility would be that before the loop on the potential targets from the defenseManager, we do another loop on targets from the gametypeManager (so on targetedRelics) and do the loop on defenseManager.targetList only if nothing is found. |
| binaries/data/mods/public/simulation/ai/petra/gameTypeManager.js | ||
| 561 ↗ | (On Diff #2090) | No more needed if attackManager loops on this.targetedRelics as proposed above |
Comment Actions
Remove duplicated code for raids, and rename targetEntity function to raidTargetEntity.
| binaries/data/mods/public/simulation/ai/petra/attackManager.js | ||
|---|---|---|
| 504 ↗ | (On Diff #2090) | I feel rather inclined to keep this code the way it is, so I've removed the duplication in the update function. Also, targetedRelics consists of relics that currently have an attack plan targeting them, so I'm not sure if the second option you proposed would work. |
Comment Actions
I missed something previously: it is not clear to me if the targetedRelics should only contains gaia relics (which i would support as a raid may not be adapted to fight for an enemy relic) or not: if yes, i've indicated inlined some changes which are needed. If not (i.e. if targetedRelics also contains player relics), we should add a way to remove them if the relic has been moved into enemy territory.
| binaries/data/mods/public/simulation/ai/petra/gameTypeManager.js | ||
|---|---|---|
| 21 ↗ | (On Diff #2137) | Do i understand correctly that it contains only gaia relics? if so, this.targetedGaiaRelics would be more explicit |
| 259 ↗ | (On Diff #2137) | We should also take into account the case where a gaia relic is captured by somebody else, should be removed from this list if only gaia relic in targetedRelics, or should be removed when displaced if all relics in targetedRelics. |
| 557 ↗ | (On Diff #2137) | do you really want to also target relics which have already been captured by an enemy, but are still in our territory? or only gaia ones? |
Comment Actions
Sorry, I wasn't clear :/, targetedRelics indeed only contains gaia relics that have an attack plan assigned to them, so yes, targetedGaiaRelics would be a much better name.
| binaries/data/mods/public/simulation/ai/petra/gameTypeManager.js | ||
|---|---|---|
| 21 ↗ | (On Diff #2137) | Yes. |
| 259 ↗ | (On Diff #2137) | Okay. |
| 557 ↗ | (On Diff #2137) | I guess it would be more sensible to check only for gaia here. |
Comment Actions
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/1314/ for more details.
Comment Actions
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/1315/ for more details.
Comment Actions
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/1320/ for more details.
Comment Actions
Thanks for the changes. I've a last inlined comment :) but i can also modify it when commiting (not needed to upload a new pach), just let me know if you agree.
| binaries/data/mods/public/simulation/ai/petra/gameTypeManager.js | ||
|---|---|---|
| 246 ↗ | (On Diff #2143) | we should not have this continue if evt.to == PlayerID as we want to add it in criticalEnts. |
| binaries/data/mods/public/simulation/ai/petra/gameTypeManager.js | ||
|---|---|---|
| 246 ↗ | (On Diff #2143) | Yes, I agree with this change. |