Clean up cancellation handling in `WorkingBeatmap`

After the recent changes introducing cancellation support to
`WorkingBeatmap`, it turned out that if the cancellation support was
used, `GetPlayableBeatmap()` would raise timeout exceptions rather than
the expected `OperationCanceledException`.

To that end, split off a separate overload for the typical usage, that
catches `OperationCanceledException` and converts them to beatmap load
timeout exceptions, and use normal `OperationCanceledException`s in the
overload that requires a cancellation token to work.

osu.Game.Tests/Beatmaps/WorkingBeatmapTest.cs

@@ -9,6 +9,7 @@
 using NUnit.Framework;
 using osu.Game.Beatmaps;
 using osu.Game.Rulesets;
+using osu.Game.Rulesets.Mods;
 using osu.Game.Rulesets.Objects;
 using osu.Game.Rulesets.Osu;
 using osu.Game.Rulesets.Osu.Beatmaps;
@@ -40,7 +41,7 @@ public void TestGetPlayableCancellationToken()
             Task.Factory.StartNew(() =>
             {
                 loadStarted.Set();
-                Assert.Throws<OperationCanceledException>(() => working.GetPlayableBeatmap(new OsuRuleset().RulesetInfo, cancellationToken: cts.Token));
+                Assert.Throws<OperationCanceledException>(() => working.GetPlayableBeatmap(new OsuRuleset().RulesetInfo, Array.Empty<Mod>(), cts.Token));
                 loadCompleted.Set();
             }, TaskCreationOptions.LongRunning);
 
@@ -58,7 +59,7 @@ public void TestGetPlayableDefaultTimeout()
         {
             var working = new TestNeverLoadsWorkingBeatmap();
 
-            Assert.Throws<OperationCanceledException>(() => working.GetPlayableBeatmap(new OsuRuleset().RulesetInfo));
+            Assert.Throws(Is.InstanceOf<TimeoutException>(), () => working.GetPlayableBeatmap(new OsuRuleset().RulesetInfo));
 
             working.ResetEvent.Set();
         }

osu.Game/Beatmaps/IWorkingBeatmap.cs

@@ -90,12 +90,31 @@ public interface IWorkingBeatmap
         /// have been applied, and <see cref="HitObject"/>s have been fully constructed.
         /// </para>
         /// </summary>
+        /// <remarks>
+        /// By default, the beatmap load process will be interrupted after 10 seconds.
+        /// For finer-grained control over the load process, use the
+        /// <see cref="GetPlayableBeatmap(osu.Game.Rulesets.IRulesetInfo,System.Collections.Generic.IReadOnlyList{osu.Game.Rulesets.Mods.Mod},System.Threading.CancellationToken)"/>
+        /// overload instead.
+        /// </remarks>
         /// <param name="ruleset">The <see cref="RulesetInfo"/> to create a playable <see cref="IBeatmap"/> for.</param>
         /// <param name="mods">The <see cref="Mod"/>s to apply to the <see cref="IBeatmap"/>.</param>
-        /// <param name="cancellationToken">Cancellation token that cancels the beatmap loading process. If not provided, a default timeout of 10,000ms will be applied to the load process.</param>
         /// <returns>The converted <see cref="IBeatmap"/>.</returns>
         /// <exception cref="BeatmapInvalidForRulesetException">If <see cref="Beatmap"/> could not be converted to <paramref name="ruleset"/>.</exception>
-        IBeatmap GetPlayableBeatmap(IRulesetInfo ruleset, IReadOnlyList<Mod> mods = null, CancellationToken? cancellationToken = null);
+        IBeatmap GetPlayableBeatmap(IRulesetInfo ruleset, params Mod[] mods);
+
+        /// <summary>
+        /// Constructs a playable <see cref="IBeatmap"/> from <see cref="Beatmap"/> using the applicable converters for a specific <see cref="RulesetInfo"/>.
+        /// <para>
+        /// The returned <see cref="IBeatmap"/> is in a playable state - all <see cref="HitObject"/> and <see cref="BeatmapDifficulty"/> <see cref="Mod"/>s
+        /// have been applied, and <see cref="HitObject"/>s have been fully constructed.
+        /// </para>
+        /// </summary>
+        /// <param name="ruleset">The <see cref="RulesetInfo"/> to create a playable <see cref="IBeatmap"/> for.</param>
+        /// <param name="mods">The <see cref="Mod"/>s to apply to the <see cref="IBeatmap"/>.</param>
+        /// <param name="cancellationToken">Cancellation token that cancels the beatmap loading process.</param>
+        /// <returns>The converted <see cref="IBeatmap"/>.</returns>
+        /// <exception cref="BeatmapInvalidForRulesetException">If <see cref="Beatmap"/> could not be converted to <paramref name="ruleset"/>.</exception>
+        IBeatmap GetPlayableBeatmap(IRulesetInfo ruleset, IReadOnlyList<Mod> mods, CancellationToken cancellationToken);
 
         /// <summary>
         /// Load a new audio track instance for this beatmap. This should be called once before accessing <see cref="Track"/>.

osu.Game/Beatmaps/WorkingBeatmap.cs

