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 all 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
11 changes: 6 additions & 5 deletions osu.Android/GameplayScreenRotationLocker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool> localUserPlaying = null!;
private IBindable<LocalUserPlayingState> 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<bool> userPlaying)
private void updateLock(ValueChangedEvent<LocalUserPlayingState> userPlaying)
{
gameActivity.RunOnUiThread(() =>
{
gameActivity.RequestedOrientation = userPlaying.NewValue ? ScreenOrientation.Locked : gameActivity.DefaultOrientation;
gameActivity.RequestedOrientation = userPlaying.NewValue != LocalUserPlayingState.NotPlaying ? ScreenOrientation.Locked : gameActivity.DefaultOrientation;
});
}
}
Expand Down
40 changes: 24 additions & 16 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 != LocalUserPlayingState.NotPlaying;

private UpdateInfo? pendingUpdate;

public VelopackUpdateManager()
Expand All @@ -43,18 +45,18 @@ private void load(INotificationOverlay notifications)

protected override async Task<bool> PerformUpdateCheck() => await checkForUpdateAsync().ConfigureAwait(false);

private async Task<bool> checkForUpdateAsync(UpdateProgressNotification? notification = null)
private async Task<bool> checkForUpdateAsync()
{
// whether to check again in 30 minutes. generally only if there's an error or no update was found (yet).
bool scheduleRecheck = false;

try
{
// Avoid any kind of update checking while gameplay is running.
if (localUserInfo?.IsPlaying.Value == true)
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.
Expand Down Expand Up @@ -84,27 +86,22 @@ private async Task<bool> 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;
},
};

Schedule(() => 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);

notification.State = ProgressNotificationState.Completed;
runOutsideOfGameplay(() => notification.State = ProgressNotificationState.Completed);
}
catch (Exception e)
{
Expand All @@ -131,6 +128,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<LocalUserPlayingState> 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 == LocalUserPlayingState.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<LocalUserPlayingState> PlayingState => isPlaying;

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

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 = LocalUserPlayingState.NotPlaying);
}

[Test]
Expand Down Expand Up @@ -89,7 +89,7 @@ public void TestDifficultyProcessingWhilePlaying()
});
});

AddStep("Set playing", () => isPlaying.Value = true);
AddStep("Set playing", () => isPlaying.Value = LocalUserPlayingState.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 = LocalUserPlayingState.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<LocalUserPlayingState> playingState = new Bindable<LocalUserPlayingState>();

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

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

// a bit dodgy.
AddStep("bind playing state", () => ((IBindable<LocalUserPlayingState>)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 ? LocalUserPlayingState.Playing : LocalUserPlayingState.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<LocalUserPlayingState> playingState = new Bindable<LocalUserPlayingState>();

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 ? LocalUserPlayingState.Playing : LocalUserPlayingState.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 != LocalUserPlayingState.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<LocalUserPlayingState> 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 == LocalUserPlayingState.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 != LocalUserPlayingState.Playing;
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.

Arguable. I set this to avoid the silly hold ring from showing during breaks. And there's nothing at pause / fail that needs right click.

But if you feel strong about this then I'm fine with this change I guess.

Prefer review comments for stuff like this instead


public readonly BindableBool LocalUserPlaying = new BindableBool();
public readonly IBindable<LocalUserPlayingState> PlayingState = new Bindable<LocalUserPlayingState>();

internal OsuUserInputManager()
{
Expand Down
Loading
Loading