From 1075b87fb8bfb84ce3404bf93253940992e86671 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 16 Jan 2024 22:19:20 +0100 Subject: [PATCH 1/2] Add failing test coverage --- .../Formats/LegacyScoreDecoderTest.cs | 121 ++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs index 4e281cf28e..6e7c8c3631 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs @@ -3,14 +3,18 @@ #nullable disable +using System; using System.Collections.Generic; using System.Globalization; using System.IO; using System.Linq; using NUnit.Framework; +using osu.Framework.Extensions; using osu.Framework.Utils; using osu.Game.Beatmaps; using osu.Game.Beatmaps.Formats; +using osu.Game.Beatmaps.Legacy; +using osu.Game.IO.Legacy; using osu.Game.Replays; using osu.Game.Rulesets; using osu.Game.Rulesets.Catch; @@ -247,6 +251,123 @@ namespace osu.Game.Tests.Beatmaps.Formats }); } + [Test] + public void AccuracyAndRankOfStableScorePreserved() + { + var memoryStream = new MemoryStream(); + + // local partial implementation of legacy score encoder + // this is done half for readability, half because `LegacyScoreEncoder` forces `LATEST_VERSION` + // and we want to emulate a stable score here + using (var sw = new SerializationWriter(memoryStream, true)) + { + sw.Write((byte)0); // ruleset id (osu!) + sw.Write(20240116); // version (anything below `LegacyScoreEncoder.FIRST_LAZER_VERSION` is stable) + sw.Write(string.Empty.ComputeMD5Hash()); // beatmap hash, irrelevant to this test + sw.Write("username"); // irrelevant to this test + sw.Write(string.Empty.ComputeMD5Hash()); // score hash, irrelevant to this test + sw.Write((ushort)198); // count300 + sw.Write((ushort)1); // count100 + sw.Write((ushort)0); // count50 + sw.Write((ushort)0); // countGeki + sw.Write((ushort)0); // countKatu + sw.Write((ushort)1); // countMiss + sw.Write(12345678); // total score, irrelevant to this test + sw.Write((ushort)1000); // max combo, irrelevant to this test + sw.Write(false); // full combo, irrelevant to this test + sw.Write((int)LegacyMods.Hidden); // mods + sw.Write(string.Empty); // hp graph, irrelevant + sw.Write(DateTime.Now); // date, irrelevant + sw.Write(Array.Empty()); // replay data, irrelevant + sw.Write((long)1234); // legacy online ID, irrelevant + } + + memoryStream.Seek(0, SeekOrigin.Begin); + var decoded = new TestLegacyScoreDecoder().Parse(memoryStream); + + Assert.Multiple(() => + { + Assert.That(decoded.ScoreInfo.Accuracy, Is.EqualTo((double)(198 * 300 + 100) / (200 * 300))); + Assert.That(decoded.ScoreInfo.Rank, Is.EqualTo(ScoreRank.A)); + }); + } + + [Test] + public void AccuracyAndRankOfLazerScorePreserved() + { + var ruleset = new OsuRuleset().RulesetInfo; + + var scoreInfo = TestResources.CreateTestScoreInfo(ruleset); + scoreInfo.Mods = new Mod[] { new OsuModFlashlight() }; + scoreInfo.Statistics = new Dictionary + { + [HitResult.Great] = 199, + [HitResult.Miss] = 1, + [HitResult.LargeTickHit] = 1, + }; + scoreInfo.MaximumStatistics = new Dictionary + { + [HitResult.Great] = 200, + [HitResult.LargeTickHit] = 1, + }; + + var beatmap = new TestBeatmap(ruleset); + var score = new Score + { + ScoreInfo = scoreInfo, + }; + + var decodedAfterEncode = encodeThenDecode(LegacyBeatmapDecoder.LATEST_VERSION, score, beatmap); + + Assert.Multiple(() => + { + Assert.That(decodedAfterEncode.ScoreInfo.Accuracy, Is.EqualTo((double)(199 * 300 + 30) / (200 * 300 + 30))); + Assert.That(decodedAfterEncode.ScoreInfo.Rank, Is.EqualTo(ScoreRank.SH)); + }); + } + + [Test] + public void AccuracyAndRankOfLazerScoreWithoutLegacyReplaySoloScoreInfoUsesBestEffortFallbackToLegacy() + { + var memoryStream = new MemoryStream(); + + // local partial implementation of legacy score encoder + // this is done half for readability, half because we want to emulate an old lazer score here + // that does not have everything that `LegacyScoreEncoder` now writes to the replay + using (var sw = new SerializationWriter(memoryStream, true)) + { + sw.Write((byte)0); // ruleset id (osu!) + sw.Write(LegacyScoreEncoder.FIRST_LAZER_VERSION); // version + sw.Write(string.Empty.ComputeMD5Hash()); // beatmap hash, irrelevant to this test + sw.Write("username"); // irrelevant to this test + sw.Write(string.Empty.ComputeMD5Hash()); // score hash, irrelevant to this test + sw.Write((ushort)198); // count300 + sw.Write((ushort)0); // count100 + sw.Write((ushort)1); // count50 + sw.Write((ushort)0); // countGeki + sw.Write((ushort)0); // countKatu + sw.Write((ushort)1); // countMiss + sw.Write(12345678); // total score, irrelevant to this test + sw.Write((ushort)1000); // max combo, irrelevant to this test + sw.Write(false); // full combo, irrelevant to this test + sw.Write((int)LegacyMods.Hidden); // mods + sw.Write(string.Empty); // hp graph, irrelevant + sw.Write(DateTime.Now); // date, irrelevant + sw.Write(Array.Empty()); // replay data, irrelevant + sw.Write((long)1234); // legacy online ID, irrelevant + // importantly, no compressed `LegacyReplaySoloScoreInfo` here + } + + memoryStream.Seek(0, SeekOrigin.Begin); + var decoded = new TestLegacyScoreDecoder().Parse(memoryStream); + + Assert.Multiple(() => + { + Assert.That(decoded.ScoreInfo.Accuracy, Is.EqualTo((double)(198 * 300 + 50) / (200 * 300))); + Assert.That(decoded.ScoreInfo.Rank, Is.EqualTo(ScoreRank.A)); + }); + } + private static Score encodeThenDecode(int beatmapVersion, Score score, TestBeatmap beatmap) { var encodeStream = new MemoryStream(); From 17b9d842ab9a051ff6bc43d4ac5eb3d90dedfd93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 16 Jan 2024 21:08:02 +0100 Subject: [PATCH 2/2] Fix incorrect accuracy and rank population when decoding lazer replays Closes https://github.com/ppy/osu/issues/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 https://github.com/ppy/osu/issues/24061#issuecomment-1888438151. 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... --- osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs index ed11691674..b30fc7aee1 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs +++ b/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs @@ -11,6 +11,7 @@ using Newtonsoft.Json; using osu.Game.Beatmaps; using osu.Game.Beatmaps.Formats; using osu.Game.Beatmaps.Legacy; +using osu.Game.Database; using osu.Game.IO.Legacy; using osu.Game.Online.API.Requests.Responses; using osu.Game.Replays; @@ -37,6 +38,7 @@ namespace osu.Game.Scoring.Legacy }; WorkingBeatmap workingBeatmap; + byte[] compressedScoreInfo = null; using (SerializationReader sr = new SerializationReader(stream)) { @@ -105,8 +107,6 @@ namespace osu.Game.Scoring.Legacy else if (version >= 20121008) scoreInfo.LegacyOnlineID = sr.ReadInt32(); - byte[] compressedScoreInfo = null; - if (version >= 30000001) compressedScoreInfo = sr.ReadByteArray(); @@ -130,7 +130,10 @@ namespace osu.Game.Scoring.Legacy } } - PopulateAccuracy(score.ScoreInfo); + if (score.ScoreInfo.IsLegacyScore || compressedScoreInfo == null) + PopulateLegacyAccuracyAndRank(score.ScoreInfo); + else + populateLazerAccuracyAndRank(score.ScoreInfo); // 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. @@ -174,7 +177,7 @@ namespace osu.Game.Scoring.Legacy /// Legacy use only. /// /// The to populate. - public static void PopulateAccuracy(ScoreInfo score) + public static void PopulateLegacyAccuracyAndRank(ScoreInfo score) { int countMiss = score.GetCountMiss() ?? 0; int count50 = score.GetCount50() ?? 0; @@ -273,6 +276,18 @@ namespace osu.Game.Scoring.Legacy } } + private void populateLazerAccuracyAndRank(ScoreInfo scoreInfo) + { + scoreInfo.Accuracy = StandardisedScoreMigrationTools.ComputeAccuracy(scoreInfo); + + var rank = currentRuleset.CreateScoreProcessor().RankFromAccuracy(scoreInfo.Accuracy); + + foreach (var mod in scoreInfo.Mods.OfType()) + rank = mod.AdjustRank(rank, scoreInfo.Accuracy); + + scoreInfo.Rank = rank; + } + private void readLegacyReplay(Replay replay, StreamReader reader) { float lastTime = beatmapOffset;