Details
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 8431 Build 13774: Vulcan Build Jenkins Build 13773: arc lint + arc unit
Event Timeline
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/differential/1799/display/redirect
binaries/data/mods/public/simulation/templates/special/formations/anti_cavalry.xml | ||
---|---|---|
0–1 | Oops, my mistake, https://trac.wildfiregames.com/wiki/EnglishStyleGuide suggests classes ought to be capitalized; wasn't done previously for formations. | |
binaries/data/mods/public/simulation/templates/special/formations/box.xml | ||
6 | Soldiers and siege but not ships. Any term that covers that? |
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/differential/1837/display/redirect
It's a bit unfortunate that tooltips speaking of classes are in formation templates, whereas it's actually unit templates that decide what formations they can have.
Is it a good idea to change that? Not sure.
Yes, it's unfortunate. And although the classes which could join a certain formation are defined in the templates, the formations that are actually available are defined in the {civ}.json files.
binaries/data/mods/public/simulation/templates/special/formations/skirmish.xml | ||
---|---|---|
5 | Requires only Ranged Soldiers for me can mean "Requires just a ranged soldier but may have more classes", Only ranged units allowed. (or Only Ranged Soldiers allowed.) might have a different style but does say what is meant. |
binaries/data/mods/public/simulation/templates/special/formations/skirmish.xml | ||
---|---|---|
5 | Actually the formation is also enabled when there are non-Ranged units in the selection so probably Requires a Ranged Soldier is more appropriate |
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/119/display/redirect
Reads correct and complete now
It's a bit unfortunate that tooltips speaking of classes are in formation templates, whereas it's actually unit templates that decide what formations they can have.
Is it a good idea to change that? Not sure.
Sounds like that should be changed, formation templates should imo define what units can go in (as the formation parents the units).
Thanks! D2006 is a similar patch, perhaps you're interested in reviewing that one as well?
It's a bit unfortunate that tooltips speaking of classes are in formation templates, whereas it's actually unit templates that decide what formations they can have.
Is it a good idea to change that? Not sure.
Sounds like that should be changed, formation templates should imo define what units can go in (as the formation parents the units).
Yes, that would probably an improvement.
I wrote "unfortunate" because I didn't see an obvious way to fix this. I guess the best way would be to specify groups of identity classes that are needed to be able to have this formation, but that also sounds like it might be too general, and we might want to keep entities as spec-ing that.