From 8854db0264b322456e09dfb0eeb247c42e46c23a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 16 May 2025 12:45:47 +0200 Subject: [PATCH 1/2] Add failing test case --- .../Visual/TestSceneReplayStability.cs | 59 +++++++++++++++++++ .../Tests/Visual/ReplayStabilityTestScene.cs | 11 +++- 2 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 osu.Game.Tests/Visual/TestSceneReplayStability.cs diff --git a/osu.Game.Tests/Visual/TestSceneReplayStability.cs b/osu.Game.Tests/Visual/TestSceneReplayStability.cs new file mode 100644 index 0000000000..749493c4b1 --- /dev/null +++ b/osu.Game.Tests/Visual/TestSceneReplayStability.cs @@ -0,0 +1,59 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Linq; +using NUnit.Framework; +using osu.Game.Replays; +using osu.Game.Rulesets.Osu; +using osu.Game.Rulesets.Osu.Beatmaps; +using osu.Game.Rulesets.Osu.Objects; +using osu.Game.Rulesets.Osu.Replays; +using osu.Game.Rulesets.Osu.UI; +using osu.Game.Rulesets.Replays; +using osu.Game.Rulesets.Scoring; +using osuTK; + +namespace osu.Game.Tests.Visual +{ + public partial class TestSceneReplayStability : ReplayStabilityTestScene + { + [Test] + public void TestOutrageouslyLargeLeadInTime() + { + // "graciously borrowed" from https://osu.ppy.sh/beatmapsets/948643#osu/1981090 + const double lead_in_time = 2147272727; + const double hit_circle_time = 100; + + var beatmap = new OsuBeatmap + { + HitObjects = + { + new HitCircle + { + StartTime = hit_circle_time, + Position = OsuPlayfield.BASE_SIZE / 2 + } + }, + AudioLeadIn = lead_in_time, + BeatmapInfo = + { + Ruleset = new OsuRuleset().RulesetInfo, + }, + }; + + var replay = new Replay + { + Frames = Enumerable.Range(0, 300).Select(t => new OsuReplayFrame(-lead_in_time + 40 * t, new Vector2(t), t % 2 == 0 ? [] : [OsuAction.LeftButton])) + .Concat([ + new OsuReplayFrame(0, OsuPlayfield.BASE_SIZE / 2), + new OsuReplayFrame(hit_circle_time, OsuPlayfield.BASE_SIZE / 2, OsuAction.LeftButton), + new OsuReplayFrame(hit_circle_time + 20, OsuPlayfield.BASE_SIZE / 2), + ]) + .Cast() + .ToList(), + }; + + RunTest(beatmap, replay, [HitResult.Great]); + } + } +} diff --git a/osu.Game/Tests/Visual/ReplayStabilityTestScene.cs b/osu.Game/Tests/Visual/ReplayStabilityTestScene.cs index 13abedf611..af41617a7b 100644 --- a/osu.Game/Tests/Visual/ReplayStabilityTestScene.cs +++ b/osu.Game/Tests/Visual/ReplayStabilityTestScene.cs @@ -19,7 +19,7 @@ using osu.Game.Screens.Play; namespace osu.Game.Tests.Visual { /// - /// The goal of this abstract test class is to ensure that the process of exporting of a replay does not affect its playback. + /// The goal of this abstract test class is to ensure that the process of exporting and re-importing of a replay does not affect its playback. /// Use to exercise that property. /// [HeadlessTest] @@ -51,6 +51,7 @@ namespace osu.Game.Tests.Visual AddStep(@"push player", () => pushNewPlayer(originalScore)); AddUntilStep(@"wait until player is loaded", () => currentPlayer.IsCurrentScreen()); + skipIntroIfPresent(); AddUntilStep(@"wait for completion", () => currentPlayer.GameplayState.HasCompleted); AddAssert(@"judgement results before encode are correct", () => results.Select(r => r.Type), () => Is.EquivalentTo(expectedResults)); @@ -71,6 +72,7 @@ namespace osu.Game.Tests.Visual AddStep(@"push player", () => pushNewPlayer(decodedScore)); AddUntilStep(@"Wait until player is loaded", () => currentPlayer.IsCurrentScreen()); + skipIntroIfPresent(); AddUntilStep(@"Wait for completion", () => currentPlayer.GameplayState.HasCompleted); AddAssert(@"judgement results after encode are correct", () => results.Select(r => r.Type), () => Is.EquivalentTo(expectedResults)); } @@ -90,6 +92,13 @@ namespace osu.Game.Tests.Visual results.Clear(); } + private void skipIntroIfPresent() => + AddStep(@"skip intro if present", () => + { + if (currentPlayer.ChildrenOfType().Single().CurrentTime < 0) + currentPlayer.Seek(0); + }); + private class TestScoreDecoder : LegacyScoreDecoder { private readonly WorkingBeatmap beatmap; From 56e17dd7ba751784e4821d01935c6329b7036640 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 16 May 2025 12:52:49 +0200 Subject: [PATCH 2/2] Fix possible replay playback inaccuracy with very large lead-in time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes https://github.com/ppy/osu/issues/22086 While this *is* a case of aspire-tier breakage, I hesitate to dismiss it as wontfix, because the fix is somewhat easy, and probably results in better accuracy across the board. The issue in question here manifests when there is a significant amount of lead-in time, like what the beatmap linked in the aforementioned issue (https://osu.ppy.sh/beatmapsets/948643#osu/1981090) does. Both stable and lazer record replay frames using absolute timestamps, *but* the legacy replay format after the first frame uses time *deltas*, i.e. amounts of time elapsed since the previous frame. This means that the decoding process of the replay has to reconstruct absolute timestamps by doing cumulative summation starting from the first frame. When the very first frame has a timestamp that is very large in magnitude (say, like the negative 2 billion that the beatmap in question uses), this will lead to error if the cumulative summation is using floating point numbers, because it will be adding a small magnitude frame delta to a large magnitude cumulative absolute time. Which means that sometimes adding the frame delta to the cumulative time *will not change the cumulative time*, leading to the loss of time and thus replay de-synchronisation. Knowing that the legacy replay format only deals in integral time values, however, this can be circumvented by just using a large enough integral number type for the cumulative time tracking instead. I think `long` in this case can be safely used "large enough" for our purposes: > Console.WriteLine(long.MaxValue); 9223372036854775807 9 223 372 036 854 775 807 ms equals 292 277 024,6269277149 years --- osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs index 2eec12ac28..a32c05c4eb 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs +++ b/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs @@ -31,7 +31,7 @@ namespace osu.Game.Scoring.Legacy private IBeatmap currentBeatmap; private Ruleset currentRuleset; - private float beatmapOffset; + private long beatmapOffset; public Score Parse(Stream stream) { @@ -262,7 +262,7 @@ namespace osu.Game.Scoring.Legacy private void readLegacyReplay(Replay replay, StreamReader reader) { - float lastTime = beatmapOffset; + long lastTime = beatmapOffset; var legacyFrames = new List(); string[] frames = reader.ReadToEnd().Split(','); @@ -283,7 +283,7 @@ namespace osu.Game.Scoring.Legacy // In mania, mouseX encodes the pressed keys in the lower 20 bits int mouseXParseLimit = currentRuleset.RulesetInfo.OnlineID == 3 ? (1 << 20) - 1 : Parsing.MAX_COORDINATE_VALUE; - float diff = Parsing.ParseFloat(split[0]); + int diff = Parsing.ParseInt(split[0]); float mouseX = Parsing.ParseFloat(split[1], mouseXParseLimit); float mouseY = Parsing.ParseFloat(split[2], Parsing.MAX_COORDINATE_VALUE);