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

Fix looping sounds playing even when paused

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

@@ -0,0 +1,105 @@
+// 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.Allocation;
+using osu.Framework.Graphics;
+using osu.Framework.Graphics.Audio;
+using osu.Framework.Graphics.Containers;
+using osu.Framework.Testing;
+using osu.Framework.Timing;
+using osu.Game.Audio;
+using osu.Game.Screens.Play;
+using osu.Game.Skinning;
+
+namespace osu.Game.Tests.Visual.Gameplay
+{
+    public class TestSceneSkinnableSound : OsuTestScene
+    {
+        [Cached]
+        private GameplayClock gameplayClock = new GameplayClock(new FramedClock());
+
+        private SkinnableSound skinnableSound;
+
+        [SetUp]
+        public void SetUp() => Schedule(() =>
+        {
+            gameplayClock.IsPaused.Value = false;
+
+            Children = new Drawable[]
+            {
+                new Container
+                {
+                    Clock = gameplayClock,
+                    RelativeSizeAxes = Axes.Both,
+                    Child = skinnableSound = new SkinnableSound(new SampleInfo("normal-sliderslide"))
+                },
+            };
+        });
+
+        [Test]
+        public void TestStoppedSoundDoesntResumeAfterPause()
+        {
+            DrawableSample sample = null;
+            AddStep("start sample with looping", () =>
+            {
+                sample = skinnableSound.ChildrenOfType<DrawableSample>().First();
+
+                skinnableSound.Looping = true;
+                skinnableSound.Play();
+            });
+
+            AddUntilStep("wait for sample to start playing", () => sample.Playing);
+
+            AddStep("stop sample", () => skinnableSound.Stop());
+
+            AddUntilStep("wait for sample to stop playing", () => !sample.Playing);
+
+            AddStep("pause gameplay clock", () => gameplayClock.IsPaused.Value = true);
+            AddStep("resume gameplay clock", () => gameplayClock.IsPaused.Value = false);
+
+            AddWaitStep("wait a bit", 5);
+            AddAssert("sample not playing", () => !sample.Playing);
+        }
+
+        [Test]
+        public void TestLoopingSoundResumesAfterPause()
+        {
+            DrawableSample sample = null;
+            AddStep("start sample with looping", () =>
+            {
+                skinnableSound.Looping = true;
+                skinnableSound.Play();
+                sample = skinnableSound.ChildrenOfType<DrawableSample>().First();
+            });
+
+            AddUntilStep("wait for sample to start playing", () => sample.Playing);
+
+            AddStep("pause gameplay clock", () => gameplayClock.IsPaused.Value = true);
+            AddUntilStep("wait for sample to stop playing", () => !sample.Playing);
+        }
+
+        [Test]
+        public void TestNonLoopingStopsWithPause()
+        {
+            DrawableSample sample = null;
+            AddStep("start sample", () =>
+            {
+                skinnableSound.Play();
+                sample = skinnableSound.ChildrenOfType<DrawableSample>().First();
+            });
+
+            AddAssert("sample playing", () => sample.Playing);
+
+            AddStep("pause gameplay clock", () => gameplayClock.IsPaused.Value = true);
+            AddUntilStep("wait for sample to stop playing", () => !sample.Playing);
+
+            AddStep("resume gameplay clock", () => gameplayClock.IsPaused.Value = false);
+
+            AddAssert("sample not playing", () => !sample.Playing);
+            AddAssert("sample not playing", () => !sample.Playing);
+            AddAssert("sample not playing", () => !sample.Playing);
+        }
+    }
+}

osu.Game/Skinning/SkinnableSound.cs

@@ -12,6 +12,7 @@
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Transforms;
 using osu.Game.Audio;
