Page MenuHomeWildfire Games

territory pull again
ClosedPublic

Authored by Nescio on Jul 15 2019, 8:31 PM.

Details

Summary

See lengthy discussion at https://code.wildfiregames.com/rP22266

This patch does the following:

  • move territory_pull.xml out of special/ to make it placeable
  • insert <Visibility> node to solve Atlas errors
  • convert it into a generic template (template_territory_pull.xml) with a related territory_pulls/ subdirectory for its children
  • created six territory pull children (each with a Weight : Radius ratio of 1000 : 1)
    • only one is used, in maps/scenarios/Campaign Test Map.xml, but the others might become useful when people start making campaigns in the future
Test Plan

Open Atlas and observe all pull entities can be placed without errors.
Then load scenarios/Campaign Test Map.xml.

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

Nescio created this revision.Jul 15 2019, 8:31 PM
Owners added a subscriber: Restricted Owners Package.Jul 15 2019, 8:31 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/90/display/redirect

Nescio updated this revision to Diff 8918.Jul 15 2019, 8:33 PM
Nescio edited the test plan for this revision. (Show Details)

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/91/display/redirect

It seems Jenkins is failing the build of the map file again, presumably because of spaces in file name (see also D1042).

I tested the existing and newly proposed entity a bit.

They change ownership depending on territory owner / territory decay.
So the owner determined in the map is only the initial owner that is quickly corrected by the terrain.

So for the vision entities, it can scout an area initially. But if someone built a building tehre that provides territory, he will capture this vision entity thing, so it will provide that vision.
It seems a bit weird, the player can't explain where that effect is coming from, unless there is some story told somewhere.

Same with the territory entity. It will buff or nerf the territory influence in that area to whomever currently owns that territory.

Perhaps some scenario may use it for special entities, otherwise their use seems hypothetical / unlikely?

i.e. deleting the folder might also be an option, but if there is a decribed use case, the folder structure seems right.

The name pulls is a bit weird. It's not a triggerpoint, it's not an aura, nor an aura source, it's not a statuseffect.

