From 2202863e1a1caf323aa341dec967fdaaefd4a237 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Apr 2022 17:49:28 +0900 Subject: [PATCH 1/6] Split out `IPerformFromScreenRunner` to allow for easier testing --- osu.Game/OsuGame.cs | 8 +----- .../Dashboard/CurrentlyPlayingDisplay.cs | 5 ++-- .../Sections/DebugSettings/GeneralSettings.cs | 5 ++-- .../StableDirectoryLocationDialog.cs | 7 ++--- osu.Game/Screens/IPerformFromScreenRunner.cs | 26 +++++++++++++++++++ osu.Game/Screens/Menu/MainMenu.cs | 4 +-- .../Skinning/Editor/SkinEditorSceneLibrary.cs | 7 ++--- 7 files changed, 43 insertions(+), 19 deletions(-) create mode 100644 osu.Game/Screens/IPerformFromScreenRunner.cs diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 73121f6e7d..76ccbe0a08 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -63,7 +63,7 @@ namespace osu.Game /// The full osu! experience. Builds on top of to add menus and binding logic /// for initial components that are generally retrieved via DI. /// - public class OsuGame : OsuGameBase, IKeyBindingHandler, ILocalUserPlayInfo + public class OsuGame : OsuGameBase, IKeyBindingHandler, ILocalUserPlayInfo, IPerformFromScreenRunner { /// /// The amount of global offset to apply when a left/right anchored overlay is displayed (ie. settings or notifications). @@ -586,12 +586,6 @@ namespace osu.Game private PerformFromMenuRunner performFromMainMenuTask; - /// - /// Perform an action only after returning to a specific screen as indicated by . - /// Eagerly tries to exit the current screen until it succeeds. - /// - /// The action to perform once we are in the correct state. - /// An optional collection of valid screen types. If any of these screens are already current we can perform the action immediately, else the first valid parent will be made current before performing the action. is used if not specified. public void PerformFromScreen(Action action, IEnumerable validScreens = null) { performFromMainMenuTask?.Cancel(); diff --git a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs index 117de88166..786401b7a8 100644 --- a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs +++ b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs @@ -14,6 +14,7 @@ using osu.Game.Database; using osu.Game.Online.API; using osu.Game.Online.API.Requests.Responses; using osu.Game.Online.Spectator; +using osu.Game.Screens; using osu.Game.Screens.OnlinePlay.Match.Components; using osu.Game.Screens.Play; using osu.Game.Users; @@ -106,7 +107,7 @@ namespace osu.Game.Overlays.Dashboard public readonly APIUser User; [Resolved(canBeNull: true)] - private OsuGame game { get; set; } + private IPerformFromScreenRunner performer { get; set; } public PlayingUserPanel(APIUser user) { @@ -140,7 +141,7 @@ namespace osu.Game.Overlays.Dashboard Text = "Watch", Anchor = Anchor.TopCentre, Origin = Anchor.TopCentre, - Action = () => game?.PerformFromScreen(s => s.Push(new SoloSpectator(User))), + Action = () => performer?.PerformFromScreen(s => s.Push(new SoloSpectator(User))), Enabled = { Value = User.Id != api.LocalUser.Value.Id } } } diff --git a/osu.Game/Overlays/Settings/Sections/DebugSettings/GeneralSettings.cs b/osu.Game/Overlays/Settings/Sections/DebugSettings/GeneralSettings.cs index 60540a089e..8833420523 100644 --- a/osu.Game/Overlays/Settings/Sections/DebugSettings/GeneralSettings.cs +++ b/osu.Game/Overlays/Settings/Sections/DebugSettings/GeneralSettings.cs @@ -7,6 +7,7 @@ using osu.Framework.Graphics; using osu.Framework.Localisation; using osu.Framework.Screens; using osu.Game.Localisation; +using osu.Game.Screens; using osu.Game.Screens.Import; namespace osu.Game.Overlays.Settings.Sections.DebugSettings @@ -16,7 +17,7 @@ namespace osu.Game.Overlays.Settings.Sections.DebugSettings protected override LocalisableString Header => DebugSettingsStrings.GeneralHeader; [BackgroundDependencyLoader(true)] - private void load(FrameworkDebugConfigManager config, FrameworkConfigManager frameworkConfig, OsuGame game) + private void load(FrameworkDebugConfigManager config, FrameworkConfigManager frameworkConfig, IPerformFromScreenRunner performer) { Children = new Drawable[] { @@ -34,7 +35,7 @@ namespace osu.Game.Overlays.Settings.Sections.DebugSettings Add(new SettingsButton { Text = DebugSettingsStrings.ImportFiles, - Action = () => game?.PerformFromScreen(menu => menu.Push(new FileImportScreen())) + Action = () => performer?.PerformFromScreen(menu => menu.Push(new FileImportScreen())) }); } } diff --git a/osu.Game/Overlays/Settings/Sections/Maintenance/StableDirectoryLocationDialog.cs b/osu.Game/Overlays/Settings/Sections/Maintenance/StableDirectoryLocationDialog.cs index 904c9deaae..4e81cd63b8 100644 --- a/osu.Game/Overlays/Settings/Sections/Maintenance/StableDirectoryLocationDialog.cs +++ b/osu.Game/Overlays/Settings/Sections/Maintenance/StableDirectoryLocationDialog.cs @@ -6,13 +6,14 @@ using osu.Framework.Allocation; using osu.Framework.Graphics.Sprites; using osu.Framework.Screens; using osu.Game.Overlays.Dialog; +using osu.Game.Screens; namespace osu.Game.Overlays.Settings.Sections.Maintenance { public class StableDirectoryLocationDialog : PopupDialog { - [Resolved] - private OsuGame game { get; set; } + [Resolved(canBeNull: true)] + private IPerformFromScreenRunner performer { get; set; } public StableDirectoryLocationDialog(TaskCompletionSource taskCompletionSource) { @@ -25,7 +26,7 @@ namespace osu.Game.Overlays.Settings.Sections.Maintenance new PopupDialogOkButton { Text = "Sure! I know where it is located!", - Action = () => Schedule(() => game.PerformFromScreen(screen => screen.Push(new StableDirectorySelectScreen(taskCompletionSource)))) + Action = () => Schedule(() => performer.PerformFromScreen(screen => screen.Push(new StableDirectorySelectScreen(taskCompletionSource)))) }, new PopupDialogCancelButton { diff --git a/osu.Game/Screens/IPerformFromScreenRunner.cs b/osu.Game/Screens/IPerformFromScreenRunner.cs new file mode 100644 index 0000000000..655bebdeb0 --- /dev/null +++ b/osu.Game/Screens/IPerformFromScreenRunner.cs @@ -0,0 +1,26 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using System.Collections.Generic; +using osu.Framework.Allocation; +using osu.Framework.Screens; +using osu.Game.Screens.Menu; + +namespace osu.Game.Screens +{ + /// + /// Manages a global screen stack to allow nested components a guarantee of where work is executed. + /// + [Cached] + public interface IPerformFromScreenRunner + { + /// + /// Perform an action only after returning to a specific screen as indicated by . + /// Eagerly tries to exit the current screen until it succeeds. + /// + /// The action to perform once we are in the correct state. + /// An optional collection of valid screen types. If any of these screens are already current we can perform the action immediately, else the first valid parent will be made current before performing the action. is used if not specified. + void PerformFromScreen(Action action, IEnumerable validScreens = null); + } +} diff --git a/osu.Game/Screens/Menu/MainMenu.cs b/osu.Game/Screens/Menu/MainMenu.cs index e2d79b4015..442a536cf9 100644 --- a/osu.Game/Screens/Menu/MainMenu.cs +++ b/osu.Game/Screens/Menu/MainMenu.cs @@ -148,14 +148,14 @@ namespace osu.Game.Screens.Menu } [Resolved(canBeNull: true)] - private OsuGame game { get; set; } + private IPerformFromScreenRunner performer { get; set; } private void confirmAndExit() { if (exitConfirmed) return; exitConfirmed = true; - game?.PerformFromScreen(menu => menu.Exit()); + performer?.PerformFromScreen(menu => menu.Exit()); } private void preloadSongSelect() diff --git a/osu.Game/Skinning/Editor/SkinEditorSceneLibrary.cs b/osu.Game/Skinning/Editor/SkinEditorSceneLibrary.cs index 0808cd157f..4507526806 100644 --- a/osu.Game/Skinning/Editor/SkinEditorSceneLibrary.cs +++ b/osu.Game/Skinning/Editor/SkinEditorSceneLibrary.cs @@ -15,6 +15,7 @@ using osu.Game.Graphics.UserInterface; using osu.Game.Overlays; using osu.Game.Rulesets; using osu.Game.Rulesets.Mods; +using osu.Game.Screens; using osu.Game.Screens.Play; using osu.Game.Screens.Select; using osuTK; @@ -28,7 +29,7 @@ namespace osu.Game.Skinning.Editor private const float padding = 10; [Resolved(canBeNull: true)] - private OsuGame game { get; set; } + private IPerformFromScreenRunner performer { get; set; } [Resolved] private IBindable ruleset { get; set; } @@ -75,7 +76,7 @@ namespace osu.Game.Skinning.Editor Text = "Song Select", Anchor = Anchor.CentreLeft, Origin = Anchor.CentreLeft, - Action = () => game?.PerformFromScreen(screen => + Action = () => performer?.PerformFromScreen(screen => { if (screen is SongSelect) return; @@ -88,7 +89,7 @@ namespace osu.Game.Skinning.Editor Text = "Gameplay", Anchor = Anchor.CentreLeft, Origin = Anchor.CentreLeft, - Action = () => game?.PerformFromScreen(screen => + Action = () => performer?.PerformFromScreen(screen => { if (screen is Player) return; From aaf5577e6a035606f19e3fd249b7a4f464f35a13 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Apr 2022 21:48:46 +0900 Subject: [PATCH 2/6] Remove unnecessary `canBeNull` specification --- .../Sections/Maintenance/StableDirectoryLocationDialog.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Settings/Sections/Maintenance/StableDirectoryLocationDialog.cs b/osu.Game/Overlays/Settings/Sections/Maintenance/StableDirectoryLocationDialog.cs index 4e81cd63b8..b16fd9a5a1 100644 --- a/osu.Game/Overlays/Settings/Sections/Maintenance/StableDirectoryLocationDialog.cs +++ b/osu.Game/Overlays/Settings/Sections/Maintenance/StableDirectoryLocationDialog.cs @@ -12,7 +12,7 @@ namespace osu.Game.Overlays.Settings.Sections.Maintenance { public class StableDirectoryLocationDialog : PopupDialog { - [Resolved(canBeNull: true)] + [Resolved] private IPerformFromScreenRunner performer { get; set; } public StableDirectoryLocationDialog(TaskCompletionSource taskCompletionSource) From 5d5e46ede71bd05ac9846da73eb03119f96cad7b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Apr 2022 21:47:47 +0900 Subject: [PATCH 3/6] Fix rider incompetency --- osu.Game/Overlays/IDialogOverlay.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Overlays/IDialogOverlay.cs b/osu.Game/Overlays/IDialogOverlay.cs index 07f28a7204..dedc164510 100644 --- a/osu.Game/Overlays/IDialogOverlay.cs +++ b/osu.Game/Overlays/IDialogOverlay.cs @@ -1,7 +1,8 @@ -#nullable enable // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +#nullable enable + using osu.Framework.Allocation; using osu.Game.Overlays.Dialog; From 3cbc6cd29767a7d47ba624dc1680c22f4a02cedd Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 18 Apr 2022 21:04:22 +0300 Subject: [PATCH 4/6] Update further tests to cache using `IDialogOverlay` --- osu.Game.Tests/Visual/Online/TestSceneChatLink.cs | 2 +- osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs | 2 +- osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapLeaderboard.cs | 2 +- .../Visual/SongSelect/TestSceneUserTopScoreContainer.cs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatLink.cs b/osu.Game.Tests/Visual/Online/TestSceneChatLink.cs index d077868175..6818147da4 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatLink.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatLink.cs @@ -49,7 +49,7 @@ namespace osu.Game.Tests.Visual.Online Dependencies.Cache(chatManager); Dependencies.Cache(new ChatOverlay()); - Dependencies.Cache(dialogOverlay); + Dependencies.CacheAs(dialogOverlay); } [SetUp] diff --git a/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs b/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs index f9c9b2a68b..2106043544 100644 --- a/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs +++ b/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs @@ -97,7 +97,7 @@ namespace osu.Game.Tests.Visual.Settings Depth = -1 }); - Dependencies.Cache(dialogOverlay); + Dependencies.CacheAs(dialogOverlay); } } } diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapLeaderboard.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapLeaderboard.cs index 1ed6648131..3b15ee9c45 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapLeaderboard.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapLeaderboard.cs @@ -31,7 +31,7 @@ namespace osu.Game.Tests.Visual.SongSelect { private readonly FailableLeaderboard leaderboard; - [Cached] + [Cached(typeof(IDialogOverlay))] private readonly DialogOverlay dialogOverlay; private ScoreManager scoreManager; diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneUserTopScoreContainer.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneUserTopScoreContainer.cs index dd7f9951bf..c71e54e9a8 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneUserTopScoreContainer.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneUserTopScoreContainer.cs @@ -19,7 +19,7 @@ namespace osu.Game.Tests.Visual.SongSelect { public class TestSceneUserTopScoreContainer : OsuTestScene { - [Cached] + [Cached(typeof(IDialogOverlay))] private readonly DialogOverlay dialogOverlay; public TestSceneUserTopScoreContainer() From 16a0efc5621ba8a751f874bb8fef91218766efa2 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 18 Apr 2022 21:06:27 +0300 Subject: [PATCH 5/6] Cache `IDialogOverlay` as its type in interface --- osu.Game/Overlays/IDialogOverlay.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/IDialogOverlay.cs b/osu.Game/Overlays/IDialogOverlay.cs index dedc164510..1c6a84cd64 100644 --- a/osu.Game/Overlays/IDialogOverlay.cs +++ b/osu.Game/Overlays/IDialogOverlay.cs @@ -11,7 +11,7 @@ namespace osu.Game.Overlays /// /// A global overlay that can show popup dialogs. /// - [Cached] + [Cached(typeof(IDialogOverlay))] public interface IDialogOverlay { /// From 1fc28552b54400ca6e49f40ad5e40f4c88d3bd6e Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 18 Apr 2022 21:07:20 +0300 Subject: [PATCH 6/6] Minor reword inline comment --- osu.Game/Overlays/Dialog/PopupDialog.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Dialog/PopupDialog.cs b/osu.Game/Overlays/Dialog/PopupDialog.cs index 9fd1a32e84..d08b6b7beb 100644 --- a/osu.Game/Overlays/Dialog/PopupDialog.cs +++ b/osu.Game/Overlays/Dialog/PopupDialog.cs @@ -216,7 +216,7 @@ namespace osu.Game.Overlays.Dialog }; // It's important we start in a visible state so our state fires on hide, even before load. - // This is used by the IDialogOverlay to know when the dialog was dismissed. + // This is used by the dialog overlay to know when the dialog was dismissed. Show(); }