Move beatmap track into MusicController

[smoogipoo]

• Depends on Make SkinnableSound an IAdjustableAudioComponent #9788
• Depends on Expose property to retrieve load state of underlying track osu-framework#3809
• Depends on Make ITrack an IAdjustableAudioComponent and IAdjustableClock osu-framework#3810

I apologise for the size of this one. It's really hard to make such a change without touching almost every file in the game, which probably spells bad news for the reach of the (now) MusicController in general.
Going forward this can probably be fixed on a per-case basis by either passing down the track or caching it locally at overarching components, which is probably a good idea in general for component isolation. I've already done this for Player because it felt wrong to DI MusicController in GameplayClockContainer and FailAnimation.

The idea of this PR is to define a single owner for the beatmap's track. Specifically, the track which plays the sound and which is stored as an on-going reference.
The goal is to eventually be able to fade between tracks when changed in song select, which needs both the single owner idea and for the track to be able to be placed in the draw hierarchy - it needs to also become a DrawableTrack.

Changes of particular note

• "Recycling" of working beatmap tracks has been removed. It is now handled by MusicController.
• The "transferring" of working beatmaps has been removed. Both the track and texture stores are passed to new WorkingBeatmap instances by BeatmapManager and the disposal of the track store is now also handled by the BeatmapManager.
• The cycling of tracks once one has complete has been moved from OsuGame to MusicController.
• MusicController has been moved to OsuGameBase. It is now loaded as part of the async procedure but before the game's Loader. I haven't fully assessed the performance impact of this since it was previously in a loadComponentSingleFile() but it doesn't seem to have a noticeable impact when loading osu! from an external USB drive with ~1000 sets.
  • Another way this can be done is to move the track handling part into its own separate component, like a "TrackController" that is DI'd in by the MusicController.

Testing

Beyond the automated CI tests, I've also tested a whole bunch of scenarios, including:

• Selecting multiplayer item with no local beatmap.
• Selecting multiplayer item with a local beatmap.
• Completing a beatmap import with the selected playlist item.
• All the intros.
• Track loops in song select.
• Track doesn't loop in main menu.
• Track loops in match subscreen.
• Track doesn't change when another beatmap from the same set is selected.
• Failing in gameplay.
• Restarting in gameplay.
• DT/NC/WU/WD mods applying at song select.
• DT/NC/WU/WD mods applying in gameplay.
• DT/NC/WU/WD mods applying after map is restarted.
• DT/NC/WU/WD mods applying in multiplayer (match subscreen and gameplay).
• Playing preview tracks to mute audio.
• Changing track while a preview track is playing.

[bdach]

Mostly out of curiosity, what is the end goal here? This seems like a pretty drastic change in direction.

[smoogipoo]

The goal is to have a dedicated owner of the track, rather than it being referenced/potentially created by everything in the game. One immediate use for this is to fade between tracks at song select, which is a 2-liner now that MusicController can .Expire() and transform the new track.

[peppy]

Are you talking about the ReadyButton usage with regards to multiplayer? or somewhere else?

[bdach]

Yeah, that was the one.

[peppy]

I've reverted that usage too. I thought it made sense to leave it as MusicController but looking at the code in the same method using gameBeatmap, revert does seem better.

[bdach]

I don't think I have any issues left with this any more, and I've also briefly play-tested this branch and had nothing funny happen, so going to throw in a tentative approve. Should get a second pair of eyes on this before merging though (hi @smoogipoo)

[smoogipoo]

As I mentioned in discord, I'm really not sure I like these.

Components that use both the beatmap and the track should outright crash if the track isn't loaded, rather than silently using the music controller's (wrong) track.

[smoogipoo]

Seems like #9843 has broken this branch a bit. Since mods no longer preserve references, ModTimeRamp doesn't have the correct track set during gameplay...

[smoogipoo]

I think this is ok, but would appreciate a second pass @peppy / @bdach , especially with my recent changes.

[bdach]

In the failing tests, doing this causes the adjustment to get re-applied twice because of this:

osu/osu.Game/Tests/Visual/PlayerTestScene.cs

Line 33 in 7e1844e

MusicController.AllowRateAdjustments = true;

Disabling this back again locally makes them pass, but this makes me quite uneasy... Maybe that true override is no longer necessary with the recent changes?

[peppy]

i would hope it's not needed.. but also hope that it didn't break anything.

[peppy]

@smoogipoo did you find the reason that was breaking things? kinda need some resolution there as it really shouldn't in my head.

[smoogipoo]

I haven't, but I can imagine why. musicController.ResetTrackAdjustments() doesn't just reset the adjustments, it also adds the MC's own adjustments via mod.ApplyToTrack(). So Player does the same and the end result is that adjustments get applied twice.
Of note, MusicController.AllowRateAdjustments is false by default, and is only set to true by OsuGame when an OsuScreen wants it (pretty much everywhere except Editor, Player, MainMenu).

I'm not super confident about this code

[peppy]

But player is also resetting?

[smoogipoo]

That's correct, check the implementation of ResetTrackAdjustments:

osu/osu.Game/Overlays/MusicController.cs

Lines 335 to 348 in dda776d

	public void ResetTrackAdjustments()
	{
	var track = current?.Track;
	if (track == null)
	return;
	track.ResetSpeedAdjustments();
	if (allowRateAdjustments)
	{
	foreach (var mod in mods.Value.OfType<IApplicableToTrack>())
	mod.ApplyToTrack(track);
	}
	}

[peppy]

ah......................... at very least let's add a mention to the xmldoc for ResetTrackAdjustments that the behaviour differs depending on AllowRateAdjustments