Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid updates (and update notifications) from appearing in more gameplay cases #30073

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 24 additions & 8 deletions osu.Desktop/Updater/VelopackUpdateManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -51,7 +53,7 @@ private async Task<bool> 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;
Expand All @@ -61,14 +63,17 @@ private async Task<bool> 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;
Expand Down Expand Up @@ -104,7 +109,7 @@ private async Task<bool> 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)
{
Expand All @@ -131,6 +136,17 @@ private async Task<bool> 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);
Expand Down
6 changes: 3 additions & 3 deletions osu.Desktop/Windows/GameplayWinKeyBlocker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace osu.Desktop.Windows
public partial class GameplayWinKeyBlocker : Component
{
private Bindable<bool> disableWinKey = null!;
private IBindable<bool> localUserPlaying = null!;
private IBindable<LocalUserPlayingStates> localUserPlaying = null!;
private IBindable<bool> isActive = null!;

[Resolved]
Expand All @@ -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();
Expand All @@ -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);
Expand Down
10 changes: 5 additions & 5 deletions osu.Game.Tests/Database/BackgroundDataStoreProcessorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ namespace osu.Game.Tests.Database
[HeadlessTest]
public partial class BackgroundDataStoreProcessorTests : OsuTestScene, ILocalUserPlayInfo
{
public IBindable<bool> IsPlaying => isPlaying;
public IBindable<LocalUserPlayingStates> PlayingState => isPlaying;

private readonly Bindable<bool> isPlaying = new Bindable<bool>();
private readonly Bindable<LocalUserPlayingStates> isPlaying = new Bindable<LocalUserPlayingStates>();

private BeatmapSetInfo importedSet = null!;

Expand All @@ -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]
Expand Down Expand Up @@ -89,7 +89,7 @@ public void TestDifficultyProcessingWhilePlaying()
});
});

AddStep("Set playing", () => isPlaying.Value = true);
AddStep("Set playing", () => isPlaying.Value = LocalUserPlayingStates.Playing);