@@ -79,14 +79,24 @@ protected virtual Track GetVirtualTrack(double emptyLength = 0)
         /// <returns>The applicable <see cref="IBeatmapConverter"/>.</returns>
         protected virtual IBeatmapConverter CreateBeatmapConverter(IBeatmap beatmap, Ruleset ruleset) => ruleset.CreateBeatmapConverter(beatmap);
 
-        public virtual IBeatmap GetPlayableBeatmap(IRulesetInfo ruleset, IReadOnlyList<Mod> mods = null, CancellationToken? cancellationToken = null)
+        public IBeatmap GetPlayableBeatmap([NotNull] IRulesetInfo ruleset, params Mod[] mods)
         {
-            var token = cancellationToken ??
+            try
-                        // don't apply the default timeout when debugger is attached (may be breakpointing / debugging).
+            {
-                        (Debugger.IsAttached ? new CancellationToken() : new CancellationTokenSource(10000).Token);
+                using (var cancellationTokenSource = new CancellationTokenSource(10_000))
-
+                {
-            mods ??= Array.Empty<Mod>();
+                    // don't apply the default timeout when debugger is attached (may be breakpointing / debugging).
+                    return GetPlayableBeatmap(ruleset, mods, Debugger.IsAttached ? new CancellationToken() : cancellationTokenSource.Token);
+                }
+            }
+            catch (OperationCanceledException)
+            {
+                throw new BeatmapLoadTimeoutException(BeatmapInfo);
+            }
+        }
 
+        public virtual IBeatmap GetPlayableBeatmap([NotNull] IRulesetInfo ruleset, [NotNull] IReadOnlyList<Mod> mods, CancellationToken token)
+        {
             var rulesetInstance = ruleset.CreateInstance();
 
             if (rulesetInstance == null)
@@ -101,9 +111,7 @@ public virtual IBeatmap GetPlayableBeatmap(IRulesetInfo ruleset, IReadOnlyList<M
             // Apply conversion mods
             foreach (var mod in mods.OfType<IApplicableToBeatmapConverter>())
             {
-                if (token.IsCancellationRequested)
+                token.ThrowIfCancellationRequested();
-                    throw new BeatmapLoadTimeoutException(BeatmapInfo);
-
                 mod.ApplyToBeatmapConverter(converter);
             }
 
@@ -113,9 +121,7 @@ public virtual IBeatmap GetPlayableBeatmap(IRulesetInfo ruleset, IReadOnlyList<M
             // Apply conversion mods to the result
             foreach (var mod in mods.OfType<IApplicableAfterBeatmapConversion>())
             {
-                if (token.IsCancellationRequested)
+                token.ThrowIfCancellationRequested();
-                    throw new BeatmapLoadTimeoutException(BeatmapInfo);
-
                 mod.ApplyToBeatmap(converted);
             }
 
@@ -124,9 +130,7 @@ public virtual IBeatmap GetPlayableBeatmap(IRulesetInfo ruleset, IReadOnlyList<M
             {
                 foreach (var mod in mods.OfType<IApplicableToDifficulty>())
                 {
-                    if (token.IsCancellationRequested)
+                    token.ThrowIfCancellationRequested();
-                        throw new BeatmapLoadTimeoutException(BeatmapInfo);
-
                     mod.ApplyToDifficulty(converted.Difficulty);
                 }
             }
@@ -139,28 +143,17 @@ public virtual IBeatmap GetPlayableBeatmap(IRulesetInfo ruleset, IReadOnlyList<M
             processor?.PreProcess();
 
             // Compute default values for hitobjects, including creating nested hitobjects in-case they're needed
-            try
+            foreach (var obj in converted.HitObjects)
             {
-                foreach (var obj in converted.HitObjects)
+                token.ThrowIfCancellationRequested();
-                {
+                obj.ApplyDefaults(converted.ControlPointInfo, converted.Difficulty, token);
-                    if (token.IsCancellationRequested)
-                        throw new BeatmapLoadTimeoutException(BeatmapInfo);
-
-                    obj.ApplyDefaults(converted.ControlPointInfo, converted.Difficulty, token);
-                }
-            }
-            catch (OperationCanceledException)
-            {
-                throw new BeatmapLoadTimeoutException(BeatmapInfo);
             }
 
             foreach (var mod in mods.OfType<IApplicableToHitObject>())
             {
                 foreach (var obj in converted.HitObjects)
                 {
-                    if (token.IsCancellationRequested)
+                    token.ThrowIfCancellationRequested();
-                        throw new BeatmapLoadTimeoutException(BeatmapInfo);
-
                     mod.ApplyToHitObject(obj);
                 }
             }

osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs

@@ -216,7 +216,7 @@ public GameplayWorkingBeatmap(IBeatmap gameplayBeatmap)
                 this.gameplayBeatmap = gameplayBeatmap;
             }
 
-            public override IBeatmap GetPlayableBeatmap(IRulesetInfo ruleset, IReadOnlyList<Mod> mods = null, CancellationToken? cancellationToken = null)
+            public override IBeatmap GetPlayableBeatmap(IRulesetInfo ruleset, IReadOnlyList<Mod> mods, CancellationToken cancellationToken)
                 => gameplayBeatmap;
 
             protected override IBeatmap GetBeatmap() => gameplayBeatmap;