Add display range to SliderBar #6618
Conversation
| /// <summary> | ||
| /// Triggered when the <see cref="Current"/> value has changed. Used to update the displayed value. | ||
| /// </summary> | ||
| /// <param name="value">The normalized <see cref="Current"/> value.</param> | ||
| /// <param name="value"> |
There was a problem hiding this comment.
I think this parameter should be renamed to position.
| } | ||
|
|
||
| currentNumberInstantaneous.SetProportional(newValue, e.ShiftPressed ? KeyboardStep : 0); | ||
| double min = double.CreateTruncating(MinDisplayRange); |
There was a problem hiding this comment.
We should probably explain here that this implementation matches BindableNumber.SetProportional (now unused and having no test coverage). Or maybe even remove that method (although note that it is public..)
|
This looks like it will work well for the scenario in question. I'd want a second @ppy/team-client review as this is a framework change altering a central component. |
|
How on earth is this the resolution to whatever the game-side problem is? This "feature" is so broken. Using the test added in this PR as-is, why can I go below zero if the Screen.Recording.2025-07-30.at.12.15.52.movWhy can I specify Screen.Recording.2025-07-30.at.12.16.56.movWhy does this allow specifying a wider range of movement than the underlying bindable therefore allowing half the slide range to be dead? Screen.Recording.2025-07-30.at.12.17.25.movI haven't examined the reason why this is apparently needed in closer detail but really? |
|
Oh and I apparently can just bypass the "display range" entirely by dragging past the bounds of the slider (set to [0,5] on this video): Screen.Recording.2025-07-30.at.12.23.15.mov |
|
@EYHN Based on the above, and the general feeling that we don't want to over-complicate the |
|
Okay, thanks for your time reviewing this. I will consider how to workaround it. |
This PR adds
MinDisplayRangeandMaxDisplayRangeproperties toSliderBar, allowing the display range to be adjusted instead of being fixed toBindableNumber.MinValueandBindableNumber.MaxValue.Context: ppy/osu#34362 (comment)
I spent some time considering variable naming and changed some original variable names, which may require discussion.
This change will not break the original behavior of
SliderBar, and osu should be able to upgrade safely.2025-07-30.130525.mp4
After this PR is merged, it will be possible to adjust the
LowerBoundandUpperBoundranges independently in osuShearedSliderBar2025-07-30.131643.mp4