Merge pull request #24361 from peppy/fix-editor-global-music-hotkey-conflicts

Disallow interacting with the global track state in `Player` and `Editor`

osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs

@@ -170,6 +170,39 @@ namespace osu.Game.Tests.Visual.Navigation
             AddUntilStep("time is correct", () => getEditor().ChildrenOfType<EditorClock>().First().CurrentTime, () => Is.EqualTo(1234));
         }
 
+        [Test]
+        public void TestAttemptGlobalMusicOperationFromEditor()
+        {
+            BeatmapSetInfo beatmapSet = null!;
+
+            AddStep("import test beatmap", () => Game.BeatmapManager.Import(TestResources.GetTestBeatmapForImport()).WaitSafely());
+            AddStep("retrieve beatmap", () => beatmapSet = Game.BeatmapManager.QueryBeatmapSet(set => !set.Protected).AsNonNull().Value.Detach());
+
+            AddStep("present beatmap", () => Game.PresentBeatmap(beatmapSet));
+            AddUntilStep("wait for song select",
+                () => Game.Beatmap.Value.BeatmapSetInfo.Equals(beatmapSet)
+                      && Game.ScreenStack.CurrentScreen is PlaySongSelect songSelect
+                      && songSelect.IsLoaded);
+
+            AddUntilStep("wait for music playing", () => Game.MusicController.IsPlaying);
+            AddStep("user request stop", () => Game.MusicController.Stop(requestedByUser: true));
+            AddUntilStep("wait for music stopped", () => !Game.MusicController.IsPlaying);
+
+            AddStep("open editor", () => ((PlaySongSelect)Game.ScreenStack.CurrentScreen).Edit(beatmapSet.Beatmaps.First(beatmap => beatmap.Ruleset.OnlineID == 0)));
+            AddUntilStep("wait for editor open", () => Game.ScreenStack.CurrentScreen is Editor editor && editor.ReadyForUse);
+
+            AddUntilStep("music still stopped", () => !Game.MusicController.IsPlaying);
+            AddStep("user request play", () => Game.MusicController.Play(requestedByUser: true));
+            AddUntilStep("music still stopped", () => !Game.MusicController.IsPlaying);
+
+            AddStep("exit to song select", () => Game.PerformFromScreen(_ => { }, typeof(PlaySongSelect).Yield()));
+            AddUntilStep("wait for song select", () => Game.ScreenStack.CurrentScreen is PlaySongSelect);
+
+            AddUntilStep("wait for music playing", () => Game.MusicController.IsPlaying);
+            AddStep("user request stop", () => Game.MusicController.Stop(requestedByUser: true));
+            AddUntilStep("wait for music stopped", () => !Game.MusicController.IsPlaying);
+        }
+
         private EditorBeatmap getEditorBeatmap() => getEditor().ChildrenOfType<EditorBeatmap>().Single();
 
         private Editor getEditor() => (Editor)Game.ScreenStack.CurrentScreen;

osu.Game.Tests/Visual/TestSceneOsuScreenStack.cs

