From a7d5f2281ce59c004991070865a6c4cf640bc637 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 15:49:13 +0900 Subject: [PATCH 1/5] Apply beatmap offsets to legacy replay frame handling --- osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs | 8 +++++++- osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs | 6 +++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs index fefee370b9..9885fe5528 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs +++ b/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs @@ -23,6 +23,8 @@ namespace osu.Game.Scoring.Legacy private IBeatmap currentBeatmap; private Ruleset currentRuleset; + private float beatmapOffset; + public Score Parse(Stream stream) { var score = new Score @@ -72,6 +74,10 @@ namespace osu.Game.Scoring.Legacy currentBeatmap = workingBeatmap.GetPlayableBeatmap(currentRuleset.RulesetInfo, scoreInfo.Mods); scoreInfo.BeatmapInfo = currentBeatmap.BeatmapInfo; + // BeatmapVersion 4 and lower had an incorrect offset (stable has this set as 24ms off) + // As this is baked into hitobject timing (see `LegacyBeatmapDecoder`) we also need to apply this to replay frame timing. + beatmapOffset = currentBeatmap.BeatmapInfo.BeatmapVersion < 5 ? 24 : 0; + /* score.HpGraphString = */ sr.ReadString(); @@ -229,7 +235,7 @@ namespace osu.Game.Scoring.Legacy private void readLegacyReplay(Replay replay, StreamReader reader) { - float lastTime = 0; + float lastTime = beatmapOffset; ReplayFrame currentFrame = null; string[] frames = reader.ReadToEnd().Split(','); diff --git a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs index f0ead05280..6a321bed7a 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs +++ b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs @@ -111,6 +111,10 @@ namespace osu.Game.Scoring.Legacy { StringBuilder replayData = new StringBuilder(); + // BeatmapVersion 4 and lower had an incorrect offset (stable has this set as 24ms off) + // As this is baked into hitobject timing (see `LegacyBeatmapDecoder`) we also need to apply this to replay frame timing. + double offset = beatmap?.BeatmapInfo.BeatmapVersion < 5 ? -24 : 0; + if (score.Replay != null) { int lastTime = 0; @@ -120,7 +124,7 @@ namespace osu.Game.Scoring.Legacy var legacyFrame = getLegacyFrame(f); // Rounding because stable could only parse integral values - int time = (int)Math.Round(legacyFrame.Time); + int time = (int)Math.Round(legacyFrame.Time + offset); replayData.Append(FormattableString.Invariant($"{time - lastTime}|{legacyFrame.MouseX ?? 0}|{legacyFrame.MouseY ?? 0}|{(int)legacyFrame.ButtonState},")); lastTime = time; } From 59a7eb532203629e8dbd6dbc8f44cbc83c03fbde Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 16:34:21 +0900 Subject: [PATCH 2/5] Add test coverage ensuring offsets are correct before and after legacy replay encode --- .../Formats/LegacyScoreDecoderTest.cs | 60 ++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs index 2ba8c51a10..a50cef238a 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs @@ -8,6 +8,7 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Utils; using osu.Game.Beatmaps; +using osu.Game.Beatmaps.Formats; using osu.Game.Replays; using osu.Game.Rulesets; using osu.Game.Rulesets.Catch; @@ -64,6 +65,55 @@ namespace osu.Game.Tests.Beatmaps.Formats } } + [TestCase(3)] + [TestCase(6)] + [TestCase(LegacyBeatmapDecoder.LATEST_VERSION)] + public void TestLegacyBeatmapReplayOffsets(int beatmapVersion) + { + const double first_frame_time = 2000; + const double second_frame_time = 3000; + + var ruleset = new OsuRuleset().RulesetInfo; + var scoreInfo = TestResources.CreateTestScoreInfo(ruleset); + var beatmap = new TestBeatmap(ruleset) + { + BeatmapInfo = + { + BeatmapVersion = beatmapVersion + } + }; + + var score = new Score + { + ScoreInfo = scoreInfo, + Replay = new Replay + { + Frames = new List + { + new OsuReplayFrame(first_frame_time, OsuPlayfield.BASE_SIZE / 2, OsuAction.LeftButton), + new OsuReplayFrame(second_frame_time, OsuPlayfield.BASE_SIZE / 2, OsuAction.LeftButton) + } + } + }; + + // the "se" culture is used here, as it encodes the negative number sign as U+2212 MINUS SIGN, + // rather than the classic ASCII U+002D HYPHEN-MINUS. + CultureInfo.CurrentCulture = new CultureInfo("se"); + + var encodeStream = new MemoryStream(); + + var encoder = new LegacyScoreEncoder(score, beatmap); + encoder.Encode(encodeStream); + + var decodeStream = new MemoryStream(encodeStream.GetBuffer()); + + var decoder = new TestLegacyScoreDecoder(beatmapVersion); + var decodedAfterEncode = decoder.Parse(decodeStream); + + Assert.That(decodedAfterEncode.Replay.Frames[0].Time, Is.EqualTo(first_frame_time)); + Assert.That(decodedAfterEncode.Replay.Frames[1].Time, Is.EqualTo(second_frame_time)); + } + [Test] public void TestCultureInvariance() { @@ -118,6 +168,8 @@ namespace osu.Game.Tests.Beatmaps.Formats private class TestLegacyScoreDecoder : LegacyScoreDecoder { + private readonly int beatmapVersion; + private static readonly Dictionary rulesets = new Ruleset[] { new OsuRuleset(), @@ -126,6 +178,11 @@ namespace osu.Game.Tests.Beatmaps.Formats new ManiaRuleset() }.ToDictionary(ruleset => ((ILegacyRuleset)ruleset).LegacyID); + public TestLegacyScoreDecoder(int beatmapVersion = LegacyBeatmapDecoder.LATEST_VERSION) + { + this.beatmapVersion = beatmapVersion; + } + protected override Ruleset GetRuleset(int rulesetId) => rulesets[rulesetId]; protected override WorkingBeatmap GetBeatmap(string md5Hash) => new TestWorkingBeatmap(new Beatmap @@ -134,7 +191,8 @@ namespace osu.Game.Tests.Beatmaps.Formats { MD5Hash = md5Hash, Ruleset = new OsuRuleset().RulesetInfo, - Difficulty = new BeatmapDifficulty() + Difficulty = new BeatmapDifficulty(), + BeatmapVersion = beatmapVersion, } }); } From 2efae031c97a904c7876f6ee89093725b70f1bc5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 16:39:56 +0900 Subject: [PATCH 3/5] Add test coverage of decode specifically --- .../Formats/LegacyScoreDecoderTest.cs | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs index a50cef238a..39586bcd8c 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs @@ -65,10 +65,29 @@ namespace osu.Game.Tests.Beatmaps.Formats } } + [TestCase(3, true)] + [TestCase(6, false)] + [TestCase(LegacyBeatmapDecoder.LATEST_VERSION, false)] + public void TestLegacyBeatmapReplayOffsetsDecode(int beatmapVersion, bool offsetApplied) + { + const double first_frame_time = 48; + const double second_frame_time = 65; + + var decoder = new TestLegacyScoreDecoder(beatmapVersion); + + using (var resourceStream = TestResources.OpenResource("Replays/mania-replay.osr")) + { + var score = decoder.Parse(resourceStream); + + Assert.That(score.Replay.Frames[0].Time, Is.EqualTo(first_frame_time + (offsetApplied ? 24 : 0))); + Assert.That(score.Replay.Frames[1].Time, Is.EqualTo(second_frame_time + (offsetApplied ? 24 : 0))); + } + } + [TestCase(3)] [TestCase(6)] [TestCase(LegacyBeatmapDecoder.LATEST_VERSION)] - public void TestLegacyBeatmapReplayOffsets(int beatmapVersion) + public void TestLegacyBeatmapReplayOffsetsEncodeDecode(int beatmapVersion) { const double first_frame_time = 2000; const double second_frame_time = 3000; From a7554dcdf77637543960f712cbbcb521fc3f410b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 16:43:41 +0900 Subject: [PATCH 4/5] Use a constant for the early version timing offset --- osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs | 4 ++-- osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs | 8 ++++++-- osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs | 3 +-- osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs | 8 ++++---- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs index 39586bcd8c..1cd910789f 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs @@ -79,8 +79,8 @@ namespace osu.Game.Tests.Beatmaps.Formats { var score = decoder.Parse(resourceStream); - Assert.That(score.Replay.Frames[0].Time, Is.EqualTo(first_frame_time + (offsetApplied ? 24 : 0))); - Assert.That(score.Replay.Frames[1].Time, Is.EqualTo(second_frame_time + (offsetApplied ? 24 : 0))); + Assert.That(score.Replay.Frames[0].Time, Is.EqualTo(first_frame_time + (offsetApplied ? LegacyBeatmapDecoder.EARLY_VERSION_TIMING_OFFSET : 0))); + Assert.That(score.Replay.Frames[1].Time, Is.EqualTo(second_frame_time + (offsetApplied ? LegacyBeatmapDecoder.EARLY_VERSION_TIMING_OFFSET : 0))); } } diff --git a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs index e2a043490f..79d8bd3bb3 100644 --- a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs @@ -19,6 +19,11 @@ namespace osu.Game.Beatmaps.Formats { public class LegacyBeatmapDecoder : LegacyDecoder { + /// + /// An offset which needs to be applied to old beatmaps (v4 and lower) to correct timing changes that were applied at a game client level. + /// + public const int EARLY_VERSION_TIMING_OFFSET = 24; + internal static RulesetStore RulesetStore; private Beatmap beatmap; @@ -50,8 +55,7 @@ namespace osu.Game.Beatmaps.Formats RulesetStore = new AssemblyRulesetStore(); } - // BeatmapVersion 4 and lower had an incorrect offset (stable has this set as 24ms off) - offset = FormatVersion < 5 ? 24 : 0; + offset = FormatVersion < 5 ? EARLY_VERSION_TIMING_OFFSET : 0; } protected override Beatmap CreateTemplateObject() diff --git a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs index 9885fe5528..754ace82c5 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs +++ b/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs @@ -74,9 +74,8 @@ namespace osu.Game.Scoring.Legacy currentBeatmap = workingBeatmap.GetPlayableBeatmap(currentRuleset.RulesetInfo, scoreInfo.Mods); scoreInfo.BeatmapInfo = currentBeatmap.BeatmapInfo; - // BeatmapVersion 4 and lower had an incorrect offset (stable has this set as 24ms off) // As this is baked into hitobject timing (see `LegacyBeatmapDecoder`) we also need to apply this to replay frame timing. - beatmapOffset = currentBeatmap.BeatmapInfo.BeatmapVersion < 5 ? 24 : 0; + beatmapOffset = currentBeatmap.BeatmapInfo.BeatmapVersion < 5 ? LegacyBeatmapDecoder.EARLY_VERSION_TIMING_OFFSET : 0; /* score.HpGraphString = */ sr.ReadString(); diff --git a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs index 6a321bed7a..ae9afbf32e 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs +++ b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs @@ -1,12 +1,15 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +#nullable enable + using System; using System.IO; using System.Linq; using System.Text; using osu.Framework.Extensions; using osu.Game.Beatmaps; +using osu.Game.Beatmaps.Formats; using osu.Game.Extensions; using osu.Game.IO.Legacy; using osu.Game.Replays.Legacy; @@ -14,8 +17,6 @@ using osu.Game.Rulesets.Replays; using osu.Game.Rulesets.Replays.Types; using SharpCompress.Compressors.LZMA; -#nullable enable - namespace osu.Game.Scoring.Legacy { public class LegacyScoreEncoder @@ -111,9 +112,8 @@ namespace osu.Game.Scoring.Legacy { StringBuilder replayData = new StringBuilder(); - // BeatmapVersion 4 and lower had an incorrect offset (stable has this set as 24ms off) // As this is baked into hitobject timing (see `LegacyBeatmapDecoder`) we also need to apply this to replay frame timing. - double offset = beatmap?.BeatmapInfo.BeatmapVersion < 5 ? -24 : 0; + double offset = beatmap?.BeatmapInfo.BeatmapVersion < 5 ? -LegacyBeatmapDecoder.EARLY_VERSION_TIMING_OFFSET : 0; if (score.Replay != null) { From d6fc53579eb0d7d5d3eed86575006b8820d9135e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 17:00:49 +0900 Subject: [PATCH 5/5] Split out shared code for encode-decode cycle (and remove unrelated culture set) --- .../Formats/LegacyScoreDecoderTest.cs | 38 ++++++++----------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs index 1cd910789f..1474f2d277 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs @@ -115,19 +115,7 @@ namespace osu.Game.Tests.Beatmaps.Formats } }; - // the "se" culture is used here, as it encodes the negative number sign as U+2212 MINUS SIGN, - // rather than the classic ASCII U+002D HYPHEN-MINUS. - CultureInfo.CurrentCulture = new CultureInfo("se"); - - var encodeStream = new MemoryStream(); - - var encoder = new LegacyScoreEncoder(score, beatmap); - encoder.Encode(encodeStream); - - var decodeStream = new MemoryStream(encodeStream.GetBuffer()); - - var decoder = new TestLegacyScoreDecoder(beatmapVersion); - var decodedAfterEncode = decoder.Parse(decodeStream); + var decodedAfterEncode = encodeThenDecode(beatmapVersion, score, beatmap); Assert.That(decodedAfterEncode.Replay.Frames[0].Time, Is.EqualTo(first_frame_time)); Assert.That(decodedAfterEncode.Replay.Frames[1].Time, Is.EqualTo(second_frame_time)); @@ -155,15 +143,7 @@ namespace osu.Game.Tests.Beatmaps.Formats // rather than the classic ASCII U+002D HYPHEN-MINUS. CultureInfo.CurrentCulture = new CultureInfo("se"); - var encodeStream = new MemoryStream(); - - var encoder = new LegacyScoreEncoder(score, beatmap); - encoder.Encode(encodeStream); - - var decodeStream = new MemoryStream(encodeStream.GetBuffer()); - - var decoder = new TestLegacyScoreDecoder(); - var decodedAfterEncode = decoder.Parse(decodeStream); + var decodedAfterEncode = encodeThenDecode(LegacyBeatmapDecoder.LATEST_VERSION, score, beatmap); Assert.Multiple(() => { @@ -179,6 +159,20 @@ namespace osu.Game.Tests.Beatmaps.Formats }); } + private static Score encodeThenDecode(int beatmapVersion, Score score, TestBeatmap beatmap) + { + var encodeStream = new MemoryStream(); + + var encoder = new LegacyScoreEncoder(score, beatmap); + encoder.Encode(encodeStream); + + var decodeStream = new MemoryStream(encodeStream.GetBuffer()); + + var decoder = new TestLegacyScoreDecoder(beatmapVersion); + var decodedAfterEncode = decoder.Parse(decodeStream); + return decodedAfterEncode; + } + [TearDown] public void TearDown() {