-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Add better submission failure messaging #27180
Conversation
These have been cropping up rather often lately, mostly courtesy of linux users, but not only: ppy#26840 ppy#27008 ppy#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.
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.
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."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 @@ 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:
Something like this may read better, I think.
Ultimately designing a custom notification for this is probably warranted, at some point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have applied verbatim.
Ultimately designing a custom notification for this is probably warranted, at some point.
Not sure what this would entail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly just adding an icon and what not.
Subjective, so I accept the possibility of this getting dumpstered on sight.
Use better messaging for selected submission failure reasons
Some failure reasons related to more stringent integrity checks have been cropping up rather often lately, mostly courtesy of linux users, but not only:
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.
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.