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.
Closes#25663 (again).
As it turns out, in some scenarios it can be the case that the current
game-global `Beatmap` is not valid for the current game-global
`Ruleset`. The validity of one and the other in conjunction is only
really validated by the song select screen; elsewhere there is no
guarantee that the global beatmap is playable using the global ruleset.
However, this only comes up in very specific circumstances, namely one:
when trying to autoplay a catch beatmap with osu! ruleset globally
active via the skin editor flow.
`Player` is responsible for retrieving the beatmap to be played. It does
so by invoking the appropriate beatmap converter and asking it if the
beatmap can be converted:
6d64538d7a/osu.Game/Beatmaps/WorkingBeatmap.cs (L262-L266)
If the code above throws, `Player` actually silently covers for this, by
trying the beatmap's default ruleset instead:
6d64538d7a/osu.Game/Screens/Play/Player.cs (L529-L536)
However, for the pairing of osu! ruleset and catch beatmap, this fails,
as `OsuBeatmapConverter`'s condition necessary for permitting conversion
is that the objects have a defined position:
6d64538d7a/osu.Game.Rulesets.Osu/Beatmaps/OsuBeatmapConverter.cs (L25)
which they will do, due to the fact that all catch beatmaps are really
just osu! beatmaps but with conversion steps applied, and thus `Player`
succeeds to load the catch beatmap in osu! ruleset.
In the skin editor scenario, this would lead to the secondary failure
of the skin editor trying to apply `CatchModAutoplay` on top of all
of that, which would fail at the hard-cast of the beatmap
to `CatchBeatmap`.
Until now, the only usage of ruleset layers was where there is both a
ruleset specific and non-ruleset-specific layer present. The matching
code was making assumptions about this.
As I tried to add a new playfield layer which breaks this assumption,
non-ruleset-specifc components were not being displayed in the toolbox.
This turned out to be due to a `target` of `null` being provided due to
the weird `getTarget` matching (that happened to *just* do what we
wanted previously due to the equals implementation, but only because
there was a container without the ruleset present in the available
targets).
I've changed this to be a more appropriate lookup method, where the
target for dependency sourcing is provided separately from the ruleset
filter.
This also moves the beatmap skin disable toggle to on toggle, in line
with review feedback.
I've decided to always apply the disable, not just on the `Player`
screen. It should be assumed that if a user is in the skin editor they
are never going to need access to this anyway.
An end result of #22674 is that `SkinBlueprintContainer`s are only ever
created by supplying a `SkinComponentsContainer` to them. However,
`SkinBlueprintContainer` still contained remnants of code that suggested
it was designed to handle cases where more than the drawable supplied to
it contained more than one `ISerialisableDrawableContainer`, or even
zero.
The zero path is totally dead right now (because every
`SkinComponentsContainer` is *by necessity* an
`ISerialisableDrawableContainer`), and the more-than-one path is dead
*for now* (and potentially forever?). Therefore, just hard-couple
`SkinBlueprintContainer` to receive a single target container.
Reproduction scenario:
1. Hover a toolbox item
2. Unhover the item, but do not hover any other item (can be done by
exiting the toolbox completely to the right)
3. Come back to the item hovered in step (1)
4. The item would spontaneously contract after a second
