From f0caa1006631c08c9f15e665e865b3108543a64c Mon Sep 17 00:00:00 2001 From: Tollii Date: Fri, 5 Nov 2021 23:53:48 +0100 Subject: [PATCH 01/14] Add support for a provided cancellation token for GetPlayableBeatmap() --- osu.Game/Beatmaps/IWorkingBeatmap.cs | 4 +++- osu.Game/Beatmaps/WorkingBeatmap.cs | 7 ++++--- osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/osu.Game/Beatmaps/IWorkingBeatmap.cs b/osu.Game/Beatmaps/IWorkingBeatmap.cs index a916b37b85..6f0f408060 100644 --- a/osu.Game/Beatmaps/IWorkingBeatmap.cs +++ b/osu.Game/Beatmaps/IWorkingBeatmap.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Threading; using osu.Framework.Audio.Track; using osu.Framework.Graphics.Textures; using osu.Game.Rulesets; @@ -57,9 +58,10 @@ namespace osu.Game.Beatmaps /// The to create a playable for. /// The s to apply to the . /// The maximum length in milliseconds to wait for load to complete. Defaults to 10,000ms. + /// Externally provided cancellation token. /// The converted . /// If could not be converted to . - IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, TimeSpan? timeout = null); + IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, TimeSpan? timeout = null, CancellationToken cancellationToken = default); /// /// Load a new audio track instance for this beatmap. This should be called once before accessing . diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index d2c0f7de0f..affc19c700 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -81,7 +81,7 @@ namespace osu.Game.Beatmaps /// The applicable . protected virtual IBeatmapConverter CreateBeatmapConverter(IBeatmap beatmap, Ruleset ruleset) => ruleset.CreateBeatmapConverter(beatmap); - public virtual IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, TimeSpan? timeout = null) + public virtual IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, TimeSpan? timeout = null, CancellationToken cancellationToken = default) { using (var cancellationSource = createCancellationTokenSource(timeout)) { @@ -105,7 +105,7 @@ namespace osu.Game.Beatmaps } // Convert - IBeatmap converted = converter.Convert(cancellationSource.Token); + IBeatmap converted = converter.Convert(cancellationToken != CancellationToken.None ? cancellationToken : cancellationSource.Token); // Apply conversion mods to the result foreach (var mod in mods.OfType()) @@ -143,7 +143,7 @@ namespace osu.Game.Beatmaps if (cancellationSource.IsCancellationRequested) throw new BeatmapLoadTimeoutException(BeatmapInfo); - obj.ApplyDefaults(converted.ControlPointInfo, converted.Difficulty, cancellationSource.Token); + obj.ApplyDefaults(converted.ControlPointInfo, converted.Difficulty, cancellationToken != CancellationToken.None ? cancellationToken : cancellationSource.Token); } } catch (OperationCanceledException) @@ -167,6 +167,7 @@ namespace osu.Game.Beatmaps foreach (var mod in mods.OfType()) { cancellationSource.Token.ThrowIfCancellationRequested(); + cancellationToken.ThrowIfCancellationRequested(); mod.ApplyToBeatmap(converted); } diff --git a/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs b/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs index ef289c2a20..922d1e6bbe 100644 --- a/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs +++ b/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs @@ -210,7 +210,7 @@ namespace osu.Game.Screens.Play.HUD this.gameplayBeatmap = gameplayBeatmap; } - public override IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, TimeSpan? timeout = null) + public override IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, TimeSpan? timeout = null, CancellationToken cancellationToken = default) => gameplayBeatmap; protected override IBeatmap GetBeatmap() => gameplayBeatmap; From eb7d04bc77da85ea95461828e0988bd1668d2e58 Mon Sep 17 00:00:00 2001 From: Tollii Date: Sat, 6 Nov 2021 00:19:48 +0100 Subject: [PATCH 02/14] Add cancellation token support for beatmap difficulty calculation. --- osu.Game/Beatmaps/BeatmapDifficultyCache.cs | 9 +++++---- osu.Game/Beatmaps/BeatmapModelManager.cs | 2 +- .../Difficulty/DifficultyCalculator.cs | 18 +++++++++++------- osu.Game/Stores/BeatmapImporter.cs | 2 +- 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs index 035f438b89..bbe5ecb0df 100644 --- a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs +++ b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs @@ -147,13 +147,13 @@ namespace osu.Game.Beatmaps if (CheckExists(lookup, out var existing)) return existing; - return computeDifficulty(lookup); + return computeDifficulty(lookup, token); }, token, TaskCreationOptions.HideScheduler | TaskCreationOptions.RunContinuationsAsynchronously, updateScheduler); } public Task> GetTimedDifficultyAttributesAsync(WorkingBeatmap beatmap, Ruleset ruleset, Mod[] mods, CancellationToken token = default) { - return Task.Factory.StartNew(() => ruleset.CreateDifficultyCalculator(beatmap).CalculateTimed(mods), + return Task.Factory.StartNew(() => ruleset.CreateDifficultyCalculator(beatmap).CalculateTimed(mods, token), token, TaskCreationOptions.HideScheduler | TaskCreationOptions.RunContinuationsAsynchronously, updateScheduler); @@ -270,8 +270,9 @@ namespace osu.Game.Beatmaps /// Computes the difficulty defined by a key, and stores it to the timed cache. /// /// The that defines the computation parameters. + /// The cancellation token. /// The . - private StarDifficulty computeDifficulty(in DifficultyCacheLookup key) + private StarDifficulty computeDifficulty(in DifficultyCacheLookup key, CancellationToken cancellationToken = default) { // In the case that the user hasn't given us a ruleset, use the beatmap's default ruleset. var beatmapInfo = key.BeatmapInfo; @@ -283,7 +284,7 @@ namespace osu.Game.Beatmaps Debug.Assert(ruleset != null); var calculator = ruleset.CreateDifficultyCalculator(beatmapManager.GetWorkingBeatmap(key.BeatmapInfo)); - var attributes = calculator.Calculate(key.OrderedMods); + var attributes = calculator.Calculate(key.OrderedMods, cancellationToken); return new StarDifficulty(attributes); } diff --git a/osu.Game/Beatmaps/BeatmapModelManager.cs b/osu.Game/Beatmaps/BeatmapModelManager.cs index f148d05aca..2166ae1dfc 100644 --- a/osu.Game/Beatmaps/BeatmapModelManager.cs +++ b/osu.Game/Beatmaps/BeatmapModelManager.cs @@ -408,7 +408,7 @@ namespace osu.Game.Beatmaps beatmap.BeatmapInfo.Ruleset = ruleset; // TODO: this should be done in a better place once we actually need to dynamically update it. - beatmap.BeatmapInfo.StarDifficulty = ruleset?.CreateInstance().CreateDifficultyCalculator(new DummyConversionBeatmap(beatmap)).Calculate().StarRating ?? 0; + beatmap.BeatmapInfo.StarDifficulty = ruleset?.CreateInstance().CreateDifficultyCalculator(new DummyConversionBeatmap(beatmap)).Calculate(null).StarRating ?? 0; beatmap.BeatmapInfo.Length = calculateLength(beatmap); beatmap.BeatmapInfo.BPM = 60000 / beatmap.GetMostCommonBeatLength(); diff --git a/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs b/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs index 5b4284dc2f..4fc984b491 100644 --- a/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs +++ b/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading; using osu.Framework.Audio.Track; using osu.Framework.Extensions.IEnumerableExtensions; using osu.Game.Beatmaps; @@ -39,10 +40,11 @@ namespace osu.Game.Rulesets.Difficulty /// Calculates the difficulty of the beatmap using a specific mod combination. /// /// The mods that should be applied to the beatmap. + /// The cancellation token. /// A structure describing the difficulty of the beatmap. - public DifficultyAttributes Calculate(params Mod[] mods) + public DifficultyAttributes Calculate(IEnumerable mods, CancellationToken cancellationToken = default) { - preProcess(mods); + preProcess(mods, cancellationToken); var skills = CreateSkills(Beatmap, playableMods, clockRate); @@ -62,10 +64,11 @@ namespace osu.Game.Rulesets.Difficulty /// Calculates the difficulty of the beatmap and returns a set of representing the difficulty at every relevant time value in the beatmap. /// /// The mods that should be applied to the beatmap. + /// The cancellation token. /// The set of . - public List CalculateTimed(params Mod[] mods) + public List CalculateTimed(IEnumerable mods, CancellationToken cancellationToken = default) { - preProcess(mods); + preProcess(mods, cancellationToken); var attribs = new List(); @@ -99,7 +102,7 @@ namespace osu.Game.Rulesets.Difficulty if (combination is MultiMod multi) yield return Calculate(multi.Mods); else - yield return Calculate(combination); + yield return Calculate(new[] { combination }); } } @@ -112,11 +115,12 @@ namespace osu.Game.Rulesets.Difficulty /// Performs required tasks before every calculation. /// /// The original list of s. - private void preProcess(Mod[] mods) + /// The cancellation cancellationToken. + private void preProcess(IEnumerable mods, CancellationToken cancellationToken = default) { playableMods = mods.Select(m => m.DeepClone()).ToArray(); - Beatmap = beatmap.GetPlayableBeatmap(ruleset.RulesetInfo, playableMods); + Beatmap = beatmap.GetPlayableBeatmap(ruleset.RulesetInfo, playableMods, cancellationToken: cancellationToken); var track = new TrackVirtual(10000); playableMods.OfType().ForEach(m => m.ApplyToTrack(track)); diff --git a/osu.Game/Stores/BeatmapImporter.cs b/osu.Game/Stores/BeatmapImporter.cs index 787b1ddd60..7916583ce1 100644 --- a/osu.Game/Stores/BeatmapImporter.cs +++ b/osu.Game/Stores/BeatmapImporter.cs @@ -284,7 +284,7 @@ namespace osu.Game.Stores decoded.BeatmapInfo.Ruleset = rulesetInstance.RulesetInfo; // TODO: this should be done in a better place once we actually need to dynamically update it. - beatmap.StarRating = rulesetInstance.CreateDifficultyCalculator(new DummyConversionBeatmap(decoded)).Calculate().StarRating; + beatmap.StarRating = rulesetInstance.CreateDifficultyCalculator(new DummyConversionBeatmap(decoded)).Calculate(null).StarRating; beatmap.Length = calculateLength(decoded); beatmap.BPM = 60000 / decoded.GetMostCommonBeatLength(); } From cf0b757b16f8369efd82968b6357aafe120e8549 Mon Sep 17 00:00:00 2001 From: Tollii Date: Sat, 6 Nov 2021 16:03:53 +0100 Subject: [PATCH 03/14] Fix PR comments. Nitpick, more cancellation token checks. --- osu.Game/Beatmaps/BeatmapModelManager.cs | 2 +- osu.Game/Beatmaps/IWorkingBeatmap.cs | 4 ++-- osu.Game/Beatmaps/WorkingBeatmap.cs | 24 +++++++++---------- .../Difficulty/DifficultyCalculator.cs | 18 ++++++++++---- .../Play/HUD/PerformancePointsCounter.cs | 2 +- osu.Game/Stores/BeatmapImporter.cs | 2 +- 6 files changed, 30 insertions(+), 22 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapModelManager.cs b/osu.Game/Beatmaps/BeatmapModelManager.cs index 2166ae1dfc..f148d05aca 100644 --- a/osu.Game/Beatmaps/BeatmapModelManager.cs +++ b/osu.Game/Beatmaps/BeatmapModelManager.cs @@ -408,7 +408,7 @@ namespace osu.Game.Beatmaps beatmap.BeatmapInfo.Ruleset = ruleset; // TODO: this should be done in a better place once we actually need to dynamically update it. - beatmap.BeatmapInfo.StarDifficulty = ruleset?.CreateInstance().CreateDifficultyCalculator(new DummyConversionBeatmap(beatmap)).Calculate(null).StarRating ?? 0; + beatmap.BeatmapInfo.StarDifficulty = ruleset?.CreateInstance().CreateDifficultyCalculator(new DummyConversionBeatmap(beatmap)).Calculate().StarRating ?? 0; beatmap.BeatmapInfo.Length = calculateLength(beatmap); beatmap.BeatmapInfo.BPM = 60000 / beatmap.GetMostCommonBeatLength(); diff --git a/osu.Game/Beatmaps/IWorkingBeatmap.cs b/osu.Game/Beatmaps/IWorkingBeatmap.cs index 6f0f408060..3bbfb2a889 100644 --- a/osu.Game/Beatmaps/IWorkingBeatmap.cs +++ b/osu.Game/Beatmaps/IWorkingBeatmap.cs @@ -58,10 +58,10 @@ namespace osu.Game.Beatmaps /// The to create a playable for. /// The s to apply to the . /// The maximum length in milliseconds to wait for load to complete. Defaults to 10,000ms. - /// Externally provided cancellation token. + /// Cancellation token that cancels the beatmap loading process. /// The converted . /// If could not be converted to . - IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, TimeSpan? timeout = null, CancellationToken cancellationToken = default); + IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, TimeSpan? timeout = null, CancellationToken timeoutToken = default); /// /// Load a new audio track instance for this beatmap. This should be called once before accessing . diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index affc19c700..e0c2ae7873 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -81,9 +81,10 @@ namespace osu.Game.Beatmaps /// The applicable . protected virtual IBeatmapConverter CreateBeatmapConverter(IBeatmap beatmap, Ruleset ruleset) => ruleset.CreateBeatmapConverter(beatmap); - public virtual IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, TimeSpan? timeout = null, CancellationToken cancellationToken = default) + public virtual IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, TimeSpan? timeout = null, CancellationToken timeoutToken = default) { - using (var cancellationSource = createCancellationTokenSource(timeout)) + using (var timeoutSource = createTimeoutTokenSource(timeout)) + using (var linkedTokenSource = CancellationTokenSource.CreateLinkedTokenSource(timeoutToken, timeoutSource.Token)) { mods ??= Array.Empty(); @@ -98,19 +99,19 @@ namespace osu.Game.Beatmaps // Apply conversion mods foreach (var mod in mods.OfType()) { - if (cancellationSource.IsCancellationRequested) + if (linkedTokenSource.IsCancellationRequested) throw new BeatmapLoadTimeoutException(BeatmapInfo); mod.ApplyToBeatmapConverter(converter); } // Convert - IBeatmap converted = converter.Convert(cancellationToken != CancellationToken.None ? cancellationToken : cancellationSource.Token); + IBeatmap converted = converter.Convert(linkedTokenSource.Token); // Apply conversion mods to the result foreach (var mod in mods.OfType()) { - if (cancellationSource.IsCancellationRequested) + if (linkedTokenSource.IsCancellationRequested) throw new BeatmapLoadTimeoutException(BeatmapInfo); mod.ApplyToBeatmap(converted); @@ -121,7 +122,7 @@ namespace osu.Game.Beatmaps { foreach (var mod in mods.OfType()) { - if (cancellationSource.IsCancellationRequested) + if (linkedTokenSource.IsCancellationRequested) throw new BeatmapLoadTimeoutException(BeatmapInfo); mod.ApplyToDifficulty(converted.Difficulty); @@ -140,10 +141,10 @@ namespace osu.Game.Beatmaps { foreach (var obj in converted.HitObjects) { - if (cancellationSource.IsCancellationRequested) + if (linkedTokenSource.IsCancellationRequested) throw new BeatmapLoadTimeoutException(BeatmapInfo); - obj.ApplyDefaults(converted.ControlPointInfo, converted.Difficulty, cancellationToken != CancellationToken.None ? cancellationToken : cancellationSource.Token); + obj.ApplyDefaults(converted.ControlPointInfo, converted.Difficulty, linkedTokenSource.Token); } } catch (OperationCanceledException) @@ -155,7 +156,7 @@ namespace osu.Game.Beatmaps { foreach (var obj in converted.HitObjects) { - if (cancellationSource.IsCancellationRequested) + if (linkedTokenSource.IsCancellationRequested) throw new BeatmapLoadTimeoutException(BeatmapInfo); mod.ApplyToHitObject(obj); @@ -166,8 +167,7 @@ namespace osu.Game.Beatmaps foreach (var mod in mods.OfType()) { - cancellationSource.Token.ThrowIfCancellationRequested(); - cancellationToken.ThrowIfCancellationRequested(); + linkedTokenSource.Token.ThrowIfCancellationRequested(); mod.ApplyToBeatmap(converted); } @@ -200,7 +200,7 @@ namespace osu.Game.Beatmaps } } - private CancellationTokenSource createCancellationTokenSource(TimeSpan? timeout) + private CancellationTokenSource createTimeoutTokenSource(TimeSpan? timeout) { if (Debugger.IsAttached) // ignore timeout when debugger is attached (may be breakpointing / debugging). diff --git a/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs b/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs index 4fc984b491..ffad6131be 100644 --- a/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs +++ b/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs @@ -42,8 +42,9 @@ namespace osu.Game.Rulesets.Difficulty /// The mods that should be applied to the beatmap. /// The cancellation token. /// A structure describing the difficulty of the beatmap. - public DifficultyAttributes Calculate(IEnumerable mods, CancellationToken cancellationToken = default) + public DifficultyAttributes Calculate(IEnumerable mods = null, CancellationToken cancellationToken = default) { + cancellationToken.ThrowIfCancellationRequested(); preProcess(mods, cancellationToken); var skills = CreateSkills(Beatmap, playableMods, clockRate); @@ -54,7 +55,10 @@ namespace osu.Game.Rulesets.Difficulty foreach (var hitObject in getDifficultyHitObjects()) { foreach (var skill in skills) + { + cancellationToken.ThrowIfCancellationRequested(); skill.ProcessInternal(hitObject); + } } return CreateDifficultyAttributes(Beatmap, playableMods, skills, clockRate); @@ -68,6 +72,7 @@ namespace osu.Game.Rulesets.Difficulty /// The set of . public List CalculateTimed(IEnumerable mods, CancellationToken cancellationToken = default) { + cancellationToken.ThrowIfCancellationRequested(); preProcess(mods, cancellationToken); var attribs = new List(); @@ -83,7 +88,10 @@ namespace osu.Game.Rulesets.Difficulty progressiveBeatmap.HitObjects.Add(hitObject.BaseObject); foreach (var skill in skills) + { + cancellationToken.ThrowIfCancellationRequested(); skill.ProcessInternal(hitObject); + } attribs.Add(new TimedDifficultyAttributes(hitObject.EndTime * clockRate, CreateDifficultyAttributes(progressiveBeatmap, playableMods, skills, clockRate))); } @@ -102,7 +110,7 @@ namespace osu.Game.Rulesets.Difficulty if (combination is MultiMod multi) yield return Calculate(multi.Mods); else - yield return Calculate(new[] { combination }); + yield return Calculate(combination.Yield()); } } @@ -115,12 +123,12 @@ namespace osu.Game.Rulesets.Difficulty /// Performs required tasks before every calculation. /// /// The original list of s. - /// The cancellation cancellationToken. - private void preProcess(IEnumerable mods, CancellationToken cancellationToken = default) + /// The cancellation timeoutToken. + private void preProcess(IEnumerable mods = null, CancellationToken cancellationToken = default) { playableMods = mods.Select(m => m.DeepClone()).ToArray(); - Beatmap = beatmap.GetPlayableBeatmap(ruleset.RulesetInfo, playableMods, cancellationToken: cancellationToken); + Beatmap = beatmap.GetPlayableBeatmap(ruleset.RulesetInfo, playableMods, timeoutToken: cancellationToken); var track = new TrackVirtual(10000); playableMods.OfType().ForEach(m => m.ApplyToTrack(track)); diff --git a/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs b/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs index 922d1e6bbe..bd06e71304 100644 --- a/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs +++ b/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs @@ -210,7 +210,7 @@ namespace osu.Game.Screens.Play.HUD this.gameplayBeatmap = gameplayBeatmap; } - public override IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, TimeSpan? timeout = null, CancellationToken cancellationToken = default) + public override IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, TimeSpan? timeout = null, CancellationToken timeoutToken = default) => gameplayBeatmap; protected override IBeatmap GetBeatmap() => gameplayBeatmap; diff --git a/osu.Game/Stores/BeatmapImporter.cs b/osu.Game/Stores/BeatmapImporter.cs index 7916583ce1..787b1ddd60 100644 --- a/osu.Game/Stores/BeatmapImporter.cs +++ b/osu.Game/Stores/BeatmapImporter.cs @@ -284,7 +284,7 @@ namespace osu.Game.Stores decoded.BeatmapInfo.Ruleset = rulesetInstance.RulesetInfo; // TODO: this should be done in a better place once we actually need to dynamically update it. - beatmap.StarRating = rulesetInstance.CreateDifficultyCalculator(new DummyConversionBeatmap(decoded)).Calculate(null).StarRating; + beatmap.StarRating = rulesetInstance.CreateDifficultyCalculator(new DummyConversionBeatmap(decoded)).Calculate().StarRating; beatmap.Length = calculateLength(decoded); beatmap.BPM = 60000 / decoded.GetMostCommonBeatLength(); } From d5f5d74a89e880d3510867bab46698455504f7cd Mon Sep 17 00:00:00 2001 From: Tollii Date: Sun, 7 Nov 2021 13:38:00 +0100 Subject: [PATCH 04/14] Rename CancellationToken variable --- osu.Game/Beatmaps/IWorkingBeatmap.cs | 4 ++-- osu.Game/Beatmaps/WorkingBeatmap.cs | 4 ++-- osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/osu.Game/Beatmaps/IWorkingBeatmap.cs b/osu.Game/Beatmaps/IWorkingBeatmap.cs index 3bbfb2a889..1e46e265ac 100644 --- a/osu.Game/Beatmaps/IWorkingBeatmap.cs +++ b/osu.Game/Beatmaps/IWorkingBeatmap.cs @@ -58,10 +58,10 @@ namespace osu.Game.Beatmaps /// The to create a playable for. /// The s to apply to the . /// The maximum length in milliseconds to wait for load to complete. Defaults to 10,000ms. - /// Cancellation token that cancels the beatmap loading process. + /// Cancellation token that cancels the beatmap loading process. /// The converted . /// If could not be converted to . - IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, TimeSpan? timeout = null, CancellationToken timeoutToken = default); + IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, TimeSpan? timeout = null, CancellationToken token = default); /// /// Load a new audio track instance for this beatmap. This should be called once before accessing . diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index e0c2ae7873..a93c50ce74 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -81,10 +81,10 @@ namespace osu.Game.Beatmaps /// The applicable . protected virtual IBeatmapConverter CreateBeatmapConverter(IBeatmap beatmap, Ruleset ruleset) => ruleset.CreateBeatmapConverter(beatmap); - public virtual IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, TimeSpan? timeout = null, CancellationToken timeoutToken = default) + public virtual IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, TimeSpan? timeout = null, CancellationToken token = default) { using (var timeoutSource = createTimeoutTokenSource(timeout)) - using (var linkedTokenSource = CancellationTokenSource.CreateLinkedTokenSource(timeoutToken, timeoutSource.Token)) + using (var linkedTokenSource = CancellationTokenSource.CreateLinkedTokenSource(token, timeoutSource.Token)) { mods ??= Array.Empty(); diff --git a/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs b/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs index ffad6131be..6630530b65 100644 --- a/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs +++ b/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs @@ -123,12 +123,12 @@ namespace osu.Game.Rulesets.Difficulty /// Performs required tasks before every calculation. /// /// The original list of s. - /// The cancellation timeoutToken. + /// The cancellation token. private void preProcess(IEnumerable mods = null, CancellationToken cancellationToken = default) { playableMods = mods.Select(m => m.DeepClone()).ToArray(); - Beatmap = beatmap.GetPlayableBeatmap(ruleset.RulesetInfo, playableMods, timeoutToken: cancellationToken); + Beatmap = beatmap.GetPlayableBeatmap(ruleset.RulesetInfo, playableMods, token: cancellationToken); var track = new TrackVirtual(10000); playableMods.OfType().ForEach(m => m.ApplyToTrack(track)); From 5b5e3dc4a2ee03256aac220341ad419d20df9e87 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 8 Nov 2021 14:33:15 +0900 Subject: [PATCH 05/14] Revert incorrect mod nullable parameter specification --- osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs b/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs index 6630530b65..a6f3a75302 100644 --- a/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs +++ b/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs @@ -124,7 +124,7 @@ namespace osu.Game.Rulesets.Difficulty /// /// The original list of s. /// The cancellation token. - private void preProcess(IEnumerable mods = null, CancellationToken cancellationToken = default) + private void preProcess(IEnumerable mods, CancellationToken cancellationToken = default) { playableMods = mods.Select(m => m.DeepClone()).ToArray(); From 97345ac9e6123c8fc3e990bc5fdc244c7ba14206 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 8 Nov 2021 14:33:32 +0900 Subject: [PATCH 06/14] Remove unnecessary `TimeSpan timeout` parameter (`CancellationToken` can now be used) --- osu.Game/Beatmaps/IWorkingBeatmap.cs | 4 +- osu.Game/Beatmaps/WorkingBeatmap.cs | 170 +++++++++--------- .../Play/HUD/PerformancePointsCounter.cs | 2 +- 3 files changed, 85 insertions(+), 91 deletions(-) diff --git a/osu.Game/Beatmaps/IWorkingBeatmap.cs b/osu.Game/Beatmaps/IWorkingBeatmap.cs index 1e46e265ac..ed100d1876 100644 --- a/osu.Game/Beatmaps/IWorkingBeatmap.cs +++ b/osu.Game/Beatmaps/IWorkingBeatmap.cs @@ -1,7 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System; using System.Collections.Generic; using System.IO; using System.Threading; @@ -57,11 +56,10 @@ namespace osu.Game.Beatmaps /// /// The to create a playable for. /// The s to apply to the . - /// The maximum length in milliseconds to wait for load to complete. Defaults to 10,000ms. /// Cancellation token that cancels the beatmap loading process. /// The converted . /// If could not be converted to . - IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, TimeSpan? timeout = null, CancellationToken token = default); + IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, CancellationToken token = default); /// /// Load a new audio track instance for this beatmap. This should be called once before accessing . diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index a93c50ce74..0527acca8f 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -81,98 +81,94 @@ namespace osu.Game.Beatmaps /// The applicable . protected virtual IBeatmapConverter CreateBeatmapConverter(IBeatmap beatmap, Ruleset ruleset) => ruleset.CreateBeatmapConverter(beatmap); - public virtual IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, TimeSpan? timeout = null, CancellationToken token = default) + public virtual IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, CancellationToken token = default) { - using (var timeoutSource = createTimeoutTokenSource(timeout)) - using (var linkedTokenSource = CancellationTokenSource.CreateLinkedTokenSource(token, timeoutSource.Token)) + mods ??= Array.Empty(); + + var rulesetInstance = ruleset.CreateInstance(); + + IBeatmapConverter converter = CreateBeatmapConverter(Beatmap, rulesetInstance); + + // Check if the beatmap can be converted + if (Beatmap.HitObjects.Count > 0 && !converter.CanConvert()) + throw new BeatmapInvalidForRulesetException($"{nameof(Beatmaps.Beatmap)} can not be converted for the ruleset (ruleset: {ruleset.InstantiationInfo}, converter: {converter})."); + + // Apply conversion mods + foreach (var mod in mods.OfType()) { - mods ??= Array.Empty(); - - var rulesetInstance = ruleset.CreateInstance(); - - IBeatmapConverter converter = CreateBeatmapConverter(Beatmap, rulesetInstance); - - // Check if the beatmap can be converted - if (Beatmap.HitObjects.Count > 0 && !converter.CanConvert()) - throw new BeatmapInvalidForRulesetException($"{nameof(Beatmaps.Beatmap)} can not be converted for the ruleset (ruleset: {ruleset.InstantiationInfo}, converter: {converter})."); - - // Apply conversion mods - foreach (var mod in mods.OfType()) - { - if (linkedTokenSource.IsCancellationRequested) - throw new BeatmapLoadTimeoutException(BeatmapInfo); - - mod.ApplyToBeatmapConverter(converter); - } - - // Convert - IBeatmap converted = converter.Convert(linkedTokenSource.Token); - - // Apply conversion mods to the result - foreach (var mod in mods.OfType()) - { - if (linkedTokenSource.IsCancellationRequested) - throw new BeatmapLoadTimeoutException(BeatmapInfo); - - mod.ApplyToBeatmap(converted); - } - - // Apply difficulty mods - if (mods.Any(m => m is IApplicableToDifficulty)) - { - foreach (var mod in mods.OfType()) - { - if (linkedTokenSource.IsCancellationRequested) - throw new BeatmapLoadTimeoutException(BeatmapInfo); - - mod.ApplyToDifficulty(converted.Difficulty); - } - } - - IBeatmapProcessor processor = rulesetInstance.CreateBeatmapProcessor(converted); - - foreach (var mod in mods.OfType()) - mod.ApplyToBeatmapProcessor(processor); - - processor?.PreProcess(); - - // Compute default values for hitobjects, including creating nested hitobjects in-case they're needed - try - { - foreach (var obj in converted.HitObjects) - { - if (linkedTokenSource.IsCancellationRequested) - throw new BeatmapLoadTimeoutException(BeatmapInfo); - - obj.ApplyDefaults(converted.ControlPointInfo, converted.Difficulty, linkedTokenSource.Token); - } - } - catch (OperationCanceledException) - { + if (token.IsCancellationRequested) throw new BeatmapLoadTimeoutException(BeatmapInfo); - } - foreach (var mod in mods.OfType()) - { - foreach (var obj in converted.HitObjects) - { - if (linkedTokenSource.IsCancellationRequested) - throw new BeatmapLoadTimeoutException(BeatmapInfo); - - mod.ApplyToHitObject(obj); - } - } - - processor?.PostProcess(); - - foreach (var mod in mods.OfType()) - { - linkedTokenSource.Token.ThrowIfCancellationRequested(); - mod.ApplyToBeatmap(converted); - } - - return converted; + mod.ApplyToBeatmapConverter(converter); } + + // Convert + IBeatmap converted = converter.Convert(token); + + // Apply conversion mods to the result + foreach (var mod in mods.OfType()) + { + if (token.IsCancellationRequested) + throw new BeatmapLoadTimeoutException(BeatmapInfo); + + mod.ApplyToBeatmap(converted); + } + + // Apply difficulty mods + if (mods.Any(m => m is IApplicableToDifficulty)) + { + foreach (var mod in mods.OfType()) + { + if (token.IsCancellationRequested) + throw new BeatmapLoadTimeoutException(BeatmapInfo); + + mod.ApplyToDifficulty(converted.Difficulty); + } + } + + IBeatmapProcessor processor = rulesetInstance.CreateBeatmapProcessor(converted); + + foreach (var mod in mods.OfType()) + mod.ApplyToBeatmapProcessor(processor); + + processor?.PreProcess(); + + // Compute default values for hitobjects, including creating nested hitobjects in-case they're needed + try + { + foreach (var obj in converted.HitObjects) + { + 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()) + { + foreach (var obj in converted.HitObjects) + { + if (token.IsCancellationRequested) + throw new BeatmapLoadTimeoutException(BeatmapInfo); + + mod.ApplyToHitObject(obj); + } + } + + processor?.PostProcess(); + + foreach (var mod in mods.OfType()) + { + token.ThrowIfCancellationRequested(); + mod.ApplyToBeatmap(converted); + } + + return converted; } private CancellationTokenSource loadCancellation = new CancellationTokenSource(); diff --git a/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs b/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs index bd06e71304..dea1964faa 100644 --- a/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs +++ b/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs @@ -210,7 +210,7 @@ namespace osu.Game.Screens.Play.HUD this.gameplayBeatmap = gameplayBeatmap; } - public override IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, TimeSpan? timeout = null, CancellationToken timeoutToken = default) + public override IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, CancellationToken timeoutToken = default) => gameplayBeatmap; protected override IBeatmap GetBeatmap() => gameplayBeatmap; From c58f21a11531860719268823ba02c2436668ccde Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 8 Nov 2021 14:43:46 +0900 Subject: [PATCH 07/14] Handle mods with overloaded method signature instead --- .../Difficulty/DifficultyCalculator.cs | 25 ++++++++++++++++--- .../Difficulty/TimedDifficultyAttributes.cs | 2 +- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs b/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs index a6f3a75302..6748158d8f 100644 --- a/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs +++ b/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Linq; using System.Threading; +using JetBrains.Annotations; using osu.Framework.Audio.Track; using osu.Framework.Extensions.IEnumerableExtensions; using osu.Game.Beatmaps; @@ -36,13 +37,21 @@ namespace osu.Game.Rulesets.Difficulty this.beatmap = beatmap; } + /// + /// Calculates the difficulty of the beatmap with no mods applied. + /// + /// The cancellation token. + /// A structure describing the difficulty of the beatmap. + public DifficultyAttributes Calculate(CancellationToken cancellationToken = default) + => Calculate(Array.Empty(), cancellationToken); + /// /// Calculates the difficulty of the beatmap using a specific mod combination. /// /// The mods that should be applied to the beatmap. /// The cancellation token. /// A structure describing the difficulty of the beatmap. - public DifficultyAttributes Calculate(IEnumerable mods = null, CancellationToken cancellationToken = default) + public DifficultyAttributes Calculate([NotNull] IEnumerable mods, CancellationToken cancellationToken = default) { cancellationToken.ThrowIfCancellationRequested(); preProcess(mods, cancellationToken); @@ -65,12 +74,20 @@ namespace osu.Game.Rulesets.Difficulty } /// - /// Calculates the difficulty of the beatmap and returns a set of representing the difficulty at every relevant time value in the beatmap. + /// Calculates the difficulty of the beatmap with no mods applied and returns a set of representing the difficulty at every relevant time value in the beatmap. + /// + /// The cancellation token. + /// The set of . + public List CalculateTimed(CancellationToken cancellationToken = default) + => CalculateTimed(Array.Empty(), cancellationToken); + + /// + /// Calculates the difficulty of the beatmap using a specific mod combination and returns a set of representing the difficulty at every relevant time value in the beatmap. /// /// The mods that should be applied to the beatmap. /// The cancellation token. /// The set of . - public List CalculateTimed(IEnumerable mods, CancellationToken cancellationToken = default) + public List CalculateTimed([NotNull] IEnumerable mods, CancellationToken cancellationToken = default) { cancellationToken.ThrowIfCancellationRequested(); preProcess(mods, cancellationToken); @@ -124,7 +141,7 @@ namespace osu.Game.Rulesets.Difficulty /// /// The original list of s. /// The cancellation token. - private void preProcess(IEnumerable mods, CancellationToken cancellationToken = default) + private void preProcess([NotNull] IEnumerable mods, CancellationToken cancellationToken = default) { playableMods = mods.Select(m => m.DeepClone()).ToArray(); diff --git a/osu.Game/Rulesets/Difficulty/TimedDifficultyAttributes.cs b/osu.Game/Rulesets/Difficulty/TimedDifficultyAttributes.cs index 2509971389..c07d1cd46e 100644 --- a/osu.Game/Rulesets/Difficulty/TimedDifficultyAttributes.cs +++ b/osu.Game/Rulesets/Difficulty/TimedDifficultyAttributes.cs @@ -7,7 +7,7 @@ namespace osu.Game.Rulesets.Difficulty { /// /// Wraps a object and adds a time value for which the attribute is valid. - /// Output by . + /// Output by DifficultyCalculator.CalculateTimed methods. /// public class TimedDifficultyAttributes : IComparable { From 6cca657a2d48c6424759493d8fe264791ed211db Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 16 Nov 2021 14:45:51 +0900 Subject: [PATCH 08/14] Standardise naming of `CancellationToken` parameters --- osu.Game/Beatmaps/BeatmapDifficultyCache.cs | 12 ++++++------ osu.Game/Beatmaps/IWorkingBeatmap.cs | 4 ++-- osu.Game/Beatmaps/WorkingBeatmap.cs | 18 +++++++++--------- .../Difficulty/DifficultyCalculator.cs | 2 +- .../Play/HUD/PerformancePointsCounter.cs | 2 +- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs index dabff0141e..d761ea3212 100644 --- a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs +++ b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs @@ -140,21 +140,21 @@ namespace osu.Game.Beatmaps return GetAsync(new DifficultyCacheLookup(localBeatmapInfo, localRulesetInfo, mods), cancellationToken); } - protected override Task ComputeValueAsync(DifficultyCacheLookup lookup, CancellationToken token = default) + protected override Task ComputeValueAsync(DifficultyCacheLookup lookup, CancellationToken cancellationToken = default) { return Task.Factory.StartNew(() => { if (CheckExists(lookup, out var existing)) return existing; - return computeDifficulty(lookup, token); - }, token, TaskCreationOptions.HideScheduler | TaskCreationOptions.RunContinuationsAsynchronously, updateScheduler); + return computeDifficulty(lookup, cancellationToken); + }, cancellationToken, TaskCreationOptions.HideScheduler | TaskCreationOptions.RunContinuationsAsynchronously, updateScheduler); } - public Task> GetTimedDifficultyAttributesAsync(WorkingBeatmap beatmap, Ruleset ruleset, Mod[] mods, CancellationToken token = default) + public Task> GetTimedDifficultyAttributesAsync(WorkingBeatmap beatmap, Ruleset ruleset, Mod[] mods, CancellationToken cancellationToken = default) { - return Task.Factory.StartNew(() => ruleset.CreateDifficultyCalculator(beatmap).CalculateTimed(mods, token), - token, + return Task.Factory.StartNew(() => ruleset.CreateDifficultyCalculator(beatmap).CalculateTimed(mods, cancellationToken), + cancellationToken, TaskCreationOptions.HideScheduler | TaskCreationOptions.RunContinuationsAsynchronously, updateScheduler); } diff --git a/osu.Game/Beatmaps/IWorkingBeatmap.cs b/osu.Game/Beatmaps/IWorkingBeatmap.cs index 39f6ad7b58..5e9b77392a 100644 --- a/osu.Game/Beatmaps/IWorkingBeatmap.cs +++ b/osu.Game/Beatmaps/IWorkingBeatmap.cs @@ -92,10 +92,10 @@ namespace osu.Game.Beatmaps /// /// The to create a playable for. /// The s to apply to the . - /// Cancellation token that cancels the beatmap loading process. + /// Cancellation token that cancels the beatmap loading process. /// The converted . /// If could not be converted to . - IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, CancellationToken token = default); + IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, CancellationToken cancellationToken = default); /// /// Load a new audio track instance for this beatmap. This should be called once before accessing . diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index b4db4b9f9f..4098a57bf2 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -79,7 +79,7 @@ namespace osu.Game.Beatmaps /// The applicable . protected virtual IBeatmapConverter CreateBeatmapConverter(IBeatmap beatmap, Ruleset ruleset) => ruleset.CreateBeatmapConverter(beatmap); - public virtual IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, CancellationToken token = default) + public virtual IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, CancellationToken cancellationToken = default) { mods ??= Array.Empty(); @@ -97,19 +97,19 @@ namespace osu.Game.Beatmaps // Apply conversion mods foreach (var mod in mods.OfType()) { - if (token.IsCancellationRequested) + if (cancellationToken.IsCancellationRequested) throw new BeatmapLoadTimeoutException(BeatmapInfo); mod.ApplyToBeatmapConverter(converter); } // Convert - IBeatmap converted = converter.Convert(token); + IBeatmap converted = converter.Convert(cancellationToken); // Apply conversion mods to the result foreach (var mod in mods.OfType()) { - if (token.IsCancellationRequested) + if (cancellationToken.IsCancellationRequested) throw new BeatmapLoadTimeoutException(BeatmapInfo); mod.ApplyToBeatmap(converted); @@ -120,7 +120,7 @@ namespace osu.Game.Beatmaps { foreach (var mod in mods.OfType()) { - if (token.IsCancellationRequested) + if (cancellationToken.IsCancellationRequested) throw new BeatmapLoadTimeoutException(BeatmapInfo); mod.ApplyToDifficulty(converted.Difficulty); @@ -139,10 +139,10 @@ namespace osu.Game.Beatmaps { foreach (var obj in converted.HitObjects) { - if (token.IsCancellationRequested) + if (cancellationToken.IsCancellationRequested) throw new BeatmapLoadTimeoutException(BeatmapInfo); - obj.ApplyDefaults(converted.ControlPointInfo, converted.Difficulty, token); + obj.ApplyDefaults(converted.ControlPointInfo, converted.Difficulty, cancellationToken); } } catch (OperationCanceledException) @@ -154,7 +154,7 @@ namespace osu.Game.Beatmaps { foreach (var obj in converted.HitObjects) { - if (token.IsCancellationRequested) + if (cancellationToken.IsCancellationRequested) throw new BeatmapLoadTimeoutException(BeatmapInfo); mod.ApplyToHitObject(obj); @@ -165,7 +165,7 @@ namespace osu.Game.Beatmaps foreach (var mod in mods.OfType()) { - token.ThrowIfCancellationRequested(); + cancellationToken.ThrowIfCancellationRequested(); mod.ApplyToBeatmap(converted); } diff --git a/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs b/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs index 6748158d8f..f5b6883b27 100644 --- a/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs +++ b/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs @@ -145,7 +145,7 @@ namespace osu.Game.Rulesets.Difficulty { playableMods = mods.Select(m => m.DeepClone()).ToArray(); - Beatmap = beatmap.GetPlayableBeatmap(ruleset.RulesetInfo, playableMods, token: cancellationToken); + Beatmap = beatmap.GetPlayableBeatmap(ruleset.RulesetInfo, playableMods, cancellationToken: cancellationToken); var track = new TrackVirtual(10000); playableMods.OfType().ForEach(m => m.ApplyToTrack(track)); diff --git a/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs b/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs index 24c6c87c7d..6e679279a4 100644 --- a/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs +++ b/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs @@ -216,7 +216,7 @@ namespace osu.Game.Screens.Play.HUD this.gameplayBeatmap = gameplayBeatmap; } - public override IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, CancellationToken timeoutToken = default) + public override IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, CancellationToken cancellationToken = default) => gameplayBeatmap; protected override IBeatmap GetBeatmap() => gameplayBeatmap; From d2a767049437b30e6bdf1eb09e301e2bd92474ea Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 16 Nov 2021 14:48:02 +0900 Subject: [PATCH 09/14] Remove no longer used helper method --- osu.Game/Beatmaps/WorkingBeatmap.cs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index 4098a57bf2..9be239e3b0 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.IO; using System.Linq; using System.Threading; @@ -188,15 +187,6 @@ namespace osu.Game.Beatmaps } } - private CancellationTokenSource createTimeoutTokenSource(TimeSpan? timeout) - { - if (Debugger.IsAttached) - // ignore timeout when debugger is attached (may be breakpointing / debugging). - return new CancellationTokenSource(); - - return new CancellationTokenSource(timeout ?? TimeSpan.FromSeconds(10)); - } - private readonly object beatmapFetchLock = new object(); private Task loadBeatmapAsync() From 13f3e2eea9d3c590cd910e556feab0ee5e9742a6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 17 Nov 2021 10:48:33 +0900 Subject: [PATCH 10/14] Add back default timeout to `GetPlayableBeatmap` --- osu.Game/Beatmaps/IWorkingBeatmap.cs | 4 ++-- osu.Game/Beatmaps/WorkingBeatmap.cs | 19 ++++++++++--------- .../Play/HUD/PerformancePointsCounter.cs | 2 +- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/osu.Game/Beatmaps/IWorkingBeatmap.cs b/osu.Game/Beatmaps/IWorkingBeatmap.cs index 5e9b77392a..6bf338db59 100644 --- a/osu.Game/Beatmaps/IWorkingBeatmap.cs +++ b/osu.Game/Beatmaps/IWorkingBeatmap.cs @@ -92,10 +92,10 @@ namespace osu.Game.Beatmaps /// /// The to create a playable for. /// The s to apply to the . - /// Cancellation token that cancels the beatmap loading process. + /// Cancellation token that cancels the beatmap loading process. If not provided, a default timeout of 10,000ms will be applied to the load process. /// The converted . /// If could not be converted to . - IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, CancellationToken cancellationToken = default); + IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, CancellationToken? cancellationToken = null); /// /// Load a new audio track instance for this beatmap. This should be called once before accessing . diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index 9be239e3b0..a04345ec5c 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -78,8 +78,9 @@ namespace osu.Game.Beatmaps /// The applicable . protected virtual IBeatmapConverter CreateBeatmapConverter(IBeatmap beatmap, Ruleset ruleset) => ruleset.CreateBeatmapConverter(beatmap); - public virtual IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, CancellationToken cancellationToken = default) + public virtual IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, CancellationToken? cancellationToken = null) { + var token = cancellationToken ?? new CancellationTokenSource(10000).Token; mods ??= Array.Empty(); var rulesetInstance = ruleset.CreateInstance(); @@ -96,19 +97,19 @@ namespace osu.Game.Beatmaps // Apply conversion mods foreach (var mod in mods.OfType()) { - if (cancellationToken.IsCancellationRequested) + if (token.IsCancellationRequested) throw new BeatmapLoadTimeoutException(BeatmapInfo); mod.ApplyToBeatmapConverter(converter); } // Convert - IBeatmap converted = converter.Convert(cancellationToken); + IBeatmap converted = converter.Convert(token); // Apply conversion mods to the result foreach (var mod in mods.OfType()) { - if (cancellationToken.IsCancellationRequested) + if (token.IsCancellationRequested) throw new BeatmapLoadTimeoutException(BeatmapInfo); mod.ApplyToBeatmap(converted); @@ -119,7 +120,7 @@ namespace osu.Game.Beatmaps { foreach (var mod in mods.OfType()) { - if (cancellationToken.IsCancellationRequested) + if (token.IsCancellationRequested) throw new BeatmapLoadTimeoutException(BeatmapInfo); mod.ApplyToDifficulty(converted.Difficulty); @@ -138,10 +139,10 @@ namespace osu.Game.Beatmaps { foreach (var obj in converted.HitObjects) { - if (cancellationToken.IsCancellationRequested) + if (token.IsCancellationRequested) throw new BeatmapLoadTimeoutException(BeatmapInfo); - obj.ApplyDefaults(converted.ControlPointInfo, converted.Difficulty, cancellationToken); + obj.ApplyDefaults(converted.ControlPointInfo, converted.Difficulty, token); } } catch (OperationCanceledException) @@ -153,7 +154,7 @@ namespace osu.Game.Beatmaps { foreach (var obj in converted.HitObjects) { - if (cancellationToken.IsCancellationRequested) + if (token.IsCancellationRequested) throw new BeatmapLoadTimeoutException(BeatmapInfo); mod.ApplyToHitObject(obj); @@ -164,7 +165,7 @@ namespace osu.Game.Beatmaps foreach (var mod in mods.OfType()) { - cancellationToken.ThrowIfCancellationRequested(); + token.ThrowIfCancellationRequested(); mod.ApplyToBeatmap(converted); } diff --git a/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs b/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs index 6e679279a4..2f46682b1a 100644 --- a/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs +++ b/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs @@ -216,7 +216,7 @@ namespace osu.Game.Screens.Play.HUD this.gameplayBeatmap = gameplayBeatmap; } - public override IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, CancellationToken cancellationToken = default) + public override IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, CancellationToken? cancellationToken = null) => gameplayBeatmap; protected override IBeatmap GetBeatmap() => gameplayBeatmap; From 4d11794d31d0822e0738f49d5adb09492843a021 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 17 Nov 2021 11:06:43 +0900 Subject: [PATCH 11/14] Add test coverage of `GetPlayableBeatmap` timeout and cancellation --- osu.Game.Tests/Beatmaps/WorkingBeatmapTest.cs | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 osu.Game.Tests/Beatmaps/WorkingBeatmapTest.cs diff --git a/osu.Game.Tests/Beatmaps/WorkingBeatmapTest.cs b/osu.Game.Tests/Beatmaps/WorkingBeatmapTest.cs new file mode 100644 index 0000000000..02e4a87a7a --- /dev/null +++ b/osu.Game.Tests/Beatmaps/WorkingBeatmapTest.cs @@ -0,0 +1,102 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using NUnit.Framework; +using osu.Game.Beatmaps; +using osu.Game.Rulesets; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Osu; +using osu.Game.Rulesets.Osu.Beatmaps; + +namespace osu.Game.Tests.Beatmaps +{ + [TestFixture] + public class WorkingBeatmapTest + { + [Test] + public void TestGetPlayableSuccess() + { + var working = new TestNeverLoadsWorkingBeatmap(); + + working.ResetEvent.Set(); + + Assert.NotNull(working.GetPlayableBeatmap(new OsuRuleset().RulesetInfo)); + } + + [Test] + public void TestGetPlayableCancellationToken() + { + var working = new TestNeverLoadsWorkingBeatmap(); + + var cts = new CancellationTokenSource(); + var loadStarted = new ManualResetEventSlim(); + var loadCompleted = new ManualResetEventSlim(); + + Task.Factory.StartNew(() => + { + loadStarted.Set(); + Assert.Throws(() => working.GetPlayableBeatmap(new OsuRuleset().RulesetInfo, cancellationToken: cts.Token)); + loadCompleted.Set(); + }, TaskCreationOptions.LongRunning); + + Assert.IsTrue(loadStarted.Wait(10000)); + + cts.Cancel(); + + Assert.IsTrue(loadCompleted.Wait(10000)); + + working.ResetEvent.Set(); + } + + [Test] + public void TestGetPlayableDefaultTimeout() + { + var working = new TestNeverLoadsWorkingBeatmap(); + + Assert.Throws(() => working.GetPlayableBeatmap(new OsuRuleset().RulesetInfo)); + + working.ResetEvent.Set(); + } + + public class TestNeverLoadsWorkingBeatmap : TestWorkingBeatmap + { + public ManualResetEventSlim ResetEvent = new ManualResetEventSlim(); + + public TestNeverLoadsWorkingBeatmap() + : base(new Beatmap()) + { + } + + protected override IBeatmapConverter CreateBeatmapConverter(IBeatmap beatmap, Ruleset ruleset) => new TestConverter(beatmap, ResetEvent); + + public class TestConverter : IBeatmapConverter + { + private readonly ManualResetEventSlim resetEvent; + + public TestConverter(IBeatmap beatmap, ManualResetEventSlim resetEvent) + { + this.resetEvent = resetEvent; + Beatmap = beatmap; + } + + public event Action> ObjectConverted; + + protected virtual void OnObjectConverted(HitObject arg1, IEnumerable arg2) => ObjectConverted?.Invoke(arg1, arg2); + + public IBeatmap Beatmap { get; } + + public bool CanConvert() => true; + + public IBeatmap Convert(CancellationToken cancellationToken = default) + { + resetEvent.Wait(cancellationToken); + return new OsuBeatmap(); + } + } + } + } +} From 8528cab40b862c4b03c27c8543ebb9c95f6ea15d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 17 Nov 2021 22:07:24 +0100 Subject: [PATCH 12/14] Add test coverage for ruleset load failure scenario --- osu.Game.Tests/Beatmaps/WorkingBeatmapTest.cs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/osu.Game.Tests/Beatmaps/WorkingBeatmapTest.cs b/osu.Game.Tests/Beatmaps/WorkingBeatmapTest.cs index 02e4a87a7a..6ad63a5fbb 100644 --- a/osu.Game.Tests/Beatmaps/WorkingBeatmapTest.cs +++ b/osu.Game.Tests/Beatmaps/WorkingBeatmapTest.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; +using Moq; using NUnit.Framework; using osu.Game.Beatmaps; using osu.Game.Rulesets; @@ -62,6 +63,17 @@ namespace osu.Game.Tests.Beatmaps working.ResetEvent.Set(); } + [Test] + public void TestGetPlayableRulesetLoadFailure() + { + var working = new TestWorkingBeatmap(new Beatmap()); + + // by default mocks return nulls if not set up, which is actually desired here to simulate a ruleset load failure scenario. + var ruleset = new Mock(); + + Assert.Throws(() => working.GetPlayableBeatmap(ruleset.Object)); + } + public class TestNeverLoadsWorkingBeatmap : TestWorkingBeatmap { public ManualResetEventSlim ResetEvent = new ManualResetEventSlim(); From 1c13b39104a14cf9dc9d7727a0460e8bfb63bab1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 17 Nov 2021 22:00:09 +0100 Subject: [PATCH 13/14] Revert incorrect ordering change --- osu.Game/Beatmaps/WorkingBeatmap.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index dc38a9af69..f46cd405b5 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -85,11 +85,11 @@ namespace osu.Game.Beatmaps var rulesetInstance = ruleset.CreateInstance(); - IBeatmapConverter converter = CreateBeatmapConverter(Beatmap, rulesetInstance); - if (rulesetInstance == null) throw new RulesetLoadException("Creating ruleset instance failed when attempting to create playable beatmap."); + IBeatmapConverter converter = CreateBeatmapConverter(Beatmap, rulesetInstance); + // Check if the beatmap can be converted if (Beatmap.HitObjects.Count > 0 && !converter.CanConvert()) throw new BeatmapInvalidForRulesetException($"{nameof(Beatmaps.Beatmap)} can not be converted for the ruleset (ruleset: {ruleset.InstantiationInfo}, converter: {converter})."); From bf8507c7b92dad894becf3d4eb41443cdff9ee5e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 18 Nov 2021 23:26:45 +0900 Subject: [PATCH 14/14] Only apply default timeout when debugger not attached --- osu.Game/Beatmaps/WorkingBeatmap.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index f46cd405b5..d2912229c6 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Linq; using System.Threading; @@ -80,7 +81,10 @@ namespace osu.Game.Beatmaps public virtual IBeatmap GetPlayableBeatmap(IRulesetInfo ruleset, IReadOnlyList mods = null, CancellationToken? cancellationToken = null) { - var token = cancellationToken ?? new CancellationTokenSource(10000).Token; + var token = cancellationToken ?? + // don't apply the default timeout when debugger is attached (may be breakpointing / debugging). + (Debugger.IsAttached ? new CancellationToken() : new CancellationTokenSource(10000).Token); + mods ??= Array.Empty(); var rulesetInstance = ruleset.CreateInstance();