Skip to content

Settings - Refactor settings menu to use a single controls group - #1827

Merged
PabstMirror merged 10 commits into
CBATeam:masterfrom
LinkIsGrim:settings-ui-rewrite
Aug 7, 2026
Merged

PabstMirror merged 10 commits into
CBATeam:masterfrom
LinkIsGrim:settings-ui-rewrite

Conversation

@LinkIsGrim

@LinkIsGrim LinkIsGrim commented Jul 29, 2026

Copy link
Copy Markdown
Contributor
  • Some code cleanup.
  • Refactor settings menu to use a single controls group per setting. Makes opening menu faster hopefully.
  • Due to above switching tabs no longer re-filters/sorts as well.
  • Opening an addon no longer walks and sorts every single setting, pulls from an index.
  • configClasses lookup is now done exactly once per row type instead of once per row.
  • Fixes a few longstanding bugs with the settings UI (particularly in regards to the overwrite checkboxes and their rendering before saving). TL;DR: changing priority or value of a setting would nil out the other in the temp namespace.
  • Because it's a single controls group, switching tabs will preserve the scrollbar position as well now.

Requires #1825.

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.
@LinkIsGrim
LinkIsGrim marked this pull request as ready for review July 29, 2026 04:50
Comment thread addons/settings/fnc_gui_addonChanged.sqf Outdated

(GVAR(default) getVariable _setting) params ["_defaultValue", "", "_settingType", "_settingData", "", "_displayName", "_tooltip", "_isGlobal"];

if (_tooltip != _setting) then { // Append setting name to bottom line

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!= intended ?


// ----- 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 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (toUpper _settingType == "LIST") then {
if (toUpper _settingType isEqualTo "LIST") then {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
(_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 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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.

Comment on lines +34 to +36
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}}};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in is kinda fast, though, so I suspect not worth the lazy eval.

Comment thread addons/settings/fnc_gui_index.sqf Outdated
Comment thread addons/settings/gui.hpp
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)});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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)});

@PabstMirror PabstMirror added this to the 3.18.7 milestone Jul 31, 2026
Co-authored-by: Jouni Järvinen <rautamiekka@users.noreply.github.com>
@PabstMirror
PabstMirror merged commit 6285644 into CBATeam:master Aug 7, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants