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.
The import preparation task can actually be non-null when exiting even
if the player hasn't passed:
- fail beatmap
- click import button to import the failed replay
Closes https://github.com/ppy/osu/issues/25247.
The scenario involved here is as follows:
1. User completes beatmap by hitting all notes.
2. `checkScoreCompleted()` determines completion, and calls
`progressToResults(withDelay: true)`.
3. `progressToResults()` schedules `resultsDisplayDelegate`, which
includes a call to `prepareAndImportScoreAsync()`, a second in the
future.
4. User presses quick retry hotkey.
This calls `Player.Restart(quickRestart: true)`,
which invokes `Player.RestartRequested`,
which in turn calls `PlayerLoader.restartRequested(true)`,
which in turn causes `PlayerLoader` to make itself current,
which means that `Player.OnExiting()` will get called.
5. `Player.OnExiting()` sees that `prepareScoreForDisplayTask` is null
(because `prepareAndImportScoreAsync()` - which sets it -
is scheduled to happen in the future), and as such assumes that
the score did not complete. Thus, it marks the score as failed.
6. `Player.Restart()` after invoking `RestartRequested` calls
`PerformExit(false)`, which then will unconditionally call
`prepareAndImportScoreAsync()`. But the score has already been marked
as failed.
The flow above can be described as "convoluted", but I'm not sure I have
it in me right now to try and refactor it again. Therefore, to fix,
switch the `prepareScoreForDisplayTask` null check in
`Player.OnExiting()` to check `GameplayState.HasPassed` instead, as it
is not susceptible to the same out-of-order read issue.
Previously, for lazer scores, the ID returned from `osu-web` was
discarded and replaced with -1, due to the fact that the appropriate
structures for unification with stable, as well as unification across
solo and multiplayer, were not in place yet.
Now we're at the point where scores from all the aforementioned sources
receive a `solo_scores` DB row, and as such, we can start treating
`solo_scores`-scheme IDs as canonical "online IDs" for a score.
I've gone ahead and matched the osu!stable behaviour for this, as it
seems like it's what people are used to and they will settle for no
less.
Closes https://github.com/ppy/osu/issues/18089.
I also took the freedom to add type checking, as we can't limit the
usage of `Add()` since it's a Container. The exception thrown also
advises of using the suggested `AddTrigger()` instead.
Now that just one method for stopping samples is left, let's just
repurpose st as the general "stop global effects" method rather than
have it there with a hyperspecific name. It also has good symmetry, as
there already was a `Start()` method in the class.
It is unnecessary, as a successful restart will exit the current player
screen, and `OnExiting()` has another `StopSampleAndRemoveFilters()`
call, which means that in the restart flow the sample was actually
getting stopped twice.
Standard exit flow is fine, it only stopped the sample once.
Allows directly referencing rather than going through `DrawableRuleset`.
Helps with testing and implementation of the new song progress display
(#22144).
