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.
It should be large enough to account for true accuracy
but small enough to disregard any hair-thin inaccuracy found
at the very end of the float value.
Instead of checking the master and music volumes separately to
see if they're <= 1%, check the aggregate of the two volumes to
see if it's <= -60 dB.
When muted notification is activated, restore the aggregate volume
level to -30 dB.
Temporary workaround for https://github.com/ppy/osu/issues/26404.
It appears that some audio files do not behave well with BASS, leading
BASS to report a contradictory state of affairs (i.e. a track that is
in playing state but also not progressing). This appears to be related
to seeking specifically, therefore only enable the validation of
playback rate in the most sensitive contexts, namely when any sort of
score submission is involved.
Closes https://github.com/ppy/osu/issues/26035.
`submitOnFailOrQuit()`, as the name suggests, can be called both when
the player has failed, or when the player screen is being exited from.
Notably, when perfect mod with auto-retry is active, the two happen
almost simultaneously.
This double call exposes a data race in `submitScore()` concerning the
handling of `scoreSubmissionSource`. The race could be experimentally
confirmed by applying the following patch:
diff --git a/osu.Game/Screens/Play/SubmittingPlayer.cs b/osu.Game/Screens/Play/SubmittingPlayer.cs
index 83adf1f960..76dd29bbdb 100644
--- a/osu.Game/Screens/Play/SubmittingPlayer.cs
+++ b/osu.Game/Screens/Play/SubmittingPlayer.cs
@@ -228,6 +228,7 @@ private Task submitScore(Score score)
return Task.CompletedTask;
}
+ Logger.Log($"{nameof(scoreSubmissionSource)} is {(scoreSubmissionSource == null ? "null" : "not null")}");
if (scoreSubmissionSource != null)
return scoreSubmissionSource.Task;
@@ -237,6 +238,7 @@ private Task submitScore(Score score)
Logger.Log($"Beginning score submission (token:{token.Value})...");
+ Logger.Log($"creating new {nameof(scoreSubmissionSource)}");
scoreSubmissionSource = new TaskCompletionSource<bool>();
var request = CreateSubmissionRequest(score, token.Value);
which would result in the following log output:
[runtime] 2024-01-02 09:54:13 [verbose]: scoreSubmissionSource is null
[runtime] 2024-01-02 09:54:13 [verbose]: scoreSubmissionSource is null
[runtime] 2024-01-02 09:54:13 [verbose]: Beginning score submission (token:36780)...
[runtime] 2024-01-02 09:54:13 [verbose]: creating new scoreSubmissionSource
[runtime] 2024-01-02 09:54:13 [verbose]: Beginning score submission (token:36780)...
[runtime] 2024-01-02 09:54:13 [verbose]: creating new scoreSubmissionSource
[network] 2024-01-02 09:54:13 [verbose]: Performing request osu.Game.Online.Solo.SubmitSoloScoreRequest
[network] 2024-01-02 09:54:14 [verbose]: Request to https://dev.ppy.sh/api/v2/beatmaps/869310/solo/scores/36780 successfully completed!
[network] 2024-01-02 09:54:14 [verbose]: SubmitSoloScoreRequest finished with response size of 639 bytes
[network] 2024-01-02 09:54:14 [verbose]: Performing request osu.Game.Online.Solo.SubmitSoloScoreRequest
[runtime] 2024-01-02 09:54:14 [verbose]: Score submission completed! (token:36780 id:20247)
[network] 2024-01-02 09:54:14 [verbose]: Request to https://dev.ppy.sh/api/v2/beatmaps/869310/solo/scores/36780 successfully completed!
[network] 2024-01-02 09:54:14 [verbose]: SubmitSoloScoreRequest finished with response size of 639 bytes
[runtime] 2024-01-02 09:54:14 [error]: An unhandled error has occurred.
[runtime] 2024-01-02 09:54:14 [error]: System.InvalidOperationException: An attempt was made to transition a task to a final state when it had already completed.
[runtime] 2024-01-02 09:54:14 [error]: at osu.Game.Screens.Play.SubmittingPlayer.<>c__DisplayClass30_0.<submitScore>b__0(MultiplayerScore s) in /home/dachb/Documents/opensource/osu/osu.Game/Screens/Play/SubmittingPlayer.cs:line 250
The intention of the submission logic was to only ever create one
`scoreSubmissionSource`, and then reuse this one if a redundant
submission request was made. However, because of the temporal proximity
of fail and quit in this particular case, combined with the fact that
the calls to `submitScore()` are taking place on TPL threads, means that
there is a read-write data race on `scoreSubmissionSource`, wherein the
source can be actually created twice.
This leads to two concurrent score submission requests, which, upon
completion, attempt to transition only _the second_
`scoreSubmissionSource` to a final state (this is because the API
success/failure request callbacks capture `this`, i.e. the entire
`SubmittingPlayer` instance, rather than the `scoreSubmissionSource`
reference specifically).
To fix, ensure correct synchronisation on the read-write critical
section, which should prevent the `scoreSubmissionSource` from being
created multiple times.
Originally when popping in, the ReplayPlayer was loaded first (if previous screen was MainMenu), and afterwards the SkinEditor component was loaded asynchronously. However, if the ReplayPlayer screen exits quickly (like in the event the beatmap has no objects), the skin editor component has not finished initializing (this is before it was even added to the component tree, so it's still not marked `Visible`), then the screen exiting will cause `OsuGame` to call SetTarget(newScreen) -> setTarget(...) which sees that the cached `skinEditor` is not visible yet, and hides/nulls the field. This is the point where LoadComponentAsync(editor, ...) finishes, and the callback sees that the cached skinEditor field is now different (null) than the one that was loaded, and never adds it to the component tree. This occurrence is unhandled and as such the SkinEditorOverlay never hides itself, consuming all input infinitely.
This PR changes the loading to start loading the ReplayPlayer *after* the SkinEditor has been loaded and added to the component tree.
Additionally, this lowers the exit delay for ReplayPlayer and changes the "no hit objects" notification to not be an error since it's a controlled exit.
