Everytime someone wants to change the main menu, add an item, a lot of intransparent XML code has to be changed, with arguments in weird places, hardcoded number of menu items (requiring the author to keep multiple places in sync with the correct magic numbers).
This patch was written first in October 2017 in https://code.wildfiregames.com/D820#37188 already, but was never finished; for #5387 I changed it a bit.
Details
- Reviewers
- None
- Commits
- rP22854: Rewrite Main Menu.
- Trac Tickets
- #5387
Notice how one can currently trigger crashes by throwing a JS exception on "init".
Consider whether or not to use the class keyword, especially for those trival handlers that dont have prototype extensions.
Consider the use cases of modders and other code reworkers.
Consider the logical grouping of code.
Consider the filenaming, especially the casing.
Consider whether to create subfolders (for example background JS+XML in a separate folder).
Compare previous vs new main menu and notice the main menu buttons have like 4px less margin or something and consider who cares.
Ensure that menu unfolding has the same behavior as before.
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
binaries/data/mods/public/gui/pregame/MainMenuButtonHandler.js | ||
---|---|---|
17 ↗ | (On Diff #9554) | onPress = this.closeSubmenu won't work due to that being called with this = calling GUI object. |
binaries/data/mods/public/gui/pregame/MainMenuItems.js | ||
1 ↗ | (On Diff #9554) | Was thinking about making this a JS Object, but it is important that this is iterated correctly.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...in Even if the order was/is deterministic, for example insertion order or alphabetic order, it would be unintentional, because the order of items should be influenced by how the menu author wants. So { "singleplayer": { ... } } just wouldn't work. If it was insertion order, then a modder couldnt insert in between. Such an id could be added for modders, if a mod wants to insert a submenu just after "Singleplayer" for example. |
binaries/data/mods/public/gui/pregame/mainmenu.js | ||
41 ↗ | (On Diff #9554) | JS function with 2 statements pretending to be a class, but not visible as such to the reader on first sight, if it wasnt for the name and surroinding consistency. The idea of doing OOP everywhere is to stop people from writing procedural code, but this example invites people to write more procedural code. That's why I wonder whether the class keyword shouldn't be used eventually. Perhaps even now? |
binaries/data/mods/public/gui/pregame/mainmenu.xml | ||
14 ↗ | (On Diff #9554) | onTick needed some C++ patch to be assignable from JS IIRC |
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Source... source/gui/GUIManager.h | 47| class·CGUIManager | | [MAJOR] CPPCheckBear (syntaxError): | | Code 'classCGUIManager{' is invalid C code. Use --std or --language to configure the language. Executing section JS... | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 5. |----| | /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | 183| 183| "tooltip": translate("Exits the game."), | 184| 184| "onPress": () => { | 185| 185| messageBox( | 186| |- 400, 200, | | 186|+ 400, 200, | 187| 187| translate("Are you sure you want to quit 0 A.D.?"), | 188| 188| translate("Confirmation"), | 189| 189| [translate("No"), translate("Yes")], | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 5. |----| | /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | 184| 184| "onPress": () => { | 185| 185| messageBox( | 186| 186| 400, 200, | 187| |- translate("Are you sure you want to quit 0 A.D.?"), | | 187|+ translate("Are you sure you want to quit 0 A.D.?"), | 188| 188| translate("Confirmation"), | 189| 189| [translate("No"), translate("Yes")], | 190| 190| [null, Engine.Exit]); | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 5. |----| | /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | 185| 185| messageBox( | 186| 186| 400, 200, | 187| 187| translate("Are you sure you want to quit 0 A.D.?"), | 188| |- translate("Confirmation"), | | 188|+ translate("Confirmation"), | 189| 189| [translate("No"), translate("Yes")], | 190| 190| [null, Engine.Exit]); | 191| 191| } | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 5. |----| | /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | 186| 186| 400, 200, | 187| 187| translate("Are you sure you want to quit 0 A.D.?"), | 188| 188| translate("Confirmation"), | 189| |- [translate("No"), translate("Yes")], | | 189|+ [translate("No"), translate("Yes")], | 190| 190| [null, Engine.Exit]); | 191| 191| } | 192| 192| } | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 5. |----| | /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | 187| 187| translate("Are you sure you want to quit 0 A.D.?"), | 188| 188| translate("Confirmation"), | 189| 189| [translate("No"), translate("Yes")], | 190| |- [null, Engine.Exit]); | | 190|+ [null, Engine.Exit]); | 191| 191| } | 192| 192| } | 193| 193| ]; | | [NORMAL] ESLintBear (spaced-comment): | | Expected space or tab after '//' in comment. |----| | /zpool0/trunk/binaries/data/mods/public/gui/pregame/mainmenu.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/pregame/mainmenu.js | 13| 13| g_BackgroundHandler = new BackgroundHandler(pickRandom(g_BackgroundLayerData)); | 14| 14| g_SplashScreenHandler = new SplashScreenHandler(data, hotloadData); | 15| 15| | 16| |- //throw new Error("This will crash, not cool"); | | 16|+ // throw new Error("This will crash, not cool"); | 17| 17| | 18| 18| MusicHandler(); | 19| 19| VersionHandler(); Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/527/display/redirect
binaries/data/mods/public/gui/pregame/MainMenuButtonHandler.js | ||
---|---|---|
17 ↗ | (On Diff #9554) | I think using an an arrow function to wrap is is quite elegant and readable. Note that if it's perf-critical, an alternative could be this.closeSubmenu.bind(this) - using the Function#bind method. I don't know about SM and our version of it in particular, but on some engines that can be faster. |
binaries/data/mods/public/gui/pregame/SplashscreenHandler.js | ||
41 ↗ | (On Diff #9554) | I think an arrow function would suffice here. |
Parallax scrolling is not working (BackgroundHandler).
Open prelobby page from mainmenu hotkey doesn't work anymore.
binaries/data/mods/public/gui/pregame/MainMenuItems.js | ||
---|---|---|
1 ↗ | (On Diff #9554) | You can use Map to guarantee key:value order. |
binaries/data/mods/public/gui/pregame/mainmenu.js | ||
16 ↗ | (On Diff #9554) | Maybe is because the c++ init caller expects init to end but doesn't ? |
source/gui/GUIManager.h | ||
169 ↗ | (On Diff #9554) | Part of this diff? |
binaries/data/mods/public/gui/common/sprites.xml | ||
---|---|---|
11 ↗ | (On Diff #9554) | Doesn't that go against the future engine split or is that file in modmod as well ? |
binaries/data/mods/public/gui/pregame/BackgroundHandler.js | ||
12 ↗ | (On Diff #9554) | I thought that was slower than normal iteration aka for loop does this have any performance impact ? |
30 ↗ | (On Diff #9554) | Still ugly hardcoding (D1680) |
I am wondering about the naming Handler. For DeveloperOverlayManager in https://code.wildfiregames.com/D1928#79847 it was deemed improvable for instance.
Test plan:
Consider whether or not to use the class keyword, especially for those trival handlers that dont have prototype extensions.
That's what I still wonder, especially for the functions that only have 2-3 comments and appear like global procedures.
binaries/data/mods/public/gui/common/sprites.xml | ||
---|---|---|
11 ↗ | (On Diff #9554) | You mean #5366. If that split proposal was right, then yes, these two sprites should be moved to the "0ad" mod and the rest to the "pyrogenesis-gui" mod. |
binaries/data/mods/public/gui/pregame/BackgroundHandler.js | ||
12 ↗ | (On Diff #9554) | In case you refer to comments I did, I mostly mentioned it for the filter function because that creates a new array and in some cases we dont need a new array if we just want to skip some items in a loop. |
binaries/data/mods/public/gui/pregame/MainMenuButtonHandler.js | ||
17 ↗ | (On Diff #9554) | Thanks, I didn't know this trick yet. I will keep it in mind because this isn't be the last time where this has the unwanted reference. |
binaries/data/mods/public/gui/pregame/MainMenuItems.js | ||
1 ↗ | (On Diff #9554) | https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map
Same problem. |
binaries/data/mods/public/gui/pregame/ProductInformation.js | ||
19 ↗ | (On Diff #9554) | String change here, it's not WARNING anymore. |
binaries/data/mods/public/gui/pregame/SplashscreenHandler.js | ||
21 ↗ | (On Diff #9554) | hotloading is new here |
source/gui/GUIManager.h | ||
169 ↗ | (On Diff #9554) | That was related to the crash debugging (as this variable is the one it fails to read from upon CallFunction failure but points to undefined data). Now I dont know which other patch to insert it. |
binaries/data/mods/public/gui/pregame/MainMenuItems.js | ||
---|---|---|
1 ↗ | (On Diff #9554) | If the order is important, would an array of objects work? Keep it simple :) |
binaries/data/mods/public/gui/pregame/MainMenuItems.js | ||
---|---|---|
1 ↗ | (On Diff #9554) | That's why it is one currently. So the question is mostly whether I should insert "id": "singleplayer" despite that not being used but possibly used by modders to figure out where they want to insert a new menu item. So the mods would have to play dirty, since they'd rely on an assumption that they can't assume (that the "singleplayer" or whatever item wasnt removed by a mod). So if they already play dirty, they can also dirtily identify the element by comparing against the translated caption. (That again however might also have been changed by a mod). So it seems hypothetical, and perhaps a mod should just be recommended to append or insert at a semi-random position. |
190 ↗ | (On Diff #9554) | -1 tab |
source/gui/GUIManager.h | ||
169 ↗ | (On Diff #9554) | Just for the record, the const doesn't help for the mentioned crash. |
source/gui/GUIManager.h | ||
---|---|---|
169 ↗ | (On Diff #9554) | What if it wasn't initialized at all ? |
Thanks for finding that, I didnt even notice!
It's because I used lastTick instead of firstTick (g_T0) in the offset computation.
Open prelobby page from mainmenu hotkey doesn't work anymore.
Ah, rebase error!
I've checked all hotkeys in https://code.wildfiregames.com/source/0ad/browse/ps/trunk/binaries/data/mods/public/gui/pregame/mainmenu.xml and they are there.
Note to self: go through all commits of the last years again.
However I never committed the patch to assign hotkeys from JS.
So this patch is actually blocked by that, as the XML objects are now anonymous placeholders under a <repeat> so cant have the hotkey set from XML.
Good. That was on the commitlist anyhow.
Thanks for that review nani!
I've actually had to move the welcome-screen PushGUIPage call back to the ugly onTick due to that crash since ages: rP13597 (#5578) and it isn't trivial how to fix it.
Add missing lobby hotkey and fix background handler scrolling as reported by nani.
Revert to ugly onTick hack from rP13597.
Fix Splashscreen hotloading.
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/61/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Source... source/gui/GUIManager.h | 47| class·CGUIManager | | [MAJOR] CPPCheckBear (syntaxError): | | Code 'classCGUIManager{' is invalid C code. Use --std or --language to configure the language. Executing section JS... | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 5. |----| | /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | 184| 184| "tooltip": translate("Exits the game."), | 185| 185| "onPress": () => { | 186| 186| messageBox( | 187| |- 400, 200, | | 187|+ 400, 200, | 188| 188| translate("Are you sure you want to quit 0 A.D.?"), | 189| 189| translate("Confirmation"), | 190| 190| [translate("No"), translate("Yes")], | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 5. |----| | /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | 185| 185| "onPress": () => { | 186| 186| messageBox( | 187| 187| 400, 200, | 188| |- translate("Are you sure you want to quit 0 A.D.?"), | | 188|+ translate("Are you sure you want to quit 0 A.D.?"), | 189| 189| translate("Confirmation"), | 190| 190| [translate("No"), translate("Yes")], | 191| 191| [null, Engine.Exit]); | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 5. |----| | /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | 186| 186| messageBox( | 187| 187| 400, 200, | 188| 188| translate("Are you sure you want to quit 0 A.D.?"), | 189| |- translate("Confirmation"), | | 189|+ translate("Confirmation"), | 190| 190| [translate("No"), translate("Yes")], | 191| 191| [null, Engine.Exit]); | 192| 192| } | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 5. |----| | /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | 187| 187| 400, 200, | 188| 188| translate("Are you sure you want to quit 0 A.D.?"), | 189| 189| translate("Confirmation"), | 190| |- [translate("No"), translate("Yes")], | | 190|+ [translate("No"), translate("Yes")], | 191| 191| [null, Engine.Exit]); | 192| 192| } | 193| 193| } | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 5. |----| | /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | 188| 188| translate("Are you sure you want to quit 0 A.D.?"), | 189| 189| translate("Confirmation"), | 190| 190| [translate("No"), translate("Yes")], | 191| |- [null, Engine.Exit]); | | 191|+ [null, Engine.Exit]); | 192| 192| } | 193| 193| } | 194| 194| ]; Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/570/display/redirect
This version of the patch would have depended on D2257 / rP22845.
Even with the patch the hotkeys are actually broken, because there is only one submenu, but multiple submenus have hotkey items.
One can pregenerate the same submenus, but I dont really see the point, especially when using the hotkey method nani proposed in said revision proposal, Engine.SetGlobalHotkey.
That only has the disadvantage (?) (or inconsistency to object assigned hotkeys) - one is either limited to assign one hotkey to one function, not one hotkey to multiple functions (multiple IGUIObjects).
That however may not be seen as invalid, because who really wants to press on two menu items with one hotkey? A mod would probably want to replace the hotkey with a function calling the existing function, refering to it by the g_MainMenu name, or something.
So the patch now depends on D2260.
There was a missing closeSubmenu call when pressing on a menu item within the submenu.
Naming: MainMenuItems vs MainMenuButtonHandler.
On the prototype vs class syntax question:, Deleted gave feedback in the lobby on Sep 1:
btw, classes are better than prototypes for just a single reason lol
lets say you were working on some web stuff and there exists a prototype with a property called a name.
you can accidentaly rename your window. but the syntax sugar would raise an error.
There are more advantages, for example private variables
It's an error to reference private fields from outside of the class; they can only be read or written within the class body. By defining things which are not visible outside of the class, you ensure that your classes' users can't depend on internals, which may change version to version.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes
So it seems attractive, and if the entire GUI is already rewritten from procedural to OOP, why not go for it while having the chance to.
Before I rewrite it, I will upload one more backup with prototypes and mentioned issues fixed.
binaries/data/mods/public/gui/pregame/MainMenuItems.js | ||
---|---|---|
107 ↗ | (On Diff #9613) | DISABLED BY BUILD shown if not disabled. It's an operator precedence problem, + binds stronger than &&, so adding parentheses |
binaries/data/mods/public/gui/pregame/ProductInformation.js | ||
4 ↗ | (On Diff #9613) | ProductInformation -> ProjectInformation |
binaries/data/mods/public/gui/pregame/mainmenu.js | ||
28 ↗ | (On Diff #9554) | isStartup is set from GameSetup.cpp, see rP12762 |
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/74/display/redirect
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 4 tabs but found 5. |----| | /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | 184| 184| "tooltip": translate("Exits the game."), | 185| 185| "onPress": () => { | 186| 186| messageBox( | 187| |- 400, 200, | | 187|+ 400, 200, | 188| 188| translate("Are you sure you want to quit 0 A.D.?"), | 189| 189| translate("Confirmation"), | 190| 190| [translate("No"), translate("Yes")], | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 5. |----| | /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | 185| 185| "onPress": () => { | 186| 186| messageBox( | 187| 187| 400, 200, | 188| |- translate("Are you sure you want to quit 0 A.D.?"), | | 188|+ translate("Are you sure you want to quit 0 A.D.?"), | 189| 189| translate("Confirmation"), | 190| 190| [translate("No"), translate("Yes")], | 191| 191| [null, Engine.Exit]); | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 5. |----| | /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | 186| 186| messageBox( | 187| 187| 400, 200, | 188| 188| translate("Are you sure you want to quit 0 A.D.?"), | 189| |- translate("Confirmation"), | | 189|+ translate("Confirmation"), | 190| 190| [translate("No"), translate("Yes")], | 191| 191| [null, Engine.Exit]); | 192| 192| } | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 5. |----| | /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | 187| 187| 400, 200, | 188| 188| translate("Are you sure you want to quit 0 A.D.?"), | 189| 189| translate("Confirmation"), | 190| |- [translate("No"), translate("Yes")], | | 190|+ [translate("No"), translate("Yes")], | 191| 191| [null, Engine.Exit]); | 192| 192| } | 193| 193| } | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 5. |----| | /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | 188| 188| translate("Are you sure you want to quit 0 A.D.?"), | 189| 189| translate("Confirmation"), | 190| 190| [translate("No"), translate("Yes")], | 191| |- [null, Engine.Exit]); | | 191|+ [null, Engine.Exit]); | 192| 192| } | 193| 193| } | 194| 194| ]; Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/583/display/redirect
binaries/data/mods/public/gui/pregame/MainMenuItemHandler.js | ||
---|---|---|
13 ↗ | (On Diff #9630) | It follows that rule of naming the variable the same as the xml object, thus then the xml name definition should be changed too to be standarized :) |
100 ↗ | (On Diff #9630) | The GUI internals force this way. |
Guess I'll fix what Jenkins wants, otherwise I'll get a whitespace cleanup patch :P
binaries/data/mods/public/gui/pregame/BackgroundHandler.js | ||
---|---|---|
12 ↗ | (On Diff #9554) | How often is this function called? |
binaries/data/mods/public/gui/pregame/MainMenuItemHandler.js | ||
13 ↗ | (On Diff #9630) | The benefit from using the same name in JS and XML is that if you do a filesearch, you immediately get all occurrences where the GUI object is used, rather than having to interpret code to find them. (No need to appeal to authority) |
53 ↗ | (On Diff #9630) | for...in |
100 ↗ | (On Diff #9630) | The C++ defined JSClass GUISize is updated, but the GUI object doesnt rebuild its size following the settings change, it only does so after a SetSetting call, below triggered from JSI_IGUIObject::setProperty. I suppose it could be implemented but who cares, actually I wonder if we really needed the JSclass when a JS object does the same, will check again. |
binaries/data/mods/public/gui/pregame/menupanel.xml | ||
18 ↗ | (On Diff #9630) | ProjectInformation |
Fall in love with JS class keyword and become disappointed by not having support for private fields.
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/83/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (no-extra-semi): | | Unnecessary semicolon. |----| | /zpool0/trunk/binaries/data/mods/public/gui/pregame/SplashscreenHandler.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/pregame/SplashscreenHandler.js | 30| 30| Engine.PushGuiPage("page_splashscreen.xml", {}, this.showRenderPathMessage); | 31| 31| else | 32| 32| this.showRenderPathMessage(); | 33| |- }; | | 33|+ } | 34| 34| | 35| 35| showRenderPathMessage() | 36| 36| { binaries/data/mods/public/gui/pregame/SplashscreenHandler.js | 33| » }; | | [NORMAL] JSHintBear: | | Unnecessary semicolon. binaries/data/mods/public/gui/pregame/mainmenu.js | 16| » new·MusicHandler(); | | [NORMAL] ESLintBear (no-new): | | Do not use 'new' for side effects. binaries/data/mods/public/gui/pregame/mainmenu.js | 16| » new·MusicHandler(); | | [MAJOR] ESLintBear (no-use-before-define): | | 'MusicHandler' was used before it was defined. binaries/data/mods/public/gui/pregame/mainmenu.js | 17| » new·ProjectInformationHandler(g_ProjectInformation); | | [NORMAL] ESLintBear (no-new): | | Do not use 'new' for side effects. binaries/data/mods/public/gui/pregame/mainmenu.js | 17| » new·ProjectInformationHandler(g_ProjectInformation); | | [MAJOR] ESLintBear (no-use-before-define): | | 'ProjectInformationHandler' was used before it was defined. binaries/data/mods/public/gui/pregame/mainmenu.js | 18| » new·CommunityButtonHandler(); | | [NORMAL] ESLintBear (no-new): | | Do not use 'new' for side effects. binaries/data/mods/public/gui/pregame/mainmenu.js | 18| » new·CommunityButtonHandler(); | | [MAJOR] ESLintBear (no-use-before-define): | | 'CommunityButtonHandler' was used before it was defined. binaries/data/mods/public/gui/pregame/mainmenu.js | 35| class·MusicHandler | | [NORMAL] JSHintBear: | | 'MusicHandler' was used before it was defined. binaries/data/mods/public/gui/pregame/mainmenu.js | 44| class·ProjectInformationHandler | | [NORMAL] JSHintBear: | | 'ProjectInformationHandler' was used before it was defined. binaries/data/mods/public/gui/pregame/mainmenu.js | 54| class·CommunityButtonHandler | | [NORMAL] JSHintBear: | | 'CommunityButtonHandler' was used before it was defined. | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 5. |----| | /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | 184| 184| "tooltip": translate("Exits the game."), | 185| 185| "onPress": () => { | 186| 186| messageBox( | 187| |- 400, 200, | | 187|+ 400, 200, | 188| 188| translate("Are you sure you want to quit 0 A.D.?"), | 189| 189| translate("Confirmation"), | 190| 190| [translate("No"), translate("Yes")], | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 5. |----| | /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | 185| 185| "onPress": () => { | 186| 186| messageBox( | 187| 187| 400, 200, | 188| |- translate("Are you sure you want to quit 0 A.D.?"), | | 188|+ translate("Are you sure you want to quit 0 A.D.?"), | 189| 189| translate("Confirmation"), | 190| 190| [translate("No"), translate("Yes")], | 191| 191| [null, Engine.Exit]); | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 5. |----| | /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | 186| 186| messageBox( | 187| 187| 400, 200, | 188| 188| translate("Are you sure you want to quit 0 A.D.?"), | 189| |- translate("Confirmation"), | | 189|+ translate("Confirmation"), | 190| 190| [translate("No"), translate("Yes")], | 191| 191| [null, Engine.Exit]); | 192| 192| } | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 5. |----| | /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | 187| 187| 400, 200, | 188| 188| translate("Are you sure you want to quit 0 A.D.?"), | 189| 189| translate("Confirmation"), | 190| |- [translate("No"), translate("Yes")], | | 190|+ [translate("No"), translate("Yes")], | 191| 191| [null, Engine.Exit]); | 192| 192| } | 193| 193| } | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 5. |----| | /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | |++++| /zpool0/trunk/binaries/data/mods/public/gui/pregame/MainMenuItems.js | 188| 188| translate("Are you sure you want to quit 0 A.D.?"), | 189| 189| translate("Confirmation"), | 190| 190| [translate("No"), translate("Yes")], | 191| |- [null, Engine.Exit]); | | 191|+ [null, Engine.Exit]); | 192| 192| } | 193| 193| } | 194| 194| ]; Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/592/display/redirect
binaries/data/mods/public/gui/pregame/MainMenuItems.js | ||
---|---|---|
107 ↗ | (On Diff #9613) | What is the intended else behaviour? Boolean false casts to string "false". I haven't tested it with this patch, but I would expect that to be happening currently. > "foo" + (!true && "if") "foofalse" Presumably the intended else-branch is to append nothing, which would be done with something like: > "foo" + (!true && "if" || "") "foo" |
binaries/data/mods/public/gui/pregame/MainMenuItems.js | ||
---|---|---|
107 ↗ | (On Diff #9613) | Whoops, you are right, and in time! Ternary for the rescue! |
binaries/data/mods/public/gui/pregame/MainMenuItemHandler.js | ||
---|---|---|
13 ↗ | (On Diff #9630) | Would probably be good. |
For the class syntax discussion, see above and http://irclogs.wildfiregames.com/2019-09/2019-09-05-QuakeNet-%230ad-dev.log