Changeset View
Standalone View
binaries/data/mods/public/gui/session/top_panel/CounterPopulation.js
/** | /** | ||||||||||
* This class manages the population counter in the top panel. | * This class manages the population counter in the top panel. | ||||||||||
* It flashes the counter if the training of any owned entity is blocked. | * It flashes the counter if the training of any owned entity is blocked. | ||||||||||
*/ | */ | ||||||||||
class CounterPopulation | class CounterPopulation | ||||||||||
{ | { | ||||||||||
constructor(resCode, panel, icon, count) | constructor(resCode, panel, icon, count, stats) | ||||||||||
Freagarach: Could you make the `extra` a bit more descriptive? | |||||||||||
{ | { | ||||||||||
this.resCode = resCode; | this.resCode = resCode; | ||||||||||
this.panel = panel; | this.panel = panel; | ||||||||||
this.icon = icon; | this.icon = icon; | ||||||||||
this.count = count; | this.count = count; | ||||||||||
this.count.onTick = this.onTick.bind(this); | this.count.onTick = this.onTick.bind(this); | ||||||||||
this.isTrainingBlocked = false; | this.isTrainingBlocked = false; | ||||||||||
this.color = this.DefaultPopulationColor; | this.color = this.DefaultPopulationColor; | ||||||||||
this.stats = stats; | |||||||||||
} | } | ||||||||||
rebuild(playerState, getAllyStatTooltip) | rebuild(playerState, getAllyStatTooltip) | ||||||||||
{ | { | ||||||||||
this.count.caption = sprintf(translate(this.CounterCaption), playerState); | this.count.caption = sprintf(translate(this.CounterCaption), playerState); | ||||||||||
let total = 0; | |||||||||||
Done Inline ActionsYou don't need the Math.floor, right? Also is the || "" needed? Freagarach: You don't need the `Math.floor`, right? Also is the `|| ""` needed? | |||||||||||
Done Inline ActionsTab indented, not spaces :) Freagarach: Tab indented, not spaces :) | |||||||||||
for (let resCode of g_ResourceData.GetCodes()) | |||||||||||
total += playerState.resourceStats[resCode].gatherers; | |||||||||||
this.stats.caption = total > 0 ? total : ""; // do not show zeroes | |||||||||||
Done Inline ActionsCan't you omit the caption when there is no total? Freagarach: Can't you omit the caption when there is no total? | |||||||||||
Done Inline ActionsLike: this.stats.caption = total > 0 ? total : ""; I must set it or it'll remains at the previous state, thus not resetting. I rather have no info than "0" gatherers under a resource I'm not collecting. mammadori: Like:
if (total > 0)
this.stats.caption = total > 0 ? total : "";
I must set it or it'll… | |||||||||||
Done Inline Actions
Although I would say one does need to show zeroes. Freagarach: Although I would say one does need to show zeroes. | |||||||||||
Done Inline ActionsLet's discuss that, AOE do not show zeroes; we may differ of course. IMHO: It clutter less the interface, it permits to recognize zero better (our brain has no need to read small font and recognize "0" symbol, something just isn't here, so it is easier to see). mammadori: Let's discuss that, AOE do not show zeroes; we may differ of course.
IMHO: It clutter less the… | |||||||||||
Done Inline ActionsSure, I'm no UX expert. Freagarach: Sure, I'm no UX expert. | |||||||||||
this.isTrainingBlocked = playerState.trainingBlocked; | this.isTrainingBlocked = playerState.trainingBlocked; | ||||||||||
this.panel.tooltip = | this.panel.tooltip = | ||||||||||
setStringTags(translate(this.PopulationTooltip), CounterManager.ResourceTitleTags) + "\n" + | setStringTags(translate(this.PopulationTooltip), CounterManager.ResourceTitleTags) + "\n" + | ||||||||||
sprintf(translate(this.MaximumPopulationTooltip), { "popCap": playerState.popMax }) + | sprintf(translate(this.MaximumPopulationTooltip), { "popCap": playerState.popMax }) + | ||||||||||
getAllyStatTooltip(this.getTooltipData.bind(this)); | getAllyStatTooltip(this.getTooltipData.bind(this)); | ||||||||||
} | } | ||||||||||
Done Inline ActionsSee e.g. sprintf(translate("%(label)s: %(garrisonLimit)s"), { "label": headerFont(translate("Garrison Limit")), "garrisonLimit": template.garrisonHolder.capacity }) from tooltips.js. Freagarach: See e.g.
``` sprintf(translate("%(label)s: %(garrisonLimit)s"), {
"label": headerFont… | |||||||||||
Done Inline ActionsActually the number displayed is not the number of units that can gather, but the number of units that are gathering. Nescio: Actually the number displayed is not the number of units that //can// gather, but the number of… | |||||||||||
Done Inline ActionsThose numbers are to mimic the AOE feature in the screenshot, which in my particular friend bubble in the 100% of the case the first question in all the first match in 0ad. Is it really so useful? Questionable, for sure, but I obviously like it ;-) mammadori: Those numbers are to mimic the AOE feature in the screenshot, which in my particular friend… | |||||||||||
Done Inline Actions"Current gatherers"? Idle worker count should be displayed at the idle unit button. Freagarach: "Current gatherers"?
Idle worker count should be displayed at the idle unit button. | |||||||||||
Done Inline ActionsWouldn't the total (i.e. active + idle) be more informative? Nescio: Wouldn't the total (i.e. active + idle) be more informative? | |||||||||||
Done Inline ActionsIt's more surprising, idle counter is in another place. Anyway it could be done, not strongly against. mammadori: It's more surprising, idle counter is in another place. Anyway it could be done, not strongly… | |||||||||||
getTooltipData(playerState, playername) | getTooltipData(playerState, playername) | ||||||||||
{ | { | ||||||||||
return { | return { | ||||||||||
"playername": playername, | "playername": playername, | ||||||||||
"statValue": sprintf(translate(this.AllyPopulationTooltip), playerState), | "statValue": sprintf(translate(this.AllyPopulationTooltip), playerState), | ||||||||||
"orderValue": playerState.popCount | "orderValue": playerState.popCount | ||||||||||
}; | }; | ||||||||||
Done Inline Actions
Silier: | |||||||||||
} | } | ||||||||||
onTick() | onTick() | ||||||||||
{ | { | ||||||||||
if (this.panel.hidden) | if (this.panel.hidden) | ||||||||||
return; | return; | ||||||||||
let newColor = this.isTrainingBlocked && Date.now() % 1000 < 500 ? | let newColor = this.isTrainingBlocked && Date.now() % 1000 < 500 ? | ||||||||||
Show All 24 Lines |
Could you make the extra a bit more descriptive?