If a modder or developer working on a GUI patch invalidly calls PopGUIPage when there is no page to pop (cant pop if there is only 1 page), then a debug_warn is triggered.
But mods shouldn't be able to trigger such "crashes" easily, especially when it can be caught easily and communicated to the JS side with JS syntax.
Details
- Reviewers
- None
- Commits
- rP22846: Don't crash if a JS GUI author calls Engine.PopGuiPage too often.
One could check whether the users to this new getter function might not want to be improved further somehow, and whether there might be a use case for exposing the entire stack instead of only the count (sounds like there might be a use case for that in the future when JS pages can communicate with each other, but not currently).
One could check whether more debug_warns or ENSUREs could be triggered from the JS GUI.
Notice that AutoRequest is necessary for ReportError, and it seems like its missing in many more places!
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
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/62/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... Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/571/display/redirect
Why not keep the haspages function as well, which makes the intent more clear ?
source/gui/scripting/JSInterface_GUIManager.cpp | ||
---|---|---|
46 ↗ | (On Diff #9614) | Can't pop a GUI page when there are less than two pages open! |
Mostly don't see the need for a second getter if its already covered by the first one.
source/gui/scripting/JSInterface_GUIManager.cpp | ||
---|---|---|
46 ↗ | (On Diff #9614) | I'll take that (last) one, thanks. |
source/ps/Game.cpp | ||
---|---|---|
323 ↗ | (On Diff #9614) | See https://en.cppreference.com/w/cpp/language/implicit_conversion
|