From 35d004cdc2706420738cf3b96f63eb4147dbab5a Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 6 Nov 2024 20:39:10 +0900 Subject: [PATCH 1/4] Fix intermittent beatmap recommendations test --- .../Visual/SongSelect/TestSceneBeatmapRecommendations.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapRecommendations.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapRecommendations.cs index 16c8bc1a6b..82791056b6 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapRecommendations.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapRecommendations.cs @@ -191,7 +191,7 @@ private void presentAndConfirm(Func getImport, int expectedDiff) { AddStep("present beatmap", () => Game.PresentBeatmap(getImport())); - AddUntilStep("wait for song select", () => Game.ScreenStack.CurrentScreen is Screens.Select.SongSelect); + AddUntilStep("wait for song select", () => Game.ScreenStack.CurrentScreen is Screens.Select.SongSelect select && select.BeatmapSetsLoaded); AddUntilStep("recommended beatmap displayed", () => Game.Beatmap.Value.BeatmapInfo.MatchesOnlineID(getImport().Beatmaps[expectedDiff - 1])); } } From bd630c189e16754e630fdbcacccae455fe0239dd Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 7 Nov 2024 17:25:42 +0900 Subject: [PATCH 2/4] Fix tests not working by forgoing beatmap updates --- .../TestSceneBeatmapRecommendations.cs | 34 +++++++++++++++++++ .../Beatmaps/BeatmapOnlineChangeIngest.cs | 4 +-- osu.Game/Beatmaps/BeatmapUpdater.cs | 16 +-------- osu.Game/Beatmaps/IBeatmapUpdater.cs | 30 ++++++++++++++++ .../Database/BackgroundDataStoreProcessor.cs | 2 +- osu.Game/OsuGameBase.cs | 6 ++-- 6 files changed, 72 insertions(+), 20 deletions(-) create mode 100644 osu.Game/Beatmaps/IBeatmapUpdater.cs diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapRecommendations.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapRecommendations.cs index 82791056b6..c9ad22e8df 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapRecommendations.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapRecommendations.cs @@ -10,9 +10,12 @@ using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Extensions.IEnumerableExtensions; +using osu.Framework.Platform; using osu.Framework.Testing; using osu.Game.Beatmaps; +using osu.Game.Database; using osu.Game.Extensions; +using osu.Game.Online.API; using osu.Game.Rulesets; using osu.Game.Rulesets.Catch; using osu.Game.Rulesets.Mania; @@ -194,5 +197,36 @@ private void presentAndConfirm(Func getImport, int expectedDiff) AddUntilStep("wait for song select", () => Game.ScreenStack.CurrentScreen is Screens.Select.SongSelect select && select.BeatmapSetsLoaded); AddUntilStep("recommended beatmap displayed", () => Game.Beatmap.Value.BeatmapInfo.MatchesOnlineID(getImport().Beatmaps[expectedDiff - 1])); } + + protected override TestOsuGame CreateTestGame() => new NoBeatmapUpdateGame(LocalStorage, API); + + private class NoBeatmapUpdateGame : TestOsuGame + { + public NoBeatmapUpdateGame(Storage storage, IAPIProvider api, string[] args = null) + : base(storage, api, args) + { + } + + protected override IBeatmapUpdater CreateBeatmapUpdater() => new TestBeatmapUpdater(); + + private class TestBeatmapUpdater : IBeatmapUpdater + { + public void Queue(Live beatmapSet, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst) + { + } + + public void Process(BeatmapSetInfo beatmapSet, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst) + { + } + + public void ProcessObjectCounts(BeatmapInfo beatmapInfo, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst) + { + } + + public void Dispose() + { + } + } + } } } diff --git a/osu.Game/Beatmaps/BeatmapOnlineChangeIngest.cs b/osu.Game/Beatmaps/BeatmapOnlineChangeIngest.cs index b160043820..965f3be0aa 100644 --- a/osu.Game/Beatmaps/BeatmapOnlineChangeIngest.cs +++ b/osu.Game/Beatmaps/BeatmapOnlineChangeIngest.cs @@ -13,11 +13,11 @@ namespace osu.Game.Beatmaps /// public partial class BeatmapOnlineChangeIngest : Component { - private readonly BeatmapUpdater beatmapUpdater; + private readonly IBeatmapUpdater beatmapUpdater; private readonly RealmAccess realm; private readonly MetadataClient metadataClient; - public BeatmapOnlineChangeIngest(BeatmapUpdater beatmapUpdater, RealmAccess realm, MetadataClient metadataClient) + public BeatmapOnlineChangeIngest(IBeatmapUpdater beatmapUpdater, RealmAccess realm, MetadataClient metadataClient) { this.beatmapUpdater = beatmapUpdater; this.realm = realm; diff --git a/osu.Game/Beatmaps/BeatmapUpdater.cs b/osu.Game/Beatmaps/BeatmapUpdater.cs index e897d28916..04ed9c2488 100644 --- a/osu.Game/Beatmaps/BeatmapUpdater.cs +++ b/osu.Game/Beatmaps/BeatmapUpdater.cs @@ -1,7 +1,6 @@ // 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; @@ -15,10 +14,7 @@ namespace osu.Game.Beatmaps { - /// - /// Handles all processing required to ensure a local beatmap is in a consistent state with any changes. - /// - public class BeatmapUpdater : IDisposable + public class BeatmapUpdater : IBeatmapUpdater { private readonly IWorkingBeatmapCache workingBeatmapCache; @@ -38,11 +34,6 @@ public BeatmapUpdater(IWorkingBeatmapCache workingBeatmapCache, BeatmapDifficult metadataLookup = new BeatmapUpdaterMetadataLookup(api, storage); } - /// - /// Queue a beatmap for background processing. - /// - /// The managed beatmap set to update. A transaction will be opened to apply changes. - /// The preferred scope to use for metadata lookup. public void Queue(Live beatmapSet, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst) { Logger.Log($"Queueing change for local beatmap {beatmapSet}"); @@ -50,11 +41,6 @@ public void Queue(Live beatmapSet, MetadataLookupScope lookupSco updateScheduler); } - /// - /// Run all processing on a beatmap immediately. - /// - /// The managed beatmap set to update. A transaction will be opened to apply changes. - /// The preferred scope to use for metadata lookup. public void Process(BeatmapSetInfo beatmapSet, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst) => beatmapSet.Realm!.Write(_ => { // Before we use below, we want to invalidate. diff --git a/osu.Game/Beatmaps/IBeatmapUpdater.cs b/osu.Game/Beatmaps/IBeatmapUpdater.cs new file mode 100644 index 0000000000..ad543e667e --- /dev/null +++ b/osu.Game/Beatmaps/IBeatmapUpdater.cs @@ -0,0 +1,30 @@ +// 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 osu.Game.Database; + +namespace osu.Game.Beatmaps +{ + /// + /// Handles all processing required to ensure a local beatmap is in a consistent state with any changes. + /// + public interface IBeatmapUpdater : IDisposable + { + /// + /// Queue a beatmap for background processing. + /// + /// The managed beatmap set to update. A transaction will be opened to apply changes. + /// The preferred scope to use for metadata lookup. + void Queue(Live beatmapSet, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst); + + /// + /// Run all processing on a beatmap immediately. + /// + /// The managed beatmap set to update. A transaction will be opened to apply changes. + /// The preferred scope to use for metadata lookup. + void Process(BeatmapSetInfo beatmapSet, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst); + + void ProcessObjectCounts(BeatmapInfo beatmapInfo, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst); + } +} diff --git a/osu.Game/Database/BackgroundDataStoreProcessor.cs b/osu.Game/Database/BackgroundDataStoreProcessor.cs index 3efd4da3aa..1512b6be93 100644 --- a/osu.Game/Database/BackgroundDataStoreProcessor.cs +++ b/osu.Game/Database/BackgroundDataStoreProcessor.cs @@ -46,7 +46,7 @@ public partial class BackgroundDataStoreProcessor : Component private RealmAccess realmAccess { get; set; } = null!; [Resolved] - private BeatmapUpdater beatmapUpdater { get; set; } = null!; + private IBeatmapUpdater beatmapUpdater { get; set; } = null!; [Resolved] private IBindable gameBeatmap { get; set; } = null!; diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index d4704d1c72..dc13924b4f 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -198,7 +198,7 @@ public virtual string Version public readonly Bindable>> AvailableMods = new Bindable>>(new Dictionary>()); private BeatmapDifficultyCache difficultyCache; - private BeatmapUpdater beatmapUpdater; + private IBeatmapUpdater beatmapUpdater; private UserLookupCache userCache; private BeatmapLookupCache beatmapCache; @@ -324,7 +324,7 @@ private void load(ReadableKeyCombinationProvider keyCombinationProvider, Framewo base.Content.Add(difficultyCache); // TODO: OsuGame or OsuGameBase? - dependencies.CacheAs(beatmapUpdater = new BeatmapUpdater(BeatmapManager, difficultyCache, API, Storage)); + dependencies.CacheAs(beatmapUpdater = CreateBeatmapUpdater()); dependencies.CacheAs(SpectatorClient = new OnlineSpectatorClient(endpoints)); dependencies.CacheAs(MultiplayerClient = new OnlineMultiplayerClient(endpoints)); dependencies.CacheAs(metadataClient = new OnlineMetadataClient(endpoints)); @@ -563,6 +563,8 @@ public bool Migrate(string path) } } + protected virtual IBeatmapUpdater CreateBeatmapUpdater() => new BeatmapUpdater(BeatmapManager, difficultyCache, API, Storage); + protected override UserInputManager CreateUserInputManager() => new OsuUserInputManager(); protected virtual BatteryInfo CreateBatteryInfo() => null; From 4d7fd236c5dc74a8abcf1588be43cad2dab8b0c9 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 7 Nov 2024 17:28:31 +0900 Subject: [PATCH 3/4] Make class partial --- .../Visual/SongSelect/TestSceneBeatmapRecommendations.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapRecommendations.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapRecommendations.cs index c9ad22e8df..66862e1b78 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapRecommendations.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapRecommendations.cs @@ -200,7 +200,7 @@ private void presentAndConfirm(Func getImport, int expectedDiff) protected override TestOsuGame CreateTestGame() => new NoBeatmapUpdateGame(LocalStorage, API); - private class NoBeatmapUpdateGame : TestOsuGame + private partial class NoBeatmapUpdateGame : TestOsuGame { public NoBeatmapUpdateGame(Storage storage, IAPIProvider api, string[] args = null) : base(storage, api, args) From c7d0a7dde216fa7a1fe8be70c0097733aa26a837 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 7 Nov 2024 18:31:06 +0900 Subject: [PATCH 4/4] Update xmldoc and make realm transactions more obvious --- osu.Game/Beatmaps/BeatmapUpdater.cs | 70 +++++++++++++++------------- osu.Game/Beatmaps/IBeatmapUpdater.cs | 5 ++ 2 files changed, 43 insertions(+), 32 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapUpdater.cs b/osu.Game/Beatmaps/BeatmapUpdater.cs index 04ed9c2488..efb432b84e 100644 --- a/osu.Game/Beatmaps/BeatmapUpdater.cs +++ b/osu.Game/Beatmaps/BeatmapUpdater.cs @@ -41,50 +41,56 @@ public void Queue(Live beatmapSet, MetadataLookupScope lookupSco updateScheduler); } - public void Process(BeatmapSetInfo beatmapSet, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst) => beatmapSet.Realm!.Write(_ => + public void Process(BeatmapSetInfo beatmapSet, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst) { - // Before we use below, we want to invalidate. - workingBeatmapCache.Invalidate(beatmapSet); - - if (lookupScope != MetadataLookupScope.None) - metadataLookup.Update(beatmapSet, lookupScope == MetadataLookupScope.OnlineFirst); - - foreach (var beatmap in beatmapSet.Beatmaps) + beatmapSet.Realm!.Write(_ => { - difficultyCache.Invalidate(beatmap); + // Before we use below, we want to invalidate. + workingBeatmapCache.Invalidate(beatmapSet); - var working = workingBeatmapCache.GetWorkingBeatmap(beatmap); - var ruleset = working.BeatmapInfo.Ruleset.CreateInstance(); + if (lookupScope != MetadataLookupScope.None) + metadataLookup.Update(beatmapSet, lookupScope == MetadataLookupScope.OnlineFirst); - Debug.Assert(ruleset != null); + foreach (var beatmap in beatmapSet.Beatmaps) + { + difficultyCache.Invalidate(beatmap); - var calculator = ruleset.CreateDifficultyCalculator(working); + var working = workingBeatmapCache.GetWorkingBeatmap(beatmap); + var ruleset = working.BeatmapInfo.Ruleset.CreateInstance(); - beatmap.StarRating = calculator.Calculate().StarRating; - beatmap.Length = working.Beatmap.CalculatePlayableLength(); - beatmap.BPM = 60000 / working.Beatmap.GetMostCommonBeatLength(); - beatmap.EndTimeObjectCount = working.Beatmap.HitObjects.Count(h => h is IHasDuration); - beatmap.TotalObjectCount = working.Beatmap.HitObjects.Count; - } + Debug.Assert(ruleset != null); - // And invalidate again afterwards as re-fetching the most up-to-date database metadata will be required. - workingBeatmapCache.Invalidate(beatmapSet); - }); + var calculator = ruleset.CreateDifficultyCalculator(working); - public void ProcessObjectCounts(BeatmapInfo beatmapInfo, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst) => beatmapInfo.Realm!.Write(_ => + beatmap.StarRating = calculator.Calculate().StarRating; + beatmap.Length = working.Beatmap.CalculatePlayableLength(); + beatmap.BPM = 60000 / working.Beatmap.GetMostCommonBeatLength(); + beatmap.EndTimeObjectCount = working.Beatmap.HitObjects.Count(h => h is IHasDuration); + beatmap.TotalObjectCount = working.Beatmap.HitObjects.Count; + } + + // And invalidate again afterwards as re-fetching the most up-to-date database metadata will be required. + workingBeatmapCache.Invalidate(beatmapSet); + }); + } + + public void ProcessObjectCounts(BeatmapInfo beatmapInfo, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst) { - // Before we use below, we want to invalidate. - workingBeatmapCache.Invalidate(beatmapInfo); + beatmapInfo.Realm!.Write(_ => + { + // Before we use below, we want to invalidate. + workingBeatmapCache.Invalidate(beatmapInfo); - var working = workingBeatmapCache.GetWorkingBeatmap(beatmapInfo); - var beatmap = working.Beatmap; + var working = workingBeatmapCache.GetWorkingBeatmap(beatmapInfo); + var beatmap = working.Beatmap; - beatmapInfo.EndTimeObjectCount = beatmap.HitObjects.Count(h => h is IHasDuration); - beatmapInfo.TotalObjectCount = beatmap.HitObjects.Count; + beatmapInfo.EndTimeObjectCount = beatmap.HitObjects.Count(h => h is IHasDuration); + beatmapInfo.TotalObjectCount = beatmap.HitObjects.Count; - // And invalidate again afterwards as re-fetching the most up-to-date database metadata will be required. - workingBeatmapCache.Invalidate(beatmapInfo); - }); + // And invalidate again afterwards as re-fetching the most up-to-date database metadata will be required. + workingBeatmapCache.Invalidate(beatmapInfo); + }); + } #region Implementation of IDisposable diff --git a/osu.Game/Beatmaps/IBeatmapUpdater.cs b/osu.Game/Beatmaps/IBeatmapUpdater.cs index ad543e667e..062984adf0 100644 --- a/osu.Game/Beatmaps/IBeatmapUpdater.cs +++ b/osu.Game/Beatmaps/IBeatmapUpdater.cs @@ -25,6 +25,11 @@ public interface IBeatmapUpdater : IDisposable /// The preferred scope to use for metadata lookup. void Process(BeatmapSetInfo beatmapSet, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst); + /// + /// Runs a subset of processing focused on updating any cached beatmap object counts. + /// + /// The managed beatmap to update. A transaction will be opened to apply changes. + /// The preferred scope to use for metadata lookup. void ProcessObjectCounts(BeatmapInfo beatmapInfo, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst); } }