@@ -56,38 +56,38 @@ namespace osu.Game.Tests.Visual
         public void AllowTrackAdjustmentsTest()
         {
             AddStep("push allowing screen", () => stack.Push(loadNewScreen<AllowScreen>()));
-            AddAssert("allows adjustments 1", () => musicController.AllowTrackAdjustments);
+            AddAssert("allows adjustments 1", () => musicController.ApplyModTrackAdjustments);
 
             AddStep("push inheriting screen", () => stack.Push(loadNewScreen<InheritScreen>()));
-            AddAssert("allows adjustments 2", () => musicController.AllowTrackAdjustments);
+            AddAssert("allows adjustments 2", () => musicController.ApplyModTrackAdjustments);
 
             AddStep("push disallowing screen", () => stack.Push(loadNewScreen<DisallowScreen>()));
-            AddAssert("disallows adjustments 3", () => !musicController.AllowTrackAdjustments);
+            AddAssert("disallows adjustments 3", () => !musicController.ApplyModTrackAdjustments);
 
             AddStep("push inheriting screen", () => stack.Push(loadNewScreen<InheritScreen>()));
-            AddAssert("disallows adjustments 4", () => !musicController.AllowTrackAdjustments);
+            AddAssert("disallows adjustments 4", () => !musicController.ApplyModTrackAdjustments);
 
             AddStep("push inheriting screen", () => stack.Push(loadNewScreen<InheritScreen>()));
-            AddAssert("disallows adjustments 5", () => !musicController.AllowTrackAdjustments);
+            AddAssert("disallows adjustments 5", () => !musicController.ApplyModTrackAdjustments);
 
             AddStep("push allowing screen", () => stack.Push(loadNewScreen<AllowScreen>()));
-            AddAssert("allows adjustments 6", () => musicController.AllowTrackAdjustments);
+            AddAssert("allows adjustments 6", () => musicController.ApplyModTrackAdjustments);
 
             // Now start exiting from screens
             AddStep("exit screen", () => stack.Exit());
-            AddAssert("disallows adjustments 7", () => !musicController.AllowTrackAdjustments);
+            AddAssert("disallows adjustments 7", () => !musicController.ApplyModTrackAdjustments);
 
             AddStep("exit screen", () => stack.Exit());
-            AddAssert("disallows adjustments 8", () => !musicController.AllowTrackAdjustments);
+            AddAssert("disallows adjustments 8", () => !musicController.ApplyModTrackAdjustments);
 
             AddStep("exit screen", () => stack.Exit());
-            AddAssert("disallows adjustments 9", () => !musicController.AllowTrackAdjustments);
+            AddAssert("disallows adjustments 9", () => !musicController.ApplyModTrackAdjustments);
 
             AddStep("exit screen", () => stack.Exit());
-            AddAssert("allows adjustments 10", () => musicController.AllowTrackAdjustments);
+            AddAssert("allows adjustments 10", () => musicController.ApplyModTrackAdjustments);
 
             AddStep("exit screen", () => stack.Exit());
-            AddAssert("allows adjustments 11", () => musicController.AllowTrackAdjustments);
+            AddAssert("allows adjustments 11", () => musicController.ApplyModTrackAdjustments);
         }
 
         public partial class TestScreen : ScreenWithBeatmapBackground
@@ -129,12 +129,12 @@ namespace osu.Game.Tests.Visual
 
         private partial class AllowScreen : OsuScreen
         {
-            public override bool? AllowTrackAdjustments => true;
+            public override bool? ApplyModTrackAdjustments => true;
         }
 
         public partial class DisallowScreen : OsuScreen
         {
-            public override bool? AllowTrackAdjustments => false;
+            public override bool? ApplyModTrackAdjustments => false;
         }
 
         private partial class InheritScreen : OsuScreen

osu.Game/Overlays/FirstRunSetup/ScreenUIScale.cs

@@ -104,7 +104,7 @@ namespace osu.Game.Overlays.FirstRunSetup
         {
             protected override bool ControlGlobalMusic => false;
 
-            public override bool? AllowTrackAdjustments => false;
+            public override bool? ApplyModTrackAdjustments => false;
         }
 
         private partial class UIScaleSlider : RoundedSliderBar<float>

osu.Game/Overlays/Music/MusicKeyBindingHandler.cs

@@ -30,20 +30,14 @@ namespace osu.Game.Overlays.Music
         [Resolved]
         private OnScreenDisplay? onScreenDisplay { get; set; }
 
