Page MenuHomeWildfire Games

removed wonder duplicates from building lists
ClosedPublic

Authored by Nescio on Nov 3 2017, 5:04 PM.

Details

Summary

The template_unit_infantry.xml template defines a list of building entities shared amongst all factions, which also includes the {civ}_wonder.
However, the majority of individual *_infantry_*_b.xml templates also added the wonder to the building list, which is:

  • unnecessary (it's already included)
  • inconsistent (not all infantry templates do this)
  • confusing (all other shared buildings are not separately added again)
  • potentially problematic (if the wonder is removed from the generic infantry template, it would work only for a minority of factions).

Therefore this patch removes the wonder from individual templates.

Test Plan

Test if everything works as expected and nothing is omitted or overlooked.

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.Nov 3 2017, 5:04 PM
Owners added a subscriber: Restricted Owners Package.Nov 3 2017, 5:04 PM

Nice :)
(You can even use a builder list in top of all structures with the {civ} replacement. So you don't have to put things like (/sele_library) in all specific template.)

Vulcan added a subscriber: Vulcan.Nov 3 2017, 5:19 PM

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

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
Nescio added a comment.Nov 3 2017, 5:25 PM

Nice :)

Thanks :)

(You can even use a builder list in top of all structures with the {civ} replacement. So you don't have to put things like (/sele_library) in all specific template.)

What do you mean exactly?

following r19952

You can move all the specific or less specific things in template_unit_infantry and template_unit_support_female_citizen

structures/{civ}_library for example

elexis awarded a token.Nov 3 2017, 5:58 PM
Nescio added a comment.Nov 3 2017, 6:09 PM

following r19952

You can move all the specific or less specific things in template_unit_infantry and template_unit_support_female_citizen

structures/{civ}_library for example

In A22 that still caused an error if the civ-specific template didn't exist. This is no longer the case in A23? Good to know!

Moving structures (e.g. elephant stables) from specific units to the generic templates could probably be better done in a separate patch at a later stage, when and if it's decided whether or not those new structures (stables etc.) are actually going to be used.

In D1008#39526, @Nescio wrote:

structures/{civ}_library for example

In A22 that still caused an error if the civ-specific template didn't exist. This is no longer the case in A23? Good to know!

yes, fixed.

Moving structures (e.g. elephant stables) from specific units to the generic templates could probably be better done in a separate patch at a later stage, when and if it's decided whether or not those new structures (stables etc.) are actually going to be used.

ah yes it would indeed be a problem

fatherbushido accepted this revision.Nov 3 2017, 6:20 PM
  • complete
  • tested (validation...)
This revision is now accepted and ready to land.Nov 3 2017, 6:20 PM

(thx for the patch)

This revision was automatically updated to reflect the committed changes.