From 758088672cf9b7e58c8c83a5c4aebae143063d56 Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Mon, 5 Oct 2020 15:07:46 +0900
Subject: [PATCH] Don't stop non-looping samples immediately when pausing

---
 .../Objects/Drawables/DrawableSlider.cs       |  4 +--
 .../Objects/Drawables/DrawableSpinner.cs      |  4 +--
 .../Objects/Drawables/DrawableHitObject.cs    | 10 +++++--
 osu.Game/Skinning/PausableSkinnableSound.cs   | 26 +++++++++----------
 4 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs
index 280ca33234..4433aac9b5 100644
--- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs
+++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs
@@ -109,9 +109,9 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
             }
         }
 
-        public override void StopAllSamples()
+        public override void StopLoopingSamples()
         {
-            base.StopAllSamples();
+            base.StopLoopingSamples();
             slidingSample?.Stop();
         }
 
diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs
index 130b4e6e53..dda0c94982 100644
--- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs
+++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs
@@ -124,9 +124,9 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
             }
         }
 
-        public override void StopAllSamples()
+        public override void StopLoopingSamples()
         {
-            base.StopAllSamples();
+            base.StopLoopingSamples();
             spinningSample?.Stop();
         }
 
diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs
index 66fc61720a..7a4970d172 100644
--- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs
+++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs
@@ -18,6 +18,7 @@ using osu.Game.Rulesets.Objects.Types;
 using osu.Game.Rulesets.Scoring;
 using osu.Game.Skinning;
 using osu.Game.Configuration;
+using osu.Game.Screens.Play;
 using osuTK.Graphics;
 
 namespace osu.Game.Rulesets.Objects.Drawables
@@ -387,7 +388,10 @@ namespace osu.Game.Rulesets.Objects.Drawables
         /// <summary>
         /// Stops playback of all samples. Automatically called when <see cref="DrawableHitObject{TObject}"/>'s lifetime has been exceeded.
         /// </summary>
-        public virtual void StopAllSamples() => Samples?.Stop();
+        public virtual void StopLoopingSamples()
+        {
+            if (Samples?.Looping == true)
+                Samples.Stop();
 
         protected override void Update()
         {
@@ -457,7 +461,9 @@ namespace osu.Game.Rulesets.Objects.Drawables
             foreach (var nested in NestedHitObjects)
                 nested.OnKilled();
 
-            StopAllSamples();
+            // failsafe to ensure looping samples don't get stuck in a playing state.
+            // this could occur in a non-frame-stable context where DrawableHitObjects get killed before a SkinnableSound has the chance to be stopped.
+            StopLoopingSamples();
 
             UpdateResult(false);
         }
diff --git a/osu.Game/Skinning/PausableSkinnableSound.cs b/osu.Game/Skinning/PausableSkinnableSound.cs
index 9819574b1d..d340f67575 100644
--- a/osu.Game/Skinning/PausableSkinnableSound.cs
+++ b/osu.Game/Skinning/PausableSkinnableSound.cs
@@ -34,21 +34,21 @@ namespace osu.Game.Skinning
                 samplePlaybackDisabled.BindTo(samplePlaybackDisabler.SamplePlaybackDisabled);
                 samplePlaybackDisabled.BindValueChanged(disabled =>
                 {
-                    if (RequestedPlaying)
+                    if (!RequestedPlaying) return;
+
+                    // let non-looping samples that have already been started play out to completion (sounds better than abruptly cutting off).
+                    if (!Looping) return;
+
+                    if (disabled.NewValue)
+                        base.Stop();
+                    else
                     {
-                        if (disabled.NewValue)
-                            base.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)
+                        // schedule so we don't start playing a sample which is no longer alive.
+                        Schedule(() =>
                         {
-                            // schedule so we don't start playing a sample which is no longer alive.
-                            Schedule(() =>
-                            {
-                                if (RequestedPlaying)
-                                    base.Play();
-                            });
-                        }
+                            if (RequestedPlaying)
+                                base.Play();
+                        });
                     }
                 });
             }