Merge pull request #24885 from peppy/clock-fix-attempt-2

Adjust clock usage in line with framework changes

osu.Android.props

@@ -10,7 +10,7 @@
     <EmbedAssembliesIntoApk>true</EmbedAssembliesIntoApk>
   </PropertyGroup>
   <ItemGroup>
-    <PackageReference Include="ppy.osu.Framework.Android" Version="2023.922.0" />
+    <PackageReference Include="ppy.osu.Framework.Android" Version="2023.1006.0" />
   </ItemGroup>
   <PropertyGroup>
     <!-- Fody does not handle Android build well, and warns when unchanged.

osu.Game.Rulesets.Taiko.Tests/TestSceneDrumSampleTriggerSource.cs

@@ -38,7 +38,7 @@ public partial class TestSceneDrumSampleTriggerSource : OsuTestScene
         [SetUp]
         public void SetUp() => Schedule(() =>
         {
-            gameplayClock = new GameplayClockContainer(manualClock)
+            gameplayClock = new GameplayClockContainer(manualClock, false, false)
             {
                 RelativeSizeAxes = Axes.Both,
                 Children = new Drawable[]

osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs

@@ -126,7 +126,7 @@ public void TestSampleHasLifetimeEndWithInitialClockTime()
         public void TestSamplePlaybackWithBeatmapHitsoundsOff()
         {
             GameplayClockContainer gameplayContainer = null;
-            TestDrawableStoryboardSample sample = null;
+            DrawableStoryboardSample sample = null;
 
             AddStep("disable beatmap hitsounds", () => config.SetValue(OsuSetting.BeatmapHitsounds, false));
 
@@ -141,7 +141,7 @@ public void TestSamplePlaybackWithBeatmapHitsoundsOff()
                     Child = beatmapSkinSourceContainer
                 });
 
-                beatmapSkinSourceContainer.Add(sample = new TestDrawableStoryboardSample(new StoryboardSampleInfo("test-sample", 1, 1))
+                beatmapSkinSourceContainer.Add(sample = new DrawableStoryboardSample(new StoryboardSampleInfo("test-sample", 1, 1))
                 {
                     Clock = gameplayContainer
                 });
@@ -199,14 +199,6 @@ public TestCustomSkinWorkingBeatmap(RulesetInfo ruleset, IStorageResourceProvide
             protected internal override ISkin GetSkin() => new TestSkin("test-sample", resources);
         }
 
-        private partial class TestDrawableStoryboardSample : DrawableStoryboardSample
-        {
-            public TestDrawableStoryboardSample(StoryboardSampleInfo sampleInfo)
-                : base(sampleInfo)
-            {
-            }
-        }
-
         #region IResourceStorageProvider
 
         public IRenderer Renderer => host.Renderer;

osu.Game.Tests/NonVisual/GameplayClockContainerTest.cs

@@ -3,6 +3,7 @@
 
 using NUnit.Framework;
 using osu.Framework.Audio;
+using osu.Framework.Audio.Track;
 using osu.Framework.Bindables;
 using osu.Framework.Timing;
 using osu.Game.Screens.Play;
@@ -16,16 +17,16 @@ public partial class GameplayClockContainerTest
         [TestCase(1)]
         public void TestTrueGameplayRateWithGameplayAdjustment(double underlyingClockRate)
         {
-            var framedClock = new FramedClock(new ManualClock { Rate = underlyingClockRate });
-            var gameplayClock = new TestGameplayClockContainer(framedClock);
+            var trackVirtual = new TrackVirtual(60000) { Frequency = { Value = underlyingClockRate } };
+            var gameplayClock = new TestGameplayClockContainer(trackVirtual);
 
             Assert.That(gameplayClock.GetTrueGameplayRate(), Is.EqualTo(2));
         }
 
         private partial class TestGameplayClockContainer : GameplayClockContainer
         {
-            public TestGameplayClockContainer(IFrameBasedClock underlyingClock)
-                : base(underlyingClock)
+            public TestGameplayClockContainer(IClock underlyingClock)
+                : base(underlyingClock, false, false)
             {
                 AdjustmentsFromMods.AddAdjustment(AdjustableProperty.Frequency, new BindableDouble(2.0));
             }

osu.Game.Tests/OnlinePlay/TestSceneCatchUpSyncManager.cs

@@ -15,6 +15,9 @@
 
 namespace osu.Game.Tests.OnlinePlay
 {
+    // NOTE: This test scene never calls ProcessFrame on clocks.
+    // The current tests are fine without this as they are testing very static scenarios, but it's worth knowing
+    // if adding further tests to this class.
     [HeadlessTest]
     public partial class TestSceneCatchUpSyncManager : OsuTestScene
     {
@@ -28,7 +31,7 @@ public partial class TestSceneCatchUpSyncManager : OsuTestScene
         [SetUp]
         public void Setup()
         {
-            syncManager = new SpectatorSyncManager(master = new GameplayClockContainer(new TestManualClock()));
+            syncManager = new SpectatorSyncManager(master = new GameplayClockContainer(new TestManualClock(), false, false));
             player1 = syncManager.CreateManagedClock();
             player2 = syncManager.CreateManagedClock();
 
@@ -188,6 +191,8 @@ public bool Seek(double position)
 
             public void Reset()
             {
+                IsRunning = false;
+                CurrentTime = 0;
             }
 
             public void ResetSpeedAdjustments()

osu.Game.Tests/Visual/Editing/TestSceneEditorClock.cs

@@ -63,7 +63,7 @@ public void TestStopAtTrackEnd()
             AddStep("seek near end", () => EditorClock.Seek(EditorClock.TrackLength - 250));
             AddUntilStep("clock stops", () => !EditorClock.IsRunning);
 
-            AddUntilStep("clock stopped at end", () => EditorClock.CurrentTime - EditorClock.TotalAppliedOffset, () => Is.EqualTo(EditorClock.TrackLength));
+            AddUntilStep("clock stopped at end", () => EditorClock.CurrentTime, () => Is.EqualTo(EditorClock.TrackLength));
 
             AddStep("start clock again", () => EditorClock.Start());
             AddAssert("clock looped to start", () => EditorClock.IsRunning && EditorClock.CurrentTime < 500);

osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs

@@ -6,11 +6,11 @@
 using System.Linq;
 using NUnit.Framework;
 using osu.Framework.Allocation;
+using osu.Framework.Audio.Track;
 using osu.Framework.Extensions.ObjectExtensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Testing;
-using osu.Framework.Timing;
 using osu.Game.Configuration;
 using osu.Game.Graphics.Containers;
 using osu.Game.Rulesets.Mods;
@@ -41,7 +41,7 @@ public partial class TestSceneHUDOverlay : OsuManualInputManagerTestScene
         private GameplayState gameplayState = TestGameplayState.Create(new OsuRuleset());
 
         [Cached(typeof(IGameplayClock))]
-        private readonly IGameplayClock gameplayClock = new GameplayClockContainer(new FramedClock());
+        private readonly IGameplayClock gameplayClock = new GameplayClockContainer(new TrackVirtual(60000), false, false);
 
         // best way to check without exposing.
         private Drawable hideTarget => hudOverlay.ChildrenOfType<SkinComponentsContainer>().First();

osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditorMultipleSkins.cs

@@ -4,10 +4,10 @@
 #nullable disable
 
 using osu.Framework.Allocation;
+using osu.Framework.Audio.Track;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Testing;
-using osu.Framework.Timing;
 using osu.Game.Overlays.SkinEditor;
 using osu.Game.Rulesets;
 using osu.Game.Rulesets.Osu;
@@ -32,7 +32,7 @@ public partial class TestSceneSkinEditorMultipleSkins : SkinnableTestScene
         private GameplayState gameplayState = TestGameplayState.Create(new OsuRuleset());
 
         [Cached(typeof(IGameplayClock))]
-        private readonly IGameplayClock gameplayClock = new GameplayClockContainer(new FramedClock());
+        private readonly IGameplayClock gameplayClock = new GameplayClockContainer(new TrackVirtual(60000), false, false);
 
         [Cached]
         public readonly EditorClipboard Clipboard = new EditorClipboard();

osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableHUDOverlay.cs

@@ -8,11 +8,11 @@
 using System.Linq;
 using NUnit.Framework;
 using osu.Framework.Allocation;
+using osu.Framework.Audio.Track;
 using osu.Framework.Extensions.IEnumerableExtensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Testing;
-using osu.Framework.Timing;
 using osu.Game.Rulesets;
 using osu.Game.Rulesets.Mods;
 using osu.Game.Rulesets.Osu;
@@ -39,7 +39,7 @@ public partial class TestSceneSkinnableHUDOverlay : SkinnableTestScene
         private GameplayState gameplayState = TestGameplayState.Create(new OsuRuleset());
 
         [Cached(typeof(IGameplayClock))]
-        private readonly IGameplayClock gameplayClock = new GameplayClockContainer(new FramedClock());
+        private readonly IGameplayClock gameplayClock = new GameplayClockContainer(new TrackVirtual(60000), false, false);
 
         private IEnumerable<HUDOverlay> hudOverlays => CreatedDrawables.OfType<HUDOverlay>();
 

osu.Game.Tests/Visual/Gameplay/TestSceneStoryboard.cs

@@ -106,14 +106,12 @@ private void loadStoryboard(Storyboard toLoad)
             if (storyboard != null)
                 storyboardContainer.Remove(storyboard, true);
 
-            var decoupledClock = new DecoupleableInterpolatingFramedClock { IsCoupled = true };
-            storyboardContainer.Clock = decoupledClock;
+            storyboardContainer.Clock = new FramedClock(Beatmap.Value.Track);
 
             storyboard = toLoad.CreateDrawable(SelectedMods.Value);
             storyboard.Passing = false;
 
             storyboardContainer.Add(storyboard);
-            decoupledClock.ChangeSource(Beatmap.Value.Track);
         }
 
         private void loadStoryboard(string filename, Action<Storyboard>? setUpStoryboard = null)

osu.Game/Beatmaps/FramedBeatmapClock.cs

@@ -5,7 +5,6 @@
 using System.Diagnostics;
 using osu.Framework;
 using osu.Framework.Allocation;
-using osu.Framework.Audio.Track;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Framework.Timing;
@@ -28,16 +27,6 @@ public partial class FramedBeatmapClock : Component, IFrameBasedClock, IAdjustab
     {
         private readonly bool applyOffsets;
 
-        /// <summary>
-        /// The length of the underlying beatmap track. Will default to 60 seconds if unavailable.
-        /// </summary>
-        public double TrackLength => Track.Length;
-
-        /// <summary>
-        /// The underlying beatmap track, if available.
-        /// </summary>
-        public Track Track { get; private set; } = new TrackVirtual(60000);
-
         /// <summary>
         /// The total frequency adjustment from pause transforms. Should eventually be handled in a better way.
         /// </summary>
@@ -53,7 +42,7 @@ public partial class FramedBeatmapClock : Component, IFrameBasedClock, IAdjustab
 
         private IDisposable? beatmapOffsetSubscription;
 
-        private readonly DecoupleableInterpolatingFramedClock decoupledClock;
+        private readonly DecouplingFramedClock decoupledTrack;
 
         [Resolved]
         private OsuConfigManager config { get; set; } = null!;
@@ -66,25 +55,21 @@ public partial class FramedBeatmapClock : Component, IFrameBasedClock, IAdjustab
 
         public bool IsRewinding { get; private set; }
 
-        public bool IsCoupled
-        {
-            get => decoupledClock.IsCoupled;
-            set => decoupledClock.IsCoupled = value;
-        }
-
-        public FramedBeatmapClock(bool applyOffsets = false)
+        public FramedBeatmapClock(bool applyOffsets, bool requireDecoupling, IClock? source = null)
         {
             this.applyOffsets = applyOffsets;
 
-            // A decoupled clock is used to ensure precise time values even when the host audio subsystem is not reporting
+            decoupledTrack = new DecouplingFramedClock(source) { AllowDecoupling = requireDecoupling };
+
+            // 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).
-            decoupledClock = new DecoupleableInterpolatingFramedClock { IsCoupled = true };
+            var interpolatedTrack = new InterpolatingFramedClock(decoupledTrack);
 
             if (applyOffsets)
             {
                 // Audio timings in general with newer BASS versions don't match stable.
                 // This only seems to be required on windows. We need to eventually figure out why, with a bit of luck.
-                platformOffsetClock = new OffsetCorrectionClock(decoupledClock, ExternalPauseFrequencyAdjust) { Offset = RuntimeInfo.OS == RuntimeInfo.Platform.Windows ? 15 : 0 };
+                platformOffsetClock = new OffsetCorrectionClock(interpolatedTrack, ExternalPauseFrequencyAdjust) { Offset = RuntimeInfo.OS == RuntimeInfo.Platform.Windows ? 15 : 0 };
 
                 // User global offset (set in settings) should also be applied.
                 userGlobalOffsetClock = new OffsetCorrectionClock(platformOffsetClock, ExternalPauseFrequencyAdjust);
@@ -94,7 +79,7 @@ public FramedBeatmapClock(bool applyOffsets = false)
             }
             else
             {
-                finalClockSource = decoupledClock;
+                finalClockSource = interpolatedTrack;
             }
         }
 
@@ -110,6 +95,7 @@ protected override void LoadComplete()
                 userAudioOffset = config.GetBindable<double>(OsuSetting.AudioOffset);
                 userAudioOffset.BindValueChanged(offset => userGlobalOffsetClock.Offset = offset.NewValue, true);
 
+                // TODO: this doesn't update when using ChangeSource() to change beatmap.
                 beatmapOffsetSubscription = realm.SubscribeToPropertyChanged(
                     r => r.Find<BeatmapInfo>(beatmap.Value.BeatmapInfo.ID)?.UserSettings,
                     settings => settings.Offset,
@@ -124,17 +110,7 @@ protected override void Update()
         {
             base.Update();
 
-            if (Source != null && Source is not IAdjustableClock && Source.CurrentTime < decoupledClock.CurrentTime - 100)
-            {
-                // InterpolatingFramedClock won't interpolate backwards unless its source has an ElapsedFrameTime.
-                // See https://github.com/ppy/osu-framework/blob/ba1385330cc501f34937e08257e586c84e35d772/osu.Framework/Timing/InterpolatingFramedClock.cs#L91-L93
-                // This is not always the case here when doing large seeks.
-                // (Of note, this is not an issue if the source is adjustable, as the source is seeked to be in time by DecoupleableInterpolatingFramedClock).
-                // Rather than trying to get around this by fixing the framework clock stack, let's work around it for now.
-                Seek(Source.CurrentTime);
-            }
-            else
-                finalClockSource.ProcessFrame();
+            finalClockSource.ProcessFrame();
 
             if (Clock.ElapsedFrameTime != 0)
                 IsRewinding = Clock.ElapsedFrameTime < 0;
@@ -157,46 +133,42 @@ public double TotalAppliedOffset
 
         #region Delegation of IAdjustableClock / ISourceChangeableClock to decoupled clock.
 
-        public void ChangeSource(IClock? source)
-        {
-            Track = source as Track ?? new TrackVirtual(60000);
-            decoupledClock.ChangeSource(source);
-        }
+        public void ChangeSource(IClock? source) => decoupledTrack.ChangeSource(source);
 
-        public IClock? Source => decoupledClock.Source;
+        public IClock Source => decoupledTrack.Source;
 
         public void Reset()
         {
-            decoupledClock.Reset();
+            decoupledTrack.Reset();
             finalClockSource.ProcessFrame();
         }
 
         public void Start()
         {
-            decoupledClock.Start();
+            decoupledTrack.Start();
             finalClockSource.ProcessFrame();
         }
 
         public void Stop()
         {
-            decoupledClock.Stop();
+            decoupledTrack.Stop();
             finalClockSource.ProcessFrame();
         }
 
         public bool Seek(double position)
         {
-            bool success = decoupledClock.Seek(position - TotalAppliedOffset);
+            bool success = decoupledTrack.Seek(position - TotalAppliedOffset);
             finalClockSource.ProcessFrame();
 
             return success;
         }
 
-        public void ResetSpeedAdjustments() => decoupledClock.ResetSpeedAdjustments();
+        public void ResetSpeedAdjustments() => decoupledTrack.ResetSpeedAdjustments();
 
         public double Rate
         {
-            get => decoupledClock.Rate;
-            set => decoupledClock.Rate = value;
+            get => decoupledTrack.Rate;
+            set => decoupledTrack.Rate = value;
         }
 
         #endregion

osu.Game/OsuGameBase.cs

@@ -215,7 +215,7 @@ public virtual string Version
         /// For now, this is used as a source specifically for beat synced components.
         /// Going forward, it could potentially be used as the single source-of-truth for beatmap timing.
         /// </summary>
-        private readonly FramedBeatmapClock beatmapClock = new FramedBeatmapClock(true);
+        private readonly FramedBeatmapClock beatmapClock = new FramedBeatmapClock(applyOffsets: true, requireDecoupling: false);
 
         protected override Container<Drawable> Content => content;
 
@@ -441,16 +441,7 @@ private void addFilesWarning()
             }
         }
 
-        private void onTrackChanged(WorkingBeatmap beatmap, TrackChangeDirection direction)
-        {
-            // FramedBeatmapClock uses a decoupled clock internally which will mutate the source if it is an `IAdjustableClock`.
-            // We don't want this for now, as the intention of beatmapClock is to be a read-only source for beat sync components.
-            //
-            // Encapsulating in a FramedClock will avoid any mutations.
-            var framedClock = new FramedClock(beatmap.Track);
-
-            beatmapClock.ChangeSource(framedClock);
-        }
+        private void onTrackChanged(WorkingBeatmap beatmap, TrackChangeDirection direction) => beatmapClock.ChangeSource(beatmap.Track);
 
         protected virtual void InitialiseFonts()
         {

osu.Game/Screens/Edit/Editor.cs

@@ -862,7 +862,7 @@ protected void SetPreviewPointToCurrentTime()
 
         private void resetTrack(bool seekToStart = false)
         {
-            Beatmap.Value.Track.Stop();
+            clock.Stop();
 
             if (seekToStart)
             {