Details

Reviewers

Group Reviewers

Restricted Owners Package

(Owns No Changed Paths)

Commits

rP23923: Alphabetize entity limits and statistics classes.

Summary

This patch alphabetizes the <EntityLimits> inside the player.xml special template. This should make mistakes and subsequent fixes such as rP23788 less likely.
While at it, it also corrects the indentation under <StatisticsTracker> in the same file,
[EDIT] and alphabetizes the <StructureClasses> and <UnitClasses>.
See also D2887.

Test Plan

Check for mistakes and omissions.

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.Jun 23 2020, 8:35 PM

Owners added a subscriber: Restricted Owners Package.Jun 23 2020, 8:36 PM

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

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2513/display/redirect

Harbormaster completed remote builds in B12214: Diff 12439.Jun 23 2020, 8:40 PM

Freagarach added a subscriber: Freagarach.Jun 23 2020, 9:42 PM

Freagarach added inline comments.

binaries/data/mods/public/simulation/templates/special/player/player.xml
94 ↗	(On Diff #12439)	While at it one could sort these also, unless there is a different order for these?

Nescio added inline comments.Jun 23 2020, 10:00 PM

binaries/data/mods/public/simulation/templates/special/player/player.xml
94 ↗	(On Diff #12439)	Isn't this the order they're listed in the summary?

Freagarach added inline comments.Jun 24 2020, 5:41 PM

binaries/data/mods/public/simulation/templates/special/player/player.xml
94 ↗	(On Diff #12439)	Ah I see. Would be nice to keep the same order indeed. Not sure why `Domestic` is before `Trader` then, for the former is not shown?

Nescio added inline comments.Jun 24 2020, 7:53 PM

binaries/data/mods/public/simulation/templates/special/player/player.xml
94 ↗	(On Diff #12439)	Good question! I don't know. I suppose `Domestic` is used for the last column of the Resources tab? Likewise, `FemaleCitizen` is not displayed directly, but presumably used for calculating the feminization number?

Nescio added inline comments.Jun 24 2020, 8:24 PM

binaries/data/mods/public/simulation/templates/special/player/player.xml
94 ↗	(On Diff #12439)	Removing `Domestic`, or any other of the classes listed here, does indeed cause errors with the summary code. Reordering the classes seems to have no consequences. Though the question is whether alphabetical order is indeed preferable to display order in this case.

Freagarach added inline comments.Jun 25 2020, 8:22 AM

binaries/data/mods/public/simulation/templates/special/player/player.xml
94 ↗	(On Diff #12439)	I suppose Domestic is used for the last column of the Resources tab? Yes, but why it is shown before the Trader was my question. I guess nobody explicitly used an order here. Likewise, FemaleCitizen Perhaps, because display ordering can not be fully achieved (due to the tabs and implicit uses), I would say alphabetical order would be nice. What say you?

Freagarach added inline comments.Jun 25 2020, 8:27 AM

binaries/data/mods/public/simulation/templates/special/player/player.xml
94 ↗	(On Diff #12439)	I've done some searching. E.g. the domestic entry was added here (actually in `StatisticsTracker`) in D1052/rP20543. Maybe at this very location so that only one line was altered instead of a comma at the previously last line and then this one. Anyway, there is no explicit reason for the current ordering it seems.

Nescio added inline comments.Jun 25 2020, 10:47 AM

binaries/data/mods/public/simulation/templates/special/player/player.xml
94 ↗	(On Diff #12439)	If you think alphabetical order is better in this case, I don't have any objections. (Also, I don't really understand why Structure and Unit classes are separate.)

alphabetize <StatisticsTracker> classes, per @Freagarach

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

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2524/display/redirect

Freagarach accepted this revision.Jun 25 2020, 11:05 AM

Freagarach added inline comments.

binaries/data/mods/public/simulation/templates/special/player/player.xml
94 ↗	(On Diff #12439)	If you think alphabetical order is better in this case, I don't have any objections. Well, the person committing this has the final word obviously ^^ (Also, I don't really understand why Structure and Unit classes are separate.) Because they are counted separately (total amount trained/built etc.).