This patch allows {civ} nad {nativ} replacement in Auras and moves _teambonuses to generic player template using {nativ}.
Details
Details
Diff Detail
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 7033 Build 11499: Vulcan Build Jenkins Build 11498: arc lint + arc unit
Event Timeline
Comment Actions
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/differential/703/display/redirect
Comment Actions
Due to the work on the related code, @fatherbushido and @mimo might have had an interest.
- Care, there might be a function that loops over all entities with Identity component and might now unintentionally process player components.
- Identity for player entities is new. Might be considered controversial or a good idea. I don't know but it should be considered. The definition in Identity.js would be violated or had to change:
"<a:help>Specifies various names and values associated with the unit type, typically for GUI display to users.</a:help>" +
Is this the best solution for player entities or would it make more sense to add the Civ property to the Player component?
- The use cases of having civ and native in Auras was not mentioned in the summary or ticket. It is for capturable entities?
So the refactoring of team auras is not the reason the code was added, because players aren't supposed to transfer player entities?
- Does it burden mod development in any way (for example adding a requirement for added civs to implement some auras, launching two mods with two new civs impossible?), if so it could that be mitigated?
| binaries/data/mods/public/simulation/components/Auras.js | ||
|---|---|---|
| 70 | (Last thing I remember about the RegEx replacement is that we wanted to unify the copies, is it the third?) | |
| binaries/data/mods/public/simulation/data/auras/teambonuses/kush_player_team_bonus.json | ||
| 14 | Unchanged file, rename can be committed separately | |
| binaries/data/mods/public/simulation/templates/special/player/player.xml | ||
| 60 | Is gaia the reasonable default for all new player entity templates? | |
Comment Actions
A point that needs addressing. @Angen, to express it differently: what should happen if a civ is added that does not have a teambonus aura? And what does happen? (Hint: it's not pretty.)
While you're at it, note that there's no reason to set the civ code inside player-templates at all. During match setup, the civ code is set from the contents of the relevant {civ}.json file. Thus, as you're removing the civ-specific teambonus aura tokens, and you don't need to define a civ code for Identity, then that leaves you with a number of otherwise empty templates. So you can remove them. 0ad doesn't actually need a civ-specific player-template to instance a civ - it will happily fallback on the special/player/player.xml file.
| binaries/data/mods/public/simulation/components/Auras.js | ||
|---|---|---|
| 70 | Using ack --type=js "replace.*(civ|native).*}" on the contents of the public mod returns ten files (across maps, gui, AI, and simulation) that use RegEx to replace "{civ}", "{native}", or both. Amongst simulation components, the current count is five. In each instance, the presence of "{native}" and "{civ}" within the string is not checked before the replace() is attempted. Whilst unifying is best left for another revision, the presence checks could be removed in this one. | |
| binaries/data/mods/public/simulation/data/auras/teambonuses/kush_player_team_bonus.json | ||
| 14 | Done as of rP22133 | |
Comment Actions
Identity for player entity have been in game before this patch. See special/player/player.xml
To get rid of <Auras datatype="tokens">teambonuses/spart_player_teambonus</Auras> from player files ( and now actually some of them can be removed too)
I am yet not sure why but by removing teambonuses files nothing happened.
Comment Actions
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/differential/1127/display/redirect
Comment Actions
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/differential/1128/display/redirect
Comment Actions
Patch needs to be rebased :)
Also careful you removed the eol style.
| binaries/data/mods/public/simulation/components/Auras.js | ||
|---|---|---|
| 13 | Useless braces. | |
Comment Actions
Looking closer this does not work.
Player entity does not have ownership (Thinking about it, it would be odd if had).
So I see two options:
adding civ code to files, but it is only exchange for aura line (could be worth if player would have more than one aura file, but does not look like it will have)
abandon this as the goal was to remove aura entries from specific player files
There could be use case for adding native civ replacement (without player.xml changes). Someone may create specific aura e.g. for civic centre which would be different for every civilisation and with this replacement one has to only add the aura line just to one template.