Page MenuHomeWildfire Games
Differential D2053

Implement wrapper around displaying numbers in gui
AbandonedPublic
Actions

Authored by Silier on Jul 7 2019, 2:38 PM.

Details

Reviewers
None
Trac Tickets
#4099
Summary

Noticed by ValihrAnt value of capture attack is not rounded to decimal places https://www.youtube.com/watch?v=C-D_CwlTBtY&t=310s

Introduced here: rP16550
I see that damageTypesToText was ignored because different capture does not have damage types, that way no rounding was introduced by accident / wanted / did not look needed.

Next patch changing things around rP18454 used value without rounding

Why to have capture attack rounded to 1 decimal place:
it is just another attack and all attack strengths and armour strengths are rounded with ToFixed(1)

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/toFixed

Test Plan

See if we should round anything else

Diff Detail

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

Event Timeline

Silier created this revision.Jul 7 2019, 2:38 PM
Owners added a subscriber: Restricted Owners Package.Jul 7 2019, 2:38 PM
Vulcan added a comment.Jul 7 2019, 2:40 PM

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

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

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

Harbormaster completed remote builds in B8231: Diff 8764.Jul 7 2019, 2:40 PM
elexis updated the Trac tickets for this revision.Jul 9 2019, 12:43 PM
Silier edited the summary of this revision. (Show Details)Jul 9 2019, 1:23 PM
Silier added a comment.Jul 9 2019, 5:09 PM

