Skip to content
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.

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

Fix extended values in difficulty adjust being truncated to 10 on beatmap change #21541

Merged
merged 2 commits into from
Dec 7, 2022

Conversation

bdach
Copy link
Collaborator

@bdach bdach commented Dec 6, 2022

Closes #21539.

Regressed in a243842. It was not fine to reorder the base call :(

The failure site exposing this is here:

public override Bindable<float?> Current
{
get => base.Current;
set
{
// Intercept and extract the internal number bindable from DifficultyBindable.
// This will provide bounds and precision specifications for the slider bar.
difficultyBindable = (DifficultyBindable)value.GetBoundCopy();
sliderDisplayCurrent.BindTo(difficultyBindable.CurrentNumber);
base.Current = difficultyBindable;
}
}

The .GetBoundCopy() call here would cause CurrentNumber to be clamped to 10, on both sides of the bind.

Cause is similar (but not 100% exactly the same) as ppy/osu-framework#5542. All bounds that may clamp the values in the bindable need to be transferred prior to the value, to ensure clamping does not occur in the process of copying values across.

This PR uses the solution from the review comment linked above, with requisite test coverage. I won't pretend it's not ugly, but I don't know how to make it not-ugly without upending more stuff than could be considered reasonable.

@smoogipoo smoogipoo requested a review from peppy December 7, 2022 04:16
@peppy peppy merged commit 70e0a04 into ppy:master Dec 7, 2022
@bdach bdach deleted the difficulty-bindable-regression branch December 7, 2022 06:10
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.

Difficulty adjust mod reverting values above 10 to 10 upon switching beatmaps
3 participants