8000 feat: Implement `maxAchievable`. by GabuTheDev · Pull Request #341 · tosuapp/tosu · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat: Implement maxAchievable. #341

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
May 5, 2025
Merged

feat: Implement maxAchievable. #341

merged 4 commits into from
May 5, 2025

Conversation

GabuTheDev
Copy link
Collaborator
@GabuTheDev GabuTheDev commented May 3, 2025 •
edited
Loading

This PR implements a new pp value called maxAchievable that indicates the maximum pp value a user could be getting from their play if they fc the rest of the map. This is similar to gosu's old maxThisPlay property, so it's a step towards gosu-tosu compatibility. (more or less...)

Q: How is this any different from play.pp.fc?
A: The play.pp.fc does not take account of misses; it shows the PP value that could have been achieved only if the player would have never missed. The maxAchievable value takes account of all hit judgements (300s,100s,50s & misses).

!! There's a few things to consider before merging, such as compatibility issues.
Since this PR also renames maxAchievedThisPlay to maxAchieved, there are two ways of approaching future issues:

  1. Announce the future deprecation of maxAchievedThisPlay, but keep both values (old name and new name) even though they provide the same value. (Assuming deprecation comes later, so people have time to switch)
  2. Deprecate maxAchievedThisPlay (announcing breaking changes) and update all counters available in the tosu counters repository with the new property name (which can be done very easily by me), but the issue still remains with the local/private pp counters that use this

Important

Either way, the socket will need to be updated with the new changes, which will be a future PR after this gets merged.

Note

The new value has only been tested on osu!std so far, I am not aware of the possible issues with the other rulesets.

@GabuTheDev GabuTheDev force-pushed the feat/max-achievable branch from 9ea5cff to 77c8afa Compare May 3, 2025 11:56
@GabuTheDev
Copy link
Collaborator Author

There are two flaws in this implementation:

  1. Rewinding replays in the lazer client breaks the calcs. One fix here would be to just set maxAchievable as the replay's pp since we already know how much the play was worth (which would cancel the calcs).
  2. For lazer scores, this calc assumes max osuSmallTickHits & osuLargeTickHits since there's no way of actually predicting them.

@GabuTheDev GabuTheDev marked this pull request as ready for review May 3, 2025 15:58
KotRikD
KotRikD requested changes May 3, 2025
View reviewed changes
@KotRikD KotRikD merged commit 41a9774 into tosuapp:master May 5, 2025
3 checks passed
@KotRikD KotRikD mentioned this pull request May 5, 2025
Merged
@GabuTheDev GabuTheDev mentioned this pull request May 5, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KotRikD KotRikD KotRikD approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@GabuTheDev @KotRikD
0