View Issue Details

IDProjectCategoryView StatusLast Update
0005869The Dark ModGUIpublic04.12.2022 02:58
Reporterstgatilov Assigned Tostgatilov  
PrioritynormalSeveritynormalReproducibilityN/A
Status resolvedResolutionfixed 
Product VersionTDM 2.10 
Target VersionTDM 2.11Fixed in VersionTDM 2.11 
Summary0005869: Better diagnostic for syntax errors in GUI code
DescriptionThere are many errors in .gui which don't provided any kind of diagnostic.
Instead, parsing just continues, despite the fact that sometimes all the subsequent code is broken by the first error.

One such example is adding a semicolon at the end of window parameters:
  noTime 1;
The code expects GUI keyword instead of it, so it gets into "else" clause in idWindow::Parse.
It calls idWindow::ParseRegEntry, which cannot find the variable, and thus creates a custom one with semicolon as its name.
One can set breakpoint to the second part of idWindow::ParseRegEntry and catch such cases.
The same thing happens if someone writes "indowDef" or "ontipe" (i.e. keyword with a typo).

Ideally, some special syntax should be used to add custom parameters, and unknown word without it should generate error.

Another weird code location is probably idGuiScript::Parse.
I guess if one forgets a semicolon at the end of script command, it can pass without error, especially for a "set" command.
Steps To ReproduceFor instance, go to mainmenu_main.gui and add semicolon here:
      windowDef MainMenuParchment
         rect 280, 0, 320, 640;
Now you don't see the menu, although you at least see an error in this case =)

Or you can replace "windowDef LoadGameText" with "indowDef LoadGameText".
The menu boots up, but who knows which parts of GUI code are broken?
TagsNo tags attached.

Relationships

related to 0005758 closedstgatilov Cutscene video shortfalls in replay & embedded audio 
related to 0005852 resolvedstgatilov Refactor objectives GUI and fix checkboxes in restart GUI 
related to 0006026 resolvedstgatilov Wrong order of evaluation in expressions 

Activities

stgatilov

stgatilov

17.07.2022 17:24

administrator   ~0015039

I added insane number of checks in C++ code, and fixed our core .gui elements accordingly.

r10003 Warn if onTime argument is not integer.
r10004 Warn if onNamedEvent type is not a name.
r10005 Explicitly allow using semicolon instead of value when declaring float variable.
r10006 Refactored idRegisterList::AddReg, apply translation in both cases.
r10007 Warn if register value has more values specified than necessary.
r10008 Warn about wrong terms used inside window expressions.
r10009 Drop unknown tokens when parsing window contents.
r10010 Warn about custom window registers without float keyword, and about register with EOF instead of value.
r10011 Warn about Set command with more than one input argument, improve warning about wrong number of arguments to command.
r10012 Now .gui file must contain single windowDef and nothing more.
r10013 Better checking of values for internal vars, warn if resetCinematics has arguments.
r10014 Warn if "if" condition is not enclosed into parentheses.
r10015 idWinVar::Set now returns false in case of wrong format, instead of posting a warning immediately.
r10016 Warn if builtin register gets value in wrong format.
r10017 Refactored system for attaching source code location to script commands (idGuiScript).
r10018 Warn about wrong format of color argument in transition command.
r10019 Warn if resetTime has wrong arguments.
r10020 Warn if setFocus or showCursor command has wrong argument.
r10021 Check duration and accel/decel-time arguments of transition command, warn if 5 arguments.
r10022 Warn about wrong destination of ordinary Set command.
r10023 Warn if Set command tries to assign incompatible value.
r10024 Refactored common case of replacing variable referenced by argument into idGSWinVar::RelinkVar method.

