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.

Sign up for GitHub

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

Remove confirmation when game is asked to be restarted by the user #26558

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Remove confirmation when game is asked to be restarted by the user #26558

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3649876
Remove confirmation when game is asked to be restarted by the user
peppy Jan 16, 2024
247fb86
Merge branch 'master' into restart-confirmation-bypass
peppy Apr 16, 2024
3f4174f
Adjust documentation and flag set location slightly
peppy Apr 16, 2024
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
8 changes: 6 additions & 2 deletions osu.Desktop/OsuGameDesktop.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
using osu.Framework.Logging;
using osu.Game.Updater;
using osu.Desktop.Windows;
using osu.Game.Configuration;
using osu.Game.IO;
using osu.Game.IPC;
using osu.Game.Online.Multiplayer;
Expand Down Expand Up @@ -109,7 +110,7 @@ protected override UpdateManager CreateUpdateManager()
}
}

public override bool RestartAppWhenExited()
public override bool Restart()
{
switch (RuntimeInfo.OS)
{
Expand All @@ -125,7 +126,10 @@ public override bool RestartAppWhenExited()
return true;
}

return base.RestartAppWhenExited();
SessionStatics.SetValue(Static.RestartRequested, true);
AttemptExit();
Copy link
Member

Choose a reason for hiding this comment

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

This is never reached in Windows because of the early return above.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems valid to me, it looks like the game would not actually exit itself.


return base.Restart();
}

protected override void LoadComplete()
Expand Down
4 changes: 2 additions & 2 deletions osu.Desktop/Updater/SquirrelUpdateManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public partial class SquirrelUpdateManager : UpdateManager
private readonly SquirrelLogger squirrelLogger = new SquirrelLogger();

[Resolved]
private OsuGameBase game { get; set; } = null!;
private OsuGame game { get; set; } = null!;

[Resolved]
private ILocalUserPlayInfo? localUserInfo { get; set; }
Expand Down Expand Up @@ -150,7 +150,7 @@ private async Task<bool> checkForUpdateAsync(bool useDeltaPatching = true, Updat
private bool restartToApplyUpdate()
{
PrepareUpdateAsync()
.ContinueWith(_ => Schedule(() => game.AttemptExit()));
.ContinueWith(_ => Schedule(() => game.Restart()));
return true;
}

Expand Down
6 changes: 1 addition & 5 deletions osu.Game.Tournament/Screens/Setup/TournamentSwitcher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,7 @@ private void load(TournamentStorage storage)

reloadTournamentsButton.Action = () => dropdown.Items = storage.ListTournaments();

Action = () =>
{
game.RestartAppWhenExited();
game.AttemptExit();
};
Action = () => game.Restart();
folderButton.Action = () => storage.PresentExternally();

ButtonText = "Close osu!";
Expand Down
6 changes: 6 additions & 0 deletions osu.Game/Configuration/SessionStatics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ public class SessionStatics : InMemoryConfigManager<Static>
protected override void InitialiseDefaults()
{
SetDefault(Static.LoginOverlayDisplayed, false);
SetDefault(Static.RestartRequested, false);
SetDefault(Static.MutedAudioNotificationShownOnce, false);
SetDefault(Static.LowBatteryNotificationShownOnce, false);
SetDefault(Static.FeaturedArtistDisclaimerShownOnce, false);
Expand Down Expand Up @@ -80,5 +81,10 @@ public enum Static
/// Stores the local user's last score (can be completed or aborted).
/// </summary>
LastLocalUserScore,

/// <summary>
/// Whether the game is in the process of being restarted. This should always be the result of user intention to restart.
/// </summary>
RestartRequested
}
}
2 changes: 1 addition & 1 deletion osu.Game/OsuGameBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ public virtual void AttemptExit()
/// If supported by the platform, the game will automatically restart after the next exit.
/// </summary>
/// <returns>Whether a restart operation was queued.</returns>
public virtual bool RestartAppWhenExited() => false;
public virtual bool Restart() => false;

public bool Migrate(string path)
{
Expand Down
6 changes: 1 addition & 5 deletions osu.Game/Overlays/Settings/Sections/Graphics/RendererSettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,7 @@ private void load(FrameworkConfigManager config, OsuConfigManager osuConfig, IDi
if (r.NewValue == RendererType.Automatic && automaticRendererInUse)
return;

if (game?.RestartAppWhenExited() == true)
{
game.AttemptExit();
}
else
Copy link
Member Author

Choose a reason for hiding this comment

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

The flow here makes me wonder whether Restart should handle the confirmation for platforms which can't restart themselves, rather than having this handled on each usage.

But then again, the usages may be so few and far between that it's fine to leave it as-is?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd say per-usage is better because places like tournament client use Restart() and I don't really wanna think about how to make that work well across such varied usages.

if (game?.Restart() != true)
Copy link
Member

@Joehuu Joehuu Apr 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

If I thinking this right, on Windows, changing the renderer will just outright restart. Would still expect a confirmation dialog here, but different wording that it will restart automatically.

Copy link
Member Author

Choose a reason for hiding this comment

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

Don't really think its necessary.

{
dialogOverlay?.Push(new ConfirmDialog(GraphicsSettingsStrings.ChangeRendererConfirmation, () => game?.AttemptExit(), () =>
{
Expand Down
8 changes: 6 additions & 2 deletions osu.Game/Screens/Menu/MainMenu.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ public partial class MainMenu : OsuScreen, IHandlePresentBeatmap, IKeyBindingHan

private Bindable<double> holdDelay;
private Bindable<bool> loginDisplayed;
private Bindable<bool> isRestarting;

private HoldToExitGameOverlay holdToExitGameOverlay;

Expand All @@ -113,6 +114,7 @@ private void load(BeatmapListingOverlay beatmapListing, SettingsOverlay settings
{
holdDelay = config.GetBindable<double>(OsuSetting.UIHoldActivationDelay);
loginDisplayed = statics.GetBindable<bool>(Static.LoginOverlayDisplayed);
isRestarting = statics.GetBindable<bool>(Static.RestartRequested);

if (host.CanExit)
{
Expand Down Expand Up @@ -388,8 +390,10 @@ public override bool OnExiting(ScreenExitEvent e)
dialogOverlay != null
// if the dialog has already displayed and been accepted by the user, we are good.
&& !exitConfirmedViaDialog
// Only require confirmation if there is either an ongoing operation or the user exited via a non-hold escape press.
&& (notifications.HasOngoingOperations || !exitConfirmedViaHoldOrClick);
// Only require confirmation if there is either an ongoing operation or the exit process wasn't triggered via user intention.
&& (notifications.HasOngoingOperations || (!exitConfirmedViaHoldOrClick && !isRestarting.Value));

isRestarting.Value = false;

if (requiresConfirmation)
{
Expand Down
Loading