From f7c036726a72271a8418acd0919dbbe932d2a086 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 13 Mar 2020 13:52:40 +0900 Subject: [PATCH 1/4] Add beatmap loading timeout --- osu.Game/Beatmaps/IWorkingBeatmap.cs | 4 +- osu.Game/Beatmaps/WorkingBeatmap.cs | 98 ++++++++++++++++------------ 2 files changed, 61 insertions(+), 41 deletions(-) diff --git a/osu.Game/Beatmaps/IWorkingBeatmap.cs b/osu.Game/Beatmaps/IWorkingBeatmap.cs index 5f1f0d1e40..61bd962648 100644 --- a/osu.Game/Beatmaps/IWorkingBeatmap.cs +++ b/osu.Game/Beatmaps/IWorkingBeatmap.cs @@ -1,6 +1,7 @@ // 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 osu.Framework.Audio.Track; using osu.Framework.Graphics.Textures; @@ -60,8 +61,9 @@ namespace osu.Game.Beatmaps /// /// The to create a playable for. /// The s to apply to the . + /// The loading timeout. /// The converted . /// If could not be converted to . - IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null); + IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, TimeSpan? timeout = null); } } diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index 1e1ffad81e..bdcfc058b4 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -83,55 +83,73 @@ namespace osu.Game.Beatmaps /// The applicable . protected virtual IBeatmapConverter CreateBeatmapConverter(IBeatmap beatmap, Ruleset ruleset) => ruleset.CreateBeatmapConverter(beatmap); - public IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null) + public IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, TimeSpan? timeout = null) { - 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()) - mod.ApplyToBeatmapConverter(converter); - - // Convert - IBeatmap converted = converter.Convert(); - - // Apply difficulty mods - if (mods.Any(m => m is IApplicableToDifficulty)) + using (var cancellationSource = new CancellationTokenSource(timeout ?? TimeSpan.FromSeconds(10))) { - converted.BeatmapInfo = converted.BeatmapInfo.Clone(); - converted.BeatmapInfo.BaseDifficulty = converted.BeatmapInfo.BaseDifficulty.Clone(); + mods ??= Array.Empty(); - foreach (var mod in mods.OfType()) - mod.ApplyToDifficulty(converted.BeatmapInfo.BaseDifficulty); - } + var rulesetInstance = ruleset.CreateInstance(); - IBeatmapProcessor processor = rulesetInstance.CreateBeatmapProcessor(converted); + IBeatmapConverter converter = CreateBeatmapConverter(Beatmap, rulesetInstance); - processor?.PreProcess(); + // 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})."); - // Compute default values for hitobjects, including creating nested hitobjects in-case they're needed - foreach (var obj in converted.HitObjects) - obj.ApplyDefaults(converted.ControlPointInfo, converted.BeatmapInfo.BaseDifficulty); + // Apply conversion mods + foreach (var mod in mods.OfType()) + { + cancellationSource.Token.ThrowIfCancellationRequested(); + mod.ApplyToBeatmapConverter(converter); + } - foreach (var mod in mods.OfType()) - { + // Convert + IBeatmap converted = converter.Convert(); + + // Apply difficulty mods + if (mods.Any(m => m is IApplicableToDifficulty)) + { + converted.BeatmapInfo = converted.BeatmapInfo.Clone(); + converted.BeatmapInfo.BaseDifficulty = converted.BeatmapInfo.BaseDifficulty.Clone(); + + foreach (var mod in mods.OfType()) + { + cancellationSource.Token.ThrowIfCancellationRequested(); + mod.ApplyToDifficulty(converted.BeatmapInfo.BaseDifficulty); + } + } + + IBeatmapProcessor processor = rulesetInstance.CreateBeatmapProcessor(converted); + + processor?.PreProcess(); + + // Compute default values for hitobjects, including creating nested hitobjects in-case they're needed foreach (var obj in converted.HitObjects) - mod.ApplyToHitObject(obj); + { + cancellationSource.Token.ThrowIfCancellationRequested(); + obj.ApplyDefaults(converted.ControlPointInfo, converted.BeatmapInfo.BaseDifficulty); + } + + foreach (var mod in mods.OfType()) + { + foreach (var obj in converted.HitObjects) + { + cancellationSource.Token.ThrowIfCancellationRequested(); + mod.ApplyToHitObject(obj); + } + } + + processor?.PostProcess(); + + foreach (var mod in mods.OfType()) + { + cancellationSource.Token.ThrowIfCancellationRequested(); + mod.ApplyToBeatmap(converted); + } + + return converted; } - - processor?.PostProcess(); - - foreach (var mod in mods.OfType()) - mod.ApplyToBeatmap(converted); - - return converted; } private CancellationTokenSource loadCancellation = new CancellationTokenSource(); From c33ca6e99c60e1fd4bbc1d321a229a55f3e99de5 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 13 Mar 2020 14:28:11 +0900 Subject: [PATCH 2/4] Decorate usages with exception management --- osu.Game/Screens/Edit/Editor.cs | 14 +++++++++- osu.Game/Screens/Select/BeatmapInfoWedge.cs | 30 +++++++++++++-------- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/osu.Game/Screens/Edit/Editor.cs b/osu.Game/Screens/Edit/Editor.cs index 3a6f02f811..cf13f8a3a1 100644 --- a/osu.Game/Screens/Edit/Editor.cs +++ b/osu.Game/Screens/Edit/Editor.cs @@ -23,6 +23,7 @@ using osuTK.Input; using System.Collections.Generic; using osu.Framework; using osu.Framework.Input.Bindings; +using osu.Framework.Logging; using osu.Game.Beatmaps; using osu.Game.Graphics.Cursor; using osu.Game.Input.Bindings; @@ -86,7 +87,18 @@ namespace osu.Game.Screens.Edit // todo: remove caching of this and consume via editorBeatmap? dependencies.Cache(beatDivisor); - playableBeatmap = Beatmap.Value.GetPlayableBeatmap(Beatmap.Value.BeatmapInfo.Ruleset); + try + { + playableBeatmap = Beatmap.Value.GetPlayableBeatmap(Beatmap.Value.BeatmapInfo.Ruleset); + } + catch (Exception e) + { + Logger.Error(e, "Could not load beatmap sucessfully!"); + //couldn't load, hard abort! + this.Exit(); + return; + } + AddInternal(editorBeatmap = new EditorBeatmap(playableBeatmap)); dependencies.CacheAs(editorBeatmap); diff --git a/osu.Game/Screens/Select/BeatmapInfoWedge.cs b/osu.Game/Screens/Select/BeatmapInfoWedge.cs index f84aac3081..59d2aca17d 100644 --- a/osu.Game/Screens/Select/BeatmapInfoWedge.cs +++ b/osu.Game/Screens/Select/BeatmapInfoWedge.cs @@ -23,6 +23,7 @@ using osu.Framework.Graphics.Cursor; using osu.Framework.Graphics.Effects; using osu.Framework.Graphics.Sprites; using osu.Framework.Localisation; +using osu.Framework.Logging; using osu.Game.Rulesets; using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.UI; @@ -311,20 +312,27 @@ namespace osu.Game.Screens.Select Content = getBPMRange(b), })); - IBeatmap playableBeatmap; - try { - // Try to get the beatmap with the user's ruleset - playableBeatmap = beatmap.GetPlayableBeatmap(ruleset, Array.Empty()); - } - catch (BeatmapInvalidForRulesetException) - { - // Can't be converted to the user's ruleset, so use the beatmap's own ruleset - playableBeatmap = beatmap.GetPlayableBeatmap(beatmap.BeatmapInfo.Ruleset, Array.Empty()); - } + IBeatmap playableBeatmap; - labels.AddRange(playableBeatmap.GetStatistics().Select(s => new InfoLabel(s))); + try + { + // Try to get the beatmap with the user's ruleset + playableBeatmap = beatmap.GetPlayableBeatmap(ruleset, Array.Empty()); + } + catch (BeatmapInvalidForRulesetException) + { + // Can't be converted to the user's ruleset, so use the beatmap's own ruleset + playableBeatmap = beatmap.GetPlayableBeatmap(beatmap.BeatmapInfo.Ruleset, Array.Empty()); + } + + labels.AddRange(playableBeatmap.GetStatistics().Select(s => new InfoLabel(s))); + } + catch (Exception e) + { + Logger.Error(e, "Could not load beatmap sucessfully!"); + } } return labels.ToArray(); From f390c1995db413005cea88a68d832e92382768e2 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Mon, 16 Mar 2020 11:29:28 +0900 Subject: [PATCH 3/4] Apply comment suggestions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Dean Herbert Co-Authored-By: Bartłomiej Dach --- osu.Game/Beatmaps/IWorkingBeatmap.cs | 2 +- osu.Game/Screens/Edit/Editor.cs | 4 ++-- osu.Game/Screens/Select/BeatmapInfoWedge.cs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game/Beatmaps/IWorkingBeatmap.cs b/osu.Game/Beatmaps/IWorkingBeatmap.cs index 61bd962648..526bc668af 100644 --- a/osu.Game/Beatmaps/IWorkingBeatmap.cs +++ b/osu.Game/Beatmaps/IWorkingBeatmap.cs @@ -61,7 +61,7 @@ namespace osu.Game.Beatmaps /// /// The to create a playable for. /// The s to apply to the . - /// The loading timeout. + /// The maximum length in milliseconds to wait for load to complete. Defaults to 10,000ms. /// The converted . /// If could not be converted to . IBeatmap GetPlayableBeatmap(RulesetInfo ruleset, IReadOnlyList mods = null, TimeSpan? timeout = null); diff --git a/osu.Game/Screens/Edit/Editor.cs b/osu.Game/Screens/Edit/Editor.cs index cf13f8a3a1..f1cbed57f1 100644 --- a/osu.Game/Screens/Edit/Editor.cs +++ b/osu.Game/Screens/Edit/Editor.cs @@ -93,8 +93,8 @@ namespace osu.Game.Screens.Edit } catch (Exception e) { - Logger.Error(e, "Could not load beatmap sucessfully!"); - //couldn't load, hard abort! + Logger.Error(e, "Could not load beatmap successfully!"); + // couldn't load, hard abort! this.Exit(); return; } diff --git a/osu.Game/Screens/Select/BeatmapInfoWedge.cs b/osu.Game/Screens/Select/BeatmapInfoWedge.cs index 59d2aca17d..7a8a1593b9 100644 --- a/osu.Game/Screens/Select/BeatmapInfoWedge.cs +++ b/osu.Game/Screens/Select/BeatmapInfoWedge.cs @@ -331,7 +331,7 @@ namespace osu.Game.Screens.Select } catch (Exception e) { - Logger.Error(e, "Could not load beatmap sucessfully!"); + Logger.Error(e, "Could not load beatmap successfully!"); } } From 9c5423734a30587063c9a1375eeccb6243cc0d78 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Mon, 16 Mar 2020 11:33:26 +0900 Subject: [PATCH 4/4] Throw timeout exceptions instead --- osu.Game/Beatmaps/WorkingBeatmap.cs | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index bdcfc058b4..dd4f893ac2 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -100,7 +100,9 @@ namespace osu.Game.Beatmaps // Apply conversion mods foreach (var mod in mods.OfType()) { - cancellationSource.Token.ThrowIfCancellationRequested(); + if (cancellationSource.IsCancellationRequested) + throw new BeatmapLoadTimeoutException(BeatmapInfo); + mod.ApplyToBeatmapConverter(converter); } @@ -115,7 +117,9 @@ namespace osu.Game.Beatmaps foreach (var mod in mods.OfType()) { - cancellationSource.Token.ThrowIfCancellationRequested(); + if (cancellationSource.IsCancellationRequested) + throw new BeatmapLoadTimeoutException(BeatmapInfo); + mod.ApplyToDifficulty(converted.BeatmapInfo.BaseDifficulty); } } @@ -127,7 +131,9 @@ namespace osu.Game.Beatmaps // Compute default values for hitobjects, including creating nested hitobjects in-case they're needed foreach (var obj in converted.HitObjects) { - cancellationSource.Token.ThrowIfCancellationRequested(); + if (cancellationSource.IsCancellationRequested) + throw new BeatmapLoadTimeoutException(BeatmapInfo); + obj.ApplyDefaults(converted.ControlPointInfo, converted.BeatmapInfo.BaseDifficulty); } @@ -135,7 +141,9 @@ namespace osu.Game.Beatmaps { foreach (var obj in converted.HitObjects) { - cancellationSource.Token.ThrowIfCancellationRequested(); + if (cancellationSource.IsCancellationRequested) + throw new BeatmapLoadTimeoutException(BeatmapInfo); + mod.ApplyToHitObject(obj); } } @@ -315,5 +323,13 @@ namespace osu.Game.Beatmaps private void recreate() => lazy = new Lazy(valueFactory, LazyThreadSafetyMode.ExecutionAndPublication); } + + private class BeatmapLoadTimeoutException : TimeoutException + { + public BeatmapLoadTimeoutException(BeatmapInfo beatmapInfo) + : base($"Timed out while loading beatmap ({beatmapInfo}).") + { + } + } } }