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 osu!catch "buzz slider" SR abuse #31126

Merged
merged 4 commits into from
Jan 18, 2025
Merged

Conversation

bastoo0
Copy link
Contributor

@bastoo0 bastoo0 commented Dec 14, 2024

This addresses the same issue as #28451 but in a simpler, more straight-forward way.

The issue being solved is fixing the SR calculation for a certain pattern we are calling "buzz slider", which is an horizontal back & forth slider that has a length smaller than the catcher's width but is large enough to be detected as movements, resulting in it being considered as very fast jumps instead of a stand still pattern. No specific map has this issue because it's not allowed by the ranking criteria, but some converts do contain it (like https://osu.ppy.sh/beatmapsets/2066239#fruits/4343468).

This bug is caused by some offsets that are added to the player's position to account for potential mispositioning between objects. It's useful in some scenarios and has effect on pretty much everything so we don't want to remove that logic until a proper rework is being done. So in the mean time, this is a temporary fix that targets this exact pattern.

This is currently a draft since I need to run the calc sheet first to dectect potential side effects.

@bastoo0 bastoo0 requested a review from a team December 14, 2024 20:46
@bastoo0 bastoo0 marked this pull request as draft December 14, 2024 20:47
@tsunyoku
Copy link
Member

!diffcalc
RULESET=catch

Copy link

@Givikap120
Copy link
Contributor

Givikap120 commented Dec 17, 2024

This addresses the same issue as #28451 but in a simpler, more straight-forward way.

the point of my approach is to make smooth transition from "you just need to stay on one place" to "you have to aim each object individually"
if you don't need this and you can just drop pp for distance being slightly smaller than threshold - then of course my solution is too complex
I thought that sudden pp drop would be a bad practice

Tho I think you can achieve smooth transition here by making multiplier fall from 1.0 to 0.0 by making double threshold

@bastoo0
Copy link
Contributor Author

bastoo0 commented Dec 18, 2024

the point of my approach is to make smooth transition from "you just need to stay on one place" to "you have to aim each object individually" if you don't need this and you can just drop pp for distance being slightly smaller than threshold - then of course my solution is too complex I thought that sudden pp drop would be a bad practice

Tho I think you can achieve smooth transition here by making multiplier fall from 1.0 to 0.0 by making double threshold

In my opinion, the sudden strain drop doesn't really matter because the SR algorithm is already smoothing up the strains because it averages them over multiple objects (or something of that effect) iirc. I thought about putting something other than 0 but in my scenario the player has already caught the beginning of the slider and is already at a stand still position so I think having it at 0 makes sense imo.

@Givikap120
Copy link
Contributor

In my opinion, the sudden strain drop doesn't really matter because the SR algorithm is already smoothing up the strains because it averages them over multiple objects (or something of that effect) iirc. I thought about putting something else than 0 but in my scenario the player has already caught the beginning of the slider and is already at a stand still position so I think having it at 0 makes sense imo.

On this map https://osu.ppy.sh/beatmapsets/1595850#osu/3505245:
Live:
CS0.7 -> CS0.8 -> CS0.9: 299pp -> 372pp -> 690pp
CS4.2 -> CS4.3: 1323pp -> 1334pp

This PR:
CS0.7 -> CS0.8 -> CS0.9: 137pp -> 276pp -> 166pp
CS4.2 -> CS4.3: 296pp -> 1334pp

My PR:
CS0.7 -> CS0.8 -> CS0.9: 134pp -> 148pp -> 548pp
CS4.2 -> CS4.3: 1322pp -> 1333pp

My PR is not as stable/smooth as I thought it was, definitely some room for improvement
Tho I think 137->276->166 in this branch is much more serious issue than jump from 296 to 1334

@bastoo0
Copy link
Contributor Author

bastoo0 commented Dec 18, 2024

On this map https://osu.ppy.sh/beatmapsets/1595850#osu/3505245: Live: CS0.7 -> CS0.8 -> CS0.9: 299pp -> 372pp -> 690pp CS4.2 -> CS4.3: 1323pp -> 1334pp

This PR: CS0.7 -> CS0.8 -> CS0.9: 137pp -> 276pp -> 166pp CS4.2 -> CS4.3: 296pp -> 1334pp

My PR: CS0.7 -> CS0.8 -> CS0.9: 134pp -> 148pp -> 548pp CS4.2 -> CS4.3: 1322pp -> 1333pp

My PR is not as stable/smooth as I thought it was, definitely some room for improvement Tho I think 137->276->166 in this branch is much more serious issue than jump from 296 to 1334

Okay that's weird. The 296 to 1334 is expected but the 137pp -> 276pp -> 166pp variation will need to be investigated.

@smoogipoo smoogipoo changed the base branch from master to pp-dev December 18, 2024 14:09
@bastoo0
Copy link
Contributor Author

bastoo0 commented Jan 4, 2025

Some update: I found that, in the mentioned beatmap, specifically at CS0.8, there are issues with the distances : Even though we are in a buzz slider case, the distance changes between each object, for some reason. So I had to do some digging.

After manually inspecting each case in the editor, my conclusion is that it comes from the hyperdashes generation. We have a case where, maybe due to some floating point inaccuracies, not all objects are converted to hyperdashes, and it (somehow) translates to varying distances.

With CS=0.7
Index : 955
Last distance : 40,361053
Distance : 40,361053
Index : 956
Last distance : 40,361053
Distance : 40,361053

With CS = 0.8 (our problematic case)
Index : 955
Last distance : 40,937286
Distance : 65,93729
Index : 956
Last distance : 65,93729
Distance : 40,937286

With CS=0.9
Index : 955
Last distance : 66,52377
Distance : 66,52377
Index : 956
Last distance : 66,52377
Distance : 66,52377

So yeah basically, when there is an hyperdash the distance is around 60 and when no hyperdash the distance is around 40 in all cases.

I would say it is due to how osu generates hyperdashes and I'm not sure if there is a way around it. We can leave this as a specific rare case or try to find a way to handle this within the current conditions (I don't want to remove the "last distance = distance" part of the equation, to avoid potential side effects). In my own opinion I would say we can ignore it.

CS 0.7:
CS_0.7

CS 0.8 (our problematic case):
CS_0.8

CS 0.9:
CS_0.9

Copy link

@Flawnpiece Flawnpiece left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the problematic maps get nerfed as they should. As for the edge-case pointed out by Givikap I think it's fine to ignore for now (the spreadsheet doesn't show anything getting any value)

@stanriders stanriders marked this pull request as ready for review January 18, 2025 13:15
@stanriders stanriders enabled auto-merge (squash) January 18, 2025 16:49
@stanriders stanriders merged commit 67723b3 into ppy:pp-dev Jan 18, 2025
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Pending Deploy
Development

Successfully merging this pull request may close these issues.

5 participants