Page MenuHomeWildfire Games

standardized structure aura descriptions
ClosedPublic

Authored by Nescio on Apr 1 2019, 11:29 AM.

Details

Summary

This patch rephrases the aura descriptions of structure auras (and also the female inspiration aura) to an uniform standard, to ensure descriptions follow the actual modifications: [class] [change] [attributes], e.g. "Garrisoned Ships +10 health regeneration rate." instead of "Repairs garrisoned ship at 10 HP per second."

See also D1720, D1807, D1808.

Test Plan

Check for mistakes and omissions.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 8904
Build 14611: Vulcan BuildJenkins
Build 14610: arc lint + arc unit

Event Timeline

Nescio created this revision.Apr 1 2019, 11:29 AM
Vulcan added a subscriber: Vulcan.Apr 1 2019, 1:38 PM

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

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

elexis added a subscriber: elexis.Apr 3 2019, 11:14 PM

What's the benefit to having .0? Didn't it use round numbers everywhere?

Stan added a subscriber: Stan.Apr 4 2019, 8:52 AM

I don't think there is any benefit. Unless JavaScript does some weird cast when it's not there ?

Do you mean the .0 in the aura descriptions? Good question. Not all additions are integers; e.g. the brit_hero_cunobelin.json aura has:

		{ "value": "Health/RegenRate", "add": 0.8 }

For consistency I chose to give all added numbers a decimal. Besides, it also helps differentiating addition (e.g. +10.0) from multiplication (e.g. +10%).
https://trac.wildfiregames.com/wiki/EnglishStyleGuide doesn't say how to format numbers in strings, so I suppose it can be argued it's a matter of personal taste. It would make sense to decide upon a standard, though.

elexis added a comment.Apr 4 2019, 2:59 PM

That .8 was necessary because its a really powerful effect (healing 200 units nearby simultaneously) and it degenerated the game balance towards people always having to train the unit.
The question is whether that one unit having one more digit than all the others makes it right to change all the others.
What if an upcoming template change uses 0.75?
If the objective of this diff to achieve consistency, one can also use consistently avoid trailing zeroes.

I guess the additional benefit of the trailing zeroes could be to inform the reader of the value range of the tech/aura effect.
Reading 1.0 informs him it could also be 1.1 for the next tech but not 1.05;
whereas 1 leaves the reader surprised if he encounters cunobelins unique 0.8?

One might even become suspicious and wonder whether the 0.8 was a good idea or whether the aura bonus shouldn't be restricted differently. I don't know if that cunobelin fix was the best idea out there, but he needed a nerf for sure and I wanted to be conservative in the gameplay changes.

There is #3972 in which I thought to propose using sprintf syntax for these strings.

So instead of

"auraDescription": "Citizen-soldiers +10% build rate and resource gather speed.",

it would say

