Page MenuHomeWildfire Games

Template loading from the AI
ClosedPublic

Authored by Sandarac on Jun 14 2017, 1:40 PM.

Details

Reviewers
mimo
Commits
rP20162: Template loading from the AI
Trac Tickets
#4611
Summary

For A23.

The issue is well-described in the trac ticket. Adding a Engine.GetTemplate(name) function to be used by the AI allows to remove the current hardcodings for dynamic templates (at the moment, a new hardcoding has to be added in shared.js for every new dynamic template added, an issue that has blocked the progress of diffs like D230).

CAIWorker now uses a TemplateLoader member var in a similar way to the MapGenerator and GUIManager. In another diff, the TemplateLoader can then be used to load templates on-demand from the AI as needed, instead of loading every single template in the game at the start (more than 1100), which will give a small performance boost. (This will be done by removing m_EntityTemplates from CAIWorker, and removing the methods StartLoadEntityTemplates, ContinueLoadEntityTemplates, ForceLoadEntityTemplates from CCmpAIManager.)

However, this diff also needs to deal with a design quirk in the AI's template handling in entity.js. It is possible to have components that simply act as "flags", i.e. the Wonder and Garrisonable components, which are non-system components with no schema. So they exist simply as f.e. <Wonder/>, <Garrisonable/> in templates. These are handled fine in the simulation, of course, as just Engine.QueryInterface(ent, IID_Garrisonable); can be done to check for the presence of the garrison component for an ent, but there is no equivalent check for the AI. this.get("Garrisonable") will always return undefined for such components, even if the Garrisonable property is present in the template. So the diff deals with this with a check for the property in the raw template (this will need to be improved to take into account entity and template modifications). The AI has been able to avoid this problem with wonder as it never checks for the wonder component, but instead checks for the Wonder class.

There was also another quirk/inconsistency found after I did some testing. The AI treats foundations as having territory influence, although they don't have it. So there either must be a check if the parent entity has territory influence, or the check can be removed, as Petra will only assign civic centers as bases anyways.

Test Plan

Read the code, compile, notice that the diff now enables the full potential of dynamic templates. Check that the canGarrison function still works correctly.

This diff applies the dynamic template from D230, and gives heroes the template in regicide games, and can be used to test.

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

Sandarac created this revision.Jun 14 2017, 1:40 PM
elexis added a subscriber: elexis.
elexis added inline comments.
binaries/data/mods/public/simulation/ai/common-api/entity.js
772 ↗(On Diff #2558)

That check is true if the value is "true" or "false" whereas before it was false if the value is "false". Is that intended?

Vulcan added a subscriber: Vulcan.Jun 14 2017, 2:28 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!
Checking XML files...

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

Itms awarded a token.Jun 14 2017, 2:37 PM
mimo awarded a token.Jun 14 2017, 7:28 PM
mimo added inline comments.Jun 14 2017, 7:38 PM
binaries/data/mods/public/simulation/ai/common-api/shared.js
68 ↗(On Diff #2558)

can be a following cleaning patch, but i've the impression we don't need anymore this _derivedTemplates, we could simply add it to _templates.

binaries/data/mods/public/simulation/ai/petra/baseManager.js
77 ↗(On Diff #2558)

another possibility would be to test on the CivilCentre class? But i agree that's not really needed as only such anchor can be build inside petra.

mimo requested changes to this revision.Aug 7 2017, 8:40 PM

Doing more tests, there is a problem with the populationBonus of foundations when planning new houses: from the foundation template, there is no way to know how much popBonus a foundation will provide and we have to go back to the built template. I'll upload a patch fixing it for the time being, but it may be cleaner in the future to add such info in the template.
Otherwise, the changes in CCmpAIManager.cpp are fine, and can be considered reviewed. I'll commit them already. That will make additionnal checks quicker as we won't need to recompile.

This revision now requires changes to proceed.Aug 7 2017, 8:40 PM
mimo added a comment.EditedAug 7 2017, 8:43 PM

Patch fixing the populationBonus problem

and c++ part commited in rP19951

mimo added a comment.Sep 9 2017, 4:39 PM

Fix for populationBonus commited in rP20143

mimo added a comment.Sep 10 2017, 2:17 PM

rP20155 added another fix needed for this patch

mimo accepted this revision.Sep 11 2017, 6:20 PM

I'm going to commit the remaining part of this patch (with some additional change in common-api/entity.js in the CanCapture function)

This revision is now accepted and ready to land.Sep 11 2017, 6:20 PM
This revision was automatically updated to reflect the committed changes.