Page MenuHomeWildfire Games

[gui] omit unnecessary decimals for resistance tooltip
Needs ReviewPublic

Authored by Nescio on Sep 1 2019, 6:39 PM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Summary

All resistance levels in 0 A.D. are integers in practice, therefore there is no point in forcing tooltips to display them with one decimal.
See also D2975.

Test Plan

Ought to be unproblematic.

Event Timeline

Nescio created this revision.Sep 1 2019, 6:39 PM
Owners added a subscriber: Restricted Owners Package.Sep 1 2019, 6:39 PM
Vulcan added a comment.Sep 1 2019, 6:40 PM

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

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/41/display/redirect

Vulcan added a comment.Sep 1 2019, 6:43 PM

Build failure - The Moirai have given mortals hearts that can endure.

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 389| 389| function getRepairTimeTooltip(entState)
| 390| 390| {
| 391| 391| 	return sprintf(translate("%(label)s %(details)s"), {
| 392|    |-			"label": headerFont(translate("Number of repairers:")),
|    | 392|+		"label": headerFont(translate("Number of repairers:")),
| 393| 393| 			"details": entState.repairable.numBuilders
| 394| 394| 		}) + "\n" + (entState.repairable.numBuilders ?
| 395| 395| 		sprintf(translatePlural(
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 390| 390| {
| 391| 391| 	return sprintf(translate("%(label)s %(details)s"), {
| 392| 392| 			"label": headerFont(translate("Number of repairers:")),
| 393|    |-			"details": entState.repairable.numBuilders
|    | 393|+		"details": entState.repairable.numBuilders
| 394| 394| 		}) + "\n" + (entState.repairable.numBuilders ?
| 395| 395| 		sprintf(translatePlural(
| 396| 396| 			"Add another worker to speed up the repairs by %(second)s second.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 2.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 391| 391| 	return sprintf(translate("%(label)s %(details)s"), {
| 392| 392| 			"label": headerFont(translate("Number of repairers:")),
| 393| 393| 			"details": entState.repairable.numBuilders
| 394|    |-		}) + "\n" + (entState.repairable.numBuilders ?
|    | 394|+	}) + "\n" + (entState.repairable.numBuilders ?
| 395| 395| 		sprintf(translatePlural(
| 396| 396| 			"Add another worker to speed up the repairs by %(second)s second.",
| 397| 397| 			"Add another worker to speed up the repairs by %(second)s seconds.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 411| 411| function getBuildTimeTooltip(entState)
| 412| 412| {
| 413| 413| 	return sprintf(translate("%(label)s %(details)s"), {
| 414|    |-			"label": headerFont(translate("Number of builders:")),
|    | 414|+		"label": headerFont(translate("Number of builders:")),
| 415| 415| 			"details": entState.foundation.numBuilders
| 416| 416| 		}) + "\n" + (entState.foundation.numBuilders ?
| 417| 417| 		sprintf(translatePlural(
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 412| 412| {
| 413| 413| 	return sprintf(translate("%(label)s %(details)s"), {
| 414| 414| 			"label": headerFont(translate("Number of builders:")),
| 415|    |-			"details": entState.foundation.numBuilders
|    | 415|+		"details": entState.foundation.numBuilders
| 416| 416| 		}) + "\n" + (entState.foundation.numBuilders ?
| 417| 417| 		sprintf(translatePlural(
| 418| 418| 			"Add another worker to speed up the construction by %(second)s second.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 2.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 413| 413| 	return sprintf(translate("%(label)s %(details)s"), {
| 414| 414| 			"label": headerFont(translate("Number of builders:")),
| 415| 415| 			"details": entState.foundation.numBuilders
| 416|    |-		}) + "\n" + (entState.foundation.numBuilders ?
|    | 416|+	}) + "\n" + (entState.foundation.numBuilders ?
| 417| 417| 		sprintf(translatePlural(
| 418| 418| 			"Add another worker to speed up the construction by %(second)s second.",
| 419| 419| 			"Add another worker to speed up the construction by %(second)s seconds.",
Executing section cli...

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

The according ticket of pain is #4099. It seems like a consistent philosophy or complete world view on rounding numbers in the GUI is missing. There is also the question as to how simulation templates and simulation components should do the rounding, so that the GUI doesnt have to round.
If it does use rounding, it's usually lying. You don't want to have 0 HP units walking around, and you don't want tradecarts with 0 trace, nor units with 1 trade that dont bump the resource counter if they finished their trip (just two random examples).

svn blame from command line in tooltips.js leads me to rP15713:
https://trac.wildfiregames.com/changeset/15713/ps/trunk/binaries/data/mods/public/gui/session/utility_functions.js

It seems after that it might have been copypasta or some consistency thing.

All armour levels in 0 A.D. are integers

Is this the truth?
If I look into the code, it looks contrary, no?

Nescio added a comment.Sep 7 2019, 8:56 PM

There is no consistency for tooltips, nor does this patch aim to implement such a thing. All it does is remove the .0 from the armour values.

All armour levels in 0 A.D. are integers

Is this the truth?
If I look into the code, it looks contrary, no?

In principle I suppose they could have decimals, but in practice all values (do a grep -r Armour in the simulation/data/ and simulation/templates/ ) are multiples of 1, therefore they're all displayed with a trailing zero, which would be removed by this patch.

elexis added a comment.Sep 8 2019, 3:04 PM
In D2247#94229, @Nescio wrote:

in practice all values (do a grep -r Armour in the simulation/data/ and simulation/templates/ ) are multiples of 1

Should also always account for mods.
The simulation templates are 0ad content, but the simulation components (even the GUI too) are meant to work with arbitrary game content (as far as I'm concerned).

There is no consistency for tooltips

Perhaps it was originally added because 3.0 crush (27%) looks more noble than 3 crush (27%) x)

I don't have strong disagreement on the armor tooltip itself, so I suppose it's okay.

But I wouldn't say there is no consistency, since the tooltip strings above and below the armor tooltip also uses one decimal place (Capture Attack, Melee Attack, Armor, Speed, Gather Rate)..

(Shouldn't Melee attack come prior to Capture Attack?)

(All techs and promotion and whatsoever also result in natural numbers for armor too?)

Also perhaps it wouldnt hurt to remove the trailing 0 in the templates.
And perhaps even going further and making Armor a natural number in the simulation component Schema?

Krinkle added a subscriber: Krinkle.Sep 8 2019, 4:42 PM

Perhaps a "temporary" compromise could be to keep 1 digit only if the number is less than 1? I think as long as it is above 1, players may be more forgiving of rounding errors

It would keep this code separate from whether or not there can be fractions between 0 and 1, or at all. If there truly are never fractions, then the conditional branch will be unused and no-one will see it. If there are (as we might learn), then it will gracefully fallback to current behaviour.

Random related ideas

I think long-term we do need a strategy around this. Perhaps we already have something. I've admittedly not yet understood how this works internally, so bare with me. Perhaps my ignorance provides a fresh perspective.

We have to choose whether we want to have a concept of fractional things that have "happened", e.g. available health and resources.

The benefit of having fractions: If you have percentage-based bonuses about the amounts of something, then you will get the true value it is meant to offer. E.g. if you get 20% bonus on 1 resource trickle per minute. If would mean you now get 1.2 resources instead of 1.0. The fractions would not be displayed in the top bar, and would not be spendable. But, it would internally be there, and visible after 5 cycles when the fraction turns over. This would mean we generally show fractions in places that show the yield of something (the damage they give, the resources they gain), but hide them in others (how much you have, how much you spend).

Not having fractions for internal amounts: This would mean all significant numbers have to be whole numbers internally as well, not just on display. This would mean if we allow bonuses on quantities (health, cost, damage, etc.) they would sometimes not be as effective as they should be. On the other hand, it certainly makes things a lot easier to reason about for the player if we don't allow fractions anywhere in the amounts. This issue of bonuses could be overcome if we ensure bonuses use time or frequency instead of amount. E.g. rather than having a 10% stronger attack, the unit would attack 10% quicker. For attack, I believe we already do this. Assuming a good internal handle on passage of time, this naturally means you do get the full value of the bonus because it's calculated at each turn. Even if the fraction landed between two points in time, it can keep applying it retroactively and track it accordingly in a way that is accurate over the long-term.

Should also always account for mods.
The simulation templates are 0ad content, but the simulation components (even the GUI too) are meant to work with arbitrary game content (as far as I'm concerned).

The armour percentage for level x is 1 - 0.9^x; although it's theoretically possible for a mod to use decimal levels, I doubt people would really use e.g. x = 1.23.

Perhaps it was originally added because 3.0 crush (27%) looks more noble than 3 crush (27%) x)

Or perhaps it's just because attack damage has (and needs) a decimal?

But I wouldn't say there is no consistency, since the tooltip strings above and below the armor tooltip also uses one decimal place (Capture Attack, Melee Attack, Armor, Speed, Gather Rate)..

Health doesn't have decimals, whereas e.g. resource trickle can have numerous.

I don't have strong disagreement on the armor tooltip itself, so I suppose it's okay.

The current situation or the proposed patch?

(Shouldn't Melee attack come prior to Capture Attack?)

Why? Isn't capturing the default?
Speaking of which, I think it would be more logical to list armour between health and capture attack; but that discussion is beyond the scope of this patch.

(All techs and promotion and whatsoever also result in natural numbers for armor too?)

Yes; unlike attack damage, resource costs, health, etc., which typically use multiply modifications, the data/ files use only add for Armour.

Also perhaps it wouldnt hurt to remove the trailing 0 in the templates.

I suppose I could write a patch for that, if I were sure someone would review it; there is already quite a review backlog.

And perhaps even going further and making Armor a natural number in the simulation component Schema?

That wouldn't hurt, but is it necessary?

@Krinkle, yes, I agree, the entire tooltip could use a critical look and perhaps a redesign; especially concerning the various resource stats.
However, what do you mean exactly with fractions? Display 1.2 as 6/5? Wouldn't that make comparing things harder for typical users?

Nescio updated this revision to Diff 11159.Jan 23 2020, 5:56 PM
Nescio retitled this revision from no decimals for armor tooltip to gui: no decimals for armor tooltip.
Nescio edited the summary of this revision. (Show Details)

Allow floating digits for mods that would want decimal armor levels, but remove the enforced .0 from the default armor tooltip.

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

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

Nescio retitled this revision from gui: no decimals for armor tooltip to [gui]: no decimals for armor tooltip.Mar 1 2020, 2:08 PM
Nescio added a subscriber: bb.
Nescio retitled this revision from [gui]: no decimals for armor tooltip to [gui] no decimals for armor tooltip.May 18 2020, 10:27 AM
Nescio added a subscriber: Imarok.Jul 22 2020, 7:06 PM

@Imarok, interested in doing this one-liner too?

Nescio updated this revision to Diff 13310.Thu, Aug 27, 1:10 PM
Nescio edited the summary of this revision. (Show Details)
  • rebased
Nescio retitled this revision from [gui] no decimals for armor tooltip to [gui] omit unnecessary decimals for resistance tooltip.Thu, Sep 3, 1:24 PM
Nescio edited the summary of this revision. (Show Details)