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 c45d46e993..c759710aba 100644 --- a/osu.Game/Screens/Play/SubmittingPlayer.cs +++ b/osu.Game/Screens/Play/SubmittingPlayer.cs @@ -140,7 +140,13 @@ namespace osu.Game.Screens.Play { 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 f7ae3eb62b..f4cf2da364 100644 --- a/osu.Game/Screens/Play/SoloPlayer.cs +++ b/osu.Game/Screens/Play/SoloPlayer.cs @@ -52,7 +52,7 @@ namespace osu.Game.Screens.Play 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 c759710aba..0873f60791 100644 --- a/osu.Game/Screens/Play/SubmittingPlayer.cs +++ b/osu.Game/Screens/Play/SubmittingPlayer.cs @@ -118,7 +118,7 @@ namespace osu.Game.Screens.Play token = r.ID; tcs.SetResult(true); }; - req.Failure += handleTokenFailure; + req.Failure += ex => handleTokenFailure(ex, displayNotification: true); api.Queue(req); @@ -128,14 +128,20 @@ namespace osu.Game.Screens.Play 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 @@ namespace osu.Game.Screens.Play 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 @@ namespace osu.Game.Screens.Play /// /// 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 @@ namespace osu.Game.Screens.Play /// /// 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 d9cae6b03b..579a1934e0 100644 --- a/osu.Game/Tests/Visual/TestPlayer.cs +++ b/osu.Game/Tests/Visual/TestPlayer.cs @@ -61,7 +61,7 @@ namespace osu.Game.Tests.Visual 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 0873f60791..ecb507f382 100644 --- a/osu.Game/Screens/Play/SubmittingPlayer.cs +++ b/osu.Game/Screens/Play/SubmittingPlayer.cs @@ -137,11 +137,11 @@ namespace osu.Game.Screens.Play 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 @@ namespace osu.Game.Screens.Play 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: