Changeset View
Standalone View
binaries/data/mods/public/gui/summary/summary.js
Context not available. | |||||
"type": [0, 0] | "type": [0, 0] | ||||
}; | }; | ||||
bb: spaces around `+` | |||||
// Remember current selected view panel | |||||
Not Done Inline ActionsThis system is driving on the fact there is exactly one such panel. It would be better to make a g_OtherPanels = ["charts"] in layout.js and concat that one with the object.keys array. from that we can then work our way to get the wanted behaviour (going to the next, except when last tab the first) bb: This system is driving on the fact there is exactly one such panel. It would be better to make… | |||||
bbUnsubmitted Not Done Inline Actionsmake jsDoc and period bb: make jsDoc and period | |||||
var g_ViewPanel = ""; | |||||
Done Inline Actionsthis is the 5st time the same array of object keys is used, perhaps store it? bb: this is the 5st time the same array of object keys is used, perhaps store it? | |||||
Not Done Inline Actionsnot sure +i+1 and +i-1 must be with spaces +i + 1 and +i - 1 ffffffff: not sure +i+1 and +i-1 must be with spaces +i + 1 and +i - 1 | |||||
Not Done Inline Actionsmeh ok like this bb: meh ok like this | |||||
Not Done Inline Actionscurrently elexis: currently | |||||
Not Done Inline Actions@elexis reload the page bb: @elexis reload the page | |||||
Not Done Inline ActionsWith the exception of typed arrays, there is only one JS number type I'm aware of and that's called number. The comment implies the type, so the type definition could be avoided too. elexis: With the exception of typed arrays, there is only one JS number type I'm aware of and that's… | |||||
function nextPanel(backward = false) | |||||
{ | |||||
let panels = Object.keys(g_ScorePanelsData).concat(["charts"]); | |||||
let panel = panels[0]; | |||||
if (g_ViewPanel != "") | |||||
bbUnsubmitted Not Done Inline ActionsNotice this check is correct, but not required, however its good for optimization I guess, so meh => just keep the check bb: Notice this check is correct, but not required, however its good for optimization I guess, so… | |||||
for (let i in panels) | |||||
bbUnsubmitted Not Done Inline Actionslet i = 0; i < panels.length; ++i ? (That way the +i below can become just i) bb: let i = 0; i < panels.length; ++i ?
(That way the `+i` below can become just `i`) | |||||
Not Done Inline Actionsmaybe is correct, maybe not: why do we need to have g_DefaultPanel? and can't we just set here g_ViewPanel = "scorePanelButton" and then just let the init overwrite that when it wants to (like it already does). bb: maybe is correct, maybe not: why do we need to have `g_DefaultPanel`? and can't we just set… | |||||
if (panels[i] + "PanelButton" == g_ViewPanel) | |||||
{ | |||||
Not Done Inline Actions= undefined; should be removed (we got a linter rule) elexis: `= undefined;` should be removed (we got a linter rule) | |||||
i = backward ? +i-1 : +i+1; | |||||
bbUnsubmitted Not Done Inline Actionsmaybe rename to j or so, to avoid confusion bb: maybe rename to j or so, to avoid confusion | |||||
Not Done Inline ActionsShow next/previous panel.? bb: Show next/previous panel.? | |||||
i = i < 0 ? panels.length - 1 : i >= panels.length ? 0 : i; | |||||
bbUnsubmitted Not Done Inline Actionsseems to be no need for this => inline bb: seems to be no need for this => inline | |||||
Not Done Inline Actionsnuke [] around direction bb: nuke [] around `direction` | |||||
panel = panels[i]; | |||||
bbUnsubmitted Not Done Inline Actionseven when we declare let j = 0; instead of let panel = panels[0]; on line 134, this line can be inlined in line 145 bb: even when we declare `let j = 0;` instead of `let panel = panels[0];` on line 134, this line… | |||||
break; | |||||
Not Done Inline Actionsnuke the optionality in the argument, just make it a required one and put the 1 in the call bb: nuke the optionality in the argument, just make it a required one and put the `1` in the call | |||||
Not Done Inline ActionsIt is a js disgrace to take the remainder as negative for negative values imo, (index + direction + panels.length) % panels.length bb: It is a js disgrace to take the remainder as negative for negative values imo, `(index +… | |||||
Not Done Inline Actionssee previous comment bb: see previous comment | |||||
} | |||||
selectPanel(Engine.GetGUIObjectByName(panel + "PanelButton")); | |||||
} | |||||
Not Done Inline ActionsCorrect since if g_ViewPanel == "" then slicing just remains "" and that is not an element of panels. (Noting for self, no change needed) bb: Correct since if `g_ViewPanel == ""` then slicing just remains `""` and that is not an element… | |||||
Not Done Inline ActionsAww well (index + direction) % panels.length means (index + direction) modulo panels.length So that returns the remainder of dividing (index + direction) by panels.length, in the range [0, panels.length - 1]. So -1 % panels.length returns panels.length - 1 thus so we can remove the second ternary. Also nextIndex is used only once so inline. bb: Aww well `(index + direction) % panels.length` means `(index + direction) modulo panels.length`… | |||||
Not Done Inline Actionsno -1 % panels.length = -1 ffffffff: no
-1 % panels.length = -1 | |||||
function selectPanel(panel) | function selectPanel(panel) | ||||
Not Done Inline Actionswhat about index = panels.indexOf(g_ViewPanel.slice(0, -"PanelButton".length)); bb: what about `index = panels.indexOf(g_ViewPanel.slice(0, -"PanelButton".length));` | |||||
{ | { | ||||
// TODO: move panel buttons to a custom parent object | // TODO: move panel buttons to a custom parent object | ||||
Not Done Inline Actionsnewline before bb: newline before | |||||
Not Done Inline ActionsAnd with all that i can and inlined bb: And with all that `i` can and inlined | |||||
Not Done Inline Actionsmaking the function a nice short two liner bb: making the function a nice short two liner | |||||
Not Done Inline Actionsthat is looping twice over the same thing, first in the map then the for. Move the map into the for scope so: for (let button of g_PanelsButton) { tab = Engine.GetGUIObjectByName(button); // (or find a better name for tabObject) blabla() } bb: that is looping twice over the same thing, first in the map then the for. Move the map into the… | |||||
Not Done Inline Actionsagree elexis: agree | |||||
Not Done Inline Actions\0/ that has annoyed me a lot bb: \0/ that has annoyed me a lot | |||||
Context not available. | |||||
updatePanelData(g_ScorePanelsData[panel.name.substr(0, panel.name.length - "PanelButton".length)]); | updatePanelData(g_ScorePanelsData[panel.name.substr(0, panel.name.length - "PanelButton".length)]); | ||||
else | else | ||||
[0, 1].forEach(updateCategoryDropdown); | [0, 1].forEach(updateCategoryDropdown); | ||||
g_ViewPanel = panel.name; | |||||
} | } | ||||
function initCharts() | function initCharts() | ||||
Not Done Inline Actions{ "bbLint": "on", "needsFix": true, "errorType": "objectSpaces" } bb: { "bbLint": "on", "needsFix": true, "errorType": "objectSpaces" } | |||||
Context not available. | |||||
function continueButton() | function continueButton() | ||||
{ | { | ||||
if (g_GameData.gui.isInGame) | if (g_GameData.gui.isInGame) | ||||
Engine.PopGuiPageCB(0); | Engine.PopGuiPageCB({ "explicitResume": 0, "viewPanel": g_ViewPanel }); | ||||
else if (g_GameData.gui.isReplay) | else if (g_GameData.gui.isReplay) | ||||
Engine.SwitchGuiPage("page_replaymenu.xml", { | Engine.SwitchGuiPage("page_replaymenu.xml", { | ||||
"replaySelectionData": g_GameData.gui.replaySelectionData | "replaySelectionData": g_GameData.gui.replaySelectionData, | ||||
"summaryViewPanel": g_ViewPanel | |||||
}); | }); | ||||
else if (Engine.HasXmppClient()) | else if (Engine.HasXmppClient()) | ||||
Engine.SwitchGuiPage("page_lobby.xml"); | Engine.SwitchGuiPage("page_lobby.xml"); | ||||
Context not available. | |||||
g_GameData = data; | g_GameData = data; | ||||
let assignedState = g_GameData.sim.playerStates[g_GameData.gui.assignedPlayer || -1]; | let assignedState = g_GameData.sim.playerStates[g_GameData.gui.assignedPlayer || -1]; | ||||
if (data && data.viewPanel) | |||||
g_ViewPanel = data.viewPanel; | |||||
Engine.GetGUIObjectByName("summaryText").caption = | Engine.GetGUIObjectByName("summaryText").caption = | ||||
g_GameData.gui.isInGame ? | g_GameData.gui.isInGame ? | ||||
translate("Current Scores") : | translate("Current Scores") : | ||||
Not Done Inline Actionsk => button bb: `k` => `button` | |||||
Not Done Inline Actionsnope bb: nope | |||||
Context not available. | |||||
calculateTeamCounterDataHelper(); | calculateTeamCounterDataHelper(); | ||||
initCharts(); | initCharts(); | ||||
selectPanel(Engine.GetGUIObjectByName("scorePanelButton")); | selectPanel(Engine.GetGUIObjectByName(g_ViewPanel != "" ? g_ViewPanel : "scorePanelButton")); | ||||
bbUnsubmitted Not Done Inline Actions"scorePanelButton" looks like a magical value to me => put it in a global g_DefaultPanel bb: "scorePanelButton" looks like a magical value to me => put it in a global `g_DefaultPanel` | |||||
Not Done Inline Actionswould be better to swap negate the statement and swap the cases bb: would be better to swap negate the statement and swap the cases
`g_ViewPanel == "" ? | |||||
Not Done Inline Actionswith removing g_DefaultPanel this would simplify to selectPanel(Engine.GetGUIObjectByName(g_ViewPanel)); bb: with removing `g_DefaultPanel` this would simplify to `selectPanel(Engine.GetGUIObjectByName… | |||||
} | } | ||||
Context not available. | |||||
Not Done Inline Actionssome more juice here. ffffffff: some more juice here.
go next/prev panel on mouse scroll wheel up/down over the tab buttons. | |||||
Not Done Inline ActionsThat should be documented too in the intro.txt Note to self: add things to the wiki when committed bb: That should be documented too in the intro.txt
Note to self: add things to the wiki when… | |||||
Not Done Inline ActionsAs we use this thing twice now Object.keys(g_ScorePanelsData).concat(["charts"]) make it a global, also do Object.keys(g_ScorePanelsData).push("charts") bb: As we use this thing twice now `Object.keys(g_ScorePanelsData).concat(["charts"])` make it a… | |||||
Not Done Inline Actionscant use push when afterwards wanna use another function on return cause push doesnt return an array. can concat stay? ffffffff: cant use push when afterwards wanna use another function on return cause push doesnt return an… | |||||
Not Done Inline ActionsWhen being in the little space between the buttons, the scrolling doesn't work, probably just meh, since it would run in some inconsistencies when we allow it. bb: When being in the little space between the buttons, the scrolling doesn't work, probably just… | |||||
Not Done Inline ActionsNice, didn't notice so far. Would also be useful in the other GUI pages with tabs and consistent. elexis: Nice, didn't notice so far. Would also be useful in the other GUI pages with tabs and… |
spaces around +