Settings - Refactor settings menu to use a single controls group - #1827
Conversation
The settings menu rescanned and sorted every setting in the game every time a category was opened, and kept its own category map for the search bar that was shaped exactly like the one the menu needed. Build both from one pass and cache them: - GVAR(categorySettings) maps a lower case category to its display name and its settings, already in the order the menu shows them. - GVAR(subCategories) marks the index each sub-category header goes at. - GVAR(searchCategories) is gone, GVAR(categorySettings) replaces it. Renames fnc_gui_searchIndex.sqf to fnc_gui_index.sqf, it no longer only serves the search bar. Nothing consumes the new indices for drawing yet, so the menu is unchanged.
Opening a category walked and sorted every setting in the game to find the ones belonging to it. Read them from GVAR(categorySettings) instead, and take the sub-category header positions from GVAR(subCategories), so the work is proportional to the category rather than to the whole modset. Turns gui_createCategory.inc.sqf into a real function while it is being rewritten anyway. It only needed the display and the category from its caller's scope, and as an include its errors were reported against gui_addonChanged. Also adds the missing default branch to the "can this be edited" switch, which left _enabled undefined for an unexpected source.
Positioning a row read its x/y/w/h from config, and greying one out enumerated its child control classes to find their IDCs. Both are the same for every row of a given class, and a category creates hundreds of them, so opening one did thousands of config reads to look up a handful of distinct answers. Cache them per class in uiNamespace, where they outlive the mission because config can't change at runtime.
Settings were positioned as they were created and then positioned again by the search filter, with the two passes duplicating the layout rules between them. Create them without a position and lay the table out once afterwards, in FUNC(gui_reflow), which is now the only thing that positions a setting. Every list setting also carried its own invisible padding control, sized to its open dropdown and parked on top of the settings below it, purely to keep the scroll area tall enough. One pad per category does the same job: the reflow stretches it to reach past the lowest dropdown of the settings that are shown.
Every control of a row carried its own copy of the setting name and the source it was showing, so changing either meant rewriting up to nine of them. Keep both on the row itself and read them back through ROW_SETTING and ROW_SOURCE where they are needed, leaving the per-control variables to hold only what is specific to that control. The row level QGVAR(params) is now QGVAR(settingData), it held something else entirely and the name collided. Colour settings kept a copy of the value alongside the sliders and mutated it in place, which aliased the stored value. Read it back off the sliders instead. Fixes two bugs on the way: - A password was replaced with asterisks only when its row was created, so importing settings put the real one on screen. It is replaced whenever the value is displayed now. This is only ever done when the box is disabled, so the mask can't be written back over the value. - Tabbing out of a colour channel updated the sliders but never stored the value or refreshed the rest of the row.
Whether a setting can be edited from the source being shown was worked out once while the row was built and applied by switching every one of its controls off. Nothing recorded the answer, so anything that later switched a control back on - the "reset to default" button, the overwrite checkboxes - could hand the player a control they aren't allowed to use. Work it out in FUNC(gui_setRowEnabled), keep it on the row, and have those places ask before they enable anything. The overwrite checkboxes are hidden by moving them off screen, which threw away where they belong. Remember that when the row is built so they can be put back.
An edited setting is stored as a value and a priority together, but only one of them is edited at a time and writing either one stored a nil in place of the other. Everything that works out which source wins then read that nil instead of falling back to the setting's real value or priority: - Editing a value in the server tab stored no priority, so the server was treated as having none and the clients appeared to overwrite it. The lock column turned into a red "overwritten by clients" as soon as anything was edited, and only corrected itself if a priority was written afterwards. - Ticking an overwrite checkbox stored no value, so the check for whether the overwriting source holds the same value compared against nothing. The lock column kept the icon it had and never picked up the "overwritten" colour or tooltip. Fill in whichever half isn't being written with what is in effect right now, which is what FUNC(gui_saveTempData) already did when the settings were saved. Also restores "overwrite clients" to ticked when "overwrite mission" is unticked on a setting that was saved overwriting the mission. Nothing recorded what it had been, and overwriting the mission includes overwriting the clients, so it drops back to that rather than to nothing.
Every setting was built three times over, once for each of the server, mission and client tabs, and switching tab swapped which of the three sets of controls was shown. Two thirds of what a category created was never looked at. Build one row per setting and point it at the source being shown. Nothing about a row depended on the source any more except which overwrite checkboxes it has and whether it can be edited at all, and neither of those is expensive to redo, so switching tab is now FUNC(gui_refresh) putting the values back rather than hundreds of controls being hidden and shown. Which settings are shown doesn't depend on the source either, so a search no longer has to be re-applied and the table no longer has to be laid out again every time the tab changes. The controls groups were kept in a display variable per category and source with the two joined into its name. There is one per category now, so they are kept in a hashmap and the "was this category built" flag is gone with them.
|
|
||
| (GVAR(default) getVariable _setting) params ["_defaultValue", "", "_settingType", "_settingData", "", "_displayName", "_tooltip", "_isGlobal"]; | ||
|
|
||
| if (_tooltip != _setting) then { // Append setting name to bottom line |
|
|
||
| // ----- how far an open dropdown reaches below its row, so the table can | ||
| // ----- keep the scroll area tall enough for it | ||
| if (toUpper _settingType == "LIST") then { |
There was a problem hiding this comment.
| if (toUpper _settingType == "LIST") then { | |
| if (toUpper _settingType isEqualTo "LIST") then { |
There was a problem hiding this comment.
Can you stop with these suggestions of using isEqualTo where it is completely unnecessary and less redable?
This is the 3rd repository you are doing this on where the same people are involved. It is at this point only annoying.
| private _fields = [_displayName, _setting, _tooltip, _category, _subCategory]; | ||
|
|
||
| // list labels and their tooltips are searchable as well | ||
| if (toUpper _settingType == "LIST") then { |
There was a problem hiding this comment.
| if (toUpper _settingType == "LIST") then { | |
| if (toUpper _settingType isEqualTo "LIST") then { |
| _categorySettings getOrDefault [_key, [_category, []], true]; | ||
|
|
||
| // Make sure empty-subcategory is always sorted first (fixing unicode) | ||
| (_sortable getOrDefault [_key, [], true]) pushBack [parseNumber (_subCategory != ""), _subCategory, _forEachIndex, _setting]; |
There was a problem hiding this comment.
| (_sortable getOrDefault [_key, [], true]) pushBack [parseNumber (_subCategory != ""), _subCategory, _forEachIndex, _setting]; | |
| (_sortable getOrDefault [_key, [], true]) pushBack [parseNumber (_subCategory isNotEqualTo ""), _subCategory, _forEachIndex, _setting]; |
| // what "overwrite clients" was before that. Overwriting the mission | ||
| // includes overwriting the clients, so unticking it drops back to just | ||
| // the clients rather than to nothing. | ||
| if (isNil {_ctrlOverwriteClient getVariable QGVAR(state)}) then { |
There was a problem hiding this comment.
| if (isNil {_ctrlOverwriteClient getVariable QGVAR(state)}) then { | |
| if (_ctrlOverwriteClient isNil QGVAR(state)) then { |
I realized only just now you can just replace getVariable with isNil, much cleaner, not to mention faster.
| case "client": {CAN_SET_CLIENT_SETTINGS && {isNil {GVAR(userconfig) getVariable _setting}}}; | ||
| case "mission": {CAN_SET_MISSION_SETTINGS && {isNil {GVAR(missionConfig) getVariable _setting}}}; | ||
| case "server": {CAN_SET_SERVER_SETTINGS && {isNil {GVAR(serverConfig) getVariable _setting}}}; |
There was a problem hiding this comment.
| case "client": {CAN_SET_CLIENT_SETTINGS && {isNil {GVAR(userconfig) getVariable _setting}}}; | |
| case "mission": {CAN_SET_MISSION_SETTINGS && {isNil {GVAR(missionConfig) getVariable _setting}}}; | |
| case "server": {CAN_SET_SERVER_SETTINGS && {isNil {GVAR(serverConfig) getVariable _setting}}}; | |
| case "client": {CAN_SET_CLIENT_SETTINGS && {GVAR(userconfig) isNil _setting}}; | |
| case "mission": {CAN_SET_MISSION_SETTINGS && {GVAR(missionConfig) isNil _setting}}; | |
| case "server": {CAN_SET_SERVER_SETTINGS && {GVAR(serverConfig) isNil _setting}}; |
I suspect not worth the lazy eval around isNil.
| }; | ||
|
|
||
| { | ||
| if (_categories isEqualTo [] || {_x in _categories}) then { |
There was a problem hiding this comment.
in is kinda fast, though, so I suspect not worth the lazy eval.
| onSetFocus = QUOTE((ctrlParent (_this select 0)) setVariable [ARR_2(QQGVAR(searchFocus),true)]); | ||
| onKillFocus = QUOTE((ctrlParent (_this select 0)) setVariable [ARR_2(QQGVAR(searchFocus),false)]); | ||
| // right click clears the search | ||
| onMouseButtonClick = QUOTE(if ((_this select 1) == 1) then {(_this select 0) ctrlSetText ''; [ctrlParent (_this select 0)] call FUNC(gui_search)}); |
There was a problem hiding this comment.
| onMouseButtonClick = QUOTE(if ((_this select 1) == 1) then {(_this select 0) ctrlSetText ''; [ctrlParent (_this select 0)] call FUNC(gui_search)}); | |
| onMouseButtonClick = QUOTE(if ((_this select 1) isEqualTo 1) then {(_this select 0) ctrlSetText ''; [ctrlParent (_this select 0)] call FUNC(gui_search)}); |
Co-authored-by: Jouni Järvinen <rautamiekka@users.noreply.github.com>
Requires #1825.