Merge pull request #10063 from frenzibyte/fix-music-controller-regressed

Move OsuGame-dependent music functionalities outside of MusicController

osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs

@@ -0,0 +1,83 @@
+// 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.Testing;
+using osu.Game.Beatmaps;
+using osu.Game.Input.Bindings;
+using osu.Game.Overlays;
+using osu.Game.Tests.Resources;
+using osu.Game.Tests.Visual.Navigation;
+
+namespace osu.Game.Tests.Visual.Menus
+{
+    public class TestSceneMusicActionHandling : OsuGameTestScene
+    {
+        private GlobalActionContainer globalActionContainer => Game.ChildrenOfType<GlobalActionContainer>().First();
+
+        [Test]
+        public void TestMusicPlayAction()
+        {
+            AddStep("ensure playing something", () => Game.MusicController.EnsurePlayingSomething());
+            AddStep("toggle playback", () => globalActionContainer.TriggerPressed(GlobalAction.MusicPlay));
+            AddAssert("music paused", () => !Game.MusicController.IsPlaying && Game.MusicController.IsUserPaused);
+            AddStep("toggle playback", () => globalActionContainer.TriggerPressed(GlobalAction.MusicPlay));
+            AddAssert("music resumed", () => Game.MusicController.IsPlaying && !Game.MusicController.IsUserPaused);
+        }
+
+        [Test]
+        public void TestMusicNavigationActions()
+        {
+            int importId = 0;
+            Queue<(WorkingBeatmap working, TrackChangeDirection changeDirection)> trackChangeQueue = null;
+
+            // ensure we have at least two beatmaps available to identify the direction the music controller navigated to.
+            AddRepeatStep("import beatmap", () => Game.BeatmapManager.Import(new BeatmapSetInfo
+            {
+                Beatmaps = new List<BeatmapInfo>
+                {
+                    new BeatmapInfo
+                    {
+                        BaseDifficulty = new BeatmapDifficulty(),
+                    }
+                },
+                Metadata = new BeatmapMetadata
+                {
+                    Artist = $"a test map {importId++}",
+                    Title = "title",
+                }
+            }).Wait(), 5);
+
+            AddStep("import beatmap with track", () =>
+            {
+                var setWithTrack = Game.BeatmapManager.Import(TestResources.GetTestBeatmapForImport()).Result;
+                Beatmap.Value = Game.BeatmapManager.GetWorkingBeatmap(setWithTrack.Beatmaps.First());
+            });
+
+            AddStep("bind to track change", () =>
+            {
+                trackChangeQueue = new Queue<(WorkingBeatmap, TrackChangeDirection)>();
+                Game.MusicController.TrackChanged += (working, changeDirection) => trackChangeQueue.Enqueue((working, changeDirection));
+            });
+
+            AddStep("seek track to 6 second", () => Game.MusicController.SeekTo(6000));
+            AddUntilStep("wait for current time to update", () => Game.MusicController.CurrentTrack.CurrentTime > 5000);
+
+            AddStep("press previous", () => globalActionContainer.TriggerPressed(GlobalAction.MusicPrev));
+            AddAssert("no track change", () => trackChangeQueue.Count == 0);
+            AddUntilStep("track restarted", () => Game.MusicController.CurrentTrack.CurrentTime < 5000);
+
+            AddStep("press previous", () => globalActionContainer.TriggerPressed(GlobalAction.MusicPrev));
+            AddAssert("track changed to previous", () =>
+                trackChangeQueue.Count == 1 &&
+                trackChangeQueue.Dequeue().changeDirection == TrackChangeDirection.Prev);
+
+            AddStep("press next", () => globalActionContainer.TriggerPressed(GlobalAction.MusicNext));
+            AddAssert("track changed to next", () =>
+                trackChangeQueue.Count == 1 &&
+                trackChangeQueue.Dequeue().changeDirection == TrackChangeDirection.Next);
+        }
+    }
+}

