The <Identity/Tooltip> string is supposed to briefly summarize the characteristics of the entity. This patch revisits the tooltips of all structures and attempts to standardize them.
Things that already have their own tooltip entries (e.g. auras, garrisoned heal, population, resource trickle) are removed. Some important things that were not mentioned earlier (buildable territory, resource dropsite, territory root) are inserted. Classes are capitalized. Phrasings are standardized (e.g. “Garrison Soldiers for additional arrows.”); “improvements” is replaced with “technologies”.
Details
- Reviewers
Gallaecio Freagarach
Check for mistakes, omissions, possible improvements. Be critical.
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 10856 Build 18984: Vulcan Build Jenkins Build 18983: Vulcan Build (macOS) Jenkins Build 18982: Vulcan Build (Windows) Jenkins Build 18981: arc lint + arc unit
Event Timeline
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1593/display/redirect
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1594/display/redirect
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1596/display/redirect
For territory root see also D2005.
Yeah, but it's unclear if and when that one will be committed, or in what form. Removing "Territory root." from the <Tooltip> strings later shouldn't be much work.
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/1075/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/1076/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/1078/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/macos-differential/171/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/macos-differential/172/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/macos-differential/174/display/redirect
binaries/data/mods/public/simulation/templates/structures/cart_embassy_italiote.xml | ||
---|---|---|
20 | Should we also change “improvements” to “technologies” here? | |
binaries/data/mods/public/simulation/templates/structures/ptol_mercenary_camp.xml | ||
32 | I see “Buildable in neutral territory.” here and “Build in neutral and own territories.” in a later chance, and I can’t help but wonder if they should be unified, maybe as “Build in neutral territory.”? Maybe in a later patch? | |
binaries/data/mods/public/simulation/templates/template_structure_civic_civil_centre.xml | ||
88 | “food, wood, stone, and metal” (GUI sorting, add “and”) | |
binaries/data/mods/public/simulation/templates/template_structure_defensive_tower.xml | ||
52 | “tech” → “technology” | |
binaries/data/mods/public/simulation/templates/template_structure_military_dock.xml | ||
26 | “Build upon a shoreline in own or neutral territory”? “food, wood, stone, and metal” |
- build restrictions
- technologies
- wood, stone, *and* metal
- redistributed fortress tooltips
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/1081/display/redirect
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1599/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/macos-differential/177/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/1083/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/macos-differential/179/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1601/display/redirect
I’ve left one more comment, but overall this is an amazing change for the better, great work!
binaries/data/mods/public/simulation/templates/structures/cart_embassy.xml | ||
---|---|---|
20 ↗ | (On Diff #11092) | Why is “mercenaries” lowercase but “Mercenary” capitalized? |
binaries/data/mods/public/simulation/templates/structures/cart_embassy.xml | ||
---|---|---|
20 ↗ | (On Diff #11092) | Thanks for pointing that out! Mercenary is a visible class and thus ought to be capitalized, just like Cavalry, Champion, Citizen, Soldier, etc. |
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1610/display/redirect
Just wondering: train seems like D1245 and recruit more like what is done when clicking the button? I think this can be comitted as is and we can discuss "train" vs "recruit" elsewhere, if you want?
Requests:
- The structures/maur_defense_tower is missing a ..
- The structures/gaul_wall_tower could use an update as well.
- structures/brit_wall_tower idem.
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1628/display/redirect
The structures/maur_defense_tower is missing a ..
Not done: this patch standardizes <Identity> tooltips. <Upgrade> tooltips could be done in a future patch.
Just wondering: train seems like D1245 and recruit more like what is done when clicking the button? I think this can be comitted as is and we can discuss "train" vs "recruit" elsewhere, if you want?
https://trac.wildfiregames.com/wiki/EnglishStyleGuide#Actions says “train (human units)” and that's also the verb used in the aura descriptions and technology tooltips (“training time”), so for consistency's sake we should stick with that in this patch.
A future patch could replace train with conscript/enlist/hire/muster/recruit, if people think that's clearer.
[EDIT]: “drill” is the verb I'd use for what D1245 proposes.
- Because wall segments are not buildable on their own, moved their tooltip to template_wallset.xml.
- Also standardized <Identity> and <Upgrade> tooltips of all gates.
- Purged meaningless tower <Upgrade> tooltips, since the entity it will be upgraded to and the resources upgrading cost are already present in the automatically displayed tooltip. Having and additional <Tooltip> string just means more text, not more information.
- While at it, renamed tower <Upgrade> nodes for clarity and consistency.
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1650/display/redirect
binaries/data/mods/public/simulation/templates/structures/palisades_long.xml | ||
---|---|---|
19 ↗ | (On Diff #11174) | to → into |
binaries/data/mods/public/simulation/templates/structures/ptol_lighthouse.xml | ||
21 | I wonder if we should order words about territory ownership as follows: own, allied, neutral, enemy. | |
binaries/data/mods/public/simulation/templates/template_structure_civic_temple.xml | ||
34 | Would “Healer technologies” be accurate? (i.e. do these technologies impact only Healer units, or also allow wider things, such as automatic healing of all units, of higher hitpoints?). If it is accurate, it may be more consistent with similar strings (Mercenary technologies, Ship technologies). |
binaries/data/mods/public/simulation/templates/structures/ptol_lighthouse.xml | ||
---|---|---|
21 | Just to be clear, neutral mean's no man's land, not the territory of neutral players.
Basically the order is from more to less likely. That it's revered alphabetical is a coincidence. An argument in favour of listing ally before neutral is that's the order in simulation/components/BuildRestrictions.js, though that could easily be rearranged, if deemed necessary. |
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1660/display/redirect
binaries/data/mods/public/simulation/templates/structures/ptol_lighthouse.xml | ||
---|---|---|
21 | I still prefer and order based on owner affinity, so while neutral before allied may make sense since neutral here means no-man’s land, I would still put enemy last. I do not have a strong opinion about it, though, so if you still prefer “own, neutral, enemy, allied” I won’t oppose. I do wonder, given that there can be neutral players, if we should find an alternative to ‘neutral’ for this use case, and leave ‘neutral’ to refer to territory of neutral players. ‘Unclaimed’, maybe? |
binaries/data/mods/public/simulation/templates/structures/ptol_lighthouse.xml | ||
---|---|---|
21 |
Which is exactly why I prefer this order:
There aren't any structures buildable both in ally and enemy territory, though.
The automatically generated tooltips use those precise words: Therefore I'd strongly recommend sticking with that to avoid unnecessary confusion. If you want to use a different word, then it has to be changed in the relevant JavaScript code files, i.e. beyond the scope of this patch. Interestingly, the territory of neutral players is considered “enemy” territory. I guess that makes sense, because you don't share anything with them, unlike allies, so their territory is as hostile as that of enemies. Though maybe there ought to be a separate case for the territory of neutral players? I don't know. If desired, again something to change in the JS code, thus outside the scope of this patch. |