-        [Resolved]
-        private OsuGame game { get; set; } = null!;
-
         public bool OnPressed(KeyBindingPressEvent<GlobalAction> e)
         {
-            if (e.Repeat)
+            if (e.Repeat || !musicController.AllowTrackControl.Value)
                 return false;
 
             switch (e.Action)
             {
                 case GlobalAction.MusicPlay:
-                    if (game.LocalUserPlaying.Value)
-                        return false;
-
                     // use previous state as TogglePause may not update the track's state immediately (state update is run on the audio thread see https://github.com/ppy/osu/issues/9880#issuecomment-674668842)
                     bool wasPlaying = musicController.IsPlaying;
 

osu.Game/Overlays/MusicController.cs

@@ -40,6 +40,11 @@ namespace osu.Game.Overlays
         /// </summary>
         public bool UserPauseRequested { get; private set; }
 
+        /// <summary>
+        /// Whether user control of the global track should be allowed.
+        /// </summary>
+        public readonly BindableBool AllowTrackControl = new BindableBool(true);
+
         /// <summary>
         /// Fired when the global <see cref="WorkingBeatmap"/> has changed.
         /// Includes direction information for display purposes.
@@ -92,8 +97,10 @@ namespace osu.Game.Overlays
             seekDelegate?.Cancel();
             seekDelegate = Schedule(() =>
             {
-                if (!beatmap.Disabled)
+                if (beatmap.Disabled || !AllowTrackControl.Value)
-                    CurrentTrack.Seek(position);
+                    return;
+
+                CurrentTrack.Seek(position);
             });
         }
 
@@ -107,7 +114,7 @@ namespace osu.Game.Overlays
 
             if (CurrentTrack.IsDummyDevice || beatmap.Value.BeatmapSetInfo.DeletePending)
             {
-                if (beatmap.Disabled)
+                if (beatmap.Disabled || !AllowTrackControl.Value)
                     return;
 
                 Logger.Log($"{nameof(MusicController)} skipping next track to {nameof(EnsurePlayingSomething)}");
@@ -132,6 +139,9 @@ namespace osu.Game.Overlays
         /// <returns>Whether the operation was successful.</returns>
         public bool Play(bool restart = false, bool requestedByUser = false)
         {
+            if (requestedByUser && !AllowTrackControl.Value)
+                return false;
+
             if (requestedByUser)
                 UserPauseRequested = false;
 
@@ -153,6 +163,9 @@ namespace osu.Game.Overlays
         /// </param>
         public void Stop(bool requestedByUser = false)
         {
+            if (requestedByUser && !AllowTrackControl.Value)
+                return;
+
             UserPauseRequested |= requestedByUser;
             if (CurrentTrack.IsRunning)
                 CurrentTrack.StopAsync();
@@ -164,6 +177,9 @@ namespace osu.Game.Overlays
         /// <returns>Whether the operation was successful.</returns>
         public bool TogglePause()
         {
+            if (!AllowTrackControl.Value)
+                return false;
+
             if (CurrentTrack.IsRunning)
                 Stop(true);
             else
@@ -189,7 +205,7 @@ namespace osu.Game.Overlays
         /// <returns>The <see cref="PreviousTrackResult"/> that indicate the decided action.</returns>
         private PreviousTrackResult prev()
         {
-            if (beatmap.Disabled)
+            if (beatmap.Disabled || !AllowTrackControl.Value)
                 return PreviousTrackResult.None;
 
             double currentTrackPosition = CurrentTrack.CurrentTime;
@@ -229,7 +245,7 @@ namespace osu.Game.Overlays
 
         private bool next()
         {
-            if (beatmap.Disabled)
+            if (beatmap.Disabled || !AllowTrackControl.Value)
                 return false;
 
             queuedDirection = TrackChangeDirection.Next;
@@ -352,24 +368,24 @@ namespace osu.Game.Overlays
 
         private void onTrackCompleted()
         {
-            if (!CurrentTrack.Looping && !beatmap.Disabled)
+            if (!CurrentTrack.Looping && !beatmap.Disabled && AllowTrackControl.Value)
                 NextTrack();
         }
 
-        private bool allowTrackAdjustments;
+        private bool applyModTrackAdjustments;
 
         /// <summary>
         /// Whether mod track adjustments are allowed to be applied.
         /// </summary>
-        public bool AllowTrackAdjustments
+        public bool ApplyModTrackAdjustments
         {
-            get => allowTrackAdjustments;
+            get => applyModTrackAdjustments;
             set
             {
-                if (allowTrackAdjustments == value)
+                if (applyModTrackAdjustments == value)
                     return;
 
-                allowTrackAdjustments = value;
+                applyModTrackAdjustments = value;
                 ResetTrackAdjustments();
             }
         }
@@ -377,7 +393,7 @@ namespace osu.Game.Overlays
         private AudioAdjustments modTrackAdjustments;
 
         /// <summary>
-        /// Resets the adjustments currently applied on <see cref="CurrentTrack"/> and applies the mod adjustments if <see cref="AllowTrackAdjustments"/> is <c>true</c>.
+        /// Resets the adjustments currently applied on <see cref="CurrentTrack"/> and applies the mod adjustments if <see cref="ApplyModTrackAdjustments"/> is <c>true</c>.
         /// </summary>
         /// <remarks>
         /// Does not reset any adjustments applied directly to the beatmap track.
@@ -390,7 +406,7 @@ namespace osu.Game.Overlays
             CurrentTrack.RemoveAllAdjustments(AdjustableProperty.Tempo);
             CurrentTrack.RemoveAllAdjustments(AdjustableProperty.Volume);
 
-            if (allowTrackAdjustments)
+            if (applyModTrackAdjustments)
             {
                 CurrentTrack.BindAdjustments(modTrackAdjustments = new AudioAdjustments());
 

osu.Game/Overlays/NowPlayingOverlay.cs

@@ -68,6 +68,8 @@ namespace osu.Game.Overlays
         [Resolved]
         private OsuColour colours { get; set; }
 
+        private Bindable<bool> allowTrackControl;
+
         public NowPlayingOverlay()
         {
             Width = 400;
@@ -220,8 +222,10 @@ namespace osu.Game.Overlays
         {
             base.LoadComplete();
 
-            beatmap.BindDisabledChanged(_ => Scheduler.AddOnce(beatmapDisabledChanged));
+            beatmap.BindDisabledChanged(_ => Scheduler.AddOnce(updateEnabledStates));
-            beatmapDisabledChanged();
+
+            allowTrackControl = musicController.AllowTrackControl.GetBoundCopy();
+            allowTrackControl.BindValueChanged(_ => Scheduler.AddOnce(updateEnabledStates), true);
 
             musicController.TrackChanged += trackChanged;
             trackChanged(beatmap.Value);
@@ -334,16 +338,18 @@ namespace osu.Game.Overlays
             };
         }
 
-        private void beatmapDisabledChanged()
+        private void updateEnabledStates()
         {
-            bool disabled = beatmap.Disabled;
+            bool beatmapDisabled = beatmap.Disabled;
+            bool trackControlDisabled = !musicController.AllowTrackControl.Value;
 
-            if (disabled)
+            if (beatmapDisabled || trackControlDisabled)
                 playlist?.Hide();
 
-            prevButton.Enabled.Value = !disabled;
+            prevButton.Enabled.Value = !beatmapDisabled && !trackControlDisabled;
-            nextButton.Enabled.Value = !disabled;
+            nextButton.Enabled.Value = !beatmapDisabled && !trackControlDisabled;
-            playlistButton.Enabled.Value = !disabled;
+            playlistButton.Enabled.Value = !beatmapDisabled && !trackControlDisabled;
+            playButton.Enabled.Value = !trackControlDisabled;
         }
 
         protected override void Dispose(bool isDisposing)

osu.Game/Screens/Edit/Editor.cs

@@ -66,7 +66,7 @@ namespace osu.Game.Screens.Edit
 
         public override bool DisallowExternalBeatmapRulesetChanges => true;
 
-        public override bool? AllowTrackAdjustments => false;
+        public override bool? ApplyModTrackAdjustments => false;
 
         protected override bool PlayExitSound => !ExitConfirmed && !switchingDifficulty;
 

osu.Game/Screens/Edit/EditorLoader.cs

@@ -42,6 +42,8 @@ namespace osu.Game.Screens.Edit
 
         public override bool DisallowExternalBeatmapRulesetChanges => true;
 
+        public override bool? AllowGlobalTrackControl => false;
+
         [Resolved]
         private BeatmapManager beatmapManager { get; set; }
 

osu.Game/Screens/IOsuScreen.cs

@@ -67,7 +67,13 @@ namespace osu.Game.Screens
         /// Whether mod track adjustments should be applied on entering this screen.
         /// A <see langword="null"/> value means that the parent screen's value of this setting will be used.
         /// </summary>
-        bool? AllowTrackAdjustments { get; }
+        bool? ApplyModTrackAdjustments { get; }
+
+        /// <summary>
+        /// Whether control of the global track should be allowed via the music controller / now playing overlay.
+        /// A <see langword="null"/> value means that the parent screen's value of this setting will be used.
+        /// </summary>
+        bool? AllowGlobalTrackControl { get; }
 
         /// <summary>
         /// Invoked when the back button has been pressed to close any overlays before exiting this <see cref="IOsuScreen"/>.

osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs

@@ -41,7 +41,7 @@ namespace osu.Game.Screens.OnlinePlay.Match
         [Cached(typeof(IBindable<PlaylistItem>))]
         public readonly Bindable<PlaylistItem> SelectedItem = new Bindable<PlaylistItem>();
 
-        public override bool? AllowTrackAdjustments => true;
+        public override bool? ApplyModTrackAdjustments => true;
 
         protected override BackgroundScreen CreateBackground() => new RoomBackgroundScreen(Room.Playlist.FirstOrDefault())
         {

osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs

@@ -29,7 +29,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
         public override bool DisallowExternalBeatmapRulesetChanges => true;
 
         // We are managing our own adjustments. For now, this happens inside the Player instances themselves.
-        public override bool? AllowTrackAdjustments => false;
+        public override bool? ApplyModTrackAdjustments => false;
 
         /// <summary>
         /// Whether all spectating players have finished loading.

osu.Game/Screens/OsuScreen.cs

@@ -85,7 +85,9 @@ namespace osu.Game.Screens
         [Resolved]
         private MusicController musicController { get; set; }
 
-        public virtual bool? AllowTrackAdjustments => null;
+        public virtual bool? ApplyModTrackAdjustments => null;
+
+        public virtual bool? AllowGlobalTrackControl => null;
 
         public Bindable<WorkingBeatmap> Beatmap { get; private set; }
 
@@ -95,7 +97,9 @@ namespace osu.Game.Screens
 
         private OsuScreenDependencies screenDependencies;
 
-        private bool? trackAdjustmentStateAtSuspend;
+        private bool? globalMusicControlStateAtSuspend;
+
+        private bool? modTrackAdjustmentStateAtSuspend;
 
         internal void CreateLeasedDependencies(IReadOnlyDependencyContainer dependencies) => createDependencies(dependencies);
 
@@ -178,8 +182,10 @@ namespace osu.Game.Screens
 
             // it's feasible to resume to a screen if the target screen never loaded successfully.
             // in such a case there's no need to restore this value.
-            if (trackAdjustmentStateAtSuspend != null)
+            if (modTrackAdjustmentStateAtSuspend != null)
-                musicController.AllowTrackAdjustments = trackAdjustmentStateAtSuspend.Value;
+                musicController.ApplyModTrackAdjustments = modTrackAdjustmentStateAtSuspend.Value;
+            if (globalMusicControlStateAtSuspend != null)
+                musicController.AllowTrackControl.Value = globalMusicControlStateAtSuspend.Value;
 
             base.OnResuming(e);
         }
@@ -188,7 +194,8 @@ namespace osu.Game.Screens
         {
             base.OnSuspending(e);
 
-            trackAdjustmentStateAtSuspend = musicController.AllowTrackAdjustments;
+            modTrackAdjustmentStateAtSuspend = musicController.ApplyModTrackAdjustments;
+            globalMusicControlStateAtSuspend = musicController.AllowTrackControl.Value;
 
             onSuspendingLogo();
         }
@@ -197,8 +204,11 @@ namespace osu.Game.Screens
         {
             applyArrivingDefaults(false);
 
-            if (AllowTrackAdjustments != null)
+            if (ApplyModTrackAdjustments != null)
-                musicController.AllowTrackAdjustments = AllowTrackAdjustments.Value;
+                musicController.ApplyModTrackAdjustments = ApplyModTrackAdjustments.Value;
+
+            if (AllowGlobalTrackControl != null)
+                musicController.AllowTrackControl.Value = AllowGlobalTrackControl.Value;
 
             if (backgroundStack?.Push(ownedBackground = CreateBackground()) != true)
             {

osu.Game/Screens/Play/Player.cs

@@ -70,7 +70,7 @@ namespace osu.Game.Screens.Play
         protected override OverlayActivation InitialOverlayActivationMode => OverlayActivation.UserTriggered;
 
         // We are managing our own adjustments (see OnEntering/OnExiting).
-        public override bool? AllowTrackAdjustments => false;
+        public override bool? ApplyModTrackAdjustments => false;
 
         private readonly IBindable<bool> gameActive = new Bindable<bool>(true);
 

osu.Game/Screens/Play/PlayerLoader.cs

@@ -46,6 +46,8 @@ namespace osu.Game.Screens.Play
 
         public override bool DisallowExternalBeatmapRulesetChanges => true;
 
+        public override bool? AllowGlobalTrackControl => false;
+
         // Here because IsHovered will not update unless we do so.
         public override bool HandlePositionalInput => true;
 

osu.Game/Screens/Ranking/ResultsScreen.cs

@@ -36,6 +36,8 @@ namespace osu.Game.Screens.Ranking
 
         public override bool DisallowExternalBeatmapRulesetChanges => true;
 
+        public override bool? AllowGlobalTrackControl => true;
+
         // Temporary for now to stop dual transitions. Should respect the current toolbar mode, but there's no way to do so currently.
         public override bool HideOverlaysOnEnter => true;
 

osu.Game/Screens/Select/SongSelect.cs

@@ -60,7 +60,7 @@ namespace osu.Game.Screens.Select
 
         protected virtual bool ShowFooter => true;
 
-        public override bool? AllowTrackAdjustments => true;
+        public override bool? ApplyModTrackAdjustments => true;
 
         /// <summary>
         /// Can be null if <see cref="ShowFooter"/> is false.