Page MenuHomeWildfire Games

standardize formation tooltips
ClosedPublic

Authored by Nescio on Jun 23 2019, 7:26 PM.

Details

Summary

Standardize formation tooltips to "Requires at least [number] [class]".
(Prompted by @elexis in rP22081.)

Test Plan

Check for mistakes.

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

Event Timeline

Nescio created this revision.Jun 23 2019, 7:26 PM

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

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

elexis added inline comments.Jun 24 2019, 4:11 AM
binaries/data/mods/public/simulation/templates/special/formations/anti_cavalry.xml
0–1

Since when is melee and infantry not capitalized? (I remember being corrected on that)

binaries/data/mods/public/simulation/templates/special/formations/box.xml
6

Soldiers?

Nescio added inline comments.Jun 24 2019, 9:40 AM
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?

Nescio updated this revision to Diff 8639.Jun 29 2019, 1:21 PM

Capitalized classes.

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.

bb added a subscriber: bb.Jul 17 2019, 7:54 PM
bb added inline comments.
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.

bb added inline comments.Jul 17 2019, 7:57 PM
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

Nescio updated this revision to Diff 8964.Jul 17 2019, 9:50 PM

Rephrased skirmish formation tooltip, per @bb.

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

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

bb accepted this revision.Jul 17 2019, 10:19 PM

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).

This revision is now accepted and ready to land.Jul 17 2019, 10:19 PM
This revision was automatically updated to reflect the committed changes.

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.

In D2007#87235, @bb wrote:

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.

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.