From b9193aae6dcec54c1e5ff1a51f6863e240de7c74 Mon Sep 17 00:00:00 2001 From: AbstractQbit <38468635+AbstractQbit@users.noreply.github.com> Date: Tue, 14 Sep 2021 17:37:57 +0300 Subject: [PATCH 01/15] Make IOsuScreen.AllowTrackAdjustments nullable Allows for inheriting value from the previous screen if undefined --- osu.Game/OsuGame.cs | 29 +++++++++++++++++-- .../Maintenance/DirectorySelectScreen.cs | 2 +- osu.Game/Screens/Edit/Editor.cs | 2 +- osu.Game/Screens/IOsuScreen.cs | 3 +- osu.Game/Screens/Import/FileImportScreen.cs | 2 +- osu.Game/Screens/Menu/MainMenu.cs | 2 +- .../Spectate/MultiSpectatorScreen.cs | 2 +- .../Screens/OnlinePlay/OnlinePlayScreen.cs | 2 +- .../Screens/OnlinePlay/OnlinePlaySubScreen.cs | 2 ++ osu.Game/Screens/OsuScreen.cs | 2 +- osu.Game/Screens/Play/Player.cs | 2 +- .../Play/ScreenWithBeatmapBackground.cs | 2 ++ osu.Game/Screens/StartupScreen.cs | 2 +- 13 files changed, 42 insertions(+), 12 deletions(-) diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 2107b3a0e9..c6a07bbbba 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -1075,8 +1075,6 @@ namespace osu.Game OverlayActivationMode.BindTo(newOsuScreen.OverlayActivationMode); API.Activity.BindTo(newOsuScreen.Activity); - MusicController.AllowTrackAdjustments = newOsuScreen.AllowTrackAdjustments; - if (newOsuScreen.HideOverlaysOnEnter) CloseAllOverlays(); else @@ -1093,6 +1091,24 @@ namespace osu.Game { ScreenChanged(lastScreen, newScreen); Logger.Log($"Screen changed → {newScreen}"); + + // set AllowTrackAdjustments if new screen defines it, inherit otherwise + if (newScreen is IOsuScreen newOsuScreen && newOsuScreen.AllowTrackAdjustments.HasValue) + { + allowTrackAdjustmentsStack.Push(newOsuScreen.AllowTrackAdjustments.Value); + Logger.Log($"Screen's AllowTrackAdjustments explicit → {allowTrackAdjustmentsStack.First()}"); + } + else if (allowTrackAdjustmentsStack.Any()) + { + allowTrackAdjustmentsStack.Push(allowTrackAdjustmentsStack.First()); + Logger.Log($"Screen's AllowTrackAdjustments inherit → {allowTrackAdjustmentsStack.First()}"); + } + else + { + allowTrackAdjustmentsStack.Push(false); + } + + MusicController.AllowTrackAdjustments = allowTrackAdjustmentsStack.First(); } private void screenExited(IScreen lastScreen, IScreen newScreen) @@ -1102,8 +1118,17 @@ namespace osu.Game if (newScreen == null) Exit(); + + if (allowTrackAdjustmentsStack.Count > 1) + { + allowTrackAdjustmentsStack.Pop(); + MusicController.AllowTrackAdjustments = allowTrackAdjustmentsStack.First(); + Logger.Log($"Screen's AllowTrackAdjustments return ← {allowTrackAdjustmentsStack.First()}"); + } } + private Stack allowTrackAdjustmentsStack = new Stack(); + IBindable ILocalUserPlayInfo.IsPlaying => LocalUserPlaying; } } diff --git a/osu.Game/Overlays/Settings/Sections/Maintenance/DirectorySelectScreen.cs b/osu.Game/Overlays/Settings/Sections/Maintenance/DirectorySelectScreen.cs index 1d67968ab1..6d0e79e2c7 100644 --- a/osu.Game/Overlays/Settings/Sections/Maintenance/DirectorySelectScreen.cs +++ b/osu.Game/Overlays/Settings/Sections/Maintenance/DirectorySelectScreen.cs @@ -24,7 +24,7 @@ namespace osu.Game.Overlays.Settings.Sections.Maintenance private OsuDirectorySelector directorySelector; - public override bool AllowTrackAdjustments => false; + public override bool? AllowTrackAdjustments => false; /// /// Text to display in the header to inform the user of what they are selecting. diff --git a/osu.Game/Screens/Edit/Editor.cs b/osu.Game/Screens/Edit/Editor.cs index 28ae7e620e..13e4a11915 100644 --- a/osu.Game/Screens/Edit/Editor.cs +++ b/osu.Game/Screens/Edit/Editor.cs @@ -56,7 +56,7 @@ namespace osu.Game.Screens.Edit public override bool DisallowExternalBeatmapRulesetChanges => true; - public override bool AllowTrackAdjustments => false; + public override bool? AllowTrackAdjustments => false; protected bool HasUnsavedChanges => lastSavedHash != changeHandler.CurrentStateHash; diff --git a/osu.Game/Screens/IOsuScreen.cs b/osu.Game/Screens/IOsuScreen.cs index 17384c161c..a1e4d9ed01 100644 --- a/osu.Game/Screens/IOsuScreen.cs +++ b/osu.Game/Screens/IOsuScreen.cs @@ -60,8 +60,9 @@ namespace osu.Game.Screens /// /// Whether mod track adjustments are allowed to be applied. + /// Null means to inherit from the parent screen. /// - bool AllowTrackAdjustments { get; } + bool? AllowTrackAdjustments { get; } /// /// Invoked when the back button has been pressed to close any overlays before exiting this . diff --git a/osu.Game/Screens/Import/FileImportScreen.cs b/osu.Game/Screens/Import/FileImportScreen.cs index 606174193d..69fcf31876 100644 --- a/osu.Game/Screens/Import/FileImportScreen.cs +++ b/osu.Game/Screens/Import/FileImportScreen.cs @@ -23,7 +23,7 @@ namespace osu.Game.Screens.Import { public override bool HideOverlaysOnEnter => true; - public override bool AllowTrackAdjustments => false; + public override bool? AllowTrackAdjustments => false; private OsuFileSelector fileSelector; private Container contentContainer; diff --git a/osu.Game/Screens/Menu/MainMenu.cs b/osu.Game/Screens/Menu/MainMenu.cs index 8b2ec43e3e..00885a91c5 100644 --- a/osu.Game/Screens/Menu/MainMenu.cs +++ b/osu.Game/Screens/Menu/MainMenu.cs @@ -36,7 +36,7 @@ namespace osu.Game.Screens.Menu public override bool AllowExternalScreenChange => true; - public override bool AllowTrackAdjustments => false; + public override bool? AllowTrackAdjustments => false; private Screen songSelect; diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs index bf7c738882..c45e3a79da 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs @@ -26,7 +26,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? AllowTrackAdjustments => false; /// /// Whether all spectating players have finished loading. diff --git a/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs b/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs index 62bfd2cfed..e3945c9cac 100644 --- a/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs +++ b/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs @@ -24,7 +24,7 @@ namespace osu.Game.Screens.OnlinePlay [Cached] protected readonly OverlayColourProvider ColourProvider = new OverlayColourProvider(OverlayColourScheme.Plum); - public override bool AllowTrackAdjustments => false; + public override bool? AllowTrackAdjustments => false; public override bool CursorVisible => (screenStack?.CurrentScreen as IOnlinePlaySubScreen)?.CursorVisible ?? true; diff --git a/osu.Game/Screens/OnlinePlay/OnlinePlaySubScreen.cs b/osu.Game/Screens/OnlinePlay/OnlinePlaySubScreen.cs index 3411c4afb1..8c4f0c1394 100644 --- a/osu.Game/Screens/OnlinePlay/OnlinePlaySubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/OnlinePlaySubScreen.cs @@ -11,6 +11,8 @@ namespace osu.Game.Screens.OnlinePlay { public override bool DisallowExternalBeatmapRulesetChanges => false; + public override bool? AllowTrackAdjustments => true; + public virtual string ShortTitle => Title; [Resolved(CanBeNull = true)] diff --git a/osu.Game/Screens/OsuScreen.cs b/osu.Game/Screens/OsuScreen.cs index 9aec2a5c19..78908b5d8a 100644 --- a/osu.Game/Screens/OsuScreen.cs +++ b/osu.Game/Screens/OsuScreen.cs @@ -81,7 +81,7 @@ namespace osu.Game.Screens public virtual float BackgroundParallaxAmount => 1; - public virtual bool AllowTrackAdjustments => true; + public virtual bool? AllowTrackAdjustments => null; public Bindable Beatmap { get; private set; } diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index e8a2790c94..9927467bd6 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -56,7 +56,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? AllowTrackAdjustments => false; private readonly IBindable gameActive = new Bindable(true); diff --git a/osu.Game/Screens/Play/ScreenWithBeatmapBackground.cs b/osu.Game/Screens/Play/ScreenWithBeatmapBackground.cs index 88dab88d42..9bec320e22 100644 --- a/osu.Game/Screens/Play/ScreenWithBeatmapBackground.cs +++ b/osu.Game/Screens/Play/ScreenWithBeatmapBackground.cs @@ -10,6 +10,8 @@ namespace osu.Game.Screens.Play { protected override BackgroundScreen CreateBackground() => new BackgroundScreenBeatmap(Beatmap.Value); + public override bool? AllowTrackAdjustments => true; + public void ApplyToBackground(Action action) => base.ApplyToBackground(b => action.Invoke((BackgroundScreenBeatmap)b)); } } diff --git a/osu.Game/Screens/StartupScreen.cs b/osu.Game/Screens/StartupScreen.cs index 15f75d7cff..7b73d36fdf 100644 --- a/osu.Game/Screens/StartupScreen.cs +++ b/osu.Game/Screens/StartupScreen.cs @@ -16,7 +16,7 @@ namespace osu.Game.Screens public override bool CursorVisible => false; - public override bool AllowTrackAdjustments => false; + public override bool? AllowTrackAdjustments => false; protected override OverlayActivation InitialOverlayActivationMode => OverlayActivation.Disabled; } From 01d2f4f17a98bfb372410838eb8159c260b6fa8a Mon Sep 17 00:00:00 2001 From: AbstractQbit <38468635+AbstractQbit@users.noreply.github.com> Date: Tue, 14 Sep 2021 18:04:43 +0300 Subject: [PATCH 02/15] Make `allowTrackAdjustmentsStack` readonly --- osu.Game/OsuGame.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index c6a07bbbba..5f1b2ac87d 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -1127,7 +1127,7 @@ namespace osu.Game } } - private Stack allowTrackAdjustmentsStack = new Stack(); + private readonly Stack allowTrackAdjustmentsStack = new Stack(); IBindable ILocalUserPlayInfo.IsPlaying => LocalUserPlaying; } From bd18c581c11a6437a9a055ae4b267425d8463d6f Mon Sep 17 00:00:00 2001 From: AbstractQbit <38468635+AbstractQbit@users.noreply.github.com> Date: Tue, 14 Sep 2021 21:14:24 +0300 Subject: [PATCH 03/15] Replace `allowTrackAdjustmentsStack` with a Dictionary --- osu.Game/OsuGame.cs | 31 ++++++++++--------------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 5f1b2ac87d..499b561718 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -1094,21 +1094,13 @@ namespace osu.Game // set AllowTrackAdjustments if new screen defines it, inherit otherwise if (newScreen is IOsuScreen newOsuScreen && newOsuScreen.AllowTrackAdjustments.HasValue) - { - allowTrackAdjustmentsStack.Push(newOsuScreen.AllowTrackAdjustments.Value); - Logger.Log($"Screen's AllowTrackAdjustments explicit → {allowTrackAdjustmentsStack.First()}"); - } - else if (allowTrackAdjustmentsStack.Any()) - { - allowTrackAdjustmentsStack.Push(allowTrackAdjustmentsStack.First()); - Logger.Log($"Screen's AllowTrackAdjustments inherit → {allowTrackAdjustmentsStack.First()}"); - } + allowTrackAdjustmentsDict[newScreen] = newOsuScreen.AllowTrackAdjustments.Value; + else if (allowTrackAdjustmentsDict.ContainsKey(lastScreen)) + allowTrackAdjustmentsDict[newScreen] = allowTrackAdjustmentsDict[lastScreen]; else - { - allowTrackAdjustmentsStack.Push(false); - } + allowTrackAdjustmentsDict[newScreen] = true; - MusicController.AllowTrackAdjustments = allowTrackAdjustmentsStack.First(); + MusicController.AllowTrackAdjustments = allowTrackAdjustmentsDict[newScreen]; } private void screenExited(IScreen lastScreen, IScreen newScreen) @@ -1116,18 +1108,15 @@ namespace osu.Game ScreenChanged(lastScreen, newScreen); Logger.Log($"Screen changed ← {newScreen}"); + allowTrackAdjustmentsDict.Remove(lastScreen); + if (newScreen == null) Exit(); - - if (allowTrackAdjustmentsStack.Count > 1) - { - allowTrackAdjustmentsStack.Pop(); - MusicController.AllowTrackAdjustments = allowTrackAdjustmentsStack.First(); - Logger.Log($"Screen's AllowTrackAdjustments return ← {allowTrackAdjustmentsStack.First()}"); - } + else + MusicController.AllowTrackAdjustments = allowTrackAdjustmentsDict[newScreen]; } - private readonly Stack allowTrackAdjustmentsStack = new Stack(); + private readonly Dictionary allowTrackAdjustmentsDict = new Dictionary(); IBindable ILocalUserPlayInfo.IsPlaying => LocalUserPlaying; } From b87af3dd68e3de7b19483210e6d94437a6a6c651 Mon Sep 17 00:00:00 2001 From: AbstractQbit <38468635+AbstractQbit@users.noreply.github.com> Date: Wed, 15 Sep 2021 10:55:16 +0300 Subject: [PATCH 04/15] Move the inherited `AllowTrackAdjustments` into `OsuScreen` --- osu.Game/OsuGame.cs | 21 +++++---------------- osu.Game/Screens/IOsuScreen.cs | 2 +- osu.Game/Screens/OsuScreen.cs | 8 +++++++- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 499b561718..f2f925a778 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -1075,6 +1075,11 @@ namespace osu.Game OverlayActivationMode.BindTo(newOsuScreen.OverlayActivationMode); API.Activity.BindTo(newOsuScreen.Activity); + if (newOsuScreen.AllowTrackAdjustments.HasValue) + MusicController.AllowTrackAdjustments = newOsuScreen.AllowTrackAdjustments.Value; + else + newOsuScreen.AllowTrackAdjustments = MusicController.AllowTrackAdjustments; + if (newOsuScreen.HideOverlaysOnEnter) CloseAllOverlays(); else @@ -1091,16 +1096,6 @@ namespace osu.Game { ScreenChanged(lastScreen, newScreen); Logger.Log($"Screen changed → {newScreen}"); - - // set AllowTrackAdjustments if new screen defines it, inherit otherwise - if (newScreen is IOsuScreen newOsuScreen && newOsuScreen.AllowTrackAdjustments.HasValue) - allowTrackAdjustmentsDict[newScreen] = newOsuScreen.AllowTrackAdjustments.Value; - else if (allowTrackAdjustmentsDict.ContainsKey(lastScreen)) - allowTrackAdjustmentsDict[newScreen] = allowTrackAdjustmentsDict[lastScreen]; - else - allowTrackAdjustmentsDict[newScreen] = true; - - MusicController.AllowTrackAdjustments = allowTrackAdjustmentsDict[newScreen]; } private void screenExited(IScreen lastScreen, IScreen newScreen) @@ -1108,16 +1103,10 @@ namespace osu.Game ScreenChanged(lastScreen, newScreen); Logger.Log($"Screen changed ← {newScreen}"); - allowTrackAdjustmentsDict.Remove(lastScreen); - if (newScreen == null) Exit(); - else - MusicController.AllowTrackAdjustments = allowTrackAdjustmentsDict[newScreen]; } - private readonly Dictionary allowTrackAdjustmentsDict = new Dictionary(); - IBindable ILocalUserPlayInfo.IsPlaying => LocalUserPlaying; } } diff --git a/osu.Game/Screens/IOsuScreen.cs b/osu.Game/Screens/IOsuScreen.cs index a1e4d9ed01..262bbfedc6 100644 --- a/osu.Game/Screens/IOsuScreen.cs +++ b/osu.Game/Screens/IOsuScreen.cs @@ -62,7 +62,7 @@ namespace osu.Game.Screens /// Whether mod track adjustments are allowed to be applied. /// Null means to inherit from the parent screen. /// - bool? AllowTrackAdjustments { get; } + bool? AllowTrackAdjustments { set; get; } /// /// Invoked when the back button has been pressed to close any overlays before exiting this . diff --git a/osu.Game/Screens/OsuScreen.cs b/osu.Game/Screens/OsuScreen.cs index 78908b5d8a..0deaa3e80e 100644 --- a/osu.Game/Screens/OsuScreen.cs +++ b/osu.Game/Screens/OsuScreen.cs @@ -81,7 +81,13 @@ namespace osu.Game.Screens public virtual float BackgroundParallaxAmount => 1; - public virtual bool? AllowTrackAdjustments => null; + private bool? allowTrackAdjustments = null; + + public virtual bool? AllowTrackAdjustments + { + set => allowTrackAdjustments = value; + get => allowTrackAdjustments; + } public Bindable Beatmap { get; private set; } From 48cf98ef9325cfd40fca7ab91bb50cc102517c53 Mon Sep 17 00:00:00 2001 From: AbstractQbit <38468635+AbstractQbit@users.noreply.github.com> Date: Wed, 15 Sep 2021 11:00:49 +0300 Subject: [PATCH 05/15] Rephrase null meaning in `IOsuScreen.AllowTrackAdjustments` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bartłomiej Dach --- osu.Game/Screens/IOsuScreen.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/IOsuScreen.cs b/osu.Game/Screens/IOsuScreen.cs index 262bbfedc6..735853e462 100644 --- a/osu.Game/Screens/IOsuScreen.cs +++ b/osu.Game/Screens/IOsuScreen.cs @@ -60,7 +60,7 @@ namespace osu.Game.Screens /// /// Whether mod track adjustments are allowed to be applied. - /// Null means to inherit from the parent screen. + /// A value means that the parent screen's value of this setting will be used. /// bool? AllowTrackAdjustments { set; get; } From 9b101ea9eb6576d7d579150ce90eb887ff61f6e8 Mon Sep 17 00:00:00 2001 From: AbstractQbit <38468635+AbstractQbit@users.noreply.github.com> Date: Wed, 15 Sep 2021 11:40:23 +0300 Subject: [PATCH 06/15] Add a test for `AllowTrackAdjustments` --- .../Menus/TestSceneMusicActionHandling.cs | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs b/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs index 9037338e23..50226ae2e2 100644 --- a/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs +++ b/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs @@ -9,6 +9,7 @@ using osu.Game.Beatmaps; using osu.Game.Database; using osu.Game.Input.Bindings; using osu.Game.Overlays; +using osu.Game.Screens; using osu.Game.Tests.Resources; namespace osu.Game.Tests.Visual.Menus @@ -79,5 +80,55 @@ namespace osu.Game.Tests.Visual.Menus trackChangeQueue.Count == 1 && trackChangeQueue.Dequeue().changeDirection == TrackChangeDirection.Next); } + + [Test] + public void TestAllowTrackAdjustments() + { + AddStep("push allowing screen", () => Game.ScreenStack.Push(new AllowScreen())); + AddAssert("allows adjustments", () => Game.MusicController.AllowTrackAdjustments); + + AddStep("push inheriting screen", () => Game.ScreenStack.Push(new InheritScreen())); + AddAssert("allows adjustments", () => Game.MusicController.AllowTrackAdjustments); + + AddStep("push disallowing screen", () => Game.ScreenStack.Push(new DisallowScreen())); + AddAssert("disallows adjustments", () => !Game.MusicController.AllowTrackAdjustments); + + AddStep("push inheriting screen", () => Game.ScreenStack.Push(new InheritScreen())); + AddAssert("disallows adjustments", () => !Game.MusicController.AllowTrackAdjustments); + + AddStep("push inheriting screen", () => Game.ScreenStack.Push(new InheritScreen())); + AddAssert("disallows adjustments", () => !Game.MusicController.AllowTrackAdjustments); + + AddStep("push allowing screen", () => Game.ScreenStack.Push(new AllowScreen())); + AddAssert("allows adjustments", () => Game.MusicController.AllowTrackAdjustments); + + // Now start exiting from screens + AddStep("exit screen", () => Game.ScreenStack.Exit()); + AddAssert("disallows adjustments", () => !Game.MusicController.AllowTrackAdjustments); + + AddStep("exit screen", () => Game.ScreenStack.Exit()); + AddAssert("disallows adjustments", () => !Game.MusicController.AllowTrackAdjustments); + + AddStep("exit screen", () => Game.ScreenStack.Exit()); + AddAssert("disallows adjustments", () => !Game.MusicController.AllowTrackAdjustments); + + AddStep("exit screen", () => Game.ScreenStack.Exit()); + AddAssert("allows adjustments", () => Game.MusicController.AllowTrackAdjustments); + + AddStep("exit screen", () => Game.ScreenStack.Exit()); + AddAssert("allows adjustments", () => Game.MusicController.AllowTrackAdjustments); + } + + private class AllowScreen : OsuScreen + { + public override bool? AllowTrackAdjustments => true; + } + + private class DisallowScreen : OsuScreen + { + public override bool? AllowTrackAdjustments => false; + } + + private class InheritScreen : OsuScreen { } } } From 1181317c729455c2e81b409933c37167e84d55df Mon Sep 17 00:00:00 2001 From: AbstractQbit <38468635+AbstractQbit@users.noreply.github.com> Date: Wed, 15 Sep 2021 12:01:56 +0300 Subject: [PATCH 07/15] Fix issues found by code quality ci --- .../Visual/Menus/TestSceneMusicActionHandling.cs | 4 +++- osu.Game/Screens/IOsuScreen.cs | 2 +- osu.Game/Screens/OsuScreen.cs | 8 +------- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs b/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs index 50226ae2e2..79acaedb73 100644 --- a/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs +++ b/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs @@ -129,6 +129,8 @@ namespace osu.Game.Tests.Visual.Menus public override bool? AllowTrackAdjustments => false; } - private class InheritScreen : OsuScreen { } + private class InheritScreen : OsuScreen + { + } } } diff --git a/osu.Game/Screens/IOsuScreen.cs b/osu.Game/Screens/IOsuScreen.cs index 735853e462..fd884586d1 100644 --- a/osu.Game/Screens/IOsuScreen.cs +++ b/osu.Game/Screens/IOsuScreen.cs @@ -62,7 +62,7 @@ namespace osu.Game.Screens /// Whether mod track adjustments are allowed to be applied. /// A value means that the parent screen's value of this setting will be used. /// - bool? AllowTrackAdjustments { set; get; } + bool? AllowTrackAdjustments { get; set; } /// /// Invoked when the back button has been pressed to close any overlays before exiting this . diff --git a/osu.Game/Screens/OsuScreen.cs b/osu.Game/Screens/OsuScreen.cs index 0deaa3e80e..9b4d7f9eda 100644 --- a/osu.Game/Screens/OsuScreen.cs +++ b/osu.Game/Screens/OsuScreen.cs @@ -81,13 +81,7 @@ namespace osu.Game.Screens public virtual float BackgroundParallaxAmount => 1; - private bool? allowTrackAdjustments = null; - - public virtual bool? AllowTrackAdjustments - { - set => allowTrackAdjustments = value; - get => allowTrackAdjustments; - } + public virtual bool? AllowTrackAdjustments { get; set; } public Bindable Beatmap { get; private set; } From f0439ef50b0dd9217e43508217b34c0e47dcba7b Mon Sep 17 00:00:00 2001 From: AbstractQbit <38468635+AbstractQbit@users.noreply.github.com> Date: Wed, 15 Sep 2021 13:12:57 +0300 Subject: [PATCH 08/15] Remove unnecessary `AllowTrackAdjustments` overrides, add true to `SongSelect` --- .../Settings/Sections/Maintenance/DirectorySelectScreen.cs | 2 -- osu.Game/Screens/Import/FileImportScreen.cs | 2 -- osu.Game/Screens/Menu/MainMenu.cs | 2 -- osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs | 2 -- osu.Game/Screens/Play/ScreenWithBeatmapBackground.cs | 2 -- osu.Game/Screens/Select/SongSelect.cs | 2 ++ osu.Game/Screens/StartupScreen.cs | 2 -- 7 files changed, 2 insertions(+), 12 deletions(-) diff --git a/osu.Game/Overlays/Settings/Sections/Maintenance/DirectorySelectScreen.cs b/osu.Game/Overlays/Settings/Sections/Maintenance/DirectorySelectScreen.cs index 6d0e79e2c7..e509cac2f1 100644 --- a/osu.Game/Overlays/Settings/Sections/Maintenance/DirectorySelectScreen.cs +++ b/osu.Game/Overlays/Settings/Sections/Maintenance/DirectorySelectScreen.cs @@ -24,8 +24,6 @@ namespace osu.Game.Overlays.Settings.Sections.Maintenance private OsuDirectorySelector directorySelector; - public override bool? AllowTrackAdjustments => false; - /// /// Text to display in the header to inform the user of what they are selecting. /// diff --git a/osu.Game/Screens/Import/FileImportScreen.cs b/osu.Game/Screens/Import/FileImportScreen.cs index 69fcf31876..7e1d55b3e2 100644 --- a/osu.Game/Screens/Import/FileImportScreen.cs +++ b/osu.Game/Screens/Import/FileImportScreen.cs @@ -23,8 +23,6 @@ namespace osu.Game.Screens.Import { public override bool HideOverlaysOnEnter => true; - public override bool? AllowTrackAdjustments => false; - private OsuFileSelector fileSelector; private Container contentContainer; private TextFlowContainer currentFileText; diff --git a/osu.Game/Screens/Menu/MainMenu.cs b/osu.Game/Screens/Menu/MainMenu.cs index 00885a91c5..221b31f855 100644 --- a/osu.Game/Screens/Menu/MainMenu.cs +++ b/osu.Game/Screens/Menu/MainMenu.cs @@ -36,8 +36,6 @@ namespace osu.Game.Screens.Menu public override bool AllowExternalScreenChange => true; - public override bool? AllowTrackAdjustments => false; - private Screen songSelect; private MenuSideFlashes sideFlashes; diff --git a/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs b/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs index e3945c9cac..fc20b21b60 100644 --- a/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs +++ b/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs @@ -24,8 +24,6 @@ namespace osu.Game.Screens.OnlinePlay [Cached] protected readonly OverlayColourProvider ColourProvider = new OverlayColourProvider(OverlayColourScheme.Plum); - public override bool? AllowTrackAdjustments => false; - public override bool CursorVisible => (screenStack?.CurrentScreen as IOnlinePlaySubScreen)?.CursorVisible ?? true; // this is required due to PlayerLoader eventually being pushed to the main stack diff --git a/osu.Game/Screens/Play/ScreenWithBeatmapBackground.cs b/osu.Game/Screens/Play/ScreenWithBeatmapBackground.cs index 9bec320e22..88dab88d42 100644 --- a/osu.Game/Screens/Play/ScreenWithBeatmapBackground.cs +++ b/osu.Game/Screens/Play/ScreenWithBeatmapBackground.cs @@ -10,8 +10,6 @@ namespace osu.Game.Screens.Play { protected override BackgroundScreen CreateBackground() => new BackgroundScreenBeatmap(Beatmap.Value); - public override bool? AllowTrackAdjustments => true; - public void ApplyToBackground(Action action) => base.ApplyToBackground(b => action.Invoke((BackgroundScreenBeatmap)b)); } } diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index f11f9fd614..1f0f134ba7 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -53,6 +53,8 @@ namespace osu.Game.Screens.Select protected virtual bool DisplayStableImportPrompt => stableImportManager?.SupportsImportFromStable == true; + public override bool? AllowTrackAdjustments => true; + /// /// Can be null if is false. /// diff --git a/osu.Game/Screens/StartupScreen.cs b/osu.Game/Screens/StartupScreen.cs index 7b73d36fdf..be217d6b1f 100644 --- a/osu.Game/Screens/StartupScreen.cs +++ b/osu.Game/Screens/StartupScreen.cs @@ -16,8 +16,6 @@ namespace osu.Game.Screens public override bool CursorVisible => false; - public override bool? AllowTrackAdjustments => false; - protected override OverlayActivation InitialOverlayActivationMode => OverlayActivation.Disabled; } } From 318f0941ca8f53af7bff53cc8ce54226f8a0fbba Mon Sep 17 00:00:00 2001 From: AbstractQbit <38468635+AbstractQbit@users.noreply.github.com> Date: Wed, 15 Sep 2021 21:25:39 +0300 Subject: [PATCH 09/15] Move all the "inherit previous `AllowTrackAdjustments`" logic into `OsuScreen` --- osu.Game/OsuGame.cs | 5 +---- osu.Game/Screens/Edit/Editor.cs | 2 +- osu.Game/Screens/IOsuScreen.cs | 2 +- .../Multiplayer/Spectate/MultiSpectatorScreen.cs | 2 +- osu.Game/Screens/OnlinePlay/OnlinePlaySubScreen.cs | 2 +- osu.Game/Screens/OsuScreen.cs | 7 ++++++- osu.Game/Screens/Play/Player.cs | 2 +- osu.Game/Screens/Select/SongSelect.cs | 2 +- 8 files changed, 13 insertions(+), 11 deletions(-) diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index f2f925a778..2107b3a0e9 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -1075,10 +1075,7 @@ namespace osu.Game OverlayActivationMode.BindTo(newOsuScreen.OverlayActivationMode); API.Activity.BindTo(newOsuScreen.Activity); - if (newOsuScreen.AllowTrackAdjustments.HasValue) - MusicController.AllowTrackAdjustments = newOsuScreen.AllowTrackAdjustments.Value; - else - newOsuScreen.AllowTrackAdjustments = MusicController.AllowTrackAdjustments; + MusicController.AllowTrackAdjustments = newOsuScreen.AllowTrackAdjustments; if (newOsuScreen.HideOverlaysOnEnter) CloseAllOverlays(); diff --git a/osu.Game/Screens/Edit/Editor.cs b/osu.Game/Screens/Edit/Editor.cs index 028662172d..5bb47e1c11 100644 --- a/osu.Game/Screens/Edit/Editor.cs +++ b/osu.Game/Screens/Edit/Editor.cs @@ -56,7 +56,7 @@ namespace osu.Game.Screens.Edit public override bool DisallowExternalBeatmapRulesetChanges => true; - public override bool? AllowTrackAdjustments => false; + public override bool AllowTrackAdjustments => false; protected bool HasUnsavedChanges => lastSavedHash != changeHandler.CurrentStateHash; diff --git a/osu.Game/Screens/IOsuScreen.cs b/osu.Game/Screens/IOsuScreen.cs index fd884586d1..b12baf233f 100644 --- a/osu.Game/Screens/IOsuScreen.cs +++ b/osu.Game/Screens/IOsuScreen.cs @@ -62,7 +62,7 @@ namespace osu.Game.Screens /// Whether mod track adjustments are allowed to be applied. /// A value means that the parent screen's value of this setting will be used. /// - bool? AllowTrackAdjustments { get; set; } + bool AllowTrackAdjustments { get; } /// /// Invoked when the back button has been pressed to close any overlays before exiting this . diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs index c45e3a79da..bf7c738882 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs @@ -26,7 +26,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 AllowTrackAdjustments => false; /// /// Whether all spectating players have finished loading. diff --git a/osu.Game/Screens/OnlinePlay/OnlinePlaySubScreen.cs b/osu.Game/Screens/OnlinePlay/OnlinePlaySubScreen.cs index 8c4f0c1394..054009a228 100644 --- a/osu.Game/Screens/OnlinePlay/OnlinePlaySubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/OnlinePlaySubScreen.cs @@ -11,7 +11,7 @@ namespace osu.Game.Screens.OnlinePlay { public override bool DisallowExternalBeatmapRulesetChanges => false; - public override bool? AllowTrackAdjustments => true; + public override bool AllowTrackAdjustments => true; public virtual string ShortTitle => Title; diff --git a/osu.Game/Screens/OsuScreen.cs b/osu.Game/Screens/OsuScreen.cs index 9b4d7f9eda..01dc703b66 100644 --- a/osu.Game/Screens/OsuScreen.cs +++ b/osu.Game/Screens/OsuScreen.cs @@ -81,7 +81,12 @@ namespace osu.Game.Screens public virtual float BackgroundParallaxAmount => 1; - public virtual bool? AllowTrackAdjustments { get; set; } + [Resolved] + private MusicController musicController { get; set; } + + private bool? allowTrackAdjustments; + + public virtual bool AllowTrackAdjustments => allowTrackAdjustments ??= (musicController?.AllowTrackAdjustments ?? false); public Bindable Beatmap { get; private set; } diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 9927467bd6..e8a2790c94 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -56,7 +56,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 AllowTrackAdjustments => false; private readonly IBindable gameActive = new Bindable(true); diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 1f0f134ba7..9b6cbad7d1 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -53,7 +53,7 @@ namespace osu.Game.Screens.Select protected virtual bool DisplayStableImportPrompt => stableImportManager?.SupportsImportFromStable == true; - public override bool? AllowTrackAdjustments => true; + public override bool AllowTrackAdjustments => true; /// /// Can be null if is false. From 30c458c662e403bcadaf25a8ac975242989ba35a Mon Sep 17 00:00:00 2001 From: AbstractQbit <38468635+AbstractQbit@users.noreply.github.com> Date: Wed, 15 Sep 2021 21:34:41 +0300 Subject: [PATCH 10/15] Oops, fix not compiling test --- osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs b/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs index 79acaedb73..8f6ab5e755 100644 --- a/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs +++ b/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs @@ -121,12 +121,12 @@ namespace osu.Game.Tests.Visual.Menus private class AllowScreen : OsuScreen { - public override bool? AllowTrackAdjustments => true; + public override bool AllowTrackAdjustments => true; } private class DisallowScreen : OsuScreen { - public override bool? AllowTrackAdjustments => false; + public override bool AllowTrackAdjustments => false; } private class InheritScreen : OsuScreen From 3cd3e133ce2a98205988c480cd44c4e61a5fae59 Mon Sep 17 00:00:00 2001 From: AbstractQbit <38468635+AbstractQbit@users.noreply.github.com> Date: Thu, 16 Sep 2021 01:21:29 +0300 Subject: [PATCH 11/15] Move `AllowTrackAdjustments` test to `TestSceneOsuScreenStack` --- .../Menus/TestSceneMusicActionHandling.cs | 53 +----------- .../Visual/TestSceneOsuScreenStack.cs | 83 ++++++++++++++++++- 2 files changed, 80 insertions(+), 56 deletions(-) diff --git a/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs b/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs index 8f6ab5e755..bf6491cd81 100644 --- a/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs +++ b/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs @@ -4,6 +4,7 @@ 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.Database; @@ -80,57 +81,5 @@ namespace osu.Game.Tests.Visual.Menus trackChangeQueue.Count == 1 && trackChangeQueue.Dequeue().changeDirection == TrackChangeDirection.Next); } - - [Test] - public void TestAllowTrackAdjustments() - { - AddStep("push allowing screen", () => Game.ScreenStack.Push(new AllowScreen())); - AddAssert("allows adjustments", () => Game.MusicController.AllowTrackAdjustments); - - AddStep("push inheriting screen", () => Game.ScreenStack.Push(new InheritScreen())); - AddAssert("allows adjustments", () => Game.MusicController.AllowTrackAdjustments); - - AddStep("push disallowing screen", () => Game.ScreenStack.Push(new DisallowScreen())); - AddAssert("disallows adjustments", () => !Game.MusicController.AllowTrackAdjustments); - - AddStep("push inheriting screen", () => Game.ScreenStack.Push(new InheritScreen())); - AddAssert("disallows adjustments", () => !Game.MusicController.AllowTrackAdjustments); - - AddStep("push inheriting screen", () => Game.ScreenStack.Push(new InheritScreen())); - AddAssert("disallows adjustments", () => !Game.MusicController.AllowTrackAdjustments); - - AddStep("push allowing screen", () => Game.ScreenStack.Push(new AllowScreen())); - AddAssert("allows adjustments", () => Game.MusicController.AllowTrackAdjustments); - - // Now start exiting from screens - AddStep("exit screen", () => Game.ScreenStack.Exit()); - AddAssert("disallows adjustments", () => !Game.MusicController.AllowTrackAdjustments); - - AddStep("exit screen", () => Game.ScreenStack.Exit()); - AddAssert("disallows adjustments", () => !Game.MusicController.AllowTrackAdjustments); - - AddStep("exit screen", () => Game.ScreenStack.Exit()); - AddAssert("disallows adjustments", () => !Game.MusicController.AllowTrackAdjustments); - - AddStep("exit screen", () => Game.ScreenStack.Exit()); - AddAssert("allows adjustments", () => Game.MusicController.AllowTrackAdjustments); - - AddStep("exit screen", () => Game.ScreenStack.Exit()); - AddAssert("allows adjustments", () => Game.MusicController.AllowTrackAdjustments); - } - - private class AllowScreen : OsuScreen - { - public override bool AllowTrackAdjustments => true; - } - - private class DisallowScreen : OsuScreen - { - public override bool AllowTrackAdjustments => false; - } - - private class InheritScreen : OsuScreen - { - } } } diff --git a/osu.Game.Tests/Visual/TestSceneOsuScreenStack.cs b/osu.Game.Tests/Visual/TestSceneOsuScreenStack.cs index c55988d1bb..ed3935e101 100644 --- a/osu.Game.Tests/Visual/TestSceneOsuScreenStack.cs +++ b/osu.Game.Tests/Visual/TestSceneOsuScreenStack.cs @@ -5,8 +5,8 @@ using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Screens; -using osu.Framework.Testing; using osu.Game.Graphics.Sprites; +using osu.Game.Overlays; using osu.Game.Screens; using osu.Game.Screens.Play; using osuTK.Graphics; @@ -18,10 +18,20 @@ namespace osu.Game.Tests.Visual { private TestOsuScreenStack stack; - [SetUpSteps] - public void SetUpSteps() + [Cached] + private MusicController musicController = new MusicController(); + + [BackgroundDependencyLoader] + private void load() { - AddStep("Create new screen stack", () => { Child = stack = new TestOsuScreenStack { RelativeSizeAxes = Axes.Both }; }); + stack = new TestOsuScreenStack { RelativeSizeAxes = Axes.Both }; + stack.ScreenPushed += screenChanged; + stack.ScreenExited += screenChanged; + + Add(musicController); + Add(stack); + + LoadComponent(stack); } [Test] @@ -42,6 +52,44 @@ namespace osu.Game.Tests.Visual AddAssert("Parallax is off", () => stack.ParallaxAmount == 0); } + [Test] + public void AllowTrackAdjustmentsTest() + { + AddStep("push allowing screen", () => stack.Push(loadNewScreen())); + AddAssert("allows adjustments 1", () => musicController.AllowTrackAdjustments); + + AddStep("push inheriting screen", () => stack.Push(loadNewScreen())); + AddAssert("allows adjustments 2", () => musicController.AllowTrackAdjustments); + + AddStep("push disallowing screen", () => stack.Push(loadNewScreen())); + AddAssert("disallows adjustments 3", () => !musicController.AllowTrackAdjustments); + + AddStep("push inheriting screen", () => stack.Push(loadNewScreen())); + AddAssert("disallows adjustments 4", () => !musicController.AllowTrackAdjustments); + + AddStep("push inheriting screen", () => stack.Push(loadNewScreen())); + AddAssert("disallows adjustments 5", () => !musicController.AllowTrackAdjustments); + + AddStep("push allowing screen", () => stack.Push(loadNewScreen())); + AddAssert("allows adjustments 6", () => musicController.AllowTrackAdjustments); + + // Now start exiting from screens + AddStep("exit screen", () => stack.Exit()); + AddAssert("disallows adjustments 7", () => !musicController.AllowTrackAdjustments); + + AddStep("exit screen", () => stack.Exit()); + AddAssert("disallows adjustments 8", () => !musicController.AllowTrackAdjustments); + + AddStep("exit screen", () => stack.Exit()); + AddAssert("disallows adjustments 9", () => !musicController.AllowTrackAdjustments); + + AddStep("exit screen", () => stack.Exit()); + AddAssert("allows adjustments 10", () => musicController.AllowTrackAdjustments); + + AddStep("exit screen", () => stack.Exit()); + AddAssert("allows adjustments 11", () => musicController.AllowTrackAdjustments); + } + public class TestScreen : ScreenWithBeatmapBackground { private readonly string screenText; @@ -78,5 +126,32 @@ namespace osu.Game.Tests.Visual { public new float ParallaxAmount => base.ParallaxAmount; } + + private class AllowScreen : OsuScreen + { + public override bool AllowTrackAdjustments => true; + } + + public class DisallowScreen : OsuScreen + { + public override bool AllowTrackAdjustments => false; + } + + private class InheritScreen : OsuScreen + { + } + + private OsuScreen loadNewScreen() where T : OsuScreen, new() + { + OsuScreen screen = new T(); + LoadComponent(screen); + return screen; + } + + private void screenChanged(IScreen current, IScreen newScreen) + { + if (newScreen is IOsuScreen newOsuScreen) + musicController.AllowTrackAdjustments = newOsuScreen.AllowTrackAdjustments; + } } } From 9057be1a02069917e065a94290685933dd9cf73e Mon Sep 17 00:00:00 2001 From: AbstractQbit <38468635+AbstractQbit@users.noreply.github.com> Date: Thu, 16 Sep 2021 01:30:53 +0300 Subject: [PATCH 12/15] Remove unused usings --- osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs b/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs index bf6491cd81..9037338e23 100644 --- a/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs +++ b/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs @@ -4,13 +4,11 @@ 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.Database; using osu.Game.Input.Bindings; using osu.Game.Overlays; -using osu.Game.Screens; using osu.Game.Tests.Resources; namespace osu.Game.Tests.Visual.Menus From fa693bb8a83ff8ec32c59e31f2a31211af76b8ea Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 16 Sep 2021 16:08:09 +0900 Subject: [PATCH 13/15] Move `MusicController` adjustment set to inside `OsuScreen` itself (and result `nullable`) --- .../Visual/TestSceneOsuScreenStack.cs | 12 ++--------- osu.Game/OsuGame.cs | 2 -- osu.Game/Screens/Edit/Editor.cs | 2 +- osu.Game/Screens/IOsuScreen.cs | 4 ++-- .../Spectate/MultiSpectatorScreen.cs | 2 +- .../Screens/OnlinePlay/OnlinePlaySubScreen.cs | 2 +- osu.Game/Screens/OsuScreen.cs | 20 +++++++++++++------ osu.Game/Screens/Play/Player.cs | 2 +- osu.Game/Screens/Select/SongSelect.cs | 2 +- 9 files changed, 23 insertions(+), 25 deletions(-) diff --git a/osu.Game.Tests/Visual/TestSceneOsuScreenStack.cs b/osu.Game.Tests/Visual/TestSceneOsuScreenStack.cs index ed3935e101..7729ad0ff3 100644 --- a/osu.Game.Tests/Visual/TestSceneOsuScreenStack.cs +++ b/osu.Game.Tests/Visual/TestSceneOsuScreenStack.cs @@ -25,8 +25,6 @@ namespace osu.Game.Tests.Visual private void load() { stack = new TestOsuScreenStack { RelativeSizeAxes = Axes.Both }; - stack.ScreenPushed += screenChanged; - stack.ScreenExited += screenChanged; Add(musicController); Add(stack); @@ -129,12 +127,12 @@ namespace osu.Game.Tests.Visual private class AllowScreen : OsuScreen { - public override bool AllowTrackAdjustments => true; + public override bool? AllowTrackAdjustments => true; } public class DisallowScreen : OsuScreen { - public override bool AllowTrackAdjustments => false; + public override bool? AllowTrackAdjustments => false; } private class InheritScreen : OsuScreen @@ -147,11 +145,5 @@ namespace osu.Game.Tests.Visual LoadComponent(screen); return screen; } - - private void screenChanged(IScreen current, IScreen newScreen) - { - if (newScreen is IOsuScreen newOsuScreen) - musicController.AllowTrackAdjustments = newOsuScreen.AllowTrackAdjustments; - } } } diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 2107b3a0e9..ce84c4bd2a 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -1075,8 +1075,6 @@ namespace osu.Game OverlayActivationMode.BindTo(newOsuScreen.OverlayActivationMode); API.Activity.BindTo(newOsuScreen.Activity); - MusicController.AllowTrackAdjustments = newOsuScreen.AllowTrackAdjustments; - if (newOsuScreen.HideOverlaysOnEnter) CloseAllOverlays(); else diff --git a/osu.Game/Screens/Edit/Editor.cs b/osu.Game/Screens/Edit/Editor.cs index 5bb47e1c11..028662172d 100644 --- a/osu.Game/Screens/Edit/Editor.cs +++ b/osu.Game/Screens/Edit/Editor.cs @@ -56,7 +56,7 @@ namespace osu.Game.Screens.Edit public override bool DisallowExternalBeatmapRulesetChanges => true; - public override bool AllowTrackAdjustments => false; + public override bool? AllowTrackAdjustments => false; protected bool HasUnsavedChanges => lastSavedHash != changeHandler.CurrentStateHash; diff --git a/osu.Game/Screens/IOsuScreen.cs b/osu.Game/Screens/IOsuScreen.cs index b12baf233f..910a0c7d61 100644 --- a/osu.Game/Screens/IOsuScreen.cs +++ b/osu.Game/Screens/IOsuScreen.cs @@ -59,10 +59,10 @@ namespace osu.Game.Screens Bindable Ruleset { get; } /// - /// Whether mod track adjustments are allowed to be applied. + /// Whether mod track adjustments should be applied on entering this screen. /// A value means that the parent screen's value of this setting will be used. /// - bool AllowTrackAdjustments { get; } + bool? AllowTrackAdjustments { get; } /// /// Invoked when the back button has been pressed to close any overlays before exiting this . diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs index bf7c738882..c45e3a79da 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs @@ -26,7 +26,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? AllowTrackAdjustments => false; /// /// Whether all spectating players have finished loading. diff --git a/osu.Game/Screens/OnlinePlay/OnlinePlaySubScreen.cs b/osu.Game/Screens/OnlinePlay/OnlinePlaySubScreen.cs index 054009a228..8c4f0c1394 100644 --- a/osu.Game/Screens/OnlinePlay/OnlinePlaySubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/OnlinePlaySubScreen.cs @@ -11,7 +11,7 @@ namespace osu.Game.Screens.OnlinePlay { public override bool DisallowExternalBeatmapRulesetChanges => false; - public override bool AllowTrackAdjustments => true; + public override bool? AllowTrackAdjustments => true; public virtual string ShortTitle => Title; diff --git a/osu.Game/Screens/OsuScreen.cs b/osu.Game/Screens/OsuScreen.cs index 01dc703b66..d3981b715c 100644 --- a/osu.Game/Screens/OsuScreen.cs +++ b/osu.Game/Screens/OsuScreen.cs @@ -11,11 +11,11 @@ using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Screens; using osu.Game.Beatmaps; -using osu.Game.Rulesets; -using osu.Game.Screens.Menu; using osu.Game.Overlays; -using osu.Game.Users; +using osu.Game.Rulesets; using osu.Game.Rulesets.Mods; +using osu.Game.Screens.Menu; +using osu.Game.Users; namespace osu.Game.Screens { @@ -84,9 +84,7 @@ namespace osu.Game.Screens [Resolved] private MusicController musicController { get; set; } - private bool? allowTrackAdjustments; - - public virtual bool AllowTrackAdjustments => allowTrackAdjustments ??= (musicController?.AllowTrackAdjustments ?? false); + public virtual bool? AllowTrackAdjustments => null; public Bindable Beatmap { get; private set; } @@ -96,6 +94,8 @@ namespace osu.Game.Screens private OsuScreenDependencies screenDependencies; + private bool trackAdjustmentStateAtSuspend; + internal void CreateLeasedDependencies(IReadOnlyDependencyContainer dependencies) => createDependencies(dependencies); protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) @@ -175,8 +175,11 @@ namespace osu.Game.Screens { if (PlayResumeSound) sampleExit?.Play(); + applyArrivingDefaults(true); + musicController.AllowTrackAdjustments = trackAdjustmentStateAtSuspend; + base.OnResuming(last); } @@ -184,6 +187,8 @@ namespace osu.Game.Screens { base.OnSuspending(next); + trackAdjustmentStateAtSuspend = musicController.AllowTrackAdjustments; + onSuspendingLogo(); } @@ -191,6 +196,9 @@ namespace osu.Game.Screens { applyArrivingDefaults(false); + if (AllowTrackAdjustments != null) + musicController.AllowTrackAdjustments = AllowTrackAdjustments.Value; + if (backgroundStack?.Push(ownedBackground = CreateBackground()) != true) { // If the constructed instance was not actually pushed to the background stack, we don't want to track it unnecessarily. diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index e8a2790c94..9927467bd6 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -56,7 +56,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? AllowTrackAdjustments => false; private readonly IBindable gameActive = new Bindable(true); diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 9b6cbad7d1..1f0f134ba7 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -53,7 +53,7 @@ namespace osu.Game.Screens.Select protected virtual bool DisplayStableImportPrompt => stableImportManager?.SupportsImportFromStable == true; - public override bool AllowTrackAdjustments => true; + public override bool? AllowTrackAdjustments => true; /// /// Can be null if is false. From b58415fe198b571dd00640f8689fe22f304bcebd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 16 Sep 2021 16:12:14 +0900 Subject: [PATCH 14/15] Make suspend stored state nullable to ensure we don't break it --- osu.Game/Screens/OsuScreen.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/OsuScreen.cs b/osu.Game/Screens/OsuScreen.cs index d3981b715c..f425144c6b 100644 --- a/osu.Game/Screens/OsuScreen.cs +++ b/osu.Game/Screens/OsuScreen.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using JetBrains.Annotations; using osu.Framework.Allocation; using osu.Framework.Audio; @@ -94,7 +95,7 @@ namespace osu.Game.Screens private OsuScreenDependencies screenDependencies; - private bool trackAdjustmentStateAtSuspend; + private bool? trackAdjustmentStateAtSuspend; internal void CreateLeasedDependencies(IReadOnlyDependencyContainer dependencies) => createDependencies(dependencies); @@ -178,7 +179,9 @@ namespace osu.Game.Screens applyArrivingDefaults(true); - musicController.AllowTrackAdjustments = trackAdjustmentStateAtSuspend; + Debug.Assert(trackAdjustmentStateAtSuspend != null); + + musicController.AllowTrackAdjustments = trackAdjustmentStateAtSuspend.Value; base.OnResuming(last); } From 3495fae5191eaca5fb0842873830071e3c65b9ce Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 16 Sep 2021 16:31:41 +0900 Subject: [PATCH 15/15] Handle potential for `OnResuming` call without an `OnSuspending` first --- osu.Game/Screens/OsuScreen.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/OsuScreen.cs b/osu.Game/Screens/OsuScreen.cs index f425144c6b..ccc891d3bf 100644 --- a/osu.Game/Screens/OsuScreen.cs +++ b/osu.Game/Screens/OsuScreen.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using JetBrains.Annotations; using osu.Framework.Allocation; using osu.Framework.Audio; @@ -179,9 +178,10 @@ namespace osu.Game.Screens applyArrivingDefaults(true); - Debug.Assert(trackAdjustmentStateAtSuspend != null); - - musicController.AllowTrackAdjustments = trackAdjustmentStateAtSuspend.Value; + // 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) + musicController.AllowTrackAdjustments = trackAdjustmentStateAtSuspend.Value; base.OnResuming(last); }