From 5495c2090a76bfd6f6a5efd0fd61530395cc6a68 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 29 Feb 2024 14:15:29 +0800 Subject: [PATCH 1/9] Add test coverage of gameplay only running forwards --- .../Visual/Gameplay/TestScenePause.cs | 66 ++++++++++++------- 1 file changed, 43 insertions(+), 23 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs index 73aa3be73d..030f2592ed 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs @@ -16,6 +16,7 @@ using osu.Game.Configuration; using osu.Game.Graphics.Containers; using osu.Game.Graphics.Cursor; using osu.Game.Rulesets; +using osu.Game.Rulesets.UI; using osu.Game.Screens.Play; using osu.Game.Skinning; using osuTK; @@ -31,6 +32,9 @@ namespace osu.Game.Tests.Visual.Gameplay protected override Container Content => content; + private bool gameplayClockAlwaysGoingForward = true; + private double lastForwardCheckTime; + public TestScenePause() { base.Content.Add(content = new GlobalCursorDisplay { RelativeSizeAxes = Axes.Both }); @@ -67,12 +71,20 @@ namespace osu.Game.Tests.Visual.Gameplay confirmPausedWithNoOverlay(); } + [Test] + public void TestForwardPlaybackGuarantee() + { + hookForwardPlaybackCheck(); + + AddUntilStep("wait for forward playback", () => Player.GameplayClockContainer.CurrentTime > 1000); + AddStep("seek before gameplay", () => Player.GameplayClockContainer.Seek(-5000)); + + checkForwardPlayback(); + } + [Test] public void TestPauseWithLargeOffset() { - double lastStopTime; - bool alwaysGoingForward = true; - AddStep("force large offset", () => { var offset = (BindableDouble)LocalConfig.GetBindable(OsuSetting.AudioOffset); @@ -82,25 +94,7 @@ namespace osu.Game.Tests.Visual.Gameplay offset.Value = -5000; }); - AddStep("add time forward check hook", () => - { - lastStopTime = double.MinValue; - alwaysGoingForward = true; - - Player.OnUpdate += _ => - { - var masterClock = (MasterGameplayClockContainer)Player.GameplayClockContainer; - - double currentTime = masterClock.CurrentTime; - - bool goingForward = currentTime >= lastStopTime; - - alwaysGoingForward &= goingForward; - - if (!goingForward) - Logger.Log($"Went too far backwards (last stop: {lastStopTime:N1} current: {currentTime:N1})"); - }; - }); + hookForwardPlaybackCheck(); AddStep("move cursor outside", () => InputManager.MoveMouseTo(Player.ScreenSpaceDrawQuad.TopLeft - new Vector2(10))); @@ -108,11 +102,37 @@ namespace osu.Game.Tests.Visual.Gameplay resumeAndConfirm(); - AddAssert("time didn't go too far backwards", () => alwaysGoingForward); + checkForwardPlayback(); AddStep("reset offset", () => LocalConfig.SetValue(OsuSetting.AudioOffset, 0.0)); } + private void checkForwardPlayback() => AddAssert("time didn't go too far backwards", () => gameplayClockAlwaysGoingForward); + + private void hookForwardPlaybackCheck() + { + AddStep("add time forward check hook", () => + { + lastForwardCheckTime = double.MinValue; + gameplayClockAlwaysGoingForward = true; + + Player.OnUpdate += _ => + { + var frameStableClock = Player.ChildrenOfType().Single().Clock; + + double currentTime = frameStableClock.CurrentTime; + + bool goingForward = currentTime >= lastForwardCheckTime; + lastForwardCheckTime = currentTime; + + gameplayClockAlwaysGoingForward &= goingForward; + + if (!goingForward) + Logger.Log($"Went too far backwards (last stop: {lastForwardCheckTime:N1} current: {currentTime:N1})"); + }; + }); + } + [Test] public void TestPauseResume() { From 76e8aee9ccc13637691918de7c98d0cfaa8d1a7d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 29 Feb 2024 13:45:46 +0800 Subject: [PATCH 2/9] Disallow backwards seeks during frame stable operation when a replay is not attached --- osu.Game/Rulesets/UI/FrameStabilityContainer.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs index 8c9cb262af..4011034396 100644 --- a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs +++ b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs @@ -150,6 +150,17 @@ namespace osu.Game.Rulesets.UI state = PlaybackState.NotValid; } + // This is a hotfix for https://github.com/ppy/osu/issues/26879 while we figure how the hell time is seeking + // backwards by 11,850 ms for some users during gameplay. + // + // It basically says that "while we're running in frame stable mode, and don't have a replay attached, + // time should never go backwards". If it does, we stop running gameplay until it returns to normal. + if (!hasReplayAttached && FrameStablePlayback && proposedTime > referenceClock.CurrentTime) + { + state = PlaybackState.NotValid; + return; + } + // if the proposed time is the same as the current time, assume that the clock will continue progressing in the same direction as previously. // this avoids spurious flips in direction from -1 to 1 during rewinds. if (state == PlaybackState.Valid && proposedTime != manualClock.CurrentTime) From 3355764a68535facc627433f7c58d1040f3928e2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 29 Feb 2024 18:21:44 +0800 Subject: [PATCH 3/9] "Fix" tests --- osu.Game/Rulesets/UI/FrameStabilityContainer.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs index 4011034396..487c12830f 100644 --- a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs +++ b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs @@ -6,6 +6,7 @@ using System.Diagnostics; using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Bindables; +using osu.Framework.Development; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Timing; @@ -155,7 +156,7 @@ namespace osu.Game.Rulesets.UI // // It basically says that "while we're running in frame stable mode, and don't have a replay attached, // time should never go backwards". If it does, we stop running gameplay until it returns to normal. - if (!hasReplayAttached && FrameStablePlayback && proposedTime > referenceClock.CurrentTime) + if (!hasReplayAttached && FrameStablePlayback && proposedTime > referenceClock.CurrentTime && !DebugUtils.IsNUnitRunning) { state = PlaybackState.NotValid; return; From 3a780e2b676eee99f0b0e845c12348977df22695 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 29 Feb 2024 18:27:28 +0800 Subject: [PATCH 4/9] Add logging when workaround is hit --- osu.Game/Rulesets/UI/FrameStabilityContainer.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs index 487c12830f..03f3b8788f 100644 --- a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs +++ b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs @@ -9,6 +9,7 @@ using osu.Framework.Bindables; using osu.Framework.Development; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; +using osu.Framework.Logging; using osu.Framework.Timing; using osu.Game.Input.Handlers; using osu.Game.Screens.Play; @@ -158,6 +159,7 @@ namespace osu.Game.Rulesets.UI // time should never go backwards". If it does, we stop running gameplay until it returns to normal. if (!hasReplayAttached && FrameStablePlayback && proposedTime > referenceClock.CurrentTime && !DebugUtils.IsNUnitRunning) { + Logger.Log($"Denying backwards seek during gameplay (reference: {referenceClock.CurrentTime:N2} stable: {proposedTime:N2})"); state = PlaybackState.NotValid; return; } From 4184a5c1ef380318dae27492c0f68c4ba211aa0e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 29 Feb 2024 20:34:38 +0800 Subject: [PATCH 5/9] Add flag to allow backwards seeks in tests --- .../TestSceneTimingBasedNoteColouring.cs | 21 ++++++++++++------- .../NonVisual/FirstAvailableHitWindowsTest.cs | 1 + .../TestSceneCompletionCancellation.cs | 2 ++ .../TestSceneFrameStabilityContainer.cs | 3 +++ .../TestSceneGameplaySamplePlayback.cs | 2 ++ .../TestSceneGameplaySampleTriggerSource.cs | 2 ++ .../Visual/Gameplay/TestSceneHitErrorMeter.cs | 1 + .../Gameplay/TestScenePoolingRuleset.cs | 1 + .../Gameplay/TestSceneStoryboardWithOutro.cs | 2 ++ osu.Game/Rulesets/UI/DrawableRuleset.cs | 20 ++++++++++++++++++ .../Rulesets/UI/FrameStabilityContainer.cs | 5 +++-- osu.Game/Tests/Visual/PlayerTestScene.cs | 12 ++++++++++- 12 files changed, 61 insertions(+), 11 deletions(-) diff --git a/osu.Game.Rulesets.Mania.Tests/TestSceneTimingBasedNoteColouring.cs b/osu.Game.Rulesets.Mania.Tests/TestSceneTimingBasedNoteColouring.cs index 81557c198d..b5b265792b 100644 --- a/osu.Game.Rulesets.Mania.Tests/TestSceneTimingBasedNoteColouring.cs +++ b/osu.Game.Rulesets.Mania.Tests/TestSceneTimingBasedNoteColouring.cs @@ -34,16 +34,21 @@ namespace osu.Game.Rulesets.Mania.Tests [SetUpSteps] public void SetUpSteps() { - AddStep("setup hierarchy", () => Child = new Container + AddStep("setup hierarchy", () => { - Clock = new FramedClock(clock = new ManualClock()), - RelativeSizeAxes = Axes.Both, - Anchor = Anchor.Centre, - Origin = Anchor.Centre, - Children = new[] + Child = new Container { - drawableRuleset = (DrawableManiaRuleset)Ruleset.Value.CreateInstance().CreateDrawableRulesetWith(createTestBeatmap()) - } + Clock = new FramedClock(clock = new ManualClock()), + RelativeSizeAxes = Axes.Both, + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Children = new[] + { + drawableRuleset = (DrawableManiaRuleset)Ruleset.Value.CreateInstance().CreateDrawableRulesetWith(createTestBeatmap()) + } + }; + + drawableRuleset.AllowBackwardsSeeks = true; }); AddStep("retrieve config bindable", () => { diff --git a/osu.Game.Tests/NonVisual/FirstAvailableHitWindowsTest.cs b/osu.Game.Tests/NonVisual/FirstAvailableHitWindowsTest.cs index 0bdd0ceae6..d4b69c1be2 100644 --- a/osu.Game.Tests/NonVisual/FirstAvailableHitWindowsTest.cs +++ b/osu.Game.Tests/NonVisual/FirstAvailableHitWindowsTest.cs @@ -100,6 +100,7 @@ namespace osu.Game.Tests.NonVisual public override Container FrameStableComponents { get; } public override IFrameStableClock FrameStableClock { get; } internal override bool FrameStablePlayback { get; set; } + public override bool AllowBackwardsSeeks { get; set; } public override IReadOnlyList Mods { get; } public override double GameplayStartTime { get; } diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneCompletionCancellation.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneCompletionCancellation.cs index 434d853992..f19f4b6690 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneCompletionCancellation.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneCompletionCancellation.cs @@ -29,6 +29,8 @@ namespace osu.Game.Tests.Visual.Gameplay protected override bool AllowFail => false; + protected override bool AllowBackwardsSeeks => true; + [SetUpSteps] public override void SetUpSteps() { diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneFrameStabilityContainer.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneFrameStabilityContainer.cs index 98a97e1d23..0cff675b28 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneFrameStabilityContainer.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneFrameStabilityContainer.cs @@ -131,6 +131,9 @@ namespace osu.Game.Tests.Visual.Gameplay private void createStabilityContainer(double gameplayStartTime = double.MinValue) => AddStep("create container", () => mainContainer.Child = new FrameStabilityContainer(gameplayStartTime) + { + AllowBackwardsSeeks = true, + } .WithChild(consumer = new ClockConsumingChild())); private void seekManualTo(double time) => AddStep($"seek manual clock to {time}", () => manualClock.CurrentTime = time); diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySamplePlayback.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySamplePlayback.cs index 3d35860fef..057197e819 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySamplePlayback.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySamplePlayback.cs @@ -16,6 +16,8 @@ namespace osu.Game.Tests.Visual.Gameplay { public partial class TestSceneGameplaySamplePlayback : PlayerTestScene { + protected override bool AllowBackwardsSeeks => true; + [Test] public void TestAllSamplesStopDuringSeek() { diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySampleTriggerSource.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySampleTriggerSource.cs index 3cbd5eefac..6981591193 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySampleTriggerSource.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySampleTriggerSource.cs @@ -28,6 +28,8 @@ namespace osu.Game.Tests.Visual.Gameplay { public partial class TestSceneGameplaySampleTriggerSource : PlayerTestScene { + protected override bool AllowBackwardsSeeks => true; + private TestGameplaySampleTriggerSource sampleTriggerSource = null!; protected override Ruleset CreatePlayerRuleset() => new OsuRuleset(); diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs index 56900a0549..e57177498d 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs @@ -288,6 +288,7 @@ namespace osu.Game.Tests.Visual.Gameplay public override Container FrameStableComponents { get; } public override IFrameStableClock FrameStableClock { get; } internal override bool FrameStablePlayback { get; set; } + public override bool AllowBackwardsSeeks { get; set; } public override IReadOnlyList Mods { get; } public override double GameplayStartTime { get; } diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePoolingRuleset.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePoolingRuleset.cs index b567e8de8d..88effb4a7b 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePoolingRuleset.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePoolingRuleset.cs @@ -269,6 +269,7 @@ namespace osu.Game.Tests.Visual.Gameplay drawableRuleset = (TestDrawablePoolingRuleset)ruleset.CreateDrawableRulesetWith(CreateWorkingBeatmap(beatmap).GetPlayableBeatmap(ruleset.RulesetInfo)); drawableRuleset.FrameStablePlayback = true; + drawableRuleset.AllowBackwardsSeeks = true; drawableRuleset.PoolSize = poolSize; Child = new Container diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneStoryboardWithOutro.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneStoryboardWithOutro.cs index 98825b27d4..f532921d63 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneStoryboardWithOutro.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneStoryboardWithOutro.cs @@ -31,6 +31,8 @@ namespace osu.Game.Tests.Visual.Gameplay { protected override bool HasCustomSteps => true; + protected override bool AllowBackwardsSeeks => true; + protected new OutroPlayer Player => (OutroPlayer)base.Player; private double currentBeatmapDuration; diff --git a/osu.Game/Rulesets/UI/DrawableRuleset.cs b/osu.Game/Rulesets/UI/DrawableRuleset.cs index 4aeb3d4862..13e28279e6 100644 --- a/osu.Game/Rulesets/UI/DrawableRuleset.cs +++ b/osu.Game/Rulesets/UI/DrawableRuleset.cs @@ -81,6 +81,19 @@ namespace osu.Game.Rulesets.UI public override IFrameStableClock FrameStableClock => frameStabilityContainer; + private bool allowBackwardsSeeks; + + public override bool AllowBackwardsSeeks + { + get => allowBackwardsSeeks; + set + { + allowBackwardsSeeks = value; + if (frameStabilityContainer != null) + frameStabilityContainer.AllowBackwardsSeeks = value; + } + } + private bool frameStablePlayback = true; internal override bool FrameStablePlayback @@ -178,6 +191,7 @@ namespace osu.Game.Rulesets.UI InternalChild = frameStabilityContainer = new FrameStabilityContainer(GameplayStartTime) { FrameStablePlayback = FrameStablePlayback, + AllowBackwardsSeeks = AllowBackwardsSeeks, Children = new Drawable[] { FrameStableComponents, @@ -463,6 +477,12 @@ namespace osu.Game.Rulesets.UI /// internal abstract bool FrameStablePlayback { get; set; } + /// + /// When a replay is not attached, we usually block any backwards seeks. + /// This will bypass the check. Should only be used for tests. + /// + public abstract bool AllowBackwardsSeeks { get; set; } + /// /// The mods which are to be applied. /// diff --git a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs index 03f3b8788f..ab48711955 100644 --- a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs +++ b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs @@ -6,7 +6,6 @@ using System.Diagnostics; using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Bindables; -using osu.Framework.Development; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Logging; @@ -26,6 +25,8 @@ namespace osu.Game.Rulesets.UI { public ReplayInputHandler? ReplayInputHandler { get; set; } + public bool AllowBackwardsSeeks { get; set; } + /// /// The number of CPU milliseconds to spend at most during seek catch-up. /// @@ -157,7 +158,7 @@ namespace osu.Game.Rulesets.UI // // It basically says that "while we're running in frame stable mode, and don't have a replay attached, // time should never go backwards". If it does, we stop running gameplay until it returns to normal. - if (!hasReplayAttached && FrameStablePlayback && proposedTime > referenceClock.CurrentTime && !DebugUtils.IsNUnitRunning) + if (!hasReplayAttached && FrameStablePlayback && proposedTime > referenceClock.CurrentTime && !AllowBackwardsSeeks) { Logger.Log($"Denying backwards seek during gameplay (reference: {referenceClock.CurrentTime:N2} stable: {proposedTime:N2})"); state = PlaybackState.NotValid; diff --git a/osu.Game/Tests/Visual/PlayerTestScene.cs b/osu.Game/Tests/Visual/PlayerTestScene.cs index ee184c1f35..43d779261c 100644 --- a/osu.Game/Tests/Visual/PlayerTestScene.cs +++ b/osu.Game/Tests/Visual/PlayerTestScene.cs @@ -70,10 +70,20 @@ namespace osu.Game.Tests.Visual AddStep($"Load player for {CreatePlayerRuleset().Description}", LoadPlayer); AddUntilStep("player loaded", () => Player.IsLoaded && Player.Alpha == 1); + + if (AllowBackwardsSeeks) + { + AddStep("allow backwards seeking", () => + { + Player.DrawableRuleset.AllowBackwardsSeeks = AllowBackwardsSeeks; + }); + } } protected virtual bool AllowFail => false; + protected virtual bool AllowBackwardsSeeks => false; + protected virtual bool Autoplay => false; protected void LoadPlayer() => LoadPlayer(Array.Empty()); @@ -126,6 +136,6 @@ namespace osu.Game.Tests.Visual protected sealed override Ruleset CreateRuleset() => CreatePlayerRuleset(); - protected virtual TestPlayer CreatePlayer(Ruleset ruleset) => new TestPlayer(false, false); + protected virtual TestPlayer CreatePlayer(Ruleset ruleset) => new TestPlayer(false, false, AllowBackwardsSeeks); } } From cc8b838bd45d5df2be723b6cb1ddd82bec5622b8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 29 Feb 2024 23:03:27 +0800 Subject: [PATCH 6/9] Add comprehensive log output to help figure out problematic clocks --- osu.Game/Beatmaps/FramedBeatmapClock.cs | 26 ++++++++++++++++++- .../Rulesets/UI/FrameStabilityContainer.cs | 7 +++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/FramedBeatmapClock.cs b/osu.Game/Beatmaps/FramedBeatmapClock.cs index 49dff96ff1..af7be235fc 100644 --- a/osu.Game/Beatmaps/FramedBeatmapClock.cs +++ b/osu.Game/Beatmaps/FramedBeatmapClock.cs @@ -38,6 +38,7 @@ namespace osu.Game.Beatmaps private IDisposable? beatmapOffsetSubscription; private readonly DecouplingFramedClock decoupledTrack; + private readonly InterpolatingFramedClock interpolatedTrack; [Resolved] private OsuConfigManager config { get; set; } = null!; @@ -58,7 +59,7 @@ namespace osu.Game.Beatmaps // An interpolating clock is used to ensure precise time values even when the host audio subsystem is not reporting // high precision times (on windows there's generally only 5-10ms reporting intervals, as an example). - var interpolatedTrack = new InterpolatingFramedClock(decoupledTrack); + interpolatedTrack = new InterpolatingFramedClock(decoupledTrack); if (applyOffsets) { @@ -190,5 +191,28 @@ namespace osu.Game.Beatmaps base.Dispose(isDisposing); beatmapOffsetSubscription?.Dispose(); } + + public string GetSnapshot() + { + return + $"originalSource: {output(Source)}\n" + + $"userGlobalOffsetClock: {output(userGlobalOffsetClock)}\n" + + $"platformOffsetClock: {output(platformOffsetClock)}\n" + + $"userBeatmapOffsetClock: {output(userBeatmapOffsetClock)}\n" + + $"interpolatedTrack: {output(interpolatedTrack)}\n" + + $"decoupledTrack: {output(decoupledTrack)}\n" + + $"finalClockSource: {output(finalClockSource)}\n"; + + string output(IClock? clock) + { + if (clock == null) + return "null"; + + if (clock is IFrameBasedClock framed) + return $"current: {clock.CurrentTime:N2} running: {clock.IsRunning} rate: {clock.Rate} elapsed: {framed.ElapsedFrameTime:N2}"; + + return $"current: {clock.CurrentTime:N2} running: {clock.IsRunning} rate: {clock.Rate}"; + } + } } } diff --git a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs index ab48711955..c09018e8ca 100644 --- a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs +++ b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs @@ -3,13 +3,16 @@ using System; using System.Diagnostics; +using System.Linq; using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Logging; +using osu.Framework.Testing; using osu.Framework.Timing; +using osu.Game.Beatmaps; using osu.Game.Input.Handlers; using osu.Game.Screens.Play; @@ -161,6 +164,10 @@ namespace osu.Game.Rulesets.UI if (!hasReplayAttached && FrameStablePlayback && proposedTime > referenceClock.CurrentTime && !AllowBackwardsSeeks) { Logger.Log($"Denying backwards seek during gameplay (reference: {referenceClock.CurrentTime:N2} stable: {proposedTime:N2})"); + + if (parentGameplayClock is GameplayClockContainer gcc) + Logger.Log($"{gcc.ChildrenOfType().Single().GetSnapshot()}"); + state = PlaybackState.NotValid; return; } From 59b9d29a79c2520cda4b1fca7d2b8373e501c71d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 29 Feb 2024 23:29:50 +0800 Subject: [PATCH 7/9] Fix formatting? --- .../Visual/Gameplay/TestSceneFrameStabilityContainer.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneFrameStabilityContainer.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneFrameStabilityContainer.cs index 0cff675b28..c2999e3f5a 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneFrameStabilityContainer.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneFrameStabilityContainer.cs @@ -130,11 +130,12 @@ namespace osu.Game.Tests.Visual.Gameplay } private void createStabilityContainer(double gameplayStartTime = double.MinValue) => AddStep("create container", () => + { mainContainer.Child = new FrameStabilityContainer(gameplayStartTime) - { - AllowBackwardsSeeks = true, - } - .WithChild(consumer = new ClockConsumingChild())); + { + AllowBackwardsSeeks = true, + }.WithChild(consumer = new ClockConsumingChild()); + }); private void seekManualTo(double time) => AddStep($"seek manual clock to {time}", () => manualClock.CurrentTime = time); From 4ad8bbb9e2d17c6c35fb26f37004c077af87dddf Mon Sep 17 00:00:00 2001 From: cdwcgt Date: Fri, 1 Mar 2024 13:20:37 +0900 Subject: [PATCH 8/9] remove useless DrawablePool --- osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs b/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs index 0f2f9dc323..8bb5ee3617 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs @@ -107,8 +107,6 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters JudgementSpacing.BindValueChanged(_ => updateMetrics(), true); } - private readonly DrawablePool judgementLinePool = new DrawablePool(50); - public void Push(HitErrorShape shape) { Add(shape); From 19ed78eef57828c1da4210e5796bd5e7b7fcdb48 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 1 Mar 2024 12:34:21 +0800 Subject: [PATCH 9/9] Log backwards seeks to sentry --- osu.Game/OsuGame.cs | 3 +++ .../Rulesets/UI/FrameStabilityContainer.cs | 15 ++++++++++--- .../Utils/SentryOnlyDiagnosticsException.cs | 21 +++++++++++++++++++ 3 files changed, 36 insertions(+), 3 deletions(-) create mode 100644 osu.Game/Utils/SentryOnlyDiagnosticsException.cs diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 7d128a808a..eb1219f183 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -1190,6 +1190,9 @@ namespace osu.Game { if (entry.Level < LogLevel.Important || entry.Target > LoggingTarget.Database || entry.Target == null) return; + if (entry.Exception is SentryOnlyDiagnosticsException) + return; + const int short_term_display_limit = 3; if (recentLogCount < short_term_display_limit) diff --git a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs index c09018e8ca..884310e44c 100644 --- a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs +++ b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs @@ -15,6 +15,7 @@ using osu.Framework.Timing; using osu.Game.Beatmaps; using osu.Game.Input.Handlers; using osu.Game.Screens.Play; +using osu.Game.Utils; namespace osu.Game.Rulesets.UI { @@ -29,6 +30,7 @@ namespace osu.Game.Rulesets.UI public ReplayInputHandler? ReplayInputHandler { get; set; } public bool AllowBackwardsSeeks { get; set; } + private double? lastBackwardsSeekLogTime; /// /// The number of CPU milliseconds to spend at most during seek catch-up. @@ -163,10 +165,17 @@ namespace osu.Game.Rulesets.UI // time should never go backwards". If it does, we stop running gameplay until it returns to normal. if (!hasReplayAttached && FrameStablePlayback && proposedTime > referenceClock.CurrentTime && !AllowBackwardsSeeks) { - Logger.Log($"Denying backwards seek during gameplay (reference: {referenceClock.CurrentTime:N2} stable: {proposedTime:N2})"); + if (lastBackwardsSeekLogTime == null || Math.Abs(Clock.CurrentTime - lastBackwardsSeekLogTime.Value) > 1000) + { + lastBackwardsSeekLogTime = Clock.CurrentTime; - if (parentGameplayClock is GameplayClockContainer gcc) - Logger.Log($"{gcc.ChildrenOfType().Single().GetSnapshot()}"); + string loggableContent = $"Denying backwards seek during gameplay (reference: {referenceClock.CurrentTime:N2} stable: {proposedTime:N2})"; + + if (parentGameplayClock is GameplayClockContainer gcc) + loggableContent += $"\n{gcc.ChildrenOfType().Single().GetSnapshot()}"; + + Logger.Error(new SentryOnlyDiagnosticsException("backwards seek"), loggableContent); + } state = PlaybackState.NotValid; return; diff --git a/osu.Game/Utils/SentryOnlyDiagnosticsException.cs b/osu.Game/Utils/SentryOnlyDiagnosticsException.cs new file mode 100644 index 0000000000..1659b8a213 --- /dev/null +++ b/osu.Game/Utils/SentryOnlyDiagnosticsException.cs @@ -0,0 +1,21 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; + +namespace osu.Game.Utils +{ + /// + /// Log to sentry without showing an error notification to the user. + /// + /// + /// This can be used to convey important diagnostics to us developers without + /// getting in the user's way. Should be used sparingly. + internal class SentryOnlyDiagnosticsException : Exception + { + public SentryOnlyDiagnosticsException(string message) + : base(message) + { + } + } +}