Change difficulty cache storage type to nullable

The recent changes related to adding support for working beatmap load
cancellation exposed a flaw in the beatmap difficulty cache. With the
way the difficulty computation logic was written, any error in the
calculation process (including beatmap load timeout, or cancellation)
would result in a 0.00 star rating being permanently cached in memory
for the given beatmap.

To resolve, change the difficulty cache's return type to nullable.
In failure scenarios, `null` is returned, rather than
`default(StarDifficulty)` as done previously.
This commit is contained in:
Bartłomiej Dach 2021-11-20 16:54:58 +01:00
parent b1fcb840a9
commit 15feb17da8
No known key found for this signature in database
GPG Key ID: BCECCD4FA41F6497
7 changed files with 40 additions and 25 deletions

View File

@ -164,9 +164,9 @@ namespace osu.Game.Tests.Beatmaps
{
public Func<DifficultyCacheLookup, StarDifficulty> ComputeDifficulty { get; set; }
protected override Task<StarDifficulty> ComputeValueAsync(DifficultyCacheLookup lookup, CancellationToken token = default)
protected override Task<StarDifficulty?> ComputeValueAsync(DifficultyCacheLookup lookup, CancellationToken token = default)
{
return Task.FromResult(ComputeDifficulty?.Invoke(lookup) ?? new StarDifficulty(BASE_STARS, 0));
return Task.FromResult<StarDifficulty?>(ComputeDifficulty?.Invoke(lookup) ?? new StarDifficulty(BASE_STARS, 0));
}
}
}

View File

@ -140,7 +140,7 @@ namespace osu.Game.Tests.Visual.SongSelect
}
}
public override async Task<StarDifficulty> GetDifficultyAsync(IBeatmapInfo beatmapInfo, IRulesetInfo rulesetInfo = null, IEnumerable<Mod> mods = null, CancellationToken cancellationToken = default)
public override async Task<StarDifficulty?> GetDifficultyAsync(IBeatmapInfo beatmapInfo, IRulesetInfo rulesetInfo = null, IEnumerable<Mod> mods = null, CancellationToken cancellationToken = default)
{
if (blockCalculation)
await calculationBlocker.Task.ConfigureAwait(false);

View File

@ -27,7 +27,7 @@ namespace osu.Game.Beatmaps
/// A component which performs and acts as a central cache for difficulty calculations of beatmap/ruleset/mod combinations.
/// Currently not persisted between game sessions.
/// </summary>
public class BeatmapDifficultyCache : MemoryCachingComponent<BeatmapDifficultyCache.DifficultyCacheLookup, StarDifficulty>
public class BeatmapDifficultyCache : MemoryCachingComponent<BeatmapDifficultyCache.DifficultyCacheLookup, StarDifficulty?>
{
// Too many simultaneous updates can lead to stutters. One thread seems to work fine for song select display purposes.
private readonly ThreadedTaskScheduler updateScheduler = new ThreadedTaskScheduler(1, nameof(BeatmapDifficultyCache));
@ -120,9 +120,13 @@ namespace osu.Game.Beatmaps
/// <param name="rulesetInfo">The <see cref="IRulesetInfo"/> to get the difficulty with.</param>
/// <param name="mods">The <see cref="Mod"/>s to get the difficulty with.</param>
/// <param name="cancellationToken">An optional <see cref="CancellationToken"/> which stops computing the star difficulty.</param>
/// <returns>The <see cref="StarDifficulty"/>.</returns>
public virtual Task<StarDifficulty> GetDifficultyAsync([NotNull] IBeatmapInfo beatmapInfo, [CanBeNull] IRulesetInfo rulesetInfo = null,
[CanBeNull] IEnumerable<Mod> mods = null, CancellationToken cancellationToken = default)
/// <returns>
/// The requested <see cref="StarDifficulty"/>, if non-<see langword="null"/>.
/// A <see langword="null"/> return value indicates that the difficulty process failed or was interrupted early,
/// and as such there is no usable star difficulty value to be returned.
/// </returns>
public virtual Task<StarDifficulty?> GetDifficultyAsync([NotNull] IBeatmapInfo beatmapInfo, [CanBeNull] IRulesetInfo rulesetInfo = null,
[CanBeNull] IEnumerable<Mod> mods = null, CancellationToken cancellationToken = default)
{
// In the case that the user hasn't given us a ruleset, use the beatmap's default ruleset.
rulesetInfo ??= beatmapInfo.Ruleset;
@ -134,13 +138,13 @@ namespace osu.Game.Beatmaps
if (localBeatmapInfo == null || localBeatmapInfo.ID == 0 || localRulesetInfo == null)
{
// If not, fall back to the existing star difficulty (e.g. from an online source).
return Task.FromResult(new StarDifficulty(beatmapInfo.StarRating, (beatmapInfo as IBeatmapOnlineInfo)?.MaxCombo ?? 0));
return Task.FromResult<StarDifficulty?>(new StarDifficulty(beatmapInfo.StarRating, (beatmapInfo as IBeatmapOnlineInfo)?.MaxCombo ?? 0));
}
return GetAsync(new DifficultyCacheLookup(localBeatmapInfo, localRulesetInfo, mods), cancellationToken);
}
protected override Task<StarDifficulty> ComputeValueAsync(DifficultyCacheLookup lookup, CancellationToken cancellationToken = default)
protected override Task<StarDifficulty?> ComputeValueAsync(DifficultyCacheLookup lookup, CancellationToken cancellationToken = default)
{
return Task.Factory.StartNew(() =>
{
@ -151,6 +155,8 @@ namespace osu.Game.Beatmaps
}, cancellationToken, TaskCreationOptions.HideScheduler | TaskCreationOptions.RunContinuationsAsynchronously, updateScheduler);
}
protected override bool CacheNullValues => false;
public Task<List<TimedDifficultyAttributes>> GetTimedDifficultyAttributesAsync(IWorkingBeatmap beatmap, Ruleset ruleset, Mod[] mods, CancellationToken cancellationToken = default)
{
return Task.Factory.StartNew(() => ruleset.CreateDifficultyCalculator(beatmap).CalculateTimed(mods, cancellationToken),
@ -260,7 +266,7 @@ namespace osu.Game.Beatmaps
// We're on a threadpool thread, but we should exit back to the update thread so consumers can safely handle value-changed events.
Schedule(() =>
{
if (!cancellationToken.IsCancellationRequested)
if (!cancellationToken.IsCancellationRequested && t.Result != null)
bindable.Value = t.Result;
});
}, cancellationToken);
@ -272,7 +278,7 @@ namespace osu.Game.Beatmaps
/// <param name="key">The <see cref="DifficultyCacheLookup"/> that defines the computation parameters.</param>
/// <param name="cancellationToken">The cancellation token.</param>
/// <returns>The <see cref="StarDifficulty"/>.</returns>
private StarDifficulty computeDifficulty(in DifficultyCacheLookup key, CancellationToken cancellationToken = default)
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;
@ -293,11 +299,11 @@ namespace osu.Game.Beatmaps
if (rulesetInfo.Equals(beatmapInfo.Ruleset))
Logger.Error(e, $"Failed to convert {beatmapInfo.OnlineID} to the beatmap's default ruleset ({beatmapInfo.Ruleset}).");
return new StarDifficulty();
return null;
}
catch
{
return new StarDifficulty();
return null;
}
}

