Skip to content

Fix sliders allowing values outside their number of stops #5749

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Halavus wants to merge 3 commits into
base: develop
Choose a base branch
from
+9 −3
Open

Fix sliders allowing values outside their number of stops #5749

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1e9f89e
clamp Slider indices when stepping with clicks
Halavus Mar 13, 2026
0d56b23
clamp Slider indices when dragging
Halavus Mar 14, 2026
3b39e45
Merge branch 'develop' into gui-slider-min-max
ab9rf Jul 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/changelog.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ Template for new versions:
## New Features

## Fixes
- GUI: Fix sliders allowing values outside their number of stops (num_stops) when incremented with the mouse.
- `autolabor`: Fix running 1 tick less frequently than intended.
- `buildingplan`: fixed non-clickable pressure plates's triggers (issue #5736)
- `getplants`: added protective code to avoid misoperation when a plant has an invalid material (which should never happen, but...)
Expand Down
5 changes: 5 additions & 0 deletions library/lua/gui/widgets/slide_core.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,18 @@ function slide_core:get_min_stops()
return self.is_single and 1 or 2
end

function slide_core:clamp_idx(idx)
return math.max(1, math.min(self.num_stops, idx))
end

local function do_drag(self, width_per_idx)
local x = self.frame_body:localXY(dfhack.screen.getMousePos())
local cur_pos = x - self.is_dragging_idx
cur_pos = math.max(0, cur_pos)
cur_pos = math.min(width_per_idx*(self.num_stops-1)+7, cur_pos)
local offset = self.is_dragging_target == 'right' and -2 or 1
local new_idx = math.max(0, cur_pos+offset)//width_per_idx + 1
new_idx = self:clamp_idx(new_idx)
local new_left_idx, new_right_idx
if self.is_dragging_target == 'right' then
new_right_idx = new_idx
Expand Down
6 changes: 3 additions & 3 deletions library/lua/gui/widgets/slider.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ local core = require('gui.widgets.slide_core')
---@field super widgets.Widget
---@field ATTRS widgets.Slider.attrs|fun(attributes: widgets.Slider.attrs.partial)
---@overload fun(init_table: widgets.Slider.initTable): self
Slider = defclass(Slider, slide_core)
Slider = defclass(Slider, core)

@ab9rf ab9rf Jul 11, 2026

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.

why is slide_core changed to core here?

@Halavus Halavus Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Because otherwise local core in line 1 is pointless.

@SilasD SilasD Jul 12, 2026

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.

This is the wrong way to do it. revert the change here, and change line 1 to
local slide_core = require('gui.widgets.slide_core')
or things just get too confusing.

the class's name is slide_core, it should be named exactly that by all code that uses it. this is realSquidCoder's mistake, not yours, but this is a good time to fix it.

this is especially important because every script has a global variable named slide_core that is a reference to the slide_core class, so the existing code does work as written, despite that unused local variable core.

per our conventions, slide_core should also be named using CamelCase. _SliderCore maybe? i.e. starting the class name with a single underscore to hopefully flag that it's not intended to be instantiated. that's out of scope of the intended fix, but it's a good time to make the change.

Slider.ATTRS{
get_idx_fn=DEFAULT_NIL,
on_change=DEFAULT_NIL,
Expand All @@ -41,7 +41,7 @@ function Slider:onInput(keys)
local left_pos = width_per_idx*(left_idx-1)
local right_pos = width_per_idx*(right_idx-1) + 4
if x < left_pos then
self.on_change(self.get_idx_fn() - 1)
self.on_change(self:clamp_idx(self.get_idx_fn() - 1))
elseif x < left_pos+3 then
self.is_dragging_target = 'left'
self.is_dragging_idx = x - left_pos
Expand All @@ -52,7 +52,7 @@ function Slider:onInput(keys)
self.is_dragging_target = 'right'
self.is_dragging_idx = x - right_pos
else
self.on_change(self.get_idx_fn() + 1)
self.on_change(self:clamp_idx(self.get_idx_fn() + 1))
end
return true
end
Expand Down
Loading