Page MenuHomeWildfire Games

Allow {native} and {civ} in auras
Changes PlannedPublic

Authored by Angen on Aug 3 2018, 2:24 PM.

Details

Reviewers
s0600204
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Trac Tickets
#4588
Summary

This patch allows {civ} nad {nativ} replacement in Auras and moves _teambonuses to generic player template using {nativ}.

Test Plan

Check that auras works fine.

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 BuildJenkins
Build 11498: arc lint + arc unit

Event Timeline

Angen created this revision.Aug 3 2018, 2:24 PM
Vulcan added a subscriber: Vulcan.Aug 3 2018, 2:28 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/703/display/redirect

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?

Stan added a reviewer: Restricted Owners Package.Mar 9 2019, 10:49 AM
s0600204 requested changes to this revision.Mar 18 2019, 10:59 PM
s0600204 added a subscriber: s0600204.
In D1603#64225, @elexis wrote:
  • 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?

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

This revision now requires changes to proceed.Mar 18 2019, 10:59 PM
Angen updated this revision to Diff 7604.Mar 20 2019, 9:58 PM

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.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1127/display/redirect

Angen updated this revision to Diff 7605.Mar 20 2019, 10:02 PM

sele ptol civ tag revert

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1128/display/redirect

Angen planned changes to this revision.Mar 20 2019, 10:27 PM
Stan added a subscriber: Stan.Mar 21 2019, 8:40 AM

Patch needs to be rebased :)
Also careful you removed the eol style.

binaries/data/mods/public/simulation/components/Auras.js
13

Useless braces.

Angen added a comment.Apr 3 2019, 10:42 AM

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.