I'm not exactly sure on the reproduction scenario here, but I have had
switching ruleset with converts disabled crash on me a few times
today. It appears to happen sometimes when after the switch the expanded
group no longer exists in the set mapping, because a filter just ran and
removed that group from set of possible groups because there'd be no
beatmaps under it.
I tried to manufacture a quick test but it's not a quick one to test
because filtering intereference is required to reproduce, I think.
I will try again on request but I mostly just want to get this fix out
ASAP before I finish up for the day.
Addresses https://github.com/ppy/osu/issues/33443, maybe.
I considered adding tests but they'd likely be janky and take a long
time to write, so decided against until there's a demand for it.
Closes https://github.com/ppy/osu/issues/34062.
The root cause of the issue is that `OnEntering()` calls
`onArrivingAtScreen()`, which calls `ensureGlobalBeatmapValid()`, which
would call `checkBeatmapValidForSelection()` with a `FilterCriteria`
instance retrieved from the `FilterControl`.
The problem with that is at the time that this call chain is happening,
`FilterControl` is not yet loaded, which means in particular that it has
not bound itself to the config bindable, as that happens on
`LoadComplete()`:
https://github.com/ppy/osu/blob/bff07010d1f9874125baf2918f02c5cf61a5ea60/osu.Game/Screens/SelectV2/FilterControl.cs#L198
To resolve this, retrieve the bindable in `SongSelect` itself, which
ensures it is valid for reading at the time the above call chain
happens.
Because it is 99% sure that doing so will fail and spam the user with
"this beatmap doesn't match the online version" notifications, and
because the map status is "locally modified", they should be pretty
aware of that already. This fixes the primary mode of the failure that
https://github.com/ppy/osu/pull/35173 was attempting to hack around.
This will have regressed somewhere around the time that BSS went live,
because at that point the editor stopped resetting online IDs for
beatmaps that got locally modified, making the `beatmapId <= 0` guards
no longer prevent attempts of submission.
A relatively recent regression. It's maybe not a huge one, in that it
probably doesn't matter all that much, but it is somewhat important to
keep the "locally modified" status of the set for as long as possible.
One reason for that is that keeping the "locally modified" status will
pull up a dialog when the user attempts to update the beatmap, warning
them that they will lose their local changes - this dialog will not show
if the online lookup flow is allowed to overwrite the map status with
something else.