Page MenuHomeWildfire Games

Dynamic Special Templates
ClosedPublic

Authored by elexis on Mar 12 2017, 1:28 AM.

Details

Summary

In April 2016, leper wrote a patch in #2951 to replace hardcoded template modifications in the TemplateManager with XML files.
This has the advantage that developers that are not familiar with the TemplateManager C++ code or not eager to change that code can
write a new special template, even without compiling anything.

These special templates allow changing a common (unit or structure) template by the values that are defined in the special template.
For example:

  • "actor|" + template means that this simulation template will have all its components stripped (thus becomes a ghost).
  • "foundation|" + template means that the TemplateLoader will construct a template that will appear as a foundation, i.e. 1 health, doesn't block obstructions and no vision range.
  • "construction|" + template yields a construction site, i.e. removes almost all properties except the position and model (which rises when building).

This feature is very important for adding new gameplay mechanics and every month we stumble upon some feature or issue that could have been solved with one these special templates.
For example the gamesetup option to disable garrisoning of heroes in Regicide gamemode is intending to use this: D104.

Another example might be disabling of the Capture component when we don't want a gaia settlement to be taken over by player territory: D204.
Or marking a unit as invincible, like the treasure seaker woman on survival.

While these things could all be changed from trigger scripts accessing the simulation components, the Petra bot doesn't know about simulation component changes, unless
those deltas are sent through the GUIInterface (which is annoying to keep in sync each feature changed and also bad for performance).
A disadvantage of using templates and not using the simulation component is that we can't change the property dynamically in running games.

Regardless, that C++ code needs to vanish and we should have been able to use this feature months ago already.

This patch encompasses lepers two diffs uploaded in #2951 and a bugfix and rebases of mine.
Excluded are the contested string/newline changes in the test files and the change to not test the simulation if the public mod isn't available by me in August 2016 at |#2951#comment:13.

In order to not add the duplicate files required for the test (that have to be updated each time one changes the component schema), Itms has planned something and I'm sure he will be happy if he doesn't have to rebase, review and include this diff in his proposal.

The diff proposed here can be reviewed as individual commits in this github branch:
https://github.com/elexis1/0ad/tree/2951_dynamic_templates_2017

The old branch with the contested, now dropped commits is still available at:
https://github.com/elexis1/0ad/tree/2951-dynamic-templates

Test Plan
  1. Practical Survival Test:

With rP19288, apply this survival diff (D145?id=573) https://code.wildfiregames.com/file/data/3y3dep3haewhz5wgvf7u/PHID-FILE-rhjkdjfsqj2uw2b2dslk/D145.id573.diff
and replay bb's testgame at https://code.wildfiregames.com/file/data/yqhiguuji2xhn45gfxfj/PHID-FILE-wkjymol3c7626evkmjye/commands.txt
It isn't hashed, but we can observe the identical game with and without the patch (in particular ending with 4 elephants destroying his fortress).

  1. Practical Ptol Lighthouse Test:

This was a bug noticed in the original proposal.

  1. start unpatched recent svn, either disable persist match settings :D or select a skirmish map (before selecting a random map)
  2. enter a multiplayer gamesetup (as only those are hashed), select a naval map, allow cheats and pick ptol
  3. use the "gift from the gods" cheat and build a lighthouse
  4. delete the lighthouse and exit the game
  5. replay that game with the patched version

    Do the same with the lighthouse change reverted and observe that the shoreline is revealed when placing the foundation already and that the replay in which it occurs triggers an OOS.
  1. Compare the changed templates line by line with the removed C++ code because it is easy to confuse the different operations.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 742
Build 1178: Vulcan BuildJenkins
Build 1177: arc lint + arc unit

Event Timeline

elexis created this revision.Mar 12 2017, 1:28 AM
elexis updated the Trac tickets for this revision.Mar 12 2017, 1:33 AM
elexis updated this revision to Diff 781.Mar 12 2017, 1:42 AM

Gah, that's exactly what I was talking about.

Thank you elexis for all of your work on continuing the development of this feature! It will be great to have this.

Vulcan added a subscriber: Vulcan.Mar 12 2017, 3:12 AM

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

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

elexis edited reviewers, added: Sandarac; removed: leper.Mar 16 2017, 1:41 AM

Considering the absence of a response in here and the statement in D104

consider this patch either the responsibility of someone who decided or proclaimed to be taking over, or the proposal retracted

Otherwise feel free to change the reviewers or make use of the commandeer feature.

Sandarac resigned from this revision.Mar 16 2017, 1:49 AM

At this point I am not qualified to review this.

elexis added a reviewer: Itms.Mar 16 2017, 3:39 AM

Capture the Relic mode D152 also should use dynamic tempaltes to set invulnerability afaik.

Going to upload a replay from an svn game from today, included 6 players sending commands, hash values, actors, foundations, construction sites, , sheep resources, mirages and ptolemian lighthouses (no previews I guess).
If there is any bug in the XML files, there must be a hash mismatch (which is how I found that lighthouse bug originally as well).

elexis added a subscriber: borg-.Mar 16 2017, 3:57 AM

Thanks @borg- for being being by far the best at this game, testing svn and uploading the replay with which we can verify this patch!

This revision was automatically updated to reflect the committed changes.
In D215#8301, @elexis wrote:

Considering the absence of a response in here and the statement in D104