IRC discussion (http://irclogs.wildfiregames.com/2019-07/2019-07-09-QuakeNet-%230ad-dev.log)

10:54 -!- Angen_ [webchat@cloak-3qwyuii6.178-40-11.t-com.sk] has joined #0ad-dev
10:54 < Angen_> I think we can freely use toFixed instead floor
10:54 < Angen_> https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/toFixed
10:55 < elexis> there are many places where one could round
10:55 < elexis> (or use toFixed)
10:55 < elexis> templates, simulation, gui
10:55 < Angen> yes if one wants 0 decimal places then use round
10:55 < elexis> and if one rounds in the templates and simulation, then one doesnt need it in the UI
10:55 < elexis> (1) is the part that seems like a workaround
10:56 < elexis> magic number
10:56 < elexis> what if some template author wanted 0.25 
10:56 < elexis> the templates might specifcy that perhaps, dunno
10:56 < Angen> the same can be said about other attacks
10:56 < elexis> correct
10:57 < elexis> perhaps it already was said
10:57 < elexis> xd
10:58 < Angen> for me running any attack with toFixed (1) except capture is not consistant as it is. let me find revision 
10:58 < elexis> prima facie is sometimes falsified
10:58 < Angen> *rounding
11:02 < wraitii> Indeed we should check what we've decided in terms of rounding
11:02 -!- FeXoR [~fexor@p5DD0A997.dip0.t-ipconnect.de] has joined #0ad-dev
11:02 < wraitii> The best approach is probably a globalscript function that rounds
11:02 < wraitii> RoundForGUI() or something
11:04 < elexis> Angen: rounding or flooring (0, 1 or n digits) can also mean that it displays 0 capture points when there is more than zero remaining
11:04 < elexis> if that was considered a bug that needs to be fixed, then that patch would be making the bugs in the codebase more consistent
11:05 < elexis> so it may be a good patch but is it the right one
11:17 < wraitii> Rounding might be better done in GUiInterface.js 
11:19 < elexis> wont fix the 0 issue if its not in the sim
11:21 < historicbruno> what kind of values are we talking about?
11:21 < wraitii> Here it's attack values
11:21 < historicbruno> like what's the normal range you expect to see
11:22 < wraitii> I guess in general we want rounding to 1 or 2 decimal places (or perhaps to increments of 0.05), but for HP and other '0 means death' values we probably need some special-cashing
11:22 < historicbruno> yeah there are definitely multiple cases
11:24 < WildfireBot> A​n​g​e​n updated the summary of D2053 (Round value of capture attack) https://code.wildfiregames.com/D2053
11:24 < elexis> or trade carts transporting 0 resources
11:25 < wraitii> Then again I guess transporting 0.01 resources is effectively equivalent to 0
11:25 < Angen> transporting 0.1 is strange enough, I think that should not be the case at all
11:25 < wraitii> So maybe that ought to be a sim check?
11:26 < elexis> check as in rounding when changing values?
11:26 < wraitii> no as in refusing to trade if the value would be below X
11:26 < elexis> and X is yet another magic constant?
11:26 < wraitii> I mean in that instance showing "1" when the value is "0.01" sounds like lying
11:27 < elexis> perhaps it could be set in the templates
11:27 < Angen_> it would show 1 only if 0.5 but ok :)
11:27 < wraitii> yea I was talking about "ceil"
11:27 < historicbruno> not 0.4999999999?
11:27 < Angen_> ah right
11:28 < wraitii> showing 0 HP when it's 0.01 is worse than showing 1 HP imo, but showing 1 resource when it's 0.01 is worse than showing 0
11:28 < wraitii> constants for human-understanding are all magic :p 
11:29 < historicbruno> you could just make trading integer only.  Partial resources aren't really that useful are they?
11:29 < historicbruno> I mean if the value is 20 and it really technically should be 20.25 or 20.5 ...
11:30 -!- bb [~bb@a80-101-100-143.adsl.xs4all.nl] has joined #0ad-dev
11:30 < historicbruno> surely the UI already shows resources rounded as integers?
11:30 < elexis> avoid the word "surely"
11:31 < wraitii> well the issue here is "what should the UI do"
11:31 < historicbruno> I can't remember is what I mean xD
11:31 < historicbruno> I know we put lots of rounding code in years ago
11:31 < elexis> sometimes I wonder if the codebase is subject to the mandela effect
11:31 < Angen_> definitly UI should not show 32.5455555555
11:31 < elexis> why not, if that's what its value is?
11:32 < historicbruno> that's just a poor UX
11:32 < wraitii> yeah it's unreadable
11:32 < elexis> it's better to show a wrong number?
11:32 < wraitii> I guess we should think in terms of significant digits
11:32 < wraitii> it is, yes
11:32 < elexis> it is out of sync
11:32 < elexis> GUI and simulation
11:33 < Angen_> player does not care if it shows 32.55 and value is 32.5554
11:33 < elexis> showing a wrong number and showing an ugly number seems like a false dichotomy
11:33 < historicbruno> that's pretty much unavoidable with float precision
11:33 < elexis> 0.00001 is an issue I guess
11:34 < wraitii> I'd prefer to see 0.01, 0.15, 1.4 and 25 over '0.0999', '0.1495848', '1.38968738' and '24.698774'
11:34 < elexis> perhaps one could make the Fixed type more configurable
11:34 < elexis> for example steps of 0.25
11:34 < elexis> and specifying that in the number type in the template and using that in the simulation
11:35 < Angen_> elexis: yes 0.0001 for health or capture is issue
11:36 < elexis> (not sure if it would work, but it would satisfy the two conditions of esthetically pleasing numbers and display being accurate)
11:36 < historicbruno> wraitii: right, long decimal numbers should clearly be truncated or rounded for UI purposes.  You won't find any game with things like "1.38968738" in the UI, unless it's some very scientific simulation or something (we're not)
11:36 < wraitii> that sounds like a terribly complicated solution to a rather simple problem to be honest
11:36 < elexis> historicbruno: yes, that's out of question
11:37 < elexis> the question is rather whether the approach to round it in the GUI is the only choice that follows logically
11:37 < wraitii> I think we just need to ceil but with some significant-enough decimals as outlined above, in all cases
11:37 < historicbruno> elexis: not at all
11:37 < wraitii> and what should be an integer stays an integer
11:37 < historicbruno> elexis: but there are multiple cases
11:37 < elexis> yes, time resource is fractional
11:37 < elexis> maybe
11:37 < historicbruno> why not make a list and classify them all? :)
11:38 < wraitii> So for example "30.0001" gets 'ceiled' to "30.01" which itself gets rounded to 30
11:38 < historicbruno> I personally can't keep them all straight, and so many have been added since I last played
11:38 < wraitii> But "0.001" gets ceiled to "0.01" which gets 'rounded' to "O.01" as that's the significant digit
11:38 < wraitii> it would be more complex logic but it'd be nice
11:38 < elexis> you can be sure that there will be many more of these one-line GUI rounding patches unless someone revisits everything
11:38 < bb> (in science one usually says "n decimals significant" so if we says 2 decimals significant "0.00001", becomes "0.000010" and "1 gets 1.0" and such however 100 gets 1.0 *10^2)
11:39 < historicbruno> (like:  Value, Range, UI round, Sim round)
11:39 < historicbruno> since there clearly is no one-size-fits-all solution
11:40 < wraitii> bb: right, I know, should have used a different word
11:40 < historicbruno> if we see how many cases there are, we can make general functions for them, and not have to keep reinventing the wheel
11:41 < bb> wraitii: actually wasn't responding to your message, more like oping a possibility
11:41 < wraitii> I think my solution above would be generally good enough
11:42 < wraitii> Having an HP shown as "0.04/3000", which would go "23/3000" once HP is high enough seems fine to me
Silier added a comment.Jul 10 2019, 11:02 AM

