Merge pull request #11561 from Mysfit/fix-storyboard-samples

Fix storyboard samples continuing to play when the beatmap is paused or intro is skipped

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

@@ -0,0 +1,74 @@
+// 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.Collections.Generic;
+using System.Linq;
+using NUnit.Framework;
+using osu.Framework.Allocation;
+using osu.Framework.Testing;
+using osu.Game.Beatmaps;
+using osu.Game.Configuration;
+using osu.Game.Rulesets;
+using osu.Game.Rulesets.Osu;
+using osu.Game.Storyboards;
+using osu.Game.Storyboards.Drawables;
+
+namespace osu.Game.Tests.Visual.Gameplay
+{
+    public class TestSceneStoryboardSamplePlayback : PlayerTestScene
+    {
+        private Storyboard storyboard;
+
+        [BackgroundDependencyLoader]
+        private void load(OsuConfigManager config)
+        {
+            config.Set(OsuSetting.ShowStoryboard, true);
+
+            storyboard = new Storyboard();
+            var backgroundLayer = storyboard.GetLayer("Background");
+            backgroundLayer.Add(new StoryboardSampleInfo("Intro/welcome.mp3", time: -7000, volume: 20));
+            backgroundLayer.Add(new StoryboardSampleInfo("Intro/welcome.mp3", time: -5000, volume: 20));
+            backgroundLayer.Add(new StoryboardSampleInfo("Intro/welcome.mp3", time: 0, volume: 20));
+        }
+
+        [Test]
+        public void TestStoryboardSamplesStopDuringPause()
+        {
+            checkForFirstSamplePlayback();
+
+            AddStep("player paused", () => Player.Pause());
+            AddAssert("player is currently paused", () => Player.GameplayClockContainer.IsPaused.Value);
+            AddAssert("all storyboard samples stopped immediately", () => allStoryboardSamples.All(sound => !sound.IsPlaying));
+
+            AddStep("player resume", () => Player.Resume());
+            AddUntilStep("any storyboard samples playing after resume", () => allStoryboardSamples.Any(sound => sound.IsPlaying));
+        }
+
+        [Test]
+        public void TestStoryboardSamplesStopOnSkip()
+        {
+            checkForFirstSamplePlayback();
+
+            AddStep("skip intro", () => InputManager.Key(osuTK.Input.Key.Space));
+            AddAssert("all storyboard samples stopped immediately", () => allStoryboardSamples.All(sound => !sound.IsPlaying));
+
+            AddUntilStep("any storyboard samples playing after skip", () => allStoryboardSamples.Any(sound => sound.IsPlaying));
+        }
+
+        private void checkForFirstSamplePlayback()
+        {
+            AddUntilStep("storyboard loaded", () => Player.Beatmap.Value.StoryboardLoaded);
+            AddUntilStep("any storyboard samples playing", () => allStoryboardSamples.Any(sound => sound.IsPlaying));
+        }
+
+        private IEnumerable<DrawableStoryboardSample> allStoryboardSamples => Player.ChildrenOfType<DrawableStoryboardSample>();
+
+        protected override bool AllowFail => false;
+
+        protected override Ruleset CreatePlayerRuleset() => new OsuRuleset();
+        protected override TestPlayer CreatePlayer(Ruleset ruleset) => new TestPlayer(true, false);
+
+        protected override WorkingBeatmap CreateWorkingBeatmap(IBeatmap beatmap, Storyboard storyboard = null) =>
+            new ClockBackedTestWorkingBeatmap(beatmap, storyboard ?? this.storyboard, Clock, Audio);
+    }
+}

osu.Game/Screens/Play/Player.cs

@@ -353,7 +353,7 @@ namespace osu.Game.Screens.Play
                     },
                     skipOverlay = new SkipOverlay(DrawableRuleset.GameplayStartTime)
                     {
-                        RequestSkip = GameplayClockContainer.Skip
+                        RequestSkip = performUserRequestedSkip
                     },
                     FailOverlay = new FailOverlay
                     {
@@ -488,6 +488,17 @@ namespace osu.Game.Screens.Play
                 this.Exit();
         }
 
+        private void performUserRequestedSkip()
+        {
+            // user requested skip
+            // disable sample playback to stop currently playing samples and perform skip
+            samplePlaybackDisabled.Value = true;
+            GameplayClockContainer.Skip();
+
+            // return samplePlaybackDisabled.Value to what is defined by the beatmap's current state
+            updateSampleDisabledState();
+        }
+
         private void performUserRequestedExit()
         {
             if (ValidForResume && HasFailed && !FailOverlay.IsPresent)

osu.Game/Skinning/PausableSkinnableSound.cs

@@ -43,26 +43,28 @@ namespace osu.Game.Skinning
             if (samplePlaybackDisabler != null)
             {
                 samplePlaybackDisabled.BindTo(samplePlaybackDisabler.SamplePlaybackDisabled);
-                samplePlaybackDisabled.BindValueChanged(disabled =>
+                samplePlaybackDisabled.BindValueChanged(SamplePlaybackDisabledChanged);
+            }
+        }
+
+        protected virtual void SamplePlaybackDisabledChanged(ValueChangedEvent<bool> disabled)
+        {
+            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;
+
+            cancelPendingStart();
+
+            if (disabled.NewValue)
+                base.Stop();
+            else
+            {
+                // schedule so we don't start playing a sample which is no longer alive.
+                scheduledStart = Schedule(() =>
                 {
-                    if (!RequestedPlaying) return;
+                    if (RequestedPlaying)
-
+                        base.Play();
-                    // let non-looping samples that have already been started play out to completion (sounds better than abruptly cutting off).
-                    if (!Looping) return;
-
-                    cancelPendingStart();
-
-                    if (disabled.NewValue)
-                        base.Stop();
-                    else
-                    {
-                        // schedule so we don't start playing a sample which is no longer alive.
-                        scheduledStart = Schedule(() =>
-                        {
-                            if (RequestedPlaying)
-                                base.Play();
-                        });
-                    }
                 });
             }
         }

osu.Game/Storyboards/Drawables/DrawableStoryboardSample.cs

@@ -42,6 +42,21 @@ namespace osu.Game.Storyboards.Drawables
             }
         }
 
+        protected override void SamplePlaybackDisabledChanged(ValueChangedEvent<bool> disabled)
+        {
+            if (!RequestedPlaying) return;
+
+            if (!Looping && disabled.NewValue)
+            {
+                // the default behaviour for sample disabling is to allow one-shot samples to play out.
+                // storyboards regularly have long running samples that can cause this behaviour to lead to unintended results.
+                // for this reason, we immediately stop such samples.
+                Stop();
+            }
+
+            base.SamplePlaybackDisabledChanged(disabled);
+        }
+
         protected override void Update()
         {
             base.Update();