From 95e745c6fbabce01dda192e567546185bfe62e9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 15 Feb 2024 10:16:06 +0100 Subject: [PATCH 1/3] Use better messaging for selected submission failure reasons These have been cropping up rather often lately, mostly courtesy of linux users, but not only: https://github.com/ppy/osu/issues/26840 https://github.com/ppy/osu/issues/27008 https://github.com/ppy/osu/discussions/26962 so this is a proposal for slightly improved messaging for such cases to hopefully get users on the right track. The original error is still logged to network log, so there's no information loss. --- osu.Game/Screens/Play/SubmittingPlayer.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Play/SubmittingPlayer.cs b/osu.Game/Screens/Play/SubmittingPlayer.cs index c45d46e9938d..c759710abaec 100644 --- a/osu.Game/Screens/Play/SubmittingPlayer.cs +++ b/osu.Game/Screens/Play/SubmittingPlayer.cs @@ -140,7 +140,13 @@ void handleTokenFailure(Exception exception) { switch (exception.Message) { - case "expired token": + case @"missing token header": + case @"invalid client hash": + case @"invalid verification hash": + Logger.Log("You are not able to submit a score. Please ensure that you are using the latest version of the official game releases.", level: LogLevel.Important); + break; + + case @"expired token": Logger.Log("Score submission failed because your system clock is set incorrectly. Please check your system time, date and timezone.", level: LogLevel.Important); break; From 898d5ce88bd4d249f98b8fe7cc6dd5e7cdd635d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 15 Feb 2024 10:40:40 +0100 Subject: [PATCH 2/3] Show selected submission failure messages even in solo Previously, if a `SubmittingPlayer` instance deemed it okay to proceed with gameplay despite submission failure, it would silently log all errors and proceed, but the score would still not be submitted. This feels a bit anti-user in the cases wherein something is genuinely wrong with either the client or web, so things like token verification failures or API failures are now shown as notifications to give the user an indication that something went wrong at all. Selected cases (non-user-playable mod, logged out, beatmap is not online) are still logged silently because those are either known and expected, or someone is messing with things. --- osu.Game/Screens/Play/SoloPlayer.cs | 2 +- osu.Game/Screens/Play/SubmittingPlayer.cs | 33 ++++++++++++----------- osu.Game/Tests/Visual/TestPlayer.cs | 2 +- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/osu.Game/Screens/Play/SoloPlayer.cs b/osu.Game/Screens/Play/SoloPlayer.cs index f7ae3eb62b7b..f4cf2da364dd 100644 --- a/osu.Game/Screens/Play/SoloPlayer.cs +++ b/osu.Game/Screens/Play/SoloPlayer.cs @@ -52,7 +52,7 @@ protected override GameplayLeaderboard CreateGameplayLeaderboard() => Scores = { BindTarget = LeaderboardScores } }; - protected override bool HandleTokenRetrievalFailure(Exception exception) => false; + protected override bool ShouldExitOnTokenRetrievalFailure(Exception exception) => false; protected override Task ImportScore(Score score) { diff --git a/osu.Game/Screens/Play/SubmittingPlayer.cs b/osu.Game/Screens/Play/SubmittingPlayer.cs index c759710abaec..0873f6079161 100644 --- a/osu.Game/Screens/Play/SubmittingPlayer.cs +++ b/osu.Game/Screens/Play/SubmittingPlayer.cs @@ -118,7 +118,7 @@ private bool handleTokenRetrieval() token = r.ID; tcs.SetResult(true); }; - req.Failure += handleTokenFailure; + req.Failure += ex => handleTokenFailure(ex, displayNotification: true); api.Queue(req); @@ -128,14 +128,20 @@ private bool handleTokenRetrieval() return true; - void handleTokenFailure(Exception exception) + void handleTokenFailure(Exception exception, bool displayNotification = false) { tcs.SetResult(false); - if (HandleTokenRetrievalFailure(exception)) + bool shouldExit = ShouldExitOnTokenRetrievalFailure(exception); + + if (displayNotification || shouldExit) { + string whatWillHappen = shouldExit + ? "You are not able to submit a score." + : "The following score will not be submitted."; + if (string.IsNullOrEmpty(exception.Message)) - Logger.Error(exception, "Failed to retrieve a score submission token."); + Logger.Error(exception, $"{whatWillHappen} Failed to retrieve a score submission token."); else { switch (exception.Message) @@ -143,31 +149,28 @@ void handleTokenFailure(Exception exception) case @"missing token header": case @"invalid client hash": case @"invalid verification hash": - Logger.Log("You are not able to submit a score. Please ensure that you are using the latest version of the official game releases.", level: LogLevel.Important); + Logger.Log($"{whatWillHappen} Please ensure that you are using the latest version of the official game releases.", level: LogLevel.Important); break; case @"expired token": - Logger.Log("Score submission failed because your system clock is set incorrectly. Please check your system time, date and timezone.", level: LogLevel.Important); + Logger.Log($"{whatWillHappen} Your system clock is set incorrectly. Please check your system time, date and timezone.", level: LogLevel.Important); break; default: - Logger.Log($"You are not able to submit a score: {exception.Message}", level: LogLevel.Important); + Logger.Log($"{whatWillHappen} {exception.Message}", level: LogLevel.Important); break; } } + } + if (shouldExit) + { Schedule(() => { ValidForResume = false; this.Exit(); }); } - else - { - // Gameplay is allowed to continue, but we still should keep track of the error. - // In the future, this should be visible to the user in some way. - Logger.Log($"Score submission token retrieval failed ({exception.Message})"); - } } } @@ -176,7 +179,7 @@ void handleTokenFailure(Exception exception) /// /// The error causing the failure. /// Whether gameplay should be immediately exited as a result. Returning false allows the gameplay session to continue. Defaults to true. - protected virtual bool HandleTokenRetrievalFailure(Exception exception) => true; + protected virtual bool ShouldExitOnTokenRetrievalFailure(Exception exception) => true; protected override async Task PrepareScoreForResultsAsync(Score score) { @@ -237,7 +240,7 @@ private void submitFromFailOrQuit() /// /// Construct a request to be used for retrieval of the score token. - /// Can return null, at which point will be fired. + /// Can return null, at which point will be fired. /// [CanBeNull] protected abstract APIRequest CreateTokenRequest(); diff --git a/osu.Game/Tests/Visual/TestPlayer.cs b/osu.Game/Tests/Visual/TestPlayer.cs index d9cae6b03b97..579a1934e020 100644 --- a/osu.Game/Tests/Visual/TestPlayer.cs +++ b/osu.Game/Tests/Visual/TestPlayer.cs @@ -61,7 +61,7 @@ public TestPlayer(bool allowPause = true, bool showResults = true, bool pauseOnF PauseOnFocusLost = pauseOnFocusLost; } - protected override bool HandleTokenRetrievalFailure(Exception exception) => false; + protected override bool ShouldExitOnTokenRetrievalFailure(Exception exception) => false; protected override APIRequest CreateTokenRequest() { From ec26ab51d18d5e4e46ee30782ebc388461c4073f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 19 Feb 2024 13:56:21 +0100 Subject: [PATCH 3/3] Use different wording --- osu.Game/Screens/Play/SubmittingPlayer.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/osu.Game/Screens/Play/SubmittingPlayer.cs b/osu.Game/Screens/Play/SubmittingPlayer.cs index 0873f6079161..ecb507f38231 100644 --- a/osu.Game/Screens/Play/SubmittingPlayer.cs +++ b/osu.Game/Screens/Play/SubmittingPlayer.cs @@ -137,11 +137,11 @@ void handleTokenFailure(Exception exception, bool displayNotification = false) if (displayNotification || shouldExit) { string whatWillHappen = shouldExit - ? "You are not able to submit a score." - : "The following score will not be submitted."; + ? "Play in this state is not permitted." + : "Your score will not be submitted."; if (string.IsNullOrEmpty(exception.Message)) - Logger.Error(exception, $"{whatWillHappen} Failed to retrieve a score submission token."); + Logger.Error(exception, $"Failed to retrieve a score submission token.\n\n{whatWillHappen}"); else { switch (exception.Message) @@ -149,11 +149,11 @@ void handleTokenFailure(Exception exception, bool displayNotification = false) case @"missing token header": case @"invalid client hash": case @"invalid verification hash": - Logger.Log($"{whatWillHappen} Please ensure that you are using the latest version of the official game releases.", level: LogLevel.Important); + Logger.Log($"Please ensure that you are using the latest version of the official game releases.\n\n{whatWillHappen}", level: LogLevel.Important); break; case @"expired token": - Logger.Log($"{whatWillHappen} Your system clock is set incorrectly. Please check your system time, date and timezone.", level: LogLevel.Important); + Logger.Log($"Your system clock is set incorrectly. Please check your system time, date and timezone.\n\n{whatWillHappen}", level: LogLevel.Important); break; default: