This reverts commit f3faad74d5, reversing
changes made to 712e7bc7bf.
Several issues arose after migrating to 5.0, including, but possibly not
limited to, performance regressions in song select, as well as failures
when attempting to save beatmaps after metadata changes in the editor.
It turns out the SQLite API isn't smart enough to handle nullables
directly, so we need to help it out a bit.
Stops the following from being thrown:
```
System.InvalidOperationException: Value must be set.
at Microsoft.Data.Sqlite.SqliteParameter.Bind(sqlite3_stmt stmt) = 3
at
at Microsoft.Data.Sqlite.SqliteParameterCollection.Bind(sqlite3_stmt
stmt) = 3 at
at Microsoft.Data.Sqlite.SqliteCommand.ExecuteReader(CommandBehavior
behavior)
at Microsoft.Data.Sqlite.SqliteCommand.ExecuteReader()
at
osu.Game.Beatmaps.BeatmapManager.BeatmapOnlineLookupQueue.checkLocalCache(BeatmapSetInfo
set, BeatmapInfo beatmap) in
/Users/dean/Projects/osu/osu.Game/Beatmaps/BeatmapManager_BeatmapOnlineLookupQueue.cs:line
166 = 166
```
The previous code would run a calcaulation for the beatmap's own ruleset
if the current one failed. While this does make sense, with the current
way we use this component (and the implementation flow) it is quite unsafe.
The to the call on `.Result` in the `catch` block, this would 100%
deadlock due to the thread concurrency of the `ThreadedTaskScheduler`
being 1. Even if the nested run could be run inline (it should be), the
task scheduler won't even get to the point of checking whether this is
feasible due to it being saturated by the already running task.
I'm not sure if we still need this fallback lookup logic. After removing
it, it's feasible that 0 stars will be returned during the scenario that
previously caused a deadlock, but I don't necessarily think this is
incorrect. There may be another reason for this needing to exist which
I'm not aware of (diffcalc?) but if that's the case we may want to move
the try-catch handling to the point of usage.
To reproduce the deadlock scenario with 100% success (the repro
instructions in the linked issue aren't that simple and require some
patience and good timing), the main portion of the lookup can be changed
to randomly trigger a nested lookup:
```
if (RNG.NextSingle() > 0.5f)
return GetAsync(new
DifficultyCacheLookup(key.Beatmap, key.Beatmap.Ruleset,
key.OrderedMods)).Result;
else
return new StarDifficulty(attributes);
```
After switching beatmap once or twice, pausing debug and viewing the
state of threads should show exactly what is going on.