AddStep("Reset difficulty", () =>
{
Expand Down Expand Up @@ -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", () =>
{
Expand Down
15 changes: 14 additions & 1 deletion osu.Game.Tests/Input/ConfineMouseTrackerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,34 @@

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
{
[HeadlessTest]
public partial class ConfineMouseTrackerTest : OsuGameTestScene
{
private readonly Bindable<LocalUserPlayingStates> playingState = new Bindable<LocalUserPlayingStates>();

[Resolved]
private FrameworkConfigManager frameworkConfigManager { get; set; } = null!;

[SetUpSteps]
public override void SetUpSteps()
{
base.SetUpSteps();

// a bit dodgy.
AddStep("bind playing state", () => ((IBindable<LocalUserPlayingStates>)playingState).BindTo(((ILocalUserPlayInfo)Game).PlayingState));
}

[TestCase(WindowMode.Windowed)]
[TestCase(WindowMode.Borderless)]
public void TestDisableConfining(WindowMode windowMode)
Expand Down Expand Up @@ -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<OsuConfineMouseMode>(OsuSetting.ConfineMouseMode) == mode);
Expand Down
4 changes: 1 addition & 3 deletions osu.Game.Tests/Visual/Gameplay/TestSceneOverlayActivation.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

#nullable disable

using System.Linq;
using NUnit.Framework;
using osu.Game.Overlays;
Expand All @@ -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()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@ public partial class TestSceneGameplayChatDisplay : OsuManualInputManagerTestSce
[Cached(typeof(ILocalUserPlayInfo))]
private ILocalUserPlayInfo localUserInfo;

private readonly Bindable<bool> localUserPlaying = new Bindable<bool>();
private readonly Bindable<LocalUserPlayingStates> playingState = new Bindable<LocalUserPlayingStates>();

private TextBox textBox => chatDisplay.ChildrenOfType<TextBox>().First();

public TestSceneGameplayChatDisplay()
{
var mockLocalUserInfo = new Mock<ILocalUserPlayInfo>();
mockLocalUserInfo.SetupGet(i => i.IsPlaying).Returns(localUserPlaying);
mockLocalUserInfo.SetupGet(i => i.PlayingState).Returns(playingState);

localUserInfo = mockLocalUserInfo.Object;
}
Expand Down Expand Up @@ -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);
}
}
2 changes: 1 addition & 1 deletion osu.Game/Database/BackgroundDataStoreProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions osu.Game/Input/ConfineMouseTracker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace osu.Game.Input
{
/// <summary>
/// Connects <see cref="OsuSetting.ConfineMouseMode"/> with <see cref="FrameworkSetting.ConfineMouseMode"/>.
/// If <see cref="OsuGame.LocalUserPlaying"/> is true, we should also confine the mouse cursor if it has been
/// If <see cref="ILocalUserPlayInfo.PlayingState"/> is playing, we should also confine the mouse cursor if it has been
/// requested with <see cref="OsuConfineMouseMode.DuringGameplay"/>.
/// </summary>
public partial class ConfineMouseTracker : Component
Expand All @@ -25,7 +25,7 @@ public partial class ConfineMouseTracker : Component
private Bindable<bool> frameworkMinimiseOnFocusLossInFullscreen;

private Bindable<OsuConfineMouseMode> osuConfineMode;
private IBindable<bool> localUserPlaying;
private IBindable<LocalUserPlayingStates> localUserPlaying;

[BackgroundDependencyLoader]
private void load(ILocalUserPlayInfo localUserInfo, FrameworkConfigManager frameworkConfigManager, OsuConfigManager osuConfigManager)
Expand All @@ -37,7 +37,7 @@ private void load(ILocalUserPlayInfo localUserInfo, FrameworkConfigManager frame
frameworkMinimiseOnFocusLossInFullscreen.BindValueChanged(_ => updateConfineMode());

osuConfineMode = osuConfigManager.GetBindable<OsuConfineMouseMode>(OsuSetting.ConfineMouseMode);
localUserPlaying = localUserInfo.IsPlaying.GetBoundCopy();
localUserPlaying = localUserInfo.PlayingState.GetBoundCopy();

osuConfineMode.ValueChanged += _ => updateConfineMode();
localUserPlaying.BindValueChanged(_ => updateConfineMode(), true);
Expand All @@ -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:
Expand Down
5 changes: 3 additions & 2 deletions osu.Game/Input/OsuUserInputManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<LocalUserPlayingStates> PlayingState = new Bindable<LocalUserPlayingStates>();

internal OsuUserInputManager()
{
Expand Down
36 changes: 18 additions & 18 deletions osu.Game/OsuGame.cs
Original file line number Diff line number Diff line change
Expand Up @@ -175,14 +175,9 @@ public partial class OsuGame : OsuGameBase, IKeyBindingHandler<GlobalAction>, IL
/// </summary>
public readonly IBindable<OverlayActivation> OverlayActivationMode = new Bindable<OverlayActivation>();

/// <summary>
/// Whether the local user is currently interacting with the game in a way that should not be interrupted.
/// </summary>
/// <remarks>
/// This is exclusively managed by <see cref="Player"/>. If other components are mutating this state, a more
/// resilient method should be used to ensure correct state.
/// </remarks>
public Bindable<bool> LocalUserPlaying = new BindableBool();
IBindable<LocalUserPlayingStates> ILocalUserPlayInfo.PlayingState => playingState;

private readonly Bindable<LocalUserPlayingStates> playingState = new Bindable<LocalUserPlayingStates>();

protected OsuScreenStack ScreenStack;

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this stops imports from running during break time and on the fail screen, which is an extra win.

}, true);

IsActive.BindValueChanged(active => updateActiveState(active.NewValue), true);
Expand Down Expand Up @@ -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:
Expand All @@ -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);
Expand Down Expand Up @@ -1621,7 +1623,5 @@ private void screenExited(IScreen lastScreen, IScreen newScreen)
if (newScreen == null)
Exit();
}

IBindable<bool> ILocalUserPlayInfo.IsPlaying => LocalUserPlaying;
}
}
2 changes: 1 addition & 1 deletion osu.Game/Overlays/BeatmapListingOverlay.cs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ protected override void LoadComplete()
apiUser.BindValueChanged(_ => Schedule(() =>
{
if (api.IsLoggedIn)
replaceResultsAreaContent(Drawable.Empty());
replaceResultsAreaContent(Empty());
}));
}

Expand Down
4 changes: 2 additions & 2 deletions osu.Game/Overlays/Chat/DaySeparator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ private void load()
Height = LineHeight,
Colour = colourProvider?.Background5 ?? Colour4.White,
},
Drawable.Empty(),
Empty(),
new OsuSpriteText
{
Anchor = Anchor.CentreRight,
Expand All @@ -87,7 +87,7 @@ private void load()
}
},
},
Drawable.Empty(),
Empty(),
new Circle
{
Anchor = Anchor.Centre,
Expand Down
Loading
Loading