From 1f157d729dbc0d1bc2cdbed374b41bbef650e121 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 21 Jan 2022 19:39:49 +0900 Subject: [PATCH] Update existing subscriptions to new style Fix missing detach calls in `MusicController` --- osu.Game.Tests/Database/GeneralUsageTests.cs | 3 ++- osu.Game.Tests/Database/RealmLiveTests.cs | 4 ++- osu.Game/Database/RealmContextFactory.cs | 5 +--- .../Bindings/DatabasedKeyBindingContainer.cs | 26 +++++++++--------- osu.Game/Online/BeatmapDownloadTracker.cs | 4 +-- .../OnlinePlayBeatmapAvailabilityTracker.cs | 4 +-- osu.Game/Online/ScoreDownloadTracker.cs | 4 +-- osu.Game/Overlays/MusicController.cs | 17 +++++++----- .../Sections/DebugSettings/MemorySettings.cs | 4 +++ .../Overlays/Settings/Sections/SkinSection.cs | 22 +++++++-------- osu.Game/Screens/Select/BeatmapCarousel.cs | 17 ++++++------ .../Screens/Select/Carousel/TopLocalRank.cs | 25 ++++++++--------- .../Select/Leaderboards/BeatmapLeaderboard.cs | 27 ++++++++++--------- osu.Game/Screens/Spectate/SpectatorScreen.cs | 20 +++++++------- 14 files changed, 98 insertions(+), 84 deletions(-) diff --git a/osu.Game.Tests/Database/GeneralUsageTests.cs b/osu.Game.Tests/Database/GeneralUsageTests.cs index 9ebe94b383..c82c1b6e59 100644 --- a/osu.Game.Tests/Database/GeneralUsageTests.cs +++ b/osu.Game.Tests/Database/GeneralUsageTests.cs @@ -46,7 +46,7 @@ namespace osu.Game.Tests.Database { bool callbackRan = false; - realmFactory.Run(realm => + realmFactory.Register(realm => { var subscription = realm.All().QueryAsyncWithNotifications((sender, changes, error) => { @@ -60,6 +60,7 @@ namespace osu.Game.Tests.Database realmFactory.Run(r => r.Refresh()); subscription?.Dispose(); + return null; }); Assert.IsTrue(callbackRan); diff --git a/osu.Game.Tests/Database/RealmLiveTests.cs b/osu.Game.Tests/Database/RealmLiveTests.cs index 2f16df4624..5549c140f6 100644 --- a/osu.Game.Tests/Database/RealmLiveTests.cs +++ b/osu.Game.Tests/Database/RealmLiveTests.cs @@ -234,7 +234,7 @@ namespace osu.Game.Tests.Database { int changesTriggered = 0; - realmFactory.Run(outerRealm => + realmFactory.Register(outerRealm => { outerRealm.All().QueryAsyncWithNotifications(gotChange); ILive? liveBeatmap = null; @@ -277,6 +277,8 @@ namespace osu.Game.Tests.Database r.Remove(resolved); }); }); + + return null; }); void gotChange(IRealmCollection sender, ChangeSet changes, Exception error) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index 62717eb880..c11ff61039 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -244,7 +244,6 @@ namespace osu.Game.Database if (!ThreadSafety.IsUpdateThread) throw new InvalidOperationException(@$"{nameof(Register)} must be called from the update thread."); - subscriptionActions.Add(action, null); registerSubscription(action); return new InvokeOnDisposal(() => @@ -264,10 +263,8 @@ namespace osu.Game.Database lock (contextLock) { - Debug.Assert(context != null); - current_thread_subscriptions_allowed.Value = true; - subscriptionActions[action] = action(context); + subscriptionActions[action] = action(Context); current_thread_subscriptions_allowed.Value = false; } } diff --git a/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs b/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs index 03b069d431..5a9797ffce 100644 --- a/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs +++ b/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs @@ -23,7 +23,6 @@ namespace osu.Game.Input.Bindings private readonly int? variant; private IDisposable realmSubscription; - private IQueryable realmKeyBindings; [Resolved] private RealmContextFactory realmFactory { get; set; } @@ -47,22 +46,25 @@ namespace osu.Game.Input.Bindings throw new InvalidOperationException($"{nameof(variant)} can not be null when a non-null {nameof(ruleset)} is provided."); } + private IQueryable realmKeyBindings + { + get + { + string rulesetName = ruleset?.ShortName; + return realmFactory.Context.All() + .Where(b => b.RulesetName == rulesetName && b.Variant == variant); + } + } + protected override void LoadComplete() { - string rulesetName = ruleset?.ShortName; - - realmKeyBindings = realmFactory.Context.All() - .Where(b => b.RulesetName == rulesetName && b.Variant == variant); - - realmSubscription = realmKeyBindings + realmSubscription = realmFactory.Register(realm => realmKeyBindings .QueryAsyncWithNotifications((sender, changes, error) => { - // first subscription ignored as we are handling this in LoadComplete. - if (changes == null) - return; - + // The first fire of this is a bit redundant as this is being called in base.LoadComplete, + // but this is safest in case the subscription is restored after a context recycle. ReloadMappings(); - }); + })); base.LoadComplete(); } diff --git a/osu.Game/Online/BeatmapDownloadTracker.cs b/osu.Game/Online/BeatmapDownloadTracker.cs index be5bdea6f1..d50ab5a347 100644 --- a/osu.Game/Online/BeatmapDownloadTracker.cs +++ b/osu.Game/Online/BeatmapDownloadTracker.cs @@ -42,7 +42,7 @@ namespace osu.Game.Online // Used to interact with manager classes that don't support interface types. Will eventually be replaced. var beatmapSetInfo = new BeatmapSetInfo { OnlineID = TrackedItem.OnlineID }; - realmSubscription = realmContextFactory.Context.All().Where(s => s.OnlineID == TrackedItem.OnlineID && !s.DeletePending).QueryAsyncWithNotifications((items, changes, ___) => + realmSubscription = realmContextFactory.Register(realm => realm.All().Where(s => s.OnlineID == TrackedItem.OnlineID && !s.DeletePending).QueryAsyncWithNotifications((items, changes, ___) => { if (items.Any()) Schedule(() => UpdateState(DownloadState.LocallyAvailable)); @@ -54,7 +54,7 @@ namespace osu.Game.Online attachDownload(Downloader.GetExistingDownload(beatmapSetInfo)); }); } - }); + })); } private void downloadBegan(ArchiveDownloadRequest request) => Schedule(() => diff --git a/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs b/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs index 1f77b1d383..fdcf2b39c6 100644 --- a/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs +++ b/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs @@ -78,13 +78,13 @@ namespace osu.Game.Online.Rooms // handles changes to hash that didn't occur from the import process (ie. a user editing the beatmap in the editor, somehow). realmSubscription?.Dispose(); - realmSubscription = filteredBeatmaps().QueryAsyncWithNotifications((items, changes, ___) => + realmSubscription = realmContextFactory.Register(realm => filteredBeatmaps().QueryAsyncWithNotifications((items, changes, ___) => { if (changes == null) return; Scheduler.AddOnce(updateAvailability); - }); + })); }, true); } diff --git a/osu.Game/Online/ScoreDownloadTracker.cs b/osu.Game/Online/ScoreDownloadTracker.cs index b34586567d..de1d6fd94a 100644 --- a/osu.Game/Online/ScoreDownloadTracker.cs +++ b/osu.Game/Online/ScoreDownloadTracker.cs @@ -47,7 +47,7 @@ namespace osu.Game.Online Downloader.DownloadBegan += downloadBegan; Downloader.DownloadFailed += downloadFailed; - realmSubscription = realmContextFactory.Context.All().Where(s => ((s.OnlineID > 0 && s.OnlineID == TrackedItem.OnlineID) || s.Hash == TrackedItem.Hash) && !s.DeletePending).QueryAsyncWithNotifications((items, changes, ___) => + realmSubscription = realmContextFactory.Register(realm => realm.All().Where(s => ((s.OnlineID > 0 && s.OnlineID == TrackedItem.OnlineID) || s.Hash == TrackedItem.Hash) && !s.DeletePending).QueryAsyncWithNotifications((items, changes, ___) => { if (items.Any()) Schedule(() => UpdateState(DownloadState.LocallyAvailable)); @@ -59,7 +59,7 @@ namespace osu.Game.Online attachDownload(Downloader.GetExistingDownload(scoreInfo)); }); } - }); + })); } private void downloadBegan(ArchiveDownloadRequest request) => Schedule(() => diff --git a/osu.Game/Overlays/MusicController.cs b/osu.Game/Overlays/MusicController.cs index 70f8332295..01a2c9d354 100644 --- a/osu.Game/Overlays/MusicController.cs +++ b/osu.Game/Overlays/MusicController.cs @@ -80,26 +80,31 @@ namespace osu.Game.Overlays mods.BindValueChanged(_ => ResetTrackAdjustments(), true); } + private IQueryable availableBeatmaps => realmFactory.Context + .All() + .Where(s => !s.DeletePending); + protected override void LoadComplete() { base.LoadComplete(); - var availableBeatmaps = realmFactory.Context - .All() - .Where(s => !s.DeletePending); - // ensure we're ready before completing async load. // probably not a good way of handling this (as there is a period we aren't watching for changes until the realm subscription finishes up. foreach (var s in availableBeatmaps) - beatmapSets.Add(s); + beatmapSets.Add(s.Detach()); - beatmapSubscription = availableBeatmaps.QueryAsyncWithNotifications(beatmapsChanged); + beatmapSubscription = realmFactory.Register(realm => availableBeatmaps.QueryAsyncWithNotifications(beatmapsChanged)); } private void beatmapsChanged(IRealmCollection sender, ChangeSet changes, Exception error) { if (changes == null) + { + beatmapSets.Clear(); + foreach (var s in sender) + beatmapSets.Add(s.Detach()); return; + } foreach (int i in changes.InsertedIndices) beatmapSets.Insert(i, sender[i].Detach()); diff --git a/osu.Game/Overlays/Settings/Sections/DebugSettings/MemorySettings.cs b/osu.Game/Overlays/Settings/Sections/DebugSettings/MemorySettings.cs index 8d4fc5fc9f..84a54b208c 100644 --- a/osu.Game/Overlays/Settings/Sections/DebugSettings/MemorySettings.cs +++ b/osu.Game/Overlays/Settings/Sections/DebugSettings/MemorySettings.cs @@ -33,6 +33,10 @@ namespace osu.Game.Overlays.Settings.Sections.DebugSettings using (realmFactory.BlockAllOperations()) { } + + // retrieve context to revive realm subscriptions. + // TODO: should we do this from OsuGame or RealmContextFactory or something? Answer: yes. + var _ = realmFactory.Context; } }, }; diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index 0fa6d78d4b..11a7275168 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -50,7 +50,12 @@ namespace osu.Game.Overlays.Settings.Sections private RealmContextFactory realmFactory { get; set; } private IDisposable realmSubscription; - private IQueryable realmSkins; + + private IQueryable realmSkins => + realmFactory.Context.All() + .Where(s => !s.DeletePending) + .OrderByDescending(s => s.Protected) // protected skins should be at the top. + .ThenBy(s => s.Name, StringComparer.OrdinalIgnoreCase); [BackgroundDependencyLoader(permitNulls: true)] private void load(OsuConfigManager config, [CanBeNull] SkinEditorOverlay skinEditor) @@ -78,20 +83,13 @@ namespace osu.Game.Overlays.Settings.Sections skinDropdown.Current = dropdownBindable; - realmSkins = realmFactory.Context.All() - .Where(s => !s.DeletePending) - .OrderByDescending(s => s.Protected) // protected skins should be at the top. - .ThenBy(s => s.Name, StringComparer.OrdinalIgnoreCase); - - realmSubscription = realmSkins + realmSubscription = realmFactory.Register(realm => realmSkins .QueryAsyncWithNotifications((sender, changes, error) => { - if (changes == null) - return; - - // Eventually this should be handling the individual changes rather than refreshing the whole dropdown. + // The first fire of this is a bit redundant due to the call below, + // but this is safest in case the subscription is restored after a context recycle. updateItems(); - }); + })); updateItems(); diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index f8cee2704b..6f3f467170 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -190,13 +190,13 @@ namespace osu.Game.Screens.Select { base.LoadComplete(); - subscriptionSets = getBeatmapSets(realmFactory.Context).QueryAsyncWithNotifications(beatmapSetsChanged); - subscriptionBeatmaps = realmFactory.Context.All().Where(b => !b.Hidden).QueryAsyncWithNotifications(beatmapsChanged); + subscriptionSets = realmFactory.Register(realm => getBeatmapSets(realm).QueryAsyncWithNotifications(beatmapSetsChanged)); + subscriptionBeatmaps = realmFactory.Register(realm => realm.All().Where(b => !b.Hidden).QueryAsyncWithNotifications(beatmapsChanged)); // Can't use main subscriptions because we can't lookup deleted indices. // https://github.com/realm/realm-dotnet/discussions/2634#discussioncomment-1605595. - subscriptionDeletedSets = realmFactory.Context.All().Where(s => s.DeletePending && !s.Protected).QueryAsyncWithNotifications(deletedBeatmapSetsChanged); - subscriptionHiddenBeatmaps = realmFactory.Context.All().Where(b => b.Hidden).QueryAsyncWithNotifications(beatmapsChanged); + subscriptionDeletedSets = realmFactory.Register(realm => realm.All().Where(s => s.DeletePending && !s.Protected).QueryAsyncWithNotifications(deletedBeatmapSetsChanged)); + subscriptionHiddenBeatmaps = realmFactory.Register(realm => realm.All().Where(b => b.Hidden).QueryAsyncWithNotifications(beatmapsChanged)); } private void deletedBeatmapSetsChanged(IRealmCollection sender, ChangeSet changes, Exception error) @@ -552,10 +552,11 @@ namespace osu.Game.Screens.Select private void signalBeatmapsLoaded() { - Debug.Assert(BeatmapSetsLoaded == false); - - BeatmapSetsChanged?.Invoke(); - BeatmapSetsLoaded = true; + if (!BeatmapSetsLoaded) + { + BeatmapSetsChanged?.Invoke(); + BeatmapSetsLoaded = true; + } itemsCache.Invalidate(); } diff --git a/osu.Game/Screens/Select/Carousel/TopLocalRank.cs b/osu.Game/Screens/Select/Carousel/TopLocalRank.cs index 7ac99f4935..ee3930364b 100644 --- a/osu.Game/Screens/Select/Carousel/TopLocalRank.cs +++ b/osu.Game/Screens/Select/Carousel/TopLocalRank.cs @@ -48,18 +48,19 @@ namespace osu.Game.Screens.Select.Carousel ruleset.BindValueChanged(_ => { scoreSubscription?.Dispose(); - scoreSubscription = realmFactory.Context.All() - .Filter($"{nameof(ScoreInfo.User)}.{nameof(RealmUser.OnlineID)} == $0" - + $" && {nameof(ScoreInfo.BeatmapInfo)}.{nameof(BeatmapInfo.ID)} == $1" - + $" && {nameof(ScoreInfo.Ruleset)}.{nameof(RulesetInfo.ShortName)} == $2" - + $" && {nameof(ScoreInfo.DeletePending)} == false", api.LocalUser.Value.Id, beatmapInfo.ID, ruleset.Value.ShortName) - .OrderByDescending(s => s.TotalScore) - .QueryAsyncWithNotifications((items, changes, ___) => - { - Rank = items.FirstOrDefault()?.Rank; - // Required since presence is changed via IsPresent override - Invalidate(Invalidation.Presence); - }); + scoreSubscription = realmFactory.Register(realm => + realm.All() + .Filter($"{nameof(ScoreInfo.User)}.{nameof(RealmUser.OnlineID)} == $0" + + $" && {nameof(ScoreInfo.BeatmapInfo)}.{nameof(BeatmapInfo.ID)} == $1" + + $" && {nameof(ScoreInfo.Ruleset)}.{nameof(RulesetInfo.ShortName)} == $2" + + $" && {nameof(ScoreInfo.DeletePending)} == false", api.LocalUser.Value.Id, beatmapInfo.ID, ruleset.Value.ShortName) + .OrderByDescending(s => s.TotalScore) + .QueryAsyncWithNotifications((items, changes, ___) => + { + Rank = items.FirstOrDefault()?.Rank; + // Required since presence is changed via IsPresent override + Invalidate(Invalidation.Presence); + })); }, true); } diff --git a/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs b/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs index da52b43ab6..954c2a6413 100644 --- a/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs +++ b/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs @@ -44,9 +44,13 @@ namespace osu.Game.Screens.Select.Leaderboards beatmapInfo = value; Scores = null; - UpdateScores(); - if (IsLoaded) - refreshRealmSubscription(); + if (IsOnlineScope) + UpdateScores(); + else + { + if (IsLoaded) + refreshRealmSubscription(); + } } } @@ -109,15 +113,14 @@ namespace osu.Game.Screens.Select.Leaderboards if (beatmapInfo == null) return; - scoreSubscription = realmFactory.Context.All() - .Filter($"{nameof(ScoreInfo.BeatmapInfo)}.{nameof(BeatmapInfo.ID)} = $0", beatmapInfo.ID) - .QueryAsyncWithNotifications((_, changes, ___) => - { - if (changes == null) - return; - - RefreshScores(); - }); + scoreSubscription = realmFactory.Register(realm => + realm.All() + .Filter($"{nameof(ScoreInfo.BeatmapInfo)}.{nameof(BeatmapInfo.ID)} = $0", beatmapInfo.ID) + .QueryAsyncWithNotifications((_, changes, ___) => + { + if (!IsOnlineScope) + RefreshScores(); + })); } protected override void Reset() diff --git a/osu.Game/Screens/Spectate/SpectatorScreen.cs b/osu.Game/Screens/Spectate/SpectatorScreen.cs index dd586bdd37..9fa5bb8562 100644 --- a/osu.Game/Screens/Spectate/SpectatorScreen.cs +++ b/osu.Game/Screens/Spectate/SpectatorScreen.cs @@ -79,17 +79,17 @@ namespace osu.Game.Screens.Spectate playingUserStates.BindTo(spectatorClient.PlayingUserStates); playingUserStates.BindCollectionChanged(onPlayingUserStatesChanged, true); - realmSubscription = realmContextFactory.Context - .All() - .Where(s => !s.DeletePending) - .QueryAsyncWithNotifications((items, changes, ___) => - { - if (changes?.InsertedIndices == null) - return; + realmSubscription = realmContextFactory.Register(realm => + realm.All() + .Where(s => !s.DeletePending) + .QueryAsyncWithNotifications((items, changes, ___) => + { + if (changes?.InsertedIndices == null) + return; - foreach (int c in changes.InsertedIndices) - beatmapUpdated(items[c]); - }); + foreach (int c in changes.InsertedIndices) + beatmapUpdated(items[c]); + })); foreach ((int id, var _) in userMap) spectatorClient.WatchUser(id);