The values from the enums are converted to numbers a lot, so using an enum class would be counter-productive.
Having 'left' in the global namespace appeared to be an actual problem with some Qt headers, according to Fulgen.
Consolidate the include statements scattered across the code in accordance
with the comment in C4Include.h. The advantages are listed in the same
comment.
Furthermore, it follows llvm-include-order which is the logical
extrapolation of the project's style guideline wherever possible
(C4Include.h being the most-frequent exception).
The TightGridLayout fills spaces more aggressively. This is slower but makes for a tighter layout. Finding the best layout is NP-complete. This here is just O(N^2) or so.
The window would always use its very own rectangle for clipping - bad luck if it was larger than the parent.
Now children can only additionally restrict the parent rectangle (which defaults to the whole screen at root).
Otherwise, you could open a menu on mouse-down, which would then block the mouse-up event. The control system (not the script but the engine!) would then never know that the button was released and issue a key event with repeated=1 when you pressed the button the next time.
This could lead to issues.
When an UI element was only visible to a player (via the Player property), it still allowed ALL players to click on buttons.
I am wondering why noone else noticed that bug before. It's possible that it didn't show when the visibility was set via the menu's Target (instead of the Player property).
Maybe this was the cause of the "clicks sometimes do nothing" bug?
I guess the UI elements expect their position to already be corrected by cgo.X/Y. Or they never cared because they were in front of the upper board.
I am not sure, maybe the correct solution would be to actually position all elements lower (instead of adjusting for it when passing mouse input or drawing)? Currently the position is relative to the upper board's edge.
Anyway, this works for now.
Otherwise, if a window was GUI_FitChildren and the text would NOT trigger a scrollbar, the window height was set to 0 (because the text height was only taken into account when rawTextHeight - 1 > rcBounds.Hgt).
Previously, text windows would just change their own size and leave cropping and scrolling to their parent. This made the code easier, but was apparently unintuitive for scripters.
Now text windows do not change their size but show a scrollbar themselves (unless GUI_FitChildren or GUI_NoCrop of course).
This implied some other changes, because now parents without a scroll bar need to clip, too. (Or the clipping needs to be moved to the child window. But then it would have to be made sure that menu decoration can still go out of the bounds.)
And this also needed some script fixes where scripters assumed the text windows would not scroll (and thus made them smaller than 1em).
related to https://git.openclonk.org/openclonk.git/commit/46ad28ea652fad34814a866f3b9c305aa7cc6faa
I am not sure why the 'isMainWindow' was there. It wasn't there in 7319f7b3cc and got introduced in the major rework in 049088be78 - I guess it was just an oversight that was not noticed because usually the UI windows have the text or other things that need to scroll on a deeper level.
Anyway, checking whether the window is a script-root window does not make any sense as far as I can see now.
GUI_Multiple windows are supposed to be updated from the root window in a full refresh. However, they also had the "dirty" flag set (that is only used for non GUI_Multiple windows after the initial startup), but they did not unset it when updating.
So they were first updated with the correct parent width (full screen) and then, because of the dirty flag, were updated again like a non GUI_Multiple window with the parent's size. That was incorrect. Only another update would then fix it again (because the "dirty" flag would then have been unset).
Previously it was checked after adding an element. Long elements would thus be in the same row as the other elements even if they wouldn't fit. Not long elements get their own row.
NO_OWNER means the message will be visible for noone (can f.e. be used to hide a ui window). nil means the message won't have visibility restrictions. This is probably what the scripter expects.
Previously, a value of "nil" would have been read as 0. So not setting the Player property and setting it to nil would mean two different things.
Previously, the em <-> pixels conversion was a hardcoded value. Now the GUI scales with the font size that can be selected in the options.
Sadly, all scales were off since the hardcoded value was too low.
There are some ownership rules (PropList::GetProperties, Ui::Element..) which are not very clear imo. But there also was at least one clear oversight (name retaining, that is).
The only issue would be an assertion firing at game exit.
The desync was caused by PropList->GetProperties returning the properties in an arbitrary order. They are now sorted first.
The debug logs are left in place, because I assume that I will need them again and they prove to be helpful.
Fullscreen GUIs on wide-screen monitors look stupid. This patch tackles this by restricting the maximum size to something that can still be seen with a glance.
For very high-DPI or low-DPI screens, the user would most likely adjust the font size in the options (todo) anyway and thus also change the maximum menu size.
C4ScriptGuiWindowStyleFlag should probably be a class enum. Or at least put in a separate namespace. This is TODO to prevent further name collisions. However, it's not completely trivial.
David Dormagen