Adding one more argument why have toFixed(1) and no toFixed(2) for example

https://trac.wildfiregames.com/ticket/3817#comment:5

Silier added a comment.Jul 19 2019, 3:25 PM
D:\A.0.Development\binaries\data\mods\public\gui>findstr /s /n "Math.round" *
common\color.js:156:    return [r, g, b].map(n => Math.round(n * 255));
common\colorFades.js:201:               rgb.g += Math.round(g_FadeAttackUnit.gbcolorChangeRate * Math.sqrt(data.tickCounter - g_FadeAttackUnit.blinkingTicks));
common\tooltips.js:177:         "current": Math.round(entState.hitpoints),
common\tooltips.js:178:         "max": Math.round(entState.maxHitpoints)
common\tooltips.js:210:         "percentage": (100 - Math.round(Math.pow(0.9, level) * 100))
common\tooltips.js:283:         let minRange = Math.round(template.attack[type].minRange);
common\tooltips.js:284:         let maxRange = Math.round(template.attack[type].maxRange);
common\tooltips.js:286:         let relativeRange = realRange ? Math.round(realRange - maxRange) : 0;
common\tooltips.js:350:                         "value": Math.round(template.garrisonHolder.buffHeal),
common\tooltips.js:367:                 Math.round(template.buildingAI.garrisonArrowMultiplier *
common\tooltips.js:401:                 Math.round(entState.repairable.buildTime.timeRemaining - entState.repairable.buildTime.timeRemainingNew)),
common\tooltips.js:403:                 "second": Math.round(entState.repairable.buildTime.timeRemaining - entState.repairable.buildTime.timeRemainingNew)
common\tooltips.js:408:                 Math.round(entState.repairable.buildTime.timeRemainingNew)),
common\tooltips.js:410:                 "second": Math.round(entState.repairable.buildTime.timeRemainingNew)
common\tooltips.js:423:                 Math.round(entState.foundation.buildTime.timeRemaining - entState.foundation.buildTime.timeRemainingNew)),
common\tooltips.js:425:                 "second": Math.round(entState.foundation.buildTime.timeRemaining - entState.foundation.buildTime.timeRemainingNew)
common\tooltips.js:430:                 Math.round(entState.foundation.buildTime.timeRemainingNew)),
common\tooltips.js:432:                 "second": Math.round(entState.foundation.buildTime.timeRemainingNew)
loading\loading.js:86:  let increment = Math.round(progress * middleLength / 100);
lobby\lobby.js:1017:                            Math.round(playerRatings.reduce((sum, current) => sum + current) / playerRatings.length) :
session\input.js:175:                           var averageRange = Math.round(Engine.GuiInterfaceCall("GetAverageRangeForBuildings", cmd) - cmd.range);
session\input.js:176:                           var range = Math.round(cmd.range);
session\input.js:1363:  return Math.max(Math.round(g_BatchSize), 1);
session\menu.js:847:    barterAmount.Buy.caption = "+" + Math.round(prices.sell[g_BarterSell] / prices.buy[resourceCode] * amountToSell);
session\selection_panels.js:602:                size.top = size.left + Math.round(queuedItem.progress * (size.right - size.left));
session\selection_panels.js:1124:                       size.top = size.left + Math.round(progress * (size.right - size.left));
session\selection_panels_helpers.js:48: return "color:255 0 0 " + Math.min(125, Math.round(+totalCost / 10) + 50);
session\session.js:1167:                                Math.round(playerStates[player].resourceCounts[resource]),
session\session.js:1169:                                Math.round(playerStates[player].resourceCounts[resource])
session\session.js:1281:                size.top = size.left + Math.round(researchStarted[tech].progress * (size.right - size.left));
session\session.js:1655:                playerStatistics.militaryScore += Math.round((player.sequences.enemyUnitsKilledValue[maxIndex] +
session\session.js:1657:                playerStatistics.totalScore += (total + Math.round((player.sequences.enemyUnitsKilledValue[maxIndex] +
summary\counters.js:130:        return Math.round(total / 10);
summary\counters.js:135:        return Math.round((playerState.sequences.enemyUnitsKilledValue[index] +

D:\A.0.Development\binaries\data\mods\public\gui>findstr /s /n "Math.ceil" *
common\network.js:18:                   "seconds": Math.ceil(msg.lastReceivedTime / 1000)
common\network.js:24:                   "seconds": Math.ceil(msg.lastReceivedTime / 1000)
common\tooltips.js:456:         totalCosts.time = Math.ceil(template.cost.time * (entity ? Engine.GuiInterfaceCall("GetBatchTime", {
gamesetup\gamesetup.js:1469:    let perColumn = childCount / Math.ceil(childCount / maxPerColumn);
session\input.js:1208:  let entitiesBetweenMarker = Math.ceil(5 / distanceBetweenEnts);
session\selection_details.js:115:                       "hitpoints": Math.ceil(entState.hitpoints),
session\selection_details.js:116:                       "maxHitpoints": Math.ceil(entState.maxHitpoints)
session\selection_details.js:154:                       "capturePoints": Math.ceil(entState.capturePoints[entState.player]),
session\selection_details.js:155:                       "maxCapturePoints": Math.ceil(entState.maxCapturePoints)
session\selection_details.js:191:                               "amount": Math.ceil(+entState.resourceSupply.amount),
session\selection_panels.js:554:                let numRows = Math.ceil(numberOfItems / rowLength);

D:\A.0.Development\binaries\data\mods\public\gui>findstr /s /n "Math.floor" *
common\tooltips.js:444:                 totalCosts[r] = Math.floor(template.cost[r] * trainNum);
gamesetup\gamesetup.js:1467:    let maxPerColumn = Math.floor((actualSettingsPanelSize.bottom - actualSettingsPanelSize.top) / g_SettingHeight);
pregame\userreport\userreport.js:30:    "sending": data => sprintf(translate("uploading (%f%%)"), Math.floor(100 * data[1])),
session\input.js:1501:          buildingsCountToTrainFullBatch = Math.floor(canBeAddedCount / nextBatchTrainingCount);
session\input.js:1517:          let buildingsCountToTrainFullBatch = Math.floor( g_BatchTrainingEntityAllowedCount / batchedSize);
session\menu.js:596:                            modifiedTemplate.cost[res] = Math.floor(GetSimState().players[i].spyCostMultiplier * template.cost[res]);
session\menu.js:607:                                    modifiedTemplate.cost[res] = Math.floor(costRatio * modifiedTemplate.cost[res]);
session\selection_details.js:175:                               "current": Math.floor(entState.promotion.curr),
session\selection_details.js:181:                               "current": Math.floor(entState.promotion.curr)
session\session.js:1221:                Engine.GetGUIObjectByName("resource[" + r + "]_count").caption = Math.floor(viewedPlayerState.resourceCounts[res]);
session\unit_commands.js:36:    var vIndex = Math.floor(index / rowLength);
summary\counters.js:5:  return { "percent": divisor ? Math.floor(100 * divident / divisor) : 0 };
summary\summary.js:226:                 Math.floor(playerState.color.r * 255) + " " +
summary\summary.js:227:                 Math.floor(playerState.color.g * 255) + " " +
summary\summary.js:228:                 Math.floor(playerState.color.b * 255)
summary\summary.js:409:                 Math.floor(playerState.color.r * 255) + " " +
summary\summary.js:410:                 Math.floor(playerState.color.g * 255) + " " +
summary\summary.js:411:                 Math.floor(playerState.color.b * 255);
Silier added a comment.Jul 19 2019, 3:45 PM

Here is current list of places in gui section where roof, ceil or round are used, filtered out not relevant results and splitted as I am
not sure about some cases

common\tooltips.js:177:         "current": Math.round(entState.hitpoints),
common\tooltips.js:178:         "max": Math.round(entState.maxHitpoints)
common\tooltips.js:210:         "percentage": (100 - Math.round(Math.pow(0.9, level) * 100))
common\tooltips.js:283:         let minRange = Math.round(template.attack[type].minRange);
common\tooltips.js:284:         let maxRange = Math.round(template.attack[type].maxRange);
common\tooltips.js:286:         let relativeRange = realRange ? Math.round(realRange - maxRange) : 0;
common\tooltips.js:350:                         "value": Math.round(template.garrisonHolder.buffHeal),
common\tooltips.js:367:                 Math.round(template.buildingAI.garrisonArrowMultiplier *
common\tooltips.js:401:                 Math.round(entState.repairable.buildTime.timeRemaining - entState.repairable.buildTime.timeRemainingNew)),
common\tooltips.js:403:                 "second": Math.round(entState.repairable.buildTime.timeRemaining - entState.repairable.buildTime.timeRemainingNew)
common\tooltips.js:408:                 Math.round(entState.repairable.buildTime.timeRemainingNew)),
common\tooltips.js:410:                 "second": Math.round(entState.repairable.buildTime.timeRemainingNew)
common\tooltips.js:423:                 Math.round(entState.foundation.buildTime.timeRemaining - entState.foundation.buildTime.timeRemainingNew)),
common\tooltips.js:425:                 "second": Math.round(entState.foundation.buildTime.timeRemaining - entState.foundation.buildTime.timeRemainingNew)
common\tooltips.js:430:                 Math.round(entState.foundation.buildTime.timeRemainingNew)),
common\tooltips.js:432:                 "second": Math.round(entState.foundation.buildTime.timeRemainingNew)
common\tooltips.js:444:                 totalCosts[r] = Math.floor(template.cost[r] * trainNum);
common\tooltips.js:456:         totalCosts.time = Math.ceil(template.cost.time * (entity ? Engine.GuiInterfaceCall("GetBatchTime", {

session\menu.js:847:    barterAmount.Buy.caption = "+" + Math.round(prices.sell[g_BarterSell] / prices.buy[resourceCode] * amountToSell);
session\menu.js:596:                            modifiedTemplate.cost[res] = Math.floor(GetSimState().players[i].spyCostMultiplier * template.cost[res]);
session\menu.js:607:                                    modifiedTemplate.cost[res] = Math.floor(costRatio * modifiedTemplate.cost[res]);

session\selection_panels_helpers.js:48: return "color:255 0 0 " + Math.min(125, Math.round(+totalCost / 10) + 50);

session\session.js:1221:                Engine.GetGUIObjectByName("resource[" + r + "]_count").caption = Math.floor(viewedPlayerState.resourceCounts[res]);
session\session.js:1167:                                Math.round(playerStates[player].resourceCounts[resource]),
session\session.js:1169:                                Math.round(playerStates[player].resourceCounts[resource])
session\session.js:1281:                size.top = size.left + Math.round(researchStarted[tech].progress * (size.right - size.left));
session\session.js:1655:                playerStatistics.militaryScore += Math.round((player.sequences.enemyUnitsKilledValue[maxIndex] +
session\session.js:1657:                playerStatistics.totalScore += (total + Math.round((player.sequences.enemyUnitsKilledValue[maxIndex] +

session\selection_details.js:115:                       "hitpoints": Math.ceil(entState.hitpoints),
session\selection_details.js:116:                       "maxHitpoints": Math.ceil(entState.maxHitpoints)
session\selection_details.js:154:                       "capturePoints": Math.ceil(entState.capturePoints[entState.player]),
session\selection_details.js:155:                       "maxCapturePoints": Math.ceil(entState.maxCapturePoints)
session\selection_details.js:175:                               "current": Math.floor(entState.promotion.curr),
session\selection_details.js:181:                               "current": Math.floor(entState.promotion.curr)
session\selection_details.js:191:                               "amount": Math.ceil(+entState.resourceSupply.amount),


summary\counters.js:130:        return Math.round(total / 10);
summary\counters.js:135:        return Math.round((playerState.sequences.enemyUnitsKilledValue[index] +

summary\counters.js:5:  return { "percent": divisor ? Math.floor(100 * divident / divisor) : 0 };

pregame\userreport\userreport.js:30:    "sending": data => sprintf(translate("uploading (%f%%)"), Math.floor(100 * data[1])),

I am not sure if we want to unify these under one function:

session\input.js:1501:          buildingsCountToTrainFullBatch = Math.floor(canBeAddedCount / nextBatchTrainingCount);
session\input.js:1517:          let buildingsCountToTrainFullBatch = Math.floor( g_BatchTrainingEntityAllowedCount / batchedSize);
loading\loading.js:86:  let increment = Math.round(progress * middleLength / 100);
session\selection_panels.js:602:                size.top = size.left + Math.round(queuedItem.progress * (size.right - size.left));
session\selection_panels.js:1124:                       size.top = size.left + Math.round(progress * (size.right - size.left));

And probably dont want to change:

session\input.js:175:                           var averageRange = Math.round(Engine.GuiInterfaceCall("GetAverageRangeForBuildings", cmd) - cmd.range);
session\input.js:176:                           var range = Math.round(cmd.range);
session\input.js:1363:  return Math.max(Math.round(g_BatchSize), 1);
Silier removed a reviewer: Restricted Owners Package.Nov 17 2019, 5:51 PM
Silier planned changes to this revision.Jan 9 2020, 8:00 AM
Silier retitled this revision from Round value of capture attack to Implement wrapper around displaying numbers in gui.Jul 23 2020, 3:35 PM
Silier mentioned this in D3460: Limit gather rate tooltip to 2 decimal places.Jan 24 2021, 10:10 AM
wraitii added a subscriber: wraitii.EditedJan 24 2021, 11:06 AM

I had a thought on this, basically 'rounding for humans':

  • As an upper limit, we never really need more than 2 significant digits for the decimal places: 0.25 is better than 0.246
  • However, we probably want 2 significant digits at least: 0.25 is also better than 0.3 or 0.2 (see gather rates), but we also likely want 0.0023 over 0.01 (possibly for such low numbers, just 1 significant digit, but I don't think we should skip the leading zeros)
  • But we're not scientists and we don't care about 'insignificant' significant digits: 0.1 is generally better than 0.10, a fortiori 1 is better than 1.00
    • This is debatable to an extent, and we might prefer 1.0 over 1 or 1.00 if all other related numbers have decimal places, just for visual coherence.
  • We probably don't want to round integers: 12455 over 12000, even though it's also 2 significant digits.
  • But we probably no longer care about decimals at that point: 124 over 124.2
    • I would say above 10, only 1 decimal place at most instead of 2, and above 100, none: 1.23 over 1.2, 12.6 over 12.57.
  • Multiples of .25 are probably readable enough compared to rounding to the upper/lower digit, e.g. 3.75 over 3.8 even in a "1 digit" context. Likewise, 24.25 over 24.3
    • Also debatable
  • Never round non-zero to zero.

The parameters would be # of significant digits (most likely 2 is good enough), rounding (up/down/closest) and possibly forcing a decimal zero in some cases. I would suggest those cases:

  • < 0.1, possibly show 1 significant digit: 0.02 over 0.018
  • < 1, show 2 significant digits, remove trailing zeros: 0.95, 0.7
  • < 10, show 2 digits in the decimal places, remove trailing zeros: 1.23, 5.7, but not 1.0032, just 1.0 or 1 (depending on 'forced decimal zero').
  • < 100 (possibly a lower value?), show 1 decimal digit, except for .25 and .75: 10.25, 62.4, 23 (or 23.0 if 'forced decimal zero'), but not 23.53
  • After that, show no decimal place in any case, just print the integer, rounded up or down depending on the application (HP up, gather rates down): 196, 18529
    • Ideally we'd implement a thousand separator.

For very small numbers, most likely we just want to multiply the number by some arbitrary value (e.g. show milliseconds instead of seconds, minutes instead of seconds...), so I think it's fine if the algo handles that simply in the general case

Freagarach added a subscriber: Freagarach.Feb 24 2021, 4:52 PM

Also #6063:

function formatNumber(amount)
{
	let result = amount;
	if (amount > 1000000)
		result = Math.floor(amount / 1000000) + translateWithContext("symbol to identify a million", "M");
	else if (amount > 10000)
		result = Math.floor(amount / 1000) + translateWithContext("symbol to identify a thousand", "k");
	return result;
}
Freagarach mentioned this in D3657: Fixed round typo in selection_details.js.Mar 9 2021, 11:02 AM
Freagarach mentioned this in D4332: Removing decimal points for insufficient resource warnings.Dec 17 2021, 5:43 PM
Silier abandoned this revision.Apr 20 2023, 4:51 PM

Revision Contents
Changeset List

PathSize
binaries/
data/
mods/
public/
gui/
common/
2 lines

Diff 8764

View Options

binaries/data/mods/public/gui/common/tooltips.js

Loading...
Wildfire Games · Phabricator