Page MenuHomeWildfire Games

Petra: assign guards to wonders in wonder games
ClosedPublic

Authored by Sandarac on Feb 12 2017, 8:08 AM.

Details

Summary

Petra will assign guards to wonders that it builds in the wonder victory condition. This is handled largely in the same way as heroes in regicide. Around ten units will be selected to guard, and the number is based on the defensive personality trait. Initially I planned to add guards to the wonder's foundation as well, but currently foundations can't be guarded.

This diff moves the for (let evt of events.Destroy) loop above the early return in gameTypeManager, as these events now need to be checked in wonder games, as well as regicide.

Eventually, it would be nice for Champion units to be preferentially selected as guards, possibly switching with any citizen soldiers that are already guarding.

Test Plan

In wonder games, Petra will assign available units as guards to wonders that it builds (once construction has finished). If the wonder is destroyed, the guarding units will be assigned to some other activity.

Diff Detail

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

Event Timeline

Sandarac created this revision.Feb 12 2017, 8:08 AM
Sandarac updated this revision to Diff 514.Feb 12 2017, 8:47 AM

Move the UnGarrison events loop so that it is checked in wonder games as well, and check for the criticalEntGuard in that loop.

Vulcan added a subscriber: Vulcan.Feb 12 2017, 8:59 AM

Build is green

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

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

Build is green

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

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

mimo added a subscriber: mimo.EditedFeb 12 2017, 1:10 PM

As you proposed to add healers and guards to heroes in the future, you should already foresee a structure of this.criticalEnts which allow it without changing too much code in future patches: i don't know which one would be best, either to have two Sets (one for guards and one for healers)? or only one Set but adding a counter to know how much guards and healers you have? or something else?
Edit: maybe replacing the Set by a Map which would contain {entId, isGuard, isHealer}?

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

The structure of the function is a bit strange: you now have
if (wonder) do wonderspecific
do commonpart
if (!regicide) return
do regicidespecific
specially as we may add more gametypes in the future, it would be better to rearrange it a bit to have it more symetric as for example
if (wonder) do wonderspecific
if (regicide) do regicidespecific
do commonpart

146

It seems the following code is not hero specific? so the comment should be fixed or prevent it to be run for wonder (the second option would seem better to me as you may have guards coming from another island in a naval map).

150

if run for wonder, what's the meaning of healersPerCriticalEnt. You should treat differently the two cases.

257

As proposed in the following, better add the loop on criticalEntities here. But in addition, assignGuardToCritical does not add it to the one you are looking at, but to the one with minimal guards. So you should not loop on the criticalEntities at the first place, or add a 3rd argument to assignGuardToCritical which when not undefined would indicate which critical entity you want to be guarded.

269

Why this value of -10? you could reuse the values -2 and -3 (otherwise you'd have to add the test on -10 everywhere in the code when we want to select a unit and check on -2 and -3).

402

Do you need this loop here? it could be done directly in manageWonderGuards. That would then avoid the second argument of manageWonderGuards

Sandarac updated this revision to Diff 519.Feb 13 2017, 8:57 AM

Update.

Sandarac marked 6 inline comments as done.Feb 13 2017, 9:00 AM
Sandarac added inline comments.
binaries/data/mods/public/simulation/ai/petra/gameTypeManager.js
146

I assumed that the loop wouldn't be run in wonder, as wonders would not ungarrison off of ships (at least not currently). I added an early return if wonder mode.

Build is green

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

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

mimo added inline comments.Feb 13 2017, 6:02 PM
binaries/data/mods/public/simulation/ai/petra/gameTypeManager.js
221

the first part (getGameType === wonder) should be removed? as currently its criticalEnts are wonder which can't ungarrison, and if we ever have a mixed mode with wonder+regicide, it shouldn't be here.

228

size should be replaced by a function which count only the healers (even if currently, you only have healers for heroes). Or do you intend to fix that in your next patch adding guards to heroes?

235

could use now the variant with 3 args, as we know to which criticalEnt we want to add them

260

The function could be made more general (manageCriticalEntGuards) with a max guard depending on the Hero or Wonder (to prepare your next patch), or the loop should be done only on wonder (i.e. add a continue if hero) as we could have both wonder and regicide enabled.

265

I wonder if we shouldn't do this loop in two steps: first one keeping only potential guards with the same access, and a second one keeping all if needed (to avoid useless transports)

280

same comment as before for a two steps loop

Sandarac updated this revision to Diff 524.Feb 14 2017, 8:22 AM
Sandarac marked an inline comment as done.
Sandarac marked 5 inline comments as done.Feb 14 2017, 8:28 AM

Hello mimo,

I added a call to m.returnResources() in assignGuardToCriticalEnt, which is useful for citizen soldiers.

Also, there was an error in my previous two diffs, which I noticed after testing with more than one AI (which I fixed). In the for (let evt of events.ConstructionFinished) loop, there was no check if the newly-constructed wonder actually belonged to the AI.

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

I think this is for another patch.

272

I feel using a loop like this works well, but maybe there's a better way to first check for the same access.

Build is green

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

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

This revision was automatically updated to reflect the committed changes.