+using osu.Game.Screens.Play;
 
 namespace osu.Game.Skinning
 {
@@ -22,9 +23,13 @@ public class SkinnableSound : SkinReloadableDrawable
         [Resolved]
         private ISampleStore samples { get; set; }
 
+        private bool requestedPlaying;
+
         public override bool RemoveWhenNotAlive => false;
         public override bool RemoveCompletedTransforms => false;
 
+        private readonly AudioContainer<DrawableSample> samplesContainer;
+
         public SkinnableSound(ISampleInfo hitSamples)
             : this(new[] { hitSamples })
         {
@@ -36,9 +41,99 @@ public SkinnableSound(IEnumerable<ISampleInfo> hitSamples)
             InternalChild = samplesContainer = new AudioContainer<DrawableSample>();
         }
 
+        private Bindable<bool> gameplayClockPaused;
+
+        [BackgroundDependencyLoader(true)]
+        private void load(GameplayClock gameplayClock)
+        {
+            // if in a gameplay context, pause sample playback when gameplay is paused.
+            gameplayClockPaused = gameplayClock?.IsPaused.GetBoundCopy();
+            gameplayClockPaused?.BindValueChanged(paused =>
+            {
+                if (requestedPlaying)
+                {
+                    if (paused.NewValue)
+                        stop();
+                    // it's not easy to know if a sample has finished playing (to end).
+                    // to keep things simple only resume playing looping samples.
+                    else if (Looping)
+                        play();
+                }
+            });
+        }
+
         private bool looping;
 
-        private readonly AudioContainer<DrawableSample> samplesContainer;
+        public bool Looping
+        {
+            get => looping;
+            set
+            {
+                if (value == looping) return;
+
+                looping = value;
+
+                samplesContainer.ForEach(c => c.Looping = looping);
+            }
+        }
+
+        public void Play()
+        {
+            requestedPlaying = true;
+            play();
+        }
+
+        private void play()
+        {
+            samplesContainer.ForEach(c =>
+            {
+                if (c.AggregateVolume.Value > 0)
+                    c.Play();
+            });
+        }
+
+        public void Stop()
+        {
+            requestedPlaying = false;
+            stop();
+        }
+
+        private void stop()
+        {
+            samplesContainer.ForEach(c => c.Stop());
+        }
+
+        protected override void SkinChanged(ISkinSource skin, bool allowFallback)
+        {
+            var channels = hitSamples.Select(s =>
+            {
+                var ch = skin.GetSample(s);
+
+                if (ch == null && allowFallback)
+                {
+                    foreach (var lookup in s.LookupNames)
+                    {
+                        if ((ch = samples.Get($"Gameplay/{lookup}")) != null)
+                            break;
+                    }
+                }
+
+                if (ch != null)
+                {
+                    ch.Looping = looping;
+                    ch.Volume.Value = s.Volume / 100.0;
+                }
+
+                return ch;
+            }).Where(c => c != null);
+
+            samplesContainer.ChildrenEnumerable = channels.Select(c => new DrawableSample(c));
+
+            if (requestedPlaying)
+                Play();
+        }
+
+        #region Re-expose AudioContainer
 
         public BindableNumber<double> Volume => samplesContainer.Volume;
 
@@ -48,8 +143,6 @@ public SkinnableSound(IEnumerable<ISampleInfo> hitSamples)
 
         public BindableNumber<double> Tempo => samplesContainer.Tempo;
 
-        public override bool IsPresent => Scheduler.HasPendingTasks || IsPlaying;
-
         public bool IsPlaying => samplesContainer.Any(s => s.Playing);
 
         /// <summary>
@@ -80,57 +173,6 @@ public TransformSequence<DrawableAudioWrapper> FrequencyTo(double newFrequency,
         public TransformSequence<DrawableAudioWrapper> TempoTo(double newTempo, double duration = 0, Easing easing = Easing.None) =>
             samplesContainer.TempoTo(newTempo, duration, easing);
 
-        public bool Looping
-        {
-            get => looping;
-            set
-            {
-                if (value == looping) return;
-
-                looping = value;
-
-                samplesContainer.ForEach(c => c.Looping = looping);
-            }
-        }
-
-        public void Play() => samplesContainer.ForEach(c =>
-        {
-            if (c.AggregateVolume.Value > 0)
-                c.Play();
-        });
-
-        public void Stop() => samplesContainer.ForEach(c => c.Stop());
-
-        protected override void SkinChanged(ISkinSource skin, bool allowFallback)
-        {
-            bool wasPlaying = samplesContainer.Any(s => s.Playing);
-
-            var channels = hitSamples.Select(s =>
-            {
-                var ch = skin.GetSample(s);
-
-                if (ch == null && allowFallback)
-                {
-                    foreach (var lookup in s.LookupNames)
-                    {
-                        if ((ch = samples.Get($"Gameplay/{lookup}")) != null)
-                            break;
-                    }
-                }
-
-                if (ch != null)
-                {
-                    ch.Looping = looping;
-                    ch.Volume.Value = s.Volume / 100.0;
-                }
-
-                return ch;
-            }).Where(c => c != null);
-
-            samplesContainer.ChildrenEnumerable = channels.Select(c => new DrawableSample(c));
-
-            if (wasPlaying)
-                Play();
-        }
+        #endregion
     }
 }