Skip to content

Fix sliders allowing values outside their number of stops#5749

Open
Halavus wants to merge 3 commits into
DFHack:developfrom
Halavus:gui-slider-min-max
Open

Fix sliders allowing values outside their number of stops#5749
Halavus wants to merge 3 commits into
DFHack:developfrom
Halavus:gui-slider-min-max

Conversation

@Halavus

@Halavus Halavus commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Clicking left and right only recorded -1 or +1 based on mouse position without any safeguard against potential larger frame than the slider itself. EDIT: Dragging was affected too.

Capture d'écran 2026-03-13 233008

If this PR makes an externally-visible change in behavior or API, please add an appropriate line to docs/changelog.txt.

Halavus added 2 commits March 14, 2026 00:13
Clicking left and right only did -1 & +1 without checking the validity of the result.
---@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)

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?

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.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants