Skip to content

Adjust sheared range slider style to avoid overlap#34362

Open
EYHN wants to merge 3 commits intoppy:masterfrom
EYHN:eyhn/style/difficulty-range
Open

Adjust sheared range slider style to avoid overlap#34362
EYHN wants to merge 3 commits intoppy:masterfrom
EYHN:eyhn/style/difficulty-range

Conversation

@EYHN
Copy link
Copy Markdown
Contributor

@EYHN EYHN commented Jul 24, 2025

Fix #34334

Just a quick try. If it doesn't fit the design goals, please feel free to close this PR.

osu.development.2025-07-24.12-01-37.mp4

@peppy
Copy link
Copy Markdown
Member

peppy commented Jul 24, 2025

I guess this means the mapping is no longer accurate due to the size of the nubs being excluded in the width calculation. Not sure about that.

Maybe okay if we just consider the colour spectrum a rough visual guide.

@EYHN
Copy link
Copy Markdown
Contributor Author

EYHN commented Jul 24, 2025

The actual effective width of the slider is shorter than before. and the actual location where the nubs point to also changed. It used to be at the center of the nub, now it is at the edge (the red line in image)

image image

So the mapping here is a bit inaccurate, but personally, i'm not overly concerned with a perfect mapping.

I believe the new slider significantly improves readability within a small range. For instance, when practicing 1.5* to 2* songs:

Before

image

Now

image

@peppy peppy changed the title Adjust sheared range slider style to avoid overlip Adjust sheared range slider style to avoid overlap Jul 25, 2025
@peppy peppy self-requested a review July 25, 2025 06:45
@peppy
Copy link
Copy Markdown
Member

peppy commented Jul 25, 2025

I think I can get behind this change with one consideration addressed: The two nubs should not be touching. The space between should represent the 0.1 range they are still separated by.

Comment on lines +196 to +201
float nubOriginOffset = Nub.OriginPosition.X - Nub.DrawWidth / 2f;
float nubPosition = Nub.DrawPosition.X - nubOriginOffset;
LeftBox.Size = new Vector2(
Math.Clamp(RangePadding + nubPosition - Nub.DrawWidth / 2f + ShearedNub.CORNER_RADIUS - 0.5f, 0, Math.Max(0, DrawWidth)), 1);
RightBox.Size = new Vector2(
Math.Clamp(DrawWidth - RangePadding - nubPosition - Nub.DrawWidth / 2f + ShearedNub.CORNER_RADIUS - 0.5f, 0,
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.

I'm not sure how you got to this code, but it reads very bad. For instance, the split out variables are being used in two places which are also subtracting DrawWidth/2 a second time, I don't quite follow what is going on here anymore.

@EYHN
Copy link
Copy Markdown
Contributor Author

EYHN commented Jul 29, 2025

The two nubs should not be touching. The space between should represent the 0.1 range they are still separated by.

This issue stems from the UpperBound and LowerBound having different ranges.

Currently
the UpperBound is 0.1~1.1,
the LowerBound is 0.0~1.0.

This 0.1 offset is causing the problem.

If both the UpperBound and LowerBound were 0.0~1.0 (as they are in osu tests), this wouldn't be an issue.

image

My proposed solution is to refactor ShearedRangeSlider to allow UpperBound and LowerBound to have their own ranges. The overall slider width would then represent LowerBound.Min to UpperBound.Max.

Since the logic for handling mouse input in the lower-level SliderBar, this would also necessitate changes to osu-framework. I plan to add two new properties to SliderBar like this:

new SliderBar
{
    Current = new BindableDouble
    {
        MinValue = 1,
        MaxValue = 10
    },
    MinValue = 0,
    MaxValue = 20,
}

This would create a SliderBar that appears to have a total width of 0~20, but with an adjustable range only from 1~10.

This would not break the existing SliderBar, and the new properties would be optional.

What are your thoughts on this ? @peppy If you agree, I can submit a pull request for this change soon.

@peppy
Copy link
Copy Markdown
Member

peppy commented Jul 29, 2025

That may be workable. I'd definitely call the new variable MaxDisplayRange and MinDisplayRange or something similar, to clearly separate them from the actual min/max on the bindable.

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.

SongSelect: Star rating minimum slider head have small click area when overlapping with other slider head

3 participants