Merge pull request #10371 from peppy/fix-gameplay-seek-sample-pausing

Fix seeking in replays not correctly pausing samples

osu.Game.Rulesets.Taiko/UI/InputDrum.cs

@@ -163,7 +163,6 @@ namespace osu.Game.Rulesets.Taiko.UI
                     target = centreHit;
                     back = centre;
 
-                    if (gameplayClock?.IsSeeking != true)
                     drumSample.Centre?.Play();
                 }
                 else if (action == RimAction)
@@ -171,7 +170,6 @@ namespace osu.Game.Rulesets.Taiko.UI
                     target = rimHit;
                     back = rim;
 
-                    if (gameplayClock?.IsSeeking != true)
                     drumSample.Rim?.Play();
                 }
 

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

@@ -0,0 +1,61 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. 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.Framework.Graphics.Audio;
+using osu.Framework.Testing;
+using osu.Game.Rulesets;
+using osu.Game.Rulesets.Osu;
+using osu.Game.Rulesets.Osu.Objects.Drawables;
+using osu.Game.Rulesets.UI;
+using osu.Game.Screens.Play;
+using osu.Game.Skinning;
+
+namespace osu.Game.Tests.Visual.Gameplay
+{
+    public class TestSceneGameplaySamplePlayback : PlayerTestScene
+    {
+        [Test]
+        public void TestAllSamplesStopDuringSeek()
+        {
+            DrawableSlider slider = null;
+            DrawableSample[] samples = null;
+            ISamplePlaybackDisabler gameplayClock = null;
+
+            AddStep("get variables", () =>
+            {
+                gameplayClock = Player.ChildrenOfType<FrameStabilityContainer>().First().GameplayClock;
+                slider = Player.ChildrenOfType<DrawableSlider>().OrderBy(s => s.HitObject.StartTime).First();
+                samples = slider.ChildrenOfType<DrawableSample>().ToArray();
+            });
+
+            AddUntilStep("wait for slider sliding then seek", () =>
+            {
+                if (!slider.Tracking.Value)
+                    return false;
+
+                if (!samples.Any(s => s.Playing))
+                    return false;
+
+                Player.ChildrenOfType<GameplayClockContainer>().First().Seek(40000);
+                return true;
+            });
+
+            AddAssert("sample playback disabled", () => gameplayClock.SamplePlaybackDisabled.Value);
+
+            // because we are in frame stable context, it's quite likely that not all samples are "played" at this point.
+            // the important thing is that at least one started, and that sample has since stopped.
+            AddAssert("no samples are playing", () => Player.ChildrenOfType<PausableSkinnableSound>().All(s => !s.IsPlaying));
+
+            AddAssert("sample playback still disabled", () => gameplayClock.SamplePlaybackDisabled.Value);
+
+            AddUntilStep("seek finished, sample playback enabled", () => !gameplayClock.SamplePlaybackDisabled.Value);
+            AddUntilStep("any sample is playing", () => Player.ChildrenOfType<PausableSkinnableSound>().Any(s => s.IsPlaying));
+        }
+
+        protected override bool Autoplay => true;
+
+        protected override Ruleset CreatePlayerRuleset() => new OsuRuleset();
+    }
+}

osu.Game/Rulesets/UI/FrameStabilityContainer.cs

@@ -35,6 +35,7 @@ namespace osu.Game.Rulesets.UI
         public GameplayClock GameplayClock => stabilityGameplayClock;
 
         [Cached(typeof(GameplayClock))]
+        [Cached(typeof(ISamplePlaybackDisabler))]
         private readonly StabilityGameplayClock stabilityGameplayClock;
 
         public FrameStabilityContainer(double gameplayStartTime = double.MinValue)
@@ -58,13 +59,16 @@ namespace osu.Game.Rulesets.UI
         private int direction;
 
         [BackgroundDependencyLoader(true)]
-        private void load(GameplayClock clock)
+        private void load(GameplayClock clock, ISamplePlaybackDisabler sampleDisabler)
         {
             if (clock != null)
             {
                 parentGameplayClock = stabilityGameplayClock.ParentGameplayClock = clock;
                 GameplayClock.IsPaused.BindTo(clock.IsPaused);
             }
+
+            // this is a bit temporary. should really be done inside of GameplayClock (but requires large structural changes).
+            stabilityGameplayClock.ParentSampleDisabler = sampleDisabler;
         }
 
         protected override void LoadComplete()
@@ -206,12 +210,16 @@ namespace osu.Game.Rulesets.UI
         }
 
         private void setClock()
+        {
+            if (parentGameplayClock == null)
             {
                 // in case a parent gameplay clock isn't available, just use the parent clock.
                 parentGameplayClock ??= Clock;
-
+            }
+            else
+            {
                 Clock = GameplayClock;
-            ProcessCustomClock = false;
+            }
         }
 
         public ReplayInputHandler ReplayInputHandler { get; set; }
@@ -220,6 +228,8 @@ namespace osu.Game.Rulesets.UI
         {
             public GameplayClock ParentGameplayClock;
 
+            public ISamplePlaybackDisabler ParentSampleDisabler;
+
             public override IEnumerable<Bindable<double>> NonGameplayAdjustments => ParentGameplayClock?.NonGameplayAdjustments ?? Enumerable.Empty<Bindable<double>>();
 
             public StabilityGameplayClock(FramedClock underlyingClock)
@@ -227,7 +237,11 @@ namespace osu.Game.Rulesets.UI
             {
             }
 
-            public override bool IsSeeking => ParentGameplayClock != null && Math.Abs(CurrentTime - ParentGameplayClock.CurrentTime) > 200;
+            protected override bool ShouldDisableSamplePlayback =>
+                // handle the case where playback is catching up to real-time.
+                base.ShouldDisableSamplePlayback
+                || ParentSampleDisabler?.SamplePlaybackDisabled.Value == true
+                || (ParentGameplayClock != null && Math.Abs(CurrentTime - ParentGameplayClock.CurrentTime) > 200);
         }
     }
 }

osu.Game/Screens/Play/GameplayClock.cs

@@ -28,6 +28,8 @@ namespace osu.Game.Screens.Play
         /// </summary>
         public virtual IEnumerable<Bindable<double>> NonGameplayAdjustments => Enumerable.Empty<Bindable<double>>();
 
+        private readonly Bindable<bool> samplePlaybackDisabled = new Bindable<bool>();
+
         public GameplayClock(IFrameBasedClock underlyingClock)
         {
             this.underlyingClock = underlyingClock;
@@ -62,13 +64,15 @@ namespace osu.Game.Screens.Play
         public bool IsRunning => underlyingClock.IsRunning;
 
         /// <summary>
-        /// Whether an ongoing seek operation is active.
+        /// Whether nested samples supporting the <see cref="ISamplePlaybackDisabler"/> interface should be paused.
         /// </summary>
-        public virtual bool IsSeeking => false;
+        protected virtual bool ShouldDisableSamplePlayback => IsPaused.Value;
 
         public void ProcessFrame()
         {
-            // we do not want to process the underlying clock.
+            // intentionally not updating the underlying clock (handled externally).
+
+            samplePlaybackDisabled.Value = ShouldDisableSamplePlayback;
         }
 
         public double ElapsedFrameTime => underlyingClock.ElapsedFrameTime;
@@ -79,6 +83,6 @@ namespace osu.Game.Screens.Play
 
         public IClock Source => underlyingClock;
 
-        public IBindable<bool> SamplePlaybackDisabled => IsPaused;
+        IBindable<bool> ISamplePlaybackDisabler.SamplePlaybackDisabled => samplePlaybackDisabled;
     }
 }