View File

@ -162,7 +162,12 @@ namespace osu.Game.Scoring
// We can compute the max combo locally after the async beatmap difficulty computation.
var difficulty = await difficulties().GetDifficultyAsync(score.BeatmapInfo, score.Ruleset, score.Mods, cancellationToken).ConfigureAwait(false);
beatmapMaxCombo = difficulty.MaxCombo;
// Something failed during difficulty calculation. Fall back to provided score.
if (difficulty == null)
return score.TotalScore;
beatmapMaxCombo = difficulty.Value.MaxCombo;
}
}
else

View File

@ -37,12 +37,12 @@ namespace osu.Game.Scoring
var attributes = await difficultyCache.GetDifficultyAsync(score.BeatmapInfo, score.Ruleset, score.Mods, token).ConfigureAwait(false);
// Performance calculation requires the beatmap and ruleset to be locally available. If not, return a default value.
if (attributes.Attributes == null)
if (attributes?.Attributes == null)
return null;
token.ThrowIfCancellationRequested();
var calculator = score.Ruleset.CreateInstance().CreatePerformanceCalculator(attributes.Attributes, score);
var calculator = score.Ruleset.CreateInstance().CreatePerformanceCalculator(attributes.Value.Attributes, score);
return calculator?.Calculate();
}

View File

@ -143,14 +143,6 @@ namespace osu.Game.Screens.Ranking.Expanded
Origin = Anchor.TopCentre,
AutoSizeAxes = Axes.Both,
Spacing = new Vector2(5, 0),
Children = new Drawable[]
{
new StarRatingDisplay(starDifficulty)
{
Anchor = Anchor.CentreLeft,
Origin = Anchor.CentreLeft
},
}
},
new FillFlowContainer
{
@ -231,6 +223,15 @@ namespace osu.Game.Screens.Ranking.Expanded
if (score.Date != default)
AddInternal(new PlayedOnText(score.Date));
if (starDifficulty != null)
{
starAndModDisplay.Add(new StarRatingDisplay(starDifficulty.Value)
{
Anchor = Anchor.CentreLeft,
Origin = Anchor.CentreLeft
});
}
if (score.Mods.Any())
{
starAndModDisplay.Add(new ModDisplay

View File

@ -152,7 +152,10 @@ namespace osu.Game.Screens.Select.Details
Task.WhenAll(normalStarDifficulty, moddedStarDifficulty).ContinueWith(_ => Schedule(() =>
{
starDifficulty.Value = ((float)normalStarDifficulty.Result.Stars, (float)moddedStarDifficulty.Result.Stars);
if (normalStarDifficulty.Result == null || moddedStarDifficulty.Result == null)
return;
starDifficulty.Value = ((float)normalStarDifficulty.Result.Value.Stars, (float)moddedStarDifficulty.Result.Value.Stars);
}), starDifficultyCancellationSource.Token, TaskContinuationOptions.OnlyOnRanToCompletion, TaskScheduler.Current);
}