Skip to content

Score grade is incorrect when lazer replay is exported and re-imported #24061

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

Closed
JacksonChen666 opened this issue Jun 27, 2023 · 11 comments · Fixed by #26579
Closed

Score grade is incorrect when lazer replay is exported and re-imported #24061

JacksonChen666 opened this issue Jun 27, 2023 · 11 comments · Fixed by #26579
Assignees
Labels
area:replay priority:0 Showstopper. Critical to the next release.

Comments

@JacksonChen666
Copy link

JacksonChen666 commented Jun 27, 2023

Type

Cosmetic

Bug description

I imported a score of my own, clicked to view then saw a contradiction. (See screenshot)

I don't know how to better describe these things in the title.

Map

Replay (in a .zip)

Screenshots or videos

Version

2023.621.0-lazer

Logs

database.log
input.log
network.log
performance-audio.log
performance-draw.log
performance-input.log
performance-update.log
performance.log
runtime.log

@monochrome22

This comment was marked as off-topic.

@JacksonChen666
Copy link
Author

Should be duplicate of #20857.

No failed plays are involved here.

@monochrome22

This comment was marked as off-topic.

@JacksonChen666
Copy link
Author

There was a fail at 0:23, it can be seen by importing the score into stable and viewing the replay at 1x speed. Lazer has issues with replay playback, so the fail might not show on Lazer.

