From 5e0763ae33c65105ef38a38cddc833abc921854d Mon Sep 17 00:00:00 2001 From: MK56 <74463310+mk56-spn@users.noreply.github.com> Date: Thu, 25 Nov 2021 10:24:00 +0100 Subject: [PATCH 01/65] Expand the functionality of Approach different --- osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs b/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs index d832411104..d3a01e5def 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs @@ -26,7 +26,7 @@ namespace osu.Game.Rulesets.Osu.Mods public BindableFloat Scale { get; } = new BindableFloat(4) { Precision = 0.1f, - MinValue = 2, + MinValue = 1.5f, MaxValue = 10, }; @@ -58,6 +58,9 @@ namespace osu.Game.Rulesets.Osu.Mods case AnimationStyle.Accelerate1: return Easing.In; + case AnimationStyle.Linear: + return Easing.None; + case AnimationStyle.Accelerate2: return Easing.InCubic; @@ -86,6 +89,7 @@ namespace osu.Game.Rulesets.Osu.Mods public enum AnimationStyle { + Linear, Gravity, InOut1, InOut2, From 5e56122d650f78c1a69e9890b608aa4d9021c446 Mon Sep 17 00:00:00 2001 From: MK-56 Date: Fri, 26 Nov 2021 09:48:38 +0100 Subject: [PATCH 02/65] Linear approach type moved per peppys request --- osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs b/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs index d3a01e5def..ae5ca0d679 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs @@ -89,7 +89,6 @@ namespace osu.Game.Rulesets.Osu.Mods public enum AnimationStyle { - Linear, Gravity, InOut1, InOut2, @@ -99,6 +98,7 @@ namespace osu.Game.Rulesets.Osu.Mods Decelerate1, Decelerate2, Decelerate3, + Linear, } } } From 0df97744ad8425e7b62dd8ea305a07310d921066 Mon Sep 17 00:00:00 2001 From: mk-56 Date: Sat, 27 Nov 2021 13:34:09 +0100 Subject: [PATCH 03/65] Moved linear to a better place. i decided to go with leaving it under gravity, its plenty visible and fine there, since the public enum list self orders I wasn't sure about how i could set a default that wasn't the topmost option --- osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs b/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs index ae5ca0d679..6aeb378c69 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs @@ -1,4 +1,4 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. using System; @@ -90,6 +90,7 @@ namespace osu.Game.Rulesets.Osu.Mods public enum AnimationStyle { Gravity, + Linear, InOut1, InOut2, Accelerate1, @@ -98,7 +99,6 @@ namespace osu.Game.Rulesets.Osu.Mods Decelerate1, Decelerate2, Decelerate3, - Linear, } } } From 1ae41118cd611d4965272a3272035f2c307acc7a Mon Sep 17 00:00:00 2001 From: mk-56 Date: Sat, 27 Nov 2021 22:59:09 +0100 Subject: [PATCH 04/65] Added gravity as a default. --- osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs b/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs index 6aeb378c69..db74790221 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs @@ -31,7 +31,7 @@ namespace osu.Game.Rulesets.Osu.Mods }; [SettingSource("Style", "Change the animation style of the approach circles.", 1)] - public Bindable Style { get; } = new Bindable(); + public Bindable Style { get; } = new Bindable(AnimationStyle.Gravity); public void ApplyToDrawableHitObject(DrawableHitObject drawable) { @@ -89,8 +89,8 @@ namespace osu.Game.Rulesets.Osu.Mods public enum AnimationStyle { - Gravity, Linear, + Gravity, InOut1, InOut2, Accelerate1, From 93b5aec23ec00b85b75a4aebcc22dfc40d68750d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 28 Nov 2021 15:00:40 +0100 Subject: [PATCH 05/65] Fix realm migration failures when upgrading from old versions * `RealmRulesetSetting` was added in 2021.916.0. * `RealmKeyBinding` was added in 2021.703.0. Attempting to upgrade from older releases than the above would cause migrations up to schema versions 10 and 11 to fail. --- osu.Game/Database/RealmContextFactory.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index 3c5dfeafe8..2bc77934a8 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -307,6 +307,9 @@ namespace osu.Game.Database case 10: string rulesetSettingClassName = getMappedOrOriginalName(typeof(RealmRulesetSetting)); + if (!migration.OldRealm.Schema.TryFindObjectSchema(rulesetSettingClassName, out _)) + return; + var oldSettings = migration.OldRealm.DynamicApi.All(rulesetSettingClassName); var newSettings = migration.NewRealm.All().ToList(); @@ -329,6 +332,9 @@ namespace osu.Game.Database case 11: string keyBindingClassName = getMappedOrOriginalName(typeof(RealmKeyBinding)); + if (!migration.OldRealm.Schema.TryFindObjectSchema(keyBindingClassName, out _)) + return; + var oldKeyBindings = migration.OldRealm.DynamicApi.All(keyBindingClassName); var newKeyBindings = migration.NewRealm.All().ToList(); From aba9ed624a408d7b0b28e45796d6dd1e096b977f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 28 Nov 2021 15:16:33 +0100 Subject: [PATCH 06/65] Remove unnecessary whitespace --- osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs b/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs index db74790221..d784320d22 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs @@ -90,7 +90,7 @@ namespace osu.Game.Rulesets.Osu.Mods public enum AnimationStyle { Linear, - Gravity, + Gravity, InOut1, InOut2, Accelerate1, From 5001412a0dada04c25400ebeb8676e5d927f1f9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 28 Nov 2021 15:19:12 +0100 Subject: [PATCH 07/65] Reorder easing mapping to match enum order and throw on unknown animation style --- .../Mods/OsuModApproachDifferent.cs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs b/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs index d784320d22..8b323eefa6 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModApproachDifferent.cs @@ -52,24 +52,27 @@ namespace osu.Game.Rulesets.Osu.Mods { switch (style) { - default: + case AnimationStyle.Linear: return Easing.None; + case AnimationStyle.Gravity: + return Easing.InBack; + + case AnimationStyle.InOut1: + return Easing.InOutCubic; + + case AnimationStyle.InOut2: + return Easing.InOutQuint; + case AnimationStyle.Accelerate1: return Easing.In; - case AnimationStyle.Linear: - return Easing.None; - case AnimationStyle.Accelerate2: return Easing.InCubic; case AnimationStyle.Accelerate3: return Easing.InQuint; - case AnimationStyle.Gravity: - return Easing.InBack; - case AnimationStyle.Decelerate1: return Easing.Out; @@ -79,11 +82,8 @@ namespace osu.Game.Rulesets.Osu.Mods case AnimationStyle.Decelerate3: return Easing.OutQuint; - case AnimationStyle.InOut1: - return Easing.InOutCubic; - - case AnimationStyle.InOut2: - return Easing.InOutQuint; + default: + throw new ArgumentOutOfRangeException(nameof(style), style, @"Unsupported animation style"); } } From 87d6a743dd9cd153d347a6257cb7086b69449b1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 28 Nov 2021 16:14:12 +0100 Subject: [PATCH 08/65] Migrate custom tournament client assets to uppercased directories It has transpired that on filename-case-sensitive filesystems, the tournament client does not consistently handle custom asset paths. Videos and mods could only be looked up from `videos` and `mods` directories (lowercase), while flags could only be looked up from the `Flags` directory (uppercase). A complicating circumstance is that default country flags, coming from osu-resources, also depend on the flag lookup being uppercased. To attempt to clean up the handling as much as it appears to be possible, automatically move user-supplied lowercase directories to uppercase. --- .../NonVisual/CustomTourneyDirectoryTest.cs | 12 ++++++------ .../Components/TournamentModIcon.cs | 2 +- osu.Game.Tournament/IO/TournamentStorage.cs | 17 +++++++++++++++++ .../IO/TournamentVideoResourceStore.cs | 2 +- .../Screens/TeamIntro/SeedingScreen.cs | 2 +- 5 files changed, 26 insertions(+), 9 deletions(-) diff --git a/osu.Game.Tournament.Tests/NonVisual/CustomTourneyDirectoryTest.cs b/osu.Game.Tournament.Tests/NonVisual/CustomTourneyDirectoryTest.cs index 94472a8bc7..d149ec145b 100644 --- a/osu.Game.Tournament.Tests/NonVisual/CustomTourneyDirectoryTest.cs +++ b/osu.Game.Tournament.Tests/NonVisual/CustomTourneyDirectoryTest.cs @@ -87,9 +87,9 @@ namespace osu.Game.Tournament.Tests.NonVisual // Recreate the old setup that uses "tournament" as the base path. string oldPath = Path.Combine(osuRoot, "tournament"); - string videosPath = Path.Combine(oldPath, "videos"); - string modsPath = Path.Combine(oldPath, "mods"); - string flagsPath = Path.Combine(oldPath, "flags"); + string videosPath = Path.Combine(oldPath, "Videos"); + string modsPath = Path.Combine(oldPath, "Mods"); + string flagsPath = Path.Combine(oldPath, "Flags"); Directory.CreateDirectory(videosPath); Directory.CreateDirectory(modsPath); @@ -123,9 +123,9 @@ namespace osu.Game.Tournament.Tests.NonVisual string migratedPath = Path.Combine(host.Storage.GetFullPath("."), "tournaments", "default"); - videosPath = Path.Combine(migratedPath, "videos"); - modsPath = Path.Combine(migratedPath, "mods"); - flagsPath = Path.Combine(migratedPath, "flags"); + videosPath = Path.Combine(migratedPath, "Videos"); + modsPath = Path.Combine(migratedPath, "Mods"); + flagsPath = Path.Combine(migratedPath, "Flags"); videoFile = Path.Combine(videosPath, "video.mp4"); modFile = Path.Combine(modsPath, "mod.png"); diff --git a/osu.Game.Tournament/Components/TournamentModIcon.cs b/osu.Game.Tournament/Components/TournamentModIcon.cs index 7c4e9c69a2..0fde263bc8 100644 --- a/osu.Game.Tournament/Components/TournamentModIcon.cs +++ b/osu.Game.Tournament/Components/TournamentModIcon.cs @@ -31,7 +31,7 @@ namespace osu.Game.Tournament.Components [BackgroundDependencyLoader] private void load(TextureStore textures, LadderInfo ladderInfo) { - var customTexture = textures.Get($"mods/{modAcronym}"); + var customTexture = textures.Get($"Mods/{modAcronym}"); if (customTexture != null) { diff --git a/osu.Game.Tournament/IO/TournamentStorage.cs b/osu.Game.Tournament/IO/TournamentStorage.cs index 044b60bbd5..02cf567837 100644 --- a/osu.Game.Tournament/IO/TournamentStorage.cs +++ b/osu.Game.Tournament/IO/TournamentStorage.cs @@ -51,6 +51,23 @@ namespace osu.Game.Tournament.IO Logger.Log("Changing tournament storage: " + GetFullPath(string.Empty)); } + protected override void ChangeTargetStorage(Storage newStorage) + { + // due to an unfortunate oversight, on OSes that are sensitive to pathname casing + // the custom flags directory needed to be named `Flags` (uppercase), + // while custom mods and videos directories needed to be named `mods` and `videos` respectively (lowercase). + // to unify handling to uppercase, move any non-compliant directories automatically for the user to migrate. + // can be removed 20220528 + if (newStorage.ExistsDirectory("flags")) + AttemptOperation(() => Directory.Move(newStorage.GetFullPath("flags"), newStorage.GetFullPath("Flags"))); + if (newStorage.ExistsDirectory("mods")) + AttemptOperation(() => Directory.Move(newStorage.GetFullPath("mods"), newStorage.GetFullPath("Mods"))); + if (newStorage.ExistsDirectory("videos")) + AttemptOperation(() => Directory.Move(newStorage.GetFullPath("videos"), newStorage.GetFullPath("Videos"))); + + base.ChangeTargetStorage(newStorage); + } + public IEnumerable ListTournaments() => AllTournaments.GetDirectories(string.Empty); public override void Migrate(Storage newStorage) diff --git a/osu.Game.Tournament/IO/TournamentVideoResourceStore.cs b/osu.Game.Tournament/IO/TournamentVideoResourceStore.cs index 4b26840b79..964d03220d 100644 --- a/osu.Game.Tournament/IO/TournamentVideoResourceStore.cs +++ b/osu.Game.Tournament/IO/TournamentVideoResourceStore.cs @@ -9,7 +9,7 @@ namespace osu.Game.Tournament.IO public class TournamentVideoResourceStore : NamespacedResourceStore { public TournamentVideoResourceStore(Storage storage) - : base(new StorageBackedResourceStore(storage), "videos") + : base(new StorageBackedResourceStore(storage), "Videos") { AddExtension("m4v"); AddExtension("avi"); diff --git a/osu.Game.Tournament/Screens/TeamIntro/SeedingScreen.cs b/osu.Game.Tournament/Screens/TeamIntro/SeedingScreen.cs index d34a8583b9..cd74a75b10 100644 --- a/osu.Game.Tournament/Screens/TeamIntro/SeedingScreen.cs +++ b/osu.Game.Tournament/Screens/TeamIntro/SeedingScreen.cs @@ -197,7 +197,7 @@ namespace osu.Game.Tournament.Screens.TeamIntro { row.Add(new Sprite { - Texture = textures.Get($"mods/{mods.ToLower()}"), + Texture = textures.Get($"Mods/{mods.ToLower()}"), Scale = new Vector2(0.5f) }); } From 21e8ab835769c5be5c66f16b241bdf6cb774b848 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 26 Nov 2021 14:40:09 +0900 Subject: [PATCH 09/65] Return `ILive` from `ToLive` calls instead of realm-typed instance --- osu.Game/Database/RealmObjectExtensions.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Database/RealmObjectExtensions.cs b/osu.Game/Database/RealmObjectExtensions.cs index 18a926fa8c..ac4ca436ad 100644 --- a/osu.Game/Database/RealmObjectExtensions.cs +++ b/osu.Game/Database/RealmObjectExtensions.cs @@ -49,13 +49,13 @@ namespace osu.Game.Database return mapper.Map(item); } - public static List> ToLive(this IEnumerable realmList) + public static List> ToLive(this IEnumerable realmList) where T : RealmObject, IHasGuidPrimaryKey { - return realmList.Select(l => new RealmLive(l)).ToList(); + return realmList.Select(l => new RealmLive(l)).Cast>().ToList(); } - public static RealmLive ToLive(this T realmObject) + public static ILive ToLive(this T realmObject) where T : RealmObject, IHasGuidPrimaryKey { return new RealmLive(realmObject); From 3bd4872520ca02a1ec1af4de5ad4fdf548731e9d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 26 Nov 2021 16:36:05 +0900 Subject: [PATCH 10/65] Add note about indexing support for `RealmNamedFileUsage.Filename` --- osu.Game/Models/RealmNamedFileUsage.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game/Models/RealmNamedFileUsage.cs b/osu.Game/Models/RealmNamedFileUsage.cs index ba12d51d0b..17e32510a8 100644 --- a/osu.Game/Models/RealmNamedFileUsage.cs +++ b/osu.Game/Models/RealmNamedFileUsage.cs @@ -16,6 +16,7 @@ namespace osu.Game.Models { public RealmFile File { get; set; } = null!; + // [Indexed] cannot be used on `EmbeddedObject`s as it only applies to top-level queries. May need to reconsider this if performance becomes a concern. public string Filename { get; set; } = null!; public RealmNamedFileUsage(RealmFile file, string filename) From e40e5096ea7a7d71e4070aa9e1e095b5b4cc1f7b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 26 Nov 2021 14:43:35 +0900 Subject: [PATCH 11/65] Remove `RealmLive` context re-fetch optimisation for now --- osu.Game/Database/RealmLive.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/osu.Game/Database/RealmLive.cs b/osu.Game/Database/RealmLive.cs index 73e6715aaa..dae4197309 100644 --- a/osu.Game/Database/RealmLive.cs +++ b/osu.Game/Database/RealmLive.cs @@ -114,7 +114,10 @@ namespace osu.Game.Database } } - private bool originalDataValid => !IsManaged || (isCorrectThread && data.IsValid); + // TODO: Revisit adding these conditionals back as an optimisation: || (isCorrectThread && data.IsValid); + // They have temporarily been removed due to an oversight involving .AsQueryable, see https://github.com/realm/realm-dotnet/discussions/2734. + // This means we are fetching a new context every `PerformRead` or `PerformWrite`, even when on the correct thread. + private bool originalDataValid => !IsManaged; // this matches realm's internal thread validation (see https://github.com/realm/realm-dotnet/blob/903b4d0b304f887e37e2d905384fb572a6496e70/Realm/Realm/Native/SynchronizationContextScheduler.cs#L72) private bool isCorrectThread From cb8fa803524d951d94dbb406322de050b304f9be Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 26 Nov 2021 21:33:11 +0900 Subject: [PATCH 12/65] Don't dispose fetched realm instance when using `RealmLive.Value` See https://github.com/realm/realm-dotnet/discussions/2734#discussioncomment-1705038 for reasoning. --- osu.Game/Database/RealmLive.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/osu.Game/Database/RealmLive.cs b/osu.Game/Database/RealmLive.cs index dae4197309..7ae7d8544a 100644 --- a/osu.Game/Database/RealmLive.cs +++ b/osu.Game/Database/RealmLive.cs @@ -102,10 +102,11 @@ namespace osu.Game.Database if (originalDataValid) return data; - T retrieved; + if (!isCorrectThread) + throw new InvalidOperationException($"Can't use {nameof(Value)} unless on the same thread the original data was fetched from."); - using (var realm = Realm.GetInstance(data.Realm.Config)) - retrieved = realm.Find(ID); + var realm = Realm.GetInstance(data.Realm.Config); + var retrieved = realm.Find(ID); if (!retrieved.IsValid) throw new InvalidOperationException("Attempted to access value without an open context"); From d73e81ee63f1bc385b76a2e52c4663d04776df44 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 14:48:28 +0900 Subject: [PATCH 13/65] Add test coverage of non-optimised existing model check --- .../Database/BeatmapImporterTests.cs | 41 ++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Database/BeatmapImporterTests.cs b/osu.Game.Tests/Database/BeatmapImporterTests.cs index 9ee237cfd9..d2193350ad 100644 --- a/osu.Game.Tests/Database/BeatmapImporterTests.cs +++ b/osu.Game.Tests/Database/BeatmapImporterTests.cs @@ -12,6 +12,7 @@ using NUnit.Framework; using osu.Framework.Extensions; using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Logging; +using osu.Framework.Platform; using osu.Game.Beatmaps; using osu.Game.Database; using osu.Game.Extensions; @@ -474,7 +475,7 @@ namespace osu.Game.Tests.Database } [Test] - public void TestImportThenDeleteThenImport() + public void TestImportThenDeleteThenImportOptimisedPath() { RunTestWithRealmAsync(async (realmFactory, storage) => { @@ -485,11 +486,39 @@ namespace osu.Game.Tests.Database deleteBeatmapSet(imported, realmFactory.Context); + Assert.IsTrue(imported.DeletePending); + var importedSecondTime = await LoadOszIntoStore(importer, realmFactory.Context); // check the newly "imported" beatmap is actually just the restored previous import. since it matches hash. Assert.IsTrue(imported.ID == importedSecondTime.ID); Assert.IsTrue(imported.Beatmaps.First().ID == importedSecondTime.Beatmaps.First().ID); + Assert.IsFalse(imported.DeletePending); + Assert.IsFalse(importedSecondTime.DeletePending); + }); + } + + [Test] + public void TestImportThenDeleteThenImportNonOptimisedPath() + { + RunTestWithRealmAsync(async (realmFactory, storage) => + { + using var importer = new NonOptimisedBeatmapImporter(realmFactory, storage); + using var store = new RealmRulesetStore(realmFactory, storage); + + var imported = await LoadOszIntoStore(importer, realmFactory.Context); + + deleteBeatmapSet(imported, realmFactory.Context); + + Assert.IsTrue(imported.DeletePending); + + var importedSecondTime = await LoadOszIntoStore(importer, realmFactory.Context); + + // check the newly "imported" beatmap is actually just the restored previous import. since it matches hash. + Assert.IsTrue(imported.ID == importedSecondTime.ID); + Assert.IsTrue(imported.Beatmaps.First().ID == importedSecondTime.Beatmaps.First().ID); + Assert.IsFalse(imported.DeletePending); + Assert.IsFalse(importedSecondTime.DeletePending); }); } @@ -867,5 +896,15 @@ namespace osu.Game.Tests.Database Assert.Fail(failureMessage); } + + public class NonOptimisedBeatmapImporter : BeatmapImporter + { + public NonOptimisedBeatmapImporter(RealmContextFactory realmFactory, Storage storage) + : base(realmFactory, storage) + { + } + + protected override bool HasCustomHashFunction => true; + } } } From 2ee08b5f4cb35e832dedbda3fc1c8b9876d629d4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 26 Nov 2021 19:05:34 +0900 Subject: [PATCH 14/65] Fix undelete on existing not committing transaction --- osu.Game/Stores/RealmArchiveModelImporter.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game/Stores/RealmArchiveModelImporter.cs b/osu.Game/Stores/RealmArchiveModelImporter.cs index b74670e722..56469184a9 100644 --- a/osu.Game/Stores/RealmArchiveModelImporter.cs +++ b/osu.Game/Stores/RealmArchiveModelImporter.cs @@ -387,7 +387,9 @@ namespace osu.Game.Stores if (CanReuseExisting(existing, item)) { LogForModel(item, @$"Found existing {HumanisedModelName} for {item} (ID {existing.ID}) – skipping import."); + existing.DeletePending = false; + transaction.Commit(); return existing.ToLive(); } From 791f7e380139b40389cde6d8927e4683dc25607e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 15:14:27 +0900 Subject: [PATCH 15/65] Update `RealmLive` tests in line with modified behaviour --- osu.Game.Tests/Database/RealmLiveTests.cs | 68 ++++++++++++++--------- 1 file changed, 42 insertions(+), 26 deletions(-) diff --git a/osu.Game.Tests/Database/RealmLiveTests.cs b/osu.Game.Tests/Database/RealmLiveTests.cs index f86761fdc8..63566394d6 100644 --- a/osu.Game.Tests/Database/RealmLiveTests.cs +++ b/osu.Game.Tests/Database/RealmLiveTests.cs @@ -66,19 +66,29 @@ namespace osu.Game.Tests.Database Task.Factory.StartNew(() => { - Assert.DoesNotThrow(() => + // TODO: The commented code is the behaviour we hope to obtain, but is temporarily disabled. + // See https://github.com/ppy/osu/pull/15851 + using (realmFactory.CreateContext()) { - using (realmFactory.CreateContext()) + Assert.Throws(() => { - var resolved = liveBeatmap.Value; + var __ = liveBeatmap.Value; + }); + } - Assert.IsTrue(resolved.Realm.IsClosed); - Assert.IsTrue(resolved.IsValid); - - // can access properties without a crash. - Assert.IsFalse(resolved.Hidden); - } - }); + // Assert.DoesNotThrow(() => + // { + // using (realmFactory.CreateContext()) + // { + // var resolved = liveBeatmap.Value; + // + // Assert.IsTrue(resolved.Realm.IsClosed); + // Assert.IsTrue(resolved.IsValid); + // + // // can access properties without a crash. + // Assert.IsFalse(resolved.Hidden); + // } + // }); }, TaskCreationOptions.LongRunning | TaskCreationOptions.HideScheduler).Wait(); }); } @@ -199,23 +209,29 @@ namespace osu.Game.Tests.Database Assert.AreEqual(0, updateThreadContext.All().Count()); Assert.AreEqual(0, changesTriggered); - var resolved = liveBeatmap.Value; - - // retrieval causes an implicit refresh. even changes that aren't related to the retrieval are fired at this point. - Assert.AreEqual(2, updateThreadContext.All().Count()); - Assert.AreEqual(1, changesTriggered); - - // even though the realm that this instance was resolved for was closed, it's still valid. - Assert.IsTrue(resolved.Realm.IsClosed); - Assert.IsTrue(resolved.IsValid); - - // can access properties without a crash. - Assert.IsFalse(resolved.Hidden); - - updateThreadContext.Write(r => + // TODO: Originally the following was using `liveBeatmap.Value`. This has been temporarily disabled. + // See https://github.com/ppy/osu/pull/15851 + liveBeatmap.PerformRead(resolved => { - // can use with the main context. - r.Remove(resolved); + // retrieval causes an implicit refresh. even changes that aren't related to the retrieval are fired at this point. + // ReSharper disable once AccessToDisposedClosure + Assert.AreEqual(2, updateThreadContext.All().Count()); + Assert.AreEqual(1, changesTriggered); + + // TODO: as above, temporarily disabled as it doesn't make sense inside a `PerformRead`. + // // even though the realm that this instance was resolved for was closed, it's still valid. + // Assert.IsTrue(resolved.Realm.IsClosed); + // Assert.IsTrue(resolved.IsValid); + + // can access properties without a crash. + Assert.IsFalse(resolved.Hidden); + + // ReSharper disable once AccessToDisposedClosure + updateThreadContext.Write(r => + { + // can use with the main context. + r.Remove(resolved); + }); }); } From 54798eabc9c34f83466bd42296236e4cec26c531 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 16:27:16 +0900 Subject: [PATCH 16/65] Add test coverage of potential deeadlock scenario with nested realm context fetching --- osu.Game.Tests/Database/GeneralUsageTests.cs | 34 ++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/osu.Game.Tests/Database/GeneralUsageTests.cs b/osu.Game.Tests/Database/GeneralUsageTests.cs index 3e8b6091fd..fec1a0dedd 100644 --- a/osu.Game.Tests/Database/GeneralUsageTests.cs +++ b/osu.Game.Tests/Database/GeneralUsageTests.cs @@ -5,6 +5,9 @@ using System; using System.Threading; using System.Threading.Tasks; using NUnit.Framework; +using osu.Game.Database; +using osu.Game.Models; +using Realms; #nullable enable @@ -33,6 +36,37 @@ namespace osu.Game.Tests.Database }); } + [Test] + public void TestNestedContextCreation() + { + RunTestWithRealm((realmFactory, _) => + { + var mainContext = realmFactory.Context; + bool callbackRan = false; + + var subscription = mainContext.All().SubscribeForNotifications((sender, changes, error) => + { + realmFactory.CreateContext(); + callbackRan = true; + }); + + Task.Factory.StartNew(() => + { + using (var threadContext = realmFactory.CreateContext()) + { + threadContext.Write(r => r.Add(new RealmBeatmap(CreateRuleset(), new RealmBeatmapDifficulty(), new RealmBeatmapMetadata()))); + } + }, TaskCreationOptions.LongRunning | TaskCreationOptions.HideScheduler).Wait(); + + // will create a context but also run the callback above (Refresh is implicitly run when getting a new context). + realmFactory.CreateContext(); + + Assert.IsTrue(callbackRan); + + subscription.Dispose(); + }); + } + [Test] public void TestBlockOperationsWithContention() { From c98451a7bf2a75af6dab4ed82818c80f72c994c8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 16:18:57 +0900 Subject: [PATCH 17/65] Fix potential deadlock on nested context creation requests --- osu.Game/Database/RealmContextFactory.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index 2bc77934a8..08d7580db7 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -52,6 +52,8 @@ namespace osu.Game.Database /// private readonly SemaphoreSlim contextCreationLock = new SemaphoreSlim(1); + private bool canCreateContexts; + private static readonly GlobalStatistic refreshes = GlobalStatistics.Get(@"Realm", @"Dirty Refreshes"); private static readonly GlobalStatistic contexts_created = GlobalStatistics.Get(@"Realm", @"Contexts (Created)"); @@ -153,7 +155,13 @@ namespace osu.Game.Database try { - contextCreationLock.Wait(); + if (!canCreateContexts) + contextCreationLock.Wait(); + + // the semaphore is used to stop all context creation. + // once the semaphore has been taken by this code section, it is safe to create further contexts. + // this can happen if a realm subscription is active and triggers a callback which has user code that calls `CreateContext`. + canCreateContexts = true; contexts_created.Value++; @@ -162,6 +170,7 @@ namespace osu.Game.Database finally { contextCreationLock.Release(); + canCreateContexts = false; } } From 071a8c670942c04a2566e493f278acc4778352b1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 17:34:56 +0900 Subject: [PATCH 18/65] Add test coverage showing `RealmLive` failure after an attach --- osu.Game.Tests/Database/RealmLiveTests.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/osu.Game.Tests/Database/RealmLiveTests.cs b/osu.Game.Tests/Database/RealmLiveTests.cs index f86761fdc8..a4b82fb8b1 100644 --- a/osu.Game.Tests/Database/RealmLiveTests.cs +++ b/osu.Game.Tests/Database/RealmLiveTests.cs @@ -29,6 +29,22 @@ namespace osu.Game.Tests.Database }); } + [Test] + public void TestAccessAfterAttach() + { + RunTestWithRealm((realmFactory, _) => + { + var beatmap = new RealmBeatmap(CreateRuleset(), new RealmBeatmapDifficulty(), new RealmBeatmapMetadata()); + + var liveBeatmap = beatmap.ToLive(); + + using (var context = realmFactory.CreateContext()) + context.Write(r => r.Add(beatmap)); + + Assert.IsFalse(liveBeatmap.PerformRead(l => l.Hidden)); + }); + } + [Test] public void TestAccessNonManaged() { From 6dcc244e2185ae77622c261b1a4c19c3cc901c8c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 17:21:51 +0900 Subject: [PATCH 19/65] Fix `RealmLive` not working is data is attached to realm post-`ToLive` call --- osu.Game/Database/RealmLive.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/osu.Game/Database/RealmLive.cs b/osu.Game/Database/RealmLive.cs index 73e6715aaa..5ee40f5b4d 100644 --- a/osu.Game/Database/RealmLive.cs +++ b/osu.Game/Database/RealmLive.cs @@ -17,7 +17,7 @@ namespace osu.Game.Database { public Guid ID { get; } - public bool IsManaged { get; } + public bool IsManaged => data.IsManaged; private readonly SynchronizationContext? fetchedContext; private readonly int fetchedThreadId; @@ -37,8 +37,6 @@ namespace osu.Game.Database if (data.IsManaged) { - IsManaged = true; - fetchedContext = SynchronizationContext.Current; fetchedThreadId = Thread.CurrentThread.ManagedThreadId; } From 673481ebc5eaece0e99f0bc61124655fd3944d4c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 17:38:42 +0900 Subject: [PATCH 20/65] Use `ThreadLocal` to avoid potential threading issues --- 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 08d7580db7..9e66248d78 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -52,7 +52,7 @@ namespace osu.Game.Database /// private readonly SemaphoreSlim contextCreationLock = new SemaphoreSlim(1); - private bool canCreateContexts; + private ThreadLocal currentThreadCanCreateContexts = new ThreadLocal(); private static readonly GlobalStatistic refreshes = GlobalStatistics.Get(@"Realm", @"Dirty Refreshes"); private static readonly GlobalStatistic contexts_created = GlobalStatistics.Get(@"Realm", @"Contexts (Created)"); @@ -155,13 +155,13 @@ namespace osu.Game.Database try { - if (!canCreateContexts) + if (!currentThreadCanCreateContexts.Value) contextCreationLock.Wait(); // the semaphore is used to stop all context creation. // once the semaphore has been taken by this code section, it is safe to create further contexts. // this can happen if a realm subscription is active and triggers a callback which has user code that calls `CreateContext`. - canCreateContexts = true; + currentThreadCanCreateContexts.Value = true; contexts_created.Value++; @@ -170,7 +170,7 @@ namespace osu.Game.Database finally { contextCreationLock.Release(); - canCreateContexts = false; + currentThreadCanCreateContexts.Value = false; } } From f19cfcc82e8decff5d63c1d4e098cfcb746e7b03 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 17:38:54 +0900 Subject: [PATCH 21/65] Make `readonly` --- osu.Game/Database/RealmContextFactory.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index 9e66248d78..b1e97a45d0 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -52,7 +52,7 @@ namespace osu.Game.Database /// private readonly SemaphoreSlim contextCreationLock = new SemaphoreSlim(1); - private ThreadLocal currentThreadCanCreateContexts = new ThreadLocal(); + private readonly ThreadLocal currentThreadCanCreateContexts = new ThreadLocal(); private static readonly GlobalStatistic refreshes = GlobalStatistics.Get(@"Realm", @"Dirty Refreshes"); private static readonly GlobalStatistic contexts_created = GlobalStatistics.Get(@"Realm", @"Contexts (Created)"); From a7e4e7be3adad487acf9e190ea0a6291396ca4e7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 18:08:02 +0900 Subject: [PATCH 22/65] Remove the ability to specify a filename in `ReplaceFile` --- osu.Game/Beatmaps/BeatmapManager.cs | 4 ++-- osu.Game/Database/ArchiveModelManager.cs | 5 ++--- osu.Game/Database/IModelFileManager.cs | 3 +-- osu.Game/Screens/Edit/Setup/ResourcesSection.cs | 11 +++++------ 4 files changed, 10 insertions(+), 13 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index d920b194b5..8502b91096 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -288,9 +288,9 @@ namespace osu.Game.Beatmaps #region Implementation of IModelFileManager - public void ReplaceFile(BeatmapSetInfo model, BeatmapSetFileInfo file, Stream contents, string filename = null) + public void ReplaceFile(BeatmapSetInfo model, BeatmapSetFileInfo file, Stream contents) { - beatmapModelManager.ReplaceFile(model, file, contents, filename); + beatmapModelManager.ReplaceFile(model, file, contents); } public void DeleteFile(BeatmapSetInfo model, BeatmapSetFileInfo file) diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index e73f4a7f6e..8cabb55cc3 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -453,13 +453,12 @@ namespace osu.Game.Database /// The item to operate on. /// The existing file to be replaced. /// The new file contents. - /// An optional filename for the new file. Will use the previous filename if not specified. - public void ReplaceFile(TModel model, TFileModel file, Stream contents, string filename = null) + public void ReplaceFile(TModel model, TFileModel file, Stream contents) { using (ContextFactory.GetForWrite()) { DeleteFile(model, file); - AddFile(model, contents, filename ?? file.Filename); + AddFile(model, contents, file.Filename); } } diff --git a/osu.Game/Database/IModelFileManager.cs b/osu.Game/Database/IModelFileManager.cs index c74b945eb7..4bc1e2d29b 100644 --- a/osu.Game/Database/IModelFileManager.cs +++ b/osu.Game/Database/IModelFileManager.cs @@ -15,8 +15,7 @@ namespace osu.Game.Database /// The item to operate on. /// The existing file to be replaced. /// The new file contents. - /// An optional filename for the new file. Will use the previous filename if not specified. - void ReplaceFile(TModel model, TFileModel file, Stream contents, string filename = null); + void ReplaceFile(TModel model, TFileModel file, Stream contents); /// /// Delete an existing file. diff --git a/osu.Game/Screens/Edit/Setup/ResourcesSection.cs b/osu.Game/Screens/Edit/Setup/ResourcesSection.cs index 8e739a786f..1e97218074 100644 --- a/osu.Game/Screens/Edit/Setup/ResourcesSection.cs +++ b/osu.Game/Screens/Edit/Setup/ResourcesSection.cs @@ -78,9 +78,9 @@ namespace osu.Game.Screens.Edit.Setup using (var stream = info.OpenRead()) { if (oldFile != null) - beatmaps.ReplaceFile(set, oldFile, stream, info.Name); - else - beatmaps.AddFile(set, stream, info.Name); + beatmaps.DeleteFile(set, oldFile); + + beatmaps.AddFile(set, stream, info.Name); } working.Value.Metadata.BackgroundFile = info.Name; @@ -105,9 +105,8 @@ namespace osu.Game.Screens.Edit.Setup using (var stream = info.OpenRead()) { if (oldFile != null) - beatmaps.ReplaceFile(set, oldFile, stream, info.Name); - else - beatmaps.AddFile(set, stream, info.Name); + beatmaps.DeleteFile(set, oldFile); + beatmaps.AddFile(set, stream, info.Name); } working.Value.Metadata.AudioFile = info.Name; From 6aad90674eeccdc505cc8b431ac0b39a27b5ab55 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 18:06:21 +0900 Subject: [PATCH 23/65] Remove `computeHashFast` usage in `RealmArchiveModelImporter.ComputeHash` --- osu.Game/Stores/RealmArchiveModelImporter.cs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/osu.Game/Stores/RealmArchiveModelImporter.cs b/osu.Game/Stores/RealmArchiveModelImporter.cs index 56469184a9..1681dad750 100644 --- a/osu.Game/Stores/RealmArchiveModelImporter.cs +++ b/osu.Game/Stores/RealmArchiveModelImporter.cs @@ -294,12 +294,8 @@ namespace osu.Game.Stores /// /// In the case of no matching files, a hash will be generated from the passed archive's . /// - protected virtual string ComputeHash(TModel item, ArchiveReader? reader = null) + protected virtual string ComputeHash(TModel item) { - if (reader != null) - // fast hashing for cases where the item's files may not be populated. - return computeHashFast(reader); - // for now, concatenate all hashable files in the set to create a unique hash. MemoryStream hashable = new MemoryStream(); @@ -374,7 +370,7 @@ namespace osu.Game.Stores // TODO: look into rollback of file additions (or delayed commit). item.Files.AddRange(createFileInfos(archive, Files, realm)); - item.Hash = ComputeHash(item, archive); + item.Hash = ComputeHash(item); // TODO: we may want to run this outside of the transaction. await Populate(item, archive, realm, cancellationToken).ConfigureAwait(false); From bac79663371dfdd4efa40c5f73192ce38efa782a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 18:00:28 +0900 Subject: [PATCH 24/65] Update `RealmLiveTests` to use interface type --- osu.Game.Tests/Database/RealmLiveTests.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/osu.Game.Tests/Database/RealmLiveTests.cs b/osu.Game.Tests/Database/RealmLiveTests.cs index f86761fdc8..d6eca80274 100644 --- a/osu.Game.Tests/Database/RealmLiveTests.cs +++ b/osu.Game.Tests/Database/RealmLiveTests.cs @@ -51,7 +51,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealm((realmFactory, _) => { - RealmLive? liveBeatmap = null; + ILive? liveBeatmap = null; Task.Factory.StartNew(() => { using (var threadContext = realmFactory.CreateContext()) @@ -88,7 +88,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealm((realmFactory, _) => { - RealmLive? liveBeatmap = null; + ILive? liveBeatmap = null; Task.Factory.StartNew(() => { using (var threadContext = realmFactory.CreateContext()) @@ -117,7 +117,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealm((realmFactory, _) => { - RealmLive? liveBeatmap = null; + ILive? liveBeatmap = null; Task.Factory.StartNew(() => { using (var threadContext = realmFactory.CreateContext()) @@ -143,7 +143,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealm((realmFactory, _) => { - RealmLive? liveBeatmap = null; + ILive? liveBeatmap = null; Task.Factory.StartNew(() => { using (var threadContext = realmFactory.CreateContext()) @@ -176,7 +176,7 @@ namespace osu.Game.Tests.Database using (var updateThreadContext = realmFactory.CreateContext()) { updateThreadContext.All().SubscribeForNotifications(gotChange); - RealmLive? liveBeatmap = null; + ILive? liveBeatmap = null; Task.Factory.StartNew(() => { From 94b10492be658d028bec1fc8768400c303357f51 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 18:20:07 +0900 Subject: [PATCH 25/65] Update legacy `SkinManager` to match new interface --- osu.Game/Skinning/SkinManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 8d07dd046a..26ff4457af 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -171,7 +171,7 @@ namespace osu.Game.Skinning var oldFile = skin.SkinInfo.Files.FirstOrDefault(f => f.Filename == filename); if (oldFile != null) - skinModelManager.ReplaceFile(skin.SkinInfo, oldFile, streamContent, oldFile.Filename); + skinModelManager.ReplaceFile(skin.SkinInfo, oldFile, streamContent); else skinModelManager.AddFile(skin.SkinInfo, streamContent, filename); } From 23fded4a3ac22be9eb0f92a38dc173402feed386 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 18:26:37 +0900 Subject: [PATCH 26/65] Fix potential oversight in semaphore release logic --- osu.Game/Database/RealmContextFactory.cs | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index b1e97a45d0..6948918fe7 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -153,15 +153,22 @@ namespace osu.Game.Database if (isDisposed) throw new ObjectDisposedException(nameof(RealmContextFactory)); + bool tookSemaphoreLock = false; + try { if (!currentThreadCanCreateContexts.Value) + { contextCreationLock.Wait(); - - // the semaphore is used to stop all context creation. - // once the semaphore has been taken by this code section, it is safe to create further contexts. - // this can happen if a realm subscription is active and triggers a callback which has user code that calls `CreateContext`. - currentThreadCanCreateContexts.Value = true; + tookSemaphoreLock = true; + } + else + { + // the semaphore is used to handle blocking of all context creation during certain periods. + // once the semaphore has been taken by this code section, it is safe to create further contexts on the same thread. + // this can happen if a realm subscription is active and triggers a callback which has user code that calls `CreateContext`. + currentThreadCanCreateContexts.Value = true; + } contexts_created.Value++; @@ -169,8 +176,11 @@ namespace osu.Game.Database } finally { - contextCreationLock.Release(); - currentThreadCanCreateContexts.Value = false; + if (tookSemaphoreLock) + { + contextCreationLock.Release(); + currentThreadCanCreateContexts.Value = false; + } } } From 0bcfb8e199004cb123598d9d5972d97ebf383cd2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 18:52:29 +0900 Subject: [PATCH 27/65] Fix regression in implementation of fix --- osu.Game/Database/RealmContextFactory.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index 6948918fe7..a20139e830 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -160,6 +160,7 @@ namespace osu.Game.Database if (!currentThreadCanCreateContexts.Value) { contextCreationLock.Wait(); + currentThreadCanCreateContexts.Value = true; tookSemaphoreLock = true; } else @@ -167,7 +168,6 @@ namespace osu.Game.Database // the semaphore is used to handle blocking of all context creation during certain periods. // once the semaphore has been taken by this code section, it is safe to create further contexts on the same thread. // this can happen if a realm subscription is active and triggers a callback which has user code that calls `CreateContext`. - currentThreadCanCreateContexts.Value = true; } contexts_created.Value++; From 566e10f8ccff29b3c0780832ec1811c13f1e4276 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 18:54:01 +0900 Subject: [PATCH 28/65] Refactor test to be easier to follow --- osu.Game.Tests/Database/GeneralUsageTests.cs | 36 +++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/osu.Game.Tests/Database/GeneralUsageTests.cs b/osu.Game.Tests/Database/GeneralUsageTests.cs index fec1a0dedd..d15e34f2da 100644 --- a/osu.Game.Tests/Database/GeneralUsageTests.cs +++ b/osu.Game.Tests/Database/GeneralUsageTests.cs @@ -36,34 +36,36 @@ namespace osu.Game.Tests.Database }); } + /// + /// Test to ensure that a `CreateContext` call nested inside a subscription doesn't cause any deadlocks + /// due to context fetching semaphores. + /// [Test] - public void TestNestedContextCreation() + public void TestNestedContextCreationWithSubscription() { RunTestWithRealm((realmFactory, _) => { - var mainContext = realmFactory.Context; bool callbackRan = false; - var subscription = mainContext.All().SubscribeForNotifications((sender, changes, error) => + using (var context = realmFactory.CreateContext()) { - realmFactory.CreateContext(); - callbackRan = true; - }); - - Task.Factory.StartNew(() => - { - using (var threadContext = realmFactory.CreateContext()) + var subscription = context.All().SubscribeForNotifications((sender, changes, error) => { - threadContext.Write(r => r.Add(new RealmBeatmap(CreateRuleset(), new RealmBeatmapDifficulty(), new RealmBeatmapMetadata()))); - } - }, TaskCreationOptions.LongRunning | TaskCreationOptions.HideScheduler).Wait(); + using (realmFactory.CreateContext()) + { + callbackRan = true; + } + }); - // will create a context but also run the callback above (Refresh is implicitly run when getting a new context). - realmFactory.CreateContext(); + // Force the callback above to run. + using (realmFactory.CreateContext()) + { + } + + subscription.Dispose(); + } Assert.IsTrue(callbackRan); - - subscription.Dispose(); }); } From 431ac1d97b951f988be2be3cbb87f300842f6457 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 29 Nov 2021 20:57:55 +0100 Subject: [PATCH 29/65] Remove unused using directive --- osu.Game.Tests/Database/GeneralUsageTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game.Tests/Database/GeneralUsageTests.cs b/osu.Game.Tests/Database/GeneralUsageTests.cs index d15e34f2da..841bf2de43 100644 --- a/osu.Game.Tests/Database/GeneralUsageTests.cs +++ b/osu.Game.Tests/Database/GeneralUsageTests.cs @@ -5,7 +5,6 @@ using System; using System.Threading; using System.Threading.Tasks; using NUnit.Framework; -using osu.Game.Database; using osu.Game.Models; using Realms; From 0fc4d6dc2a85636d8390a144dcb13ee5ecab9eaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 28 Nov 2021 21:43:56 +0100 Subject: [PATCH 30/65] Implement beatmap card difficulty list --- .../TestSceneBeatmapCardDifficultyList.cs | 71 ++++++++++++ .../Cards/BeatmapCardDifficultyList.cs | 103 ++++++++++++++++++ 2 files changed, 174 insertions(+) create mode 100644 osu.Game.Tests/Visual/Beatmaps/TestSceneBeatmapCardDifficultyList.cs create mode 100644 osu.Game/Beatmaps/Drawables/Cards/BeatmapCardDifficultyList.cs diff --git a/osu.Game.Tests/Visual/Beatmaps/TestSceneBeatmapCardDifficultyList.cs b/osu.Game.Tests/Visual/Beatmaps/TestSceneBeatmapCardDifficultyList.cs new file mode 100644 index 0000000000..aec75884d6 --- /dev/null +++ b/osu.Game.Tests/Visual/Beatmaps/TestSceneBeatmapCardDifficultyList.cs @@ -0,0 +1,71 @@ +// 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.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Shapes; +using osu.Game.Beatmaps.Drawables.Cards; +using osu.Game.Graphics; +using osu.Game.Online.API.Requests.Responses; +using osu.Game.Overlays; + +namespace osu.Game.Tests.Visual.Beatmaps +{ + public class TestSceneBeatmapCardDifficultyList : OsuTestScene + { + [Cached] + private OverlayColourProvider colourProvider = new OverlayColourProvider(OverlayColourScheme.Blue); + + [BackgroundDependencyLoader] + private void load(OsuColour colours) + { + var beatmapSet = new APIBeatmapSet + { + Beatmaps = new[] + { + new APIBeatmap { RulesetID = 1, StarRating = 5.76, DifficultyName = "Oni" }, + new APIBeatmap { RulesetID = 1, StarRating = 3.20, DifficultyName = "Muzukashii" }, + new APIBeatmap { RulesetID = 1, StarRating = 2.45, DifficultyName = "Futsuu" }, + + new APIBeatmap { RulesetID = 0, StarRating = 2.04, DifficultyName = "Normal" }, + new APIBeatmap { RulesetID = 0, StarRating = 3.51, DifficultyName = "Hard" }, + new APIBeatmap { RulesetID = 0, StarRating = 5.25, DifficultyName = "Insane" }, + + new APIBeatmap { RulesetID = 2, StarRating = 2.64, DifficultyName = "Salad" }, + new APIBeatmap { RulesetID = 2, StarRating = 3.56, DifficultyName = "Platter" }, + new APIBeatmap { RulesetID = 2, StarRating = 4.65, DifficultyName = "Rain" }, + + new APIBeatmap { RulesetID = 3, StarRating = 1.93, DifficultyName = "[7K] Normal" }, + new APIBeatmap { RulesetID = 3, StarRating = 3.18, DifficultyName = "[7K] Hyper" }, + new APIBeatmap { RulesetID = 3, StarRating = 4.82, DifficultyName = "[7K] Another" }, + + new APIBeatmap { RulesetID = 4, StarRating = 9.99, DifficultyName = "Unknown?!" }, + } + }; + + Child = new Container + { + Width = 300, + AutoSizeAxes = Axes.Y, + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Children = new Drawable[] + { + new Box + { + RelativeSizeAxes = Axes.Both, + Colour = colourProvider.Background2 + }, + new Container + { + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Padding = new MarginPadding(10), + Child = new BeatmapCardDifficultyList(beatmapSet) + } + } + }; + } + } +} diff --git a/osu.Game/Beatmaps/Drawables/Cards/BeatmapCardDifficultyList.cs b/osu.Game/Beatmaps/Drawables/Cards/BeatmapCardDifficultyList.cs new file mode 100644 index 0000000000..7753d8480a --- /dev/null +++ b/osu.Game/Beatmaps/Drawables/Cards/BeatmapCardDifficultyList.cs @@ -0,0 +1,103 @@ +// 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 osu.Framework.Allocation; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Sprites; +using osu.Game.Graphics; +using osu.Game.Graphics.Containers; +using osu.Game.Online.Chat; +using osu.Game.Rulesets; +using osuTK; + +namespace osu.Game.Beatmaps.Drawables.Cards +{ + public class BeatmapCardDifficultyList : CompositeDrawable + { + public BeatmapCardDifficultyList(IBeatmapSetInfo beatmapSetInfo) + { + RelativeSizeAxes = Axes.X; + AutoSizeAxes = Axes.Y; + + FillFlowContainer flow; + + InternalChild = flow = new FillFlowContainer + { + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Direction = FillDirection.Vertical, + Spacing = new Vector2(0, 3) + }; + + bool firstGroup = true; + + foreach (var group in beatmapSetInfo.Beatmaps.GroupBy(beatmap => beatmap.Ruleset.OnlineID).OrderBy(group => group.Key)) + { + if (!firstGroup) + { + flow.Add(Empty().With(s => + { + s.RelativeSizeAxes = Axes.X; + s.Height = 4; + })); + } + + foreach (var difficulty in group.OrderBy(b => b.StarRating)) + flow.Add(new BeatmapCardDifficultyRow(difficulty)); + + firstGroup = false; + } + } + + private class BeatmapCardDifficultyRow : CompositeDrawable + { + private readonly IBeatmapInfo beatmapInfo; + + public BeatmapCardDifficultyRow(IBeatmapInfo beatmapInfo) + { + this.beatmapInfo = beatmapInfo; + } + + [BackgroundDependencyLoader] + private void load(RulesetStore rulesets) + { + RelativeSizeAxes = Axes.X; + AutoSizeAxes = Axes.Y; + + InternalChild = new FillFlowContainer + { + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Direction = FillDirection.Horizontal, + Spacing = new Vector2(4, 0), + Children = new[] + { + (rulesets.GetRuleset(beatmapInfo.Ruleset.OnlineID)?.CreateInstance().CreateIcon() ?? new SpriteIcon { Icon = FontAwesome.Regular.QuestionCircle }).With(icon => + { + icon.Anchor = icon.Origin = Anchor.CentreLeft; + icon.Size = new Vector2(16); + }), + new StarRatingDisplay(new StarDifficulty(beatmapInfo.StarRating, 0), StarRatingDisplaySize.Small) + { + Anchor = Anchor.CentreLeft, + Origin = Anchor.CentreLeft + }, + new LinkFlowContainer(s => + { + s.Font = OsuFont.Default.With(size: 14, weight: FontWeight.SemiBold); + }).With(d => + { + d.AutoSizeAxes = Axes.Both; + d.Anchor = Anchor.CentreLeft; + d.Origin = Anchor.CentreLeft; + d.Padding = new MarginPadding { Bottom = 2 }; + d.AddLink(beatmapInfo.DifficultyName, LinkAction.OpenBeatmap, beatmapInfo.OnlineID.ToString()); + }) + } + }; + } + } + } +} From 45656c359936b935e17b8ef0ec4b6ada043cc85f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 29 Nov 2021 22:06:11 +0100 Subject: [PATCH 31/65] Fix difficulty spectrum display not ordering ruleset groups by ID --- osu.Game/Beatmaps/Drawables/DifficultySpectrumDisplay.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/Drawables/DifficultySpectrumDisplay.cs b/osu.Game/Beatmaps/Drawables/DifficultySpectrumDisplay.cs index f4501f0633..5b211084ab 100644 --- a/osu.Game/Beatmaps/Drawables/DifficultySpectrumDisplay.cs +++ b/osu.Game/Beatmaps/Drawables/DifficultySpectrumDisplay.cs @@ -62,7 +62,7 @@ namespace osu.Game.Beatmaps.Drawables // matching web: https://github.com/ppy/osu-web/blob/d06d8c5e735eb1f48799b1654b528e9a7afb0a35/resources/assets/lib/beatmapset-panel.tsx#L127 bool collapsed = beatmapSet.Beatmaps.Count() > 12; - foreach (var rulesetGrouping in beatmapSet.Beatmaps.GroupBy(beatmap => beatmap.Ruleset.OnlineID)) + foreach (var rulesetGrouping in beatmapSet.Beatmaps.GroupBy(beatmap => beatmap.Ruleset.OnlineID).OrderBy(group => group.Key)) { flow.Add(new RulesetDifficultyGroup(rulesetGrouping.Key, rulesetGrouping, collapsed)); } From 4a34a5c738fe5d0a765f85c1b167926bdc678952 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 30 Nov 2021 11:11:42 +0900 Subject: [PATCH 32/65] Refactor difficulty adjustment mod combinations test --- ...DifficultyAdjustmentModCombinationsTest.cs | 136 ++++++++++-------- 1 file changed, 75 insertions(+), 61 deletions(-) diff --git a/osu.Game.Tests/NonVisual/DifficultyAdjustmentModCombinationsTest.cs b/osu.Game.Tests/NonVisual/DifficultyAdjustmentModCombinationsTest.cs index e458e66ab7..612927337f 100644 --- a/osu.Game.Tests/NonVisual/DifficultyAdjustmentModCombinationsTest.cs +++ b/osu.Game.Tests/NonVisual/DifficultyAdjustmentModCombinationsTest.cs @@ -3,12 +3,14 @@ using System; using System.Collections.Generic; +using System.Linq; using NUnit.Framework; using osu.Game.Beatmaps; using osu.Game.Rulesets.Difficulty; using osu.Game.Rulesets.Difficulty.Preprocessing; using osu.Game.Rulesets.Difficulty.Skills; using osu.Game.Rulesets.Mods; +using osu.Game.Utils; namespace osu.Game.Tests.NonVisual { @@ -20,8 +22,10 @@ namespace osu.Game.Tests.NonVisual { var combinations = new TestLegacyDifficultyCalculator().CreateDifficultyAdjustmentModCombinations(); - Assert.AreEqual(1, combinations.Length); - Assert.IsTrue(combinations[0] is ModNoMod); + assertCombinations(new[] + { + new[] { typeof(ModNoMod) } + }, combinations); } [Test] @@ -29,9 +33,11 @@ namespace osu.Game.Tests.NonVisual { var combinations = new TestLegacyDifficultyCalculator(new ModA()).CreateDifficultyAdjustmentModCombinations(); - Assert.AreEqual(2, combinations.Length); - Assert.IsTrue(combinations[0] is ModNoMod); - Assert.IsTrue(combinations[1] is ModA); + assertCombinations(new[] + { + new[] { typeof(ModNoMod) }, + new[] { typeof(ModA) } + }, combinations); } [Test] @@ -39,14 +45,13 @@ namespace osu.Game.Tests.NonVisual { var combinations = new TestLegacyDifficultyCalculator(new ModA(), new ModB()).CreateDifficultyAdjustmentModCombinations(); - Assert.AreEqual(4, combinations.Length); - Assert.IsTrue(combinations[0] is ModNoMod); - Assert.IsTrue(combinations[1] is ModA); - Assert.IsTrue(combinations[2] is MultiMod); - Assert.IsTrue(combinations[3] is ModB); - - Assert.IsTrue(((MultiMod)combinations[2]).Mods[0] is ModA); - Assert.IsTrue(((MultiMod)combinations[2]).Mods[1] is ModB); + assertCombinations(new[] + { + new[] { typeof(ModNoMod) }, + new[] { typeof(ModA) }, + new[] { typeof(ModA), typeof(ModB) }, + new[] { typeof(ModB) } + }, combinations); } [Test] @@ -54,10 +59,12 @@ namespace osu.Game.Tests.NonVisual { var combinations = new TestLegacyDifficultyCalculator(new ModA(), new ModIncompatibleWithA()).CreateDifficultyAdjustmentModCombinations(); - Assert.AreEqual(3, combinations.Length); - Assert.IsTrue(combinations[0] is ModNoMod); - Assert.IsTrue(combinations[1] is ModA); - Assert.IsTrue(combinations[2] is ModIncompatibleWithA); + assertCombinations(new[] + { + new[] { typeof(ModNoMod) }, + new[] { typeof(ModA) }, + new[] { typeof(ModIncompatibleWithA) } + }, combinations); } [Test] @@ -65,22 +72,17 @@ namespace osu.Game.Tests.NonVisual { var combinations = new TestLegacyDifficultyCalculator(new ModA(), new ModB(), new ModIncompatibleWithA(), new ModIncompatibleWithAAndB()).CreateDifficultyAdjustmentModCombinations(); - Assert.AreEqual(8, combinations.Length); - Assert.IsTrue(combinations[0] is ModNoMod); - Assert.IsTrue(combinations[1] is ModA); - Assert.IsTrue(combinations[2] is MultiMod); - Assert.IsTrue(combinations[3] is ModB); - Assert.IsTrue(combinations[4] is MultiMod); - Assert.IsTrue(combinations[5] is ModIncompatibleWithA); - Assert.IsTrue(combinations[6] is MultiMod); - Assert.IsTrue(combinations[7] is ModIncompatibleWithAAndB); - - Assert.IsTrue(((MultiMod)combinations[2]).Mods[0] is ModA); - Assert.IsTrue(((MultiMod)combinations[2]).Mods[1] is ModB); - Assert.IsTrue(((MultiMod)combinations[4]).Mods[0] is ModB); - Assert.IsTrue(((MultiMod)combinations[4]).Mods[1] is ModIncompatibleWithA); - Assert.IsTrue(((MultiMod)combinations[6]).Mods[0] is ModIncompatibleWithA); - Assert.IsTrue(((MultiMod)combinations[6]).Mods[1] is ModIncompatibleWithAAndB); + assertCombinations(new[] + { + new[] { typeof(ModNoMod) }, + new[] { typeof(ModA) }, + new[] { typeof(ModA), typeof(ModB) }, + new[] { typeof(ModB) }, + new[] { typeof(ModB), typeof(ModIncompatibleWithA) }, + new[] { typeof(ModIncompatibleWithA) }, + new[] { typeof(ModIncompatibleWithA), typeof(ModIncompatibleWithAAndB) }, + new[] { typeof(ModIncompatibleWithAAndB) }, + }, combinations); } [Test] @@ -88,10 +90,12 @@ namespace osu.Game.Tests.NonVisual { var combinations = new TestLegacyDifficultyCalculator(new ModAofA(), new ModIncompatibleWithAofA()).CreateDifficultyAdjustmentModCombinations(); - Assert.AreEqual(3, combinations.Length); - Assert.IsTrue(combinations[0] is ModNoMod); - Assert.IsTrue(combinations[1] is ModAofA); - Assert.IsTrue(combinations[2] is ModIncompatibleWithAofA); + assertCombinations(new[] + { + new[] { typeof(ModNoMod) }, + new[] { typeof(ModAofA) }, + new[] { typeof(ModIncompatibleWithAofA) } + }, combinations); } [Test] @@ -99,17 +103,13 @@ namespace osu.Game.Tests.NonVisual { var combinations = new TestLegacyDifficultyCalculator(new ModA(), new MultiMod(new ModB(), new ModC())).CreateDifficultyAdjustmentModCombinations(); - Assert.AreEqual(4, combinations.Length); - Assert.IsTrue(combinations[0] is ModNoMod); - Assert.IsTrue(combinations[1] is ModA); - Assert.IsTrue(combinations[2] is MultiMod); - Assert.IsTrue(combinations[3] is MultiMod); - - Assert.IsTrue(((MultiMod)combinations[2]).Mods[0] is ModA); - Assert.IsTrue(((MultiMod)combinations[2]).Mods[1] is ModB); - Assert.IsTrue(((MultiMod)combinations[2]).Mods[2] is ModC); - Assert.IsTrue(((MultiMod)combinations[3]).Mods[0] is ModB); - Assert.IsTrue(((MultiMod)combinations[3]).Mods[1] is ModC); + assertCombinations(new[] + { + new[] { typeof(ModNoMod) }, + new[] { typeof(ModA) }, + new[] { typeof(ModA), typeof(ModB), typeof(ModC) }, + new[] { typeof(ModB), typeof(ModC) } + }, combinations); } [Test] @@ -117,13 +117,12 @@ namespace osu.Game.Tests.NonVisual { var combinations = new TestLegacyDifficultyCalculator(new ModA(), new MultiMod(new ModB(), new ModIncompatibleWithA())).CreateDifficultyAdjustmentModCombinations(); - Assert.AreEqual(3, combinations.Length); - Assert.IsTrue(combinations[0] is ModNoMod); - Assert.IsTrue(combinations[1] is ModA); - Assert.IsTrue(combinations[2] is MultiMod); - - Assert.IsTrue(((MultiMod)combinations[2]).Mods[0] is ModB); - Assert.IsTrue(((MultiMod)combinations[2]).Mods[1] is ModIncompatibleWithA); + assertCombinations(new[] + { + new[] { typeof(ModNoMod) }, + new[] { typeof(ModA) }, + new[] { typeof(ModB), typeof(ModIncompatibleWithA) } + }, combinations); } [Test] @@ -131,13 +130,28 @@ namespace osu.Game.Tests.NonVisual { var combinations = new TestLegacyDifficultyCalculator(new ModA(), new MultiMod(new ModA(), new ModB())).CreateDifficultyAdjustmentModCombinations(); - Assert.AreEqual(3, combinations.Length); - Assert.IsTrue(combinations[0] is ModNoMod); - Assert.IsTrue(combinations[1] is ModA); - Assert.IsTrue(combinations[2] is MultiMod); + assertCombinations(new[] + { + new[] { typeof(ModNoMod) }, + new[] { typeof(ModA) }, + new[] { typeof(ModA), typeof(ModB) } + }, combinations); + } - Assert.IsTrue(((MultiMod)combinations[2]).Mods[0] is ModA); - Assert.IsTrue(((MultiMod)combinations[2]).Mods[1] is ModB); + private void assertCombinations(Type[][] expectedCombinations, Mod[] actualCombinations) + { + Assert.AreEqual(expectedCombinations.Length, actualCombinations.Length); + + foreach (Type[] expected in expectedCombinations) + { + Type[] actualTypes = ModUtils.FlattenMods(actualCombinations).Select(m => m.GetType()).ToArray(); + + Assert.Multiple(() => + { + foreach (var expectedType in expected) + Assert.Contains(expectedType, actualTypes); + }); + } } private class ModA : Mod From 35d68d6ab039318bf024853ca3214c4d25368d2d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 Nov 2021 11:47:29 +0900 Subject: [PATCH 33/65] Remove all optimisations from `RealmLive` --- osu.Game/Database/RealmLive.cs | 43 +++++++++------------------------- 1 file changed, 11 insertions(+), 32 deletions(-) diff --git a/osu.Game/Database/RealmLive.cs b/osu.Game/Database/RealmLive.cs index 7ae7d8544a..0b422fd36f 100644 --- a/osu.Game/Database/RealmLive.cs +++ b/osu.Game/Database/RealmLive.cs @@ -2,7 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; -using System.Threading; +using osu.Framework.Development; using Realms; #nullable enable @@ -17,10 +17,7 @@ namespace osu.Game.Database { public Guid ID { get; } - public bool IsManaged { get; } - - private readonly SynchronizationContext? fetchedContext; - private readonly int fetchedThreadId; + public bool IsManaged => data.IsManaged; /// /// The original live data used to create this instance. @@ -35,14 +32,6 @@ namespace osu.Game.Database { this.data = data; - if (data.IsManaged) - { - IsManaged = true; - - fetchedContext = SynchronizationContext.Current; - fetchedThreadId = Thread.CurrentThread.ManagedThreadId; - } - ID = data.ID; } @@ -52,7 +41,7 @@ namespace osu.Game.Database /// The action to perform. public void PerformRead(Action perform) { - if (originalDataValid) + if (!IsManaged) { perform(data); return; @@ -71,7 +60,7 @@ namespace osu.Game.Database if (typeof(RealmObjectBase).IsAssignableFrom(typeof(TReturn))) throw new InvalidOperationException($"Realm live objects should not exit the scope of {nameof(PerformRead)}."); - if (originalDataValid) + if (!IsManaged) return perform(data); using (var realm = Realm.GetInstance(data.Realm.Config)) @@ -99,31 +88,21 @@ namespace osu.Game.Database { get { - if (originalDataValid) + if (!IsManaged) return data; - if (!isCorrectThread) - throw new InvalidOperationException($"Can't use {nameof(Value)} unless on the same thread the original data was fetched from."); + if (!ThreadSafety.IsUpdateThread) + throw new InvalidOperationException($"Can't use {nameof(Value)} on managed objects from non-update threads"); + + // When using Value, we rely on garbage collection for the realm instance used to retrieve the instance. + // As we are sure that this is on the same thread var realm = Realm.GetInstance(data.Realm.Config); - var retrieved = realm.Find(ID); - if (!retrieved.IsValid) - throw new InvalidOperationException("Attempted to access value without an open context"); - - return retrieved; + return realm.Find(ID); } } - // TODO: Revisit adding these conditionals back as an optimisation: || (isCorrectThread && data.IsValid); - // They have temporarily been removed due to an oversight involving .AsQueryable, see https://github.com/realm/realm-dotnet/discussions/2734. - // This means we are fetching a new context every `PerformRead` or `PerformWrite`, even when on the correct thread. - private bool originalDataValid => !IsManaged; - - // this matches realm's internal thread validation (see https://github.com/realm/realm-dotnet/blob/903b4d0b304f887e37e2d905384fb572a6496e70/Realm/Realm/Native/SynchronizationContextScheduler.cs#L72) - private bool isCorrectThread - => (fetchedContext != null && SynchronizationContext.Current == fetchedContext) || fetchedThreadId == Thread.CurrentThread.ManagedThreadId; - public bool Equals(ILive? other) => ID == other?.ID; } } From 2e31f5a338988fb69dd77c709c1e36f742dbb943 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 Nov 2021 11:55:13 +0900 Subject: [PATCH 34/65] Update tests to match new behaviour --- osu.Game.Tests/Database/RealmLiveTests.cs | 107 +++++++++++----------- 1 file changed, 53 insertions(+), 54 deletions(-) diff --git a/osu.Game.Tests/Database/RealmLiveTests.cs b/osu.Game.Tests/Database/RealmLiveTests.cs index 63566394d6..41c82399dc 100644 --- a/osu.Game.Tests/Database/RealmLiveTests.cs +++ b/osu.Game.Tests/Database/RealmLiveTests.cs @@ -46,53 +46,6 @@ namespace osu.Game.Tests.Database Assert.IsFalse(liveBeatmap.PerformRead(l => l.Hidden)); } - [Test] - public void TestValueAccessWithOpenContext() - { - RunTestWithRealm((realmFactory, _) => - { - RealmLive? liveBeatmap = null; - Task.Factory.StartNew(() => - { - using (var threadContext = realmFactory.CreateContext()) - { - var beatmap = threadContext.Write(r => r.Add(new RealmBeatmap(CreateRuleset(), new RealmBeatmapDifficulty(), new RealmBeatmapMetadata()))); - - liveBeatmap = beatmap.ToLive(); - } - }, TaskCreationOptions.LongRunning | TaskCreationOptions.HideScheduler).Wait(); - - Debug.Assert(liveBeatmap != null); - - Task.Factory.StartNew(() => - { - // TODO: The commented code is the behaviour we hope to obtain, but is temporarily disabled. - // See https://github.com/ppy/osu/pull/15851 - using (realmFactory.CreateContext()) - { - Assert.Throws(() => - { - var __ = liveBeatmap.Value; - }); - } - - // Assert.DoesNotThrow(() => - // { - // using (realmFactory.CreateContext()) - // { - // var resolved = liveBeatmap.Value; - // - // Assert.IsTrue(resolved.Realm.IsClosed); - // Assert.IsTrue(resolved.IsValid); - // - // // can access properties without a crash. - // Assert.IsFalse(resolved.Hidden); - // } - // }); - }, TaskCreationOptions.LongRunning | TaskCreationOptions.HideScheduler).Wait(); - }); - } - [Test] public void TestScopedReadWithoutContext() { @@ -148,6 +101,59 @@ namespace osu.Game.Tests.Database }); } + [Test] + public void TestValueAccessNonManaged() + { + RunTestWithRealm((realmFactory, _) => + { + var beatmap = new RealmBeatmap(CreateRuleset(), new RealmBeatmapDifficulty(), new RealmBeatmapMetadata()); + var liveBeatmap = beatmap.ToLive(); + + Assert.DoesNotThrow(() => + { + var __ = liveBeatmap.Value; + }); + }); + } + + [Test] + public void TestValueAccessWithOpenContextFails() + { + RunTestWithRealm((realmFactory, _) => + { + RealmLive? liveBeatmap = null; + Task.Factory.StartNew(() => + { + using (var threadContext = realmFactory.CreateContext()) + { + var beatmap = threadContext.Write(r => r.Add(new RealmBeatmap(CreateRuleset(), new RealmBeatmapDifficulty(), new RealmBeatmapMetadata()))); + + liveBeatmap = beatmap.ToLive(); + } + }, TaskCreationOptions.LongRunning | TaskCreationOptions.HideScheduler).Wait(); + + Debug.Assert(liveBeatmap != null); + + Task.Factory.StartNew(() => + { + // Can't be used, without a valid context. + Assert.Throws(() => + { + var __ = liveBeatmap.Value; + }); + + // Can't be used, even from within a valid context. + using (realmFactory.CreateContext()) + { + Assert.Throws(() => + { + var __ = liveBeatmap.Value; + }); + } + }, TaskCreationOptions.LongRunning | TaskCreationOptions.HideScheduler).Wait(); + }); + } + [Test] public void TestValueAccessWithoutOpenContextFails() { @@ -209,8 +215,6 @@ namespace osu.Game.Tests.Database Assert.AreEqual(0, updateThreadContext.All().Count()); Assert.AreEqual(0, changesTriggered); - // TODO: Originally the following was using `liveBeatmap.Value`. This has been temporarily disabled. - // See https://github.com/ppy/osu/pull/15851 liveBeatmap.PerformRead(resolved => { // retrieval causes an implicit refresh. even changes that aren't related to the retrieval are fired at this point. @@ -218,11 +222,6 @@ namespace osu.Game.Tests.Database Assert.AreEqual(2, updateThreadContext.All().Count()); Assert.AreEqual(1, changesTriggered); - // TODO: as above, temporarily disabled as it doesn't make sense inside a `PerformRead`. - // // even though the realm that this instance was resolved for was closed, it's still valid. - // Assert.IsTrue(resolved.Realm.IsClosed); - // Assert.IsTrue(resolved.IsValid); - // can access properties without a crash. Assert.IsFalse(resolved.Hidden); From f3f77fa05316faa2e046edf7ef116058f190c9fa Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 Nov 2021 11:56:53 +0900 Subject: [PATCH 35/65] Update missed xmldoc/comments --- osu.Game/Database/ILive.cs | 4 ++-- osu.Game/Database/RealmLive.cs | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/osu.Game/Database/ILive.cs b/osu.Game/Database/ILive.cs index a863339f11..3011754bc1 100644 --- a/osu.Game/Database/ILive.cs +++ b/osu.Game/Database/ILive.cs @@ -38,10 +38,10 @@ namespace osu.Game.Database bool IsManaged { get; } /// - /// Resolve the value of this instance on the current thread's context. + /// Resolve the value of this instance on the update thread. /// /// - /// After resolving the data should not be passed between threads. + /// After resolving, the data should not be passed between threads. /// T Value { get; } } diff --git a/osu.Game/Database/RealmLive.cs b/osu.Game/Database/RealmLive.cs index 0b422fd36f..2accea305a 100644 --- a/osu.Game/Database/RealmLive.cs +++ b/osu.Game/Database/RealmLive.cs @@ -95,8 +95,7 @@ namespace osu.Game.Database throw new InvalidOperationException($"Can't use {nameof(Value)} on managed objects from non-update threads"); // When using Value, we rely on garbage collection for the realm instance used to retrieve the instance. - // As we are sure that this is on the same thread - + // As we are sure that this is on the update thread, there should always be an open and constantly refreshing realm instance to ensure file size growth is a non-issue. var realm = Realm.GetInstance(data.Realm.Config); return realm.Find(ID); From a73919917c3f829bee58a827c9071a8e145ddb51 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 Nov 2021 12:32:08 +0900 Subject: [PATCH 36/65] Fix intermittent test failures in `TestScenBeatmapInfoWedge` due to async load https://github.com/ppy/osu/runs/4358685294?check_suite_focus=true Occurs due to the wedge content also reloading on ruleset change, which wasn't being accounted for. In a fail case, the content would change during the "select beatmap" step's async load wait, causing incorrect results. https://github.com/ppy/osu/blob/51a353e12db189f9958228d30fe045b8460c6b92/osu.Game/Screens/Select/BeatmapInfoWedge.cs#L70 --- .../SongSelect/TestSceneBeatmapInfoWedge.cs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapInfoWedge.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapInfoWedge.cs index e7c54efa8c..9ad5242df4 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapInfoWedge.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapInfoWedge.cs @@ -82,7 +82,7 @@ namespace osu.Game.Tests.Visual.SongSelect beatmaps.Add(testBeatmap); - AddStep("set ruleset", () => Ruleset.Value = rulesetInfo); + setRuleset(rulesetInfo); selectBeatmap(testBeatmap); @@ -167,6 +167,22 @@ namespace osu.Game.Tests.Visual.SongSelect label => label.Statistic.Name == "BPM" && label.Statistic.Content == target.ToString(CultureInfo.InvariantCulture))); } + private void setRuleset(RulesetInfo rulesetInfo) + { + Container containerBefore = null; + + AddStep("set ruleset", () => + { + // wedge content is only refreshed if the ruleset changes, so only wait for load in that case. + if (!rulesetInfo.Equals(Ruleset.Value)) + containerBefore = infoWedge.DisplayedContent; + + Ruleset.Value = rulesetInfo; + }); + + AddUntilStep("wait for async load", () => infoWedge.DisplayedContent != containerBefore); + } + private void selectBeatmap([CanBeNull] IBeatmap b) { Container containerBefore = null; From f921acc681fbef678c39a15cc3502b58d286148c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 Nov 2021 13:01:40 +0900 Subject: [PATCH 37/65] Fix chat tab dropdown not being reachable at default sizing Would have liked to fix this in a more local way, but the structure of the dropdowns is just a pain in the ass to work with, so this will do for now. --- osu.Game/Graphics/UserInterface/OsuTabDropdown.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Graphics/UserInterface/OsuTabDropdown.cs b/osu.Game/Graphics/UserInterface/OsuTabDropdown.cs index 68ffc6bf4e..b7e25ae4e7 100644 --- a/osu.Game/Graphics/UserInterface/OsuTabDropdown.cs +++ b/osu.Game/Graphics/UserInterface/OsuTabDropdown.cs @@ -66,7 +66,7 @@ namespace osu.Game.Graphics.UserInterface Origin = Anchor.TopRight; BackgroundColour = Color4.Black.Opacity(0.7f); - MaxHeight = 400; + MaxHeight = 200; } protected override DrawableDropdownMenuItem CreateDrawableDropdownMenuItem(MenuItem item) => new DrawableOsuTabDropdownMenuItem(item); From 384a0664c27101c9f32712942e817913a12cb73e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 Nov 2021 14:05:44 +0900 Subject: [PATCH 38/65] Remove unused migration method --- osu.Game/OsuGameBase.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 88c9ab370c..bfc0da468b 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -442,10 +442,6 @@ namespace osu.Game protected override Storage CreateStorage(GameHost host, Storage defaultStorage) => new OsuStorage(host, defaultStorage); - private void migrateDataToRealm() - { - } - private void onRulesetChanged(ValueChangedEvent r) { if (r.NewValue?.Available != true) From 6e4cd91b7bb782572e2e7abbd5b6dc9926dca95e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 Nov 2021 14:12:36 +0900 Subject: [PATCH 39/65] Fix update thread realm context never being `Refresh()`ed --- osu.Game/OsuGameBase.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index bfc0da468b..6eb67b34e8 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -377,6 +377,13 @@ namespace osu.Game FrameStatistics.ValueChanged += e => fpsDisplayVisible.Value = e.NewValue != FrameStatisticsMode.None; } + protected override void Update() + { + base.Update(); + + realmFactory.Refresh(); + } + protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) => dependencies = new DependencyContainer(base.CreateChildDependencies(parent)); From 0feec0996665a0009479d4667a4500d3edd8b8b1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 Nov 2021 14:12:49 +0900 Subject: [PATCH 40/65] Refactor beatmap importer tests to ensure realm is refreshed when waiting on state --- .../Database/BeatmapImporterTests.cs | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/osu.Game.Tests/Database/BeatmapImporterTests.cs b/osu.Game.Tests/Database/BeatmapImporterTests.cs index d2193350ad..a6edd6cb5f 100644 --- a/osu.Game.Tests/Database/BeatmapImporterTests.cs +++ b/osu.Game.Tests/Database/BeatmapImporterTests.cs @@ -852,7 +852,11 @@ namespace osu.Game.Tests.Database { IQueryable? resultSets = null; - waitForOrAssert(() => (resultSets = realm.All().Where(s => !s.DeletePending && s.OnlineID == 241526)).Any(), + waitForOrAssert(() => + { + realm.Refresh(); + return (resultSets = realm.All().Where(s => !s.DeletePending && s.OnlineID == 241526)).Any(); + }, @"BeatmapSet did not import to the database in allocated time.", timeout); // ensure we were stored to beatmap database backing... @@ -865,16 +869,16 @@ namespace osu.Game.Tests.Database // ReSharper disable once PossibleUnintendedReferenceComparison IEnumerable queryBeatmaps() => realm.All().Where(s => s.BeatmapSet != null && s.BeatmapSet == set); - waitForOrAssert(() => queryBeatmaps().Count() == 12, @"Beatmaps did not import to the database in allocated time", timeout); - waitForOrAssert(() => queryBeatmapSets().Count() == 1, @"BeatmapSet did not import to the database in allocated time", timeout); + Assert.AreEqual(12, queryBeatmaps().Count(), @"Beatmap count was not correct"); + Assert.AreEqual(1, queryBeatmapSets().Count(), @"Beatmapset count was not correct"); - int countBeatmapSetBeatmaps = 0; - int countBeatmaps = 0; + int countBeatmapSetBeatmaps; + int countBeatmaps; - waitForOrAssert(() => - (countBeatmapSetBeatmaps = queryBeatmapSets().First().Beatmaps.Count) == - (countBeatmaps = queryBeatmaps().Count()), - $@"Incorrect database beatmap count post-import ({countBeatmaps} but should be {countBeatmapSetBeatmaps}).", timeout); + Assert.AreEqual( + countBeatmapSetBeatmaps = queryBeatmapSets().First().Beatmaps.Count, + countBeatmaps = queryBeatmaps().Count(), + $@"Incorrect database beatmap count post-import ({countBeatmaps} but should be {countBeatmapSetBeatmaps})."); foreach (RealmBeatmap b in set.Beatmaps) Assert.IsTrue(set.Beatmaps.Any(c => c.OnlineID == b.OnlineID)); From 049d9ce5ef5b906278b39bf83d9e5eac17284993 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 Nov 2021 17:09:32 +0900 Subject: [PATCH 41/65] Add test coverage of match type propagating to other users' settings --- .../Visual/Multiplayer/TestSceneTeamVersus.cs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs index c70906927e..7763e11609 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs @@ -11,6 +11,7 @@ using osu.Framework.Testing; using osu.Game.Beatmaps; using osu.Game.Database; using osu.Game.Online.API.Requests.Responses; +using osu.Game.Online.Multiplayer; using osu.Game.Online.Multiplayer.MatchTypes.TeamVersus; using osu.Game.Online.Rooms; using osu.Game.Rulesets; @@ -118,6 +119,25 @@ namespace osu.Game.Tests.Visual.Multiplayer AddAssert("user still on team 0", () => (client.Room?.Users.FirstOrDefault()?.MatchState as TeamVersusUserState)?.TeamID == 0); } + [Test] + public void TestSettingsUpdatedWhenChangingQueueMode() + { + createRoom(() => new Room + { + Name = { Value = "Test Room" }, + Type = { Value = MatchType.TeamVersus } + }); + + AddUntilStep("match type versus", () => client.APIRoom?.Type.Value == MatchType.HeadToHead); + + AddStep("change match type", () => client.ChangeSettings(new MultiplayerRoomSettings + { + MatchType = MatchType.TeamVersus + })); + + AddUntilStep("api room updated", () => client.APIRoom?.Type.Value == MatchType.TeamVersus); + } + [Test] public void TestChangeTypeViaMatchSettings() { From 25b9575de01d4a7d35dc19ece9b6748506f60cab Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 Nov 2021 17:09:40 +0900 Subject: [PATCH 42/65] Fix missing transfer of match type to settings --- osu.Game/Online/Multiplayer/MultiplayerClient.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game/Online/Multiplayer/MultiplayerClient.cs b/osu.Game/Online/Multiplayer/MultiplayerClient.cs index 4c472164d6..02fb94133a 100644 --- a/osu.Game/Online/Multiplayer/MultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/MultiplayerClient.cs @@ -694,6 +694,7 @@ namespace osu.Game.Online.Multiplayer Room.Settings = settings; APIRoom.Name.Value = Room.Settings.Name; APIRoom.Password.Value = Room.Settings.Password; + APIRoom.Type.Value = Room.Settings.MatchType; APIRoom.QueueMode.Value = Room.Settings.QueueMode; RoomUpdated?.Invoke(); From 8de06803a8f3285d683c8db38a818e5a9c4b4ced Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 Nov 2021 17:16:30 +0900 Subject: [PATCH 43/65] Fix incorrect test step text Co-authored-by: Dan Balasescu --- osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs index 7763e11609..e00939a4de 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs @@ -120,7 +120,7 @@ namespace osu.Game.Tests.Visual.Multiplayer } [Test] - public void TestSettingsUpdatedWhenChangingQueueMode() + public void TestSettingsUpdatedWhenChangingMatchType() { createRoom(() => new Room { From 23e297d414815477ed71f4e6e9360aee2bb8f7ce Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 Nov 2021 19:08:51 +0900 Subject: [PATCH 44/65] Log output response sizes Visibility is the first step towards action. Or something. --- osu.Game/Online/API/APIRequest.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/osu.Game/Online/API/APIRequest.cs b/osu.Game/Online/API/APIRequest.cs index 43195811dc..efb0b102d0 100644 --- a/osu.Game/Online/API/APIRequest.cs +++ b/osu.Game/Online/API/APIRequest.cs @@ -38,7 +38,12 @@ namespace osu.Game.Online.API protected override void PostProcess() { base.PostProcess(); - Response = ((OsuJsonWebRequest)WebRequest)?.ResponseObject; + + if (WebRequest != null) + { + Response = ((OsuJsonWebRequest)WebRequest).ResponseObject; + Logger.Log($"{GetType()} finished with response size of {WebRequest.ResponseStream.Length:#,0} bytes"); + } } internal void TriggerSuccess(T result) From 87883f1fe433608090aebdeaa981632310fa5158 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 Nov 2021 19:27:43 +0900 Subject: [PATCH 45/65] Add `BeatmapLookupCache` --- osu.Game/Database/BeatmapLookupCache.cs | 143 ++++++++++++++++++++++++ osu.Game/OsuGameBase.cs | 4 + 2 files changed, 147 insertions(+) create mode 100644 osu.Game/Database/BeatmapLookupCache.cs diff --git a/osu.Game/Database/BeatmapLookupCache.cs b/osu.Game/Database/BeatmapLookupCache.cs new file mode 100644 index 0000000000..c4e20d59b6 --- /dev/null +++ b/osu.Game/Database/BeatmapLookupCache.cs @@ -0,0 +1,143 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using JetBrains.Annotations; +using osu.Framework.Allocation; +using osu.Game.Online.API; +using osu.Game.Online.API.Requests; +using osu.Game.Online.API.Requests.Responses; + +namespace osu.Game.Database +{ + // This class is based on `UserLookupCache` which is well tested. + // If modifications are to be made here, a base abstract implementation should likely be created and shared between the two. + public class BeatmapLookupCache : MemoryCachingComponent + { + [Resolved] + private IAPIProvider api { get; set; } + + /// + /// Perform an API lookup on the specified beatmap, populating a model. + /// + /// The beatmap to lookup. + /// An optional cancellation token. + /// The populated beatmap, or null if the beatmap does not exist or the request could not be satisfied. + [ItemCanBeNull] + public Task GetBeatmapAsync(int beatmapId, CancellationToken token = default) => GetAsync(beatmapId, token); + + /// + /// Perform an API lookup on the specified beatmaps, populating a model. + /// + /// The beatmaps to lookup. + /// An optional cancellation token. + /// The populated beatmaps. May include null results for failed retrievals. + public Task GetBeatmapsAsync(int[] beatmapIds, CancellationToken token = default) + { + var beatmapLookupTasks = new List>(); + + foreach (int u in beatmapIds) + { + beatmapLookupTasks.Add(GetBeatmapAsync(u, token).ContinueWith(task => + { + if (!task.IsCompletedSuccessfully) + return null; + + return task.Result; + }, token)); + } + + return Task.WhenAll(beatmapLookupTasks); + } + + protected override async Task ComputeValueAsync(int lookup, CancellationToken token = default) + => await queryBeatmap(lookup).ConfigureAwait(false); + + private readonly Queue<(int id, TaskCompletionSource)> pendingBeatmapTasks = new Queue<(int, TaskCompletionSource)>(); + private Task pendingRequestTask; + private readonly object taskAssignmentLock = new object(); + + private Task queryBeatmap(int beatmapId) + { + lock (taskAssignmentLock) + { + var tcs = new TaskCompletionSource(); + + // Add to the queue. + pendingBeatmapTasks.Enqueue((beatmapId, tcs)); + + // Create a request task if there's not already one. + if (pendingRequestTask == null) + createNewTask(); + + return tcs.Task; + } + } + + private void performLookup() + { + // contains at most 50 unique beatmap IDs from beatmapTasks, which is used to perform the lookup. + var beatmapTasks = new Dictionary>>(); + + // Grab at most 50 unique beatmap IDs from the queue. + lock (taskAssignmentLock) + { + while (pendingBeatmapTasks.Count > 0 && beatmapTasks.Count < 1) + { + (int id, TaskCompletionSource task) next = pendingBeatmapTasks.Dequeue(); + + // Perform a secondary check for existence, in case the beatmap was queried in a previous batch. + if (CheckExists(next.id, out var existing)) + next.task.SetResult(existing); + else + { + if (beatmapTasks.TryGetValue(next.id, out var tasks)) + tasks.Add(next.task); + else + beatmapTasks[next.id] = new List> { next.task }; + } + } + } + + // Query the beatmaps. + var request = new GetBeatmapRequest(new APIBeatmap { OnlineID = beatmapTasks.Keys.First() }); + + // rather than queueing, we maintain our own single-threaded request stream. + // todo: we probably want retry logic here. + api.Perform(request); + + // Create a new request task if there's still more beatmaps to query. + lock (taskAssignmentLock) + { + pendingRequestTask = null; + if (pendingBeatmapTasks.Count > 0) + createNewTask(); + } + + List foundBeatmaps = new List { request.Response }; + + foreach (var beatmap in foundBeatmaps) + { + if (beatmapTasks.TryGetValue(beatmap.OnlineID, out var tasks)) + { + foreach (var task in tasks) + task.SetResult(beatmap); + + beatmapTasks.Remove(beatmap.OnlineID); + } + } + + // if any tasks remain which were not satisfied, return null. + foreach (var tasks in beatmapTasks.Values) + { + foreach (var task in tasks) + task.SetResult(null); + } + } + + private void createNewTask() => pendingRequestTask = Task.Run(performLookup); + } +} diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 6eb67b34e8..34344f8022 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -142,6 +142,7 @@ namespace osu.Game private BeatmapDifficultyCache difficultyCache; private UserLookupCache userCache; + private BeatmapLookupCache beatmapCache; private FileStore fileStore; @@ -265,6 +266,9 @@ namespace osu.Game dependencies.Cache(userCache = new UserLookupCache()); AddInternal(userCache); + dependencies.Cache(beatmapCache = new BeatmapLookupCache()); + AddInternal(beatmapCache); + var scorePerformanceManager = new ScorePerformanceCache(); dependencies.Cache(scorePerformanceManager); AddInternal(scorePerformanceManager); From f58c5cd9c0f2d9a6ffaf0457d11cb62cc124249e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 Nov 2021 19:36:25 +0900 Subject: [PATCH 46/65] Update `MultiplayerClient` to use `BeatmapLookupCache` --- .../Online/Multiplayer/MultiplayerClient.cs | 20 +++++--------- .../Multiplayer/OnlineMultiplayerClient.cs | 27 +++++-------------- .../Multiplayer/TestMultiplayerClient.cs | 15 ++++++----- 3 files changed, 20 insertions(+), 42 deletions(-) diff --git a/osu.Game/Online/Multiplayer/MultiplayerClient.cs b/osu.Game/Online/Multiplayer/MultiplayerClient.cs index 4c472164d6..aa91d5e56f 100644 --- a/osu.Game/Online/Multiplayer/MultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/MultiplayerClient.cs @@ -702,15 +702,7 @@ namespace osu.Game.Online.Multiplayer private async Task createPlaylistItem(MultiplayerPlaylistItem item) { - var set = await GetOnlineBeatmapSet(item.BeatmapID).ConfigureAwait(false); - - // The incoming response is deserialised without circular reference handling currently. - // Because we require using metadata from this instance, populate the nested beatmaps' sets manually here. - foreach (var b in set.Beatmaps) - b.BeatmapSet = set; - - var beatmap = set.Beatmaps.Single(b => b.OnlineID == item.BeatmapID); - beatmap.Checksum = item.BeatmapChecksum; + var apiBeatmap = await GetOnlineBeatmapSet(item.BeatmapID).ConfigureAwait(false); var ruleset = Rulesets.GetRuleset(item.RulesetID); var rulesetInstance = ruleset.CreateInstance(); @@ -719,7 +711,7 @@ namespace osu.Game.Online.Multiplayer { ID = item.ID, OwnerID = item.OwnerID, - Beatmap = { Value = beatmap }, + Beatmap = { Value = apiBeatmap }, Ruleset = { Value = ruleset }, Expired = item.Expired }; @@ -731,12 +723,12 @@ namespace osu.Game.Online.Multiplayer } /// - /// Retrieves a from an online source. + /// Retrieves a from an online source. /// - /// The beatmap set ID. + /// The beatmap ID. /// A token to cancel the request. - /// The retrieval task. - protected abstract Task GetOnlineBeatmapSet(int beatmapId, CancellationToken cancellationToken = default); + /// The retrieval task. + protected abstract Task GetOnlineBeatmapSet(int beatmapId, CancellationToken cancellationToken = default); /// /// For the provided user ID, update whether the user is included in . diff --git a/osu.Game/Online/Multiplayer/OnlineMultiplayerClient.cs b/osu.Game/Online/Multiplayer/OnlineMultiplayerClient.cs index 7308c03ec3..de974cb3e1 100644 --- a/osu.Game/Online/Multiplayer/OnlineMultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/OnlineMultiplayerClient.cs @@ -9,8 +9,8 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.SignalR.Client; using osu.Framework.Allocation; using osu.Framework.Bindables; +using osu.Game.Database; using osu.Game.Online.API; -using osu.Game.Online.API.Requests; using osu.Game.Online.API.Requests.Responses; using osu.Game.Online.Rooms; @@ -29,6 +29,9 @@ namespace osu.Game.Online.Multiplayer private HubConnection? connection => connector?.CurrentConnection; + [Resolved] + private BeatmapLookupCache beatmapLookupCache { get; set; } = null!; + public OnlineMultiplayerClient(EndpointConfiguration endpoints) { endpoint = endpoints.MultiplayerEndpointUrl; @@ -159,27 +162,9 @@ namespace osu.Game.Online.Multiplayer return connection.InvokeAsync(nameof(IMultiplayerServer.AddPlaylistItem), item); } - protected override Task GetOnlineBeatmapSet(int beatmapId, CancellationToken cancellationToken = default) + protected override Task GetOnlineBeatmapSet(int beatmapId, CancellationToken cancellationToken = default) { - var tcs = new TaskCompletionSource(); - var req = new GetBeatmapSetRequest(beatmapId, BeatmapSetLookupType.BeatmapId); - - req.Success += res => - { - if (cancellationToken.IsCancellationRequested) - { - tcs.SetCanceled(); - return; - } - - tcs.SetResult(res); - }; - - req.Failure += e => tcs.SetException(e); - - API.Queue(req); - - return tcs.Task; + return beatmapLookupCache.GetBeatmapAsync(beatmapId, cancellationToken); } protected override void Dispose(bool isDisposing) diff --git a/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs b/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs index 2d77e17513..39199ae304 100644 --- a/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs +++ b/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs @@ -336,7 +336,7 @@ namespace osu.Game.Tests.Visual.Multiplayer public override Task AddPlaylistItem(MultiplayerPlaylistItem item) => AddUserPlaylistItem(api.LocalUser.Value.OnlineID, item); - protected override Task GetOnlineBeatmapSet(int beatmapId, CancellationToken cancellationToken = default) + protected override Task GetOnlineBeatmapSet(int beatmapId, CancellationToken cancellationToken = default) { IBeatmapSetInfo? set = roomManager.ServerSideRooms.SelectMany(r => r.Playlist) .FirstOrDefault(p => p.BeatmapID == beatmapId)?.Beatmap.Value.BeatmapSet @@ -345,13 +345,14 @@ namespace osu.Game.Tests.Visual.Multiplayer if (set == null) throw new InvalidOperationException("Beatmap not found."); - var apiSet = new APIBeatmapSet + return Task.FromResult(new APIBeatmap { - OnlineID = set.OnlineID, - Beatmaps = set.Beatmaps.Select(b => new APIBeatmap { OnlineID = b.OnlineID }).ToArray(), - }; - - return Task.FromResult(apiSet); + BeatmapSet = new APIBeatmapSet + { + OnlineID = set.OnlineID, + }, + OnlineID = set.Beatmaps.First().OnlineID + }); } private async Task changeMatchType(MatchType type) From 01bc330d1ce8468cecd95f83bca02be187730867 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 Nov 2021 19:42:46 +0900 Subject: [PATCH 47/65] Rename method to match new purpose --- osu.Game/Online/Multiplayer/MultiplayerClient.cs | 4 ++-- osu.Game/Online/Multiplayer/OnlineMultiplayerClient.cs | 2 +- osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game/Online/Multiplayer/MultiplayerClient.cs b/osu.Game/Online/Multiplayer/MultiplayerClient.cs index aa91d5e56f..478da983cd 100644 --- a/osu.Game/Online/Multiplayer/MultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/MultiplayerClient.cs @@ -702,7 +702,7 @@ namespace osu.Game.Online.Multiplayer private async Task createPlaylistItem(MultiplayerPlaylistItem item) { - var apiBeatmap = await GetOnlineBeatmapSet(item.BeatmapID).ConfigureAwait(false); + var apiBeatmap = await GetAPIBeatmap(item.BeatmapID).ConfigureAwait(false); var ruleset = Rulesets.GetRuleset(item.RulesetID); var rulesetInstance = ruleset.CreateInstance(); @@ -728,7 +728,7 @@ namespace osu.Game.Online.Multiplayer /// The beatmap ID. /// A token to cancel the request. /// The retrieval task. - protected abstract Task GetOnlineBeatmapSet(int beatmapId, CancellationToken cancellationToken = default); + protected abstract Task GetAPIBeatmap(int beatmapId, CancellationToken cancellationToken = default); /// /// For the provided user ID, update whether the user is included in . diff --git a/osu.Game/Online/Multiplayer/OnlineMultiplayerClient.cs b/osu.Game/Online/Multiplayer/OnlineMultiplayerClient.cs index de974cb3e1..41687b54b0 100644 --- a/osu.Game/Online/Multiplayer/OnlineMultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/OnlineMultiplayerClient.cs @@ -162,7 +162,7 @@ namespace osu.Game.Online.Multiplayer return connection.InvokeAsync(nameof(IMultiplayerServer.AddPlaylistItem), item); } - protected override Task GetOnlineBeatmapSet(int beatmapId, CancellationToken cancellationToken = default) + protected override Task GetAPIBeatmap(int beatmapId, CancellationToken cancellationToken = default) { return beatmapLookupCache.GetBeatmapAsync(beatmapId, cancellationToken); } diff --git a/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs b/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs index 39199ae304..037dccc63f 100644 --- a/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs +++ b/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs @@ -336,7 +336,7 @@ namespace osu.Game.Tests.Visual.Multiplayer public override Task AddPlaylistItem(MultiplayerPlaylistItem item) => AddUserPlaylistItem(api.LocalUser.Value.OnlineID, item); - protected override Task GetOnlineBeatmapSet(int beatmapId, CancellationToken cancellationToken = default) + protected override Task GetAPIBeatmap(int beatmapId, CancellationToken cancellationToken = default) { IBeatmapSetInfo? set = roomManager.ServerSideRooms.SelectMany(r => r.Playlist) .FirstOrDefault(p => p.BeatmapID == beatmapId)?.Beatmap.Value.BeatmapSet From 0fae10500a2860933f7125da02b1c4c2e09d80b1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 Nov 2021 20:02:15 +0900 Subject: [PATCH 48/65] Fix failing tests --- .../Tests/Visual/Multiplayer/TestMultiplayerClient.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs b/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs index 037dccc63f..05b7c11e34 100644 --- a/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs +++ b/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs @@ -347,11 +347,9 @@ namespace osu.Game.Tests.Visual.Multiplayer return Task.FromResult(new APIBeatmap { - BeatmapSet = new APIBeatmapSet - { - OnlineID = set.OnlineID, - }, - OnlineID = set.Beatmaps.First().OnlineID + BeatmapSet = new APIBeatmapSet { OnlineID = set.OnlineID }, + OnlineID = beatmapId, + Checksum = set.Beatmaps.First(b => b.OnlineID == beatmapId).MD5Hash }); } From d18417aa2706d1eae6ac1094d942d4538372c0e9 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 23 Nov 2021 12:04:27 +0300 Subject: [PATCH 49/65] Enable build-only iOS CI --- .github/workflows/ci.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 68f8ef51ef..3c52802cf6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -77,10 +77,6 @@ jobs: run: msbuild osu.Android/osu.Android.csproj /restore /p:Configuration=Debug build-only-ios: - # While this workflow technically *can* run, it fails as iOS builds are blocked by multiple issues. - # See https://github.com/ppy/osu-framework/issues/4677 for the details. - # The job can be unblocked once those issues are resolved and game deployments can happen again. - if: false name: Build only (iOS) runs-on: macos-latest timeout-minutes: 60 From 03e1305b3f54639fd4375d715faaf7257c8adec2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 1 Dec 2021 01:55:14 +0900 Subject: [PATCH 50/65] Fix toast display potentially causing a child mutation before load complete --- osu.Game/Overlays/OnScreenDisplay.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Overlays/OnScreenDisplay.cs b/osu.Game/Overlays/OnScreenDisplay.cs index af6d24fc65..be9d3cd794 100644 --- a/osu.Game/Overlays/OnScreenDisplay.cs +++ b/osu.Game/Overlays/OnScreenDisplay.cs @@ -95,13 +95,13 @@ namespace osu.Game.Overlays /// Displays the provided temporarily. /// /// - public void Display(Toast toast) + public void Display(Toast toast) => Schedule(() => { box.Child = toast; DisplayTemporarily(box); - } + }); - private void displayTrackedSettingChange(SettingDescription description) => Schedule(() => Display(new TrackedSettingToast(description))); + private void displayTrackedSettingChange(SettingDescription description) => Display(new TrackedSettingToast(description)); private TransformSequence fadeIn; private ScheduledDelegate fadeOut; From dda7142f48226f9db18e96efd8ce06b1fb286f47 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 1 Dec 2021 02:06:40 +0900 Subject: [PATCH 51/65] Fix double screen exit in multiplayer song select tests potentially causing failure --- .../Visual/Multiplayer/TestSceneMultiplayerMatchSongSelect.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSongSelect.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSongSelect.cs index 84b24ba3a1..a5229702a8 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSongSelect.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSongSelect.cs @@ -144,7 +144,8 @@ namespace osu.Game.Tests.Visual.Multiplayer AddStep("set mods", () => SelectedMods.Value = new[] { new TaikoModDoubleTime() }); AddStep("confirm selection", () => songSelect.FinaliseSelection()); - AddStep("exit song select", () => songSelect.Exit()); + + AddUntilStep("song select exited", () => !songSelect.IsCurrentScreen()); AddAssert("beatmap not changed", () => Beatmap.Value.BeatmapInfo.Equals(selectedBeatmap)); AddAssert("ruleset not changed", () => Ruleset.Value.Equals(new TaikoRuleset().RulesetInfo)); From b74b09eb3a7f18634bc84696ba9a57dbca0f7f1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 30 Nov 2021 20:56:11 +0100 Subject: [PATCH 52/65] Add extra until step to make cause of potential failures more obvious --- osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs index e00939a4de..ab9dc9c989 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs @@ -172,6 +172,7 @@ namespace osu.Game.Tests.Visual.Multiplayer AddUntilStep("wait for room open", () => this.ChildrenOfType().FirstOrDefault()?.IsLoaded == true); AddWaitStep("wait for transition", 2); + AddUntilStep("create room button enabled", () => this.ChildrenOfType().Single().Enabled.Value); AddStep("create room", () => { InputManager.MoveMouseTo(this.ChildrenOfType().Single()); From de034b4d9c93d09e9ce4695fbccf13fe648f9414 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 30 Nov 2021 20:57:02 +0100 Subject: [PATCH 53/65] Fix test failures due to wrong asserts & uninitialised playlist * The "create room" button was disabled headless due to not specifying the imported beatmap. In visual tests it seems to work due to selecting from the local database randomly. * The test asserts are brought in line with expectations. --- .../Visual/Multiplayer/TestSceneTeamVersus.cs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs index ab9dc9c989..981989c28a 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs @@ -125,17 +125,25 @@ namespace osu.Game.Tests.Visual.Multiplayer createRoom(() => new Room { Name = { Value = "Test Room" }, - Type = { Value = MatchType.TeamVersus } + Type = { Value = MatchType.HeadToHead }, + Playlist = + { + new PlaylistItem + { + Beatmap = { Value = beatmaps.GetWorkingBeatmap(importedSet.Beatmaps.First(b => b.RulesetID == 0)).BeatmapInfo }, + Ruleset = { Value = new OsuRuleset().RulesetInfo }, + } + } }); - AddUntilStep("match type versus", () => client.APIRoom?.Type.Value == MatchType.HeadToHead); + AddUntilStep("match type head to head", () => client.APIRoom?.Type.Value == MatchType.HeadToHead); AddStep("change match type", () => client.ChangeSettings(new MultiplayerRoomSettings { MatchType = MatchType.TeamVersus })); - AddUntilStep("api room updated", () => client.APIRoom?.Type.Value == MatchType.TeamVersus); + AddUntilStep("api room updated to team versus", () => client.APIRoom?.Type.Value == MatchType.TeamVersus); } [Test] From 0479027f644c686cf65d3ee9ceb65e14bd0e42e4 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 1 Dec 2021 11:29:02 +0900 Subject: [PATCH 54/65] Apply assertion fix from review --- .../DifficultyAdjustmentModCombinationsTest.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/osu.Game.Tests/NonVisual/DifficultyAdjustmentModCombinationsTest.cs b/osu.Game.Tests/NonVisual/DifficultyAdjustmentModCombinationsTest.cs index 612927337f..ae8eec2629 100644 --- a/osu.Game.Tests/NonVisual/DifficultyAdjustmentModCombinationsTest.cs +++ b/osu.Game.Tests/NonVisual/DifficultyAdjustmentModCombinationsTest.cs @@ -142,16 +142,16 @@ namespace osu.Game.Tests.NonVisual { Assert.AreEqual(expectedCombinations.Length, actualCombinations.Length); - foreach (Type[] expected in expectedCombinations) + Assert.Multiple(() => { - Type[] actualTypes = ModUtils.FlattenMods(actualCombinations).Select(m => m.GetType()).ToArray(); - - Assert.Multiple(() => + for (int i = 0; i < expectedCombinations.Length; ++i) { - foreach (var expectedType in expected) - Assert.Contains(expectedType, actualTypes); - }); - } + Type[] expectedTypes = expectedCombinations[i]; + Type[] actualTypes = ModUtils.FlattenMod(actualCombinations[i]).Select(m => m.GetType()).ToArray(); + + Assert.That(expectedTypes, Is.EquivalentTo(actualTypes)); + } + }); } private class ModA : Mod From 4306420922ef927a88101f1322b3cc5b181ce2ba Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 1 Dec 2021 15:09:51 +0900 Subject: [PATCH 55/65] Add extension methods to add extra safety to realm subscriptions Also adjusts the naming and documentation to make it (hopefully) easier to understand what this method/process implies. --- osu.Game.Tests/Database/GeneralUsageTests.cs | 6 +- osu.Game.Tests/Database/RealmLiveTests.cs | 2 +- osu.Game/Database/RealmObjectExtensions.cs | 106 ++++++++++++++++++ .../Bindings/DatabasedKeyBindingContainer.cs | 3 +- 4 files changed, 111 insertions(+), 6 deletions(-) diff --git a/osu.Game.Tests/Database/GeneralUsageTests.cs b/osu.Game.Tests/Database/GeneralUsageTests.cs index 841bf2de43..2285b22a3a 100644 --- a/osu.Game.Tests/Database/GeneralUsageTests.cs +++ b/osu.Game.Tests/Database/GeneralUsageTests.cs @@ -5,8 +5,8 @@ using System; using System.Threading; using System.Threading.Tasks; using NUnit.Framework; +using osu.Game.Database; using osu.Game.Models; -using Realms; #nullable enable @@ -48,7 +48,7 @@ namespace osu.Game.Tests.Database using (var context = realmFactory.CreateContext()) { - var subscription = context.All().SubscribeForNotifications((sender, changes, error) => + var subscription = context.All().QueryAsyncWithNotifications((sender, changes, error) => { using (realmFactory.CreateContext()) { @@ -61,7 +61,7 @@ namespace osu.Game.Tests.Database { } - subscription.Dispose(); + subscription?.Dispose(); } Assert.IsTrue(callbackRan); diff --git a/osu.Game.Tests/Database/RealmLiveTests.cs b/osu.Game.Tests/Database/RealmLiveTests.cs index 8ab19c8329..9b6769b788 100644 --- a/osu.Game.Tests/Database/RealmLiveTests.cs +++ b/osu.Game.Tests/Database/RealmLiveTests.cs @@ -208,7 +208,7 @@ namespace osu.Game.Tests.Database using (var updateThreadContext = realmFactory.CreateContext()) { - updateThreadContext.All().SubscribeForNotifications(gotChange); + updateThreadContext.All().QueryAsyncWithNotifications(gotChange); ILive? liveBeatmap = null; Task.Factory.StartNew(() => diff --git a/osu.Game/Database/RealmObjectExtensions.cs b/osu.Game/Database/RealmObjectExtensions.cs index ac4ca436ad..170756e743 100644 --- a/osu.Game/Database/RealmObjectExtensions.cs +++ b/osu.Game/Database/RealmObjectExtensions.cs @@ -1,12 +1,16 @@ // 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 AutoMapper; +using osu.Framework.Development; using osu.Game.Input.Bindings; using Realms; +#nullable enable + namespace osu.Game.Database { public static class RealmObjectExtensions @@ -60,5 +64,107 @@ namespace osu.Game.Database { return new RealmLive(realmObject); } + + /// + /// Register a callback to be invoked each time this changes. + /// + /// + /// + /// This adds osu! specific thread and managed state safety checks on top of . + /// + /// + /// The first callback will be invoked with the initial after the asynchronous query completes, + /// and then called again after each write transaction which changes either any of the objects in the collection, or + /// which objects are in the collection. The changes parameter will + /// be null the first time the callback is invoked with the initial results. For each call after that, + /// it will contain information about which rows in the results were added, removed or modified. + /// + /// + /// If a write transaction did not modify any objects in this , the callback is not invoked at all. + /// If an error occurs the callback will be invoked with null for the sender parameter and a non-null error. + /// Currently the only errors that can occur are when opening the on the background worker thread. + /// + /// + /// At the time when the block is called, the object will be fully evaluated + /// and up-to-date, and as long as you do not perform a write transaction on the same thread + /// or explicitly call , accessing it will never perform blocking work. + /// + /// + /// Notifications are delivered via the standard event loop, and so can't be delivered while the event loop is blocked by other activity. + /// When notifications can't be delivered instantly, multiple notifications may be coalesced into a single notification. + /// This can include the notification with the initial collection. + /// + /// + /// The to observe for changes. + /// 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 . + /// + /// + /// + 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."); + + return collection.SubscribeForNotifications(callback); + } + + /// + /// A convenience method that casts to and subscribes for change notifications. + /// + /// + /// This adds osu! specific thread and managed state safety checks on top of . + /// + /// 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 static IDisposable? QueryAsyncWithNotifications(this IQueryable list, NotificationCallbackDelegate callback) + where T : RealmObjectBase + { + // Subscribing to non-managed instances doesn't work. + // In this usage, the skin may be non-managed in tests. + if (!(list is IRealmCollection realmCollection)) + return null; + + return QueryAsyncWithNotifications(realmCollection, callback); + } + + /// + /// A convenience method that casts to and subscribes for change notifications. + /// + /// + /// This adds osu! specific thread and managed state safety checks on top of . + /// + /// 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 static IDisposable? QueryAsyncWithNotifications(this IList list, NotificationCallbackDelegate callback) + where T : RealmObjectBase + { + // Subscribing to non-managed instances doesn't work. + // In this usage, the skin may be non-managed in tests. + if (!(list is IRealmCollection realmCollection)) + return null; + + return QueryAsyncWithNotifications(realmCollection, callback); + } } } diff --git a/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs b/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs index baa5b9ff9c..f95c884fe5 100644 --- a/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs +++ b/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs @@ -8,7 +8,6 @@ using osu.Framework.Allocation; using osu.Framework.Input.Bindings; using osu.Game.Database; using osu.Game.Rulesets; -using Realms; namespace osu.Game.Input.Bindings { @@ -56,7 +55,7 @@ namespace osu.Game.Input.Bindings .Where(b => b.RulesetName == rulesetName && b.Variant == variant); realmSubscription = realmKeyBindings - .SubscribeForNotifications((sender, changes, error) => + .QueryAsyncWithNotifications((sender, changes, error) => { // first subscription ignored as we are handling this in LoadComplete. if (changes == null) From a439209535ec8a439aab350225b1e6640c6e3cc6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 1 Dec 2021 15:23:28 +0900 Subject: [PATCH 56/65] Add `BannedSymbols` rules for `SubscribeForNotifications` variants we use --- CodeAnalysis/BannedSymbols.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CodeAnalysis/BannedSymbols.txt b/CodeAnalysis/BannedSymbols.txt index b72803482d..c567adc0ae 100644 --- a/CodeAnalysis/BannedSymbols.txt +++ b/CodeAnalysis/BannedSymbols.txt @@ -10,3 +10,6 @@ T:Microsoft.EntityFrameworkCore.Internal.EnumerableExtensions;Don't use internal T:Microsoft.EntityFrameworkCore.Internal.TypeExtensions;Don't use internal extension methods. T:NuGet.Packaging.CollectionExtensions;Don't use internal extension methods. M:System.Enum.HasFlag(System.Enum);Use osu.Framework.Extensions.EnumExtensions.HasFlagFast() instead. +M:Realms.IRealmCollection`1.SubscribeForNotifications`1(Realms.NotificationCallbackDelegate{``0});Use osu.Game.Database.RealmObjectExtensions.QueryAsyncWithNotifications(IRealmCollection,NotificationCallbackDelegate) instead. +M:Realms.CollectionExtensions.SubscribeForNotifications`1(System.Linq.IQueryable{``0},Realms.NotificationCallbackDelegate{``0});Use osu.Game.Database.RealmObjectExtensions.QueryAsyncWithNotifications(IQueryable,NotificationCallbackDelegate) instead. +M:Realms.CollectionExtensions.SubscribeForNotifications`1(System.Collections.Generic.IList{``0},Realms.NotificationCallbackDelegate{``0});Use osu.Game.Database.RealmObjectExtensions.QueryAsyncWithNotifications(IList,NotificationCallbackDelegate) instead. From 09817ff161aa6768aa6fbf9828b62f572d075402 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 1 Dec 2021 15:27:10 +0900 Subject: [PATCH 57/65] Add missing `returns` additional documentation to main method call --- osu.Game/Database/RealmObjectExtensions.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game/Database/RealmObjectExtensions.cs b/osu.Game/Database/RealmObjectExtensions.cs index 170756e743..cb38c910da 100644 --- a/osu.Game/Database/RealmObjectExtensions.cs +++ b/osu.Game/Database/RealmObjectExtensions.cs @@ -100,6 +100,8 @@ namespace osu.Game.Database /// /// 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. /// /// /// From 81f82c24c39014decc49b676e68cca913db95645 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 1 Dec 2021 17:45:41 +0900 Subject: [PATCH 58/65] Use new API endpoint to do batch lookups --- osu.Game/Database/BeatmapLookupCache.cs | 19 ++++++++------- .../Online/API/Requests/GetBeatmapsRequest.cs | 24 +++++++++++++++++++ .../API/Requests/GetBeatmapsResponse.cs | 15 ++++++++++++ 3 files changed, 50 insertions(+), 8 deletions(-) create mode 100644 osu.Game/Online/API/Requests/GetBeatmapsRequest.cs create mode 100644 osu.Game/Online/API/Requests/GetBeatmapsResponse.cs diff --git a/osu.Game/Database/BeatmapLookupCache.cs b/osu.Game/Database/BeatmapLookupCache.cs index c4e20d59b6..2082031714 100644 --- a/osu.Game/Database/BeatmapLookupCache.cs +++ b/osu.Game/Database/BeatmapLookupCache.cs @@ -85,7 +85,7 @@ namespace osu.Game.Database // Grab at most 50 unique beatmap IDs from the queue. lock (taskAssignmentLock) { - while (pendingBeatmapTasks.Count > 0 && beatmapTasks.Count < 1) + while (pendingBeatmapTasks.Count > 0 && beatmapTasks.Count < 50) { (int id, TaskCompletionSource task) next = pendingBeatmapTasks.Dequeue(); @@ -103,7 +103,7 @@ namespace osu.Game.Database } // Query the beatmaps. - var request = new GetBeatmapRequest(new APIBeatmap { OnlineID = beatmapTasks.Keys.First() }); + var request = new GetBeatmapsRequest(beatmapTasks.Keys.ToArray()); // rather than queueing, we maintain our own single-threaded request stream. // todo: we probably want retry logic here. @@ -117,16 +117,19 @@ namespace osu.Game.Database createNewTask(); } - List foundBeatmaps = new List { request.Response }; + List foundBeatmaps = request.Response?.Beatmaps; - foreach (var beatmap in foundBeatmaps) + if (foundBeatmaps != null) { - if (beatmapTasks.TryGetValue(beatmap.OnlineID, out var tasks)) + foreach (var beatmap in foundBeatmaps) { - foreach (var task in tasks) - task.SetResult(beatmap); + if (beatmapTasks.TryGetValue(beatmap.OnlineID, out var tasks)) + { + foreach (var task in tasks) + task.SetResult(beatmap); - beatmapTasks.Remove(beatmap.OnlineID); + beatmapTasks.Remove(beatmap.OnlineID); + } } } diff --git a/osu.Game/Online/API/Requests/GetBeatmapsRequest.cs b/osu.Game/Online/API/Requests/GetBeatmapsRequest.cs new file mode 100644 index 0000000000..1d71e22b77 --- /dev/null +++ b/osu.Game/Online/API/Requests/GetBeatmapsRequest.cs @@ -0,0 +1,24 @@ +// 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.Online.API.Requests +{ + public class GetBeatmapsRequest : APIRequest + { + private readonly int[] beatmapIds; + + private const int max_ids_per_request = 50; + + public GetBeatmapsRequest(int[] beatmapIds) + { + if (beatmapIds.Length > max_ids_per_request) + throw new ArgumentException($"{nameof(GetBeatmapsRequest)} calls only support up to {max_ids_per_request} IDs at once"); + + this.beatmapIds = beatmapIds; + } + + protected override string Target => "beatmaps/?ids[]=" + string.Join("&ids[]=", beatmapIds); + } +} diff --git a/osu.Game/Online/API/Requests/GetBeatmapsResponse.cs b/osu.Game/Online/API/Requests/GetBeatmapsResponse.cs new file mode 100644 index 0000000000..c450c3269c --- /dev/null +++ b/osu.Game/Online/API/Requests/GetBeatmapsResponse.cs @@ -0,0 +1,15 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using Newtonsoft.Json; +using osu.Game.Online.API.Requests.Responses; + +namespace osu.Game.Online.API.Requests +{ + public class GetBeatmapsResponse : ResponseWithCursor + { + [JsonProperty("beatmaps")] + public List Beatmaps; + } +} From bf5a186a2b03774d7f51dd6a591a7d529894f534 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 1 Dec 2021 17:47:16 +0900 Subject: [PATCH 59/65] Add early abort to avoid sending empty lookup requests --- osu.Game/Database/BeatmapLookupCache.cs | 3 +++ osu.Game/Database/UserLookupCache.cs | 3 +++ 2 files changed, 6 insertions(+) diff --git a/osu.Game/Database/BeatmapLookupCache.cs b/osu.Game/Database/BeatmapLookupCache.cs index 2082031714..c6f8244494 100644 --- a/osu.Game/Database/BeatmapLookupCache.cs +++ b/osu.Game/Database/BeatmapLookupCache.cs @@ -102,6 +102,9 @@ namespace osu.Game.Database } } + if (beatmapTasks.Count == 0) + return; + // Query the beatmaps. var request = new GetBeatmapsRequest(beatmapTasks.Keys.ToArray()); diff --git a/osu.Game/Database/UserLookupCache.cs b/osu.Game/Database/UserLookupCache.cs index dae2d2549c..26f4e9fb3b 100644 --- a/osu.Game/Database/UserLookupCache.cs +++ b/osu.Game/Database/UserLookupCache.cs @@ -100,6 +100,9 @@ namespace osu.Game.Database } } + if (userTasks.Count == 0) + return; + // Query the users. var request = new GetUsersRequest(userTasks.Keys.ToArray()); From 7224f6bac54127ad9c4559b02fb92eeb9d20277a Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 1 Dec 2021 20:00:31 +0900 Subject: [PATCH 60/65] Fix testable online IDs starting at 0 --- osu.Game/Tests/Visual/OnlinePlay/TestRoomRequestsHandler.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Tests/Visual/OnlinePlay/TestRoomRequestsHandler.cs b/osu.Game/Tests/Visual/OnlinePlay/TestRoomRequestsHandler.cs index abcf31c007..a4586dea12 100644 --- a/osu.Game/Tests/Visual/OnlinePlay/TestRoomRequestsHandler.cs +++ b/osu.Game/Tests/Visual/OnlinePlay/TestRoomRequestsHandler.cs @@ -25,9 +25,9 @@ namespace osu.Game.Tests.Visual.OnlinePlay private readonly List serverSideRooms = new List(); - private int currentRoomId; - private int currentPlaylistItemId; - private int currentScoreId; + private int currentRoomId = 1; + private int currentPlaylistItemId = 1; + private int currentScoreId = 1; /// /// Handles an API request, while also updating the local state to match From 685bdd522e1111749243e95b5dca854bd7064b31 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 1 Dec 2021 20:17:26 +0900 Subject: [PATCH 61/65] Replace 'skin' in comments with 'instance' --- osu.Game/Database/RealmObjectExtensions.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Database/RealmObjectExtensions.cs b/osu.Game/Database/RealmObjectExtensions.cs index cb38c910da..b38e21453c 100644 --- a/osu.Game/Database/RealmObjectExtensions.cs +++ b/osu.Game/Database/RealmObjectExtensions.cs @@ -135,7 +135,7 @@ namespace osu.Game.Database where T : RealmObjectBase { // Subscribing to non-managed instances doesn't work. - // In this usage, the skin may be non-managed in tests. + // In this usage, the instance may be non-managed in tests. if (!(list is IRealmCollection realmCollection)) return null; @@ -162,7 +162,7 @@ namespace osu.Game.Database where T : RealmObjectBase { // Subscribing to non-managed instances doesn't work. - // In this usage, the skin may be non-managed in tests. + // In this usage, the instance may be non-managed in tests. if (!(list is IRealmCollection realmCollection)) return null; From 0adfb75cf36decb00fec3ac4189d38513b366408 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 2 Dec 2021 14:53:06 +0900 Subject: [PATCH 62/65] Combine similarly named `StatefulMultiplayerClient` tests --- .../StatefulMultiplayerClientTest.cs | 21 +++++++++++ .../StatefulMultiplayerClientTest.cs | 35 ------------------- 2 files changed, 21 insertions(+), 35 deletions(-) delete mode 100644 osu.Game.Tests/OnlinePlay/StatefulMultiplayerClientTest.cs diff --git a/osu.Game.Tests/NonVisual/Multiplayer/StatefulMultiplayerClientTest.cs b/osu.Game.Tests/NonVisual/Multiplayer/StatefulMultiplayerClientTest.cs index 840ff20a83..42305ccd81 100644 --- a/osu.Game.Tests/NonVisual/Multiplayer/StatefulMultiplayerClientTest.cs +++ b/osu.Game.Tests/NonVisual/Multiplayer/StatefulMultiplayerClientTest.cs @@ -16,6 +16,27 @@ namespace osu.Game.Tests.NonVisual.Multiplayer [HeadlessTest] public class StatefulMultiplayerClientTest : MultiplayerTestScene { + [Test] + public void TestUserAddedOnJoin() + { + var user = new APIUser { Id = 33 }; + + AddRepeatStep("add user multiple times", () => Client.AddUser(user), 3); + AddAssert("room has 2 users", () => Client.Room?.Users.Count == 2); + } + + [Test] + public void TestUserRemovedOnLeave() + { + var user = new APIUser { Id = 44 }; + + AddStep("add user", () => Client.AddUser(user)); + AddAssert("room has 2 users", () => Client.Room?.Users.Count == 2); + + AddRepeatStep("remove user multiple times", () => Client.RemoveUser(user), 3); + AddAssert("room has 1 user", () => Client.Room?.Users.Count == 1); + } + [Test] public void TestPlayingUserTracking() { diff --git a/osu.Game.Tests/OnlinePlay/StatefulMultiplayerClientTest.cs b/osu.Game.Tests/OnlinePlay/StatefulMultiplayerClientTest.cs deleted file mode 100644 index 5a621ecf84..0000000000 --- a/osu.Game.Tests/OnlinePlay/StatefulMultiplayerClientTest.cs +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -using NUnit.Framework; -using osu.Framework.Testing; -using osu.Game.Online.API.Requests.Responses; -using osu.Game.Tests.Visual.Multiplayer; - -namespace osu.Game.Tests.OnlinePlay -{ - [HeadlessTest] - public class StatefulMultiplayerClientTest : MultiplayerTestScene - { - [Test] - public void TestUserAddedOnJoin() - { - var user = new APIUser { Id = 33 }; - - AddRepeatStep("add user multiple times", () => Client.AddUser(user), 3); - AddAssert("room has 2 users", () => Client.Room?.Users.Count == 2); - } - - [Test] - public void TestUserRemovedOnLeave() - { - var user = new APIUser { Id = 44 }; - - AddStep("add user", () => Client.AddUser(user)); - AddAssert("room has 2 users", () => Client.Room?.Users.Count == 2); - - AddRepeatStep("remove user multiple times", () => Client.RemoveUser(user), 3); - AddAssert("room has 1 user", () => Client.Room?.Users.Count == 1); - } - } -} From abf7735b8403efb9e76e80b7d30ba49f4de8367d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 3 Dec 2021 14:18:03 +0900 Subject: [PATCH 63/65] Update framework --- osu.Android.props | 2 +- osu.Game/osu.Game.csproj | 2 +- osu.iOS.props | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Android.props b/osu.Android.props index eff0eed278..cd2710c3ba 100644 --- a/osu.Android.props +++ b/osu.Android.props @@ -52,7 +52,7 @@ - + diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index 7cc8893d8d..a4a4661822 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -36,7 +36,7 @@ runtime; build; native; contentfiles; analyzers; buildtransitive - + diff --git a/osu.iOS.props b/osu.iOS.props index 9c21f76617..c095d3ef74 100644 --- a/osu.iOS.props +++ b/osu.iOS.props @@ -60,7 +60,7 @@ - + @@ -83,7 +83,7 @@ - + From 614256697442c2c543d0120d1271be1264dfb858 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 3 Dec 2021 14:26:53 +0900 Subject: [PATCH 64/65] Update resources --- osu.Android.props | 2 +- osu.Game/osu.Game.csproj | 2 +- osu.iOS.props | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Android.props b/osu.Android.props index cd2710c3ba..17b5cb67e9 100644 --- a/osu.Android.props +++ b/osu.Android.props @@ -51,7 +51,7 @@ - + diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index a4a4661822..53a3337c9d 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -37,7 +37,7 @@ - + diff --git a/osu.iOS.props b/osu.iOS.props index c095d3ef74..f3dc163a67 100644 --- a/osu.iOS.props +++ b/osu.iOS.props @@ -61,7 +61,7 @@ - + From 9803e63e6f0c9a2a14f6427c9faafb629c5ef2ac Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 3 Dec 2021 14:30:15 +0900 Subject: [PATCH 65/65] Update IPC usage to return `null` --- osu.Game/IPC/ArchiveImportIPCChannel.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game/IPC/ArchiveImportIPCChannel.cs b/osu.Game/IPC/ArchiveImportIPCChannel.cs index d9d0e4c0ea..3ef1dc051d 100644 --- a/osu.Game/IPC/ArchiveImportIPCChannel.cs +++ b/osu.Game/IPC/ArchiveImportIPCChannel.cs @@ -25,6 +25,8 @@ namespace osu.Game.IPC { if (t.Exception != null) throw t.Exception; }, TaskContinuationOptions.OnlyOnFaulted); + + return null; }; }