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

Use score.MaximumStatistics in places which assume the entire beatmap was played #32137

Open
wants to merge 1 commit into
base: pp-dev
Choose a base branch
from

Conversation

tsunyoku
Copy link
Member

totalPlayableHits is a new field which uses MaximumStatistics to sum the amount of hits in the beatmap, regardless of how far into the beatmap the given score was. My assumption is that using IsBasic is good enough to get what is effectively a sum of the circles, sliders and spinners in a beatmap - based on reading that appears to be true, but please scrutinise this line of code.

This allows for a proper fix of what was attempted in #31741 as it prevents the cases where these variables could go negative, rather than just pushing them to zero if it happened.

My logic for where I kept totalHits was that those places are better using the hits in the score thus far - miss penalty, length bonus etc. all make sense to react to the amount of hits the score actually achieved rather than what's in the beatmap.

Testing against the replay provided in the attached PR still gives zero with the relevant guards removed due to these changes, and I'll run a sheet to confirm that this doesn't affect submitted plays. PP counter in-game also appears to progress as expected.

@tsunyoku
Copy link
Member Author

!diffcalc
RULESET=osu
OSU_A=https://github.com/ppy/osu/tree/pp-dev
OSU_B=#32137

Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Pending Review
Development

Successfully merging this pull request may close these issues.

1 participant