Casual players may not care for the countdown timers or the stress they may cause, so let's have an option to disable them. (See https://code.wildfiregames.com/rP21121#29216)
Details
- Reviewers
mimo
The build/repair tooltips should be changed as well. They could say "Number of builders/repairers" (without showing the number, since that's displayed in the text area now), and the the speed-up calculation could be removed since casual players aren't interested in time? But I'm open to suggestions.
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 5495 Build 9285: Vulcan Build Jenkins Build 9284: arc lint + arc unit
Event Timeline
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Default... Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (curly): | | Unnecessary { after 'if' condition. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/selection_panels.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/selection_panels.js | 462| 462| continue; | 463| 463| | 464| 464| if (state.pack.progress == 0) | 465| |- { | | 465|+ | 466| 466| if (state.pack.packed) | 467| 467| checks.unpackButton = true; | 468| 468| else | 469| 469| checks.packButton = true; | 470| |- } | | 470|+ | 471| 471| else if (state.pack.packed) | 472| 472| checks.unpackCancelButton = true; | 473| 473| else | | [NORMAL] ESLintBear (space-before-function-paren): | | Unexpected space before function parentheses. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/selection_panels.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/selection_panels.js | 777| 777| addResearchToQueue(data.item.researchFacilityId, tech); | 778| 778| }; | 779| 779| | 780| |- button.onPressRight = function () { | | 780|+ button.onPressRight = function() { | 781| 781| let researcherTemplate; | 782| 782| for (let selectedEntity of data.unitEntStates) | 783| 783| if (selectedEntity.id == data.item.researchFacilityId) binaries/data/mods/public/gui/session/selection_panels.js | 48| » » » switch·(data.item) | | [NORMAL] ESLintBear (default-case): | | Expected a default case. binaries/data/mods/public/gui/session/selection_panels.js | 59| » » switch·(data.item) | | [NORMAL] ESLintBear (default-case): | | Expected a default case. binaries/data/mods/public/gui/session/selection_panels.js | 745| » » » » » » switch·(entity.check) | | [NORMAL] ESLintBear (default-case): | | Expected a default case. | | [NORMAL] ESLintBear (indent): | | Expected indentation of 6 tabs but found 5. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | 414| 414| // Players see colors depending on diplomacy | 415| 415| g_DisplayedPlayerColors[i] = | 416| 416| g_ViewedPlayer == i ? g_DiplomacyColorPalette.Self : | 417| |- g_Players[g_ViewedPlayer].isAlly[i] ? g_DiplomacyColorPalette.Ally : | | 417|+ g_Players[g_ViewedPlayer].isAlly[i] ? g_DiplomacyColorPalette.Ally : | 418| 418| g_Players[g_ViewedPlayer].isNeutral[i] ? g_DiplomacyColorPalette.Neutral : | 419| 419| g_DiplomacyColorPalette.Enemy; | 420| 420| | | [NORMAL] ESLintBear (indent): | | Expected indentation of 7 tabs but found 5. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | 415| 415| g_DisplayedPlayerColors[i] = | 416| 416| g_ViewedPlayer == i ? g_DiplomacyColorPalette.Self : | 417| 417| g_Players[g_ViewedPlayer].isAlly[i] ? g_DiplomacyColorPalette.Ally : | 418| |- g_Players[g_ViewedPlayer].isNeutral[i] ? g_DiplomacyColorPalette.Neutral : | | 418|+ g_Players[g_ViewedPlayer].isNeutral[i] ? g_DiplomacyColorPalette.Neutral : | 419| 419| g_DiplomacyColorPalette.Enemy; | 420| 420| | 421| 421| g_DisplayedPlayerColors[0] = g_Players[0].color; | | [NORMAL] ESLintBear (indent): | | Expected indentation of 8 tabs but found 5. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | 416| 416| g_ViewedPlayer == i ? g_DiplomacyColorPalette.Self : | 417| 417| g_Players[g_ViewedPlayer].isAlly[i] ? g_DiplomacyColorPalette.Ally : | 418| 418| g_Players[g_ViewedPlayer].isNeutral[i] ? g_DiplomacyColorPalette.Neutral : | 419| |- g_DiplomacyColorPalette.Enemy; | | 419|+ g_DiplomacyColorPalette.Enemy; | 420| 420| | 421| 421| g_DisplayedPlayerColors[0] = g_Players[0].color; | 422| 422| } | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | 661| 661| "civ": setStringTags(g_CivData[g_Players[g_ViewedPlayer].civ].Name, { "font": "sans-bold-stroke-14" }), | 662| 662| "hotkey_civinfo": colorizeHotkey("%(hotkey)s", "civinfo"), | 663| 663| "hotkey_structree": colorizeHotkey("%(hotkey)s", "structree") | 664| |- }); | | 664|+ }); | 665| 665| } | 666| 666| | 667| 667| Engine.GetGUIObjectByName("optionFollowPlayer").hidden = !g_IsObserver || | | [NORMAL] ESLintBear (indent): | | Expected indentation of 3 tabs but found 2. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js |1258|1258| |1259|1259| let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" : |1260|1260| "\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), { |1261| |- "hotkey": setStringTags("\\[Click]", g_HotkeyTags), | |1261|+ "hotkey": setStringTags("\\[Click]", g_HotkeyTags), |1262|1262| "order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending") |1263|1263| }); |1264|1264| | | [NORMAL] ESLintBear (indent): | | Expected indentation of 3 tabs but found 2. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js |1259|1259| let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" : |1260|1260| "\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), { |1261|1261| "hotkey": setStringTags("\\[Click]", g_HotkeyTags), |1262| |- "order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending") | |1262|+ "order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending") |1263|1263| }); |1264|1264| |1265|1265| let resCodes = g_ResourceData.GetCodes(); | | [NORMAL] ESLintBear (indent): | | Expected indentation of 2 tabs but found 1. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js |1260|1260| "\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), { |1261|1261| "hotkey": setStringTags("\\[Click]", g_HotkeyTags), |1262|1262| "order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending") |1263| |- }); | |1263|+ }); |1264|1264| |1265|1265| let resCodes = g_ResourceData.GetCodes(); |1266|1266| for (let r = 0; r < resCodes.length; ++r) | | [NORMAL] ESLintBear (indent): | | Expected indentation of 1 tab but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js |1749|1749| for (let rct of resourcesCounterTypes) |1750|1750| for (let rt of resourcesTypes) |1751|1751| reportObject[rt + rct.substr(9)] = playerStatistics[rct][rt]; |1752| |- // eg. rt = food rct.substr = Gathered rct = resourcesGathered | |1752|+ // eg. rt = food rct.substr = Gathered rct = resourcesGathered |1753|1753| |1754|1754| reportObject.vegetarianFoodGathered = playerStatistics.resourcesGathered.vegetarianFood; |1755|1755| for (let type of unitsClasses) binaries/data/mods/public/gui/session/session.js |1086| » let·getPanelEntNameTooltip·=·panelEntState·=>·"[font=\"sans-bold-16\"]"·+·template.name.specific·+·"[/font]"; | | [NORMAL] ESLintBear (no-shadow): | | 'panelEntState' is already declared in the upper scope. binaries/data/mods/public/gui/session/session.js |1161| » » button.onpress·=·(function(i)·{·return·function()·{·performGroup((Engine.HotkeyIsPressed("selection.add")·?·"add"·:·"select"),·i);·};·})(i); | | [NORMAL] ESLintBear (no-shadow): | | 'i' is already declared in the upper scope. binaries/data/mods/public/gui/session/session.js |1162| » » button.ondoublepress·=·(function(i)·{·return·function()·{·performGroup("snap",·i);·};·})(i); | | [NORMAL] ESLintBear (no-shadow): | | 'i' is already declared in the upper scope. binaries/data/mods/public/gui/session/session.js |1163| » » button.onpressright·=·(function(i)·{·return·function()·{·performGroup("breakUp",·i);·};·})(i); | | [NORMAL] ESLintBear (no-shadow): | | 'i' is already declared in the upper scope.
Link to build: https://jenkins.wildfiregames.com/job/differential/223/display/redirect
(Wondering if a showCountdownTimers() would make it more readable, but probably should remain consistent)
binaries/data/config/default.cfg | ||
---|---|---|
30 | Should be in [gui.session]. If the detailed tooltips is only seen in the session it should be there too, otherwise in gui`. |
Although i stressed in the discussion you linked that such a change would be useful, i'm wondering if adding such a specific option (countdown timers) is really what is needed. We should take the time of thinking about what we want and how: my view was to design two ways of displaying information, one most aimed at competitive players and one for others. Countdown are certainly part of it, but there may be other displayed information which could benefit from such an option.
In the meantime (i.e. as long as we don't have a clear idea of how this option would work best), and for what was the subject of the discussion, my main point was that, if for competitive gamers the remaining time is the important information, for most others i believe the number of builders is the relevant one. So displaying both would be a solution. Something like " 0:21 / 6 " when 6 builders instead of the current " 0:21 " would have all information.
Then, if we go with the patch as it is, option countdown should show the remaining time, and the number of builders and modified time be in the tooltip, while option nocountdown would show the number of builders with both times in the tooltip. I don't think we should remove any information, it's only how to show it and which one should be highlighted.
Concerning tooltips, they would have to be changed anyway. Currently the tooltip does not describe what is in the panel: it displays the remaining time while the tooltip says "number of builders n". It should first explain what is shown, and then add the additionnal information.
There is detailed tooltips, which is a similar idea. It hides information so the player isn't overwhelmed.
In the meantime (i.e. as long as we don't have a clear idea of how this option would work best), and for what was the subject of the discussion, my main point was that, if for competitive gamers the remaining time is the important information, for most others i believe the number of builders is the relevant one. So displaying both would be a solution. Something like " 0:21 / 6 " when 6 builders instead of the current " 0:21 " would have all information.
The argument that resonated with me was the stress of having timers everywhere. Casual players might not like D1300, for example.
Then, if we go with the patch as it is, option countdown should show the remaining time, and the number of builders and modified time be in the tooltip, while option nocountdown would show the number of builders with both times in the tooltip. I don't think we should remove any information, it's only how to show it and which one should be highlighted.
Concerning tooltips, they would have to be changed anyway. Currently the tooltip does not describe what is in the panel: it displays the remaining time while the tooltip says "number of builders n". It should first explain what is shown, and then add the additionnal information.
I don't think we need to explain the countdown timer, it's pretty obvious what's happening.
binaries/data/config/default.cfg | ||
---|---|---|
30 | gui.session.ceasefirecounter but apparently it's not in default.cfg. I'll investigate. |
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Default... Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (curly): | | Unnecessary { after 'if' condition. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/selection_panels.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/selection_panels.js | 462| 462| continue; | 463| 463| | 464| 464| if (state.pack.progress == 0) | 465| |- { | | 465|+ | 466| 466| if (state.pack.packed) | 467| 467| checks.unpackButton = true; | 468| 468| else | 469| 469| checks.packButton = true; | 470| |- } | | 470|+ | 471| 471| else if (state.pack.packed) | 472| 472| checks.unpackCancelButton = true; | 473| 473| else | | [NORMAL] ESLintBear (space-before-function-paren): | | Unexpected space before function parentheses. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/selection_panels.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/selection_panels.js | 778| 778| addResearchToQueue(data.item.researchFacilityId, tech); | 779| 779| }; | 780| 780| | 781| |- button.onPressRight = function () { | | 781|+ button.onPressRight = function() { | 782| 782| let researcherTemplate; | 783| 783| for (let selectedEntity of data.unitEntStates) | 784| 784| if (selectedEntity.id == data.item.researchFacilityId) binaries/data/mods/public/gui/session/selection_panels.js | 48| » » » switch·(data.item) | | [NORMAL] ESLintBear (default-case): | | Expected a default case. binaries/data/mods/public/gui/session/selection_panels.js | 59| » » switch·(data.item) | | [NORMAL] ESLintBear (default-case): | | Expected a default case. binaries/data/mods/public/gui/session/selection_panels.js | 746| » » » » » » switch·(entity.check) | | [NORMAL] ESLintBear (default-case): | | Expected a default case. | | [NORMAL] ESLintBear (indent): | | Expected indentation of 6 tabs but found 5. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | 414| 414| // Players see colors depending on diplomacy | 415| 415| g_DisplayedPlayerColors[i] = | 416| 416| g_ViewedPlayer == i ? g_DiplomacyColorPalette.Self : | 417| |- g_Players[g_ViewedPlayer].isAlly[i] ? g_DiplomacyColorPalette.Ally : | | 417|+ g_Players[g_ViewedPlayer].isAlly[i] ? g_DiplomacyColorPalette.Ally : | 418| 418| g_Players[g_ViewedPlayer].isNeutral[i] ? g_DiplomacyColorPalette.Neutral : | 419| 419| g_DiplomacyColorPalette.Enemy; | 420| 420| | | [NORMAL] ESLintBear (indent): | | Expected indentation of 7 tabs but found 5. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | 415| 415| g_DisplayedPlayerColors[i] = | 416| 416| g_ViewedPlayer == i ? g_DiplomacyColorPalette.Self : | 417| 417| g_Players[g_ViewedPlayer].isAlly[i] ? g_DiplomacyColorPalette.Ally : | 418| |- g_Players[g_ViewedPlayer].isNeutral[i] ? g_DiplomacyColorPalette.Neutral : | | 418|+ g_Players[g_ViewedPlayer].isNeutral[i] ? g_DiplomacyColorPalette.Neutral : | 419| 419| g_DiplomacyColorPalette.Enemy; | 420| 420| | 421| 421| g_DisplayedPlayerColors[0] = g_Players[0].color; | | [NORMAL] ESLintBear (indent): | | Expected indentation of 8 tabs but found 5. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | 416| 416| g_ViewedPlayer == i ? g_DiplomacyColorPalette.Self : | 417| 417| g_Players[g_ViewedPlayer].isAlly[i] ? g_DiplomacyColorPalette.Ally : | 418| 418| g_Players[g_ViewedPlayer].isNeutral[i] ? g_DiplomacyColorPalette.Neutral : | 419| |- g_DiplomacyColorPalette.Enemy; | | 419|+ g_DiplomacyColorPalette.Enemy; | 420| 420| | 421| 421| g_DisplayedPlayerColors[0] = g_Players[0].color; | 422| 422| } | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | 661| 661| "civ": setStringTags(g_CivData[g_Players[g_ViewedPlayer].civ].Name, { "font": "sans-bold-stroke-14" }), | 662| 662| "hotkey_civinfo": colorizeHotkey("%(hotkey)s", "civinfo"), | 663| 663| "hotkey_structree": colorizeHotkey("%(hotkey)s", "structree") | 664| |- }); | | 664|+ }); | 665| 665| } | 666| 666| | 667| 667| Engine.GetGUIObjectByName("optionFollowPlayer").hidden = !g_IsObserver || | | [NORMAL] ESLintBear (indent): | | Expected indentation of 3 tabs but found 2. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js |1258|1258| |1259|1259| let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" : |1260|1260| "\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), { |1261| |- "hotkey": setStringTags("\\[Click]", g_HotkeyTags), | |1261|+ "hotkey": setStringTags("\\[Click]", g_HotkeyTags), |1262|1262| "order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending") |1263|1263| }); |1264|1264| | | [NORMAL] ESLintBear (indent): | | Expected indentation of 3 tabs but found 2. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js |1259|1259| let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" : |1260|1260| "\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), { |1261|1261| "hotkey": setStringTags("\\[Click]", g_HotkeyTags), |1262| |- "order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending") | |1262|+ "order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending") |1263|1263| }); |1264|1264| |1265|1265| let resCodes = g_ResourceData.GetCodes(); | | [NORMAL] ESLintBear (indent): | | Expected indentation of 2 tabs but found 1. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js |1260|1260| "\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), { |1261|1261| "hotkey": setStringTags("\\[Click]", g_HotkeyTags), |1262|1262| "order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending") |1263| |- }); | |1263|+ }); |1264|1264| |1265|1265| let resCodes = g_ResourceData.GetCodes(); |1266|1266| for (let r = 0; r < resCodes.length; ++r) | | [NORMAL] ESLintBear (indent): | | Expected indentation of 1 tab but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/session.js |1750|1750| for (let rct of resourcesCounterTypes) |1751|1751| for (let rt of resourcesTypes) |1752|1752| reportObject[rt + rct.substr(9)] = playerStatistics[rct][rt]; |1753| |- // eg. rt = food rct.substr = Gathered rct = resourcesGathered | |1753|+ // eg. rt = food rct.substr = Gathered rct = resourcesGathered |1754|1754| |1755|1755| reportObject.vegetarianFoodGathered = playerStatistics.resourcesGathered.vegetarianFood; |1756|1756| for (let type of unitsClasses) binaries/data/mods/public/gui/session/session.js |1086| » let·getPanelEntNameTooltip·=·panelEntState·=>·"[font=\"sans-bold-16\"]"·+·template.name.specific·+·"[/font]"; | | [NORMAL] ESLintBear (no-shadow): | | 'panelEntState' is already declared in the upper scope. binaries/data/mods/public/gui/session/session.js |1161| » » button.onpress·=·(function(i)·{·return·function()·{·performGroup((Engine.HotkeyIsPressed("selection.add")·?·"add"·:·"select"),·i);·};·})(i); | | [NORMAL] ESLintBear (no-shadow): | | 'i' is already declared in the upper scope. binaries/data/mods/public/gui/session/session.js |1162| » » button.ondoublepress·=·(function(i)·{·return·function()·{·performGroup("snap",·i);·};·})(i); | | [NORMAL] ESLintBear (no-shadow): | | 'i' is already declared in the upper scope. binaries/data/mods/public/gui/session/session.js |1163| » » button.onpressright·=·(function(i)·{·return·function()·{·performGroup("breakUp",·i);·};·})(i); | | [NORMAL] ESLintBear (no-shadow): | | 'i' is already declared in the upper scope.
Link to build: https://jenkins.wildfiregames.com/job/differential/231/display/redirect