From 2006620a2c02d60c83792803b4276cb163a3b4a9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 21 Jan 2022 17:55:27 +0900 Subject: [PATCH 01/46] Fix `IntroScreen` retrieving and iterating all realm beatmap sets --- osu.Game/Screens/Menu/IntroScreen.cs | 82 ++++++++++++++-------------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/osu.Game/Screens/Menu/IntroScreen.cs b/osu.Game/Screens/Menu/IntroScreen.cs index d98cb8056f..db9d9b83c4 100644 --- a/osu.Game/Screens/Menu/IntroScreen.cs +++ b/osu.Game/Screens/Menu/IntroScreen.cs @@ -21,6 +21,7 @@ using osu.Game.Overlays; using osu.Game.Screens.Backgrounds; using osuTK; using osuTK.Graphics; +using Realms; namespace osu.Game.Screens.Menu { @@ -93,58 +94,57 @@ namespace osu.Game.Screens.Menu MenuMusic = config.GetBindable(OsuSetting.MenuMusic); seeya = audio.Samples.Get(SeeyaSampleName); - ILive setInfo = null; - // if the user has requested not to play theme music, we should attempt to find a random beatmap from their collection. if (!MenuMusic.Value) { - var sets = beatmaps.GetAllUsableBeatmapSets(); - - if (sets.Count > 0) + realmContextFactory.Run(realm => { - setInfo = beatmaps.QueryBeatmapSet(s => s.ID == sets[RNG.Next(0, sets.Count - 1)].ID); - setInfo?.PerformRead(s => + var sets = realm.All().Where(s => !s.DeletePending && !s.Protected).AsRealmCollection(); + + int setCount = sets.Count; + + if (sets.Any()) + { + var found = sets[RNG.Next(0, setCount - 1)].Beatmaps.FirstOrDefault(); + + if (found != null) + initialBeatmap = beatmaps.GetWorkingBeatmap(found); + } + }); + + // we generally want a song to be playing on startup, so use the intro music even if a user has specified not to if no other track is available. + if (initialBeatmap == null) + { + if (!loadThemedIntro()) + { + // if we detect that the theme track or beatmap is unavailable this is either first startup or things are in a bad state. + // this could happen if a user has nuked their files store. for now, reimport to repair this. + var import = beatmaps.Import(new ZipArchiveReader(game.Resources.GetStream($"Tracks/{BeatmapFile}"), BeatmapFile)).GetResultSafely(); + + import?.PerformWrite(b => b.Protected = true); + + loadThemedIntro(); + } + } + + bool loadThemedIntro() + { + var setInfo = beatmaps.QueryBeatmapSet(b => b.Hash == BeatmapHash); + + if (setInfo == null) + return false; + + setInfo.PerformRead(s => { if (s.Beatmaps.Count == 0) return; - initialBeatmap = beatmaps.GetWorkingBeatmap(s.Beatmaps[0]); + initialBeatmap = beatmaps.GetWorkingBeatmap(s.Beatmaps.First()); }); + + return UsingThemedIntro = initialBeatmap != null; } } - - // we generally want a song to be playing on startup, so use the intro music even if a user has specified not to if no other track is available. - if (setInfo == null) - { - if (!loadThemedIntro()) - { - // if we detect that the theme track or beatmap is unavailable this is either first startup or things are in a bad state. - // this could happen if a user has nuked their files store. for now, reimport to repair this. - var import = beatmaps.Import(new ZipArchiveReader(game.Resources.GetStream($"Tracks/{BeatmapFile}"), BeatmapFile)).GetResultSafely(); - - import?.PerformWrite(b => b.Protected = true); - - loadThemedIntro(); - } - } - - bool loadThemedIntro() - { - setInfo = beatmaps.QueryBeatmapSet(b => b.Hash == BeatmapHash); - - if (setInfo == null) - return false; - - setInfo.PerformRead(s => - { - if (s.Beatmaps.Count == 0) - return; - - initialBeatmap = beatmaps.GetWorkingBeatmap(s.Beatmaps.First()); - }); - - return UsingThemedIntro = initialBeatmap != null; - } } public override void OnResuming(IScreen last) From 18bf690a30bc9d5c5c6d53133469fc462bcf62ec Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 21 Jan 2022 18:13:21 +0900 Subject: [PATCH 02/46] Add `Register` method for subscription purposes and update safeties --- osu.Game/Database/RealmContextFactory.cs | 28 ++++++++++++++++++++++ osu.Game/Database/RealmObjectExtensions.cs | 6 ++--- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index 888ffb1dd5..169333bff8 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -88,6 +88,10 @@ namespace osu.Game.Database } } + internal static bool CurrentThreadSubscriptionsAllowed => current_thread_subscriptions_allowed.Value; + + private static readonly ThreadLocal current_thread_subscriptions_allowed = new ThreadLocal(); + /// /// Construct a new instance of a realm context factory. /// @@ -222,6 +226,30 @@ namespace osu.Game.Database } } + /// + /// Run work on realm that will be run every time the update thread realm context gets recycled. + /// + /// The work to run. + public void Register(Action action) + { + if (!ThreadSafety.IsUpdateThread && context != null) + throw new InvalidOperationException(@$"{nameof(BlockAllOperations)} must be called from the update thread."); + + if (ThreadSafety.IsUpdateThread) + { + current_thread_subscriptions_allowed.Value = true; + action(Context); + current_thread_subscriptions_allowed.Value = false; + } + else + { + current_thread_subscriptions_allowed.Value = true; + using (var realm = createContext()) + action(realm); + current_thread_subscriptions_allowed.Value = false; + } + } + private Realm createContext() { if (isDisposed) diff --git a/osu.Game/Database/RealmObjectExtensions.cs b/osu.Game/Database/RealmObjectExtensions.cs index c25aeab336..c30e1699b9 100644 --- a/osu.Game/Database/RealmObjectExtensions.cs +++ b/osu.Game/Database/RealmObjectExtensions.cs @@ -7,7 +7,6 @@ using System.Linq; using System.Runtime.Serialization; using AutoMapper; using AutoMapper.Internal; -using osu.Framework.Development; using osu.Game.Beatmaps; using osu.Game.Input.Bindings; using osu.Game.Models; @@ -272,9 +271,8 @@ namespace osu.Game.Database public static IDisposable? QueryAsyncWithNotifications(this IRealmCollection collection, NotificationCallbackDelegate callback) where T : RealmObjectBase { - // Subscriptions can only work on the main thread. - if (!ThreadSafety.IsUpdateThread) - throw new InvalidOperationException("Cannot subscribe for realm notifications from a non-update thread."); + if (!RealmContextFactory.CurrentThreadSubscriptionsAllowed) + throw new InvalidOperationException($"Make sure to call {nameof(RealmContextFactory)}.{nameof(RealmContextFactory.Register)}"); return collection.SubscribeForNotifications(callback); } From 45aea9add5209953d7e76a093a74f5ad91732a60 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 21 Jan 2022 18:50:25 +0900 Subject: [PATCH 03/46] Implement full subscription flow --- osu.Game/Database/RealmContextFactory.cs | 46 +++++++++++++++++------- 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index 169333bff8..62717eb880 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -2,6 +2,8 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Linq; using System.Reflection; @@ -80,6 +82,10 @@ namespace osu.Game.Database { context = createContext(); Logger.Log(@$"Opened realm ""{context.Config.DatabasePath}"" at version {context.Config.SchemaVersion}"); + + // Resubscribe any subscriptions + foreach (var action in subscriptionActions.Keys) + registerSubscription(action); } // creating a context will ensure our schema is up-to-date and migrated. @@ -226,26 +232,42 @@ namespace osu.Game.Database } } + private readonly Dictionary, IDisposable?> subscriptionActions = new Dictionary, IDisposable?>(); + /// /// Run work on realm that will be run every time the update thread realm context gets recycled. /// - /// The work to run. - public void Register(Action action) + /// The work to run. Return value should be an from QueryAsyncWithNotifications, or an to clean up any bindings. + /// An which should be disposed to unsubscribe any inner subscription. + public IDisposable Register(Func action) { - if (!ThreadSafety.IsUpdateThread && context != null) - throw new InvalidOperationException(@$"{nameof(BlockAllOperations)} must be called from the update thread."); + if (!ThreadSafety.IsUpdateThread) + throw new InvalidOperationException(@$"{nameof(Register)} must be called from the update thread."); - if (ThreadSafety.IsUpdateThread) + subscriptionActions.Add(action, null); + registerSubscription(action); + + return new InvokeOnDisposal(() => { - current_thread_subscriptions_allowed.Value = true; - action(Context); - current_thread_subscriptions_allowed.Value = false; - } - else + // TODO: this likely needs to be run on the update thread. + if (subscriptionActions.TryGetValue(action, out var unsubscriptionAction)) + { + unsubscriptionAction?.Dispose(); + subscriptionActions.Remove(action); + } + }); + } + + private void registerSubscription(Func action) + { + Debug.Assert(ThreadSafety.IsUpdateThread); + + lock (contextLock) { + Debug.Assert(context != null); + current_thread_subscriptions_allowed.Value = true; - using (var realm = createContext()) - action(realm); + subscriptionActions[action] = action(context); current_thread_subscriptions_allowed.Value = false; } } From 1f157d729dbc0d1bc2cdbed374b41bbef650e121 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 21 Jan 2022 19:39:49 +0900 Subject: [PATCH 04/46] 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); From 63226f7def9f10d10460e43be72a139852f83cc8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 21 Jan 2022 19:56:32 +0900 Subject: [PATCH 05/46] Remove pointless initial `MusicController` beatmap set population Looks to pass tests and all usages look safe enough. --- osu.Game/Overlays/MusicController.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/osu.Game/Overlays/MusicController.cs b/osu.Game/Overlays/MusicController.cs index 01a2c9d354..c19a069343 100644 --- a/osu.Game/Overlays/MusicController.cs +++ b/osu.Game/Overlays/MusicController.cs @@ -87,12 +87,6 @@ namespace osu.Game.Overlays protected override void LoadComplete() { base.LoadComplete(); - - // 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.Detach()); - beatmapSubscription = realmFactory.Register(realm => availableBeatmaps.QueryAsyncWithNotifications(beatmapsChanged)); } From a86c0014fe3093069a23f7014c537d31f6fdf8bf Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 21 Jan 2022 20:07:43 +0900 Subject: [PATCH 06/46] Remove unnecessary exception/check --- osu.Game/Overlays/MusicController.cs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/osu.Game/Overlays/MusicController.cs b/osu.Game/Overlays/MusicController.cs index c19a069343..0671e6d2c7 100644 --- a/osu.Game/Overlays/MusicController.cs +++ b/osu.Game/Overlays/MusicController.cs @@ -30,16 +30,7 @@ namespace osu.Game.Overlays [Resolved] private BeatmapManager beatmaps { get; set; } - public IBindableList BeatmapSets - { - get - { - if (LoadState < LoadState.Ready) - throw new InvalidOperationException($"{nameof(BeatmapSets)} should not be accessed before the music controller is loaded."); - - return beatmapSets; - } - } + public IBindableList BeatmapSets => beatmapSets; /// /// Point in time after which the current track will be restarted on triggering a "previous track" action. From 3b11235d3c65c7416e8e092d0a8d4aeb45c0c85c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 21 Jan 2022 20:45:10 +0900 Subject: [PATCH 07/46] Fix potentially cyclic subscription registration if a subscription's delegate accesses `Context` --- osu.Game/Database/RealmContextFactory.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index c11ff61039..d9c66ccc69 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -88,6 +88,8 @@ namespace osu.Game.Database registerSubscription(action); } + Debug.Assert(context != null); + // creating a context will ensure our schema is up-to-date and migrated. return context; } @@ -261,10 +263,13 @@ namespace osu.Game.Database { Debug.Assert(ThreadSafety.IsUpdateThread); + // Get context outside of flag update to ensure beyond doubt this can't be cyclic. + var realm = Context; + lock (contextLock) { current_thread_subscriptions_allowed.Value = true; - subscriptionActions[action] = action(Context); + subscriptionActions[action] = action(realm); current_thread_subscriptions_allowed.Value = false; } } From 9b63f15e68577a2d2b88d2613a6f61aff554097a Mon Sep 17 00:00:00 2001 From: Susko3 <16479013+Susko3@users.noreply.github.com> Date: Fri, 21 Jan 2022 13:58:30 +0100 Subject: [PATCH 08/46] Add failing test --- .../Visual/Menus/TestSceneLoginPanel.cs | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/osu.Game.Tests/Visual/Menus/TestSceneLoginPanel.cs b/osu.Game.Tests/Visual/Menus/TestSceneLoginPanel.cs index 4754a73f83..642cc68de5 100644 --- a/osu.Game.Tests/Visual/Menus/TestSceneLoginPanel.cs +++ b/osu.Game.Tests/Visual/Menus/TestSceneLoginPanel.cs @@ -8,6 +8,8 @@ using osu.Framework.Testing; using osu.Game.Graphics.UserInterface; using osu.Game.Online.API; using osu.Game.Overlays.Login; +using osu.Game.Users.Drawables; +using osuTK.Input; namespace osu.Game.Tests.Visual.Menus { @@ -15,6 +17,7 @@ namespace osu.Game.Tests.Visual.Menus public class TestSceneLoginPanel : OsuManualInputManagerTestScene { private LoginPanel loginPanel; + private int hideCount; [SetUpSteps] public void SetUpSteps() @@ -26,6 +29,7 @@ namespace osu.Game.Tests.Visual.Menus Anchor = Anchor.Centre, Origin = Anchor.Centre, Width = 0.5f, + RequestHide = () => hideCount++, }); }); } @@ -51,5 +55,22 @@ namespace osu.Game.Tests.Visual.Menus AddStep("enter password", () => loginPanel.ChildrenOfType().First().Text = "password"); AddStep("submit", () => loginPanel.ChildrenOfType().First(b => b.Text.ToString() == "Sign in").TriggerClick()); } + + [Test] + public void TestClickingOnFlagClosesPanel() + { + AddStep("reset hide count", () => hideCount = 0); + + AddStep("logout", () => API.Logout()); + AddStep("enter password", () => loginPanel.ChildrenOfType().First().Text = "password"); + AddStep("submit", () => loginPanel.ChildrenOfType().First(b => b.Text.ToString() == "Sign in").TriggerClick()); + + AddStep("click on flag", () => + { + InputManager.MoveMouseTo(loginPanel.ChildrenOfType().First()); + InputManager.Click(MouseButton.Left); + }); + AddAssert("hide requested", () => hideCount == 1); + } } } From 529610ee2e9c0d73c3a9c72c0531d4b716264644 Mon Sep 17 00:00:00 2001 From: Susko3 <16479013+Susko3@users.noreply.github.com> Date: Fri, 21 Jan 2022 14:01:49 +0100 Subject: [PATCH 09/46] Call the UserPanel `Action` when clicking on the flag --- osu.Game/Users/Drawables/UpdateableFlag.cs | 8 ++++++++ osu.Game/Users/ExtendedUserPanel.cs | 3 ++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/osu.Game/Users/Drawables/UpdateableFlag.cs b/osu.Game/Users/Drawables/UpdateableFlag.cs index 7db834bf83..e5debc0683 100644 --- a/osu.Game/Users/Drawables/UpdateableFlag.cs +++ b/osu.Game/Users/Drawables/UpdateableFlag.cs @@ -1,6 +1,7 @@ // 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.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; @@ -23,6 +24,12 @@ namespace osu.Game.Users.Drawables /// public bool ShowPlaceholderOnNull = true; + /// + /// Perform an action in addition to showing the country ranking. + /// This should be used to perform auxiliary tasks and not as a primary action for clicking a flag (to maintain a consistent UX). + /// + public Action Action; + public UpdateableFlag(Country country = null) { Country = country; @@ -52,6 +59,7 @@ namespace osu.Game.Users.Drawables protected override bool OnClick(ClickEvent e) { + Action?.Invoke(); rankingsOverlay?.ShowCountry(Country); return true; } diff --git a/osu.Game/Users/ExtendedUserPanel.cs b/osu.Game/Users/ExtendedUserPanel.cs index fc5e1eca5f..d0f693c37c 100644 --- a/osu.Game/Users/ExtendedUserPanel.cs +++ b/osu.Game/Users/ExtendedUserPanel.cs @@ -53,7 +53,8 @@ namespace osu.Game.Users protected UpdateableFlag CreateFlag() => new UpdateableFlag(User.Country) { - Size = new Vector2(39, 26) + Size = new Vector2(39, 26), + Action = Action, }; protected SpriteIcon CreateStatusIcon() => statusIcon = new SpriteIcon From ad3a01dc06215e17458357189cac15b9c603d093 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 21 Jan 2022 22:40:18 +0900 Subject: [PATCH 10/46] Use a more reliable method of reviving the update thread realm after blocking --- osu.Game/Database/RealmContextFactory.cs | 44 +++++++++++-------- .../Sections/DebugSettings/MemorySettings.cs | 4 -- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index d9c66ccc69..6fb9a2996b 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -69,30 +69,29 @@ namespace osu.Game.Database private Realm? context; - public Realm Context + public Realm Context => ensureUpdateContext(); + + private Realm ensureUpdateContext() { - get + if (!ThreadSafety.IsUpdateThread) + throw new InvalidOperationException(@$"Use {nameof(createContext)} when performing realm operations from a non-update thread"); + + lock (contextLock) { - if (!ThreadSafety.IsUpdateThread) - throw new InvalidOperationException(@$"Use {nameof(createContext)} when performing realm operations from a non-update thread"); - - lock (contextLock) + if (context == null) { - if (context == null) - { - context = createContext(); - Logger.Log(@$"Opened realm ""{context.Config.DatabasePath}"" at version {context.Config.SchemaVersion}"); + context = createContext(); + Logger.Log(@$"Opened realm ""{context.Config.DatabasePath}"" at version {context.Config.SchemaVersion}"); - // Resubscribe any subscriptions - foreach (var action in subscriptionActions.Keys) - registerSubscription(action); - } - - Debug.Assert(context != null); - - // creating a context will ensure our schema is up-to-date and migrated. - return context; + // Resubscribe any subscriptions + foreach (var action in subscriptionActions.Keys) + registerSubscription(action); } + + Debug.Assert(context != null); + + // creating a context will ensure our schema is up-to-date and migrated. + return context; } } @@ -506,6 +505,8 @@ namespace osu.Game.Database if (isDisposed) throw new ObjectDisposedException(nameof(RealmContextFactory)); + SynchronizationContext syncContext; + try { contextCreationLock.Wait(); @@ -515,6 +516,8 @@ namespace osu.Game.Database if (!ThreadSafety.IsUpdateThread && context != null) throw new InvalidOperationException(@$"{nameof(BlockAllOperations)} must be called from the update thread."); + syncContext = SynchronizationContext.Current; + Logger.Log(@"Blocking realm operations.", LoggingTarget.Database); context?.Dispose(); @@ -553,6 +556,9 @@ namespace osu.Game.Database { factory.contextCreationLock.Release(); Logger.Log(@"Restoring realm operations.", LoggingTarget.Database); + + // Post back to the update thread to revive any subscriptions. + syncContext?.Post(_ => ensureUpdateContext(), null); }); } diff --git a/osu.Game/Overlays/Settings/Sections/DebugSettings/MemorySettings.cs b/osu.Game/Overlays/Settings/Sections/DebugSettings/MemorySettings.cs index 84a54b208c..8d4fc5fc9f 100644 --- a/osu.Game/Overlays/Settings/Sections/DebugSettings/MemorySettings.cs +++ b/osu.Game/Overlays/Settings/Sections/DebugSettings/MemorySettings.cs @@ -33,10 +33,6 @@ 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; } }, }; From 9946003069f7666f74bc383ac1371043cf6d0b35 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 22 Jan 2022 05:09:40 +0900 Subject: [PATCH 11/46] Update osu.Game/Screens/Menu/IntroScreen.cs Co-authored-by: Salman Ahmed --- osu.Game/Screens/Menu/IntroScreen.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Menu/IntroScreen.cs b/osu.Game/Screens/Menu/IntroScreen.cs index db9d9b83c4..10be90a5f0 100644 --- a/osu.Game/Screens/Menu/IntroScreen.cs +++ b/osu.Game/Screens/Menu/IntroScreen.cs @@ -103,7 +103,7 @@ namespace osu.Game.Screens.Menu int setCount = sets.Count; - if (sets.Any()) + if (setCount > 0) { var found = sets[RNG.Next(0, setCount - 1)].Beatmaps.FirstOrDefault(); From 735414bc49381df02d6fb243c25054ba8f35e204 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 22 Jan 2022 17:27:27 +0100 Subject: [PATCH 12/46] Replace `TimeSignatures` enum with struct for storage of arbitrary meter --- .../Skinning/Default/CirclePiece.cs | 2 +- .../Formats/LegacyBeatmapDecoderTest.cs | 6 +-- .../NonVisual/BarLineGeneratorTest.cs | 6 +-- .../TestSceneNightcoreBeatContainer.cs | 4 +- .../ControlPoints/TimingControlPoint.cs | 4 +- .../Beatmaps/Formats/LegacyBeatmapDecoder.cs | 4 +- .../Beatmaps/Formats/LegacyBeatmapEncoder.cs | 2 +- osu.Game/Beatmaps/Timing/TimeSignature.cs | 45 +++++++++++++++++++ osu.Game/Beatmaps/Timing/TimeSignatures.cs | 4 +- osu.Game/Rulesets/Mods/Metronome.cs | 2 +- osu.Game/Rulesets/Mods/ModNightcore.cs | 10 ++--- osu.Game/Rulesets/Objects/BarLineGenerator.cs | 6 +-- .../Timeline/TimelineTickDisplay.cs | 2 +- .../RowAttributes/TimingRowAttribute.cs | 2 +- osu.Game/Screens/Edit/Timing/TimingSection.cs | 11 +++-- osu.Game/Screens/Menu/MenuSideFlashes.cs | 4 +- osu.Game/Screens/Menu/OsuLogo.cs | 2 +- 17 files changed, 84 insertions(+), 32 deletions(-) create mode 100644 osu.Game/Beatmaps/Timing/TimeSignature.cs diff --git a/osu.Game.Rulesets.Taiko/Skinning/Default/CirclePiece.cs b/osu.Game.Rulesets.Taiko/Skinning/Default/CirclePiece.cs index 8ca996159b..a106c4f629 100644 --- a/osu.Game.Rulesets.Taiko/Skinning/Default/CirclePiece.cs +++ b/osu.Game.Rulesets.Taiko/Skinning/Default/CirclePiece.cs @@ -153,7 +153,7 @@ namespace osu.Game.Rulesets.Taiko.Skinning.Default if (!effectPoint.KiaiMode) return; - if (beatIndex % (int)timingPoint.TimeSignature != 0) + if (beatIndex % timingPoint.TimeSignature.Numerator != 0) return; double duration = timingPoint.BeatLength * 2; diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs index 6ec14e6351..0459753b28 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs @@ -178,17 +178,17 @@ namespace osu.Game.Tests.Beatmaps.Formats var timingPoint = controlPoints.TimingPointAt(0); Assert.AreEqual(956, timingPoint.Time); Assert.AreEqual(329.67032967033, timingPoint.BeatLength); - Assert.AreEqual(TimeSignatures.SimpleQuadruple, timingPoint.TimeSignature); + Assert.AreEqual(TimeSignature.SimpleQuadruple, timingPoint.TimeSignature); timingPoint = controlPoints.TimingPointAt(48428); Assert.AreEqual(956, timingPoint.Time); Assert.AreEqual(329.67032967033d, timingPoint.BeatLength); - Assert.AreEqual(TimeSignatures.SimpleQuadruple, timingPoint.TimeSignature); + Assert.AreEqual(TimeSignature.SimpleQuadruple, timingPoint.TimeSignature); timingPoint = controlPoints.TimingPointAt(119637); Assert.AreEqual(119637, timingPoint.Time); Assert.AreEqual(659.340659340659, timingPoint.BeatLength); - Assert.AreEqual(TimeSignatures.SimpleQuadruple, timingPoint.TimeSignature); + Assert.AreEqual(TimeSignature.SimpleQuadruple, timingPoint.TimeSignature); var difficultyPoint = controlPoints.DifficultyPointAt(0); Assert.AreEqual(0, difficultyPoint.Time); diff --git a/osu.Game.Tests/NonVisual/BarLineGeneratorTest.cs b/osu.Game.Tests/NonVisual/BarLineGeneratorTest.cs index 834c05fd08..6ae8231deb 100644 --- a/osu.Game.Tests/NonVisual/BarLineGeneratorTest.cs +++ b/osu.Game.Tests/NonVisual/BarLineGeneratorTest.cs @@ -26,7 +26,7 @@ namespace osu.Game.Tests.NonVisual const int beat_length_numerator = 2000; const int beat_length_denominator = 7; - const TimeSignatures signature = TimeSignatures.SimpleQuadruple; + TimeSignature signature = TimeSignature.SimpleQuadruple; var beatmap = new Beatmap { @@ -49,7 +49,7 @@ namespace osu.Game.Tests.NonVisual for (int i = 0; i * beat_length_denominator < barLines.Count; i++) { var barLine = barLines[i * beat_length_denominator]; - int expectedTime = beat_length_numerator * (int)signature * i; + int expectedTime = beat_length_numerator * signature.Numerator * i; // every seventh bar's start time should be at least greater than the whole number we expect. // It cannot be less, as that can affect overlapping scroll algorithms @@ -60,7 +60,7 @@ namespace osu.Game.Tests.NonVisual Assert.IsTrue(Precision.AlmostEquals(barLine.StartTime, expectedTime)); // check major/minor lines for good measure too - Assert.AreEqual(i % (int)signature == 0, barLine.Major); + Assert.AreEqual(i % signature.Numerator == 0, barLine.Major); } } diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneNightcoreBeatContainer.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneNightcoreBeatContainer.cs index 951ee1489d..759e4fa4ec 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneNightcoreBeatContainer.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneNightcoreBeatContainer.cs @@ -24,8 +24,8 @@ namespace osu.Game.Tests.Visual.Gameplay Add(new ModNightcore.NightcoreBeatContainer()); - AddStep("change signature to quadruple", () => Beatmap.Value.Beatmap.ControlPointInfo.TimingPoints.ForEach(p => p.TimeSignature = TimeSignatures.SimpleQuadruple)); - AddStep("change signature to triple", () => Beatmap.Value.Beatmap.ControlPointInfo.TimingPoints.ForEach(p => p.TimeSignature = TimeSignatures.SimpleTriple)); + AddStep("change signature to quadruple", () => Beatmap.Value.Beatmap.ControlPointInfo.TimingPoints.ForEach(p => p.TimeSignature = TimeSignature.SimpleQuadruple)); + AddStep("change signature to triple", () => Beatmap.Value.Beatmap.ControlPointInfo.TimingPoints.ForEach(p => p.TimeSignature = TimeSignature.SimpleTriple)); } } } diff --git a/osu.Game/Beatmaps/ControlPoints/TimingControlPoint.cs b/osu.Game/Beatmaps/ControlPoints/TimingControlPoint.cs index ec20328fab..922439fcb8 100644 --- a/osu.Game/Beatmaps/ControlPoints/TimingControlPoint.cs +++ b/osu.Game/Beatmaps/ControlPoints/TimingControlPoint.cs @@ -13,7 +13,7 @@ namespace osu.Game.Beatmaps.ControlPoints /// /// The time signature at this control point. /// - public readonly Bindable TimeSignatureBindable = new Bindable(TimeSignatures.SimpleQuadruple) { Default = TimeSignatures.SimpleQuadruple }; + public readonly Bindable TimeSignatureBindable = new Bindable(TimeSignature.SimpleQuadruple); /// /// Default length of a beat in milliseconds. Used whenever there is no beatmap or track playing. @@ -35,7 +35,7 @@ namespace osu.Game.Beatmaps.ControlPoints /// /// The time signature at this control point. /// - public TimeSignatures TimeSignature + public TimeSignature TimeSignature { get => TimeSignatureBindable.Value; set => TimeSignatureBindable.Value = value; diff --git a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs index 893eb8ab78..8f3f05aa9f 100644 --- a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs @@ -340,9 +340,9 @@ namespace osu.Game.Beatmaps.Formats double beatLength = Parsing.ParseDouble(split[1].Trim()); double speedMultiplier = beatLength < 0 ? 100.0 / -beatLength : 1; - TimeSignatures timeSignature = TimeSignatures.SimpleQuadruple; + TimeSignature timeSignature = TimeSignature.SimpleQuadruple; if (split.Length >= 3) - timeSignature = split[2][0] == '0' ? TimeSignatures.SimpleQuadruple : (TimeSignatures)Parsing.ParseInt(split[2]); + timeSignature = split[2][0] == '0' ? TimeSignature.SimpleQuadruple : new TimeSignature(Parsing.ParseInt(split[2])); LegacySampleBank sampleSet = defaultSampleBank; if (split.Length >= 4) diff --git a/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs b/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs index ebdc882d2f..4cf6d3335f 100644 --- a/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs @@ -227,7 +227,7 @@ namespace osu.Game.Beatmaps.Formats if (effectPoint.OmitFirstBarLine) effectFlags |= LegacyEffectFlags.OmitFirstBarLine; - writer.Write(FormattableString.Invariant($"{(int)legacyControlPoints.TimingPointAt(time).TimeSignature},")); + writer.Write(FormattableString.Invariant($"{legacyControlPoints.TimingPointAt(time).TimeSignature.Numerator},")); writer.Write(FormattableString.Invariant($"{(int)toLegacySampleBank(tempHitSample.Bank)},")); writer.Write(FormattableString.Invariant($"{toLegacyCustomSampleBank(tempHitSample)},")); writer.Write(FormattableString.Invariant($"{tempHitSample.Volume},")); diff --git a/osu.Game/Beatmaps/Timing/TimeSignature.cs b/osu.Game/Beatmaps/Timing/TimeSignature.cs new file mode 100644 index 0000000000..5bfeea5e9b --- /dev/null +++ b/osu.Game/Beatmaps/Timing/TimeSignature.cs @@ -0,0 +1,45 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; + +namespace osu.Game.Beatmaps.Timing +{ + /// + /// Stores the time signature of a track. + /// For now, the lower numeral can only be 4; support for other denominators can be considered at a later date. + /// + public class TimeSignature : IEquatable + { + /// + /// The numerator of a signature. + /// + public int Numerator { get; } + + // TODO: support time signatures with a denominator other than 4 + // this in particular requires a new beatmap format. + + public TimeSignature(int numerator) + { + if (numerator < 1) + throw new ArgumentOutOfRangeException(nameof(numerator), numerator, "The numerator of a time signature must be positive."); + + Numerator = numerator; + } + + public static TimeSignature SimpleTriple => new TimeSignature(3); + public static TimeSignature SimpleQuadruple => new TimeSignature(4); + + public override string ToString() => $"{Numerator}/4"; + + public bool Equals(TimeSignature other) + { + if (ReferenceEquals(null, other)) return false; + if (ReferenceEquals(this, other)) return true; + + return Numerator == other.Numerator; + } + + public override int GetHashCode() => Numerator; + } +} diff --git a/osu.Game/Beatmaps/Timing/TimeSignatures.cs b/osu.Game/Beatmaps/Timing/TimeSignatures.cs index 33e6342ae6..d783d3f9ec 100644 --- a/osu.Game/Beatmaps/Timing/TimeSignatures.cs +++ b/osu.Game/Beatmaps/Timing/TimeSignatures.cs @@ -1,11 +1,13 @@ // 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.ComponentModel; namespace osu.Game.Beatmaps.Timing { - public enum TimeSignatures + [Obsolete("Use osu.Game.Beatmaps.Timing.TimeSignature instead.")] + public enum TimeSignatures // can be removed 20220722 { [Description("4/4")] SimpleQuadruple = 4, diff --git a/osu.Game/Rulesets/Mods/Metronome.cs b/osu.Game/Rulesets/Mods/Metronome.cs index 8b6d86c45f..b85a341577 100644 --- a/osu.Game/Rulesets/Mods/Metronome.cs +++ b/osu.Game/Rulesets/Mods/Metronome.cs @@ -33,7 +33,7 @@ namespace osu.Game.Rulesets.Mods if (!IsBeatSyncedWithTrack) return; - int timeSignature = (int)timingPoint.TimeSignature; + int timeSignature = timingPoint.TimeSignature.Numerator; // play metronome from one measure before the first object. if (BeatSyncClock.CurrentTime < firstHitTime - timingPoint.BeatLength * timeSignature) diff --git a/osu.Game/Rulesets/Mods/ModNightcore.cs b/osu.Game/Rulesets/Mods/ModNightcore.cs index a44967c21c..993efead33 100644 --- a/osu.Game/Rulesets/Mods/ModNightcore.cs +++ b/osu.Game/Rulesets/Mods/ModNightcore.cs @@ -85,7 +85,7 @@ namespace osu.Game.Rulesets.Mods { base.OnNewBeat(beatIndex, timingPoint, effectPoint, amplitudes); - int beatsPerBar = (int)timingPoint.TimeSignature; + int beatsPerBar = timingPoint.TimeSignature.Numerator; int segmentLength = beatsPerBar * Divisor * bars_per_segment; if (!IsBeatSyncedWithTrack) @@ -102,14 +102,14 @@ namespace osu.Game.Rulesets.Mods playBeatFor(beatIndex % segmentLength, timingPoint.TimeSignature); } - private void playBeatFor(int beatIndex, TimeSignatures signature) + private void playBeatFor(int beatIndex, TimeSignature signature) { if (beatIndex == 0) finishSample?.Play(); - switch (signature) + switch (signature.Numerator) { - case TimeSignatures.SimpleTriple: + case 3: switch (beatIndex % 6) { case 0: @@ -127,7 +127,7 @@ namespace osu.Game.Rulesets.Mods break; - case TimeSignatures.SimpleQuadruple: + case 4: switch (beatIndex % 4) { case 0: diff --git a/osu.Game/Rulesets/Objects/BarLineGenerator.cs b/osu.Game/Rulesets/Objects/BarLineGenerator.cs index e78aa5a5a0..d71a499119 100644 --- a/osu.Game/Rulesets/Objects/BarLineGenerator.cs +++ b/osu.Game/Rulesets/Objects/BarLineGenerator.cs @@ -41,9 +41,9 @@ namespace osu.Game.Rulesets.Objects int currentBeat = 0; // Stop on the beat before the next timing point, or if there is no next timing point stop slightly past the last object - double endTime = i < timingPoints.Count - 1 ? timingPoints[i + 1].Time - currentTimingPoint.BeatLength : lastHitTime + currentTimingPoint.BeatLength * (int)currentTimingPoint.TimeSignature; + double endTime = i < timingPoints.Count - 1 ? timingPoints[i + 1].Time - currentTimingPoint.BeatLength : lastHitTime + currentTimingPoint.BeatLength * currentTimingPoint.TimeSignature.Numerator; - double barLength = currentTimingPoint.BeatLength * (int)currentTimingPoint.TimeSignature; + double barLength = currentTimingPoint.BeatLength * currentTimingPoint.TimeSignature.Numerator; for (double t = currentTimingPoint.Time; Precision.DefinitelyBigger(endTime, t); t += barLength, currentBeat++) { @@ -60,7 +60,7 @@ namespace osu.Game.Rulesets.Objects BarLines.Add(new TBarLine { StartTime = t, - Major = currentBeat % (int)currentTimingPoint.TimeSignature == 0 + Major = currentBeat % currentTimingPoint.TimeSignature.Numerator == 0 }); } } diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineTickDisplay.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineTickDisplay.cs index 1415014e59..cc4041394d 100644 --- a/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineTickDisplay.cs +++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineTickDisplay.cs @@ -125,7 +125,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline if (beat == 0 && i == 0) nextMinTick = float.MinValue; - int indexInBar = beat % ((int)point.TimeSignature * beatDivisor.Value); + int indexInBar = beat % (point.TimeSignature.Numerator * beatDivisor.Value); int divisor = BindableBeatDivisor.GetDivisorForBeatIndex(beat, beatDivisor.Value); var colour = BindableBeatDivisor.GetColourFor(divisor, colours); diff --git a/osu.Game/Screens/Edit/Timing/RowAttributes/TimingRowAttribute.cs b/osu.Game/Screens/Edit/Timing/RowAttributes/TimingRowAttribute.cs index ab840e56a7..f8ec4aef25 100644 --- a/osu.Game/Screens/Edit/Timing/RowAttributes/TimingRowAttribute.cs +++ b/osu.Game/Screens/Edit/Timing/RowAttributes/TimingRowAttribute.cs @@ -13,7 +13,7 @@ namespace osu.Game.Screens.Edit.Timing.RowAttributes public class TimingRowAttribute : RowAttribute { private readonly BindableNumber beatLength; - private readonly Bindable timeSignature; + private readonly Bindable timeSignature; private OsuSpriteText text; public TimingRowAttribute(TimingControlPoint timing) diff --git a/osu.Game/Screens/Edit/Timing/TimingSection.cs b/osu.Game/Screens/Edit/Timing/TimingSection.cs index a0bb9ac506..e0b09ce980 100644 --- a/osu.Game/Screens/Edit/Timing/TimingSection.cs +++ b/osu.Game/Screens/Edit/Timing/TimingSection.cs @@ -15,7 +15,7 @@ namespace osu.Game.Screens.Edit.Timing internal class TimingSection : Section { private SettingsSlider bpmSlider; - private SettingsEnumDropdown timeSignature; + private SettingsDropdown timeSignature; private BPMTextBox bpmTextEntry; [BackgroundDependencyLoader] @@ -25,9 +25,14 @@ namespace osu.Game.Screens.Edit.Timing { bpmTextEntry = new BPMTextBox(), bpmSlider = new BPMSlider(), - timeSignature = new SettingsEnumDropdown + timeSignature = new SettingsDropdown { - LabelText = "Time Signature" + LabelText = "Time Signature", + Items = new[] + { + TimeSignature.SimpleTriple, + TimeSignature.SimpleQuadruple + } }, }); } diff --git a/osu.Game/Screens/Menu/MenuSideFlashes.cs b/osu.Game/Screens/Menu/MenuSideFlashes.cs index bdcd3020f8..cd0c75c1a1 100644 --- a/osu.Game/Screens/Menu/MenuSideFlashes.cs +++ b/osu.Game/Screens/Menu/MenuSideFlashes.cs @@ -94,9 +94,9 @@ namespace osu.Game.Screens.Menu if (beatIndex < 0) return; - if (effectPoint.KiaiMode ? beatIndex % 2 == 0 : beatIndex % (int)timingPoint.TimeSignature == 0) + if (effectPoint.KiaiMode ? beatIndex % 2 == 0 : beatIndex % timingPoint.TimeSignature.Numerator == 0) flash(leftBox, timingPoint.BeatLength, effectPoint.KiaiMode, amplitudes); - if (effectPoint.KiaiMode ? beatIndex % 2 == 1 : beatIndex % (int)timingPoint.TimeSignature == 0) + if (effectPoint.KiaiMode ? beatIndex % 2 == 1 : beatIndex % timingPoint.TimeSignature.Numerator == 0) flash(rightBox, timingPoint.BeatLength, effectPoint.KiaiMode, amplitudes); } diff --git a/osu.Game/Screens/Menu/OsuLogo.cs b/osu.Game/Screens/Menu/OsuLogo.cs index f9388097ac..c82efe2d32 100644 --- a/osu.Game/Screens/Menu/OsuLogo.cs +++ b/osu.Game/Screens/Menu/OsuLogo.cs @@ -282,7 +282,7 @@ namespace osu.Game.Screens.Menu { this.Delay(early_activation).Schedule(() => { - if (beatIndex % (int)timingPoint.TimeSignature == 0) + if (beatIndex % timingPoint.TimeSignature.Numerator == 0) sampleDownbeat.Play(); else sampleBeat.Play(); From f39f2c93b52526a3e5d5aa06e18d0f0ac85020c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 22 Jan 2022 18:54:14 +0100 Subject: [PATCH 13/46] Add control for arbitrary-numerator time signatures --- .../Editing/TestSceneLabelledTimeSignature.cs | 88 ++++++++++++++++++ .../Edit/Timing/LabelledTimeSignature.cs | 93 +++++++++++++++++++ 2 files changed, 181 insertions(+) create mode 100644 osu.Game.Tests/Visual/Editing/TestSceneLabelledTimeSignature.cs create mode 100644 osu.Game/Screens/Edit/Timing/LabelledTimeSignature.cs diff --git a/osu.Game.Tests/Visual/Editing/TestSceneLabelledTimeSignature.cs b/osu.Game.Tests/Visual/Editing/TestSceneLabelledTimeSignature.cs new file mode 100644 index 0000000000..cedbb5025f --- /dev/null +++ b/osu.Game.Tests/Visual/Editing/TestSceneLabelledTimeSignature.cs @@ -0,0 +1,88 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Linq; +using NUnit.Framework; +using osu.Framework.Graphics; +using osu.Framework.Testing; +using osu.Game.Beatmaps.Timing; +using osu.Game.Graphics.UserInterface; +using osu.Game.Screens.Edit.Timing; + +namespace osu.Game.Tests.Visual.Editing +{ + public class TestSceneLabelledTimeSignature : OsuManualInputManagerTestScene + { + private LabelledTimeSignature timeSignature; + + private void createLabelledTimeSignature(TimeSignature initial) => AddStep("create labelled time signature", () => + { + Child = timeSignature = new LabelledTimeSignature + { + Label = "Time Signature", + RelativeSizeAxes = Axes.None, + Width = 400, + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Current = { Value = initial } + }; + }); + + private OsuTextBox numeratorTextBox => timeSignature.ChildrenOfType().Single(); + + [Test] + public void TestInitialValue() + { + createLabelledTimeSignature(TimeSignature.SimpleTriple); + AddAssert("current is 3/4", () => timeSignature.Current.Value.Equals(TimeSignature.SimpleTriple)); + } + + [Test] + public void TestChangeViaCurrent() + { + createLabelledTimeSignature(TimeSignature.SimpleQuadruple); + AddAssert("current is 4/4", () => timeSignature.Current.Value.Equals(TimeSignature.SimpleQuadruple)); + + AddStep("set current to 5/4", () => timeSignature.Current.Value = new TimeSignature(5)); + + AddAssert("current is 5/4", () => timeSignature.Current.Value.Equals(new TimeSignature(5))); + AddAssert("numerator is 5", () => numeratorTextBox.Current.Value == "5"); + + AddStep("set current to 3/4", () => timeSignature.Current.Value = TimeSignature.SimpleTriple); + + AddAssert("current is 3/4", () => timeSignature.Current.Value.Equals(TimeSignature.SimpleTriple)); + AddAssert("numerator is 5", () => numeratorTextBox.Current.Value == "3"); + } + + [Test] + public void TestChangeNumerator() + { + createLabelledTimeSignature(TimeSignature.SimpleQuadruple); + AddAssert("current is 4/4", () => timeSignature.Current.Value.Equals(TimeSignature.SimpleQuadruple)); + + AddStep("focus text box", () => InputManager.ChangeFocus(numeratorTextBox)); + + AddStep("set numerator to 7", () => numeratorTextBox.Current.Value = "7"); + AddAssert("current is 4/4", () => timeSignature.Current.Value.Equals(TimeSignature.SimpleQuadruple)); + + AddStep("drop focus", () => InputManager.ChangeFocus(null)); + AddAssert("current is 7/4", () => timeSignature.Current.Value.Equals(new TimeSignature(7))); + } + + [Test] + public void TestInvalidChangeRollbackOnCommit() + { + createLabelledTimeSignature(TimeSignature.SimpleQuadruple); + AddAssert("current is 4/4", () => timeSignature.Current.Value.Equals(TimeSignature.SimpleQuadruple)); + + AddStep("focus text box", () => InputManager.ChangeFocus(numeratorTextBox)); + + AddStep("set numerator to 0", () => numeratorTextBox.Current.Value = "0"); + AddAssert("current is 4/4", () => timeSignature.Current.Value.Equals(TimeSignature.SimpleQuadruple)); + + AddStep("drop focus", () => InputManager.ChangeFocus(null)); + AddAssert("current is 4/4", () => timeSignature.Current.Value.Equals(TimeSignature.SimpleQuadruple)); + AddAssert("numerator is 4", () => numeratorTextBox.Current.Value == "4"); + } + } +} diff --git a/osu.Game/Screens/Edit/Timing/LabelledTimeSignature.cs b/osu.Game/Screens/Edit/Timing/LabelledTimeSignature.cs new file mode 100644 index 0000000000..52aece75ad --- /dev/null +++ b/osu.Game/Screens/Edit/Timing/LabelledTimeSignature.cs @@ -0,0 +1,93 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using osu.Framework.Allocation; +using osu.Framework.Bindables; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.UserInterface; +using osu.Game.Beatmaps.Timing; +using osu.Game.Graphics; +using osu.Game.Graphics.Sprites; +using osu.Game.Graphics.UserInterface; +using osu.Game.Graphics.UserInterfaceV2; + +namespace osu.Game.Screens.Edit.Timing +{ + internal class LabelledTimeSignature : LabelledComponent + { + public LabelledTimeSignature() + : base(false) + { + } + + protected override TimeSignatureBox CreateComponent() => new TimeSignatureBox(); + + internal class TimeSignatureBox : CompositeDrawable, IHasCurrentValue + { + private readonly BindableWithCurrent current = new BindableWithCurrent(TimeSignature.SimpleQuadruple); + + public Bindable Current + { + get => current.Current; + set => current.Current = value; + } + + private OsuNumberBox numeratorBox; + + [BackgroundDependencyLoader] + private void load() + { + AutoSizeAxes = Axes.Both; + InternalChild = new FillFlowContainer + { + AutoSizeAxes = Axes.Both, + Direction = FillDirection.Horizontal, + Children = new Drawable[] + { + numeratorBox = new OsuNumberBox + { + Width = 40, + Anchor = Anchor.CentreLeft, + Origin = Anchor.CentreLeft, + CornerRadius = CORNER_RADIUS, + CommitOnFocusLost = true + }, + new OsuSpriteText + { + Anchor = Anchor.CentreLeft, + Origin = Anchor.CentreLeft, + Margin = new MarginPadding(10), + Text = "/4", + Font = OsuFont.Default.With(size: 20) + } + } + }; + } + + protected override void LoadComplete() + { + base.LoadComplete(); + + Current.BindValueChanged(_ => updateFromCurrent(), true); + numeratorBox.OnCommit += (_, __) => updateFromNumeratorBox(); + } + + private void updateFromCurrent() + { + numeratorBox.Current.Value = Current.Value.Numerator.ToString(); + } + + private void updateFromNumeratorBox() + { + if (int.TryParse(numeratorBox.Current.Value, out int numerator) && numerator > 0) + Current.Value = new TimeSignature(numerator); + else + { + // trigger `Current` change to restore the numerator box's text to a valid value. + Current.TriggerChange(); + } + } + } + } +} From 54f7b1b8d0cec1638c67492fadf7c7fca8577f5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 22 Jan 2022 18:54:28 +0100 Subject: [PATCH 14/46] Use new time signature control on timing screen --- osu.Game/Screens/Edit/Timing/TimingSection.cs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/osu.Game/Screens/Edit/Timing/TimingSection.cs b/osu.Game/Screens/Edit/Timing/TimingSection.cs index e0b09ce980..cd0b56d338 100644 --- a/osu.Game/Screens/Edit/Timing/TimingSection.cs +++ b/osu.Game/Screens/Edit/Timing/TimingSection.cs @@ -6,7 +6,6 @@ using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Game.Beatmaps.ControlPoints; -using osu.Game.Beatmaps.Timing; using osu.Game.Graphics.UserInterfaceV2; using osu.Game.Overlays.Settings; @@ -15,7 +14,7 @@ namespace osu.Game.Screens.Edit.Timing internal class TimingSection : Section { private SettingsSlider bpmSlider; - private SettingsDropdown timeSignature; + private LabelledTimeSignature timeSignature; private BPMTextBox bpmTextEntry; [BackgroundDependencyLoader] @@ -25,15 +24,10 @@ namespace osu.Game.Screens.Edit.Timing { bpmTextEntry = new BPMTextBox(), bpmSlider = new BPMSlider(), - timeSignature = new SettingsDropdown + timeSignature = new LabelledTimeSignature { - LabelText = "Time Signature", - Items = new[] - { - TimeSignature.SimpleTriple, - TimeSignature.SimpleQuadruple - } - }, + Label = "Time Signature" + } }); } From 1ea5a2e6a75301341355f7a934b6b79a5f282d7c Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sun, 23 Jan 2022 10:11:07 +0300 Subject: [PATCH 15/46] Fix incorrect assert step name --- osu.Game.Tests/Visual/Editing/TestSceneLabelledTimeSignature.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneLabelledTimeSignature.cs b/osu.Game.Tests/Visual/Editing/TestSceneLabelledTimeSignature.cs index cedbb5025f..b34974dfc7 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneLabelledTimeSignature.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneLabelledTimeSignature.cs @@ -51,7 +51,7 @@ namespace osu.Game.Tests.Visual.Editing AddStep("set current to 3/4", () => timeSignature.Current.Value = TimeSignature.SimpleTriple); AddAssert("current is 3/4", () => timeSignature.Current.Value.Equals(TimeSignature.SimpleTriple)); - AddAssert("numerator is 5", () => numeratorTextBox.Current.Value == "3"); + AddAssert("numerator is 3", () => numeratorTextBox.Current.Value == "3"); } [Test] From e4758c9dbbee726593078de2c111fd243b4db582 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sun, 23 Jan 2022 10:13:32 +0300 Subject: [PATCH 16/46] Mark `LabelledTimeSignature` as public --- osu.Game/Screens/Edit/Timing/LabelledTimeSignature.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Edit/Timing/LabelledTimeSignature.cs b/osu.Game/Screens/Edit/Timing/LabelledTimeSignature.cs index 52aece75ad..66bd341393 100644 --- a/osu.Game/Screens/Edit/Timing/LabelledTimeSignature.cs +++ b/osu.Game/Screens/Edit/Timing/LabelledTimeSignature.cs @@ -14,7 +14,7 @@ using osu.Game.Graphics.UserInterfaceV2; namespace osu.Game.Screens.Edit.Timing { - internal class LabelledTimeSignature : LabelledComponent + public class LabelledTimeSignature : LabelledComponent { public LabelledTimeSignature() : base(false) @@ -23,7 +23,7 @@ namespace osu.Game.Screens.Edit.Timing protected override TimeSignatureBox CreateComponent() => new TimeSignatureBox(); - internal class TimeSignatureBox : CompositeDrawable, IHasCurrentValue + public class TimeSignatureBox : CompositeDrawable, IHasCurrentValue { private readonly BindableWithCurrent current = new BindableWithCurrent(TimeSignature.SimpleQuadruple); From a5493ce0d1f70b6db0d9ce846ed14c42f360946e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 23 Jan 2022 17:51:32 +0900 Subject: [PATCH 17/46] Fix incorrect nesting of statements causing completely broken logic --- osu.Game/Screens/Menu/IntroScreen.cs | 54 ++++++++++++++-------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/osu.Game/Screens/Menu/IntroScreen.cs b/osu.Game/Screens/Menu/IntroScreen.cs index 10be90a5f0..e66ecc74e1 100644 --- a/osu.Game/Screens/Menu/IntroScreen.cs +++ b/osu.Game/Screens/Menu/IntroScreen.cs @@ -99,51 +99,51 @@ namespace osu.Game.Screens.Menu { realmContextFactory.Run(realm => { - var sets = realm.All().Where(s => !s.DeletePending && !s.Protected).AsRealmCollection(); + var usableBeatmapSets = realm.All().Where(s => !s.DeletePending && !s.Protected).AsRealmCollection(); - int setCount = sets.Count; + int setCount = usableBeatmapSets.Count; if (setCount > 0) { - var found = sets[RNG.Next(0, setCount - 1)].Beatmaps.FirstOrDefault(); + var found = usableBeatmapSets[RNG.Next(0, setCount - 1)].Beatmaps.FirstOrDefault(); if (found != null) initialBeatmap = beatmaps.GetWorkingBeatmap(found); } }); + } - // we generally want a song to be playing on startup, so use the intro music even if a user has specified not to if no other track is available. - if (initialBeatmap == null) + // we generally want a song to be playing on startup, so use the intro music even if a user has specified not to if no other track is available. + if (initialBeatmap == null) + { + if (!loadThemedIntro()) { - if (!loadThemedIntro()) - { - // if we detect that the theme track or beatmap is unavailable this is either first startup or things are in a bad state. - // this could happen if a user has nuked their files store. for now, reimport to repair this. - var import = beatmaps.Import(new ZipArchiveReader(game.Resources.GetStream($"Tracks/{BeatmapFile}"), BeatmapFile)).GetResultSafely(); + // if we detect that the theme track or beatmap is unavailable this is either first startup or things are in a bad state. + // this could happen if a user has nuked their files store. for now, reimport to repair this. + var import = beatmaps.Import(new ZipArchiveReader(game.Resources.GetStream($"Tracks/{BeatmapFile}"), BeatmapFile)).GetResultSafely(); - import?.PerformWrite(b => b.Protected = true); + import?.PerformWrite(b => b.Protected = true); - loadThemedIntro(); - } + loadThemedIntro(); } + } - bool loadThemedIntro() + bool loadThemedIntro() + { + var setInfo = beatmaps.QueryBeatmapSet(b => b.Hash == BeatmapHash); + + if (setInfo == null) + return false; + + setInfo.PerformRead(s => { - var setInfo = beatmaps.QueryBeatmapSet(b => b.Hash == BeatmapHash); + if (s.Beatmaps.Count == 0) + return; - if (setInfo == null) - return false; + initialBeatmap = beatmaps.GetWorkingBeatmap(s.Beatmaps.First()); + }); - setInfo.PerformRead(s => - { - if (s.Beatmaps.Count == 0) - return; - - initialBeatmap = beatmaps.GetWorkingBeatmap(s.Beatmaps.First()); - }); - - return UsingThemedIntro = initialBeatmap != null; - } + return UsingThemedIntro = initialBeatmap != null; } } From 70a120ea8a5e7694c068624b6495fa857f95a4e8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 23 Jan 2022 18:00:24 +0900 Subject: [PATCH 18/46] Add missing lock coverage when using `subscriptionActions` dictionary --- osu.Game/Database/RealmContextFactory.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index 6fb9a2996b..606871337c 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -249,11 +249,13 @@ namespace osu.Game.Database return new InvokeOnDisposal(() => { - // TODO: this likely needs to be run on the update thread. - if (subscriptionActions.TryGetValue(action, out var unsubscriptionAction)) + lock (contextLock) { - unsubscriptionAction?.Dispose(); - subscriptionActions.Remove(action); + if (subscriptionActions.TryGetValue(action, out var unsubscriptionAction)) + { + unsubscriptionAction?.Dispose(); + subscriptionActions.Remove(action); + } } }); } From bd0eda7e908e7366198e0007d6db9434faded441 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 23 Jan 2022 18:01:39 +0900 Subject: [PATCH 19/46] Use method instead of property for realm query retrieval --- .../Bindings/DatabasedKeyBindingContainer.cs | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs b/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs index 5a9797ffce..ac33c64391 100644 --- a/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs +++ b/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs @@ -46,19 +46,16 @@ 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 + private IQueryable queryRealmKeyBindings() { - get - { - string rulesetName = ruleset?.ShortName; - return realmFactory.Context.All() - .Where(b => b.RulesetName == rulesetName && b.Variant == variant); - } + string rulesetName = ruleset?.ShortName; + return realmFactory.Context.All() + .Where(b => b.RulesetName == rulesetName && b.Variant == variant); } protected override void LoadComplete() { - realmSubscription = realmFactory.Register(realm => realmKeyBindings + realmSubscription = realmFactory.Register(realm => queryRealmKeyBindings() .QueryAsyncWithNotifications((sender, changes, error) => { // The first fire of this is a bit redundant as this is being called in base.LoadComplete, @@ -80,11 +77,11 @@ namespace osu.Game.Input.Bindings { var defaults = DefaultKeyBindings.ToList(); - List newBindings = realmKeyBindings.Detach() - // this ordering is important to ensure that we read entries from the database in the order - // enforced by DefaultKeyBindings. allow for song select to handle actions that may otherwise - // have been eaten by the music controller due to query order. - .OrderBy(b => defaults.FindIndex(d => (int)d.Action == b.ActionInt)).ToList(); + List newBindings = queryRealmKeyBindings().Detach() + // this ordering is important to ensure that we read entries from the database in the order + // enforced by DefaultKeyBindings. allow for song select to handle actions that may otherwise + // have been eaten by the music controller due to query order. + .OrderBy(b => defaults.FindIndex(d => (int)d.Action == b.ActionInt)).ToList(); // In the case no bindings were found in the database, presume this usage is for a non-databased ruleset. // This actually should never be required and can be removed if it is ever deemed to cause a problem. From f39ff1eacb3b5179f70aa47f0a892d23e2651e7d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 23 Jan 2022 18:13:28 +0900 Subject: [PATCH 20/46] Add unregistration on blocking This is the first part of the requirement of sending a `ChangeSet` event to ensure correct state during blocking time --- osu.Game/Database/RealmContextFactory.cs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index 606871337c..3e0f278e30 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -247,6 +247,8 @@ namespace osu.Game.Database registerSubscription(action); + // This token is returned to the consumer only. + // It will cause the registration to be permanently removed. return new InvokeOnDisposal(() => { lock (contextLock) @@ -269,12 +271,27 @@ namespace osu.Game.Database lock (contextLock) { + Debug.Assert(!customSubscriptionActions.TryGetValue(action, out var found) || found == null); + current_thread_subscriptions_allowed.Value = true; subscriptionActions[action] = action(realm); current_thread_subscriptions_allowed.Value = false; } } + /// + /// Unregister all subscriptions when the realm context is to be recycled. + /// Subscriptions will still remain and will be re-subscribed when the realm context returns. + /// + private void unregisterAllSubscriptions() + { + foreach (var action in subscriptionActions) + { + action.Value?.Dispose(); + subscriptionActions[action.Key] = null; + } + } + private Realm createContext() { if (isDisposed) @@ -519,6 +536,7 @@ namespace osu.Game.Database throw new InvalidOperationException(@$"{nameof(BlockAllOperations)} must be called from the update thread."); syncContext = SynchronizationContext.Current; + unregisterAllSubscriptions(); Logger.Log(@"Blocking realm operations.", LoggingTarget.Database); From 61cef42be977be4fe3cd612122de1f52556e03d1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 23 Jan 2022 19:42:26 +0900 Subject: [PATCH 21/46] Proof of concept realm subscriptions via `Register` --- osu.Game/Database/EmptyRealmSet.cs | 74 ++++++++++++++++++++ osu.Game/Database/RealmContextFactory.cs | 33 +++++++-- osu.Game/Screens/Select/BeatmapCarousel.cs | 4 +- osu.Game/Screens/Spectate/SpectatorScreen.cs | 21 +++--- 4 files changed, 112 insertions(+), 20 deletions(-) create mode 100644 osu.Game/Database/EmptyRealmSet.cs diff --git a/osu.Game/Database/EmptyRealmSet.cs b/osu.Game/Database/EmptyRealmSet.cs new file mode 100644 index 0000000000..2fecfcbe07 --- /dev/null +++ b/osu.Game/Database/EmptyRealmSet.cs @@ -0,0 +1,74 @@ +// 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.Collections; +using System.Collections.Generic; +using System.Collections.Specialized; +using System.ComponentModel; +using Realms; +using Realms.Schema; + +#nullable enable + +namespace osu.Game.Database +{ + public class EmptyRealmSet : IRealmCollection + { + private static List emptySet => new List(); + + public IEnumerator GetEnumerator() + { + return emptySet.GetEnumerator(); + } + + IEnumerator IEnumerable.GetEnumerator() + { + return ((IEnumerable)emptySet).GetEnumerator(); + } + + public int Count => emptySet.Count; + + public T this[int index] => emptySet[index]; + + public event NotifyCollectionChangedEventHandler? CollectionChanged + { + add => throw new NotImplementedException(); + remove => throw new NotImplementedException(); + } + + public event PropertyChangedEventHandler? PropertyChanged + { + add => throw new NotImplementedException(); + remove => throw new NotImplementedException(); + } + + public int IndexOf(object item) + { + return emptySet.IndexOf((T)item); + } + + public bool Contains(object item) + { + return emptySet.Contains((T)item); + } + + public IRealmCollection Freeze() + { + throw new NotImplementedException(); + } + + public IDisposable SubscribeForNotifications(NotificationCallbackDelegate callback) + { + throw new NotImplementedException(); + } + + public bool IsValid => throw new NotImplementedException(); + + public Realm Realm => throw new NotImplementedException(); + + public ObjectSchema ObjectSchema => throw new NotImplementedException(); + + public bool IsFrozen => throw new NotImplementedException(); + } +} diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index 3e0f278e30..32f7ac99c1 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -84,7 +84,7 @@ namespace osu.Game.Database Logger.Log(@$"Opened realm ""{context.Config.DatabasePath}"" at version {context.Config.SchemaVersion}"); // Resubscribe any subscriptions - foreach (var action in subscriptionActions.Keys) + foreach (var action in customSubscriptionActions.Keys) registerSubscription(action); } @@ -233,7 +233,22 @@ namespace osu.Game.Database } } - private readonly Dictionary, IDisposable?> subscriptionActions = new Dictionary, IDisposable?>(); + private readonly Dictionary, IDisposable?> customSubscriptionActions = new Dictionary, IDisposable?>(); + + private readonly Dictionary, Action> realmSubscriptionsResetMap = new Dictionary, Action>(); + + public IDisposable Register(Func> query, NotificationCallbackDelegate onChanged) + where T : RealmObjectBase + { + if (!ThreadSafety.IsUpdateThread) + throw new InvalidOperationException(@$"{nameof(Register)} must be called from the update thread."); + + realmSubscriptionsResetMap.Add(action, () => onChanged(new EmptyRealmSet(), null, null)); + + return Register(action); + + IDisposable? action(Realm realm) => query(realm).QueryAsyncWithNotifications(onChanged); + } /// /// Run work on realm that will be run every time the update thread realm context gets recycled. @@ -253,10 +268,11 @@ namespace osu.Game.Database { lock (contextLock) { - if (subscriptionActions.TryGetValue(action, out var unsubscriptionAction)) + if (customSubscriptionActions.TryGetValue(action, out var unsubscriptionAction)) { unsubscriptionAction?.Dispose(); - subscriptionActions.Remove(action); + customSubscriptionActions.Remove(action); + realmSubscriptionsResetMap.Remove(action); } } }); @@ -274,7 +290,7 @@ namespace osu.Game.Database Debug.Assert(!customSubscriptionActions.TryGetValue(action, out var found) || found == null); current_thread_subscriptions_allowed.Value = true; - subscriptionActions[action] = action(realm); + customSubscriptionActions[action] = action(realm); current_thread_subscriptions_allowed.Value = false; } } @@ -285,10 +301,13 @@ namespace osu.Game.Database /// private void unregisterAllSubscriptions() { - foreach (var action in subscriptionActions) + foreach (var action in realmSubscriptionsResetMap.Values) + action(); + + foreach (var action in customSubscriptionActions) { action.Value?.Dispose(); - subscriptionActions[action.Key] = null; + customSubscriptionActions[action.Key] = null; } } diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 6f3f467170..10ba23985e 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -190,7 +190,7 @@ namespace osu.Game.Screens.Select { base.LoadComplete(); - subscriptionSets = realmFactory.Register(realm => getBeatmapSets(realm).QueryAsyncWithNotifications(beatmapSetsChanged)); + subscriptionSets = realmFactory.Register(getBeatmapSets, 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. @@ -274,7 +274,7 @@ namespace osu.Game.Screens.Select } } - private IRealmCollection getBeatmapSets(Realm realm) => realm.All().Where(s => !s.DeletePending && !s.Protected).AsRealmCollection(); + private IQueryable getBeatmapSets(Realm realm) => realm.All().Where(s => !s.DeletePending && !s.Protected); public void RemoveBeatmapSet(BeatmapSetInfo beatmapSet) => removeBeatmapSet(beatmapSet.ID); diff --git a/osu.Game/Screens/Spectate/SpectatorScreen.cs b/osu.Game/Screens/Spectate/SpectatorScreen.cs index 9fa5bb8562..904648f727 100644 --- a/osu.Game/Screens/Spectate/SpectatorScreen.cs +++ b/osu.Game/Screens/Spectate/SpectatorScreen.cs @@ -17,6 +17,7 @@ using osu.Game.Online.Spectator; using osu.Game.Replays; using osu.Game.Rulesets; using osu.Game.Scoring; +using Realms; namespace osu.Game.Screens.Spectate { @@ -79,23 +80,21 @@ namespace osu.Game.Screens.Spectate playingUserStates.BindTo(spectatorClient.PlayingUserStates); playingUserStates.BindCollectionChanged(onPlayingUserStatesChanged, true); - 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]); - })); + realmSubscription = realmContextFactory.Register( + realm => realm.All().Where(s => !s.DeletePending), beatmapsChanged); foreach ((int id, var _) in userMap) spectatorClient.WatchUser(id); })); } + private void beatmapsChanged(IRealmCollection items, ChangeSet changes, Exception ___) + { + if (changes?.InsertedIndices == null) return; + + foreach (int c in changes.InsertedIndices) beatmapUpdated(items[c]); + } + private void beatmapUpdated(BeatmapSetInfo beatmapSet) { foreach ((int userId, _) in userMap) From e9e3e024a19d4867673152efa06f89842937d032 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 23 Jan 2022 19:50:29 +0900 Subject: [PATCH 22/46] Update all usages of `QueryAsyncWithNotifications` to use new `Register` pathway --- .../Bindings/DatabasedKeyBindingContainer.cs | 13 +++++----- osu.Game/Online/BeatmapDownloadTracker.cs | 4 ++-- .../OnlinePlayBeatmapAvailabilityTracker.cs | 4 ++-- osu.Game/Online/ScoreDownloadTracker.cs | 4 ++-- osu.Game/Overlays/MusicController.cs | 2 +- .../Overlays/Settings/Sections/SkinSection.cs | 19 +++++++-------- osu.Game/Screens/Select/BeatmapCarousel.cs | 6 ++--- .../Screens/Select/Carousel/TopLocalRank.cs | 24 +++++++++---------- .../Select/Leaderboards/BeatmapLeaderboard.cs | 14 +++++------ 9 files changed, 44 insertions(+), 46 deletions(-) diff --git a/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs b/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs index ac33c64391..4ad5693867 100644 --- a/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs +++ b/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs @@ -55,13 +55,12 @@ namespace osu.Game.Input.Bindings protected override void LoadComplete() { - realmSubscription = realmFactory.Register(realm => queryRealmKeyBindings() - .QueryAsyncWithNotifications((sender, changes, error) => - { - // 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(); - })); + realmSubscription = realmFactory.Register(realm => queryRealmKeyBindings(), (sender, changes, error) => + { + // 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 d50ab5a347..70599a167b 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.Register(realm => realm.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), (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 fdcf2b39c6..8dd28f5417 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 = realmContextFactory.Register(realm => filteredBeatmaps().QueryAsyncWithNotifications((items, changes, ___) => + realmSubscription = realmContextFactory.Register(realm => filteredBeatmaps(), (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 de1d6fd94a..72e95bd6df 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.Register(realm => realm.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), (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 01a2c9d354..24d907285a 100644 --- a/osu.Game/Overlays/MusicController.cs +++ b/osu.Game/Overlays/MusicController.cs @@ -93,7 +93,7 @@ namespace osu.Game.Overlays foreach (var s in availableBeatmaps) beatmapSets.Add(s.Detach()); - beatmapSubscription = realmFactory.Register(realm => availableBeatmaps.QueryAsyncWithNotifications(beatmapsChanged)); + beatmapSubscription = realmFactory.Register(realm => availableBeatmaps, beatmapsChanged); } private void beatmapsChanged(IRealmCollection sender, ChangeSet changes, Exception error) diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index 11a7275168..c767edec71 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -51,7 +51,7 @@ namespace osu.Game.Overlays.Settings.Sections private IDisposable realmSubscription; - private IQueryable realmSkins => + private IQueryable queryRealmSkins() => realmFactory.Context.All() .Where(s => !s.DeletePending) .OrderByDescending(s => s.Protected) // protected skins should be at the top. @@ -83,13 +83,12 @@ namespace osu.Game.Overlays.Settings.Sections skinDropdown.Current = dropdownBindable; - realmSubscription = realmFactory.Register(realm => realmSkins - .QueryAsyncWithNotifications((sender, changes, error) => - { - // 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(); - })); + realmSubscription = realmFactory.Register(realm => queryRealmSkins(), (sender, changes, error) => + { + // 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(); @@ -129,9 +128,9 @@ namespace osu.Game.Overlays.Settings.Sections private void updateItems() { - int protectedCount = realmSkins.Count(s => s.Protected); + int protectedCount = queryRealmSkins().Count(s => s.Protected); - skinItems = realmSkins.ToLive(realmFactory); + skinItems = queryRealmSkins().ToLive(realmFactory); skinItems.Insert(protectedCount, random_skin_info); diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 10ba23985e..889a6f5b79 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -191,12 +191,12 @@ namespace osu.Game.Screens.Select base.LoadComplete(); subscriptionSets = realmFactory.Register(getBeatmapSets, beatmapSetsChanged); - subscriptionBeatmaps = realmFactory.Register(realm => realm.All().Where(b => !b.Hidden).QueryAsyncWithNotifications(beatmapsChanged)); + subscriptionBeatmaps = realmFactory.Register(realm => realm.All().Where(b => !b.Hidden), 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.Register(realm => realm.All().Where(s => s.DeletePending && !s.Protected).QueryAsyncWithNotifications(deletedBeatmapSetsChanged)); - subscriptionHiddenBeatmaps = realmFactory.Register(realm => realm.All().Where(b => b.Hidden).QueryAsyncWithNotifications(beatmapsChanged)); + subscriptionDeletedSets = realmFactory.Register(realm => realm.All().Where(s => s.DeletePending && !s.Protected), deletedBeatmapSetsChanged); + subscriptionHiddenBeatmaps = realmFactory.Register(realm => realm.All().Where(b => b.Hidden), beatmapsChanged); } private void deletedBeatmapSetsChanged(IRealmCollection sender, ChangeSet changes, Exception error) diff --git a/osu.Game/Screens/Select/Carousel/TopLocalRank.cs b/osu.Game/Screens/Select/Carousel/TopLocalRank.cs index ee3930364b..fc2188e597 100644 --- a/osu.Game/Screens/Select/Carousel/TopLocalRank.cs +++ b/osu.Game/Screens/Select/Carousel/TopLocalRank.cs @@ -49,18 +49,18 @@ namespace osu.Game.Screens.Select.Carousel { scoreSubscription?.Dispose(); 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); - })); + 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), + (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 954c2a6413..463f878e2a 100644 --- a/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs +++ b/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs @@ -114,13 +114,13 @@ namespace osu.Game.Screens.Select.Leaderboards return; scoreSubscription = realmFactory.Register(realm => - realm.All() - .Filter($"{nameof(ScoreInfo.BeatmapInfo)}.{nameof(BeatmapInfo.ID)} = $0", beatmapInfo.ID) - .QueryAsyncWithNotifications((_, changes, ___) => - { - if (!IsOnlineScope) - RefreshScores(); - })); + realm.All() + .Filter($"{nameof(ScoreInfo.BeatmapInfo)}.{nameof(BeatmapInfo.ID)} = $0", beatmapInfo.ID), + (_, changes, ___) => + { + if (!IsOnlineScope) + RefreshScores(); + }); } protected override void Reset() From db8639435538108982acffad2d9e8da710dd71e7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 23 Jan 2022 20:27:35 +0900 Subject: [PATCH 23/46] Fix `TestResources` returning a test `BeatmapSetInfo` that can't be laoded directly into realm --- osu.Game.Tests/Resources/TestResources.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Resources/TestResources.cs b/osu.Game.Tests/Resources/TestResources.cs index d2cab09ac9..81b624f908 100644 --- a/osu.Game.Tests/Resources/TestResources.cs +++ b/osu.Game.Tests/Resources/TestResources.cs @@ -80,7 +80,10 @@ namespace osu.Game.Tests.Resources public static BeatmapSetInfo CreateTestBeatmapSetInfo(int? difficultyCount = null, RulesetInfo[] rulesets = null) { int j = 0; - RulesetInfo getRuleset() => rulesets?[j++ % rulesets.Length] ?? new OsuRuleset().RulesetInfo; + + rulesets ??= new[] { new OsuRuleset().RulesetInfo }; + + RulesetInfo getRuleset() => rulesets?[j++ % rulesets.Length]; int setId = Interlocked.Increment(ref importId); From 0709a2ac9be3376193a4aeb266b497683b1bde7b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 23 Jan 2022 20:28:13 +0900 Subject: [PATCH 24/46] Add test coverage of realm subscription scenarios --- .../RealmSubscriptionRegistrationTests.cs | 138 ++++++++++++++++++ 1 file changed, 138 insertions(+) create mode 100644 osu.Game.Tests/Database/RealmSubscriptionRegistrationTests.cs diff --git a/osu.Game.Tests/Database/RealmSubscriptionRegistrationTests.cs b/osu.Game.Tests/Database/RealmSubscriptionRegistrationTests.cs new file mode 100644 index 0000000000..16e9f31c7b --- /dev/null +++ b/osu.Game.Tests/Database/RealmSubscriptionRegistrationTests.cs @@ -0,0 +1,138 @@ +// 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.Collections.Generic; +using System.Linq; +using NUnit.Framework; +using osu.Framework.Allocation; +using osu.Game.Beatmaps; +using osu.Game.Rulesets; +using osu.Game.Tests.Resources; +using Realms; + +#nullable enable + +namespace osu.Game.Tests.Database +{ + [TestFixture] + public class RealmSubscriptionRegistrationTests : RealmTest + { + [Test] + public void TestSubscriptionWithContextLoss() + { + IEnumerable? resolvedItems = null; + ChangeSet? lastChanges = null; + + RunTestWithRealm((realmFactory, _) => + { + realmFactory.Write(realm => realm.Add(TestResources.CreateTestBeatmapSetInfo())); + + var registration = realmFactory.Register(realm => realm.All(), onChanged); + + testEventsArriving(true); + + // All normal until here. + // Now let's yank the main realm context. + resolvedItems = null; + lastChanges = null; + + using (realmFactory.BlockAllOperations()) + Assert.That(resolvedItems, Is.Empty); + + realmFactory.Write(realm => realm.Add(TestResources.CreateTestBeatmapSetInfo())); + + testEventsArriving(true); + + // Now let's try unsubscribing. + resolvedItems = null; + lastChanges = null; + + registration.Dispose(); + + realmFactory.Write(realm => realm.Add(TestResources.CreateTestBeatmapSetInfo())); + + testEventsArriving(false); + + // And make sure even after another context loss we don't get firings. + using (realmFactory.BlockAllOperations()) + Assert.That(resolvedItems, Is.Null); + + realmFactory.Write(realm => realm.Add(TestResources.CreateTestBeatmapSetInfo())); + + testEventsArriving(false); + + void testEventsArriving(bool shouldArrive) + { + realmFactory.Run(realm => realm.Refresh()); + + if (shouldArrive) + Assert.That(resolvedItems, Has.One.Items); + else + Assert.That(resolvedItems, Is.Null); + + realmFactory.Write(realm => + { + realm.RemoveAll(); + realm.RemoveAll(); + }); + + realmFactory.Run(realm => realm.Refresh()); + + if (shouldArrive) + Assert.That(lastChanges?.DeletedIndices, Has.One.Items); + else + Assert.That(lastChanges, Is.Null); + } + }); + + void onChanged(IRealmCollection sender, ChangeSet? changes, Exception error) + { + if (changes == null) + resolvedItems = sender; + + lastChanges = changes; + } + } + + [Test] + public void TestCustomRegisterWithContextLoss() + { + RunTestWithRealm((realmFactory, _) => + { + BeatmapSetInfo? beatmapSetInfo = null; + + realmFactory.Write(realm => realm.Add(TestResources.CreateTestBeatmapSetInfo())); + + var subscription = realmFactory.Register(realm => + { + beatmapSetInfo = realm.All().First(); + + return new InvokeOnDisposal(() => beatmapSetInfo = null); + }); + + Assert.That(beatmapSetInfo, Is.Not.Null); + + using (realmFactory.BlockAllOperations()) + { + // custom disposal action fired when context lost. + Assert.That(beatmapSetInfo, Is.Null); + } + + // re-registration after context restore. + realmFactory.Run(realm => realm.Refresh()); + Assert.That(beatmapSetInfo, Is.Not.Null); + + subscription.Dispose(); + + Assert.That(beatmapSetInfo, Is.Null); + + using (realmFactory.BlockAllOperations()) + Assert.That(beatmapSetInfo, Is.Null); + + realmFactory.Run(realm => realm.Refresh()); + Assert.That(beatmapSetInfo, Is.Null); + }); + } + } +} From 5e7993c35afaae58a3dec41dddd0463b4e07a53a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 23 Jan 2022 20:38:34 +0900 Subject: [PATCH 25/46] Post disposal to synchronisation context --- osu.Game/Database/RealmContextFactory.cs | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index 32f7ac99c1..26943c1951 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -260,19 +260,29 @@ namespace osu.Game.Database if (!ThreadSafety.IsUpdateThread) throw new InvalidOperationException(@$"{nameof(Register)} must be called from the update thread."); + var syncContext = SynchronizationContext.Current; + registerSubscription(action); // This token is returned to the consumer only. // It will cause the registration to be permanently removed. return new InvokeOnDisposal(() => { - lock (contextLock) + if (ThreadSafety.IsUpdateThread) + unsubscribe(); + else + syncContext.Post(_ => unsubscribe(), null); + + void unsubscribe() { - if (customSubscriptionActions.TryGetValue(action, out var unsubscriptionAction)) + lock (contextLock) { - unsubscriptionAction?.Dispose(); - customSubscriptionActions.Remove(action); - realmSubscriptionsResetMap.Remove(action); + if (customSubscriptionActions.TryGetValue(action, out var unsubscriptionAction)) + { + unsubscriptionAction?.Dispose(); + customSubscriptionActions.Remove(action); + realmSubscriptionsResetMap.Remove(action); + } } } }); From 249f0f9697f828649c1734eeec281e0b0876ba77 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 23 Jan 2022 23:15:39 +0900 Subject: [PATCH 26/46] Add more lengthy comment explaining cyclic avoidance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bartłomiej Dach --- osu.Game/Database/RealmContextFactory.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index 26943c1951..c8fa298f91 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -292,7 +292,9 @@ namespace osu.Game.Database { Debug.Assert(ThreadSafety.IsUpdateThread); - // Get context outside of flag update to ensure beyond doubt this can't be cyclic. + // Retrieve context outside of flag update to ensure that the context is constructed, + // as attempting to access it inside the subscription if it's not constructed would lead to + // cyclic invocations of the subscription callback. var realm = Context; lock (contextLock) From deb167086212faa8d6c51fe911bcd54c1cc85c73 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 23 Jan 2022 23:17:33 +0900 Subject: [PATCH 27/46] Use `Array.Empty` instead of constructed list --- osu.Game/Database/EmptyRealmSet.cs | 42 +++++------------------------- 1 file changed, 7 insertions(+), 35 deletions(-) diff --git a/osu.Game/Database/EmptyRealmSet.cs b/osu.Game/Database/EmptyRealmSet.cs index 2fecfcbe07..b7f27ba035 100644 --- a/osu.Game/Database/EmptyRealmSet.cs +++ b/osu.Game/Database/EmptyRealmSet.cs @@ -15,21 +15,14 @@ namespace osu.Game.Database { public class EmptyRealmSet : IRealmCollection { - private static List emptySet => new List(); - - public IEnumerator GetEnumerator() - { - return emptySet.GetEnumerator(); - } - - IEnumerator IEnumerable.GetEnumerator() - { - return ((IEnumerable)emptySet).GetEnumerator(); - } + private IList emptySet => Array.Empty(); + public IEnumerator GetEnumerator() => emptySet.GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => emptySet.GetEnumerator(); public int Count => emptySet.Count; - public T this[int index] => emptySet[index]; + public int IndexOf(object item) => emptySet.IndexOf((T)item); + public bool Contains(object item) => emptySet.Contains((T)item); public event NotifyCollectionChangedEventHandler? CollectionChanged { @@ -43,32 +36,11 @@ namespace osu.Game.Database remove => throw new NotImplementedException(); } - public int IndexOf(object item) - { - return emptySet.IndexOf((T)item); - } - - public bool Contains(object item) - { - return emptySet.Contains((T)item); - } - - public IRealmCollection Freeze() - { - throw new NotImplementedException(); - } - - public IDisposable SubscribeForNotifications(NotificationCallbackDelegate callback) - { - throw new NotImplementedException(); - } - + public IRealmCollection Freeze() => throw new NotImplementedException(); + public IDisposable SubscribeForNotifications(NotificationCallbackDelegate callback) => throw new NotImplementedException(); public bool IsValid => throw new NotImplementedException(); - public Realm Realm => throw new NotImplementedException(); - public ObjectSchema ObjectSchema => throw new NotImplementedException(); - public bool IsFrozen => throw new NotImplementedException(); } } From 351c766ea1f211b8eebe8ff58cb7d5fd0e813739 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 23 Jan 2022 23:20:03 +0900 Subject: [PATCH 28/46] Fix one remaining instance of realm query as property --- osu.Game/Overlays/MusicController.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/osu.Game/Overlays/MusicController.cs b/osu.Game/Overlays/MusicController.cs index 24d907285a..8caa69e78c 100644 --- a/osu.Game/Overlays/MusicController.cs +++ b/osu.Game/Overlays/MusicController.cs @@ -80,9 +80,10 @@ namespace osu.Game.Overlays mods.BindValueChanged(_ => ResetTrackAdjustments(), true); } - private IQueryable availableBeatmaps => realmFactory.Context - .All() - .Where(s => !s.DeletePending); + private IQueryable queryRealmBeatmapSets() => + realmFactory.Context + .All() + .Where(s => !s.DeletePending); protected override void LoadComplete() { @@ -90,10 +91,10 @@ namespace osu.Game.Overlays // 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) + foreach (var s in queryRealmBeatmapSets()) beatmapSets.Add(s.Detach()); - beatmapSubscription = realmFactory.Register(realm => availableBeatmaps, beatmapsChanged); + beatmapSubscription = realmFactory.Register(realm => queryRealmBeatmapSets(), beatmapsChanged); } private void beatmapsChanged(IRealmCollection sender, ChangeSet changes, Exception error) From 4e5a1f27a8c35f462385dfecdeffe9e11d8c14b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 23 Jan 2022 15:14:53 +0100 Subject: [PATCH 29/46] Initialise `Simple{Triple,Quadruple}` only once ever rather than create every time --- osu.Game/Beatmaps/Timing/TimeSignature.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Beatmaps/Timing/TimeSignature.cs b/osu.Game/Beatmaps/Timing/TimeSignature.cs index 5bfeea5e9b..eebbcc34cd 100644 --- a/osu.Game/Beatmaps/Timing/TimeSignature.cs +++ b/osu.Game/Beatmaps/Timing/TimeSignature.cs @@ -27,8 +27,8 @@ namespace osu.Game.Beatmaps.Timing Numerator = numerator; } - public static TimeSignature SimpleTriple => new TimeSignature(3); - public static TimeSignature SimpleQuadruple => new TimeSignature(4); + public static TimeSignature SimpleTriple { get; } = new TimeSignature(3); + public static TimeSignature SimpleQuadruple { get; } = new TimeSignature(4); public override string ToString() => $"{Numerator}/4"; From bd748686fad63ccb40f13ae9498a53f2c3f47570 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 23 Jan 2022 15:20:51 +0100 Subject: [PATCH 30/46] Adjust spacing of time signature numerator input box --- osu.Game/Screens/Edit/Timing/LabelledTimeSignature.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Edit/Timing/LabelledTimeSignature.cs b/osu.Game/Screens/Edit/Timing/LabelledTimeSignature.cs index 66bd341393..51b58bd3dc 100644 --- a/osu.Game/Screens/Edit/Timing/LabelledTimeSignature.cs +++ b/osu.Game/Screens/Edit/Timing/LabelledTimeSignature.cs @@ -57,8 +57,12 @@ namespace osu.Game.Screens.Edit.Timing { Anchor = Anchor.CentreLeft, Origin = Anchor.CentreLeft, - Margin = new MarginPadding(10), - Text = "/4", + Margin = new MarginPadding + { + Left = 5, + Right = CONTENT_PADDING_HORIZONTAL + }, + Text = "/ 4", Font = OsuFont.Default.With(size: 20) } } From e236f5d604ee30d92fffccac17702ea4d341abc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 23 Jan 2022 20:28:19 +0100 Subject: [PATCH 31/46] Add failing test coverage for correct beatmap filename generation on save --- osu.Game.Tests/Visual/Editing/TestSceneEditorSaving.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneEditorSaving.cs b/osu.Game.Tests/Visual/Editing/TestSceneEditorSaving.cs index f89be0adf3..bf3b46c6f7 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneEditorSaving.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneEditorSaving.cs @@ -46,6 +46,7 @@ namespace osu.Game.Tests.Visual.Editing editorBeatmap.BeatmapInfo.Metadata.Artist = "artist"; editorBeatmap.BeatmapInfo.Metadata.Title = "title"; }); + AddStep("Set author", () => editorBeatmap.BeatmapInfo.Metadata.Author.Username = "author"); AddStep("Set difficulty name", () => editorBeatmap.BeatmapInfo.DifficultyName = "difficulty"); AddStep("Add timing point", () => editorBeatmap.ControlPointInfo.Add(0, new TimingControlPoint())); @@ -64,6 +65,7 @@ namespace osu.Game.Tests.Visual.Editing AddStep("Save", () => InputManager.Keys(PlatformAction.Save)); checkMutations(); + AddAssert("Beatmap has correct .osu file path", () => editorBeatmap.BeatmapInfo.Path == "artist - title (author) [difficulty].osu"); AddStep("Exit", () => InputManager.Key(Key.Escape)); @@ -88,6 +90,7 @@ namespace osu.Game.Tests.Visual.Editing AddAssert("Beatmap contains single hitcircle", () => editorBeatmap.HitObjects.Count == 1); AddAssert("Beatmap has correct overall difficulty", () => editorBeatmap.Difficulty.OverallDifficulty == 7); AddAssert("Beatmap has correct metadata", () => editorBeatmap.BeatmapInfo.Metadata.Artist == "artist" && editorBeatmap.BeatmapInfo.Metadata.Title == "title"); + AddAssert("Beatmap has correct author", () => editorBeatmap.BeatmapInfo.Metadata.Author.Username == "author"); AddAssert("Beatmap has correct difficulty name", () => editorBeatmap.BeatmapInfo.DifficultyName == "difficulty"); } } From 838a9f69ed078e6e954ca393e1aa5d6fee9ac4b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 23 Jan 2022 19:41:41 +0100 Subject: [PATCH 32/46] Fix saved beatmap filename depending on `ToString()` implementation --- osu.Game/Beatmaps/BeatmapModelManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/BeatmapModelManager.cs b/osu.Game/Beatmaps/BeatmapModelManager.cs index 44d6af5b73..ead86c1059 100644 --- a/osu.Game/Beatmaps/BeatmapModelManager.cs +++ b/osu.Game/Beatmaps/BeatmapModelManager.cs @@ -88,7 +88,7 @@ namespace osu.Game.Beatmaps private static string getFilename(BeatmapInfo beatmapInfo) { var metadata = beatmapInfo.Metadata; - return $"{metadata.Artist} - {metadata.Title} ({metadata.Author}) [{beatmapInfo.DifficultyName}].osu".GetValidArchiveContentFilename(); + return $"{metadata.Artist} - {metadata.Title} ({metadata.Author.Username}) [{beatmapInfo.DifficultyName}].osu".GetValidArchiveContentFilename(); } /// From 997c13f64367f07fcd81f0107408774d2f0fd241 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 24 Jan 2022 13:36:51 +0900 Subject: [PATCH 33/46] Add locking over `realmSubscriptionsResetMap` for sanity --- osu.Game/Database/RealmContextFactory.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index c8fa298f91..522a5fdfd9 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -243,9 +243,11 @@ namespace osu.Game.Database if (!ThreadSafety.IsUpdateThread) throw new InvalidOperationException(@$"{nameof(Register)} must be called from the update thread."); - realmSubscriptionsResetMap.Add(action, () => onChanged(new EmptyRealmSet(), null, null)); - - return Register(action); + lock (contextLock) + { + realmSubscriptionsResetMap.Add(action, () => onChanged(new EmptyRealmSet(), null, null)); + return Register(action); + } IDisposable? action(Realm realm) => query(realm).QueryAsyncWithNotifications(onChanged); } From d7a9c5fd410e370f89e83ef1bd3475a418ffe4ed Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 24 Jan 2022 14:36:48 +0900 Subject: [PATCH 34/46] Add settings buttons to allow temporarily blocking realm access --- .../Sections/DebugSettings/MemorySettings.cs | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/osu.Game/Overlays/Settings/Sections/DebugSettings/MemorySettings.cs b/osu.Game/Overlays/Settings/Sections/DebugSettings/MemorySettings.cs index 8d4fc5fc9f..f058c274e4 100644 --- a/osu.Game/Overlays/Settings/Sections/DebugSettings/MemorySettings.cs +++ b/osu.Game/Overlays/Settings/Sections/DebugSettings/MemorySettings.cs @@ -1,6 +1,8 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System.Threading; +using System.Threading.Tasks; using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Localisation; @@ -17,6 +19,9 @@ namespace osu.Game.Overlays.Settings.Sections.DebugSettings [BackgroundDependencyLoader] private void load(GameHost host, RealmContextFactory realmFactory) { + SettingsButton blockAction; + SettingsButton unblockAction; + Children = new Drawable[] { new SettingsButton @@ -35,6 +40,43 @@ namespace osu.Game.Overlays.Settings.Sections.DebugSettings } } }, + blockAction = new SettingsButton + { + Text = "Block realm", + }, + unblockAction = new SettingsButton + { + Text = "Unblock realm", + }, + }; + + blockAction.Action = () => + { + var blocking = realmFactory.BlockAllOperations(); + blockAction.Enabled.Value = false; + + // As a safety measure, unblock after 10 seconds. + // This is to handle the case where a dev may block, but then something on the update thread + // accesses realm and blocks for eternity. + Task.Factory.StartNew(() => + { + Thread.Sleep(10000); + unblock(); + }); + + unblockAction.Action = unblock; + + void unblock() + { + blocking?.Dispose(); + blocking = null; + + Scheduler.Add(() => + { + blockAction.Enabled.Value = true; + unblockAction.Action = null; + }); + } }; } } From 40aa8731900bd7f856e5f5b417aa85897bf79f0f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 24 Jan 2022 14:37:36 +0900 Subject: [PATCH 35/46] Rename register methods to better explain their purpose --- osu.Game.Tests/Database/GeneralUsageTests.cs | 2 +- osu.Game.Tests/Database/RealmLiveTests.cs | 2 +- .../RealmSubscriptionRegistrationTests.cs | 4 ++-- osu.Game/Database/RealmContextFactory.cs | 18 +++++++++--------- osu.Game/Database/RealmObjectExtensions.cs | 2 +- .../Bindings/DatabasedKeyBindingContainer.cs | 2 +- osu.Game/Online/BeatmapDownloadTracker.cs | 2 +- .../OnlinePlayBeatmapAvailabilityTracker.cs | 2 +- osu.Game/Online/ScoreDownloadTracker.cs | 2 +- osu.Game/Overlays/MusicController.cs | 2 +- .../Overlays/Settings/Sections/SkinSection.cs | 2 +- osu.Game/Screens/Select/BeatmapCarousel.cs | 8 ++++---- .../Screens/Select/Carousel/TopLocalRank.cs | 2 +- .../Select/Leaderboards/BeatmapLeaderboard.cs | 2 +- osu.Game/Screens/Spectate/SpectatorScreen.cs | 2 +- 15 files changed, 27 insertions(+), 27 deletions(-) diff --git a/osu.Game.Tests/Database/GeneralUsageTests.cs b/osu.Game.Tests/Database/GeneralUsageTests.cs index c82c1b6e59..3c62153d9e 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.Register(realm => + realmFactory.RegisterCustomSubscription(realm => { var subscription = realm.All().QueryAsyncWithNotifications((sender, changes, error) => { diff --git a/osu.Game.Tests/Database/RealmLiveTests.cs b/osu.Game.Tests/Database/RealmLiveTests.cs index 0e6ad910d9..d53fcb9ac7 100644 --- a/osu.Game.Tests/Database/RealmLiveTests.cs +++ b/osu.Game.Tests/Database/RealmLiveTests.cs @@ -239,7 +239,7 @@ namespace osu.Game.Tests.Database { int changesTriggered = 0; - realmFactory.Register(outerRealm => + realmFactory.RegisterCustomSubscription(outerRealm => { outerRealm.All().QueryAsyncWithNotifications(gotChange); ILive? liveBeatmap = null; diff --git a/osu.Game.Tests/Database/RealmSubscriptionRegistrationTests.cs b/osu.Game.Tests/Database/RealmSubscriptionRegistrationTests.cs index 16e9f31c7b..1799b95905 100644 --- a/osu.Game.Tests/Database/RealmSubscriptionRegistrationTests.cs +++ b/osu.Game.Tests/Database/RealmSubscriptionRegistrationTests.cs @@ -28,7 +28,7 @@ namespace osu.Game.Tests.Database { realmFactory.Write(realm => realm.Add(TestResources.CreateTestBeatmapSetInfo())); - var registration = realmFactory.Register(realm => realm.All(), onChanged); + var registration = realmFactory.RegisterForNotifications(realm => realm.All(), onChanged); testEventsArriving(true); @@ -104,7 +104,7 @@ namespace osu.Game.Tests.Database realmFactory.Write(realm => realm.Add(TestResources.CreateTestBeatmapSetInfo())); - var subscription = realmFactory.Register(realm => + var subscription = realmFactory.RegisterCustomSubscription(realm => { beatmapSetInfo = realm.All().First(); diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index 522a5fdfd9..697caf8cd3 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -63,6 +63,10 @@ namespace osu.Game.Database private readonly ThreadLocal currentThreadCanCreateContexts = new ThreadLocal(); + private readonly Dictionary, IDisposable?> customSubscriptionActions = new Dictionary, IDisposable?>(); + + private readonly Dictionary, Action> realmSubscriptionsResetMap = new Dictionary, Action>(); + private static readonly GlobalStatistic contexts_created = GlobalStatistics.Get(@"Realm", @"Contexts (Created)"); private readonly object contextLock = new object(); @@ -233,20 +237,16 @@ namespace osu.Game.Database } } - private readonly Dictionary, IDisposable?> customSubscriptionActions = new Dictionary, IDisposable?>(); - - private readonly Dictionary, Action> realmSubscriptionsResetMap = new Dictionary, Action>(); - - public IDisposable Register(Func> query, NotificationCallbackDelegate onChanged) + public IDisposable RegisterForNotifications(Func> query, NotificationCallbackDelegate onChanged) where T : RealmObjectBase { if (!ThreadSafety.IsUpdateThread) - throw new InvalidOperationException(@$"{nameof(Register)} must be called from the update thread."); + throw new InvalidOperationException(@$"{nameof(RegisterForNotifications)} must be called from the update thread."); lock (contextLock) { realmSubscriptionsResetMap.Add(action, () => onChanged(new EmptyRealmSet(), null, null)); - return Register(action); + return RegisterCustomSubscription(action); } IDisposable? action(Realm realm) => query(realm).QueryAsyncWithNotifications(onChanged); @@ -257,10 +257,10 @@ namespace osu.Game.Database /// /// The work to run. Return value should be an from QueryAsyncWithNotifications, or an to clean up any bindings. /// An which should be disposed to unsubscribe any inner subscription. - public IDisposable Register(Func action) + public IDisposable RegisterCustomSubscription(Func action) { if (!ThreadSafety.IsUpdateThread) - throw new InvalidOperationException(@$"{nameof(Register)} must be called from the update thread."); + throw new InvalidOperationException(@$"{nameof(RegisterForNotifications)} must be called from the update thread."); var syncContext = SynchronizationContext.Current; diff --git a/osu.Game/Database/RealmObjectExtensions.cs b/osu.Game/Database/RealmObjectExtensions.cs index c30e1699b9..d9026d165d 100644 --- a/osu.Game/Database/RealmObjectExtensions.cs +++ b/osu.Game/Database/RealmObjectExtensions.cs @@ -272,7 +272,7 @@ namespace osu.Game.Database where T : RealmObjectBase { if (!RealmContextFactory.CurrentThreadSubscriptionsAllowed) - throw new InvalidOperationException($"Make sure to call {nameof(RealmContextFactory)}.{nameof(RealmContextFactory.Register)}"); + throw new InvalidOperationException($"Make sure to call {nameof(RealmContextFactory)}.{nameof(RealmContextFactory.RegisterForNotifications)}"); return collection.SubscribeForNotifications(callback); } diff --git a/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs b/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs index 4ad5693867..9cd9865441 100644 --- a/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs +++ b/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs @@ -55,7 +55,7 @@ namespace osu.Game.Input.Bindings protected override void LoadComplete() { - realmSubscription = realmFactory.Register(realm => queryRealmKeyBindings(), (sender, changes, error) => + realmSubscription = realmFactory.RegisterForNotifications(realm => queryRealmKeyBindings(), (sender, changes, error) => { // 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. diff --git a/osu.Game/Online/BeatmapDownloadTracker.cs b/osu.Game/Online/BeatmapDownloadTracker.cs index 70599a167b..3f45afb6b2 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.Register(realm => realm.All().Where(s => s.OnlineID == TrackedItem.OnlineID && !s.DeletePending), (items, changes, ___) => + realmSubscription = realmContextFactory.RegisterForNotifications(realm => realm.All().Where(s => s.OnlineID == TrackedItem.OnlineID && !s.DeletePending), (items, changes, ___) => { if (items.Any()) Schedule(() => UpdateState(DownloadState.LocallyAvailable)); diff --git a/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs b/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs index 8dd28f5417..28c41bec33 100644 --- a/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs +++ b/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs @@ -78,7 +78,7 @@ 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 = realmContextFactory.Register(realm => filteredBeatmaps(), (items, changes, ___) => + realmSubscription = realmContextFactory.RegisterForNotifications(realm => filteredBeatmaps(), (items, changes, ___) => { if (changes == null) return; diff --git a/osu.Game/Online/ScoreDownloadTracker.cs b/osu.Game/Online/ScoreDownloadTracker.cs index 72e95bd6df..64ceee9fe6 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.Register(realm => realm.All().Where(s => ((s.OnlineID > 0 && s.OnlineID == TrackedItem.OnlineID) || s.Hash == TrackedItem.Hash) && !s.DeletePending), (items, changes, ___) => + realmSubscription = realmContextFactory.RegisterForNotifications(realm => realm.All().Where(s => ((s.OnlineID > 0 && s.OnlineID == TrackedItem.OnlineID) || s.Hash == TrackedItem.Hash) && !s.DeletePending), (items, changes, ___) => { if (items.Any()) Schedule(() => UpdateState(DownloadState.LocallyAvailable)); diff --git a/osu.Game/Overlays/MusicController.cs b/osu.Game/Overlays/MusicController.cs index 8caa69e78c..8df7ed9736 100644 --- a/osu.Game/Overlays/MusicController.cs +++ b/osu.Game/Overlays/MusicController.cs @@ -94,7 +94,7 @@ namespace osu.Game.Overlays foreach (var s in queryRealmBeatmapSets()) beatmapSets.Add(s.Detach()); - beatmapSubscription = realmFactory.Register(realm => queryRealmBeatmapSets(), beatmapsChanged); + beatmapSubscription = realmFactory.RegisterForNotifications(realm => queryRealmBeatmapSets(), beatmapsChanged); } private void beatmapsChanged(IRealmCollection sender, ChangeSet changes, Exception error) diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index c767edec71..6e5cd0da2d 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -83,7 +83,7 @@ namespace osu.Game.Overlays.Settings.Sections skinDropdown.Current = dropdownBindable; - realmSubscription = realmFactory.Register(realm => queryRealmSkins(), (sender, changes, error) => + realmSubscription = realmFactory.RegisterForNotifications(realm => queryRealmSkins(), (sender, changes, error) => { // 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. diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 889a6f5b79..5a6295fe74 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 = realmFactory.Register(getBeatmapSets, beatmapSetsChanged); - subscriptionBeatmaps = realmFactory.Register(realm => realm.All().Where(b => !b.Hidden), beatmapsChanged); + subscriptionSets = realmFactory.RegisterForNotifications(getBeatmapSets, beatmapSetsChanged); + subscriptionBeatmaps = realmFactory.RegisterForNotifications(realm => realm.All().Where(b => !b.Hidden), 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.Register(realm => realm.All().Where(s => s.DeletePending && !s.Protected), deletedBeatmapSetsChanged); - subscriptionHiddenBeatmaps = realmFactory.Register(realm => realm.All().Where(b => b.Hidden), beatmapsChanged); + subscriptionDeletedSets = realmFactory.RegisterForNotifications(realm => realm.All().Where(s => s.DeletePending && !s.Protected), deletedBeatmapSetsChanged); + subscriptionHiddenBeatmaps = realmFactory.RegisterForNotifications(realm => realm.All().Where(b => b.Hidden), beatmapsChanged); } private void deletedBeatmapSetsChanged(IRealmCollection sender, ChangeSet changes, Exception error) diff --git a/osu.Game/Screens/Select/Carousel/TopLocalRank.cs b/osu.Game/Screens/Select/Carousel/TopLocalRank.cs index fc2188e597..c03d464ef6 100644 --- a/osu.Game/Screens/Select/Carousel/TopLocalRank.cs +++ b/osu.Game/Screens/Select/Carousel/TopLocalRank.cs @@ -48,7 +48,7 @@ namespace osu.Game.Screens.Select.Carousel ruleset.BindValueChanged(_ => { scoreSubscription?.Dispose(); - scoreSubscription = realmFactory.Register(realm => + scoreSubscription = realmFactory.RegisterForNotifications(realm => realm.All() .Filter($"{nameof(ScoreInfo.User)}.{nameof(RealmUser.OnlineID)} == $0" + $" && {nameof(ScoreInfo.BeatmapInfo)}.{nameof(BeatmapInfo.ID)} == $1" diff --git a/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs b/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs index 463f878e2a..5f288b972b 100644 --- a/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs +++ b/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs @@ -113,7 +113,7 @@ namespace osu.Game.Screens.Select.Leaderboards if (beatmapInfo == null) return; - scoreSubscription = realmFactory.Register(realm => + scoreSubscription = realmFactory.RegisterForNotifications(realm => realm.All() .Filter($"{nameof(ScoreInfo.BeatmapInfo)}.{nameof(BeatmapInfo.ID)} = $0", beatmapInfo.ID), (_, changes, ___) => diff --git a/osu.Game/Screens/Spectate/SpectatorScreen.cs b/osu.Game/Screens/Spectate/SpectatorScreen.cs index 904648f727..4abde56ea6 100644 --- a/osu.Game/Screens/Spectate/SpectatorScreen.cs +++ b/osu.Game/Screens/Spectate/SpectatorScreen.cs @@ -80,7 +80,7 @@ namespace osu.Game.Screens.Spectate playingUserStates.BindTo(spectatorClient.PlayingUserStates); playingUserStates.BindCollectionChanged(onPlayingUserStatesChanged, true); - realmSubscription = realmContextFactory.Register( + realmSubscription = realmContextFactory.RegisterForNotifications( realm => realm.All().Where(s => !s.DeletePending), beatmapsChanged); foreach ((int id, var _) in userMap) From cb319cebdbae8cf70e4b750b1bd80ce655e41183 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 24 Jan 2022 14:48:55 +0900 Subject: [PATCH 36/46] Refactor naming and add more comments to help understanding in `RealmContextFactory` subscription logic --- osu.Game/Database/RealmContextFactory.cs | 39 ++++++++++++++++-------- 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index 697caf8cd3..0137e22e94 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -63,8 +63,22 @@ namespace osu.Game.Database private readonly ThreadLocal currentThreadCanCreateContexts = new ThreadLocal(); - private readonly Dictionary, IDisposable?> customSubscriptionActions = new Dictionary, IDisposable?>(); + /// + /// Holds a map of functions registered via and and a coinciding action which when triggered, + /// will unregister the subscription from realm. + /// + /// Put another way, the key is an action which registers the subscription with realm. The returned from the action is stored as the value and only + /// used internally. + /// + /// Entries in this dictionary are only removed when a consumer signals that the subscription should be permanently ceased (via their own ). + /// + private readonly Dictionary, IDisposable?> customSubscriptionsResetMap = new Dictionary, IDisposable?>(); + /// + /// Holds a map of functions registered via and a coinciding action which when triggered, + /// fires a change set event with an empty collection. This is used to inform subscribers when a realm context goes away, and ensure they don't use invalidated + /// managed realm objects from a previous firing. + /// private readonly Dictionary, Action> realmSubscriptionsResetMap = new Dictionary, Action>(); private static readonly GlobalStatistic contexts_created = GlobalStatistics.Get(@"Realm", @"Contexts (Created)"); @@ -88,7 +102,7 @@ namespace osu.Game.Database Logger.Log(@$"Opened realm ""{context.Config.DatabasePath}"" at version {context.Config.SchemaVersion}"); // Resubscribe any subscriptions - foreach (var action in customSubscriptionActions.Keys) + foreach (var action in customSubscriptionsResetMap.Keys) registerSubscription(action); } @@ -245,11 +259,12 @@ namespace osu.Game.Database lock (contextLock) { + Func action = realm => query(realm).QueryAsyncWithNotifications(onChanged); + + // Store an action which is used when blocking to ensure consumers don't use results of a stale changeset firing. realmSubscriptionsResetMap.Add(action, () => onChanged(new EmptyRealmSet(), null, null)); return RegisterCustomSubscription(action); } - - IDisposable? action(Realm realm) => query(realm).QueryAsyncWithNotifications(onChanged); } /// @@ -266,8 +281,8 @@ namespace osu.Game.Database registerSubscription(action); - // This token is returned to the consumer only. - // It will cause the registration to be permanently removed. + // This token is returned to the consumer. + // When disposed, it will cause the registration to be permanently ceased (unsubscribed with realm and unregistered by this class). return new InvokeOnDisposal(() => { if (ThreadSafety.IsUpdateThread) @@ -279,10 +294,10 @@ namespace osu.Game.Database { lock (contextLock) { - if (customSubscriptionActions.TryGetValue(action, out var unsubscriptionAction)) + if (customSubscriptionsResetMap.TryGetValue(action, out var unsubscriptionAction)) { unsubscriptionAction?.Dispose(); - customSubscriptionActions.Remove(action); + customSubscriptionsResetMap.Remove(action); realmSubscriptionsResetMap.Remove(action); } } @@ -301,10 +316,10 @@ namespace osu.Game.Database lock (contextLock) { - Debug.Assert(!customSubscriptionActions.TryGetValue(action, out var found) || found == null); + Debug.Assert(!customSubscriptionsResetMap.TryGetValue(action, out var found) || found == null); current_thread_subscriptions_allowed.Value = true; - customSubscriptionActions[action] = action(realm); + customSubscriptionsResetMap[action] = action(realm); current_thread_subscriptions_allowed.Value = false; } } @@ -318,10 +333,10 @@ namespace osu.Game.Database foreach (var action in realmSubscriptionsResetMap.Values) action(); - foreach (var action in customSubscriptionActions) + foreach (var action in customSubscriptionsResetMap) { action.Value?.Dispose(); - customSubscriptionActions[action.Key] = null; + customSubscriptionsResetMap[action.Key] = null; } } From 1e483ece322f4c78c4ff8eab95c1993d388a1f9c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 24 Jan 2022 16:40:16 +0900 Subject: [PATCH 37/46] Avoid adding "exit all screens" step when running tests interactively --- osu.Game/Tests/Visual/ScreenTestScene.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/osu.Game/Tests/Visual/ScreenTestScene.cs b/osu.Game/Tests/Visual/ScreenTestScene.cs index c44a848275..b6f6ca6daa 100644 --- a/osu.Game/Tests/Visual/ScreenTestScene.cs +++ b/osu.Game/Tests/Visual/ScreenTestScene.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using osu.Framework.Allocation; +using osu.Framework.Development; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Logging; @@ -48,7 +49,11 @@ namespace osu.Game.Tests.Visual public virtual void SetUpSteps() => addExitAllScreensStep(); [TearDownSteps] - public virtual void TearDownSteps() => addExitAllScreensStep(); + public virtual void TearDownSteps() + { + if (DebugUtils.IsNUnitRunning) + addExitAllScreensStep(); + } private void addExitAllScreensStep() { From e22aea0613698af3fa5870fd745764306a9273dd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 24 Jan 2022 17:05:49 +0900 Subject: [PATCH 38/46] Apply same fix to `OsuGameTestScene` --- osu.Game/Tests/Visual/OsuGameTestScene.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/osu.Game/Tests/Visual/OsuGameTestScene.cs b/osu.Game/Tests/Visual/OsuGameTestScene.cs index 6a11bd3fea..ebbd9bb5dd 100644 --- a/osu.Game/Tests/Visual/OsuGameTestScene.cs +++ b/osu.Game/Tests/Visual/OsuGameTestScene.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using osu.Framework.Allocation; using osu.Framework.Bindables; +using osu.Framework.Development; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Shapes; @@ -72,8 +73,11 @@ namespace osu.Game.Tests.Visual [TearDownSteps] public void TearDownSteps() { - AddStep("exit game", () => Game.Exit()); - AddUntilStep("wait for game exit", () => Game.Parent == null); + if (DebugUtils.IsNUnitRunning) + { + AddStep("exit game", () => Game.Exit()); + AddUntilStep("wait for game exit", () => Game.Parent == null); + } } protected void CreateGame() From 52cd906af6552ecb099d40454d10e2e266a4e92f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 24 Jan 2022 17:45:31 +0900 Subject: [PATCH 39/46] Move context retrieval inside lock --- osu.Game/Database/RealmContextFactory.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index 0137e22e94..be484329bc 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -309,13 +309,13 @@ namespace osu.Game.Database { Debug.Assert(ThreadSafety.IsUpdateThread); - // Retrieve context outside of flag update to ensure that the context is constructed, - // as attempting to access it inside the subscription if it's not constructed would lead to - // cyclic invocations of the subscription callback. - var realm = Context; - lock (contextLock) { + // Retrieve context outside of flag update to ensure that the context is constructed, + // as attempting to access it inside the subscription if it's not constructed would lead to + // cyclic invocations of the subscription callback. + var realm = Context; + Debug.Assert(!customSubscriptionsResetMap.TryGetValue(action, out var found) || found == null); current_thread_subscriptions_allowed.Value = true; From abf14f09820bb8f5859604135174f36f0e9e7cc5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 24 Jan 2022 17:46:53 +0900 Subject: [PATCH 40/46] Lock unregistration for sanity --- osu.Game/Database/RealmContextFactory.cs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index be484329bc..5dd4e88ccd 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -330,13 +330,16 @@ namespace osu.Game.Database /// private void unregisterAllSubscriptions() { - foreach (var action in realmSubscriptionsResetMap.Values) - action(); - - foreach (var action in customSubscriptionsResetMap) + lock (contextLock) { - action.Value?.Dispose(); - customSubscriptionsResetMap[action.Key] = null; + foreach (var action in realmSubscriptionsResetMap.Values) + action(); + + foreach (var action in customSubscriptionsResetMap) + { + action.Value?.Dispose(); + customSubscriptionsResetMap[action.Key] = null; + } } } From f4e7211ef1356f6598f4ad6038d896c771f39cbd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 24 Jan 2022 17:52:36 +0900 Subject: [PATCH 41/46] Add xmldoc for `RegisterForNotifications` --- osu.Game/Database/RealmContextFactory.cs | 27 +++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index 5dd4e88ccd..f8470b8c73 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -251,7 +251,28 @@ namespace osu.Game.Database } } - public IDisposable RegisterForNotifications(Func> query, NotificationCallbackDelegate onChanged) + /// + /// Subscribe to a realm collection and begin watching for asynchronous changes. + /// + /// + /// This adds osu! specific thread and managed state safety checks on top of . + /// + /// In addition to the documented realm behaviour, we have the additional requirement of handling subscriptions over potential context loss. + /// When this happens, callback events will be automatically fired: + /// - On context loss, a callback with an empty collection and null will be invoked. + /// - On context revival, a standard initial realm callback will arrive, with null and an up-to-date collection. + /// + /// The to observe for changes. + /// Type of the elements in the list. + /// + /// The callback to be invoked with the updated . + /// + /// A subscription token. It must be kept alive for as long as you want to receive change notifications. + /// To stop receiving notifications, call . + /// + /// May be null in the case the provided collection is not managed. + /// + public IDisposable RegisterForNotifications(Func> query, NotificationCallbackDelegate callback) where T : RealmObjectBase { if (!ThreadSafety.IsUpdateThread) @@ -259,10 +280,10 @@ namespace osu.Game.Database lock (contextLock) { - Func action = realm => query(realm).QueryAsyncWithNotifications(onChanged); + Func action = realm => query(realm).QueryAsyncWithNotifications(callback); // Store an action which is used when blocking to ensure consumers don't use results of a stale changeset firing. - realmSubscriptionsResetMap.Add(action, () => onChanged(new EmptyRealmSet(), null, null)); + realmSubscriptionsResetMap.Add(action, () => callback(new EmptyRealmSet(), null, null)); return RegisterCustomSubscription(action); } } From bf5bf8d1fd147f1a0e27756321512eaa0676e3d7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 24 Jan 2022 17:58:53 +0900 Subject: [PATCH 42/46] Rename dictionaries to match methods --- osu.Game/Database/RealmContextFactory.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index f8470b8c73..738d0a70a9 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -79,7 +79,7 @@ namespace osu.Game.Database /// fires a change set event with an empty collection. This is used to inform subscribers when a realm context goes away, and ensure they don't use invalidated /// managed realm objects from a previous firing. /// - private readonly Dictionary, Action> realmSubscriptionsResetMap = new Dictionary, Action>(); + private readonly Dictionary, Action> notificationsResetMap = new Dictionary, Action>(); private static readonly GlobalStatistic contexts_created = GlobalStatistics.Get(@"Realm", @"Contexts (Created)"); @@ -283,7 +283,7 @@ namespace osu.Game.Database Func action = realm => query(realm).QueryAsyncWithNotifications(callback); // Store an action which is used when blocking to ensure consumers don't use results of a stale changeset firing. - realmSubscriptionsResetMap.Add(action, () => callback(new EmptyRealmSet(), null, null)); + notificationsResetMap.Add(action, () => callback(new EmptyRealmSet(), null, null)); return RegisterCustomSubscription(action); } } @@ -319,7 +319,7 @@ namespace osu.Game.Database { unsubscriptionAction?.Dispose(); customSubscriptionsResetMap.Remove(action); - realmSubscriptionsResetMap.Remove(action); + notificationsResetMap.Remove(action); } } } @@ -353,7 +353,7 @@ namespace osu.Game.Database { lock (contextLock) { - foreach (var action in realmSubscriptionsResetMap.Values) + foreach (var action in notificationsResetMap.Values) action(); foreach (var action in customSubscriptionsResetMap) From e3083c2477ae80b23f5c17c9360c7aa51b07dc88 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 24 Jan 2022 18:05:30 +0900 Subject: [PATCH 43/46] Fix copy pasted xmldoc --- osu.Game/Database/RealmContextFactory.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index 738d0a70a9..3956738147 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -264,14 +264,12 @@ namespace osu.Game.Database /// /// The to observe for changes. /// Type of the elements in the list. - /// /// The callback to be invoked with the updated . /// /// A subscription token. It must be kept alive for as long as you want to receive change notifications. /// To stop receiving notifications, call . - /// - /// May be null in the case the provided collection is not managed. /// + /// public IDisposable RegisterForNotifications(Func> query, NotificationCallbackDelegate callback) where T : RealmObjectBase { From b0919722ac1c77fb484a7175e3f90cf325d03f9e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 24 Jan 2022 18:24:25 +0900 Subject: [PATCH 44/46] Guard against potential exception while blocking realm --- .../Sections/DebugSettings/MemorySettings.cs | 48 +++++++++++-------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/osu.Game/Overlays/Settings/Sections/DebugSettings/MemorySettings.cs b/osu.Game/Overlays/Settings/Sections/DebugSettings/MemorySettings.cs index f058c274e4..c5854981e6 100644 --- a/osu.Game/Overlays/Settings/Sections/DebugSettings/MemorySettings.cs +++ b/osu.Game/Overlays/Settings/Sections/DebugSettings/MemorySettings.cs @@ -1,11 +1,13 @@ // 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.Threading; using System.Threading.Tasks; using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Localisation; +using osu.Framework.Logging; using osu.Framework.Platform; using osu.Game.Database; using osu.Game.Localisation; @@ -52,30 +54,38 @@ namespace osu.Game.Overlays.Settings.Sections.DebugSettings blockAction.Action = () => { - var blocking = realmFactory.BlockAllOperations(); - blockAction.Enabled.Value = false; - - // As a safety measure, unblock after 10 seconds. - // This is to handle the case where a dev may block, but then something on the update thread - // accesses realm and blocks for eternity. - Task.Factory.StartNew(() => + try { - Thread.Sleep(10000); - unblock(); - }); + var token = realmFactory.BlockAllOperations(); - unblockAction.Action = unblock; + blockAction.Enabled.Value = false; - void unblock() - { - blocking?.Dispose(); - blocking = null; - - Scheduler.Add(() => + // As a safety measure, unblock after 10 seconds. + // This is to handle the case where a dev may block, but then something on the update thread + // accesses realm and blocks for eternity. + Task.Factory.StartNew(() => { - blockAction.Enabled.Value = true; - unblockAction.Action = null; + Thread.Sleep(10000); + unblock(); }); + + unblockAction.Action = unblock; + + void unblock() + { + token?.Dispose(); + token = null; + + Scheduler.Add(() => + { + blockAction.Enabled.Value = true; + unblockAction.Action = null; + }); + } + } + catch (Exception e) + { + Logger.Error(e, "Blocking realm failed"); } }; } From 9afa034296e8f06642c907d44ab3061c076c9815 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 24 Jan 2022 18:36:16 +0900 Subject: [PATCH 45/46] Fix attempt to revive update thread realm context from non-update thread --- osu.Game/Database/RealmContextFactory.cs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index 3956738147..778092c543 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -594,7 +594,7 @@ namespace osu.Game.Database if (isDisposed) throw new ObjectDisposedException(nameof(RealmContextFactory)); - SynchronizationContext syncContext; + SynchronizationContext? syncContext = null; try { @@ -602,10 +602,20 @@ namespace osu.Game.Database lock (contextLock) { - if (!ThreadSafety.IsUpdateThread && context != null) - throw new InvalidOperationException(@$"{nameof(BlockAllOperations)} must be called from the update thread."); + if (context == null) + { + // null context means the update thread has not yet retrieved its context. + // we don't need to worry about reviving the update context in this case, so don't bother with the SynchronizationContext. + Debug.Assert(!ThreadSafety.IsUpdateThread); + } + else + { + if (!ThreadSafety.IsUpdateThread) + throw new InvalidOperationException(@$"{nameof(BlockAllOperations)} must be called from the update thread."); + + syncContext = SynchronizationContext.Current; + } - syncContext = SynchronizationContext.Current; unregisterAllSubscriptions(); Logger.Log(@"Blocking realm operations.", LoggingTarget.Database); From 66c5d77d6388686c733eb229e822d0ad4120dee1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 24 Jan 2022 18:55:15 +0900 Subject: [PATCH 46/46] Allow realm migration to run again if interrupted halfway --- osu.Game/Database/EFToRealmMigrator.cs | 258 ++++++++++++------------- osu.Game/Screens/Loader.cs | 15 +- 2 files changed, 140 insertions(+), 133 deletions(-) diff --git a/osu.Game/Database/EFToRealmMigrator.cs b/osu.Game/Database/EFToRealmMigrator.cs index bbbdac352e..edce99e302 100644 --- a/osu.Game/Database/EFToRealmMigrator.cs +++ b/osu.Game/Database/EFToRealmMigrator.cs @@ -27,7 +27,9 @@ namespace osu.Game.Database { internal class EFToRealmMigrator : CompositeDrawable { - public bool FinishedMigrating { get; private set; } + public Task MigrationCompleted => migrationCompleted.Task; + + private readonly TaskCompletionSource migrationCompleted = new TaskCompletionSource(); [Resolved] private DatabaseContextFactory efContextFactory { get; set; } = null!; @@ -99,6 +101,17 @@ namespace osu.Game.Database { using (var ef = efContextFactory.Get()) { + realmContextFactory.Write(realm => + { + // Before beginning, ensure realm is in an empty state. + // Migrations which are half-completed could lead to issues if the user tries a second time. + // Note that we only do this for beatmaps and scores since the other migrations are yonks old. + realm.RemoveAll(); + realm.RemoveAll(); + realm.RemoveAll(); + realm.RemoveAll(); + }); + migrateSettings(ef); migrateSkins(ef); migrateBeatmaps(ef); @@ -114,7 +127,7 @@ namespace osu.Game.Database Logger.Log("Your development database has been fully migrated to realm. If you switch back to a pre-realm branch and need your previous database, rename the backup file back to \"client.db\".\n\nNote that doing this can potentially leave your file store in a bad state.", level: LogLevel.Important); }, TaskCreationOptions.LongRunning).ContinueWith(t => { - FinishedMigrating = true; + migrationCompleted.SetResult(true); }); } @@ -149,87 +162,78 @@ namespace osu.Game.Database { log($"Found {count} beatmaps in EF"); - // only migrate data if the realm database is empty. - // note that this cannot be written as: `realm.All().All(s => s.Protected)`, because realm does not support `.All()`. - if (realm.All().Any(s => !s.Protected)) - { - log("Skipping migration as realm already has beatmaps loaded"); - } - else - { - var transaction = realm.BeginWrite(); - int written = 0; + var transaction = realm.BeginWrite(); + int written = 0; - try + try + { + foreach (var beatmapSet in existingBeatmapSets) { - foreach (var beatmapSet in existingBeatmapSets) + if (++written % 1000 == 0) { - if (++written % 1000 == 0) - { - transaction.Commit(); - transaction = realm.BeginWrite(); - log($"Migrated {written}/{count} beatmaps..."); - } + transaction.Commit(); + transaction = realm.BeginWrite(); + log($"Migrated {written}/{count} beatmaps..."); + } - var realmBeatmapSet = new BeatmapSetInfo + var realmBeatmapSet = new BeatmapSetInfo + { + OnlineID = beatmapSet.OnlineID ?? -1, + DateAdded = beatmapSet.DateAdded, + Status = beatmapSet.Status, + DeletePending = beatmapSet.DeletePending, + Hash = beatmapSet.Hash, + Protected = beatmapSet.Protected, + }; + + migrateFiles(beatmapSet, realm, realmBeatmapSet); + + foreach (var beatmap in beatmapSet.Beatmaps) + { + var ruleset = realm.Find(beatmap.RulesetInfo.ShortName); + var metadata = getBestMetadata(beatmap.Metadata, beatmapSet.Metadata); + + var realmBeatmap = new BeatmapInfo(ruleset, new BeatmapDifficulty(beatmap.BaseDifficulty), metadata) { - OnlineID = beatmapSet.OnlineID ?? -1, - DateAdded = beatmapSet.DateAdded, - Status = beatmapSet.Status, - DeletePending = beatmapSet.DeletePending, - Hash = beatmapSet.Hash, - Protected = beatmapSet.Protected, + DifficultyName = beatmap.DifficultyName, + Status = beatmap.Status, + OnlineID = beatmap.OnlineID ?? -1, + Length = beatmap.Length, + BPM = beatmap.BPM, + Hash = beatmap.Hash, + StarRating = beatmap.StarRating, + MD5Hash = beatmap.MD5Hash, + Hidden = beatmap.Hidden, + AudioLeadIn = beatmap.AudioLeadIn, + StackLeniency = beatmap.StackLeniency, + SpecialStyle = beatmap.SpecialStyle, + LetterboxInBreaks = beatmap.LetterboxInBreaks, + WidescreenStoryboard = beatmap.WidescreenStoryboard, + EpilepsyWarning = beatmap.EpilepsyWarning, + SamplesMatchPlaybackRate = beatmap.SamplesMatchPlaybackRate, + DistanceSpacing = beatmap.DistanceSpacing, + BeatDivisor = beatmap.BeatDivisor, + GridSize = beatmap.GridSize, + TimelineZoom = beatmap.TimelineZoom, + Countdown = beatmap.Countdown, + CountdownOffset = beatmap.CountdownOffset, + MaxCombo = beatmap.MaxCombo, + Bookmarks = beatmap.Bookmarks, + BeatmapSet = realmBeatmapSet, }; - migrateFiles(beatmapSet, realm, realmBeatmapSet); - - foreach (var beatmap in beatmapSet.Beatmaps) - { - var ruleset = realm.Find(beatmap.RulesetInfo.ShortName); - var metadata = getBestMetadata(beatmap.Metadata, beatmapSet.Metadata); - - var realmBeatmap = new BeatmapInfo(ruleset, new BeatmapDifficulty(beatmap.BaseDifficulty), metadata) - { - DifficultyName = beatmap.DifficultyName, - Status = beatmap.Status, - OnlineID = beatmap.OnlineID ?? -1, - Length = beatmap.Length, - BPM = beatmap.BPM, - Hash = beatmap.Hash, - StarRating = beatmap.StarRating, - MD5Hash = beatmap.MD5Hash, - Hidden = beatmap.Hidden, - AudioLeadIn = beatmap.AudioLeadIn, - StackLeniency = beatmap.StackLeniency, - SpecialStyle = beatmap.SpecialStyle, - LetterboxInBreaks = beatmap.LetterboxInBreaks, - WidescreenStoryboard = beatmap.WidescreenStoryboard, - EpilepsyWarning = beatmap.EpilepsyWarning, - SamplesMatchPlaybackRate = beatmap.SamplesMatchPlaybackRate, - DistanceSpacing = beatmap.DistanceSpacing, - BeatDivisor = beatmap.BeatDivisor, - GridSize = beatmap.GridSize, - TimelineZoom = beatmap.TimelineZoom, - Countdown = beatmap.Countdown, - CountdownOffset = beatmap.CountdownOffset, - MaxCombo = beatmap.MaxCombo, - Bookmarks = beatmap.Bookmarks, - BeatmapSet = realmBeatmapSet, - }; - - realmBeatmapSet.Beatmaps.Add(realmBeatmap); - } - - realm.Add(realmBeatmapSet); + realmBeatmapSet.Beatmaps.Add(realmBeatmap); } - } - finally - { - transaction.Commit(); - } - log($"Successfully migrated {count} beatmaps to realm"); + realm.Add(realmBeatmapSet); + } } + finally + { + transaction.Commit(); + } + + log($"Successfully migrated {count} beatmaps to realm"); }); } @@ -280,70 +284,62 @@ namespace osu.Game.Database { log($"Found {count} scores in EF"); - // only migrate data if the realm database is empty. - if (realm.All().Any()) - { - log("Skipping migration as realm already has scores loaded"); - } - else - { - var transaction = realm.BeginWrite(); - int written = 0; + var transaction = realm.BeginWrite(); + int written = 0; - try + try + { + foreach (var score in existingScores) { - foreach (var score in existingScores) + if (++written % 1000 == 0) { - if (++written % 1000 == 0) - { - transaction.Commit(); - transaction = realm.BeginWrite(); - log($"Migrated {written}/{count} scores..."); - } - - var beatmap = realm.All().First(b => b.Hash == score.BeatmapInfo.Hash); - var ruleset = realm.Find(score.Ruleset.ShortName); - var user = new RealmUser - { - OnlineID = score.User.OnlineID, - Username = score.User.Username - }; - - var realmScore = new ScoreInfo(beatmap, ruleset, user) - { - Hash = score.Hash, - DeletePending = score.DeletePending, - OnlineID = score.OnlineID ?? -1, - ModsJson = score.ModsJson, - StatisticsJson = score.StatisticsJson, - TotalScore = score.TotalScore, - MaxCombo = score.MaxCombo, - Accuracy = score.Accuracy, - HasReplay = ((IScoreInfo)score).HasReplay, - Date = score.Date, - PP = score.PP, - Rank = score.Rank, - HitEvents = score.HitEvents, - Passed = score.Passed, - Combo = score.Combo, - Position = score.Position, - Statistics = score.Statistics, - Mods = score.Mods, - APIMods = score.APIMods, - }; - - migrateFiles(score, realm, realmScore); - - realm.Add(realmScore); + transaction.Commit(); + transaction = realm.BeginWrite(); + log($"Migrated {written}/{count} scores..."); } - } - finally - { - transaction.Commit(); - } - log($"Successfully migrated {count} scores to realm"); + var beatmap = realm.All().First(b => b.Hash == score.BeatmapInfo.Hash); + var ruleset = realm.Find(score.Ruleset.ShortName); + var user = new RealmUser + { + OnlineID = score.User.OnlineID, + Username = score.User.Username + }; + + var realmScore = new ScoreInfo(beatmap, ruleset, user) + { + Hash = score.Hash, + DeletePending = score.DeletePending, + OnlineID = score.OnlineID ?? -1, + ModsJson = score.ModsJson, + StatisticsJson = score.StatisticsJson, + TotalScore = score.TotalScore, + MaxCombo = score.MaxCombo, + Accuracy = score.Accuracy, + HasReplay = ((IScoreInfo)score).HasReplay, + Date = score.Date, + PP = score.PP, + Rank = score.Rank, + HitEvents = score.HitEvents, + Passed = score.Passed, + Combo = score.Combo, + Position = score.Position, + Statistics = score.Statistics, + Mods = score.Mods, + APIMods = score.APIMods, + }; + + migrateFiles(score, realm, realmScore); + + realm.Add(realmScore); + } } + finally + { + transaction.Commit(); + } + + log($"Successfully migrated {count} scores to realm"); }); } diff --git a/osu.Game/Screens/Loader.cs b/osu.Game/Screens/Loader.cs index 8c4a13f2bd..a72ba89dfa 100644 --- a/osu.Game/Screens/Loader.cs +++ b/osu.Game/Screens/Loader.cs @@ -74,11 +74,22 @@ namespace osu.Game.Screens base.OnEntering(last); LoadComponentAsync(precompiler = CreateShaderPrecompiler(), AddInternal); - LoadComponentAsync(loadableScreen = CreateLoadableScreen()); // A non-null context factory means there's still content to migrate. if (efContextFactory != null) + { LoadComponentAsync(realmMigrator = new EFToRealmMigrator(), AddInternal); + realmMigrator.MigrationCompleted.ContinueWith(_ => Schedule(() => + { + // Delay initial screen loading to ensure that the migration is in a complete and sane state + // before the intro screen may import the game intro beatmap. + LoadComponentAsync(loadableScreen = CreateLoadableScreen()); + })); + } + else + { + LoadComponentAsync(loadableScreen = CreateLoadableScreen()); + } LoadComponentAsync(spinner = new LoadingSpinner(true, true) { @@ -96,7 +107,7 @@ namespace osu.Game.Screens private void checkIfLoaded() { - if (loadableScreen.LoadState != LoadState.Ready || !precompiler.FinishedCompiling || realmMigrator?.FinishedMigrating == false) + if (loadableScreen?.LoadState != LoadState.Ready || !precompiler.FinishedCompiling) { Schedule(checkIfLoaded); return;