diff --git a/osu.Game.Tests/Beatmaps/TestSceneBeatmapDifficultyCache.cs b/osu.Game.Tests/Beatmaps/TestSceneBeatmapDifficultyCache.cs index 7a05a3da5c..b63c0a8196 100644 --- a/osu.Game.Tests/Beatmaps/TestSceneBeatmapDifficultyCache.cs +++ b/osu.Game.Tests/Beatmaps/TestSceneBeatmapDifficultyCache.cs @@ -4,6 +4,7 @@ #nullable disable using System; +using System.Collections.Generic; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -15,6 +16,7 @@ using osu.Framework.Testing; using osu.Game.Beatmaps; using osu.Game.Rulesets; using osu.Game.Rulesets.Mods; +using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Osu.Mods; using osu.Game.Tests.Beatmaps.IO; using osu.Game.Tests.Visual; @@ -34,6 +36,12 @@ namespace osu.Game.Tests.Beatmaps private IBindable starDifficultyBindable; + [Resolved] + private BeatmapManager beatmapManager { get; set; } + + [Resolved] + private BeatmapDifficultyCache actualDifficultyCache { get; set; } + [BackgroundDependencyLoader] private void load(OsuGameBase osu) { @@ -55,6 +63,36 @@ namespace osu.Game.Tests.Beatmaps AddUntilStep($"star difficulty -> {BASE_STARS}", () => starDifficultyBindable.Value.Stars == BASE_STARS); } + [Test] + public void TestInvalidationFlow() + { + BeatmapInfo postEditBeatmapInfo = null; + BeatmapInfo preEditBeatmapInfo = null; + + IBindable bindableDifficulty = null; + + AddStep("get bindable stars", () => + { + preEditBeatmapInfo = importedSet.Beatmaps.First(); + bindableDifficulty = actualDifficultyCache.GetBindableDifficulty(preEditBeatmapInfo); + }); + + AddUntilStep("wait for stars retrieved", () => bindableDifficulty.Value.Stars, () => Is.GreaterThan(0)); + + AddStep("remove all hitobjects", () => + { + var working = beatmapManager.GetWorkingBeatmap(preEditBeatmapInfo); + + ((IList)working.Beatmap.HitObjects).Clear(); + + beatmapManager.Save(working.BeatmapInfo, working.Beatmap); + postEditBeatmapInfo = working.BeatmapInfo; + }); + + AddAssert("stars is now zero", () => actualDifficultyCache.GetDifficultyAsync(postEditBeatmapInfo).GetResultSafely()!.Value.Stars, () => Is.Zero); + AddUntilStep("bindable stars is now zero", () => bindableDifficulty.Value.Stars, () => Is.Zero); + } + [Test] public void TestStarDifficultyChangesOnModSettings() { @@ -122,8 +160,10 @@ namespace osu.Game.Tests.Beatmaps [Test] public void TestKeyDoesntEqualWithDifferentModSettings() { - var key1 = new BeatmapDifficultyCache.DifficultyCacheLookup(new BeatmapInfo { ID = guid }, new RulesetInfo { OnlineID = 0 }, new Mod[] { new OsuModDoubleTime { SpeedChange = { Value = 1.1 } } }); - var key2 = new BeatmapDifficultyCache.DifficultyCacheLookup(new BeatmapInfo { ID = guid }, new RulesetInfo { OnlineID = 0 }, new Mod[] { new OsuModDoubleTime { SpeedChange = { Value = 1.9 } } }); + var key1 = new BeatmapDifficultyCache.DifficultyCacheLookup(new BeatmapInfo { ID = guid }, new RulesetInfo { OnlineID = 0 }, + new Mod[] { new OsuModDoubleTime { SpeedChange = { Value = 1.1 } } }); + var key2 = new BeatmapDifficultyCache.DifficultyCacheLookup(new BeatmapInfo { ID = guid }, new RulesetInfo { OnlineID = 0 }, + new Mod[] { new OsuModDoubleTime { SpeedChange = { Value = 1.9 } } }); Assert.That(key1, Is.Not.EqualTo(key2)); Assert.That(key1.GetHashCode(), Is.Not.EqualTo(key2.GetHashCode())); @@ -132,8 +172,10 @@ namespace osu.Game.Tests.Beatmaps [Test] public void TestKeyEqualWithMatchingModSettings() { - var key1 = new BeatmapDifficultyCache.DifficultyCacheLookup(new BeatmapInfo { ID = guid }, new RulesetInfo { OnlineID = 0 }, new Mod[] { new OsuModDoubleTime { SpeedChange = { Value = 1.25 } } }); - var key2 = new BeatmapDifficultyCache.DifficultyCacheLookup(new BeatmapInfo { ID = guid }, new RulesetInfo { OnlineID = 0 }, new Mod[] { new OsuModDoubleTime { SpeedChange = { Value = 1.25 } } }); + var key1 = new BeatmapDifficultyCache.DifficultyCacheLookup(new BeatmapInfo { ID = guid }, new RulesetInfo { OnlineID = 0 }, + new Mod[] { new OsuModDoubleTime { SpeedChange = { Value = 1.25 } } }); + var key2 = new BeatmapDifficultyCache.DifficultyCacheLookup(new BeatmapInfo { ID = guid }, new RulesetInfo { OnlineID = 0 }, + new Mod[] { new OsuModDoubleTime { SpeedChange = { Value = 1.25 } } }); Assert.That(key1, Is.EqualTo(key2)); Assert.That(key1.GetHashCode(), Is.EqualTo(key2.GetHashCode())); diff --git a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs index 4ef484cb67..291239e350 100644 --- a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs +++ b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs @@ -75,6 +75,10 @@ namespace osu.Game.Beatmaps currentMods.BindValueChanged(mods => { + // A change in bindable here doesn't guarantee that mods have actually changed. + if (mods.OldValue.SequenceEqual(mods.NewValue)) + return; + modSettingChangeTracker?.Dispose(); Scheduler.AddOnce(updateTrackedBindables); @@ -82,15 +86,37 @@ namespace osu.Game.Beatmaps modSettingChangeTracker = new ModSettingChangeTracker(mods.NewValue); modSettingChangeTracker.SettingChanged += _ => { - debouncedModSettingsChange?.Cancel(); - debouncedModSettingsChange = Scheduler.AddDelayed(updateTrackedBindables, 100); + lock (bindableUpdateLock) + { + debouncedModSettingsChange?.Cancel(); + debouncedModSettingsChange = Scheduler.AddDelayed(updateTrackedBindables, 100); + } }; }, true); } - public void Invalidate(IBeatmapInfo beatmap) + /// + /// Notify this cache that a beatmap has been invalidated/updated. + /// + /// The old beatmap model. + /// The updated beatmap model. + public void Invalidate(IBeatmapInfo oldBeatmap, IBeatmapInfo newBeatmap) { - base.Invalidate(lookup => lookup.BeatmapInfo.Equals(beatmap)); + base.Invalidate(lookup => lookup.BeatmapInfo.Equals(oldBeatmap)); + + lock (bindableUpdateLock) + { + bool trackedBindablesRefreshRequired = false; + + foreach (var bsd in trackedBindables.Where(bsd => bsd.BeatmapInfo.Equals(oldBeatmap))) + { + bsd.BeatmapInfo = newBeatmap; + trackedBindablesRefreshRequired = true; + } + + if (trackedBindablesRefreshRequired) + Scheduler.AddOnce(updateTrackedBindables); + } } /// @@ -195,6 +221,9 @@ namespace osu.Game.Beatmaps { lock (bindableUpdateLock) { + debouncedModSettingsChange?.Cancel(); + debouncedModSettingsChange = null; + trackedUpdateCancellationSource.Cancel(); trackedUpdateCancellationSource = new CancellationTokenSource(); @@ -348,7 +377,7 @@ namespace osu.Game.Beatmaps private class BindableStarDifficulty : Bindable { - public readonly IBeatmapInfo BeatmapInfo; + public IBeatmapInfo BeatmapInfo; public readonly CancellationToken CancellationToken; public BindableStarDifficulty(IBeatmapInfo beatmapInfo, CancellationToken cancellationToken) diff --git a/osu.Game/Beatmaps/BeatmapUpdater.cs b/osu.Game/Beatmaps/BeatmapUpdater.cs index ff23bf1242..72c69393df 100644 --- a/osu.Game/Beatmaps/BeatmapUpdater.cs +++ b/osu.Game/Beatmaps/BeatmapUpdater.cs @@ -52,11 +52,11 @@ namespace osu.Game.Beatmaps foreach (BeatmapInfo beatmap in beatmapSet.Beatmaps) { - difficultyCache.Invalidate(beatmap); - var working = workingBeatmapCache.GetWorkingBeatmap(beatmap); - var ruleset = working.BeatmapInfo.Ruleset.CreateInstance(); + difficultyCache.Invalidate(beatmap, working.BeatmapInfo); + + var ruleset = working.BeatmapInfo.Ruleset.CreateInstance(); var calculator = ruleset.CreateDifficultyCalculator(working); beatmap.StarRating = calculator.Calculate().StarRating; diff --git a/osu.Game/Beatmaps/WorkingBeatmapCache.cs b/osu.Game/Beatmaps/WorkingBeatmapCache.cs index 0bb61798dc..75f1bd2a07 100644 --- a/osu.Game/Beatmaps/WorkingBeatmapCache.cs +++ b/osu.Game/Beatmaps/WorkingBeatmapCache.cs @@ -4,6 +4,7 @@ #nullable disable using System; +using System.Diagnostics; using System.IO; using System.Linq; using JetBrains.Annotations; @@ -104,6 +105,10 @@ namespace osu.Game.Beatmaps beatmapInfo = beatmapInfo.Detach(); + // If this ever gets hit, a request has arrived with an outdated BeatmapInfo. + // An outdated BeatmapInfo may contain a reference to a previous version of the beatmap's files on disk. + Debug.Assert(confirmFileHashIsUpToDate(beatmapInfo), "working beatmap returned with outdated path"); + workingCache.Add(working = new BeatmapManagerWorkingBeatmap(beatmapInfo, this)); // best effort; may be higher than expected. @@ -113,6 +118,12 @@ namespace osu.Game.Beatmaps } } + private bool confirmFileHashIsUpToDate(BeatmapInfo beatmapInfo) + { + string refetchPath = realm.Run(r => r.Find(beatmapInfo.ID)?.File?.File.Hash); + return refetchPath == null || refetchPath == beatmapInfo.File?.File.Hash; + } + #region IResourceStorageProvider TextureStore IBeatmapResourceProvider.LargeTextureStore => largeTextureStore;