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 slider end judgements appearing at incorrect locations when mods that flip playfield are active #26885

Merged
merged 3 commits into from
Jan 31, 2024

Conversation

bdach
Copy link
Collaborator

@bdach bdach commented Jan 31, 2024

The applied optimisation may have been valid as long as it was constrained to Slider. But it is not, as SliderTailCircle stores a local copy of the object position. And as the commit message of ce643aa states, this could be bypassed by some pretty hacky delegation from SliderTailCircle.Position to the slider, but it'd also be pretty hacky because it would make flows like PositionBindable break down.

Long-term solution is to probably remove bindables from hitobjects.

cc @OliBomby

bdach added 3 commits January 31, 2024 12:39
Closes ppy#26867.

Reverts 882f490
and ce643aa.

The applied optimisation may have been valid as long as it was
constrained to `Slider`. But it is not, as `SliderTailCircle` stores a
local copy of the object position. And as the commit message of
ce643aa states, this could be bypassed
by some pretty hacky delegation from `SliderTailCircle.Position` to the
slider, but it'd also be pretty hacky because it would make flows like
`PositionBindable` break down.

Long-term solution is to probably remove bindables from hitobjects.
@bdach bdach added ruleset/osu! area:gameplay next release Pull requests which are almost there. We'll aim to get them in the next release, but no guarantees! labels Jan 31, 2024
@OliBomby
Copy link
Contributor

I'm fine with reverting my fix for now.

The long-term solution to remove bindables from hitobjects. That would probably mean having the editor and OsuHitObjectGenerationUtils be responsible for making sure the nested hitobjects are correct after the operation.
Doing all that requires some extra consideration so I'll save it for a later PR.

@peppy peppy merged commit 89a5be1 into ppy:master Jan 31, 2024
@bdach bdach deleted the slider-end-flip-fail branch January 31, 2024 13:43
@bdach
Copy link
Collaborator Author

bdach commented Feb 2, 2024

I have come to learn that this regression likely affected diffcalc/pp too and as such we're going to have to check if it did (it's not impossible it didn't if the databased beatmap attributes weren't updated since the regression), and if it did, wipe or recalc pp for at least all HR lazer scores.

cc @ppy/team-client @ppy/osu-pp-committee

@smoogipoo
Copy link
Contributor

I believe the consensus reached is that master and live-sr are fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:gameplay next release Pull requests which are almost there. We'll aim to get them in the next release, but no guarantees! ruleset/osu! size/S
Projects
Development

Successfully merging this pull request may close these issues.

Sliderend miss judgement misaligned when using HR or Mirror
4 participants