Update BeatmapDifficultyCache to use base implementation logic

osu.Game.Tests/Beatmaps/BeatmapDifficultyManagerTest.cs

@@ -3,6 +3,7 @@
 
 using NUnit.Framework;
 using osu.Game.Beatmaps;
+using osu.Game.Rulesets;
 using osu.Game.Rulesets.Mods;
 using osu.Game.Rulesets.Osu.Mods;
 
@@ -14,8 +15,8 @@ namespace osu.Game.Tests.Beatmaps
         [Test]
         public void TestKeyEqualsWithDifferentModInstances()
         {
-            var key1 = new BeatmapDifficultyCache.DifficultyCacheLookup(1234, 0, new Mod[] { new OsuModHardRock(), new OsuModHidden() });
+            var key1 = new BeatmapDifficultyCache.DifficultyCacheLookup(new BeatmapInfo { ID = 1234 }, new RulesetInfo { ID = 0 }, new Mod[] { new OsuModHardRock(), new OsuModHidden() });
-            var key2 = new BeatmapDifficultyCache.DifficultyCacheLookup(1234, 0, new Mod[] { new OsuModHardRock(), new OsuModHidden() });
+            var key2 = new BeatmapDifficultyCache.DifficultyCacheLookup(new BeatmapInfo { ID = 1234 }, new RulesetInfo { ID = 0 }, new Mod[] { new OsuModHardRock(), new OsuModHidden() });
 
             Assert.That(key1, Is.EqualTo(key2));
         }
@@ -23,8 +24,8 @@ namespace osu.Game.Tests.Beatmaps
         [Test]
         public void TestKeyEqualsWithDifferentModOrder()
         {
-            var key1 = new BeatmapDifficultyCache.DifficultyCacheLookup(1234, 0, new Mod[] { new OsuModHardRock(), new OsuModHidden() });
+            var key1 = new BeatmapDifficultyCache.DifficultyCacheLookup(new BeatmapInfo { ID = 1234 }, new RulesetInfo { ID = 0 }, new Mod[] { new OsuModHardRock(), new OsuModHidden() });
-            var key2 = new BeatmapDifficultyCache.DifficultyCacheLookup(1234, 0, new Mod[] { new OsuModHidden(), new OsuModHardRock() });
+            var key2 = new BeatmapDifficultyCache.DifficultyCacheLookup(new BeatmapInfo { ID = 1234 }, new RulesetInfo { ID = 0 }, new Mod[] { new OsuModHidden(), new OsuModHardRock() });
 
             Assert.That(key1, Is.EqualTo(key2));
         }

osu.Game/Beatmaps/BeatmapDifficultyCache.cs

@@ -86,20 +86,30 @@ namespace osu.Game.Beatmaps
         /// <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 async Task<StarDifficulty> GetDifficultyAsync([NotNull] BeatmapInfo beatmapInfo, [CanBeNull] RulesetInfo rulesetInfo = null, [CanBeNull] IEnumerable<Mod> mods = null,
+        public Task<StarDifficulty> GetDifficultyAsync([NotNull] BeatmapInfo beatmapInfo, [CanBeNull] RulesetInfo rulesetInfo = null, [CanBeNull] IEnumerable<Mod> mods = null, CancellationToken cancellationToken = default)
-                                                             CancellationToken cancellationToken = default)
         {
-            if (tryGetExisting(beatmapInfo, rulesetInfo, mods, out var existing, out var key))
+            // In the case that the user hasn't given us a ruleset, use the beatmap's default ruleset.
-                return existing;
+            rulesetInfo ??= beatmapInfo.Ruleset;
 
-            return await Task.Factory.StartNew(() =>
+            // Difficulty can only be computed if the beatmap and ruleset are locally available.
+            if (beatmapInfo.ID == 0 || rulesetInfo.ID == null)
             {
-                // Computation may have finished in a previous task.
+                // If not, fall back to the existing star difficulty (e.g. from an online source).
-                if (tryGetExisting(beatmapInfo, rulesetInfo, mods, out existing, out _))
+                return Task.FromResult(new StarDifficulty(beatmapInfo.StarDifficulty, beatmapInfo.MaxCombo ?? 0));
+            }
+
+            return GetAsync(new DifficultyCacheLookup(beatmapInfo, rulesetInfo, mods), cancellationToken);
+        }
+
+        protected override Task<StarDifficulty> ComputeValueAsync(DifficultyCacheLookup lookup, CancellationToken token = default)
+        {
+            return Task.Factory.StartNew(() =>
+            {
+                if (CheckExists(lookup, out var existing))
                     return existing;
 
-                return computeDifficulty(key, beatmapInfo, rulesetInfo);
+                return computeDifficulty(lookup);
-            }, cancellationToken, TaskCreationOptions.HideScheduler | TaskCreationOptions.RunContinuationsAsynchronously, updateScheduler);
+            }, token, TaskCreationOptions.HideScheduler | TaskCreationOptions.RunContinuationsAsynchronously, updateScheduler);
         }
 
         /// <summary>
