1
0
mirror of https://github.com/ppy/osu.git synced 2024-11-11 22:27:25 +08:00

Merge pull request #15707 from bdach/fix-working-beatmap-wrong-exception-type

Clean up cancellation handling in `WorkingBeatmap.GetPlayableBeatmap()`
This commit is contained in:
Dean Herbert 2021-11-21 20:02:04 +09:00 committed by GitHub
commit 0641d5dddd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 58 additions and 38 deletions

View File

@ -9,6 +9,7 @@ using Moq;
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 @@ namespace osu.Game.Tests.Beatmaps
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 @@ namespace osu.Game.Tests.Beatmaps
{
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();
}

View File

@ -294,15 +294,22 @@ namespace osu.Game.Beatmaps
return new StarDifficulty(attributes);
}
catch (BeatmapInvalidForRulesetException e)
catch (OperationCanceledException)
{
// no need to log, cancellations are expected as part of normal operation.
return null;
}
catch (BeatmapInvalidForRulesetException invalidForRuleset)
{
if (rulesetInfo.Equals(beatmapInfo.Ruleset))
Logger.Error(e, $"Failed to convert {beatmapInfo.OnlineID} to the beatmap's default ruleset ({beatmapInfo.Ruleset}).");
Logger.Error(invalidForRuleset, $"Failed to convert {beatmapInfo.OnlineID} to the beatmap's default ruleset ({beatmapInfo.Ruleset}).");
return null;
}
catch
catch (Exception unknownException)
{
Logger.Error(unknownException, "Failed to calculate beatmap difficulty");
return null;
}
}

View File

@ -90,12 +90,31 @@ namespace osu.Game.Beatmaps
/// 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, IReadOnlyList<Mod> mods = null);
/// <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"/>.

View File

@ -79,14 +79,24 @@ namespace osu.Game.Beatmaps
/// <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, IReadOnlyList<Mod> mods = null)
{
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<Mod>();
try
{
using (var cancellationTokenSource = new CancellationTokenSource(10_000))
{
// don't apply the default timeout when debugger is attached (may be breakpointing / debugging).
return GetPlayableBeatmap(ruleset, mods ?? Array.Empty<Mod>(), 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 @@ namespace osu.Game.Beatmaps
// Apply conversion mods
foreach (var mod in mods.OfType<IApplicableToBeatmapConverter>())
{
if (token.IsCancellationRequested)
throw new BeatmapLoadTimeoutException(BeatmapInfo);
token.ThrowIfCancellationRequested();
mod.ApplyToBeatmapConverter(converter);
}
@ -113,9 +121,7 @@ namespace osu.Game.Beatmaps
// Apply conversion mods to the result
foreach (var mod in mods.OfType<IApplicableAfterBeatmapConversion>())
{
if (token.IsCancellationRequested)
throw new BeatmapLoadTimeoutException(BeatmapInfo);
token.ThrowIfCancellationRequested();
mod.ApplyToBeatmap(converted);
}
@ -124,9 +130,7 @@ namespace osu.Game.Beatmaps
{
foreach (var mod in mods.OfType<IApplicableToDifficulty>())
{
if (token.IsCancellationRequested)
throw new BeatmapLoadTimeoutException(BeatmapInfo);
token.ThrowIfCancellationRequested();
mod.ApplyToDifficulty(converted.Difficulty);
}
}
@ -139,28 +143,17 @@ namespace osu.Game.Beatmaps
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)
{
if (token.IsCancellationRequested)
throw new BeatmapLoadTimeoutException(BeatmapInfo);
obj.ApplyDefaults(converted.ControlPointInfo, converted.Difficulty, token);
}
}
catch (OperationCanceledException)
{
throw new BeatmapLoadTimeoutException(BeatmapInfo);
token.ThrowIfCancellationRequested();
obj.ApplyDefaults(converted.ControlPointInfo, converted.Difficulty, token);
}
foreach (var mod in mods.OfType<IApplicableToHitObject>())
{
foreach (var obj in converted.HitObjects)
{
if (token.IsCancellationRequested)
throw new BeatmapLoadTimeoutException(BeatmapInfo);
token.ThrowIfCancellationRequested();
mod.ApplyToHitObject(obj);
}
}

View File

@ -216,7 +216,7 @@ namespace osu.Game.Screens.Play.HUD
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;