r16532 Removed unused "selected 0" line.
r16533 Removed non-working CHOICE_DEF and updateGroup in video settings elements.
r16534 Fixed crouch_scale value in LightgemLeftBracket + LightgemRightBracket.
r16535 Fixed back proper alignment of "Adjust" button for gui size settings.
r16536 Fixed typo in gui::missionList_sel_0.
r16537 Removed incorrect comparisons with empty string ( == "").
r16538 Fixed usage of undefined macros in mainmenu_failure.gui.
r16539 Fixed == 0 condition on next/prev buttons in mission downloader's screenshot viewer.
r16540 Oops, CHOICE_DEF_FONT was extracted incorrectly =)
r16541 Fixed text field on startingDrop0 in shop GUI.
r16542 Fixed typo in 21x9 version of spyglass overlay.
r16544 Use correct syntax to define custom window var: noshadows 1 on compass.
r16545 Fixing wrong number of arguments in script "set" command.
r16548 Commented out LootIcon in parchment-style inventory grid.
r16550 Removed resetTime on nonexistant window HideMainMenu.
r16554 Fixed wrong destinations of "set" command.
r16558 Use "float" keyword instead of "definefloat" for compass noshadows register.

The changes in core .gui files were necessary to fix all the new warnings introduced.
stgatilov

stgatilov

17.07.2022 17:28

administrator   ~0015040

The final thoughts on the topic:

1) I think all kind of errors will now result in a warning which would reference source code.
  The only exception is set-cmd script command: it is too hard to statically validate it, since its syntax is covered in entirely different code.
2) While I'm sure all mistakes will results in console warnings, I'm not sure that engine won't crash afterwards.
  So if engine crashes after loading updated GUI, set "logFile 2", run again, and look qconsole.log for warnings.
3) In most cases I tried to retain old behavior even if something is totally wrong (after firing a warning, of course).
  However, some incredibly obscure use cases are broken: some accidentally, some deliberately.
stgatilov

stgatilov

23.07.2022 11:44

administrator   ~0015073

Last edited: 24.07.2022 07:27

While moving on to 0005852, I did a ton more of GUI cleaning and changes.

r10025 Refactoring dependencies in GUI headers.
r10026 Refactored idWindow::FindChildByName to not return pointer (to static variable sometimes).
r10027 Now source code location has its own data type, added location for windows too.
r10028 Report warning on window override and ambigous reference to a window.
r10031 Don't enable string concatenation by backslash (\) in GUI code.
r10032 Warn if dollar-reference to variable on the right side of Set is wrong.
r16568 Fixed duplicate window MissionTitleText.

r10029 Shorten displayed path in idLexer.
r10035 Rolled back r10029 "Shorten displayed path in idLexer" since it broke script save/load.
r10036 Reimplemented path shortening for warnings.

It seems that I'll probably change more...
stgatilov

stgatilov

30.07.2022 18:16

administrator   ~0015096

One more bit:
r10048 assert that we don't call idGSWinVar::RelinkVar with already linked var.

Issue History

Date Modified Username Field Change
04.01.2022 06:42 stgatilov New Issue
04.01.2022 06:42 stgatilov Status new => assigned
04.01.2022 06:42 stgatilov Assigned To => stgatilov
04.01.2022 06:42 stgatilov Relationship added related to 0005758
04.01.2022 06:49 stgatilov Steps to Reproduce Updated
17.07.2022 17:24 stgatilov Note Added: 0015039
17.07.2022 17:28 stgatilov Note Added: 0015040
17.07.2022 17:28 stgatilov Status assigned => feedback
18.07.2022 19:19 stgatilov Relationship added related to 0005852
23.07.2022 11:44 stgatilov Note Added: 0015073
24.07.2022 07:27 stgatilov Note Edited: 0015073
27.07.2022 20:46 stgatilov Relationship added related to 0006026
30.07.2022 18:16 stgatilov Note Added: 0015096
04.12.2022 02:58 nbohr1more Status feedback => resolved
04.12.2022 02:58 nbohr1more Resolution open => fixed
04.12.2022 02:58 nbohr1more Fixed in Version => TDM 2.11