@@ -111,10 +121,8 @@ namespace osu.Game.Beatmaps
         /// <returns>The <see cref="StarDifficulty"/>.</returns>
         public StarDifficulty GetDifficulty([NotNull] BeatmapInfo beatmapInfo, [CanBeNull] RulesetInfo rulesetInfo = null, [CanBeNull] IEnumerable<Mod> mods = null)
         {
-            if (tryGetExisting(beatmapInfo, rulesetInfo, mods, out var existing, out var key))
+            // this is safe in this usage because the only asynchronous part is handled by the local scheduler.
-                return existing;
+            return GetDifficultyAsync(beatmapInfo, rulesetInfo, mods).Result;
-
-            return computeDifficulty(key, beatmapInfo, rulesetInfo);
         }
 
         /// <summary>
@@ -207,38 +215,38 @@ namespace osu.Game.Beatmaps
         /// <param name="cancellationToken">A token that may be used to cancel this update.</param>
         private void updateBindable([NotNull] BindableStarDifficulty bindable, [CanBeNull] RulesetInfo rulesetInfo, [CanBeNull] IEnumerable<Mod> mods, CancellationToken cancellationToken = default)
         {
-            GetDifficultyAsync(bindable.Beatmap, rulesetInfo, mods, cancellationToken).ContinueWith(t =>
+            GetAsync(new DifficultyCacheLookup(bindable.Beatmap, rulesetInfo, mods), cancellationToken)
-            {
+                .ContinueWith(t =>
-                // 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)
+                    // We're on a threadpool thread, but we should exit back to the update thread so consumers can safely handle value-changed events.
-                        bindable.Value = t.Result;
+                    Schedule(() =>
-                });
+                    {
-            }, cancellationToken);
+                        if (!cancellationToken.IsCancellationRequested)
+                            bindable.Value = t.Result;
+                    });
+                }, cancellationToken);
         }
 
         /// <summary>
         /// Computes the difficulty defined by a <see cref="DifficultyCacheLookup"/> key, and stores it to the timed cache.
         /// </summary>
         /// <param name="key">The <see cref="DifficultyCacheLookup"/> that defines the computation parameters.</param>
-        /// <param name="beatmapInfo">The <see cref="BeatmapInfo"/> to compute the difficulty of.</param>
-        /// <param name="rulesetInfo">The <see cref="RulesetInfo"/> to compute the difficulty with.</param>
         /// <returns>The <see cref="StarDifficulty"/>.</returns>
-        private StarDifficulty computeDifficulty(in DifficultyCacheLookup key, BeatmapInfo beatmapInfo, RulesetInfo rulesetInfo)
+        private StarDifficulty computeDifficulty(in DifficultyCacheLookup key)
         {
             // In the case that the user hasn't given us a ruleset, use the beatmap's default ruleset.
-            rulesetInfo ??= beatmapInfo.Ruleset;
+            var beatmapInfo = key.Beatmap;
+            var rulesetInfo = key.Ruleset;
 
             try
             {
                 var ruleset = rulesetInfo.CreateInstance();
                 Debug.Assert(ruleset != null);
 
-                var calculator = ruleset.CreateDifficultyCalculator(beatmapManager.GetWorkingBeatmap(beatmapInfo));
+                var calculator = ruleset.CreateDifficultyCalculator(beatmapManager.GetWorkingBeatmap(key.Beatmap));
-                var attributes = calculator.Calculate(key.Mods);
+                var attributes = calculator.Calculate(key.OrderedMods);
 
-                return Cache[key] = new StarDifficulty(attributes);
+                return new StarDifficulty(attributes);
             }
             catch (BeatmapInvalidForRulesetException e)
             {
@@ -249,49 +257,17 @@ namespace osu.Game.Beatmaps
                 if (rulesetInfo.Equals(beatmapInfo.Ruleset))
                 {
                     Logger.Error(e, $"Failed to convert {beatmapInfo.OnlineBeatmapID} to the beatmap's default ruleset ({beatmapInfo.Ruleset}).");
-                    return Cache[key] = new StarDifficulty();
+                    return new StarDifficulty();
                 }
 
-                // Check the cache first because this is now a different ruleset than the one previously guarded against.
+                return GetAsync(new DifficultyCacheLookup(key.Beatmap, key.Beatmap.Ruleset, key.OrderedMods)).Result;
-                if (tryGetExisting(beatmapInfo, beatmapInfo.Ruleset, Array.Empty<Mod>(), out var existingDefault, out var existingDefaultKey))
-                    return existingDefault;
-
-                return computeDifficulty(existingDefaultKey, beatmapInfo, beatmapInfo.Ruleset);
             }
             catch
             {
-                return Cache[key] = new StarDifficulty();
+                return new StarDifficulty();
             }
         }
 