osu.Game.Tests/Visual/UserInterface/TestSceneNowPlayingOverlay.cs

@@ -1,18 +1,11 @@
 // 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.Audio;
 using osu.Framework.Graphics;
-using osu.Framework.Platform;
-using osu.Game.Beatmaps;
 using osu.Game.Overlays;
-using osu.Game.Rulesets;
 using osu.Game.Rulesets.Osu;
-using osu.Game.Tests.Resources;
 
 namespace osu.Game.Tests.Visual.UserInterface
 {
@@ -24,14 +17,9 @@ namespace osu.Game.Tests.Visual.UserInterface
 
         private NowPlayingOverlay nowPlayingOverlay;
 
-        private RulesetStore rulesets;
-
         [BackgroundDependencyLoader]
-        private void load(AudioManager audio, GameHost host)
+        private void load()
         {
-            Dependencies.Cache(rulesets = new RulesetStore(ContextFactory));
-            Dependencies.Cache(manager = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, host, Beatmap.Default));
-
             Beatmap.Value = CreateWorkingBeatmap(new OsuRuleset().RulesetInfo);
 
             nowPlayingOverlay = new NowPlayingOverlay
@@ -51,49 +39,5 @@ namespace osu.Game.Tests.Visual.UserInterface
             AddToggleStep(@"toggle beatmap lock", state => Beatmap.Disabled = state);
             AddStep(@"hide", () => nowPlayingOverlay.Hide());
         }
-
-        private BeatmapManager manager { get; set; }
-
-        private int importId;
-
-        [Test]
-        public void TestPrevTrackBehavior()
-        {
-            // ensure we have at least two beatmaps available.
-            AddRepeatStep("import beatmap", () => manager.Import(new BeatmapSetInfo
-            {
-                Beatmaps = new List<BeatmapInfo>
-                {
-                    new BeatmapInfo
-                    {
-                        BaseDifficulty = new BeatmapDifficulty(),
-                    }
-                },
-                Metadata = new BeatmapMetadata
-                {
-                    Artist = $"a test map {importId++}",
-                    Title = "title",
-                }
-            }).Wait(), 5);
-
-            WorkingBeatmap currentBeatmap = null;
-
-            AddStep("import beatmap with track", () =>
-            {
-                var setWithTrack = manager.Import(TestResources.GetTestBeatmapForImport()).Result;
-                Beatmap.Value = currentBeatmap = manager.GetWorkingBeatmap(setWithTrack.Beatmaps.First());
-            });
-
-            AddStep(@"Seek track to 6 second", () => musicController.SeekTo(6000));
-            AddUntilStep(@"Wait for current time to update", () => musicController.CurrentTrack.CurrentTime > 5000);
-
-            AddStep(@"Set previous", () => musicController.PreviousTrack());
-
-            AddAssert(@"Check beatmap didn't change", () => currentBeatmap == Beatmap.Value);
-            AddUntilStep("Wait for current time to update", () => musicController.CurrentTrack.CurrentTime < 5000);
-
-            AddStep(@"Set previous", () => musicController.PreviousTrack());
-            AddAssert(@"Check beatmap did change", () => currentBeatmap != Beatmap.Value);
-        }
     }
 }

osu.Game/OsuGame.cs

@@ -38,6 +38,7 @@ using osu.Game.Input;
 using osu.Game.Overlays.Notifications;
 using osu.Game.Input.Bindings;
 using osu.Game.Online.Chat;
+using osu.Game.Overlays.Music;
 using osu.Game.Skinning;
 using osuTK.Graphics;
 using osu.Game.Overlays.Volume;
@@ -647,6 +648,7 @@ namespace osu.Game
             chatOverlay.State.ValueChanged += state => channelManager.HighPollRate.Value = state.NewValue == Visibility.Visible;
 
             Add(externalLinkOpener = new ExternalLinkOpener());
+            Add(new MusicKeyBindingHandler());
 
             // side overlays which cancel each other.
             var singleDisplaySideOverlays = new OverlayContainer[] { Settings, notifications };

