Ensure partial failed replays are played to their end #33670
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closes ppy#24285. This is not a perfect solution, as it is still possible for a replay to play *beyond* its end if the HP system doesn't fail it after it runs out of frames, but it's probably the best that can be done at this time. Notably this removes existing F rank checks because they were really not reliable. - Scores coming from stable will never present F rank, because rank is not stored to the replay, and the lowest rank that can be produced by `StandardisedScoreMigrationTools` is D. - lazer scores set prior to ppy#28058 will present F rank as long as the user has kept them in their local database and never exported and reimported them, for the same reason as above (rank not stored to replay). Also there have been many mechanics changes since, so it's not impossible for the replay to fail *before* the user actually did even in this case. - lazer scores set after ppy#28058 could technically rely on F rank but making them rely on it is annoying for several reasons: - The PR in question didn't bump `LegacyScoreEncoder.LATEST_VERSION`, so any checks based on the replay version field would be half-reliable anyway. - *Even after* the above, the replay version is only stored to realm as `TotalScoreVersion`, which *then gets bumped* on score version upgrades. So it can't even be used for any checks from that angle, you'd have to decode it from the score. - You *could* use `ClientVersion` because that's somewhat reliable, but that's stored *as string*, so you'd have to do some snipping to split off the `-lazer` suffix, then parse the version, then compare it. I started going through the motions of that before deciding that this is an edge case of an edge case and probably not worth spending time over the simple and obvious solution of just doing away with the rank check. Until I'm proven wrong, I guess.
9147957 to
73a1f10
Compare
smoogipoo
approved these changes
Jun 25, 2025
peppy
approved these changes
Jun 30, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #24285.
This is not a perfect solution, as it is still possible for a replay to play beyond its end if the HP system doesn't fail it after it runs out of frames, but it's probably the best that can be done at this time.
Notably this removes existing F rank checks because they were really not reliable.
Scores coming from stable will never present F rank, because rank is not stored to the replay, and the lowest rank that can be produced by
StandardisedScoreMigrationToolsis D.lazer scores set prior to Preserve score rank on lazer scores during encode/decode #28058 will present F rank as long as the user has kept them in their local database and never exported and reimported them, for the same reason as above (rank not stored to replay). Also there may have been mechanics changes since, so it's not impossible for the replay to fail before the user actually did even in this case.
lazer scores set after Preserve score rank on lazer scores during encode/decode #28058 could technically rely on F rank but making them rely on it is annoying for several reasons:
LegacyScoreEncoder.LATEST_VERSION, so any checks based on the replay version field would be half-reliable anyway.TotalScoreVersion, which then gets bumped on score version upgrades. So it can't even be used from realm for any checks from that angle, you'd have to decode it from the score.ClientVersionbecause that's somewhat reliable, but that's stored as string, so you'd have to do some snipping to split off the-lazersuffix, then parse the version, then compare it. I started going through the motions of that before deciding that this is an edge case of an edge case and probably not worth spending time over the simple and obvious solution of just doing away with the rank check. Until I'm proven wrong, I guess.