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

Clamp estimateImproperlyFollowedDifficultSliders for lazer scores #30544

Merged
merged 1 commit into from
Nov 11, 2024

Conversation

Finadoggie
Copy link
Contributor

@Finadoggie Finadoggie commented Nov 8, 2024

Fixes instances of maps giving too much pp if their slider count on the servers is less than their actual slider count (or any other scenario where a negative number of dropped slider ends is feasible)

@smoogipoo
Copy link
Contributor

Sure, but isn't this hiding the issue?

@Finadoggie
Copy link
Contributor Author

I guess it is. Is that a bad thing?

@Rekunan
Copy link

Rekunan commented Nov 8, 2024

isn't this hiding the issue?

It's the same for stable scores:

estimateImproperlyFollowedDifficultSliders = Math.Clamp(Math.Min(maximumPossibleDroppedSliders, attributes.MaxCombo - scoreMaxCombo), 0, estimateDifficultSliders);

And existed before #27691 was merged:
double estimateSliderEndsDropped = Math.Clamp(Math.Min(countOk + countMeh + countMiss, attributes.MaxCombo - scoreMaxCombo), 0, estimateDifficultSliders);

So if there is an issue with this change, then we should probably remove the clamping for stable scores (and maybe reconsider other/similar occurrences in diffcalc).

@smoogipoo
Copy link
Contributor

It's the same for stable scores:

As I read it, that's because it's an estimation where the lazer path is supposed to be more accurate. But sure.

Copy link
Contributor

@smoogipoo smoogipoo left a comment

Choose a reason for hiding this comment

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

Approved but blocked on #30566 😴

@ppy ppy deleted a comment from github-actions bot Nov 11, 2024
@ppy ppy deleted a comment from github-actions bot Nov 11, 2024
@smoogipoo
Copy link
Contributor

smoogipoo commented Nov 11, 2024

Sorry, I'm going to use this PR as a bit of a testbed.

@ppy ppy deleted a comment from github-actions bot Nov 11, 2024
@smoogipoo
Copy link
Contributor

!diffcalc

Copy link

@smoogipoo smoogipoo merged commit f746999 into ppy:master Nov 11, 2024
8 of 9 checks passed
@Finadoggie Finadoggie deleted the clamp-slider branch November 16, 2024 02:54
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.

3 participants