The `SelectedMods.BindValueChanged()` callback in `ModSelectOverlay` can
in some instances run recursively. This is most heavily leaned on in
scenarios where `SelectedMods` is updated by an external component. In
such cases, the mod select overlay needs to replace the mod instances
received externally with mod instances which it owns, so that the changes
made on the overlay can propagate outwards.
This in particular means that prior to this commit, it was possible to
encounter the following scenario:
modSettingChangeTracker?.Dispose();
updateFromExternalSelection(); // mutates SelectedMods to perform the replacement
// therefore causing a recursive call
modSettingChangeTracker?.Dispose();
// inner call continues
modSettingChangeTracker = new ModSettingChangeTracker(SelectedMods.Value);
// outer call continues
modSettingChangeTracker = new ModSettingChangeTracker(SelectedMods.Value);
This leaks one `modSettingChangeTracker` instance from the inner call,
which is never disposed.
To avoid this, move the disposal to the same side of the recursion that
the creation happens on, changing the call pattern to:
updateFromExternalSelection(); // mutates SelectedMods to perform the replacement
// therefore causing a recursive call
modSettingChangeTracker?.Dispose();
// inner call continues
modSettingChangeTracker = new ModSettingChangeTracker(SelectedMods.Value);
modSettingChangeTracker?.Dispose();
// outer call continues
modSettingChangeTracker = new ModSettingChangeTracker(SelectedMods.Value);
which, while slightly wasteful, does not cause any leaks.
The solution is definitely suboptimal, but addressing this properly
would entail a major rewrite of the mod instance management in the mods
overlay, which is probably not the wisest move to make right now.
The 0.01 `Precision` spec on `DifficultyMultiplierDisplay.Current` would
cause the difficulty multiplier to use a different midpoint rounding
strategy than `double.ToString()`, which is the one that the song select
footer relies on. For example, a value of 0.015 would be rounded down
to 0.01 by `double.ToString()`, but rounded up to 0.02
by `BindableDouble`.
Fix the discrepancy by just deleting the `Precision` spec. Since the
value of the bindable would go through `ToLocalisableString(@"N2")`
anyway, it was redundant as is.
Fixes#19889.