consider this patch either the responsibility of someone who decided or proclaimed to be taking over, or the proposal retracted

Otherwise feel free to change the reviewers or make use of the commandeer feature.

I'm not sure why you'd even consider me eligible to review my own patch. That said thanks for finally taking care of this.

Just a little thing, in the demo unit maps (perhaps take care of placeable stuff?)

special_filter/corpse...
ERROR: RelaxNGValidator: Validation error: special_filter/corpse:1: Expecting an element RetainInFog, got nothing
ERROR: RelaxNGValidator: Validation error: special_filter/corpse:1: Invalid sequence in interleave
ERROR: RelaxNGValidator: Validation error: special_filter/corpse:1: Element Visibility failed to validate content
ERROR: RelaxNGValidator: Validation failed for '(null)'
ERROR: Failed to validate entity template 'special_filter/corpse'
Failed to load special_filter/corpse
special_filter/foundation...
ERROR: RelaxNGValidator: Validation error: special_filter/foundation:1: Expecting an element Max, got nothing
ERROR: RelaxNGValidator: Validation error: special_filter/foundation:1: Invalid sequence in interleave
ERROR: RelaxNGValidator: Validation error: special_filter/foundation:1: Element Health failed to validate content
ERROR: RelaxNGValidator: Validation error: special_filter/foundation:1: Expecting an element Actor, got nothing
ERROR: RelaxNGValidator: Validation error: special_filter/foundation:1: Invalid sequence in interleave
ERROR: RelaxNGValidator: Validation error: special_filter/foundation:1: Element VisualActor failed to validate content
ERROR: RelaxNGValidator: Validation failed for '(null)'
ERROR: Failed to validate entity template 'special_filter/foundation'
Failed to load special_filter/foundation
special_filter/mirage...
ERROR: RelaxNGValidator: Validation error: special_filter/mirage:1: Expecting an element Civ, got nothing
ERROR: RelaxNGValidator: Validation error: special_filter/mirage:1: Invalid sequence in interleave
ERROR: RelaxNGValidator: Validation error: special_filter/mirage:1: Element Identity failed to validate content
ERROR: RelaxNGValidator: Validation failed for '(null)'
ERROR: Failed to validate entity template 'special_filter/mirage'
Failed to load special_filter/mirage
special_filter/preview...
ERROR: RelaxNGValidator: Validation error: special_filter/preview:1: Expecting an element RetainInFog, got nothing
ERROR: RelaxNGValidator: Validation error: special_filter/preview:1: Invalid sequence in interleave
ERROR: RelaxNGValidator: Validation error: special_filter/preview:1: Element Visibility failed to validate content
ERROR: RelaxNGValidator: Validation failed for '(null)'
ERROR: Failed to validate entity template 'special_filter/preview'
Failed to load special_filter/preview

There is also that minor stuff (I didn't know how to post so I write it here), when placing special/territory_pull in the atlas.

ERROR: RelaxNGValidator: Validation error: preview%7Cspecial/territory_pull:1: Expecting an element RetainInFog, got nothing
ERROR: RelaxNGValidator: Validation error: preview%7Cspecial/territory_pull:1: Invalid sequence in interleave
ERROR: RelaxNGValidator: Validation error: preview%7Cspecial/territory_pull:1: Element Visibility failed to validate content
ERROR: RelaxNGValidator: Validation failed for '(null)'
ERROR: Failed to validate entity template 'preview|special/territory_pull'

elexis added a comment.EditedMar 31 2017, 1:01 PM

Can't reproduce, territory_pull doesn't appear in my atlas list. (Also the commit adding that file would be more related)

Update: Apparently that special entity appears in the "Entities" list but not in the "Actors (all)" list. Always thought the latter contains the former, hm.

There is also that minor stuff (I didn't know how to post so I write it here), when placing special/territory_pull in the atlas.

ERROR: RelaxNGValidator: Validation error: preview%7Cspecial/territory_pull:1: Expecting an element RetainInFog, got nothing
ERROR: RelaxNGValidator: Validation error: preview%7Cspecial/territory_pull:1: Invalid sequence in interleave
ERROR: RelaxNGValidator: Validation error: preview%7Cspecial/territory_pull:1: Element Visibility failed to validate content
ERROR: RelaxNGValidator: Validation failed for '(null)'
ERROR: Failed to validate entity template 'preview|special/territory_pull'

territory_pull is a special template that is only used for a demo map (and was unrelatedly mentioned the last time in http://trac.wildfiregames.com/ticket/3204#comment:7)
It seems we shouldn't nuke it because map makers might want to use it if they were aware of it's existence:

(so apparently not very wanted for the map makers).

rP15925 caused this error by adding the Visbility component but not specifying all mandatory properies.

@leper this sounds like preview.xml and corpse.xml should specify those missing properties, right? Afaics overwriting the existing ones also doesn't matter in this case as we know which properties previews and corpses ought to have.

In D215#10943, @elexis wrote:

rP15925 caused this error by adding the Visbility component but not specifying all mandatory properies.

Must have been a later commit, that one didn't need any properties for Visibility. I guess some later commit didn't add them to the template manager.

@leper this sounds like preview.xml and corpse.xml should specify those missing properties, right? Afaics overwriting the existing ones also doesn't matter in this case as we know which properties previews and corpses ought to have.

They either should, or the way they are included into the other templates should be changed. In this case those just seem to have been forgotten during some earlier update.