Changeset View
Standalone View
binaries/data/mods/public/gui/summary/summary.js
Context not available. | |||||
"type": [0, 0] | "type": [0, 0] | ||||
}; | }; | ||||
bb: spaces around `+` | |||||
/* | |||||
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… | |||||
Not Done Inline Actionsmake jsDoc and period bb: make jsDoc and period | |||||
* Show next/previous panel. | |||||
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 | |||||
* @param {integer} direction - 1/-1 forward, backward panel. | |||||
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(direction) | |||||
{ | |||||
let index = g_PanelButtons.indexOf(g_ViewPanel); | |||||
selectPanel(Engine.GetGUIObjectByName(g_PanelButtons[index == -1 ? 0 : index + direction < 0 ? | |||||
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… | |||||
g_PanelButtons.length -1 : (index + direction) % g_PanelButtons.length])); | |||||
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… | |||||
} | |||||
Not Done Inline Actions= undefined; should be removed (we got a linter rule) elexis: `= undefined;` should be removed (we got a linter rule) | |||||
function selectPanel(panel) | function selectPanel(panel) | ||||
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.? | |||||
{ | { | ||||
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` | |||||
// 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 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… | |||||
Not Done Inline Actionsrethinking this: using some modulo calculation and ternary would make the code cleaner so we get i = (g_ViewPanel == "" || index == -1) ? 0 : (index + direction) % panels.length; bb: rethinking this: using some modulo calculation and ternary would make the code cleaner so we… | |||||
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));` | |||||
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 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 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 | |||||
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 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; | |||||
} | } | ||||
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 | |||||
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 Actionsnope bb: nope | |||||
Context not available. | |||||
calculateTeamCounterDataHelper(); | calculateTeamCounterDataHelper(); | ||||
initCharts(); | initCharts(); | ||||
selectPanel(Engine.GetGUIObjectByName("scorePanelButton")); | selectPanel(Engine.GetGUIObjectByName(g_ViewPanel)); | ||||
Not Done Inline Actionsk => button bb: `k` => `button` | |||||
for (let panel of g_PanelButtons.map(k => Engine.GetGUIObjectByName(k))) | |||||
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… | |||||
panel.onMouseWheelUp = function() { | |||||
nextPanel(1); | |||||
}; | |||||
panel.onMouseWheelDown = function() { | |||||
nextPanel(-1); | |||||
}; | |||||
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… | |||||
} | |||||
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… | |||||
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… | |||||
} | } | ||||
Context not available. |
spaces around +