Skip to content

bugfix(gui): Remove slider bounds checks to prevent unexpected reset of custom settings#2624

Merged
xezon merged 2 commits intoTheSuperHackers:mainfrom
Skyaero42:fix/slider-bounds
Apr 21, 2026
Merged

bugfix(gui): Remove slider bounds checks to prevent unexpected reset of custom settings#2624
xezon merged 2 commits intoTheSuperHackers:mainfrom
Skyaero42:fix/slider-bounds

Conversation

@Skyaero42
Copy link
Copy Markdown

@Skyaero42 Skyaero42 commented Apr 18, 2026

When any slider value is out of bounds, an early return is initialized, causing the slider position value not be set at all.
The slider position value is then 0, even if the minimum is higher than 0.

Instead the slider UI is bound instead. If the slider is touched by the user, it will set its value between the minimum and maximum - easily allowing to reset the value if needed.

I left a comment to ensure in the future coders understand why there is no check in place. Can remove if desired.

@Skyaero42 Skyaero42 self-assigned this Apr 18, 2026
@Skyaero42 Skyaero42 added the Fix Is fixing something, but is not user facing label Apr 18, 2026
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 18, 2026

Greptile Summary

This PR fixes a bug where a slider's stored position was silently left at 0 whenever a value loaded from option.ini fell outside the slider's [minVal, maxVal] range (e.g., a user-customised value that pre-dates a min/max change). The fix removes the early break and instead clamps only the derived window-coordinate used for thumb rendering, so custom settings are preserved while the visual thumb stays within the track. The change is correctly applied to all four slider files (horizontal + vertical, Generals + Zero Hour).

Confidence Score: 5/5

Safe to merge — the fix is minimal, well-reasoned, and correctly handles out-of-bounds visual positioning via clamp.

No P0 or P1 findings. The clamp call uses the same argument order (min, value, max) as every other call-site in the codebase. The GSM_SLIDER_TRACK handler already clamps s->position when the user moves the slider, so storing an out-of-bounds value intentionally is consistent with the design. All four symmetric files are updated identically.

No files require special attention.

Important Files Changed

Filename Overview
Generals/Code/GameEngine/Source/GameClient/GUI/Gadget/GadgetHorizontalSlider.cpp Removes early-return bounds check in GSM_SET_SLIDER; adds clamp on the translated window coordinate to keep the thumb within the track while allowing s->position to store out-of-bounds values from option.ini.
Generals/Code/GameEngine/Source/GameClient/GUI/Gadget/GadgetVerticalSlider.cpp Same fix as the horizontal slider counterpart — drops out-of-bounds break in GSM_SET_SLIDER and clamps only the inverted window-coord translation, keeping visual thumb in range.
GeneralsMD/Code/GameEngine/Source/GameClient/GUI/Gadget/GadgetHorizontalSlider.cpp Zero Hour counterpart of the Generals horizontal slider fix — identical change, correctly mirrored.
GeneralsMD/Code/GameEngine/Source/GameClient/GUI/Gadget/GadgetVerticalSlider.cpp Zero Hour counterpart of the Generals vertical slider fix — identical change, correctly mirrored.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[GSM_SET_SLIDER received\nnewPos = mData1] --> B_OLD

    subgraph OLD ["Before PR"]
        B_OLD{newPos < minVal\nor newPos > maxVal?}
        B_OLD -->|Yes| C_OLD[break — position never set\ns->position stays 0]
        B_OLD -->|No| D_OLD[s->position = newPos]
        D_OLD --> E_OLD[Translate to window coords]
        E_OLD --> F_OLD[winSetPosition]
    end

    A2[GSM_SET_SLIDER received\nnewPos = mData1] --> D_NEW

    subgraph NEW ["After PR"]
        D_NEW[s->position = newPos\nalways stored, including out-of-bounds values]
        D_NEW --> E_NEW[Translate to window coords]
        E_NEW --> G_NEW[clamp translated coord to valid pixel range\n0 .. maxVal-minVal * numTicks]
        G_NEW --> F_NEW[winSetPosition — thumb always stays in track]
    end
Loading

Reviews (2): Last reviewed commit: "Updating based on feedback" | Re-trigger Greptile

Comment thread Generals/Code/GameEngine/Source/GameClient/GUI/Gadget/GadgetHorizontalSlider.cpp Outdated
Comment thread Generals/Code/GameEngine/Source/GameClient/GUI/Gadget/GadgetHorizontalSlider.cpp Outdated
Copy link
Copy Markdown

@xezon xezon left a comment

Choose a reason for hiding this comment

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

This fix is incomplete. When the underlying value is larger than the max, then the slider head surpasses the slider region.

Tested with MaxParticleCount = 6000

Image

@xezon xezon changed the title fix: Remove slider bounds check to prevent unwanted reset of custom settings. fix: Remove slider bounds check to prevent unwanted reset of custom settings Apr 19, 2026
@DevGeniusCode DevGeniusCode added the GUI For graphical user interface label Apr 19, 2026
@xezon xezon added Bug Something is not working right, typically is user facing Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour and removed Fix Is fixing something, but is not user facing labels Apr 21, 2026
@xezon xezon changed the title fix: Remove slider bounds check to prevent unwanted reset of custom settings bugfix(gui): Remove slider bounds checks to prevent unwanted reset of custom settings Apr 21, 2026
Copy link
Copy Markdown

@xezon xezon left a comment

Choose a reason for hiding this comment

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

Tested and worked.

@xezon xezon changed the title bugfix(gui): Remove slider bounds checks to prevent unwanted reset of custom settings bugfix(gui): Remove slider bounds checks to prevent unexpected reset of custom settings Apr 21, 2026
@xezon xezon merged commit 88bdb7c into TheSuperHackers:main Apr 21, 2026
17 checks passed
DevGeniusCode added a commit to DevGeniusCode/GeneralsGameCode that referenced this pull request Apr 24, 2026
DevGeniusCode added a commit to DevGeniusCode/GeneralsGameCode that referenced this pull request Apr 24, 2026
DevGeniusCode added a commit to DevGeniusCode/GeneralsGameCode that referenced this pull request Apr 24, 2026
DevGeniusCode added a commit to DevGeniusCode/GeneralsGameCode that referenced this pull request Apr 24, 2026
DevGeniusCode added a commit to DevGeniusCode/GeneralsGameCode that referenced this pull request Apr 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something is not working right, typically is user facing Gen Relates to Generals GUI For graphical user interface Minor Severity: Minor < Major < Critical < Blocker ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MaxParticles gets reset to 0 on options save if larger then 5000

3 participants