Currently the "ConquestCritical" class is added to the basic template_structure.xml file and subsequently removed in the majority of cases. This is quite ugly. A much neater way is to add it only to the cases that actually have it (Civic, Military, Wonder, and several special structures), but not to the basic parent template.
Details
Details
Effectively nothing ought to be changed.
Diff Detail
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
Comment Actions
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/differential/733/display/redirect
Comment Actions
15 removed cases, 10 added ones (if counting holds, the 15 goes down to 13 given the structure_defense_* comment)
Not convinced either way about this patch
| binaries/data/mods/public/simulation/templates/template_structure_defense_tower.xml | ||
|---|---|---|
| 46 ↗ | (On Diff #6885) | With the current design the structure_defense_* could be moved into the parent |
Comment Actions
Currently template_structure.xml adds ConquestCritical, but:
- template_structure_defense_*.xml
- template_structure_economic.xml
- template_structure_resource.xml
- template_structure_special_*.xml
remove it. Instead of adding it everywhere and subsequently removing it often, I think it's neater to simply add it only to the cases that actually use it:
- template_structure_civic.xml
- template_structure_military.xml
- template_structure_wonder.xml
- several structures/ files.
Comment Actions
Reading it again, the actual question is how to treat special (the rest looks good)
in special 10 removed, 10 added, so I guess if someone adds a structure one is as likely to add a conquestcritical one or not, so @coin and take the positive way.
One could argue: if in doubt it should be conquestcritical, so rather add to much than to less, but seeing the number of benches/fenches/tables etc. that are conquestcritical by this (those tags are remove by this patch) , I don't think that argument really holds.
However rome_arch, pers_tacara and pers_ishtar_gate should have the conquestcritical tag imo
(defense template got renamed so rebasing that)