"auraDescription": {

"string": "Citizen-soldiers +%(rate)s%% build rate and resource gather speed.",
"values": {
    "rate": "Builder/Rate"
 }

or something like that.

Then none of the descriptions are wrong, one can change the numbers without breaking the description, and one can change the number format for all strings without changing the strings.
Looks like having the problem that we want 10%, not 1.1, but that should be obtainable.

− vs. - also a bit meh, but the problem is more that the character is not commonly used if it is really considered different from hypen. In programming languages we use hyphen to express subtraction, and minus isn't on our keyboards. Needs to fix society, and I guess we can start fixing society by fixing 0ad first.

Nescio added a comment.Apr 4 2019, 3:21 PM

Personally I think it's perfectly fine some modifications use decimals; there is no reason why every addition should be an integer.
Concerning your trailing zeroes point, for me .0 indicates the number can be a decimal, but not necessarily that it is limited to exactly one decimal.

As for minus versus hyphen, the game can properly display many Unicode characters, therefore there is no reason why user-facing text strings should be limited to ascii only.

#3972 is interesting, but it also makes things less obvious for people who don't know JavaScript. Besides, what to do with technologies with multiple modifications? E.g.

{
  "affects": ["Mercenary"],
  "autoResearch": true,
  "genericName": "Mercenary Units",
  "modifications": [
    {"value": "Attack/Capture/Value",         "multiply": 1.15, "affects": "Soldier"         },
    {"value": "Attack/Melee/Crush",           "multiply": 1.10, "affects": "Melee Elephant"  },
    {"value": "Attack/Melee/Crush",           "multiply": 1.15, "affects": "Melee !Elephant" },
    {"value": "Attack/Melee/Hack",            "multiply": 1.15, "affects": "Melee"           },
    {"value": "Attack/Melee/Pierce",          "multiply": 1.15, "affects": "Melee"           },
    {"value": "Attack/Melee/Thrust",          "multiply": 1.15, "affects": "Melee"           },
    {"value": "Attack/Ranged/Crush",          "multiply": 1.15, "affects": "Ranged"          },
    {"value": "Attack/Ranged/Hack",           "multiply": 1.15, "affects": "Ranged"          },
    {"value": "Attack/Ranged/Pierce",         "multiply": 1.15, "affects": "Ranged"          },
    {"value": "Attack/Ranged/Thrust",         "multiply": 1.15, "affects": "Ranged"          },
    {"value": "Cost/BuildTime",               "multiply": 0.5                                },
    {"value": "Cost/Resources/food",          "replace":  0                                  },
    {"value": "Cost/Resources/metal",         "replace":  0                                  },
    {"value": "Cost/Resources/silver",        "multiply": 2.0                                },
    {"value": "Cost/Resources/stone",         "replace":  0                                  },
    {"value": "Cost/Resources/wood",          "replace":  0                                  },
    {"value": "Health/Max",                   "multiply": 1.1                                }
  ],
  "tooltip": "Mercenaries have +100% silver cost, 0 other resource costs, −50% training time, +10% health, +15% capture attack strength, +15% melee and ranged attack damage."
}
elexis added a comment.Apr 5 2019, 2:43 AM

The tooltip lies about elephants, quite a stuffed tech, but it should still work out, the part +15% melee and ranged attack damage could be +%(AttackMeleeHack)s%% melee and ranged attack damage, still seems less than more likely to mess up the numbers, as one now only needs to compare that ranged and melee attack is the same, instead of testing that ranged and melee attack and the number in the tooltip are the same?

Concerning your trailing zeroes point, for me .0 indicates the number can be a decimal, but not necessarily that it is limited to exactly one decimal.

Why is the format is used if it's not that?

Why is the format is used if it's not that?

For some people +1 might imply everything is an integer, which is not necessarily true. Anyway, if you think it's better to remove the .0 then I can do that.

The tooltip lies about elephants, quite a stuffed tech, but it should still work out, the part +15% melee and ranged attack damage could be +%(AttackMeleeHack)s%% melee and ranged attack damage, still seems less than more likely to mess up the numbers, as one now only needs to compare that ranged and melee attack is the same, instead of testing that ranged and melee attack and the number in the tooltip are the same?

In the example “+15% melee and ranged attack damage” groups nine modifications; AttackMeleeHack is just one value; so if only that value is changed, the statement is no longer true, because the other damage types are unchanged, thus the editor will still have to rephase the tooltip when he change a modification.
Of course, if there is exactly one phrase per statement, then it's not necessary to update the tooltip with your suggestion, but then the total sentence will be much longer and thus more time-consuming to read and understand, which is problematic for players in game.
Another example: team and civ bonuses; they are also listed in the civ files. Right now if one is changed the editor updates the tooltip in the technology file and can copy-paste it to the description in the civilization file; anyone can see at a glance if they're the same. With your suggestion, the tooltip in the bonus file no longer has to be updated, but the description in the civ file still does, since they're no longer identical.
Besides, how often are values actually changed?

bb added a subscriber: bb.Apr 6 2019, 4:02 PM
bb added inline comments.
binaries/data/mods/public/simulation/data/auras/structures/kush_temple_amun.json
8

Just want to make sure the following is considered:
this tooltip contains besides the effect also some historical reference, this patch deletes all those. Now I totally understand the complete irrelevance for gameplay of these referneces, however having such an explanation in a structree or unit viewer is a nice feature.

binaries/data/mods/public/simulation/data/auras/structures/kush_temple_amun.json
8

I saw this in https://code.wildfiregames.com/D1807 as well

Maybe we should leave these context strings for the time being, let these normalization patches focus specifically on the paragraph that needs normalization.

There are changes we could implement in the future (move history backgrounds below the aura descriptions, keep them on a separate variable so that translators can focus on the gameplay-affecting texts first, and then maybe even let players choose whether or not they are shown in game through configuration if someone asks for it), but I think they are all out of the scope of these changes.

Nescio added inline comments.Jul 21 2019, 10:24 AM
binaries/data/mods/public/simulation/data/auras/structures/kush_temple_amun.json
8

In this particular case I'd say purge it; none of the other structure auras has filler text; the reason this one does is presumably because the Kushite files weren't rigorously checked before they were merged; nor is this specific string really informative.
Besides, flavour text can be inserted in the {civ}.json files.

(Catafalques are a different story.)

If no one opposes to removing the historic text that is not expected to be in auras at the moment, then I guess the only remaining thing is removing .0 from numbers, right?

binaries/data/mods/public/simulation/data/auras/structures/kush_temple_amun.json
8

OK from my side.

Nescio added a comment.Aug 3 2019, 4:41 PM

I guess the only remaining thing is removing .0 from numbers, right?

That and a decision whether or not classes should be capitalized in descriptions and tooltips.

Nescio updated this revision to Diff 9259.Aug 7 2019, 11:18 AM
Nescio edited the summary of this revision. (Show Details)

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

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

Gallaecio added inline comments.Aug 9 2019, 7:10 AM
binaries/data/mods/public/simulation/data/auras/structures/wonder_pop_2.json
8

40.0 → +40

Shouldn’t “per wonder owned” remain in the description?

binaries/data/mods/public/simulation/data/auras/structures/workshop_repair.json
8

engine → Engine?

Nescio added inline comments.Aug 9 2019, 9:42 AM
binaries/data/mods/public/simulation/data/auras/structures/wonder_pop_2.json
8

Oops, missed that .0, will correct.

And no, we don't have that in other stackable auras (e.g. library, theatre) either.

binaries/data/mods/public/simulation/data/auras/structures/workshop_repair.json
8

The class is Siege, not SiegeEngine.

Gallaecio added inline comments.Aug 10 2019, 11:25 AM
binaries/data/mods/public/simulation/data/auras/structures/workshop_repair.json
8

We can post-pone this discussion and merge as is.

But I think that, even if the class is called Siege in code, it does not make sense to consider ‘engine’ separately for capitalization. I think in ‘siege engine’, mid sentence, we should capitalize either both words or none of them, and when referring to units with the Siege class I would capitalize both words.

Nescio updated this revision to Diff 9286.Aug 10 2019, 7:11 PM

Siege Engines

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

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

We can post-pone this discussion and merge as is.

If you mean whether or not classes should be capitalized, no, I don't think postponing that decision is wise. There are a few patches updating strings and there will be a couple more after this one is committed. Capitalization is a recurring question so we should settle one way or the other and have a clear decision for this and future patches.
I can see arguments both for and against. Excessive Capitalization Can Make Texts Harder to Read, besides, English isn't German, and having uncapitalized classes is aesthetically better. However, gameplay-wise capitalization of classes does make sense for added emphasis. Personally I don't mind either way, provided it is done consistently.

In D1806#90158, @Nescio wrote:

We can post-pone this discussion and merge as is.

If you mean whether or not classes should be capitalized […]

No, I don’t mean that. I think we should capitalize them. I mean specifically what I described. I think there are a few cases where code class names and user-visible class names must not be identical. The most obvious case is where classes are CamelCase and we add a space between the words. But here, I think the Siege class from code should translate to “Siege Engine” in user-visible text.

Again, I don’t mind merging the changes as is.

As you can see Siege Engines is capitalized, as requested. Other exceptions probably ought to be listed on the https://trac.wildfiregames.com/wiki/EnglishStyleGuide page.

I think we should capitalize them.

Agreed.

Gallaecio added inline comments.Aug 16 2019, 9:36 PM
binaries/data/mods/public/simulation/data/auras/structures/kush_temple_amun.json
8

Aren’t we keeping the flavor text for now?

binaries/data/mods/public/simulation/data/auras/structures/wonder_pop_2.json
8

Since we are changing the string already, maybe we could switch to ‘requires the “Glorious Expansion” technology’ (‘the’ added after ‘requires’).

Just ‘requires “Glorious Expansion”’ would also work, but I think making clear that it is a ‘technology’ is very important here.

Nescio added inline comments.Aug 17 2019, 11:54 AM
binaries/data/mods/public/simulation/data/auras/structures/kush_temple_amun.json
8

For catafalques (D1808), yes.
In this structure aura description, however, I think it ought to be removed, because:

  • none of the other structure (or hero) auras has flavour text
  • flavour text for structures (and heroes) can be put in the {civ}.json files
  • this specific string doesn't really add much
Nescio updated this revision to Diff 9368.Aug 17 2019, 11:55 AM

the “” technology

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

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

OK, I think we can merge once the + in +40 is back

binaries/data/mods/public/simulation/data/auras/structures/wonder_pop_2.json
8

I think the + in +40 got accidentally lost in the last change.

Nescio updated this revision to Diff 9377.Aug 18 2019, 4:03 PM
Nescio edited the summary of this revision. (Show Details)

+

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

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

Gallaecio accepted this revision.Aug 24 2019, 11:18 AM
This revision is now accepted and ready to land.Aug 24 2019, 11:18 AM
This revision was automatically updated to reflect the committed changes.