Make WorkingBeatmap non-disposable

osu.Game.Tests/WaveformTestBeatmap.cs

@@ -37,9 +37,9 @@ namespace osu.Game.Tests
             trackStore = audioManager.GetTrackStore(getZipReader());
         }
 
-        protected override void Dispose(bool isDisposing)
+        ~WaveformTestBeatmap()
         {
-            base.Dispose(isDisposing);
+            // Remove the track store from the audio manager
             trackStore?.Dispose();
         }
 

osu.Game/Beatmaps/WorkingBeatmap.cs

@@ -17,10 +17,11 @@ using osu.Game.Rulesets.Objects.Types;
 using osu.Game.Rulesets.UI;
 using osu.Game.Skinning;
 using osu.Framework.Graphics.Video;
+using osu.Framework.Logging;
 
 namespace osu.Game.Beatmaps
 {
-    public abstract class WorkingBeatmap : IWorkingBeatmap, IDisposable
+    public abstract class WorkingBeatmap : IWorkingBeatmap
     {
         public readonly BeatmapInfo BeatmapInfo;
 
@@ -133,11 +134,29 @@ namespace osu.Game.Beatmaps
             return converted;
         }
 
-        public override string ToString() => BeatmapInfo.ToString();
+        private CancellationTokenSource loadCancellation = new CancellationTokenSource();
 
-        public bool BeatmapLoaded => beatmapLoadTask?.IsCompleted ?? false;
+        /// <summary>
+        /// Beings loading the contents of this <see cref="WorkingBeatmap"/> asynchronously.
+        /// </summary>
+        public void BeginAsyncLoad()
+        {
+            loadBeatmapAsync();
+        }
 
-        public Task<IBeatmap> LoadBeatmapAsync() => beatmapLoadTask ??= Task.Factory.StartNew(() =>
+        /// <summary>
+        /// Cancels the asynchronous loading of the contents of this <see cref="WorkingBeatmap"/>.
+        /// </summary>
+        public void CancelAsyncLoad()
+        {
+            loadCancellation?.Cancel();
+            loadCancellation = new CancellationTokenSource();
+
+            if (beatmapLoadTask?.IsCompleted != true)
+                beatmapLoadTask = null;
+        }
+
+        private Task<IBeatmap> loadBeatmapAsync() => beatmapLoadTask ??= Task.Factory.StartNew(() =>
         {
             // Todo: Handle cancellation during beatmap parsing
             var b = GetBeatmap() ?? new Beatmap();
@@ -149,7 +168,11 @@ namespace osu.Game.Beatmaps
             b.BeatmapInfo = BeatmapInfo;
 
             return b;
-        }, beatmapCancellation.Token, TaskCreationOptions.LongRunning, TaskScheduler.Default);
+        }, loadCancellation.Token, TaskCreationOptions.LongRunning, TaskScheduler.Default);
+
+        public override string ToString() => BeatmapInfo.ToString();
+
+        public bool BeatmapLoaded => beatmapLoadTask?.IsCompleted ?? false;
 
         public IBeatmap Beatmap
         {
@@ -157,7 +180,7 @@ namespace osu.Game.Beatmaps
             {
                 try
                 {
-                    return LoadBeatmapAsync().Result;
+                    return loadBeatmapAsync().Result;
                 }
                 catch (AggregateException ae)
                 {
@@ -174,7 +197,6 @@ namespace osu.Game.Beatmaps
             }
         }
 
-        private readonly CancellationTokenSource beatmapCancellation = new CancellationTokenSource();
         protected abstract IBeatmap GetBeatmap();
         private Task<IBeatmap> beatmapLoadTask;
 
@@ -225,40 +247,11 @@ namespace osu.Game.Beatmaps
         /// </summary>
         public virtual void RecycleTrack() => track.Recycle();
 
-        #region Disposal
-
-        public void Dispose()
-        {
-            Dispose(true);
-            GC.SuppressFinalize(this);
-        }
-
-        private bool isDisposed;
-
-        protected virtual void Dispose(bool isDisposing)
-        {
-            if (isDisposed)
-                return;
-
-            isDisposed = true;
-
-            // recycling logic is not here for the time being, as components which use
-            // retrieved objects from WorkingBeatmap may not hold a reference to the WorkingBeatmap itself.
-            // this should be fine as each retrieved component do have their own finalizers.
-
-            // cancelling the beatmap load is safe for now since the retrieval is a synchronous
-            // operation. if we add an async retrieval method this may need to be reconsidered.
-            beatmapCancellation?.Cancel();
-            total_count.Value--;
-        }
-
         ~WorkingBeatmap()
         {
-            Dispose(false);
+            total_count.Value--;
         }
 
-        #endregion
-
         public class RecyclableLazy<T>
         {
             private Lazy<T> lazy;

osu.Game/OsuGame.cs

@@ -401,15 +401,14 @@ namespace osu.Game
             if (nextBeatmap?.Track != null)
                 nextBeatmap.Track.Completed += currentTrackCompleted;
 
-            using (var oldBeatmap = beatmap.OldValue)
-            {
-                if (oldBeatmap?.Track != null)
-                    oldBeatmap.Track.Completed -= currentTrackCompleted;
-            }
+            var oldBeatmap = beatmap.OldValue;
+            if (oldBeatmap?.Track != null)
+                oldBeatmap.Track.Completed -= currentTrackCompleted;
 
             updateModDefaults();
 
-            nextBeatmap?.LoadBeatmapAsync();
+            oldBeatmap?.CancelAsyncLoad();
+            nextBeatmap?.BeginAsyncLoad();
         }
 
         private void modsChanged(ValueChangedEvent<IReadOnlyList<Mod>> mods)

osu.Game/Tests/Visual/OsuTestScene.cs

@@ -191,9 +191,9 @@ namespace osu.Game.Tests.Visual
                     track = audio?.Tracks.GetVirtual(length);
             }
 
-            protected override void Dispose(bool isDisposing)
+            ~ClockBackedTestWorkingBeatmap()
             {
-                base.Dispose(isDisposing);
+                // Remove the track store from the audio manager
                 store?.Dispose();
             }