I did not fail during gameplay (although in this case I only barely managed to pass), even if it seems like it in osu!stable (which isn't osu!lazer).

Was your online score set in multiplayer?

No, it was played solo. The online score is pretty much the same score as the mentioned replay, except for its lack of replay downloads.

@bdach
Copy link
Collaborator

bdach commented Jun 27, 2023

@monochrome22 you're not making any sense at all. @JacksonChen666's score is a lazer score. And a pass at that.

What is actually happening here is that the rank is never actually stored to the replay on export. lazer's grade is entirely accuracy-based, but stable's wasn't, and the legacy format decoder uses the legacy logic, causing this to kick in:

case 2:
{
int totalHits = count50 + count100 + count300 + countMiss + countKatu;
score.Accuracy = totalHits > 0 ? (double)(count50 + count100 + count300) / totalHits : 1;
if (score.Accuracy == 1)
score.Rank = score.Mods.Any(m => m is ModHidden || m is ModFlashlight) ? ScoreRank.XH : ScoreRank.X;
else if (score.Accuracy > 0.98)
score.Rank = score.Mods.Any(m => m is ModHidden || m is ModFlashlight) ? ScoreRank.SH : ScoreRank.S;
else if (score.Accuracy > 0.94)
score.Rank = ScoreRank.A;
else if (score.Accuracy > 0.9)
score.Rank = ScoreRank.B;
else if (score.Accuracy > 0.85)
score.Rank = ScoreRank.C;
else
score.Rank = ScoreRank.D;
break;
}

And in the case above, this would indeed be a D score in stable terms, because accuracy is under 85%.

@bdach bdach added area:replay priority:1 Very important. Feels bad without fix. Affects the majority of users. labels Jun 27, 2023
@bdach bdach changed the title Big score grade contradicts with circle score grade Score grade is incorrect when lazer replay is exported and re-imported Jun 27, 2023
@andrewhong04
Copy link
Contributor

Is this fixed?
Screenshot 2024-01-11 at 4 47 16 AM

@bdach
Copy link
Collaborator

bdach commented Jan 11, 2024

1704968423

if you don't know how bug trackers work, the above generally means that it's not fixed

@andrewhong04
Copy link
Contributor

andrewhong04 commented Jan 11, 2024

If you look at the screenshot I provided, it looks like it's fixed before one of the members have marked it as fixed.

@frenzibyte
Copy link
Member

frenzibyte commented Jan 12, 2024

Please clarify your intention next time, and avoid posting screenshots without making sure it shows what you're referring to.

That being said, the accuracy cutoffs has indeed been adjusted in #25601 to match the code snippet mentioned in #24061 (comment).

However, this has no relevance in the issue thread's main point. As per the title of this thread, the problem is that the score rank gets populated with stable's logic, rather than lazer/ScoreProcessor.

It seems possible to not have stable's logic run for lazer scores using the IsLegacyScore property, like the following:

diff --git a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs
index ed11691674..4d651b2fb2 100644
--- a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs
+++ b/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs
@@ -132,6 +132,9 @@ public Score Parse(Stream stream)
 
             PopulateAccuracy(score.ScoreInfo);
 
+            if (!score.ScoreInfo.IsLegacyScore)
+                score.ScoreInfo.Rank = currentRuleset.CreateScoreProcessor().RankFromAccuracy(score.ScoreInfo.Accuracy);
+
             // before returning for database import, we must restore the database-sourced BeatmapInfo.
             // if not, the clone operation in GetPlayableBeatmap will cause a dereference and subsequent database exception.
             score.ScoreInfo.BeatmapInfo = workingBeatmap.BeatmapInfo;

But upon testing that in real leaderboards, it appears that the accuracy is different on import, potentially being high enough to display a different rank instead:

(left is online, right is post-import)

@peppy
Copy link
Member

peppy commented Jan 15, 2024

But upon testing that in real leaderboards, it appears that the accuracy is different on import, potentially being high enough to display a different rank instead

Did you stop investigating at this point? It's a weird place to leave things, just means the next person to follow up is going to have to repeat and continue where you left.

@peppy peppy added priority:0 Showstopper. Critical to the next release. and removed priority:1 Very important. Feels bad without fix. Affects the majority of users. labels Jan 15, 2024
@peppy
Copy link
Member

peppy commented Jan 15, 2024

Bumping to priority 0. Having broken replay round trip is going to cause a lot of headaches as users start to move to lazer.

@bdach bdach self-assigned this Jan 16, 2024
bdach added a commit to bdach/osu that referenced this issue Jan 16, 2024
Closes ppy#24061.

The gist of this change is that if the `LegacyReplaySoloScoreInfo`
bolt-on is present in the replay, then it can (and is) used to recompute
the accuracy, and rank is computed based on that.

This was the missing part of
ppy#24061 (comment).
The accuracy would change on import before that because the encode
process is _lossy_ if the `LegacyReplaySoloScoreInfo` bolt-on is not
used, as the legacy format only has 6 fields for encoding judgement
counts, and some judgements that affect accuracy in lazer do not fit
into that.

Note that this _only_ fixes _relatively_ new lazer scores looking wrong
after reimport.

- Very old lazer scores, i.e. ones that don't have the
  `LegacyReplaySoloScoreInfo` bolt-on, obviously can't use it
  to repopulate. There's really not much good that can be done there,
  so the stable pathways are used as a fallback that always works.

- For stable replays, `ScoreImporter` recalculates the accuracy of
  the score _again_ in

  https://github.com/ppy/osu/blob/15a5fd7e4c8e8e3c38382cfd3d5d676d107d7908/osu.Game/Scoring/ScoreImporter.cs#L106-L110

  as `StandardisedScoreMigrationTools.UpdateFromLegacy()` recomputes
  _both_ total score and accuracy.

  This makes a _semblance_ of sense as it attempts to make the accuracy
  of stable and lazer replays comparable. In most cases it also won't
  matter, as the only ruleset where accuracy changed between the legacy
  implementation and current lazer accuracy is mania.

  But it is also an inaccurate process (as, again, some of the required
  data is not in the replay, namely judgement counts of ticks
  and so on).

  For whatever's worth, a similar thing happens server-side in

  https://github.com/ppy/osu-queue-score-statistics/blob/106c2948dbe695efcad5972d32cd46f4b36005cc/osu.Server.Queues.ScoreStatisticsProcessor/Commands/Queue/BatchInserter.cs#L319

- However, _ranks_ of stable scores will still use the local stable
  reimplementation of ranks, i.e. a 1-miss stable score in osu! ruleset
  will be an A rather than an S. See importer:

  https://github.com/ppy/osu-queue-score-statistics/blob/106c2948dbe695efcad5972d32cd46f4b36005cc/osu.Server.Queues.ScoreStatisticsProcessor/Commands/Queue/BatchInserter.cs#L237

  (it's the same method which is renamed
  to `PopulateLegacyAccuracyAndRank()` in this commit).

  That is all a bit of a mess honestly, but I'm not sure where to even
  begin there...
bdach added a commit to bdach/osu that referenced this issue Jan 16, 2024
Closes ppy#24061.

The gist of this change is that if the `LegacyReplaySoloScoreInfo`
bolt-on is present in the replay, then it can (and is) used to recompute
the accuracy, and rank is computed based on that.

This was the missing part of
ppy#24061 (comment).
The accuracy would change on import before that because the encode
process is _lossy_ if the `LegacyReplaySoloScoreInfo` bolt-on is not
used, as the legacy format only has 6 fields for encoding judgement
counts, and some judgements that affect accuracy in lazer do not fit
into that.

Note that this _only_ fixes _relatively_ new lazer scores looking wrong
after reimport.

- Very old lazer scores, i.e. ones that don't have the
  `LegacyReplaySoloScoreInfo` bolt-on, obviously can't use it
  to repopulate. There's really not much good that can be done there,
  so the stable pathways are used as a fallback that always works.

- For stable replays, `ScoreImporter` recalculates the accuracy of
  the score _again_ in

  https://github.com/ppy/osu/blob/15a5fd7e4c8e8e3c38382cfd3d5d676d107d7908/osu.Game/Scoring/ScoreImporter.cs#L106-L110

  as `StandardisedScoreMigrationTools.UpdateFromLegacy()` recomputes
  _both_ total score and accuracy.

  This makes a _semblance_ of sense as it attempts to make the accuracy
  of stable and lazer replays comparable. In most cases it also won't
  matter, as the only ruleset where accuracy changed between the legacy
  implementation and current lazer accuracy is mania.

  But it is also an inaccurate process (as, again, some of the required
  data is not in the replay, namely judgement counts of ticks
  and so on).

  For whatever's worth, a similar thing happens server-side in

  https://github.com/ppy/osu-queue-score-statistics/blob/106c2948dbe695efcad5972d32cd46f4b36005cc/osu.Server.Queues.ScoreStatisticsProcessor/Commands/Queue/BatchInserter.cs#L319

- However, _ranks_ of stable scores will still use the local stable
  reimplementation of ranks, i.e. a 1-miss stable score in osu! ruleset
  will be an A rather than an S. See importer:

  https://github.com/ppy/osu-queue-score-statistics/blob/106c2948dbe695efcad5972d32cd46f4b36005cc/osu.Server.Queues.ScoreStatisticsProcessor/Commands/Queue/BatchInserter.cs#L237

  (it's the same method which is renamed
  to `PopulateLegacyAccuracyAndRank()` in this commit).

  That is all a bit of a mess honestly, but I'm not sure where to even
  begin there...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:replay priority:0 Showstopper. Critical to the next release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants