From 06d59b717c69ed873ca6a65cd5d02b45777f1fa4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 20 Jun 2022 18:39:53 +0900 Subject: [PATCH 01/16] Move beatmap processing tasks to new `BeatmapUpdater` class --- .../Database/BeatmapImporterTests.cs | 58 +++++++------- ...eneOnlinePlayBeatmapAvailabilityTracker.cs | 8 +- osu.Game/Beatmaps/BeatmapImporter.cs | 70 ++--------------- osu.Game/Beatmaps/BeatmapManager.cs | 13 +++- osu.Game/Beatmaps/BeatmapUpdater.cs | 78 +++++++++++++++++++ osu.Game/OsuGameBase.cs | 2 +- 6 files changed, 130 insertions(+), 99 deletions(-) create mode 100644 osu.Game/Beatmaps/BeatmapUpdater.cs diff --git a/osu.Game.Tests/Database/BeatmapImporterTests.cs b/osu.Game.Tests/Database/BeatmapImporterTests.cs index be11476c73..f54dd3eb11 100644 --- a/osu.Game.Tests/Database/BeatmapImporterTests.cs +++ b/osu.Game.Tests/Database/BeatmapImporterTests.cs @@ -35,7 +35,8 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using (var importer = new BeatmapImporter(storage, realm)) + var importer = new BeatmapImporter(storage, realm); + using (new RealmRulesetStore(realm, storage)) { var beatmapSet = await importer.Import(new ImportTask(TestResources.GetTestBeatmapStream(), "renatus.osz")); @@ -76,7 +77,8 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using (var importer = new BeatmapImporter(storage, realm)) + var importer = new BeatmapImporter(storage, realm); + using (new RealmRulesetStore(realm, storage)) { var beatmapSet = await importer.Import(new ImportTask(TestResources.GetTestBeatmapStream(), "renatus.osz")); @@ -134,7 +136,8 @@ namespace osu.Game.Tests.Database var manager = new ModelManager(storage, realm); - using (var importer = new BeatmapImporter(storage, realm)) + var importer = new BeatmapImporter(storage, realm); + using (new RealmRulesetStore(realm, storage)) { Task.Run(async () => @@ -160,7 +163,8 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using (var importer = new BeatmapImporter(storage, realm)) + var importer = new BeatmapImporter(storage, realm); + using (new RealmRulesetStore(realm, storage)) { var imported = await importer.Import(new ImportTask(TestResources.GetTestBeatmapStream(), "renatus.osz")); @@ -187,7 +191,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using var importer = new BeatmapImporter(storage, realm); + var importer = new BeatmapImporter(storage, realm); using var store = new RealmRulesetStore(realm, storage); await LoadOszIntoStore(importer, realm.Realm); @@ -199,7 +203,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using var importer = new BeatmapImporter(storage, realm); + var importer = new BeatmapImporter(storage, realm); using var store = new RealmRulesetStore(realm, storage); var imported = await LoadOszIntoStore(importer, realm.Realm); @@ -217,7 +221,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using var importer = new BeatmapImporter(storage, realm); + var importer = new BeatmapImporter(storage, realm); using var store = new RealmRulesetStore(realm, storage); var imported = await LoadOszIntoStore(importer, realm.Realm); @@ -231,7 +235,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using var importer = new BeatmapImporter(storage, realm); + var importer = new BeatmapImporter(storage, realm); using var store = new RealmRulesetStore(realm, storage); string? tempPath = TestResources.GetTestBeatmapForImport(); @@ -261,7 +265,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using var importer = new BeatmapImporter(storage, realm); + var importer = new BeatmapImporter(storage, realm); using var store = new RealmRulesetStore(realm, storage); var imported = await LoadOszIntoStore(importer, realm.Realm); @@ -281,7 +285,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using var importer = new BeatmapImporter(storage, realm); + var importer = new BeatmapImporter(storage, realm); using var store = new RealmRulesetStore(realm, storage); string? temp = TestResources.GetTestBeatmapForImport(); @@ -317,7 +321,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using var importer = new BeatmapImporter(storage, realm); + var importer = new BeatmapImporter(storage, realm); using var store = new RealmRulesetStore(realm, storage); string? temp = TestResources.GetTestBeatmapForImport(); @@ -366,7 +370,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using var importer = new BeatmapImporter(storage, realm); + var importer = new BeatmapImporter(storage, realm); using var store = new RealmRulesetStore(realm, storage); string? temp = TestResources.GetTestBeatmapForImport(); @@ -417,7 +421,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using var importer = new BeatmapImporter(storage, realm); + var importer = new BeatmapImporter(storage, realm); using var store = new RealmRulesetStore(realm, storage); string? temp = TestResources.GetTestBeatmapForImport(); @@ -465,7 +469,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using var importer = new BeatmapImporter(storage, realm); + var importer = new BeatmapImporter(storage, realm); using var store = new RealmRulesetStore(realm, storage); string? temp = TestResources.GetTestBeatmapForImport(); @@ -513,7 +517,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using var importer = new BeatmapImporter(storage, realm); + var importer = new BeatmapImporter(storage, realm); using var store = new RealmRulesetStore(realm, storage); var imported = await LoadOszIntoStore(importer, realm.Realm); @@ -548,7 +552,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using var importer = new BeatmapImporter(storage, realm); + var importer = new BeatmapImporter(storage, realm); using var store = new RealmRulesetStore(realm, storage); var progressNotification = new ImportProgressNotification(); @@ -586,7 +590,7 @@ namespace osu.Game.Tests.Database Interlocked.Increment(ref loggedExceptionCount); }; - using var importer = new BeatmapImporter(storage, realm); + var importer = new BeatmapImporter(storage, realm); using var store = new RealmRulesetStore(realm, storage); var imported = await LoadOszIntoStore(importer, realm.Realm); @@ -638,7 +642,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using var importer = new BeatmapImporter(storage, realm); + var importer = new BeatmapImporter(storage, realm); using var store = new RealmRulesetStore(realm, storage); var imported = await LoadOszIntoStore(importer, realm.Realm, batchImport: true); @@ -665,7 +669,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realmFactory, storage) => { - using var importer = new BeatmapImporter(storage, realmFactory); + var importer = new BeatmapImporter(storage, realmFactory); using var store = new RealmRulesetStore(realmFactory, storage); var imported = await LoadOszIntoStore(importer, realmFactory.Realm); @@ -697,7 +701,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using var importer = new BeatmapImporter(storage, realm); + var importer = new BeatmapImporter(storage, realm); using var store = new RealmRulesetStore(realm, storage); var imported = await LoadOszIntoStore(importer, realm.Realm); @@ -724,7 +728,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using var importer = new BeatmapImporter(storage, realm); + var importer = new BeatmapImporter(storage, realm); using var store = new RealmRulesetStore(realm, storage); var imported = await LoadOszIntoStore(importer, realm.Realm); @@ -750,7 +754,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealm((realm, storage) => { - using var importer = new BeatmapImporter(storage, realm); + var importer = new BeatmapImporter(storage, realm); using var store = new RealmRulesetStore(realm, storage); var metadata = new BeatmapMetadata @@ -798,7 +802,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using var importer = new BeatmapImporter(storage, realm); + var importer = new BeatmapImporter(storage, realm); using var store = new RealmRulesetStore(realm, storage); string? temp = TestResources.GetTestBeatmapForImport(); @@ -815,7 +819,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using var importer = new BeatmapImporter(storage, realm); + var importer = new BeatmapImporter(storage, realm); using var store = new RealmRulesetStore(realm, storage); string? temp = TestResources.GetTestBeatmapForImport(); @@ -851,7 +855,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using var importer = new BeatmapImporter(storage, realm); + var importer = new BeatmapImporter(storage, realm); using var store = new RealmRulesetStore(realm, storage); string? temp = TestResources.GetTestBeatmapForImport(); @@ -893,7 +897,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using var importer = new BeatmapImporter(storage, realm); + var importer = new BeatmapImporter(storage, realm); using var store = new RealmRulesetStore(realm, storage); string? temp = TestResources.GetTestBeatmapForImport(); @@ -944,7 +948,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using var importer = new BeatmapImporter(storage, realm); + var importer = new BeatmapImporter(storage, realm); using var store = new RealmRulesetStore(realm, storage); string? temp = TestResources.GetTestBeatmapForImport(); diff --git a/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs b/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs index 1d33a895eb..0bf47141e4 100644 --- a/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs +++ b/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs @@ -212,17 +212,17 @@ namespace osu.Game.Tests.Online { } - protected override BeatmapImporter CreateBeatmapImporter(Storage storage, RealmAccess realm, RulesetStore rulesets, BeatmapOnlineLookupQueue onlineLookupQueue) + protected override BeatmapImporter CreateBeatmapImporter(Storage storage, RealmAccess realm, RulesetStore rulesets, BeatmapUpdater beatmapUpdater) { - return new TestBeatmapImporter(this, storage, realm, onlineLookupQueue); + return new TestBeatmapImporter(this, storage, realm, beatmapUpdater); } internal class TestBeatmapImporter : BeatmapImporter { private readonly TestBeatmapManager testBeatmapManager; - public TestBeatmapImporter(TestBeatmapManager testBeatmapManager, Storage storage, RealmAccess databaseAccess, BeatmapOnlineLookupQueue beatmapOnlineLookupQueue) - : base(storage, databaseAccess, beatmapOnlineLookupQueue) + public TestBeatmapImporter(TestBeatmapManager testBeatmapManager, Storage storage, RealmAccess databaseAccess, BeatmapUpdater beatmapUpdater) + : base(storage, databaseAccess, beatmapUpdater) { this.testBeatmapManager = testBeatmapManager; } diff --git a/osu.Game/Beatmaps/BeatmapImporter.cs b/osu.Game/Beatmaps/BeatmapImporter.cs index 237088036c..0620e5d9b1 100644 --- a/osu.Game/Beatmaps/BeatmapImporter.cs +++ b/osu.Game/Beatmaps/BeatmapImporter.cs @@ -6,10 +6,8 @@ using System.Collections.Generic; using System.IO; using System.Linq; using System.Threading; -using osu.Framework.Audio.Track; using osu.Framework.Extensions; using osu.Framework.Extensions.IEnumerableExtensions; -using osu.Framework.Graphics.Textures; using osu.Framework.Logging; using osu.Framework.Platform; using osu.Framework.Testing; @@ -20,8 +18,6 @@ using osu.Game.IO; using osu.Game.IO.Archives; using osu.Game.Models; using osu.Game.Rulesets; -using osu.Game.Rulesets.Objects; -using osu.Game.Skinning; using Realms; namespace osu.Game.Beatmaps @@ -30,18 +26,18 @@ namespace osu.Game.Beatmaps /// Handles the storage and retrieval of Beatmaps/WorkingBeatmaps. /// [ExcludeFromDynamicCompile] - public class BeatmapImporter : RealmArchiveModelImporter, IDisposable + public class BeatmapImporter : RealmArchiveModelImporter { public override IEnumerable HandledExtensions => new[] { ".osz" }; protected override string[] HashableFileTypes => new[] { ".osu" }; - private readonly BeatmapOnlineLookupQueue? onlineLookupQueue; + private readonly BeatmapUpdater? beatmapUpdater; - public BeatmapImporter(Storage storage, RealmAccess realm, BeatmapOnlineLookupQueue? onlineLookupQueue = null) + public BeatmapImporter(Storage storage, RealmAccess realm, BeatmapUpdater? beatmapUpdater = null) : base(storage, realm) { - this.onlineLookupQueue = onlineLookupQueue; + this.beatmapUpdater = beatmapUpdater; } protected override bool ShouldDeleteArchive(string path) => Path.GetExtension(path).ToLowerInvariant() == ".osz"; @@ -65,8 +61,7 @@ namespace osu.Game.Beatmaps bool hadOnlineIDs = beatmapSet.Beatmaps.Any(b => b.OnlineID > 0); - onlineLookupQueue?.Update(beatmapSet); - + // TODO: this may no longer be valid as we aren't doing an online population at this point. // ensure at least one beatmap was able to retrieve or keep an online ID, else drop the set ID. if (hadOnlineIDs && !beatmapSet.Beatmaps.Any(b => b.OnlineID > 0)) { @@ -85,6 +80,8 @@ namespace osu.Game.Beatmaps // If this is ever an issue, we can consider marking as pending delete but not resetting the IDs (but care will be required for // beatmaps, which don't have their own `DeletePending` state). + beatmapUpdater?.Process(beatmapSet, realm); + if (beatmapSet.OnlineID > 0) { var existingSetWithSameOnlineID = realm.All().SingleOrDefault(b => b.OnlineID == beatmapSet.OnlineID); @@ -278,64 +275,11 @@ namespace osu.Game.Beatmaps MD5Hash = memoryStream.ComputeMD5Hash(), }; - updateBeatmapStatistics(beatmap, decoded); - beatmaps.Add(beatmap); } } return beatmaps; } - - private void updateBeatmapStatistics(BeatmapInfo beatmap, IBeatmap decoded) - { - var rulesetInstance = ((IRulesetInfo)beatmap.Ruleset).CreateInstance(); - - decoded.BeatmapInfo.Ruleset = rulesetInstance.RulesetInfo; - - // TODO: this should be done in a better place once we actually need to dynamically update it. - beatmap.StarRating = rulesetInstance.CreateDifficultyCalculator(new DummyConversionBeatmap(decoded)).Calculate().StarRating; - beatmap.Length = calculateLength(decoded); - beatmap.BPM = 60000 / decoded.GetMostCommonBeatLength(); - } - - private double calculateLength(IBeatmap b) - { - if (!b.HitObjects.Any()) - return 0; - - var lastObject = b.HitObjects.Last(); - - //TODO: this isn't always correct (consider mania where a non-last object may last for longer than the last in the list). - double endTime = lastObject.GetEndTime(); - double startTime = b.HitObjects.First().StartTime; - - return endTime - startTime; - } - - public void Dispose() - { - onlineLookupQueue?.Dispose(); - } - - /// - /// A dummy WorkingBeatmap for the purpose of retrieving a beatmap for star difficulty calculation. - /// - private class DummyConversionBeatmap : WorkingBeatmap - { - private readonly IBeatmap beatmap; - - public DummyConversionBeatmap(IBeatmap beatmap) - : base(beatmap.BeatmapInfo, null) - { - this.beatmap = beatmap; - } - - protected override IBeatmap GetBeatmap() => beatmap; - protected override Texture? GetBackground() => null; - protected override Track? GetBeatmapTrack() => null; - protected internal override ISkin? GetSkin() => null; - public override Stream? GetStream(string storagePath) => null; - } } } diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 670dba14ec..ee7fcb5b47 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -42,9 +42,10 @@ namespace osu.Game.Beatmaps private readonly WorkingBeatmapCache workingBeatmapCache; private readonly BeatmapOnlineLookupQueue? onlineBeatmapLookupQueue; + private readonly BeatmapUpdater? beatmapUpdater; public BeatmapManager(Storage storage, RealmAccess realm, RulesetStore rulesets, IAPIProvider? api, AudioManager audioManager, IResourceStore gameResources, GameHost? host = null, - WorkingBeatmap? defaultBeatmap = null, bool performOnlineLookups = false) + WorkingBeatmap? defaultBeatmap = null, BeatmapDifficultyCache? difficultyCache = null, bool performOnlineLookups = false) : base(storage, realm) { if (performOnlineLookups) @@ -52,14 +53,18 @@ namespace osu.Game.Beatmaps if (api == null) throw new ArgumentNullException(nameof(api), "API must be provided if online lookups are required."); + if (difficultyCache == null) + throw new ArgumentNullException(nameof(difficultyCache), "Difficulty cache must be provided if online lookups are required."); + onlineBeatmapLookupQueue = new BeatmapOnlineLookupQueue(api, storage); + beatmapUpdater = new BeatmapUpdater(this, onlineBeatmapLookupQueue, difficultyCache); } var userResources = new RealmFileStore(realm, storage).Store; BeatmapTrackStore = audioManager.GetTrackStore(userResources); - beatmapImporter = CreateBeatmapImporter(storage, realm, rulesets, onlineBeatmapLookupQueue); + beatmapImporter = CreateBeatmapImporter(storage, realm, rulesets, beatmapUpdater); beatmapImporter.PostNotification = obj => PostNotification?.Invoke(obj); workingBeatmapCache = CreateWorkingBeatmapCache(audioManager, gameResources, userResources, defaultBeatmap, host); @@ -71,8 +76,8 @@ namespace osu.Game.Beatmaps return new WorkingBeatmapCache(BeatmapTrackStore, audioManager, resources, storage, defaultBeatmap, host); } - protected virtual BeatmapImporter CreateBeatmapImporter(Storage storage, RealmAccess realm, RulesetStore rulesets, BeatmapOnlineLookupQueue? onlineLookupQueue) => - new BeatmapImporter(storage, realm, onlineLookupQueue); + protected virtual BeatmapImporter CreateBeatmapImporter(Storage storage, RealmAccess realm, RulesetStore rulesets, BeatmapUpdater? beatmapUpdater) => + new BeatmapImporter(storage, realm, beatmapUpdater); /// /// Create a new beatmap set, backed by a model, diff --git a/osu.Game/Beatmaps/BeatmapUpdater.cs b/osu.Game/Beatmaps/BeatmapUpdater.cs new file mode 100644 index 0000000000..8ccaf893a7 --- /dev/null +++ b/osu.Game/Beatmaps/BeatmapUpdater.cs @@ -0,0 +1,78 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +#nullable enable + +using System.Linq; +using System.Threading.Tasks; +using osu.Framework.Extensions; +using osu.Game.Database; +using osu.Game.Rulesets.Objects; + +namespace osu.Game.Beatmaps +{ + /// + /// Handles all processing required to ensure a local beatmap is in a consistent state with any changes. + /// + public class BeatmapUpdater + { + private readonly BeatmapManager beatmapManager; + private readonly BeatmapOnlineLookupQueue onlineLookupQueue; + private readonly BeatmapDifficultyCache difficultyCache; + + public BeatmapUpdater(BeatmapManager beatmapManager, BeatmapOnlineLookupQueue onlineLookupQueue, BeatmapDifficultyCache difficultyCache) + { + this.beatmapManager = beatmapManager; + this.onlineLookupQueue = onlineLookupQueue; + this.difficultyCache = difficultyCache; + } + + /// + /// Queue a beatmap for background processing. + /// + public void Queue(Live beatmap) + { + // For now, just fire off a task. + // TODO: Add actual queueing probably. + Task.Factory.StartNew(() => beatmap.PerformRead(Process)); + } + + /// + /// Run all processing on a beatmap immediately. + /// + public void Process(BeatmapSetInfo beatmapSet) + { + beatmapSet.Realm.Write(() => + { + onlineLookupQueue.Update(beatmapSet); + + foreach (var beatmap in beatmapSet.Beatmaps) + { + var working = beatmapManager.GetWorkingBeatmap(beatmap); + + // Because we aren't guaranteed all processing will happen on this thread, it's very hard to use the live realm object. + // This can be fixed by adding a synchronous flow to `BeatmapDifficultyCache`. + var detachedBeatmap = beatmap.Detach(); + + beatmap.StarRating = difficultyCache.GetDifficultyAsync(detachedBeatmap).GetResultSafely()?.Stars ?? 0; + beatmap.Length = calculateLength(working.Beatmap); + beatmap.BPM = 60000 / working.Beatmap.GetMostCommonBeatLength(); + } + }); + } + + private double calculateLength(IBeatmap b) + { + if (!b.HitObjects.Any()) + return 0; + + var lastObject = b.HitObjects.Last(); + + //TODO: this isn't always correct (consider mania where a non-last object may last for longer than the last in the list). + double endTime = lastObject.GetEndTime(); + double startTime = b.HitObjects.First().StartTime; + + return endTime - startTime; + } + } +} diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 7bbad3bb72..403c5d1b49 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -272,7 +272,7 @@ namespace osu.Game // ordering is important here to ensure foreign keys rules are not broken in ModelStore.Cleanup() dependencies.Cache(ScoreManager = new ScoreManager(RulesetStore, () => BeatmapManager, Storage, realm, Scheduler, difficultyCache, LocalConfig)); - dependencies.Cache(BeatmapManager = new BeatmapManager(Storage, realm, RulesetStore, API, Audio, Resources, Host, defaultBeatmap, performOnlineLookups: true)); + dependencies.Cache(BeatmapManager = new BeatmapManager(Storage, realm, RulesetStore, API, Audio, Resources, Host, defaultBeatmap, difficultyCache, performOnlineLookups: true)); dependencies.Cache(BeatmapDownloader = new BeatmapModelDownloader(BeatmapManager, API)); dependencies.Cache(ScoreDownloader = new ScoreModelDownloader(ScoreManager, API)); From fe570d8052356d3c5a64410719e3ea716a920f17 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 20 Jun 2022 18:57:57 +0900 Subject: [PATCH 02/16] Queue beatmaps for update after editing --- osu.Game/Beatmaps/BeatmapManager.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index ee7fcb5b47..1b85207f22 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -324,6 +324,10 @@ namespace osu.Game.Beatmaps workingBeatmapCache.Invalidate(beatmapInfo); + Debug.Assert(beatmapInfo.BeatmapSet != null); + + beatmapUpdater?.Queue(Realm.Run(r => r.Find(setInfo.ID).ToLive(Realm))); + static string createBeatmapFilenameFromMetadata(BeatmapInfo beatmapInfo) { var metadata = beatmapInfo.Metadata; From 021b16f2f3635469fd5b9673424582dc3dfafebf Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 20 Jun 2022 19:25:18 +0900 Subject: [PATCH 03/16] Ensure `WorkingBeatmap` is invalidated after update --- osu.Game/Beatmaps/BeatmapUpdater.cs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapUpdater.cs b/osu.Game/Beatmaps/BeatmapUpdater.cs index 8ccaf893a7..7689f4d007 100644 --- a/osu.Game/Beatmaps/BeatmapUpdater.cs +++ b/osu.Game/Beatmaps/BeatmapUpdater.cs @@ -16,13 +16,13 @@ namespace osu.Game.Beatmaps /// public class BeatmapUpdater { - private readonly BeatmapManager beatmapManager; + private readonly IWorkingBeatmapCache workingBeatmapCache; private readonly BeatmapOnlineLookupQueue onlineLookupQueue; private readonly BeatmapDifficultyCache difficultyCache; - public BeatmapUpdater(BeatmapManager beatmapManager, BeatmapOnlineLookupQueue onlineLookupQueue, BeatmapDifficultyCache difficultyCache) + public BeatmapUpdater(IWorkingBeatmapCache workingBeatmapCache, BeatmapOnlineLookupQueue onlineLookupQueue, BeatmapDifficultyCache difficultyCache) { - this.beatmapManager = beatmapManager; + this.workingBeatmapCache = workingBeatmapCache; this.onlineLookupQueue = onlineLookupQueue; this.difficultyCache = difficultyCache; } @@ -48,17 +48,19 @@ namespace osu.Game.Beatmaps foreach (var beatmap in beatmapSet.Beatmaps) { - var working = beatmapManager.GetWorkingBeatmap(beatmap); - // Because we aren't guaranteed all processing will happen on this thread, it's very hard to use the live realm object. // This can be fixed by adding a synchronous flow to `BeatmapDifficultyCache`. var detachedBeatmap = beatmap.Detach(); beatmap.StarRating = difficultyCache.GetDifficultyAsync(detachedBeatmap).GetResultSafely()?.Stars ?? 0; + + var working = workingBeatmapCache.GetWorkingBeatmap(beatmap); beatmap.Length = calculateLength(working.Beatmap); beatmap.BPM = 60000 / working.Beatmap.GetMostCommonBeatLength(); } }); + + workingBeatmapCache.Invalidate(beatmapSet); } private double calculateLength(IBeatmap b) From 6999933d33a4289f4dd5645079f2fb2d3f8f04e1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 24 Jun 2022 18:27:47 +0900 Subject: [PATCH 04/16] Split updater process into realm transaction and non-transaction --- osu.Game/Beatmaps/BeatmapManager.cs | 13 ++++++---- osu.Game/Beatmaps/BeatmapUpdater.cs | 39 +++++++++++++++-------------- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 1b85207f22..effa0ab094 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -319,15 +319,18 @@ namespace osu.Game.Beatmaps AddFile(setInfo, stream, createBeatmapFilenameFromMetadata(beatmapInfo)); - Realm.Write(r => setInfo.CopyChangesToRealm(r.Find(setInfo.ID))); + Realm.Write(r => + { + var liveBeatmapSet = r.Find(setInfo.ID); + + setInfo.CopyChangesToRealm(liveBeatmapSet); + + beatmapUpdater?.Process(liveBeatmapSet, r); + }); } - workingBeatmapCache.Invalidate(beatmapInfo); - Debug.Assert(beatmapInfo.BeatmapSet != null); - beatmapUpdater?.Queue(Realm.Run(r => r.Find(setInfo.ID).ToLive(Realm))); - static string createBeatmapFilenameFromMetadata(BeatmapInfo beatmapInfo) { var metadata = beatmapInfo.Metadata; diff --git a/osu.Game/Beatmaps/BeatmapUpdater.cs b/osu.Game/Beatmaps/BeatmapUpdater.cs index 7689f4d007..a24407a734 100644 --- a/osu.Game/Beatmaps/BeatmapUpdater.cs +++ b/osu.Game/Beatmaps/BeatmapUpdater.cs @@ -8,6 +8,7 @@ using System.Threading.Tasks; using osu.Framework.Extensions; using osu.Game.Database; using osu.Game.Rulesets.Objects; +using Realms; namespace osu.Game.Beatmaps { @@ -40,27 +41,27 @@ namespace osu.Game.Beatmaps /// /// Run all processing on a beatmap immediately. /// - public void Process(BeatmapSetInfo beatmapSet) + public void Process(BeatmapSetInfo beatmapSet) => beatmapSet.Realm.Write(r => Process(beatmapSet, r)); + + public void Process(BeatmapSetInfo beatmapSet, Realm realm) { - beatmapSet.Realm.Write(() => - { - onlineLookupQueue.Update(beatmapSet); - - foreach (var beatmap in beatmapSet.Beatmaps) - { - // Because we aren't guaranteed all processing will happen on this thread, it's very hard to use the live realm object. - // This can be fixed by adding a synchronous flow to `BeatmapDifficultyCache`. - var detachedBeatmap = beatmap.Detach(); - - beatmap.StarRating = difficultyCache.GetDifficultyAsync(detachedBeatmap).GetResultSafely()?.Stars ?? 0; - - var working = workingBeatmapCache.GetWorkingBeatmap(beatmap); - beatmap.Length = calculateLength(working.Beatmap); - beatmap.BPM = 60000 / working.Beatmap.GetMostCommonBeatLength(); - } - }); - + // Before we use below, we want to invalidate. workingBeatmapCache.Invalidate(beatmapSet); + + onlineLookupQueue.Update(beatmapSet); + + foreach (var beatmap in beatmapSet.Beatmaps) + { + // Because we aren't guaranteed all processing will happen on this thread, it's very hard to use the live realm object. + // This can be fixed by adding a synchronous flow to `BeatmapDifficultyCache`. + var detachedBeatmap = beatmap.Detach(); + + beatmap.StarRating = difficultyCache.GetDifficultyAsync(detachedBeatmap).GetResultSafely()?.Stars ?? 0; + + var working = workingBeatmapCache.GetWorkingBeatmap(beatmap); + beatmap.Length = calculateLength(working.Beatmap); + beatmap.BPM = 60000 / working.Beatmap.GetMostCommonBeatLength(); + } } private double calculateLength(IBeatmap b) From 66a01d1ed29a261a0b5a490eef1e63b0177d2a5c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 20 Jun 2022 19:48:46 +0900 Subject: [PATCH 05/16] Allow song select to refresh the global `WorkingBeatmap` after an external update --- osu.Game/Beatmaps/BeatmapManager.cs | 6 ++++++ osu.Game/Beatmaps/IWorkingBeatmapCache.cs | 4 ++++ osu.Game/Beatmaps/WorkingBeatmapCache.cs | 3 +++ osu.Game/Screens/Select/SongSelect.cs | 18 ++++++++++++++++++ 4 files changed, 31 insertions(+) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index effa0ab094..ff176484b6 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -453,6 +453,12 @@ namespace osu.Game.Beatmaps void IWorkingBeatmapCache.Invalidate(BeatmapSetInfo beatmapSetInfo) => workingBeatmapCache.Invalidate(beatmapSetInfo); void IWorkingBeatmapCache.Invalidate(BeatmapInfo beatmapInfo) => workingBeatmapCache.Invalidate(beatmapInfo); + public event Action? OnInvalidated + { + add => workingBeatmapCache.OnInvalidated += value; + remove => workingBeatmapCache.OnInvalidated -= value; + } + public override bool IsAvailableLocally(BeatmapSetInfo model) => Realm.Run(realm => realm.All().Any(s => s.OnlineID == model.OnlineID)); #endregion diff --git a/osu.Game/Beatmaps/IWorkingBeatmapCache.cs b/osu.Game/Beatmaps/IWorkingBeatmapCache.cs index ad9cac6957..36c53fce99 100644 --- a/osu.Game/Beatmaps/IWorkingBeatmapCache.cs +++ b/osu.Game/Beatmaps/IWorkingBeatmapCache.cs @@ -3,6 +3,8 @@ #nullable disable +using System; + namespace osu.Game.Beatmaps { public interface IWorkingBeatmapCache @@ -25,5 +27,7 @@ namespace osu.Game.Beatmaps /// /// The beatmap info to invalidate any cached entries for. void Invalidate(BeatmapInfo beatmapInfo); + + event Action OnInvalidated; } } diff --git a/osu.Game/Beatmaps/WorkingBeatmapCache.cs b/osu.Game/Beatmaps/WorkingBeatmapCache.cs index 4495fb8318..9d31c58709 100644 --- a/osu.Game/Beatmaps/WorkingBeatmapCache.cs +++ b/osu.Game/Beatmaps/WorkingBeatmapCache.cs @@ -76,10 +76,13 @@ namespace osu.Game.Beatmaps { Logger.Log($"Invalidating working beatmap cache for {info}"); workingCache.Remove(working); + OnInvalidated?.Invoke(working); } } } + public event Action OnInvalidated; + public virtual WorkingBeatmap GetWorkingBeatmap(BeatmapInfo beatmapInfo) { if (beatmapInfo?.BeatmapSet == null) diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 41fb55a856..d3635ea553 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -296,8 +296,24 @@ namespace osu.Game.Screens.Select base.LoadComplete(); modSelectOverlayRegistration = OverlayManager?.RegisterBlockingOverlay(ModSelect); + + beatmaps.OnInvalidated += workingBeatmapInvalidated; } + private void workingBeatmapInvalidated(WorkingBeatmap working) => Scheduler.AddOnce(w => + { + // The global beatmap may have already been updated (ie. by the editor). + // Only perform the actual switch if we still need to. + if (w == Beatmap.Value) + { + // Not sure if this refresh is required. + var beatmapInfo = beatmaps.QueryBeatmap(b => b.ID == w.BeatmapInfo.ID); + Beatmap.Value = beatmaps.GetWorkingBeatmap(beatmapInfo); + } + + updateComponentFromBeatmap(Beatmap.Value); + }, working); + /// /// Creates the buttons to be displayed in the footer. /// @@ -700,6 +716,8 @@ namespace osu.Game.Screens.Select music.TrackChanged -= ensureTrackLooping; modSelectOverlayRegistration?.Dispose(); + + beatmaps.OnInvalidated -= workingBeatmapInvalidated; } /// From 30b3973c9fcef6a8a1a308376815d5559eea3d65 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 24 Jun 2022 18:39:44 +0900 Subject: [PATCH 06/16] Difficulty cache invalidation flow --- osu.Game/Beatmaps/BeatmapDifficultyCache.cs | 5 +++++ osu.Game/Beatmaps/BeatmapUpdater.cs | 2 ++ osu.Game/Database/MemoryCachingComponent.cs | 10 ++++++++++ 3 files changed, 17 insertions(+) diff --git a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs index ef0fa36b16..493de5bef4 100644 --- a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs +++ b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs @@ -81,6 +81,11 @@ namespace osu.Game.Beatmaps }, true); } + public void Invalidate(IBeatmapInfo beatmap) + { + base.Invalidate(lookup => lookup.BeatmapInfo.Equals(beatmap)); + } + /// /// Retrieves a bindable containing the star difficulty of a that follows the currently-selected ruleset and mods. /// diff --git a/osu.Game/Beatmaps/BeatmapUpdater.cs b/osu.Game/Beatmaps/BeatmapUpdater.cs index a24407a734..e6703dd8d0 100644 --- a/osu.Game/Beatmaps/BeatmapUpdater.cs +++ b/osu.Game/Beatmaps/BeatmapUpdater.cs @@ -61,6 +61,8 @@ namespace osu.Game.Beatmaps var working = workingBeatmapCache.GetWorkingBeatmap(beatmap); beatmap.Length = calculateLength(working.Beatmap); beatmap.BPM = 60000 / working.Beatmap.GetMostCommonBeatLength(); + + difficultyCache.Invalidate(beatmap); } } diff --git a/osu.Game/Database/MemoryCachingComponent.cs b/osu.Game/Database/MemoryCachingComponent.cs index 6e6d928dcc..215050460b 100644 --- a/osu.Game/Database/MemoryCachingComponent.cs +++ b/osu.Game/Database/MemoryCachingComponent.cs @@ -3,6 +3,7 @@ #nullable disable +using System; using System.Collections.Concurrent; using System.Threading; using System.Threading.Tasks; @@ -39,6 +40,15 @@ namespace osu.Game.Database return computed; } + protected void Invalidate(Func invalidationFunction) + { + foreach (var kvp in cache) + { + if (invalidationFunction(kvp.Key)) + cache.TryRemove(kvp.Key, out _); + } + } + protected bool CheckExists([NotNull] TLookup lookup, out TValue value) => cache.TryGetValue(lookup, out value); From 0c3d890f76588465df93aa2e80d57bc7199b4f17 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 24 Jun 2022 19:04:20 +0900 Subject: [PATCH 07/16] Fix reprocessing not working on import due to realm threading woes --- osu.Game/Beatmaps/BeatmapImporter.cs | 9 +++++++-- osu.Game/Beatmaps/BeatmapUpdater.cs | 17 +++++++++-------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapImporter.cs b/osu.Game/Beatmaps/BeatmapImporter.cs index 0620e5d9b1..05c5a15505 100644 --- a/osu.Game/Beatmaps/BeatmapImporter.cs +++ b/osu.Game/Beatmaps/BeatmapImporter.cs @@ -80,8 +80,6 @@ namespace osu.Game.Beatmaps // If this is ever an issue, we can consider marking as pending delete but not resetting the IDs (but care will be required for // beatmaps, which don't have their own `DeletePending` state). - beatmapUpdater?.Process(beatmapSet, realm); - if (beatmapSet.OnlineID > 0) { var existingSetWithSameOnlineID = realm.All().SingleOrDefault(b => b.OnlineID == beatmapSet.OnlineID); @@ -99,6 +97,13 @@ namespace osu.Game.Beatmaps } } + protected override void PostImport(BeatmapSetInfo model, Realm realm) + { + base.PostImport(model, realm); + + beatmapUpdater?.Process(model); + } + private void validateOnlineIds(BeatmapSetInfo beatmapSet, Realm realm) { var beatmapIds = beatmapSet.Beatmaps.Where(b => b.OnlineID > 0).Select(b => b.OnlineID).ToList(); diff --git a/osu.Game/Beatmaps/BeatmapUpdater.cs b/osu.Game/Beatmaps/BeatmapUpdater.cs index e6703dd8d0..0397cfbea9 100644 --- a/osu.Game/Beatmaps/BeatmapUpdater.cs +++ b/osu.Game/Beatmaps/BeatmapUpdater.cs @@ -3,9 +3,9 @@ #nullable enable +using System.Diagnostics; using System.Linq; using System.Threading.Tasks; -using osu.Framework.Extensions; using osu.Game.Database; using osu.Game.Rulesets.Objects; using Realms; @@ -52,17 +52,18 @@ namespace osu.Game.Beatmaps foreach (var beatmap in beatmapSet.Beatmaps) { - // Because we aren't guaranteed all processing will happen on this thread, it's very hard to use the live realm object. - // This can be fixed by adding a synchronous flow to `BeatmapDifficultyCache`. - var detachedBeatmap = beatmap.Detach(); + difficultyCache.Invalidate(beatmap); - beatmap.StarRating = difficultyCache.GetDifficultyAsync(detachedBeatmap).GetResultSafely()?.Stars ?? 0; + var working = workingBeatmapCache.GetWorkingBeatmap(beatmap.Detach()); + var ruleset = working.BeatmapInfo.Ruleset.CreateInstance(); - var working = workingBeatmapCache.GetWorkingBeatmap(beatmap); + Debug.Assert(ruleset != null); + + var calculator = ruleset.CreateDifficultyCalculator(working); + + beatmap.StarRating = calculator.Calculate().StarRating; beatmap.Length = calculateLength(working.Beatmap); beatmap.BPM = 60000 / working.Beatmap.GetMostCommonBeatLength(); - - difficultyCache.Invalidate(beatmap); } } From 7692bac35a27618c8c5b74a80923d862d2340159 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 24 Jun 2022 21:02:14 +0900 Subject: [PATCH 08/16] Simplify refetch (and ensure to invalidate after processing) --- osu.Game/Beatmaps/BeatmapManager.cs | 22 +++++++++++++--------- osu.Game/Beatmaps/BeatmapUpdater.cs | 5 ++++- osu.Game/Screens/Edit/Editor.cs | 4 +--- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index ff176484b6..26f538f65b 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -430,26 +430,30 @@ namespace osu.Game.Beatmaps #region Implementation of IWorkingBeatmapCache - public WorkingBeatmap GetWorkingBeatmap(BeatmapInfo? beatmapInfo) + /// + /// Retrieve a instance for the provided + /// + /// The beatmap to lookup. + /// Whether to force a refetch from the database to ensure is up-to-date. + /// A instance correlating to the provided . + public WorkingBeatmap GetWorkingBeatmap(BeatmapInfo beatmapInfo, bool refetch = false) { // Detached sets don't come with files. // If we seem to be missing files, now is a good time to re-fetch. - if (beatmapInfo?.IsManaged == true || beatmapInfo?.BeatmapSet?.Files.Count == 0) + if (refetch || beatmapInfo.IsManaged || beatmapInfo.BeatmapSet?.Files.Count == 0) { - Realm.Run(r => - { - var refetch = r.Find(beatmapInfo.ID)?.Detach(); + workingBeatmapCache.Invalidate(beatmapInfo); - if (refetch != null) - beatmapInfo = refetch; - }); + Guid id = beatmapInfo.ID; + beatmapInfo = Realm.Run(r => r.Find(id)?.Detach()) ?? beatmapInfo; } - Debug.Assert(beatmapInfo?.IsManaged != true); + Debug.Assert(beatmapInfo.IsManaged != true); return workingBeatmapCache.GetWorkingBeatmap(beatmapInfo); } + WorkingBeatmap IWorkingBeatmapCache.GetWorkingBeatmap(BeatmapInfo beatmapInfo) => GetWorkingBeatmap(beatmapInfo); void IWorkingBeatmapCache.Invalidate(BeatmapSetInfo beatmapSetInfo) => workingBeatmapCache.Invalidate(beatmapSetInfo); void IWorkingBeatmapCache.Invalidate(BeatmapInfo beatmapInfo) => workingBeatmapCache.Invalidate(beatmapInfo); diff --git a/osu.Game/Beatmaps/BeatmapUpdater.cs b/osu.Game/Beatmaps/BeatmapUpdater.cs index 0397cfbea9..caef094701 100644 --- a/osu.Game/Beatmaps/BeatmapUpdater.cs +++ b/osu.Game/Beatmaps/BeatmapUpdater.cs @@ -54,7 +54,7 @@ namespace osu.Game.Beatmaps { difficultyCache.Invalidate(beatmap); - var working = workingBeatmapCache.GetWorkingBeatmap(beatmap.Detach()); + var working = workingBeatmapCache.GetWorkingBeatmap(beatmap); var ruleset = working.BeatmapInfo.Ruleset.CreateInstance(); Debug.Assert(ruleset != null); @@ -65,6 +65,9 @@ namespace osu.Game.Beatmaps beatmap.Length = calculateLength(working.Beatmap); beatmap.BPM = 60000 / working.Beatmap.GetMostCommonBeatLength(); } + + // And invalidate again afterwards as re-fetching the most up-to-date database metadata will be required. + workingBeatmapCache.Invalidate(beatmapSet); } private double calculateLength(IBeatmap b) diff --git a/osu.Game/Screens/Edit/Editor.cs b/osu.Game/Screens/Edit/Editor.cs index 7a9dae33b4..528a237ac4 100644 --- a/osu.Game/Screens/Edit/Editor.cs +++ b/osu.Game/Screens/Edit/Editor.cs @@ -630,9 +630,7 @@ namespace osu.Game.Screens.Edit // To update the game-wide beatmap with any changes, perform a re-fetch on exit/suspend. // This is required as the editor makes its local changes via EditorBeatmap // (which are not propagated outwards to a potentially cached WorkingBeatmap). - ((IWorkingBeatmapCache)beatmapManager).Invalidate(Beatmap.Value.BeatmapInfo); - var refetchedBeatmapInfo = beatmapManager.QueryBeatmap(b => b.ID == Beatmap.Value.BeatmapInfo.ID); - var refetchedBeatmap = beatmapManager.GetWorkingBeatmap(refetchedBeatmapInfo); + var refetchedBeatmap = beatmapManager.GetWorkingBeatmap(Beatmap.Value.BeatmapInfo, true); if (!(refetchedBeatmap is DummyWorkingBeatmap)) { From 2c5e5fed6f8edd6f0238c5c8af09b440d9421aca Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 24 Jun 2022 21:02:29 +0900 Subject: [PATCH 09/16] Add test coverage of star rating and difficulty updating on editor save --- .../Visual/Editing/TestSceneEditorSaving.cs | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneEditorSaving.cs b/osu.Game.Tests/Visual/Editing/TestSceneEditorSaving.cs index e9bbf1e33d..bcf02cd814 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneEditorSaving.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneEditorSaving.cs @@ -9,6 +9,7 @@ using osu.Framework.Allocation; using osu.Framework.Screens; using osu.Framework.Testing; using osu.Framework.Utils; +using osu.Game.Beatmaps; using osu.Game.Beatmaps.ControlPoints; using osu.Game.Screens.Edit; using osu.Game.Screens.Edit.Compose.Components.Timeline; @@ -130,6 +131,54 @@ namespace osu.Game.Tests.Visual.Editing !ReferenceEquals(EditorBeatmap.HitObjects[0].DifficultyControlPoint, DifficultyControlPoint.DEFAULT)); } + [Test] + public void TestLengthAndStarRatingUpdated() + { + WorkingBeatmap working = null; + double lastStarRating = 0; + double lastLength = 0; + + AddStep("Add timing point", () => EditorBeatmap.ControlPointInfo.Add(500, new TimingControlPoint())); + AddStep("Change to placement mode", () => InputManager.Key(Key.Number2)); + AddStep("Move to playfield", () => InputManager.MoveMouseTo(Game.ScreenSpaceDrawQuad.Centre)); + AddStep("Place single hitcircle", () => InputManager.Click(MouseButton.Left)); + AddAssert("One hitobject placed", () => EditorBeatmap.HitObjects.Count == 1); + + SaveEditor(); + AddStep("Get working beatmap", () => working = Game.BeatmapManager.GetWorkingBeatmap(EditorBeatmap.BeatmapInfo, true)); + + AddAssert("Beatmap length is zero", () => working.BeatmapInfo.Length == 0); + checkDifficultyIncreased(); + + AddStep("Move forward", () => InputManager.Key(Key.Right)); + AddStep("Place another hitcircle", () => InputManager.Click(MouseButton.Left)); + AddAssert("Two hitobjects placed", () => EditorBeatmap.HitObjects.Count == 2); + + SaveEditor(); + AddStep("Get working beatmap", () => working = Game.BeatmapManager.GetWorkingBeatmap(EditorBeatmap.BeatmapInfo, true)); + + checkDifficultyIncreased(); + checkLengthIncreased(); + + void checkLengthIncreased() + { + AddStep("Beatmap length increased", () => + { + Assert.That(working.BeatmapInfo.Length, Is.GreaterThan(lastLength)); + lastLength = working.BeatmapInfo.Length; + }); + } + + void checkDifficultyIncreased() + { + AddStep("Beatmap difficulty increased", () => + { + Assert.That(working.BeatmapInfo.StarRating, Is.GreaterThan(lastStarRating)); + lastStarRating = working.BeatmapInfo.StarRating; + }); + } + } + [Test] public void TestExitWithoutSaveFromExistingBeatmap() { From 82134ad1a668bb5a35b7d3feae7404090bce4bf5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 30 Jun 2022 16:46:28 +0900 Subject: [PATCH 10/16] Allow `null` parameter to `GetWorkingBeatmap` for now --- osu.Game/Beatmaps/BeatmapManager.cs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 26f538f65b..5f83bbb0dd 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -436,20 +436,23 @@ namespace osu.Game.Beatmaps /// The beatmap to lookup. /// Whether to force a refetch from the database to ensure is up-to-date. /// A instance correlating to the provided . - public WorkingBeatmap GetWorkingBeatmap(BeatmapInfo beatmapInfo, bool refetch = false) + public WorkingBeatmap GetWorkingBeatmap(BeatmapInfo? beatmapInfo, bool refetch = false) { - // Detached sets don't come with files. - // If we seem to be missing files, now is a good time to re-fetch. - if (refetch || beatmapInfo.IsManaged || beatmapInfo.BeatmapSet?.Files.Count == 0) + if (beatmapInfo != null) { - workingBeatmapCache.Invalidate(beatmapInfo); + // Detached sets don't come with files. + // If we seem to be missing files, now is a good time to re-fetch. + if (refetch || beatmapInfo.IsManaged || beatmapInfo.BeatmapSet?.Files.Count == 0) + { + workingBeatmapCache.Invalidate(beatmapInfo); - Guid id = beatmapInfo.ID; - beatmapInfo = Realm.Run(r => r.Find(id)?.Detach()) ?? beatmapInfo; + Guid id = beatmapInfo.ID; + beatmapInfo = Realm.Run(r => r.Find(id)?.Detach()) ?? beatmapInfo; + } + + Debug.Assert(beatmapInfo.IsManaged != true); } - Debug.Assert(beatmapInfo.IsManaged != true); - return workingBeatmapCache.GetWorkingBeatmap(beatmapInfo); } From e34c2f0acac243ecc4a53cc256b02468b032f97e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 30 Jun 2022 16:47:26 +0900 Subject: [PATCH 11/16] Remove unnecessary `nullable-enable` --- osu.Game/Beatmaps/BeatmapUpdater.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapUpdater.cs b/osu.Game/Beatmaps/BeatmapUpdater.cs index caef094701..62a6fc7293 100644 --- a/osu.Game/Beatmaps/BeatmapUpdater.cs +++ b/osu.Game/Beatmaps/BeatmapUpdater.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable enable - using System.Diagnostics; using System.Linq; using System.Threading.Tasks; From ef42c6ecdf95f659faf31980c2c27e7ebc3a1a94 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 30 Jun 2022 16:51:31 +0900 Subject: [PATCH 12/16] Add missing xmldoc --- osu.Game/Beatmaps/IWorkingBeatmapCache.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game/Beatmaps/IWorkingBeatmapCache.cs b/osu.Game/Beatmaps/IWorkingBeatmapCache.cs index 36c53fce99..e28f802e78 100644 --- a/osu.Game/Beatmaps/IWorkingBeatmapCache.cs +++ b/osu.Game/Beatmaps/IWorkingBeatmapCache.cs @@ -28,6 +28,9 @@ namespace osu.Game.Beatmaps /// The beatmap info to invalidate any cached entries for. void Invalidate(BeatmapInfo beatmapInfo); + /// + /// Fired whenever a is invalidated. + /// event Action OnInvalidated; } } From 0698471627b384e63af9cfd4e47a7f28d08abd6f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 30 Jun 2022 17:03:19 +0900 Subject: [PATCH 13/16] Move `BeatmapOnlineLookupQueue` to inside `BeatmapUpdater` --- osu.Game/Beatmaps/BeatmapManager.cs | 6 ++---- osu.Game/Beatmaps/BeatmapUpdater.cs | 21 ++++++++++++++++++--- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 5f83bbb0dd..e16a87eb50 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -41,7 +41,6 @@ namespace osu.Game.Beatmaps private readonly BeatmapImporter beatmapImporter; private readonly WorkingBeatmapCache workingBeatmapCache; - private readonly BeatmapOnlineLookupQueue? onlineBeatmapLookupQueue; private readonly BeatmapUpdater? beatmapUpdater; public BeatmapManager(Storage storage, RealmAccess realm, RulesetStore rulesets, IAPIProvider? api, AudioManager audioManager, IResourceStore gameResources, GameHost? host = null, @@ -56,8 +55,7 @@ namespace osu.Game.Beatmaps if (difficultyCache == null) throw new ArgumentNullException(nameof(difficultyCache), "Difficulty cache must be provided if online lookups are required."); - onlineBeatmapLookupQueue = new BeatmapOnlineLookupQueue(api, storage); - beatmapUpdater = new BeatmapUpdater(this, onlineBeatmapLookupQueue, difficultyCache); + beatmapUpdater = new BeatmapUpdater(this, difficultyCache, api, storage); } var userResources = new RealmFileStore(realm, storage).Store; @@ -474,7 +472,7 @@ namespace osu.Game.Beatmaps public void Dispose() { - onlineBeatmapLookupQueue?.Dispose(); + beatmapUpdater?.Dispose(); } #endregion diff --git a/osu.Game/Beatmaps/BeatmapUpdater.cs b/osu.Game/Beatmaps/BeatmapUpdater.cs index 62a6fc7293..d800b09a2b 100644 --- a/osu.Game/Beatmaps/BeatmapUpdater.cs +++ b/osu.Game/Beatmaps/BeatmapUpdater.cs @@ -1,10 +1,14 @@ // 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.Diagnostics; using System.Linq; using System.Threading.Tasks; +using osu.Framework.Extensions.ObjectExtensions; +using osu.Framework.Platform; using osu.Game.Database; +using osu.Game.Online.API; using osu.Game.Rulesets.Objects; using Realms; @@ -13,17 +17,18 @@ namespace osu.Game.Beatmaps /// /// Handles all processing required to ensure a local beatmap is in a consistent state with any changes. /// - public class BeatmapUpdater + public class BeatmapUpdater : IDisposable { private readonly IWorkingBeatmapCache workingBeatmapCache; private readonly BeatmapOnlineLookupQueue onlineLookupQueue; private readonly BeatmapDifficultyCache difficultyCache; - public BeatmapUpdater(IWorkingBeatmapCache workingBeatmapCache, BeatmapOnlineLookupQueue onlineLookupQueue, BeatmapDifficultyCache difficultyCache) + public BeatmapUpdater(IWorkingBeatmapCache workingBeatmapCache, BeatmapDifficultyCache difficultyCache, IAPIProvider api, Storage storage) { this.workingBeatmapCache = workingBeatmapCache; - this.onlineLookupQueue = onlineLookupQueue; this.difficultyCache = difficultyCache; + + onlineLookupQueue = new BeatmapOnlineLookupQueue(api, storage); } /// @@ -81,5 +86,15 @@ namespace osu.Game.Beatmaps return endTime - startTime; } + + #region Implementation of IDisposable + + public void Dispose() + { + if (onlineLookupQueue.IsNotNull()) + onlineLookupQueue.Dispose(); + } + + #endregion } } From aab4dcefbd060b514c79765ceee8e46ad321515e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 30 Jun 2022 17:13:26 +0900 Subject: [PATCH 14/16] Remove unnecessary invalidation handling flow --- osu.Game/Beatmaps/IWorkingBeatmapCache.cs | 9 --------- osu.Game/Screens/Select/SongSelect.cs | 18 ------------------ 2 files changed, 27 deletions(-) diff --git a/osu.Game/Beatmaps/IWorkingBeatmapCache.cs b/osu.Game/Beatmaps/IWorkingBeatmapCache.cs index e28f802e78..3eb33f10d6 100644 --- a/osu.Game/Beatmaps/IWorkingBeatmapCache.cs +++ b/osu.Game/Beatmaps/IWorkingBeatmapCache.cs @@ -1,10 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - -using System; - namespace osu.Game.Beatmaps { public interface IWorkingBeatmapCache @@ -27,10 +23,5 @@ namespace osu.Game.Beatmaps /// /// The beatmap info to invalidate any cached entries for. void Invalidate(BeatmapInfo beatmapInfo); - - /// - /// Fired whenever a is invalidated. - /// - event Action OnInvalidated; } } diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 73bcabba31..7abcbfca42 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -296,24 +296,8 @@ namespace osu.Game.Screens.Select base.LoadComplete(); modSelectOverlayRegistration = OverlayManager?.RegisterBlockingOverlay(ModSelect); - - beatmaps.OnInvalidated += workingBeatmapInvalidated; } - private void workingBeatmapInvalidated(WorkingBeatmap working) => Scheduler.AddOnce(w => - { - // The global beatmap may have already been updated (ie. by the editor). - // Only perform the actual switch if we still need to. - if (w == Beatmap.Value) - { - // Not sure if this refresh is required. - var beatmapInfo = beatmaps.QueryBeatmap(b => b.ID == w.BeatmapInfo.ID); - Beatmap.Value = beatmaps.GetWorkingBeatmap(beatmapInfo); - } - - updateComponentFromBeatmap(Beatmap.Value); - }, working); - /// /// Creates the buttons to be displayed in the footer. /// @@ -719,8 +703,6 @@ namespace osu.Game.Screens.Select music.TrackChanged -= ensureTrackLooping; modSelectOverlayRegistration?.Dispose(); - - beatmaps.OnInvalidated -= workingBeatmapInvalidated; } /// From 2a7321086548a6f99a35c4517357ff77d3ab996c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 30 Jun 2022 17:17:06 +0900 Subject: [PATCH 15/16] Add xmldoc and update parameter naming for `MemoryCachingComponent.Invalidate` flow --- osu.Game/Database/MemoryCachingComponent.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/osu.Game/Database/MemoryCachingComponent.cs b/osu.Game/Database/MemoryCachingComponent.cs index 215050460b..104943bbae 100644 --- a/osu.Game/Database/MemoryCachingComponent.cs +++ b/osu.Game/Database/MemoryCachingComponent.cs @@ -40,11 +40,15 @@ namespace osu.Game.Database return computed; } - protected void Invalidate(Func invalidationFunction) + /// + /// Invalidate all entries matching a provided predicate. + /// + /// The predicate to decide which keys should be invalidated. + protected void Invalidate(Func matchKeyPredicate) { foreach (var kvp in cache) { - if (invalidationFunction(kvp.Key)) + if (matchKeyPredicate(kvp.Key)) cache.TryRemove(kvp.Key, out _); } } From 0e1f08eff870b325b837b03db6cce7f1679f3d20 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 1 Jul 2022 17:10:04 +0900 Subject: [PATCH 16/16] Fix regressing `BeatmapRecommendations` tests due to diffcalc running at --- .../TestSceneBeatmapRecommendations.cs | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapRecommendations.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapRecommendations.cs index 117515977e..504ded5406 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapRecommendations.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapRecommendations.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; @@ -166,15 +167,22 @@ namespace osu.Game.Tests.Visual.SongSelect var beatmapSet = TestResources.CreateTestBeatmapSetInfo(rulesets.Length, rulesets); - for (int i = 0; i < rulesets.Length; i++) + var importedBeatmapSet = Game.BeatmapManager.Import(beatmapSet); + + Debug.Assert(importedBeatmapSet != null); + + importedBeatmapSet.PerformWrite(s => { - var beatmap = beatmapSet.Beatmaps[i]; + for (int i = 0; i < rulesets.Length; i++) + { + var beatmap = s.Beatmaps[i]; - beatmap.StarRating = i + 1; - beatmap.DifficultyName = $"SR{i + 1}"; - } + beatmap.StarRating = i + 1; + beatmap.DifficultyName = $"SR{i + 1}"; + } + }); - return Game.BeatmapManager.Import(beatmapSet)?.Value; + return importedBeatmapSet.Value; } private bool ensureAllBeatmapSetsImported(IEnumerable beatmapSets) => beatmapSets.All(set => set != null);