So I looked a bit at other sim components around to find the common denominator.
I have trouble finding further examples other than an aura source?
Invisible obstructions? Those won't be pulls nor points, I guess those would be moved to a different folder.
(I notice Settlement.js which seems a bit useless)
modifiers/ or so, eh? Pulling is the common denominator (vision pull, territory pull?)?
(I don't mind much anymore, I just want it to stop.)

Thank you for your comment.
Yes, these pulls become permanent when connected with a territory root; I suppose their usefulness is limited to scenarios where players can't build new structures (e.g. maps/scenarios/Campaign Test Map.xml).

Initially I created

template_territory_pull.xml
territory_pulls/30.xml

as suggested in rP22266, but then I realized the following:

  • we might as well have several radii and weights
  • parent template actually doesn't need a <TerritoryInfluence> node and without it its name would be misleading
  • parent template could easily be reused for other things, e.g. vision (also Age of Empires had map revealers)
  • 30.xml file name is very short and not immediately obvious

Therefore I decided to opt for the more generic approach as proposed.

As for the name, pull is territory_pull minus territory; I couldn't think of something better. modifiers/ is problematic because it's too generic and in my opinion more suitable for auras and technologies (which aren't templates). I considered points/ instead of pulls/, but rejected it, because that would imply overlap with trigger points, which are and should remain separate from territory pull, as pointed out in rP22266. Hence pulls/ seemed acceptable enough.

And yeah, this territory pull problem has been dragging on far too long and it should be solved, but in a way such that people won't revisit it all over again in a year or two. I'm hoping this patch satisfies that; please be critical.

also Age of Empires had map revealers

But did they also change ownership with territory?
There is the 3v3 skirmish map with the huge snowy hills, that has outposts in the enemy base, it sounds a bit like that. But that adds initial scouting and then disappears if the outpost disappears.
To me it seems like the use case is either to scout once (i.e. entity auto-nuked after existing briefly), or to keep the owner persistent (revealing part of the enemy base throughout the match)?

Perhaps the vision ones should not have capturable, but the territory ones? Perhaps template_pull.xml should be two files and use territory_pulls/ and vision_pulls/ to aoid ambiguity?

Nescio updated this revision to Diff 8954.Jul 17 2019, 4:00 PM
Nescio edited the summary of this revision. (Show Details)

Now without vision files (adding those is perhaps not a good idea).

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/115/display/redirect

Now without vision files (adding those is perhaps not a good idea).

vision files

If they keep ownership, maybe, but it also sounds like they'd be better off with a specific parent template rather than a bogus parent (pulls)?
Removing duplication leads one to consider a parent, but sometimes it seems to create frankenstein templates without an identity, leading people to inherit weirdly too, no?

It's not that the we need those templates, it's that we need to know the correct folder structure for people messing with "other" "special" and other miscellaneous templates and might want to remove bad examples for good examples even if used only in one demo map.

template_pull.xml -> template_territory_pull.xml and pulls/territory_60.xml -> territory_pulls/territory_pull_60.xml?

template_pull.xml -> template_territory_pull.xml and pulls/territory_60.xml -> territory_pulls/territory_pull_60.xml?

The parent template doesn't actually have a territory component, because that's specified in its children, so wouldn't it be a bit weird to name it template_territory_pull.xml? Otherwise I have no objections.

In D2083#87655, @Nescio wrote:

template_pull.xml -> template_territory_pull.xml and pulls/territory_60.xml -> territory_pulls/territory_pull_60.xml?

The parent template doesn't actually have a territory component, because that's specified in its children, so wouldn't it be a bit weird to name it template_territory_pull.xml? Otherwise I have no objections.

That's incidental.
I'd rather have a filename that makes its identity clear than a file that doesn't have an identity.
At least until someone proposes new templates with a common parent. (The vision templates weren't so bad except for the ownership change, but it all seems to be in the realm of hypotheticals currently while there is enough work available on actual issues).

Nescio updated this revision to Diff 9017.Jul 20 2019, 6:54 PM
Nescio edited the summary of this revision. (Show Details)

template_territory_pull.xml

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/150/display/redirect

The errors fixed by <Visibility> insertion:

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

radius_60.xml leaves the reader clueless, whereas territory_pull_60.xml, athen_range.xml, trigger_point_A.xml does inform one without looking at the path. Also when doing a search for specific filenames, less ambiguous filenames are benefitial, so not necessarily convinced to remove the parts of filenames that are redundant with parts of the path in general (I'm open to arguments).

rP16022 broke the pull templates? And the wall templates fixed by rP22070 and the trigger templates fixed by rP17270? But then why does Josh say in #3613 that it was working in Alpha 18 (released march 2015 after rP16022, see Changelogs)?

rP16022 broke the pull templates? And the wall templates fixed by rP22070 and the trigger templates fixed by rP17270? But then why does Josh say in #3613 that it was working in Alpha 18 (released march 2015 after rP16022, see Changelogs)?

No idea when exactly it was broken, but try placing other/territory_pull.xml in A23 in Atlas and you get these errors:

ERROR: RelaxNGValidator: Validation error: preview%7Cother/territory_pull:1: Expecting an element RetainInFog, got nothing

ERROR: RelaxNGValidator: Validation error: preview%7Cother/territory_pull:1: Invalid sequence in interleave

ERROR: RelaxNGValidator: Validation error: preview%7Cother/territory_pull:1: Element Visibility failed to validate content

ERROR: RelaxNGValidator: Validation failed for '(null)'

ERROR: Failed to validate entity template 'preview|other/territory_pull'

which can be solved by inserting that <Visibility> node.

Nescio updated this revision to Diff 9044.Sun, Jul 21, 9:32 PM

Per @elexis' earlier remarks.

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/178/display/redirect

elexis accepted this revision.Mon, Aug 12, 5:08 PM

Was just wondering what commit to mention in the commit message that this is a fix to. It seems it was rP16022 which also has some leftover concern apparently.

I will commit this now, hopefully this curse wears off :S

Unless there is something occult overseen, those entities are a nice addition actually!

Thanks for the patch

This revision is now accepted and ready to land.Mon, Aug 12, 5:08 PM
This revision was landed with ongoing or failed builds.Mon, Aug 12, 5:16 PM
This revision was automatically updated to reflect the committed changes.