-        /// <summary>
-        /// Attempts to retrieve an existing difficulty for the combination.
-        /// </summary>
-        /// <param name="beatmapInfo">The <see cref="BeatmapInfo"/>.</param>
-        /// <param name="rulesetInfo">The <see cref="RulesetInfo"/>.</param>
-        /// <param name="mods">The <see cref="Mod"/>s.</param>
-        /// <param name="existingDifficulty">The existing difficulty value, if present.</param>
-        /// <param name="key">The <see cref="DifficultyCacheLookup"/> key that was used to perform this lookup. This can be further used to query <see cref="computeDifficulty"/>.</param>
-        /// <returns>Whether an existing difficulty was found.</returns>
-        private bool tryGetExisting(BeatmapInfo beatmapInfo, RulesetInfo rulesetInfo, IEnumerable<Mod> mods, out StarDifficulty existingDifficulty, out DifficultyCacheLookup key)
-        {
-            // In the case that the user hasn't given us a ruleset, use the beatmap's default ruleset.
-            rulesetInfo ??= beatmapInfo.Ruleset;
-
-            // Difficulty can only be computed if the beatmap and ruleset are locally available.
-            if (beatmapInfo.ID == 0 || rulesetInfo.ID == null)
-            {
-                // If not, fall back to the existing star difficulty (e.g. from an online source).
-                existingDifficulty = new StarDifficulty(beatmapInfo.StarDifficulty, beatmapInfo.MaxCombo ?? 0);
-                key = default;
-
-                return true;
-            }
-
-            key = new DifficultyCacheLookup(beatmapInfo.ID, rulesetInfo.ID.Value, mods);
-            return Cache.TryGetValue(key, out existingDifficulty);
-        }
-
         protected override void Dispose(bool isDisposing)
         {
             base.Dispose(isDisposing);
@@ -302,29 +278,31 @@ namespace osu.Game.Beatmaps
 
         public readonly struct DifficultyCacheLookup : IEquatable<DifficultyCacheLookup>
         {
-            public readonly int BeatmapId;
+            public readonly BeatmapInfo Beatmap;
-            public readonly int RulesetId;
+            public readonly RulesetInfo Ruleset;
-            public readonly Mod[] Mods;
 
-            public DifficultyCacheLookup(int beatmapId, int rulesetId, IEnumerable<Mod> mods)
+            public readonly Mod[] OrderedMods;
+
+            public DifficultyCacheLookup([NotNull] BeatmapInfo beatmap, [NotNull] RulesetInfo ruleset, IEnumerable<Mod> mods)
             {
-                BeatmapId = beatmapId;
+                Beatmap = beatmap;
-                RulesetId = rulesetId;
+                Ruleset = ruleset;
-                Mods = mods?.OrderBy(m => m.Acronym).ToArray() ?? Array.Empty<Mod>();
+                OrderedMods = mods?.OrderBy(m => m.Acronym).ToArray() ?? Array.Empty<Mod>();
             }
 
             public bool Equals(DifficultyCacheLookup other)
-                => BeatmapId == other.BeatmapId
+                => Beatmap.ID == other.Beatmap.ID
-                   && RulesetId == other.RulesetId
+                   && Ruleset.ID == other.Ruleset.ID
-                   && Mods.Select(m => m.Acronym).SequenceEqual(other.Mods.Select(m => m.Acronym));
+                   && OrderedMods.Select(m => m.Acronym).SequenceEqual(other.OrderedMods.Select(m => m.Acronym));
 
             public override int GetHashCode()
             {
                 var hashCode = new HashCode();
 
-                hashCode.Add(BeatmapId);
+                hashCode.Add(Beatmap.ID);
-                hashCode.Add(RulesetId);
+                hashCode.Add(Ruleset.ID);
-                foreach (var mod in Mods)
+
+                foreach (var mod in OrderedMods)
                     hashCode.Add(mod.Acronym);
 
                 return hashCode.ToHashCode();

osu.Game/Database/MemoryCachingComponent.cs

@@ -20,7 +20,7 @@ namespace osu.Game.Database
         protected virtual bool CacheNullValues => true;
 
         /// <summary>
-        /// Retrieve the cached value for the given <see cref="TLookup"/>.
+        /// Retrieve the cached value for the given lookup.
         /// </summary>
         /// <param name="lookup">The lookup to retrieve.</param>
         /// <param name="token">An optional <see cref="CancellationToken"/> to cancel the operation.</param>
@@ -37,6 +37,9 @@ namespace osu.Game.Database
             return computed;
         }
 
+        protected bool CheckExists([NotNull] TLookup lookup, out TValue value) =>
+            cache.TryGetValue(lookup, out value);
+
         /// <summary>
         /// Called on cache miss to compute the value for the specified lookup.
         /// </summary>