Merge pull request #8129 from peppy/fix-dummy-track-completion-feedback

Fix DummyWorkingBeatmap's track completion attempting to change game-wide beatmap

osu.Game.Tests/Visual/Navigation/OsuGameTestScene.cs

@@ -102,6 +102,8 @@ namespace osu.Game.Tests.Visual.Navigation
 
             public new SettingsPanel Settings => base.Settings;
 
+            public new MusicController MusicController => base.MusicController;
+
             public new OsuConfigManager LocalConfig => base.LocalConfig;
 
             public new Bindable<WorkingBeatmap> Beatmap => base.Beatmap;

osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs

@@ -114,6 +114,22 @@ namespace osu.Game.Tests.Visual.Navigation
             AddAssert("Options overlay was closed", () => Game.Settings.State.Value == Visibility.Hidden);
         }
 
+        [Test]
+        public void TestWaitForNextTrackInMenu()
+        {
+            bool trackCompleted = false;
+
+            AddUntilStep("Wait for music controller", () => Game.MusicController.IsLoaded);
+            AddStep("Seek close to end", () =>
+            {
+                Game.MusicController.SeekTo(Game.Beatmap.Value.Track.Length - 1000);
+                Game.Beatmap.Value.Track.Completed += () => trackCompleted = true;
+            });
+
+            AddUntilStep("Track was completed", () => trackCompleted);
+            AddUntilStep("Track was restarted", () => Game.Beatmap.Value.Track.IsRunning);
+        }
+
         private void pushEscape() =>
             AddStep("Press escape", () => pressAndRelease(Key.Escape));
 

osu.Game/OsuGame.cs

@@ -399,18 +399,27 @@ namespace osu.Game
 
         private void beatmapChanged(ValueChangedEvent<WorkingBeatmap> beatmap)
         {
-            var nextBeatmap = beatmap.NewValue;
+            beatmap.OldValue?.CancelAsyncLoad();
-            if (nextBeatmap?.Track != null)
-                nextBeatmap.Track.Completed += currentTrackCompleted;
-
-            var oldBeatmap = beatmap.OldValue;
-            if (oldBeatmap?.Track != null)
-                oldBeatmap.Track.Completed -= currentTrackCompleted;
 
             updateModDefaults();
 
-            oldBeatmap?.CancelAsyncLoad();
+            var newBeatmap = beatmap.NewValue;
-            nextBeatmap?.BeginAsyncLoad();
+
+            if (newBeatmap != null)
+            {
+                newBeatmap.Track.Completed += () => Scheduler.AddOnce(() => trackCompleted(newBeatmap));
+                newBeatmap.BeginAsyncLoad();
+            }
+
+            void trackCompleted(WorkingBeatmap b)
+            {
+                // the source of track completion is the audio thread, so the beatmap may have changed before firing.
+                if (Beatmap.Value != b)
+                    return;
+
+                if (!Beatmap.Value.Track.Looping && !Beatmap.Disabled)
+                    MusicController.NextTrack();
+            }
         }
 
         private void modsChanged(ValueChangedEvent<IReadOnlyList<Mod>> mods)
@@ -431,12 +440,6 @@ namespace osu.Game
             }
         }
 
-        private void currentTrackCompleted() => Schedule(() =>
-        {
-            if (!Beatmap.Value.Track.Looping && !Beatmap.Disabled)
-                musicController.NextTrack();
-        });
-
         #endregion
 
         private ScheduledDelegate performFromMainMenuTask;
@@ -588,7 +591,7 @@ namespace osu.Game
 
             loadComponentSingleFile(new OnScreenDisplay(), Add, true);
 
-            loadComponentSingleFile(musicController = new MusicController(), Add, true);
+            loadComponentSingleFile(MusicController = new MusicController(), Add, true);
 
             loadComponentSingleFile(notifications = new NotificationOverlay
             {
@@ -897,7 +900,7 @@ namespace osu.Game
 
         private ScalingContainer screenContainer;
 
-        private MusicController musicController;
+        protected MusicController MusicController { get; private set; }
 
         protected override bool OnExiting()
         {
@@ -955,7 +958,7 @@ namespace osu.Game
             {
                 OverlayActivationMode.Value = newOsuScreen.InitialOverlayActivationMode;
 
-                musicController.AllowRateAdjustments = newOsuScreen.AllowRateAdjustments;
+                MusicController.AllowRateAdjustments = newOsuScreen.AllowRateAdjustments;
 
                 if (newOsuScreen.HideOverlaysOnEnter)
                     CloseAllOverlays();

osu.Game/OsuGameBase.cs

@@ -211,6 +211,10 @@ namespace osu.Game
             Audio.Tracks.AddAdjustment(AdjustableProperty.Volume, new BindableDouble(0.8));
 
             Beatmap = new NonNullableBindable<WorkingBeatmap>(defaultBeatmap);
+
+            // ScheduleAfterChildren is safety against something in the current frame accessing the previous beatmap's track
+            // and potentially causing a reload of it after just unloading.
+            // Note that the reason for this being added *has* been resolved, so it may be feasible to removed this if required.
             Beatmap.BindValueChanged(b => ScheduleAfterChildren(() =>
             {
                 // compare to last beatmap as sometimes the two may share a track representation (optimisation, see WorkingBeatmap.TransferTo)