From ad3007eaadf62916e9af4616e08e2942d0756b44 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Oct 2024 17:00:32 +0900 Subject: [PATCH 01/17] Adjust `ILocalUserPlayInfo` to expose whether gameplay is in a paused/break state --- osu.Desktop/Windows/GameplayWinKeyBlocker.cs | 6 ++-- .../BackgroundDataStoreProcessorTests.cs | 10 +++--- .../Input/ConfineMouseTrackerTest.cs | 15 +++++++- .../Gameplay/TestSceneOverlayActivation.cs | 4 +-- .../TestSceneGameplayChatDisplay.cs | 6 ++-- .../Database/BackgroundDataStoreProcessor.cs | 2 +- osu.Game/Input/ConfineMouseTracker.cs | 8 ++--- osu.Game/Input/OsuUserInputManager.cs | 5 +-- osu.Game/OsuGame.cs | 36 +++++++++---------- osu.Game/Overlays/BeatmapListingOverlay.cs | 2 +- osu.Game/Overlays/Chat/DaySeparator.cs | 4 +-- osu.Game/Overlays/Settings/SettingsSidebar.cs | 2 +- .../Multiplayer/GameplayChatDisplay.cs | 8 ++--- osu.Game/Screens/Play/ILocalUserPlayInfo.cs | 4 +-- .../Screens/Play/LocalUserPlayingStates.cs | 23 ++++++++++++ osu.Game/Screens/Play/Player.cs | 19 ++++++---- 16 files changed, 97 insertions(+), 57 deletions(-) create mode 100644 osu.Game/Screens/Play/LocalUserPlayingStates.cs diff --git a/osu.Desktop/Windows/GameplayWinKeyBlocker.cs b/osu.Desktop/Windows/GameplayWinKeyBlocker.cs index 560f6fdd7f2b..268dacc07743 100644 --- a/osu.Desktop/Windows/GameplayWinKeyBlocker.cs +++ b/osu.Desktop/Windows/GameplayWinKeyBlocker.cs @@ -13,7 +13,7 @@ namespace osu.Desktop.Windows public partial class GameplayWinKeyBlocker : Component { private Bindable disableWinKey = null!; - private IBindable localUserPlaying = null!; + private IBindable localUserPlaying = null!; private IBindable isActive = null!; [Resolved] @@ -22,7 +22,7 @@ public partial class GameplayWinKeyBlocker : Component [BackgroundDependencyLoader] private void load(ILocalUserPlayInfo localUserInfo, OsuConfigManager config) { - localUserPlaying = localUserInfo.IsPlaying.GetBoundCopy(); + localUserPlaying = localUserInfo.PlayingState.GetBoundCopy(); localUserPlaying.BindValueChanged(_ => updateBlocking()); isActive = host.IsActive.GetBoundCopy(); @@ -34,7 +34,7 @@ private void load(ILocalUserPlayInfo localUserInfo, OsuConfigManager config) private void updateBlocking() { - bool shouldDisable = isActive.Value && disableWinKey.Value && localUserPlaying.Value; + bool shouldDisable = isActive.Value && disableWinKey.Value && localUserPlaying.Value == LocalUserPlayingStates.Playing; if (shouldDisable) host.InputThread.Scheduler.Add(WindowsKey.Disable); diff --git a/osu.Game.Tests/Database/BackgroundDataStoreProcessorTests.cs b/osu.Game.Tests/Database/BackgroundDataStoreProcessorTests.cs index f9f9fa2622f0..21193b51615c 100644 --- a/osu.Game.Tests/Database/BackgroundDataStoreProcessorTests.cs +++ b/osu.Game.Tests/Database/BackgroundDataStoreProcessorTests.cs @@ -22,9 +22,9 @@ namespace osu.Game.Tests.Database [HeadlessTest] public partial class BackgroundDataStoreProcessorTests : OsuTestScene, ILocalUserPlayInfo { - public IBindable IsPlaying => isPlaying; + public IBindable PlayingState => isPlaying; - private readonly Bindable isPlaying = new Bindable(); + private readonly Bindable isPlaying = new Bindable(); private BeatmapSetInfo importedSet = null!; @@ -37,7 +37,7 @@ private void load(OsuGameBase osu) [SetUpSteps] public void SetUpSteps() { - AddStep("Set not playing", () => isPlaying.Value = false); + AddStep("Set not playing", () => isPlaying.Value = LocalUserPlayingStates.NotPlaying); } [Test] @@ -89,7 +89,7 @@ public void TestDifficultyProcessingWhilePlaying() }); }); - AddStep("Set playing", () => isPlaying.Value = true); + AddStep("Set playing", () => isPlaying.Value = LocalUserPlayingStates.Playing); AddStep("Reset difficulty", () => { @@ -117,7 +117,7 @@ public void TestDifficultyProcessingWhilePlaying() }); }); - AddStep("Set not playing", () => isPlaying.Value = false); + AddStep("Set not playing", () => isPlaying.Value = LocalUserPlayingStates.NotPlaying); AddUntilStep("wait for difficulties repopulated", () => { diff --git a/osu.Game.Tests/Input/ConfineMouseTrackerTest.cs b/osu.Game.Tests/Input/ConfineMouseTrackerTest.cs index 6b43ab83c5a9..c4fdb2b427ef 100644 --- a/osu.Game.Tests/Input/ConfineMouseTrackerTest.cs +++ b/osu.Game.Tests/Input/ConfineMouseTrackerTest.cs @@ -3,11 +3,13 @@ using NUnit.Framework; using osu.Framework.Allocation; +using osu.Framework.Bindables; using osu.Framework.Configuration; using osu.Framework.Input; using osu.Framework.Testing; using osu.Game.Configuration; using osu.Game.Input; +using osu.Game.Screens.Play; using osu.Game.Tests.Visual; namespace osu.Game.Tests.Input @@ -15,9 +17,20 @@ namespace osu.Game.Tests.Input [HeadlessTest] public partial class ConfineMouseTrackerTest : OsuGameTestScene { + private readonly Bindable playingState = new Bindable(); + [Resolved] private FrameworkConfigManager frameworkConfigManager { get; set; } = null!; + [SetUpSteps] + public override void SetUpSteps() + { + base.SetUpSteps(); + + // a bit dodgy. + AddStep("bind playing state", () => ((IBindable)playingState).BindTo(((ILocalUserPlayInfo)Game).PlayingState)); + } + [TestCase(WindowMode.Windowed)] [TestCase(WindowMode.Borderless)] public void TestDisableConfining(WindowMode windowMode) @@ -88,7 +101,7 @@ private void setGameSideModeTo(OsuConfineMouseMode mode) => AddStep($"set {mode} game-side", () => Game.LocalConfig.SetValue(OsuSetting.ConfineMouseMode, mode)); private void setLocalUserPlayingTo(bool playing) - => AddStep($"local user {(playing ? "playing" : "not playing")}", () => Game.LocalUserPlaying.Value = playing); + => AddStep($"local user {(playing ? "playing" : "not playing")}", () => playingState.Value = playing ? LocalUserPlayingStates.Playing : LocalUserPlayingStates.NotPlaying); private void gameSideModeIs(OsuConfineMouseMode mode) => AddAssert($"mode is {mode} game-side", () => Game.LocalConfig.Get(OsuSetting.ConfineMouseMode) == mode); diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneOverlayActivation.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneOverlayActivation.cs index 269d104fa303..751405b1d667 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneOverlayActivation.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneOverlayActivation.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System.Linq; using NUnit.Framework; using osu.Game.Overlays; @@ -12,7 +10,7 @@ namespace osu.Game.Tests.Visual.Gameplay { public partial class TestSceneOverlayActivation : OsuPlayerTestScene { - protected new OverlayTestPlayer Player => base.Player as OverlayTestPlayer; + protected new OverlayTestPlayer Player => (OverlayTestPlayer)base.Player; public override void SetUpSteps() { diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneGameplayChatDisplay.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneGameplayChatDisplay.cs index d1a914300f82..ca3d484115ef 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneGameplayChatDisplay.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneGameplayChatDisplay.cs @@ -25,14 +25,14 @@ public partial class TestSceneGameplayChatDisplay : OsuManualInputManagerTestSce [Cached(typeof(ILocalUserPlayInfo))] private ILocalUserPlayInfo localUserInfo; - private readonly Bindable localUserPlaying = new Bindable(); + private readonly Bindable playingState = new Bindable(); private TextBox textBox => chatDisplay.ChildrenOfType().First(); public TestSceneGameplayChatDisplay() { var mockLocalUserInfo = new Mock(); - mockLocalUserInfo.SetupGet(i => i.IsPlaying).Returns(localUserPlaying); + mockLocalUserInfo.SetupGet(i => i.PlayingState).Returns(playingState); localUserInfo = mockLocalUserInfo.Object; } @@ -124,6 +124,6 @@ private void assertChatFocused(bool isFocused) => AddAssert($"chat {(isFocused ? "focused" : "not focused")}", () => textBox.HasFocus == isFocused); private void setLocalUserPlaying(bool playing) => - AddStep($"local user {(playing ? "playing" : "not playing")}", () => localUserPlaying.Value = playing); + AddStep($"local user {(playing ? "playing" : "not playing")}", () => playingState.Value = playing ? LocalUserPlayingStates.Playing : LocalUserPlayingStates.NotPlaying); } } diff --git a/osu.Game/Database/BackgroundDataStoreProcessor.cs b/osu.Game/Database/BackgroundDataStoreProcessor.cs index 0fa785e49425..cc7746e34b5c 100644 --- a/osu.Game/Database/BackgroundDataStoreProcessor.cs +++ b/osu.Game/Database/BackgroundDataStoreProcessor.cs @@ -606,7 +606,7 @@ private void sleepIfRequired() { // Importantly, also sleep if high performance session is active. // If we don't do this, memory usage can become runaway due to GC running in a more lenient mode. - while (localUserPlayInfo?.IsPlaying.Value == true || highPerformanceSessionManager?.IsSessionActive == true) + while (localUserPlayInfo?.PlayingState.Value != LocalUserPlayingStates.NotPlaying || highPerformanceSessionManager?.IsSessionActive == true) { Logger.Log("Background processing sleeping due to active gameplay..."); Thread.Sleep(TimeToSleepDuringGameplay); diff --git a/osu.Game/Input/ConfineMouseTracker.cs b/osu.Game/Input/ConfineMouseTracker.cs index 926f68df45d5..0f4363e00f53 100644 --- a/osu.Game/Input/ConfineMouseTracker.cs +++ b/osu.Game/Input/ConfineMouseTracker.cs @@ -15,7 +15,7 @@ namespace osu.Game.Input { /// /// Connects with . - /// If is true, we should also confine the mouse cursor if it has been + /// If is playing, we should also confine the mouse cursor if it has been /// requested with . /// public partial class ConfineMouseTracker : Component @@ -25,7 +25,7 @@ public partial class ConfineMouseTracker : Component private Bindable frameworkMinimiseOnFocusLossInFullscreen; private Bindable osuConfineMode; - private IBindable localUserPlaying; + private IBindable localUserPlaying; [BackgroundDependencyLoader] private void load(ILocalUserPlayInfo localUserInfo, FrameworkConfigManager frameworkConfigManager, OsuConfigManager osuConfigManager) @@ -37,7 +37,7 @@ private void load(ILocalUserPlayInfo localUserInfo, FrameworkConfigManager frame frameworkMinimiseOnFocusLossInFullscreen.BindValueChanged(_ => updateConfineMode()); osuConfineMode = osuConfigManager.GetBindable(OsuSetting.ConfineMouseMode); - localUserPlaying = localUserInfo.IsPlaying.GetBoundCopy(); + localUserPlaying = localUserInfo.PlayingState.GetBoundCopy(); osuConfineMode.ValueChanged += _ => updateConfineMode(); localUserPlaying.BindValueChanged(_ => updateConfineMode(), true); @@ -63,7 +63,7 @@ private void updateConfineMode() break; case OsuConfineMouseMode.DuringGameplay: - frameworkConfineMode.Value = localUserPlaying.Value ? ConfineMouseMode.Always : ConfineMouseMode.Never; + frameworkConfineMode.Value = localUserPlaying.Value == LocalUserPlayingStates.Playing ? ConfineMouseMode.Always : ConfineMouseMode.Never; break; case OsuConfineMouseMode.Always: diff --git a/osu.Game/Input/OsuUserInputManager.cs b/osu.Game/Input/OsuUserInputManager.cs index b8fd0bb11c67..ff1b88b65afa 100644 --- a/osu.Game/Input/OsuUserInputManager.cs +++ b/osu.Game/Input/OsuUserInputManager.cs @@ -3,15 +3,16 @@ using osu.Framework.Bindables; using osu.Framework.Input; +using osu.Game.Screens.Play; using osuTK.Input; namespace osu.Game.Input { public partial class OsuUserInputManager : UserInputManager { - protected override bool AllowRightClickFromLongTouch => !LocalUserPlaying.Value; + protected override bool AllowRightClickFromLongTouch => PlayingState.Value == LocalUserPlayingStates.NotPlaying; - public readonly BindableBool LocalUserPlaying = new BindableBool(); + public readonly Bindable PlayingState = new Bindable(); internal OsuUserInputManager() { diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 44ba78762add..902a4ddb33a8 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -175,14 +175,9 @@ public partial class OsuGame : OsuGameBase, IKeyBindingHandler, IL /// public readonly IBindable OverlayActivationMode = new Bindable(); - /// - /// Whether the local user is currently interacting with the game in a way that should not be interrupted. - /// - /// - /// This is exclusively managed by . If other components are mutating this state, a more - /// resilient method should be used to ensure correct state. - /// - public Bindable LocalUserPlaying = new BindableBool(); + IBindable ILocalUserPlayInfo.PlayingState => playingState; + + private readonly Bindable playingState = new Bindable(); protected OsuScreenStack ScreenStack; @@ -302,7 +297,7 @@ public void CloseAllOverlays(bool hideToolbar = true) protected override UserInputManager CreateUserInputManager() { var userInputManager = base.CreateUserInputManager(); - (userInputManager as OsuUserInputManager)?.LocalUserPlaying.BindTo(LocalUserPlaying); + (userInputManager as OsuUserInputManager)?.PlayingState.BindTo(playingState); return userInputManager; } @@ -391,11 +386,11 @@ private void load() // Transfer any runtime changes back to configuration file. SkinManager.CurrentSkinInfo.ValueChanged += skin => configSkin.Value = skin.NewValue.ID.ToString(); - LocalUserPlaying.BindValueChanged(p => + playingState.BindValueChanged(p => { - BeatmapManager.PauseImports = p.NewValue; - SkinManager.PauseImports = p.NewValue; - ScoreManager.PauseImports = p.NewValue; + BeatmapManager.PauseImports = p.NewValue != LocalUserPlayingStates.NotPlaying; + SkinManager.PauseImports = p.NewValue != LocalUserPlayingStates.NotPlaying; + ScoreManager.PauseImports = p.NewValue != LocalUserPlayingStates.NotPlaying; }, true); IsActive.BindValueChanged(active => updateActiveState(active.NewValue), true); @@ -1553,6 +1548,12 @@ private void screenChanged(IScreen current, IScreen newScreen) scope.SetTag(@"screen", newScreen?.GetType().ReadableName() ?? @"none"); }); + // reset on screen change for sanity. + playingState.Value = LocalUserPlayingStates.NotPlaying; + + if (current is Player oldPlayer) + oldPlayer.PlayingState.UnbindFrom(playingState); + switch (newScreen) { case IntroScreen intro: @@ -1565,14 +1566,15 @@ private void screenChanged(IScreen current, IScreen newScreen) versionManager?.Show(); break; + case Player player: + player.PlayingState.BindTo(playingState); + break; + default: versionManager?.Hide(); break; } - // reset on screen change for sanity. - LocalUserPlaying.Value = false; - if (current is IOsuScreen currentOsuScreen) { OverlayActivationMode.UnbindFrom(currentOsuScreen.OverlayActivationMode); @@ -1621,7 +1623,5 @@ private void screenExited(IScreen lastScreen, IScreen newScreen) if (newScreen == null) Exit(); } - - IBindable ILocalUserPlayInfo.IsPlaying => LocalUserPlaying; } } diff --git a/osu.Game/Overlays/BeatmapListingOverlay.cs b/osu.Game/Overlays/BeatmapListingOverlay.cs index 9b2f26e8ae21..b47e2b82c035 100644 --- a/osu.Game/Overlays/BeatmapListingOverlay.cs +++ b/osu.Game/Overlays/BeatmapListingOverlay.cs @@ -98,7 +98,7 @@ protected override void LoadComplete() apiUser.BindValueChanged(_ => Schedule(() => { if (api.IsLoggedIn) - replaceResultsAreaContent(Drawable.Empty()); + replaceResultsAreaContent(Empty()); })); } diff --git a/osu.Game/Overlays/Chat/DaySeparator.cs b/osu.Game/Overlays/Chat/DaySeparator.cs index fd6b15c77858..c371877fcb7f 100644 --- a/osu.Game/Overlays/Chat/DaySeparator.cs +++ b/osu.Game/Overlays/Chat/DaySeparator.cs @@ -75,7 +75,7 @@ private void load() Height = LineHeight, Colour = colourProvider?.Background5 ?? Colour4.White, }, - Drawable.Empty(), + Empty(), new OsuSpriteText { Anchor = Anchor.CentreRight, @@ -87,7 +87,7 @@ private void load() } }, }, - Drawable.Empty(), + Empty(), new Circle { Anchor = Anchor.Centre, diff --git a/osu.Game/Overlays/Settings/SettingsSidebar.cs b/osu.Game/Overlays/Settings/SettingsSidebar.cs index ddbcd60ef695..d24c0a778ccc 100644 --- a/osu.Game/Overlays/Settings/SettingsSidebar.cs +++ b/osu.Game/Overlays/Settings/SettingsSidebar.cs @@ -66,7 +66,7 @@ public BackButton() [BackgroundDependencyLoader] private void load() { - Size = new Vector2(SettingsSidebar.EXPANDED_WIDTH); + Size = new Vector2(EXPANDED_WIDTH); Padding = new MarginPadding(40); diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/GameplayChatDisplay.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/GameplayChatDisplay.cs index d1a73457e384..165e3a2440e3 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/GameplayChatDisplay.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/GameplayChatDisplay.cs @@ -23,9 +23,9 @@ public partial class GameplayChatDisplay : MatchChatDisplay, IKeyBindingHandler< [CanBeNull] private ILocalUserPlayInfo localUserInfo { get; set; } - private readonly IBindable localUserPlaying = new Bindable(); + private readonly IBindable localUserPlaying = new Bindable(); - public override bool PropagatePositionalInputSubTree => !localUserPlaying.Value; + public override bool PropagatePositionalInputSubTree => localUserPlaying.Value != LocalUserPlayingStates.Playing; public Bindable Expanded = new Bindable(); @@ -58,7 +58,7 @@ protected override void LoadComplete() base.LoadComplete(); if (localUserInfo != null) - localUserPlaying.BindTo(localUserInfo.IsPlaying); + localUserPlaying.BindTo(localUserInfo.PlayingState); localUserPlaying.BindValueChanged(playing => { @@ -67,7 +67,7 @@ protected override void LoadComplete() TextBox.HoldFocus = false; // only hold focus (after sending a message) during breaks - TextBox.ReleaseFocusOnCommit = playing.NewValue; + TextBox.ReleaseFocusOnCommit = playing.NewValue == LocalUserPlayingStates.Playing; }, true); Expanded.BindValueChanged(_ => updateExpandedState(), true); diff --git a/osu.Game/Screens/Play/ILocalUserPlayInfo.cs b/osu.Game/Screens/Play/ILocalUserPlayInfo.cs index 2d181a09d42a..48b610dd8e97 100644 --- a/osu.Game/Screens/Play/ILocalUserPlayInfo.cs +++ b/osu.Game/Screens/Play/ILocalUserPlayInfo.cs @@ -10,8 +10,8 @@ namespace osu.Game.Screens.Play public interface ILocalUserPlayInfo { /// - /// Whether the local user is currently playing. + /// Whether the local user is currently interacting (playing) with the game in a way that should not be interrupted. /// - IBindable IsPlaying { get; } + IBindable PlayingState { get; } } } diff --git a/osu.Game/Screens/Play/LocalUserPlayingStates.cs b/osu.Game/Screens/Play/LocalUserPlayingStates.cs new file mode 100644 index 000000000000..89f9ea16f790 --- /dev/null +++ b/osu.Game/Screens/Play/LocalUserPlayingStates.cs @@ -0,0 +1,23 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +namespace osu.Game.Screens.Play +{ + public enum LocalUserPlayingStates + { + /// + /// The local player is not current in gameplay. + /// + NotPlaying, + + /// + /// The local player is in a break, paused, or failed or passed but still at the gameplay screen. + /// + Break, + + /// + /// The local user is in active gameplay. + /// + Playing, + } +} diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 2a66c3d5d303..ee11512bd468 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -94,6 +94,7 @@ public abstract partial class Player : ScreenWithBeatmapBackground, ISamplePlayb public IBindable LocalUserPlaying => localUserPlaying; private readonly Bindable localUserPlaying = new Bindable(); + private readonly Bindable playingState = new Bindable(); public int RestartCount; @@ -231,9 +232,6 @@ private void load(OsuConfigManager config, OsuGameBase game, CancellationToken c if (game != null) gameActive.BindTo(game.IsActive); - if (game is OsuGame osuGame) - LocalUserPlaying.BindTo(osuGame.LocalUserPlaying); - DrawableRuleset = ruleset.CreateDrawableRulesetWith(playableBeatmap, gameplayMods); dependencies.CacheAs(DrawableRuleset); @@ -510,9 +508,16 @@ private void onBreakTimeChanged(ValueChangedEvent isBreakTime) private void updateGameplayState() { - bool inGameplay = !DrawableRuleset.HasReplayLoaded.Value && !DrawableRuleset.IsPaused.Value && !breakTracker.IsBreakTime.Value && !GameplayState.HasFailed; - OverlayActivationMode.Value = inGameplay ? OverlayActivation.Disabled : OverlayActivation.UserTriggered; - localUserPlaying.Value = inGameplay; + bool inGameplay = !DrawableRuleset.HasReplayLoaded.Value && !GameplayState.HasFailed; + bool inBreak = breakTracker.IsBreakTime.Value || DrawableRuleset.IsPaused.Value; + + if (inGameplay) + playingState.Value = inBreak ? LocalUserPlayingStates.Break : LocalUserPlayingStates.Playing; + else + playingState.Value = LocalUserPlayingStates.NotPlaying; + + localUserPlaying.Value = playingState.Value != LocalUserPlayingStates.NotPlaying; + OverlayActivationMode.Value = playingState.Value != LocalUserPlayingStates.NotPlaying ? OverlayActivation.Disabled : OverlayActivation.UserTriggered; } private void updateSampleDisabledState() @@ -1279,6 +1284,6 @@ private void fadeOut(bool instant = false) IBindable ISamplePlaybackDisabler.SamplePlaybackDisabled => samplePlaybackDisabled; - IBindable ILocalUserPlayInfo.IsPlaying => LocalUserPlaying; + public IBindable PlayingState => playingState; } } From 3d54f4a5ab61c41b42d7bffc879b216161f1a634 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Oct 2024 17:41:59 +0900 Subject: [PATCH 02/17] Make states better defined --- osu.Game/Screens/Play/LocalUserPlayingStates.cs | 4 ++-- osu.Game/Screens/Play/Player.cs | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/osu.Game/Screens/Play/LocalUserPlayingStates.cs b/osu.Game/Screens/Play/LocalUserPlayingStates.cs index 89f9ea16f790..d1761692a72f 100644 --- a/osu.Game/Screens/Play/LocalUserPlayingStates.cs +++ b/osu.Game/Screens/Play/LocalUserPlayingStates.cs @@ -6,12 +6,12 @@ namespace osu.Game.Screens.Play public enum LocalUserPlayingStates { /// - /// The local player is not current in gameplay. + /// The local player is not current in gameplay. If watching a replay, gameplay always remains in this state. /// NotPlaying, /// - /// The local player is in a break, paused, or failed or passed but still at the gameplay screen. + /// The local player is in a break, paused, or failed but still at the gameplay screen. /// Break, diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index ee11512bd468..0e0f3ae8a3f4 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -508,16 +508,16 @@ private void onBreakTimeChanged(ValueChangedEvent isBreakTime) private void updateGameplayState() { - bool inGameplay = !DrawableRuleset.HasReplayLoaded.Value && !GameplayState.HasFailed; - bool inBreak = breakTracker.IsBreakTime.Value || DrawableRuleset.IsPaused.Value; + bool inGameplay = !DrawableRuleset.HasReplayLoaded.Value; + bool inBreak = breakTracker.IsBreakTime.Value || DrawableRuleset.IsPaused.Value || GameplayState.HasFailed; if (inGameplay) playingState.Value = inBreak ? LocalUserPlayingStates.Break : LocalUserPlayingStates.Playing; else playingState.Value = LocalUserPlayingStates.NotPlaying; - localUserPlaying.Value = playingState.Value != LocalUserPlayingStates.NotPlaying; - OverlayActivationMode.Value = playingState.Value != LocalUserPlayingStates.NotPlaying ? OverlayActivation.Disabled : OverlayActivation.UserTriggered; + localUserPlaying.Value = playingState.Value == LocalUserPlayingStates.Playing; + OverlayActivationMode.Value = playingState.Value == LocalUserPlayingStates.Playing ? OverlayActivation.Disabled : OverlayActivation.UserTriggered; } private void updateSampleDisabledState() From 4b1c2c09ee3051c173e0b3942e0f0abc6399c2ff Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Oct 2024 17:53:25 +0900 Subject: [PATCH 03/17] Avoid updates and update notifications appearing in more gameplay cases --- osu.Desktop/Updater/VelopackUpdateManager.cs | 32 +++++++++++++++----- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/osu.Desktop/Updater/VelopackUpdateManager.cs b/osu.Desktop/Updater/VelopackUpdateManager.cs index 7a79284533cb..dd0014a61f2f 100644 --- a/osu.Desktop/Updater/VelopackUpdateManager.cs +++ b/osu.Desktop/Updater/VelopackUpdateManager.cs @@ -25,6 +25,8 @@ public partial class VelopackUpdateManager : Game.Updater.UpdateManager [Resolved] private ILocalUserPlayInfo? localUserInfo { get; set; } + private bool isInGameplay => localUserInfo?.PlayingState.Value != LocalUserPlayingStates.NotPlaying; + private UpdateInfo? pendingUpdate; public VelopackUpdateManager() @@ -51,7 +53,7 @@ private async Task checkForUpdateAsync(UpdateProgressNotification? notific try { // Avoid any kind of update checking while gameplay is running. - if (localUserInfo?.IsPlaying.Value == true) + if (isInGameplay) { scheduleRecheck = true; return false; @@ -61,14 +63,17 @@ private async Task checkForUpdateAsync(UpdateProgressNotification? notific // Velopack does support this scenario (see https://github.com/ppy/osu/pull/28743#discussion_r1743495975). if (pendingUpdate != null) { - // If there is an update pending restart, show the notification to restart again. - notificationOverlay.Post(new UpdateApplicationCompleteNotification + runOutsideOfGameplay(() => { - Activated = () => + // If there is an update pending restart, show the notification to restart again. + notificationOverlay.Post(new UpdateApplicationCompleteNotification { - Task.Run(restartToApplyUpdate); - return true; - } + Activated = () => + { + Task.Run(restartToApplyUpdate); + return true; + } + }); }); return true; @@ -104,7 +109,7 @@ private async Task checkForUpdateAsync(UpdateProgressNotification? notific { await updateManager.DownloadUpdatesAsync(pendingUpdate, p => notification.Progress = p / 100f).ConfigureAwait(false); - notification.State = ProgressNotificationState.Completed; + runOutsideOfGameplay(() => notification.State = ProgressNotificationState.Completed); } catch (Exception e) { @@ -131,6 +136,17 @@ private async Task checkForUpdateAsync(UpdateProgressNotification? notific return true; } + private void runOutsideOfGameplay(Action action) + { + if (isInGameplay) + { + Scheduler.AddDelayed(() => runOutsideOfGameplay(action), 1000); + return; + } + + action(); + } + private async Task restartToApplyUpdate() { await updateManager.WaitExitThenApplyUpdatesAsync(pendingUpdate?.TargetFullRelease).ConfigureAwait(false); From 8f0fedbd2205ce1a886d8c61fcf8693466c9ab84 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Oct 2024 18:21:15 +0900 Subject: [PATCH 04/17] Fix local scores never importing due to new conditionals --- osu.Game/OsuGame.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 902a4ddb33a8..23a1ad2f4aaf 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -390,7 +390,8 @@ private void load() { BeatmapManager.PauseImports = p.NewValue != LocalUserPlayingStates.NotPlaying; SkinManager.PauseImports = p.NewValue != LocalUserPlayingStates.NotPlaying; - ScoreManager.PauseImports = p.NewValue != LocalUserPlayingStates.NotPlaying; + // For scores, we need to allow imports during "Break" state else local user's scores will never be imported. + ScoreManager.PauseImports = p.NewValue == LocalUserPlayingStates.Playing; }, true); IsActive.BindValueChanged(active => updateActiveState(active.NewValue), true); From 7977ce8a0e6265e9a948525d919ab7280c967fec Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Oct 2024 18:25:45 +0900 Subject: [PATCH 05/17] Attempt to fix android class --- osu.Android/GameplayScreenRotationLocker.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/osu.Android/GameplayScreenRotationLocker.cs b/osu.Android/GameplayScreenRotationLocker.cs index e5fc354db7e8..ffd4218ea9f8 100644 --- a/osu.Android/GameplayScreenRotationLocker.cs +++ b/osu.Android/GameplayScreenRotationLocker.cs @@ -6,28 +6,29 @@ using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Game; +using osu.Game.Screens.Play; namespace osu.Android { public partial class GameplayScreenRotationLocker : Component { - private Bindable localUserPlaying = null!; + private IBindable localUserPlaying = null!; [Resolved] private OsuGameActivity gameActivity { get; set; } = null!; [BackgroundDependencyLoader] - private void load(OsuGame game) + private void load(ILocalUserPlayInfo localUserPlayInfo) { - localUserPlaying = game.LocalUserPlaying.GetBoundCopy(); + localUserPlaying = localUserPlayInfo.PlayingState.GetBoundCopy(); localUserPlaying.BindValueChanged(updateLock, true); } - private void updateLock(ValueChangedEvent userPlaying) + private void updateLock(ValueChangedEvent userPlaying) { gameActivity.RunOnUiThread(() => { - gameActivity.RequestedOrientation = userPlaying.NewValue ? ScreenOrientation.Locked : gameActivity.DefaultOrientation; + gameActivity.RequestedOrientation = userPlaying.NewValue != LocalUserPlayingStates.NotPlaying ? ScreenOrientation.Locked : gameActivity.DefaultOrientation; }); } } From 8dba4cf760acd07735a12c3b74da7f7a15e1a34d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Oct 2024 19:55:31 +0900 Subject: [PATCH 06/17] Passing means `NotPlaying` --- osu.Game/OsuGame.cs | 3 +-- osu.Game/Screens/Play/Player.cs | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 23a1ad2f4aaf..902a4ddb33a8 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -390,8 +390,7 @@ private void load() { BeatmapManager.PauseImports = p.NewValue != LocalUserPlayingStates.NotPlaying; SkinManager.PauseImports = p.NewValue != LocalUserPlayingStates.NotPlaying; - // For scores, we need to allow imports during "Break" state else local user's scores will never be imported. - ScoreManager.PauseImports = p.NewValue == LocalUserPlayingStates.Playing; + ScoreManager.PauseImports = p.NewValue != LocalUserPlayingStates.NotPlaying; }, true); IsActive.BindValueChanged(active => updateActiveState(active.NewValue), true); diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 0e0f3ae8a3f4..e9e3757629e9 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -508,7 +508,7 @@ private void onBreakTimeChanged(ValueChangedEvent isBreakTime) private void updateGameplayState() { - bool inGameplay = !DrawableRuleset.HasReplayLoaded.Value; + bool inGameplay = !DrawableRuleset.HasReplayLoaded.Value && !GameplayState.HasPassed; bool inBreak = breakTracker.IsBreakTime.Value || DrawableRuleset.IsPaused.Value || GameplayState.HasFailed; if (inGameplay) From 8773c34fddea196d1c8b21a364587803a9c59fff Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Oct 2024 19:55:46 +0900 Subject: [PATCH 07/17] Rename enum to non-plural now that it won't conflict --- osu.Desktop/Updater/VelopackUpdateManager.cs | 2 +- osu.Desktop/Windows/GameplayWinKeyBlocker.cs | 4 ++-- .../Database/BackgroundDataStoreProcessorTests.cs | 10 +++++----- osu.Game.Tests/Input/ConfineMouseTrackerTest.cs | 6 +++--- .../Multiplayer/TestSceneGameplayChatDisplay.cs | 4 ++-- osu.Game/Database/BackgroundDataStoreProcessor.cs | 2 +- osu.Game/Input/ConfineMouseTracker.cs | 4 ++-- osu.Game/Input/OsuUserInputManager.cs | 4 ++-- osu.Game/OsuGame.cs | 12 ++++++------ .../OnlinePlay/Multiplayer/GameplayChatDisplay.cs | 6 +++--- osu.Game/Screens/Play/ILocalUserPlayInfo.cs | 2 +- ...UserPlayingStates.cs => LocalUserPlayingState.cs} | 2 +- osu.Game/Screens/Play/Player.cs | 12 ++++++------ 13 files changed, 35 insertions(+), 35 deletions(-) rename osu.Game/Screens/Play/{LocalUserPlayingStates.cs => LocalUserPlayingState.cs} (94%) diff --git a/osu.Desktop/Updater/VelopackUpdateManager.cs b/osu.Desktop/Updater/VelopackUpdateManager.cs index dd0014a61f2f..5dda03a3d307 100644 --- a/osu.Desktop/Updater/VelopackUpdateManager.cs +++ b/osu.Desktop/Updater/VelopackUpdateManager.cs @@ -25,7 +25,7 @@ public partial class VelopackUpdateManager : Game.Updater.UpdateManager [Resolved] private ILocalUserPlayInfo? localUserInfo { get; set; } - private bool isInGameplay => localUserInfo?.PlayingState.Value != LocalUserPlayingStates.NotPlaying; + private bool isInGameplay => localUserInfo?.PlayingState.Value != LocalUserPlayingState.NotPlaying; private UpdateInfo? pendingUpdate; diff --git a/osu.Desktop/Windows/GameplayWinKeyBlocker.cs b/osu.Desktop/Windows/GameplayWinKeyBlocker.cs index 268dacc07743..085c198cc171 100644 --- a/osu.Desktop/Windows/GameplayWinKeyBlocker.cs +++ b/osu.Desktop/Windows/GameplayWinKeyBlocker.cs @@ -13,7 +13,7 @@ namespace osu.Desktop.Windows public partial class GameplayWinKeyBlocker : Component { private Bindable disableWinKey = null!; - private IBindable localUserPlaying = null!; + private IBindable localUserPlaying = null!; private IBindable isActive = null!; [Resolved] @@ -34,7 +34,7 @@ private void load(ILocalUserPlayInfo localUserInfo, OsuConfigManager config) private void updateBlocking() { - bool shouldDisable = isActive.Value && disableWinKey.Value && localUserPlaying.Value == LocalUserPlayingStates.Playing; + bool shouldDisable = isActive.Value && disableWinKey.Value && localUserPlaying.Value == LocalUserPlayingState.Playing; if (shouldDisable) host.InputThread.Scheduler.Add(WindowsKey.Disable); diff --git a/osu.Game.Tests/Database/BackgroundDataStoreProcessorTests.cs b/osu.Game.Tests/Database/BackgroundDataStoreProcessorTests.cs index 21193b51615c..c40624a3a0f0 100644 --- a/osu.Game.Tests/Database/BackgroundDataStoreProcessorTests.cs +++ b/osu.Game.Tests/Database/BackgroundDataStoreProcessorTests.cs @@ -22,9 +22,9 @@ namespace osu.Game.Tests.Database [HeadlessTest] public partial class BackgroundDataStoreProcessorTests : OsuTestScene, ILocalUserPlayInfo { - public IBindable PlayingState => isPlaying; + public IBindable PlayingState => isPlaying; - private readonly Bindable isPlaying = new Bindable(); + private readonly Bindable isPlaying = new Bindable(); private BeatmapSetInfo importedSet = null!; @@ -37,7 +37,7 @@ private void load(OsuGameBase osu) [SetUpSteps] public void SetUpSteps() { - AddStep("Set not playing", () => isPlaying.Value = LocalUserPlayingStates.NotPlaying); + AddStep("Set not playing", () => isPlaying.Value = LocalUserPlayingState.NotPlaying); } [Test] @@ -89,7 +89,7 @@ public void TestDifficultyProcessingWhilePlaying() }); }); - AddStep("Set playing", () => isPlaying.Value = LocalUserPlayingStates.Playing); + AddStep("Set playing", () => isPlaying.Value = LocalUserPlayingState.Playing); AddStep("Reset difficulty", () => { @@ -117,7 +117,7 @@ public void TestDifficultyProcessingWhilePlaying() }); }); - AddStep("Set not playing", () => isPlaying.Value = LocalUserPlayingStates.NotPlaying); + AddStep("Set not playing", () => isPlaying.Value = LocalUserPlayingState.NotPlaying); AddUntilStep("wait for difficulties repopulated", () => { diff --git a/osu.Game.Tests/Input/ConfineMouseTrackerTest.cs b/osu.Game.Tests/Input/ConfineMouseTrackerTest.cs index c4fdb2b427ef..42f50efdbff2 100644 --- a/osu.Game.Tests/Input/ConfineMouseTrackerTest.cs +++ b/osu.Game.Tests/Input/ConfineMouseTrackerTest.cs @@ -17,7 +17,7 @@ namespace osu.Game.Tests.Input [HeadlessTest] public partial class ConfineMouseTrackerTest : OsuGameTestScene { - private readonly Bindable playingState = new Bindable(); + private readonly Bindable playingState = new Bindable(); [Resolved] private FrameworkConfigManager frameworkConfigManager { get; set; } = null!; @@ -28,7 +28,7 @@ public override void SetUpSteps() base.SetUpSteps(); // a bit dodgy. - AddStep("bind playing state", () => ((IBindable)playingState).BindTo(((ILocalUserPlayInfo)Game).PlayingState)); + AddStep("bind playing state", () => ((IBindable)playingState).BindTo(((ILocalUserPlayInfo)Game).PlayingState)); } [TestCase(WindowMode.Windowed)] @@ -101,7 +101,7 @@ private void setGameSideModeTo(OsuConfineMouseMode mode) => AddStep($"set {mode} game-side", () => Game.LocalConfig.SetValue(OsuSetting.ConfineMouseMode, mode)); private void setLocalUserPlayingTo(bool playing) - => AddStep($"local user {(playing ? "playing" : "not playing")}", () => playingState.Value = playing ? LocalUserPlayingStates.Playing : LocalUserPlayingStates.NotPlaying); + => AddStep($"local user {(playing ? "playing" : "not playing")}", () => playingState.Value = playing ? LocalUserPlayingState.Playing : LocalUserPlayingState.NotPlaying); private void gameSideModeIs(OsuConfineMouseMode mode) => AddAssert($"mode is {mode} game-side", () => Game.LocalConfig.Get(OsuSetting.ConfineMouseMode) == mode); diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneGameplayChatDisplay.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneGameplayChatDisplay.cs index ca3d484115ef..6a500bbe5558 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneGameplayChatDisplay.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneGameplayChatDisplay.cs @@ -25,7 +25,7 @@ public partial class TestSceneGameplayChatDisplay : OsuManualInputManagerTestSce [Cached(typeof(ILocalUserPlayInfo))] private ILocalUserPlayInfo localUserInfo; - private readonly Bindable playingState = new Bindable(); + private readonly Bindable playingState = new Bindable(); private TextBox textBox => chatDisplay.ChildrenOfType().First(); @@ -124,6 +124,6 @@ private void assertChatFocused(bool isFocused) => AddAssert($"chat {(isFocused ? "focused" : "not focused")}", () => textBox.HasFocus == isFocused); private void setLocalUserPlaying(bool playing) => - AddStep($"local user {(playing ? "playing" : "not playing")}", () => playingState.Value = playing ? LocalUserPlayingStates.Playing : LocalUserPlayingStates.NotPlaying); + AddStep($"local user {(playing ? "playing" : "not playing")}", () => playingState.Value = playing ? LocalUserPlayingState.Playing : LocalUserPlayingState.NotPlaying); } } diff --git a/osu.Game/Database/BackgroundDataStoreProcessor.cs b/osu.Game/Database/BackgroundDataStoreProcessor.cs index cc7746e34b5c..3efd4da3aa58 100644 --- a/osu.Game/Database/BackgroundDataStoreProcessor.cs +++ b/osu.Game/Database/BackgroundDataStoreProcessor.cs @@ -606,7 +606,7 @@ private void sleepIfRequired() { // Importantly, also sleep if high performance session is active. // If we don't do this, memory usage can become runaway due to GC running in a more lenient mode. - while (localUserPlayInfo?.PlayingState.Value != LocalUserPlayingStates.NotPlaying || highPerformanceSessionManager?.IsSessionActive == true) + while (localUserPlayInfo?.PlayingState.Value != LocalUserPlayingState.NotPlaying || highPerformanceSessionManager?.IsSessionActive == true) { Logger.Log("Background processing sleeping due to active gameplay..."); Thread.Sleep(TimeToSleepDuringGameplay); diff --git a/osu.Game/Input/ConfineMouseTracker.cs b/osu.Game/Input/ConfineMouseTracker.cs index 0f4363e00f53..eeda92a585b4 100644 --- a/osu.Game/Input/ConfineMouseTracker.cs +++ b/osu.Game/Input/ConfineMouseTracker.cs @@ -25,7 +25,7 @@ public partial class ConfineMouseTracker : Component private Bindable frameworkMinimiseOnFocusLossInFullscreen; private Bindable osuConfineMode; - private IBindable localUserPlaying; + private IBindable localUserPlaying; [BackgroundDependencyLoader] private void load(ILocalUserPlayInfo localUserInfo, FrameworkConfigManager frameworkConfigManager, OsuConfigManager osuConfigManager) @@ -63,7 +63,7 @@ private void updateConfineMode() break; case OsuConfineMouseMode.DuringGameplay: - frameworkConfineMode.Value = localUserPlaying.Value == LocalUserPlayingStates.Playing ? ConfineMouseMode.Always : ConfineMouseMode.Never; + frameworkConfineMode.Value = localUserPlaying.Value == LocalUserPlayingState.Playing ? ConfineMouseMode.Always : ConfineMouseMode.Never; break; case OsuConfineMouseMode.Always: diff --git a/osu.Game/Input/OsuUserInputManager.cs b/osu.Game/Input/OsuUserInputManager.cs index ff1b88b65afa..26ce2312d5f7 100644 --- a/osu.Game/Input/OsuUserInputManager.cs +++ b/osu.Game/Input/OsuUserInputManager.cs @@ -10,9 +10,9 @@ namespace osu.Game.Input { public partial class OsuUserInputManager : UserInputManager { - protected override bool AllowRightClickFromLongTouch => PlayingState.Value == LocalUserPlayingStates.NotPlaying; + protected override bool AllowRightClickFromLongTouch => PlayingState.Value == LocalUserPlayingState.NotPlaying; - public readonly Bindable PlayingState = new Bindable(); + public readonly Bindable PlayingState = new Bindable(); internal OsuUserInputManager() { diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 902a4ddb33a8..935631c8e9e9 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -175,9 +175,9 @@ public partial class OsuGame : OsuGameBase, IKeyBindingHandler, IL /// public readonly IBindable OverlayActivationMode = new Bindable(); - IBindable ILocalUserPlayInfo.PlayingState => playingState; + IBindable ILocalUserPlayInfo.PlayingState => playingState; - private readonly Bindable playingState = new Bindable(); + private readonly Bindable playingState = new Bindable(); protected OsuScreenStack ScreenStack; @@ -388,9 +388,9 @@ private void load() playingState.BindValueChanged(p => { - BeatmapManager.PauseImports = p.NewValue != LocalUserPlayingStates.NotPlaying; - SkinManager.PauseImports = p.NewValue != LocalUserPlayingStates.NotPlaying; - ScoreManager.PauseImports = p.NewValue != LocalUserPlayingStates.NotPlaying; + BeatmapManager.PauseImports = p.NewValue != LocalUserPlayingState.NotPlaying; + SkinManager.PauseImports = p.NewValue != LocalUserPlayingState.NotPlaying; + ScoreManager.PauseImports = p.NewValue != LocalUserPlayingState.NotPlaying; }, true); IsActive.BindValueChanged(active => updateActiveState(active.NewValue), true); @@ -1549,7 +1549,7 @@ private void screenChanged(IScreen current, IScreen newScreen) }); // reset on screen change for sanity. - playingState.Value = LocalUserPlayingStates.NotPlaying; + playingState.Value = LocalUserPlayingState.NotPlaying; if (current is Player oldPlayer) oldPlayer.PlayingState.UnbindFrom(playingState); diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/GameplayChatDisplay.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/GameplayChatDisplay.cs index 165e3a2440e3..d4483044e0e7 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/GameplayChatDisplay.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/GameplayChatDisplay.cs @@ -23,9 +23,9 @@ public partial class GameplayChatDisplay : MatchChatDisplay, IKeyBindingHandler< [CanBeNull] private ILocalUserPlayInfo localUserInfo { get; set; } - private readonly IBindable localUserPlaying = new Bindable(); + private readonly IBindable localUserPlaying = new Bindable(); - public override bool PropagatePositionalInputSubTree => localUserPlaying.Value != LocalUserPlayingStates.Playing; + public override bool PropagatePositionalInputSubTree => localUserPlaying.Value != LocalUserPlayingState.Playing; public Bindable Expanded = new Bindable(); @@ -67,7 +67,7 @@ protected override void LoadComplete() TextBox.HoldFocus = false; // only hold focus (after sending a message) during breaks - TextBox.ReleaseFocusOnCommit = playing.NewValue == LocalUserPlayingStates.Playing; + TextBox.ReleaseFocusOnCommit = playing.NewValue == LocalUserPlayingState.Playing; }, true); Expanded.BindValueChanged(_ => updateExpandedState(), true); diff --git a/osu.Game/Screens/Play/ILocalUserPlayInfo.cs b/osu.Game/Screens/Play/ILocalUserPlayInfo.cs index 48b610dd8e97..dd24549c558e 100644 --- a/osu.Game/Screens/Play/ILocalUserPlayInfo.cs +++ b/osu.Game/Screens/Play/ILocalUserPlayInfo.cs @@ -12,6 +12,6 @@ public interface ILocalUserPlayInfo /// /// Whether the local user is currently interacting (playing) with the game in a way that should not be interrupted. /// - IBindable PlayingState { get; } + IBindable PlayingState { get; } } } diff --git a/osu.Game/Screens/Play/LocalUserPlayingStates.cs b/osu.Game/Screens/Play/LocalUserPlayingState.cs similarity index 94% rename from osu.Game/Screens/Play/LocalUserPlayingStates.cs rename to osu.Game/Screens/Play/LocalUserPlayingState.cs index d1761692a72f..9ae413029853 100644 --- a/osu.Game/Screens/Play/LocalUserPlayingStates.cs +++ b/osu.Game/Screens/Play/LocalUserPlayingState.cs @@ -3,7 +3,7 @@ namespace osu.Game.Screens.Play { - public enum LocalUserPlayingStates + public enum LocalUserPlayingState { /// /// The local player is not current in gameplay. If watching a replay, gameplay always remains in this state. diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index e9e3757629e9..d6ac279494e1 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -94,7 +94,7 @@ public abstract partial class Player : ScreenWithBeatmapBackground, ISamplePlayb public IBindable LocalUserPlaying => localUserPlaying; private readonly Bindable localUserPlaying = new Bindable(); - private readonly Bindable playingState = new Bindable(); + private readonly Bindable playingState = new Bindable(); public int RestartCount; @@ -512,12 +512,12 @@ private void updateGameplayState() bool inBreak = breakTracker.IsBreakTime.Value || DrawableRuleset.IsPaused.Value || GameplayState.HasFailed; if (inGameplay) - playingState.Value = inBreak ? LocalUserPlayingStates.Break : LocalUserPlayingStates.Playing; + playingState.Value = inBreak ? LocalUserPlayingState.Break : LocalUserPlayingState.Playing; else - playingState.Value = LocalUserPlayingStates.NotPlaying; + playingState.Value = LocalUserPlayingState.NotPlaying; - localUserPlaying.Value = playingState.Value == LocalUserPlayingStates.Playing; - OverlayActivationMode.Value = playingState.Value == LocalUserPlayingStates.Playing ? OverlayActivation.Disabled : OverlayActivation.UserTriggered; + localUserPlaying.Value = playingState.Value == LocalUserPlayingState.Playing; + OverlayActivationMode.Value = playingState.Value == LocalUserPlayingState.Playing ? OverlayActivation.Disabled : OverlayActivation.UserTriggered; } private void updateSampleDisabledState() @@ -1284,6 +1284,6 @@ private void fadeOut(bool instant = false) IBindable ISamplePlaybackDisabler.SamplePlaybackDisabled => samplePlaybackDisabled; - public IBindable PlayingState => playingState; + public IBindable PlayingState => playingState; } } From ecf144f4a5866a29ac6d792c6ed73c0619f3b8bd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 3 Oct 2024 17:11:25 +0900 Subject: [PATCH 08/17] Add failing test of importing failed replay in `OsuGame` flow We had a test covering this but it wasn't within `OsuGame` so didn't have full import blocking coverage (see https://github.com/ppy/osu/blob/cbbe2f9dc04abb052a0d762aa798eba094fdd80c/osu.Game.Tests/Visual/Gameplay/TestScenePlayerLocalScoreImport.cs#L89-L88). --- .../Navigation/TestSceneScreenNavigation.cs | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs index eda7ce925a08..d374ae1bd9f1 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs @@ -364,6 +364,37 @@ public void TestRetryImmediatelyAfterCompletion() AddUntilStep("wait for player", () => Game.ScreenStack.CurrentScreen != getOriginalPlayer() && Game.ScreenStack.CurrentScreen is Player); } + [Test] + public void TestSaveFailedReplay() + { + Player player = null; + + Screens.Select.SongSelect songSelect = null; + PushAndConfirm(() => songSelect = new TestPlaySongSelect()); + AddUntilStep("wait for song select", () => songSelect.BeatmapSetsLoaded); + + AddStep("import beatmap", () => BeatmapImportHelper.LoadOszIntoOsu(Game, virtualTrack: true).WaitSafely()); + + AddUntilStep("wait for selected", () => !Game.Beatmap.IsDefault); + + AddStep("press enter", () => InputManager.Key(Key.Enter)); + + AddUntilStep("wait for player", () => + { + DismissAnyNotifications(); + return (player = Game.ScreenStack.CurrentScreen as Player) != null; + }); + + AddUntilStep("wait for fail", () => player.GameplayState.HasFailed); + + AddUntilStep("fail screen displayed", () => player.ChildrenOfType().First().State.Value == Visibility.Visible); + AddUntilStep("wait for button clickable", () => player.ChildrenOfType().First().ChildrenOfType().First().Enabled.Value); + + AddUntilStep("score not in database", () => Realm.Run(r => r.Find(player.Score.ScoreInfo.ID) == null)); + AddStep("click save button", () => player.ChildrenOfType().First().ChildrenOfType().First().TriggerClick()); + AddUntilStep("score in database", () => Realm.Run(r => r.Find(player.Score.ScoreInfo.ID) != null)); + } + [Test] public void TestExitImmediatelyAfterCompletion() { From 24d6ea5444a5a782d29ad64dc129237587bae222 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 3 Oct 2024 17:11:41 +0900 Subject: [PATCH 09/17] Change failed states to be considered as `NotPlaying` --- osu.Game/Screens/Play/Player.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index d6ac279494e1..536050c9bd7c 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -508,8 +508,8 @@ private void onBreakTimeChanged(ValueChangedEvent isBreakTime) private void updateGameplayState() { - bool inGameplay = !DrawableRuleset.HasReplayLoaded.Value && !GameplayState.HasPassed; - bool inBreak = breakTracker.IsBreakTime.Value || DrawableRuleset.IsPaused.Value || GameplayState.HasFailed; + bool inGameplay = !DrawableRuleset.HasReplayLoaded.Value && !GameplayState.HasPassed && !GameplayState.HasFailed; + bool inBreak = breakTracker.IsBreakTime.Value || DrawableRuleset.IsPaused.Value; if (inGameplay) playingState.Value = inBreak ? LocalUserPlayingState.Break : LocalUserPlayingState.Playing; From a41c6dce04f7185ea49d89801491a913a888fb51 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 7 Oct 2024 16:43:50 +0900 Subject: [PATCH 10/17] Fix android build failure due to enum rename --- osu.Android/GameplayScreenRotationLocker.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Android/GameplayScreenRotationLocker.cs b/osu.Android/GameplayScreenRotationLocker.cs index ffd4218ea9f8..26234545ef31 100644 --- a/osu.Android/GameplayScreenRotationLocker.cs +++ b/osu.Android/GameplayScreenRotationLocker.cs @@ -12,7 +12,7 @@ namespace osu.Android { public partial class GameplayScreenRotationLocker : Component { - private IBindable localUserPlaying = null!; + private IBindable localUserPlaying = null!; [Resolved] private OsuGameActivity gameActivity { get; set; } = null!; @@ -24,11 +24,11 @@ private void load(ILocalUserPlayInfo localUserPlayInfo) localUserPlaying.BindValueChanged(updateLock, true); } - private void updateLock(ValueChangedEvent userPlaying) + private void updateLock(ValueChangedEvent userPlaying) { gameActivity.RunOnUiThread(() => { - gameActivity.RequestedOrientation = userPlaying.NewValue != LocalUserPlayingStates.NotPlaying ? ScreenOrientation.Locked : gameActivity.DefaultOrientation; + gameActivity.RequestedOrientation = userPlaying.NewValue != LocalUserPlayingState.NotPlaying ? ScreenOrientation.Locked : gameActivity.DefaultOrientation; }); } } From 1f45b2134f9b579374119ff04b04921bb1592335 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 7 Oct 2024 17:04:52 +0900 Subject: [PATCH 11/17] Remove unnecessary `runOutsideOfGameplay` call --- osu.Desktop/Updater/VelopackUpdateManager.cs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/osu.Desktop/Updater/VelopackUpdateManager.cs b/osu.Desktop/Updater/VelopackUpdateManager.cs index 5dda03a3d307..5a02e95e1f04 100644 --- a/osu.Desktop/Updater/VelopackUpdateManager.cs +++ b/osu.Desktop/Updater/VelopackUpdateManager.cs @@ -63,17 +63,14 @@ private async Task checkForUpdateAsync(UpdateProgressNotification? notific // Velopack does support this scenario (see https://github.com/ppy/osu/pull/28743#discussion_r1743495975). if (pendingUpdate != null) { - runOutsideOfGameplay(() => + // If there is an update pending restart, show the notification to restart again. + notificationOverlay.Post(new UpdateApplicationCompleteNotification { - // If there is an update pending restart, show the notification to restart again. - notificationOverlay.Post(new UpdateApplicationCompleteNotification + Activated = () => { - Activated = () => - { - Task.Run(restartToApplyUpdate); - return true; - } - }); + Task.Run(restartToApplyUpdate); + return true; + } }); return true; From 38ee824b124b03d09dc658da9b556e7ba9a95aa9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 7 Oct 2024 17:07:25 +0900 Subject: [PATCH 12/17] Add second call of `runOutsideGameplay` on update progress notification At this point the update is already started in the background but I guess we can still block the notification from interrupting the user. --- osu.Desktop/Updater/VelopackUpdateManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Desktop/Updater/VelopackUpdateManager.cs b/osu.Desktop/Updater/VelopackUpdateManager.cs index 5a02e95e1f04..46c5793fc86f 100644 --- a/osu.Desktop/Updater/VelopackUpdateManager.cs +++ b/osu.Desktop/Updater/VelopackUpdateManager.cs @@ -97,7 +97,7 @@ private async Task checkForUpdateAsync(UpdateProgressNotification? notific }, }; - Schedule(() => notificationOverlay.Post(notification)); + runOutsideOfGameplay(() => notificationOverlay.Post(notification)); } notification.StartDownload(); From 19b586e6f7c98721d8c252b08ebb3ab3440a3536 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Mon, 7 Oct 2024 17:45:18 +0900 Subject: [PATCH 13/17] Remove unrelated test --- .../Navigation/TestSceneScreenNavigation.cs | 31 ------------------- 1 file changed, 31 deletions(-) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs index d374ae1bd9f1..eda7ce925a08 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs @@ -364,37 +364,6 @@ public void TestRetryImmediatelyAfterCompletion() AddUntilStep("wait for player", () => Game.ScreenStack.CurrentScreen != getOriginalPlayer() && Game.ScreenStack.CurrentScreen is Player); } - [Test] - public void TestSaveFailedReplay() - { - Player player = null; - - Screens.Select.SongSelect songSelect = null; - PushAndConfirm(() => songSelect = new TestPlaySongSelect()); - AddUntilStep("wait for song select", () => songSelect.BeatmapSetsLoaded); - - AddStep("import beatmap", () => BeatmapImportHelper.LoadOszIntoOsu(Game, virtualTrack: true).WaitSafely()); - - AddUntilStep("wait for selected", () => !Game.Beatmap.IsDefault); - - AddStep("press enter", () => InputManager.Key(Key.Enter)); - - AddUntilStep("wait for player", () => - { - DismissAnyNotifications(); - return (player = Game.ScreenStack.CurrentScreen as Player) != null; - }); - - AddUntilStep("wait for fail", () => player.GameplayState.HasFailed); - - AddUntilStep("fail screen displayed", () => player.ChildrenOfType().First().State.Value == Visibility.Visible); - AddUntilStep("wait for button clickable", () => player.ChildrenOfType().First().ChildrenOfType().First().Enabled.Value); - - AddUntilStep("score not in database", () => Realm.Run(r => r.Find(player.Score.ScoreInfo.ID) == null)); - AddStep("click save button", () => player.ChildrenOfType().First().ChildrenOfType().First().TriggerClick()); - AddUntilStep("score in database", () => Realm.Run(r => r.Find(player.Score.ScoreInfo.ID) != null)); - } - [Test] public void TestExitImmediatelyAfterCompletion() { From 6e659e156e4ab28acf18aabb8d0079beb9a79121 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Mon, 7 Oct 2024 17:50:23 +0900 Subject: [PATCH 14/17] Refactoring for correctness --- osu.Game/Input/OsuUserInputManager.cs | 2 +- osu.Game/OsuGame.cs | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/osu.Game/Input/OsuUserInputManager.cs b/osu.Game/Input/OsuUserInputManager.cs index 26ce2312d5f7..252c0eb7f88a 100644 --- a/osu.Game/Input/OsuUserInputManager.cs +++ b/osu.Game/Input/OsuUserInputManager.cs @@ -12,7 +12,7 @@ public partial class OsuUserInputManager : UserInputManager { protected override bool AllowRightClickFromLongTouch => PlayingState.Value == LocalUserPlayingState.NotPlaying; - public readonly Bindable PlayingState = new Bindable(); + public readonly IBindable PlayingState = new Bindable(); internal OsuUserInputManager() { diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 935631c8e9e9..dce24c6ee7e2 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -1548,11 +1548,15 @@ private void screenChanged(IScreen current, IScreen newScreen) scope.SetTag(@"screen", newScreen?.GetType().ReadableName() ?? @"none"); }); - // reset on screen change for sanity. - playingState.Value = LocalUserPlayingState.NotPlaying; + switch (current) + { + case Player player: + player.PlayingState.UnbindFrom(playingState); - if (current is Player oldPlayer) - oldPlayer.PlayingState.UnbindFrom(playingState); + // reset for sanity. + playingState.Value = LocalUserPlayingState.NotPlaying; + break; + } switch (newScreen) { From aee5f0ebf532a18510b956b29be8ab92e80ea195 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Mon, 7 Oct 2024 19:01:25 +0900 Subject: [PATCH 15/17] Fix incorrect condition --- osu.Game/Input/OsuUserInputManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Input/OsuUserInputManager.cs b/osu.Game/Input/OsuUserInputManager.cs index 252c0eb7f88a..2138a8b247f1 100644 --- a/osu.Game/Input/OsuUserInputManager.cs +++ b/osu.Game/Input/OsuUserInputManager.cs @@ -10,7 +10,7 @@ namespace osu.Game.Input { public partial class OsuUserInputManager : UserInputManager { - protected override bool AllowRightClickFromLongTouch => PlayingState.Value == LocalUserPlayingState.NotPlaying; + protected override bool AllowRightClickFromLongTouch => PlayingState.Value != LocalUserPlayingState.Playing; public readonly IBindable PlayingState = new Bindable(); From 7cc6fe3819ff355bccd8100842eea4d65f8c2ed2 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Mon, 7 Oct 2024 19:35:09 +0900 Subject: [PATCH 16/17] Return true while in gameplay A `false` value marks the user as being on the latest release, and notifies them as such when clicking the button in settings. In reality, we don't know whether this is the case yet - we're just deferring the check. Somewhat minor change because the chance of a user manually going into settings and clicking the button is very small. --- osu.Desktop/Updater/VelopackUpdateManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Desktop/Updater/VelopackUpdateManager.cs b/osu.Desktop/Updater/VelopackUpdateManager.cs index 46c5793fc86f..d92d43b0e7dc 100644 --- a/osu.Desktop/Updater/VelopackUpdateManager.cs +++ b/osu.Desktop/Updater/VelopackUpdateManager.cs @@ -56,7 +56,7 @@ private async Task checkForUpdateAsync(UpdateProgressNotification? notific if (isInGameplay) { scheduleRecheck = true; - return false; + return true; } // TODO: we should probably be checking if there's a more recent update, rather than shortcutting here. From c3f2c82b108bdd1d8658f0fcec1427fd35b13d4f Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Mon, 7 Oct 2024 19:48:57 +0900 Subject: [PATCH 17/17] Remove unused parameter --- osu.Desktop/Updater/VelopackUpdateManager.cs | 21 ++++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/osu.Desktop/Updater/VelopackUpdateManager.cs b/osu.Desktop/Updater/VelopackUpdateManager.cs index d92d43b0e7dc..33ff6c2b3739 100644 --- a/osu.Desktop/Updater/VelopackUpdateManager.cs +++ b/osu.Desktop/Updater/VelopackUpdateManager.cs @@ -45,7 +45,7 @@ private void load(INotificationOverlay notifications) protected override async Task PerformUpdateCheck() => await checkForUpdateAsync().ConfigureAwait(false); - private async Task checkForUpdateAsync(UpdateProgressNotification? notification = null) + private async Task checkForUpdateAsync() { // whether to check again in 30 minutes. generally only if there's an error or no update was found (yet). bool scheduleRecheck = false; @@ -86,26 +86,21 @@ private async Task checkForUpdateAsync(UpdateProgressNotification? notific } // An update is found, let's notify the user and start downloading it. - if (notification == null) + UpdateProgressNotification notification = new UpdateProgressNotification { - notification = new UpdateProgressNotification + CompletionClickAction = () => { - CompletionClickAction = () => - { - Task.Run(restartToApplyUpdate); - return true; - }, - }; - - runOutsideOfGameplay(() => notificationOverlay.Post(notification)); - } + Task.Run(restartToApplyUpdate); + return true; + }, + }; + runOutsideOfGameplay(() => notificationOverlay.Post(notification)); notification.StartDownload(); try { await updateManager.DownloadUpdatesAsync(pendingUpdate, p => notification.Progress = p / 100f).ConfigureAwait(false); - runOutsideOfGameplay(() => notification.State = ProgressNotificationState.Completed); } catch (Exception e)