osu.Game/OsuGameBase.cs

@@ -250,10 +250,11 @@ namespace osu.Game
                 AddInternal(apiAccess);
             AddInternal(RulesetConfigCache);
 
-            GlobalActionContainer globalBinding;
-
             MenuCursorContainer = new MenuCursorContainer { RelativeSizeAxes = Axes.Both };
-            MenuCursorContainer.Child = globalBinding = new GlobalActionContainer(this)
+
+            GlobalActionContainer globalBindings;
+
+            MenuCursorContainer.Child = globalBindings = new GlobalActionContainer(this)
             {
                 RelativeSizeAxes = Axes.Both,
                 Child = content = new OsuTooltipContainer(MenuCursorContainer.Cursor) { RelativeSizeAxes = Axes.Both }
@@ -261,8 +262,8 @@ namespace osu.Game
 
             base.Content.Add(CreateScalingContainer().WithChild(MenuCursorContainer));
 
-            KeyBindingStore.Register(globalBinding);
+            KeyBindingStore.Register(globalBindings);
-            dependencies.Cache(globalBinding);
+            dependencies.Cache(globalBindings);
 
             PreviewTrackManager previewTrackManager;
             dependencies.Cache(previewTrackManager = new PreviewTrackManager());

osu.Game/Overlays/Music/MusicKeyBindingHandler.cs

@@ -0,0 +1,79 @@
+// 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 osu.Framework.Allocation;
+using osu.Framework.Bindables;
+using osu.Framework.Graphics;
+using osu.Framework.Input.Bindings;
+using osu.Game.Beatmaps;
+using osu.Game.Input.Bindings;
+using osu.Game.Overlays.OSD;
+
+namespace osu.Game.Overlays.Music
+{
+    /// <summary>
+    /// Handles <see cref="GlobalAction"/>s related to music playback, and displays <see cref="Toast"/>s via the global <see cref="OnScreenDisplay"/> accordingly.
+    /// </summary>
+    public class MusicKeyBindingHandler : Component, IKeyBindingHandler<GlobalAction>
+    {
+        [Resolved]
+        private IBindable<WorkingBeatmap> beatmap { get; set; }
+
+        [Resolved]
+        private MusicController musicController { get; set; }
+
+        [Resolved(canBeNull: true)]
+        private OnScreenDisplay onScreenDisplay { get; set; }
+
+        public bool OnPressed(GlobalAction action)
+        {
+            if (beatmap.Disabled)
+                return false;
+
+            switch (action)
+            {
+                case GlobalAction.MusicPlay:
+                    if (musicController.TogglePause())
+                        onScreenDisplay?.Display(new MusicActionToast(musicController.IsPlaying ? "Play track" : "Pause track"));
+
+                    return true;
+
+                case GlobalAction.MusicNext:
+                    musicController.NextTrack(() => onScreenDisplay?.Display(new MusicActionToast("Next track")));
+
+                    return true;
+
+                case GlobalAction.MusicPrev:
+                    musicController.PreviousTrack(res =>
+                    {
+                        switch (res)
+                        {
+                            case PreviousTrackResult.Restart:
+                                onScreenDisplay?.Display(new MusicActionToast("Restart track"));
+                                break;
+
+                            case PreviousTrackResult.Previous:
+                                onScreenDisplay?.Display(new MusicActionToast("Previous track"));
+                                break;
+                        }
+                    });
+
+                    return true;
+            }
+
+            return false;
+        }
+
+        public void OnReleased(GlobalAction action)
+        {
+        }
+
+        private class MusicActionToast : Toast
+        {
+            public MusicActionToast(string action)
+                : base("Music Playback", action, string.Empty)
+            {
+            }
+        }
+    }
+}

osu.Game/Overlays/MusicController.cs

@@ -12,12 +12,9 @@ using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Audio;
 using osu.Framework.Graphics.Containers;
-using osu.Framework.Input.Bindings;
 using osu.Framework.Utils;
 using osu.Framework.Threading;
 using osu.Game.Beatmaps;
-using osu.Game.Input.Bindings;
-using osu.Game.Overlays.OSD;
 using osu.Game.Rulesets.Mods;
 
 namespace osu.Game.Overlays
@@ -25,7 +22,7 @@ namespace osu.Game.Overlays
     /// <summary>
     /// Handles playback of the global music track.
     /// </summary>
-    public class MusicController : CompositeDrawable, IKeyBindingHandler<GlobalAction>
+    public class MusicController : CompositeDrawable
     {
         [Resolved]
         private BeatmapManager beatmaps { get; set; }
@@ -62,9 +59,6 @@ namespace osu.Game.Overlays
         [Resolved]
         private IBindable<IReadOnlyList<Mod>> mods { get; set; }
 
-        [Resolved(canBeNull: true)]
-        private OnScreenDisplay onScreenDisplay { get; set; }
-
         [NotNull]
         public DrawableTrack CurrentTrack { get; private set; } = new DrawableTrack(new TrackVirtual(1000));
 
@@ -207,7 +201,13 @@ namespace osu.Game.Overlays
         /// <summary>
         /// Play the previous track or restart the current track if it's current time below <see cref="restart_cutoff_point"/>.
         /// </summary>
-        public void PreviousTrack() => Schedule(() => prev());
+        /// <param name="onSuccess">Invoked when the operation has been performed successfully.</param>
+        public void PreviousTrack(Action<PreviousTrackResult> onSuccess = null) => Schedule(() =>
+        {
+            PreviousTrackResult res = prev();
+            if (res != PreviousTrackResult.None)
+                onSuccess?.Invoke(res);
+        });
 
         /// <summary>
         /// Play the previous track or restart the current track if it's current time below <see cref="restart_cutoff_point"/>.
@@ -243,7 +243,14 @@ namespace osu.Game.Overlays
         /// <summary>
         /// Play the next random or playlist track.
         /// </summary>
-        public void NextTrack() => Schedule(() => next());
+        /// <param name="onSuccess">Invoked when the operation has been performed successfully.</param>
+        /// <returns>A <see cref="ScheduledDelegate"/> of the operation.</returns>
+        public void NextTrack(Action onSuccess = null) => Schedule(() =>
+        {
+            bool res = next();
+            if (res)
+                onSuccess?.Invoke();
+        });
 
         private bool next()
         {
@@ -407,54 +414,6 @@ namespace osu.Game.Overlays
                     mod.ApplyToTrack(CurrentTrack);
             }
         }
-
-        public bool OnPressed(GlobalAction action)
-        {
-            if (beatmap.Disabled)
-                return false;
-
-            switch (action)
-            {
-                case GlobalAction.MusicPlay:
-                    if (TogglePause())
-                        onScreenDisplay?.Display(new MusicControllerToast(IsPlaying ? "Play track" : "Pause track"));
-                    return true;
-
-                case GlobalAction.MusicNext:
-                    if (next())
-                        onScreenDisplay?.Display(new MusicControllerToast("Next track"));
-
-                    return true;
-
-                case GlobalAction.MusicPrev:
-                    switch (prev())
-                    {
-                        case PreviousTrackResult.Restart:
-                            onScreenDisplay?.Display(new MusicControllerToast("Restart track"));
-                            break;
-
-                        case PreviousTrackResult.Previous:
-                            onScreenDisplay?.Display(new MusicControllerToast("Previous track"));
-                            break;
-                    }
-
-                    return true;
-            }
-
-            return false;
-        }
-
-        public void OnReleased(GlobalAction action)
-        {
-        }
-
-        public class MusicControllerToast : Toast
-        {
-            public MusicControllerToast(string action)
-                : base("Music Playback", action, string.Empty)
-            {
-            }
-        }
     }
 
     public enum TrackChangeDirection