Page MenuHomeWildfire Games

Petra: Capture gaia relics
ClosedPublic

Authored by Sandarac on Apr 14 2017, 3:51 PM.

Details

Reviewers
mimo
Commits
rP19596: Petra: Capture gaia relics
Trac Tickets
#4537
Summary

Petra will capture gaia relics on occasion. Because the AI has access to all of the map and all entities in the game, these gaia capture events have to be spaced out quite a bit to make things fair.

In another diff some fine tuning can be added, maybe involving a check for any relics that happen to be very close to the AI's territory.

Test Plan

On occasion, Petra will capture relics from gaia.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 1759
Build 2783: Vulcan BuildJenkins

Event Timeline

Sandarac created this revision.Apr 14 2017, 3:51 PM
Vulcan added a subscriber: Vulcan.Apr 14 2017, 5:06 PM

Build is green

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

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

mimo edited edge metadata.Apr 15 2017, 9:50 PM

Thanks for the new patch. Here are a few comments.

binaries/data/mods/public/simulation/ai/petra/attackManager.js
374

for performance reason, it may be better to define a new collection (using gameState.updatingGlobalCollection as is done for example for civil center in headquarter.js) which would contain all relics, and use it instead on looping on enemy units.

384

I think you must also modify the functions which find the targets in attackPlan to only choose relics when the target is gaia, and abort the attack otherwise.

485

:)

binaries/data/mods/public/simulation/ai/petra/gameTypeManager.js
529

playedTurn%400 is not ok as it depends on SP (0.2 * 8 * 400 = 640s) or MP (0.5 * 8 * 400 = 1600s). Rather use gameState.ai.elapsedTime, caching the time of the last try.
Although i've not yet tried this "capture the relics", these delays seem too much to me. What about 5 mn for both SP and MP ? and even maybe (5 - 0.5*(aidifficulty-3)) mn?
In addition, we can have a variant of tryCaptureGaiaRelic which would not wait for relics inside the player's territory (as the ai would certainly be able to see it) or those the ai already tried to capture but failed.

Sandarac planned changes to this revision.Apr 16 2017, 3:03 AM

Thanks mimo for the comments, I will look at implementing them.

binaries/data/mods/public/simulation/ai/petra/gameTypeManager.js
529

I don't know if I fully understand what is meant by "variant of tryCaptureGaiaRelic".

Sandarac marked 2 inline comments as done.May 15 2017, 6:44 AM

Ok, I have done some updates.

binaries/data/mods/public/simulation/ai/petra/attackManager.js
384

Instead of adding checks to all those functions in attackPlan.js, I think it would be better to add a check in the update function of the attack manager when it loops through startedAttacks, so that if a gaia relic is not the target when the attack starts (or if the target changes later), the attack will be aborted.

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/1209/ for more details.

mimo added a comment.May 15 2017, 6:39 PM

thanks for the new patch.

binaries/data/mods/public/simulation/ai/petra/attackManager.js
384

I'm not convinced that is the best choice:

  • there are not so many attackPlan functions to modify: it seems to me that it is enough to add in getnearesttarget, in the loop to find the nearest target, something like if (player=0 && gametype = "capture_the_relic" && !ent.hasClass("Relic") continue
  • in the future, we may add other attacks targeting gaia, so your changes in attackManager should anyway be done only if gameType = "capture_the_relic"

btw in defaultTargetFinder, gameState.getEnemyUnits(playerEnemy).filter(API3.Filters.byClass("Relic")) could be replaced by allRelics.filter(relic => relic.owner() === playerEnemy) which should be faster.

binaries/data/mods/public/simulation/ai/petra/gameTypeManager.js
529

Sorry, i forgot to answer that one: my idea was to have two timers to look for relics:

  • the one you've done (with a lapse time of 5 mn) looking for any relics
  • another timer with a shorter lapse time (maybe 1 mn or even less, 30s ?) which would only look for relics we are already supposed to know the position (i.e. those inside the AI's territory or those that the AI already tried to capture but failed and which have not yet been captured by another player)

but this distinction can be in another patch.

Sandarac updated this revision to Diff 1963.May 16 2017, 8:31 AM

Thanks mimo, I've updated the patch.

binaries/data/mods/public/simulation/ai/petra/attackManager.js
384

Okay, I have moved the check.

It's just that I thought from a design perspective it might be better to have that check in attackManager.

I updated defaultTargetFinder.

binaries/data/mods/public/simulation/ai/petra/gameTypeManager.js
529

Ah, okay I see :)

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/1225/ for more details.

mimo added a comment.May 16 2017, 7:30 PM

Thanks for the changes, i'm going to commit the patch now. if you agree with the inline comment, i'll change it later.

binaries/data/mods/public/simulation/ai/petra/gameTypeManager.js
532

I've the impression this line would be better in attackManager with line 385 where we put tryCaptureGaiaRelic to false as it can take some time to start the attack. What's your opinion?

mimo accepted this revision.May 16 2017, 7:32 PM
This revision is now accepted and ready to land.May 16 2017, 7:32 PM
This revision was automatically updated to reflect the committed changes.
mimo added a comment.May 16 2017, 7:37 PM

Sorry, i forgot to add you on the commit message :(

In D333#20534, @mimo wrote:

Sorry, i forgot to add you on the commit message :(

That's okay :)

binaries/data/mods/public/simulation/ai/petra/gameTypeManager.js
532

Yes, I agree, that sounds like a good idea.

elexis updated the Trac tickets for this revision.May 17 2017, 11:59 AM