From 82906300b4abb7de2477c94435d7b9067f0b30ee Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Aug 2025 19:16:34 +0900 Subject: [PATCH 1/4] Ignore more potentially incorrect data from BASS This is still a workaround but arguably it's something we could leave in place without much loss. I think this at least feels better than the previous code. Notably, you could argue the test coverage of the fail case is lower since made it implicit that all tests will avoid the "backwards seek" detections. But we never really had tests correctly- fail on the original so I don't see any loss of value. --- .../TestSceneTimingBasedNoteColouring.cs | 2 -- .../NonVisual/FirstAvailableHitWindowsTest.cs | 1 - .../TestSceneFrameStabilityContainer.cs | 5 +---- .../Visual/Gameplay/TestSceneHitErrorMeter.cs | 1 - .../Gameplay/TestScenePoolingRuleset.cs | 1 - osu.Game/Rulesets/UI/DrawableRuleset.cs | 20 ------------------- .../Rulesets/UI/FrameStabilityContainer.cs | 17 ++++++++-------- osu.Game/Tests/Visual/PlayerTestScene.cs | 8 -------- 8 files changed, 10 insertions(+), 45 deletions(-) diff --git a/osu.Game.Rulesets.Mania.Tests/TestSceneTimingBasedNoteColouring.cs b/osu.Game.Rulesets.Mania.Tests/TestSceneTimingBasedNoteColouring.cs index b5b265792b..eb47e96670 100644 --- a/osu.Game.Rulesets.Mania.Tests/TestSceneTimingBasedNoteColouring.cs +++ b/osu.Game.Rulesets.Mania.Tests/TestSceneTimingBasedNoteColouring.cs @@ -47,8 +47,6 @@ namespace osu.Game.Rulesets.Mania.Tests 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 cfe523fdd5..69c98351ad 100644 --- a/osu.Game.Tests/NonVisual/FirstAvailableHitWindowsTest.cs +++ b/osu.Game.Tests/NonVisual/FirstAvailableHitWindowsTest.cs @@ -101,7 +101,6 @@ 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/TestSceneFrameStabilityContainer.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneFrameStabilityContainer.cs index c2999e3f5a..dfaebccf32 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneFrameStabilityContainer.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneFrameStabilityContainer.cs @@ -131,10 +131,7 @@ 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()); + mainContainer.Child = new FrameStabilityContainer(gameplayStartTime).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/TestSceneHitErrorMeter.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs index 551116e818..24215ed925 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs @@ -284,7 +284,6 @@ 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 88effb4a7b..b567e8de8d 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePoolingRuleset.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePoolingRuleset.cs @@ -269,7 +269,6 @@ 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/Rulesets/UI/DrawableRuleset.cs b/osu.Game/Rulesets/UI/DrawableRuleset.cs index 6b2387eb9b..31ff81456c 100644 --- a/osu.Game/Rulesets/UI/DrawableRuleset.cs +++ b/osu.Game/Rulesets/UI/DrawableRuleset.cs @@ -102,19 +102,6 @@ namespace osu.Game.Rulesets.UI private FrameStabilityContainer frameStabilityContainer; private DrawableRulesetDependencies dependencies; - 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 @@ -190,7 +177,6 @@ namespace osu.Game.Rulesets.UI InternalChild = frameStabilityContainer = new FrameStabilityContainer(GameplayStartTime) { FrameStablePlayback = FrameStablePlayback, - AllowBackwardsSeeks = AllowBackwardsSeeks, Children = new Drawable[] { FrameStableComponents, @@ -481,12 +467,6 @@ 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 50111e64a8..3f4700c401 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.Logging; @@ -25,7 +26,6 @@ namespace osu.Game.Rulesets.UI { public ReplayInputHandler? ReplayInputHandler { get; set; } - public bool AllowBackwardsSeeks { get; set; } private double? lastBackwardsSeekLogTime; /// @@ -154,17 +154,18 @@ 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 && !AllowBackwardsSeeks) + bool allowReferenceClockSeeks = hasReplayAttached || DebugUtils.IsNUnitRunning || !FrameStablePlayback; + + // This is a hotfix for ongoing bass issues we are trying to resolve (see https://www.un4seen.com/forum/?topic=20482.msg145474#msg145474) + // In gameplay we should always be seeking using the + if (!allowReferenceClockSeeks && Math.Abs(proposedTime - referenceClock.CurrentTime) > 1000) { if (lastBackwardsSeekLogTime == null || Math.Abs(Clock.CurrentTime - lastBackwardsSeekLogTime.Value) > 1000) { lastBackwardsSeekLogTime = Clock.CurrentTime; - Logger.Log($"Denying backwards seek during gameplay (reference: {referenceClock.CurrentTime:N2} stable: {proposedTime:N2})"); + Logger.Log("Ignoring likely invalid time value provided by BASS during gameplay"); + Logger.Log($"- provided: {referenceClock.CurrentTime:N2}"); + Logger.Log($"- expected: {proposedTime:N2}"); } state = PlaybackState.NotValid; diff --git a/osu.Game/Tests/Visual/PlayerTestScene.cs b/osu.Game/Tests/Visual/PlayerTestScene.cs index 43d779261c..709ff1d62e 100644 --- a/osu.Game/Tests/Visual/PlayerTestScene.cs +++ b/osu.Game/Tests/Visual/PlayerTestScene.cs @@ -70,14 +70,6 @@ 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; From 69647f4c9cd45fe35a2bd89a33831ec89b997114 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Aug 2025 15:29:51 +0900 Subject: [PATCH 2/4] Don't enforce hotfix during debug builds, to allow interactive tests to run correctly --- 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 3f4700c401..a3e3b33529 100644 --- a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs +++ b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs @@ -154,7 +154,8 @@ namespace osu.Game.Rulesets.UI state = PlaybackState.NotValid; } - bool allowReferenceClockSeeks = hasReplayAttached || DebugUtils.IsNUnitRunning || !FrameStablePlayback; + // TODO: replace IsDebugBuild with a framework flag which asserts we are in a test scene, interactively or otherwise. + bool allowReferenceClockSeeks = hasReplayAttached || DebugUtils.IsNUnitRunning || DebugUtils.IsDebugBuild || !FrameStablePlayback; // This is a hotfix for ongoing bass issues we are trying to resolve (see https://www.un4seen.com/forum/?topic=20482.msg145474#msg145474) // In gameplay we should always be seeking using the From 1f10bff7a747c8c544583fae6559ef32d904d4fb Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Aug 2025 15:40:30 +0900 Subject: [PATCH 3/4] Fix half written comment and adjust threshold down slightly --- osu.Game/Rulesets/UI/FrameStabilityContainer.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs index a3e3b33529..7b9d65454c 100644 --- a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs +++ b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs @@ -158,8 +158,10 @@ namespace osu.Game.Rulesets.UI bool allowReferenceClockSeeks = hasReplayAttached || DebugUtils.IsNUnitRunning || DebugUtils.IsDebugBuild || !FrameStablePlayback; // This is a hotfix for ongoing bass issues we are trying to resolve (see https://www.un4seen.com/forum/?topic=20482.msg145474#msg145474) - // In gameplay we should always be seeking using the - if (!allowReferenceClockSeeks && Math.Abs(proposedTime - referenceClock.CurrentTime) > 1000) + // + // In testing this triggers *very* rarely even when set to super low values (10 ms). The cases we're worried about involve multi-second jumps. + // A difference of more than 500 ms seems like a sane number we should never exceed. + if (!allowReferenceClockSeeks && Math.Abs(proposedTime - referenceClock.CurrentTime) > 500) { if (lastBackwardsSeekLogTime == null || Math.Abs(Clock.CurrentTime - lastBackwardsSeekLogTime.Value) > 1000) { From 126d0c6bdbf624626566848d611050845db7dd47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 15 Aug 2025 10:01:01 +0200 Subject: [PATCH 4/4] Remove failing test Test was added in 5495c2090a76bfd6f6a5efd0fd61530395cc6a68 to exercise a previous version of the hotfix-workaround of not allowing seeking back. Now that the hotfix-workaround also encompasses seeking *forward*, which breaks another 100 tests, thus requiring the hotfix-workaround to be turned off for tests, this situation is just completely out of control. So I'm just removing the test because ugh. --- osu.Game.Tests/Visual/Gameplay/TestScenePause.cs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs index f28baada9e..08317c37cf 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs @@ -81,17 +81,6 @@ namespace osu.Game.Tests.Visual.Gameplay AddAssert("pause recorded", () => Player.Score.ScoreInfo.Pauses, () => Has.Count.EqualTo(1)); } - [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() {