Fix mods wrongly adding audio adjustments after being toggled off

[bdach]

Closes #18178.

First, an explanation why this broke. On master, ModTimeRamp stores a reference to the last track supplied on IApplicableToTrack:

osu/osu.Game/Rulesets/Mods/ModTimeRamp.cs

Line 60 in 5b17e92

this.track = track;

It then uses this track reference to access CurrentTime (which is not the problem), but also to dynamically add/remove audio adjustments when the "Adjust Pitch" option is toggled (which is the problem):

osu/osu.Game/Rulesets/Mods/ModTimeRamp.cs

Lines 101 to 107 in 5b17e92

	private void applyPitchAdjustment(ValueChangedEvent<bool> adjustPitchSetting)
	{
	// remove existing old adjustment
	track?.RemoveAdjustment(adjustmentForPitchSetting(adjustPitchSetting.OldValue), SpeedChange);
	track?.AddAdjustment(adjustmentForPitchSetting(adjustPitchSetting.NewValue), SpeedChange);
	}

It turns out that this only worked correctly on the old overlay due to a particular ordering of operations. With the old mod overlay, the following happened:

• On deselection, ModButton resets settings to defaults first
  • The deselection potentially triggers a change of AdjustPitch to defaults, therefore potentially swapping a tempo adjustment for a frequency adjustment
• ModButton then signals that its selection has changed, eventually triggering a game-global SelectedMods update
• MusicController subscribes to mod changes and resets adjustments after all that.

On the new design, the ordering there ends up basically swapped - the adjustments are reset, the mod is reset to defaults after that, which causes an AdjustPitch change, which causes a dynamic adjustment to be added and triggers the bug.

Rather than focus on which ordering is correct, I initially resolved to add an IApplicableToTrack.UnapplyFromTrack() method, but then figured that this isn't really required so long as mods stop adding adjustments on held references to tracks. Which is what this pull does - ModTimeRamp and ModAdaptiveSpeed are changed so that they don't apply adjustments to held references to tracks, and ample warning is added to xmldoc to not do this. Unfortunately in ModTimeRamp I cannot fully get rid of the reference, as that mod also accesses ITrack.CurrentTime.

I know that @frenzibyte was really adamant about some fool-proof way to add adjustments that would make it impossible for this to ever happen, but (a) my brain somehow just stops working when reading any of that proposal, and (b) it doesn't solve the fact that ModTimeRamp needs ITrack to work, and at the point where you've got a track reference in hand again, you can bug out again.

[frenzibyte]

I'm just going to note that I'm personally fine with either solutions, but I thought it may be better going the fool-proof way as it wouldn't require that many lines of changes (maybe, maybe not...), and have only brought it up initially as an overall improvement suggestion, yet I was failing to understand what the concern was about it.

I've looked into it and this is pretty much what I imagine to be done (275e5dc), with the second point in OP being fixed by using playfield.Clock.CurrentTime instead of track.CurrentTime.

[smoogipoo]

I'm not against this, but I would also like to explore @frenzibyte 's version in the future, since I think that reads better.

[peppy]

Honestly I was going to just let this one fly without worrying too much about the implementation, but I think @frenzibyte's proposal holds out better. At least I can understand the code by reading it, while this PR's unbind logic is a bit... hard to follow.

@frenzibyte can you PR your version with the test from this one to confirm it works as expected?

probably alternative

[bdach]

I have... several doubts about the correctness of the alternative, on a five second read. But whatever, if it gets PRed then I'll say my piece about it and leave it be, and you can decide which one you prefer.

[bdach]

Closing as no longer required.