TerritoryManager does not update the blinking territories when recomputed: this is done only by the entities TerritoryDecay components in the next turn. And that is problem for the AI as the TerritoryMap is cloned just after being recomputed, so that the AI is never aware of blinking cells, but could also have other consequences in the simulation as the blinking cells are forbidden in the BuildRestrictions component.
This is fixed in the patch which adds a scripted component to TerritoryDecay to allow for an update in the TerritoryManager component.
Details
Details
- Reviewers
• leper Itms - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) - Commits
- rP20268: Add a TerritoryDecayManager component to update territoryMap with the blinking…
Check for example that without the patch the territoryMap sent to the AI in CCmpAIManager never have the blinking bit set for any tile, while it is correct with the patch.
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! Checking XML files...
http://jenkins-master:8080/job/phabricator/2032/ for more details.
Comment Actions
Executing section Default... Executing section Source... Executing section JS... binaries/data/mods/public/simulation/components/TerritoryDecay.js | 31| » if·(tileOwner·==·0) | | [NORMAL] JSHintBear: | | Use '===' to compare with '0'. binaries/data/mods/public/simulation/components/TerritoryDecay.js | 34| » » return·cmpPlayer.GetPlayerID()·==·0;·//·Gaia·building·on·gaia·ground·->·don't·decay | | [NORMAL] JSHintBear: | | Use '===' to compare with '0'. Executing section XML GUI... Executing section Python... Executing section Perl...
http://jenkins-master:8080/job/phabricator_lint/524/ for more details.
Comment Actions
The loop could be somewhat expensive, though probably not that bad. And if it turns out to be we could still make some sort of DecayManager that gets the list of entities passed to JS and then calls the individual ones there. (Would avoid the C++-JS call overhead, but I assume that that is either not that expensive or there are worse offenders.)
| source/simulation2/components/CCmpTerritoryManager.cpp | ||
|---|---|---|
| 88 ↗ | (On Diff #3671) | Nice catch. |
| 548 ↗ | (On Diff #3671) | Could just static_cast it to ICmpTerritoryDecay* as a few others do, and then inline the call. Since at least a few other callers of GetEntitiesWithInterface{,Unordered} do assume that the pointer is valid (which I do hope we ensure in the ComponentManager), and you also seem to assume since there is no if (cmpTerritoryDecay) below. So the whole body of the loop could be static_cast<ICmpTerritoryDecay*>(pair.second)->SetBlinkingState(); |
Comment Actions
In fact, to ease future changes, I've added a TerritoryDecayManager instead of extending the TerritoryDecay component.
I also add to change a bit the way the grid is updated in CmpAIManager as it should also be reloaded when blinking changed without territory changes (as when diplomacy changes).
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! Checking XML files...
http://jenkins-master:8080/job/phabricator/2044/ for more details.
Comment Actions
Executing section Default... Executing section Source... Executing section JS... Executing section XML GUI... Executing section Python... Executing section Perl...
http://jenkins-master:8080/job/phabricator_lint/531/ for more details.
Comment Actions
Also sounds like it would fix #4787 (according to borg- one can always build a tower near a captured barracks)
Comment Actions
As this patch is needed by the AI, i'm going to commit it. Feel free to raise a concern is needed.
In a following patch, i'll cache the list of TerritoryDecay components in the new TerritoryDecayManager to avoid having to retrieve it each time the blinking states are updated.