From 0f65c4e960667785802e2ef9f102d683628da202 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 23 Dec 2021 18:59:02 +0100 Subject: [PATCH 1/5] Extract interface for ruleset config cache --- osu.Game/Rulesets/IRulesetConfigCache.cs | 23 +++++++++++++++++++++++ osu.Game/Rulesets/RulesetConfigCache.cs | 13 +------------ 2 files changed, 24 insertions(+), 12 deletions(-) create mode 100644 osu.Game/Rulesets/IRulesetConfigCache.cs diff --git a/osu.Game/Rulesets/IRulesetConfigCache.cs b/osu.Game/Rulesets/IRulesetConfigCache.cs new file mode 100644 index 0000000000..b946b43905 --- /dev/null +++ b/osu.Game/Rulesets/IRulesetConfigCache.cs @@ -0,0 +1,23 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +#nullable enable + +using osu.Game.Rulesets.Configuration; + +namespace osu.Game.Rulesets +{ + /// + /// A cache that provides a single per-ruleset. + /// This is done to support referring to and updating ruleset configs from multiple locations in the absence of inter-config bindings. + /// + public interface IRulesetConfigCache + { + /// + /// Retrieves the for a . + /// + /// The to retrieve the for. + /// The defined by , null if doesn't define one. + public IRulesetConfigManager? GetConfigFor(Ruleset ruleset); + } +} diff --git a/osu.Game/Rulesets/RulesetConfigCache.cs b/osu.Game/Rulesets/RulesetConfigCache.cs index 262340b4ee..3d7560a99a 100644 --- a/osu.Game/Rulesets/RulesetConfigCache.cs +++ b/osu.Game/Rulesets/RulesetConfigCache.cs @@ -1,7 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System; using System.Collections.Generic; using osu.Framework.Graphics; using osu.Game.Configuration; @@ -10,11 +9,7 @@ using osu.Game.Rulesets.Configuration; namespace osu.Game.Rulesets { - /// - /// A cache that provides a single per-ruleset. - /// This is done to support referring to and updating ruleset configs from multiple locations in the absence of inter-config bindings. - /// - public class RulesetConfigCache : Component + public class RulesetConfigCache : Component, IRulesetConfigCache { private readonly RealmContextFactory realmFactory; private readonly RulesetStore rulesets; @@ -43,12 +38,6 @@ namespace osu.Game.Rulesets } } - /// - /// Retrieves the for a . - /// - /// The to retrieve the for. - /// The defined by , null if doesn't define one. - /// If doesn't have a valid . public IRulesetConfigManager GetConfigFor(Ruleset ruleset) { if (!configCache.TryGetValue(ruleset.RulesetInfo.ShortName, out var config)) From 5cbaa028eb739696df5fa0d00e7724c65aea15e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 23 Dec 2021 19:02:10 +0100 Subject: [PATCH 2/5] Use extracted ruleset config cache implementation in DI --- osu.Game.Rulesets.Mania.Tests/Editor/TestSceneEditor.cs | 5 +++-- .../TestSceneTimingBasedNoteColouring.cs | 5 +++-- osu.Game.Rulesets.Osu.Tests/TestSceneSliderSnaking.cs | 5 +++-- osu.Game.Tests/Visual/Navigation/TestSceneOsuGame.cs | 2 +- osu.Game/OsuGameBase.cs | 2 +- osu.Game/Overlays/Settings/RulesetSettingsSubsection.cs | 2 +- osu.Game/Rulesets/Edit/HitObjectComposer.cs | 2 +- osu.Game/Rulesets/UI/DrawableRulesetDependencies.cs | 2 +- 8 files changed, 14 insertions(+), 11 deletions(-) diff --git a/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneEditor.cs b/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneEditor.cs index d3afbc63eb..f186537875 100644 --- a/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneEditor.cs +++ b/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneEditor.cs @@ -4,6 +4,7 @@ using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Bindables; +using osu.Framework.Extensions.ObjectExtensions; using osu.Game.Rulesets.Mania.Configuration; using osu.Game.Rulesets.Mania.UI; using osu.Game.Tests.Visual; @@ -24,9 +25,9 @@ namespace osu.Game.Rulesets.Mania.Tests.Editor } [BackgroundDependencyLoader] - private void load(RulesetConfigCache configCache) + private void load(IRulesetConfigCache configCache) { - var config = (ManiaRulesetConfigManager)configCache.GetConfigFor(Ruleset.Value.CreateInstance()); + var config = (ManiaRulesetConfigManager)configCache.GetConfigFor(Ruleset.Value.CreateInstance()).AsNonNull(); config.BindWith(ManiaRulesetSetting.ScrollDirection, direction); } } diff --git a/osu.Game.Rulesets.Mania.Tests/TestSceneTimingBasedNoteColouring.cs b/osu.Game.Rulesets.Mania.Tests/TestSceneTimingBasedNoteColouring.cs index 449a6ff23d..94b199c5d5 100644 --- a/osu.Game.Rulesets.Mania.Tests/TestSceneTimingBasedNoteColouring.cs +++ b/osu.Game.Rulesets.Mania.Tests/TestSceneTimingBasedNoteColouring.cs @@ -14,6 +14,7 @@ using osu.Game.Rulesets.Mania.Objects; using osu.Game.Rulesets.Mania.Beatmaps; using osu.Game.Rulesets.Mania.Configuration; using osu.Framework.Bindables; +using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Testing; using osu.Framework.Utils; using osu.Game.Rulesets.Mania.Objects.Drawables; @@ -25,7 +26,7 @@ namespace osu.Game.Rulesets.Mania.Tests public class TestSceneTimingBasedNoteColouring : OsuTestScene { [Resolved] - private RulesetConfigCache configCache { get; set; } + private IRulesetConfigCache configCache { get; set; } private Bindable configTimingBasedNoteColouring; @@ -48,7 +49,7 @@ namespace osu.Game.Rulesets.Mania.Tests }); AddStep("retrieve config bindable", () => { - var config = (ManiaRulesetConfigManager)configCache.GetConfigFor(Ruleset.Value.CreateInstance()); + var config = (ManiaRulesetConfigManager)configCache.GetConfigFor(Ruleset.Value.CreateInstance()).AsNonNull(); configTimingBasedNoteColouring = config.GetBindable(ManiaRulesetSetting.TimingBasedNoteColouring); }); } diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderSnaking.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderSnaking.cs index 3252e6d912..23d2d650bf 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderSnaking.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderSnaking.cs @@ -9,6 +9,7 @@ using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Bindables; +using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Graphics.Containers; using osu.Framework.Testing; using osu.Framework.Timing; @@ -46,9 +47,9 @@ namespace osu.Game.Rulesets.Osu.Tests => new ClockBackedTestWorkingBeatmap(this.beatmap = beatmap, storyboard, new FramedClock(new ManualClock { Rate = 1 }), audioManager); [BackgroundDependencyLoader] - private void load(RulesetConfigCache configCache) + private void load(IRulesetConfigCache configCache) { - var config = (OsuRulesetConfigManager)configCache.GetConfigFor(Ruleset.Value.CreateInstance()); + var config = (OsuRulesetConfigManager)configCache.GetConfigFor(Ruleset.Value.CreateInstance()).AsNonNull(); config.BindWith(OsuRulesetSetting.SnakingInSliders, snakingIn); config.BindWith(OsuRulesetSetting.SnakingOutSliders, snakingOut); } diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneOsuGame.cs b/osu.Game.Tests/Visual/Navigation/TestSceneOsuGame.cs index 4d1e279090..28ff776d5f 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneOsuGame.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneOsuGame.cs @@ -72,7 +72,7 @@ namespace osu.Game.Tests.Visual.Navigation typeof(FileStore), typeof(ScoreManager), typeof(BeatmapManager), - typeof(RulesetConfigCache), + typeof(IRulesetConfigCache), typeof(OsuColour), typeof(IBindable), typeof(Bindable), diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 2e266e32ff..9256514a0a 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -268,7 +268,7 @@ namespace osu.Game dependencies.Cache(scorePerformanceManager); AddInternal(scorePerformanceManager); - dependencies.Cache(rulesetConfigCache = new RulesetConfigCache(realmFactory, RulesetStore)); + dependencies.CacheAs(rulesetConfigCache = new RulesetConfigCache(realmFactory, RulesetStore)); var powerStatus = CreateBatteryInfo(); if (powerStatus != null) diff --git a/osu.Game/Overlays/Settings/RulesetSettingsSubsection.cs b/osu.Game/Overlays/Settings/RulesetSettingsSubsection.cs index 93b07fbac7..3945a410ab 100644 --- a/osu.Game/Overlays/Settings/RulesetSettingsSubsection.cs +++ b/osu.Game/Overlays/Settings/RulesetSettingsSubsection.cs @@ -29,7 +29,7 @@ namespace osu.Game.Overlays.Settings { dependencies = new DependencyContainer(base.CreateChildDependencies(parent)); - Config = dependencies.Get().GetConfigFor(ruleset); + Config = dependencies.Get().GetConfigFor(ruleset); if (Config != null) dependencies.Cache(Config); diff --git a/osu.Game/Rulesets/Edit/HitObjectComposer.cs b/osu.Game/Rulesets/Edit/HitObjectComposer.cs index 8bad9aa11f..4bbfe26d7b 100644 --- a/osu.Game/Rulesets/Edit/HitObjectComposer.cs +++ b/osu.Game/Rulesets/Edit/HitObjectComposer.cs @@ -80,7 +80,7 @@ namespace osu.Game.Rulesets.Edit [BackgroundDependencyLoader] private void load() { - Config = Dependencies.Get().GetConfigFor(Ruleset); + Config = Dependencies.Get().GetConfigFor(Ruleset); try { diff --git a/osu.Game/Rulesets/UI/DrawableRulesetDependencies.cs b/osu.Game/Rulesets/UI/DrawableRulesetDependencies.cs index aacb8dbe83..53996b6b84 100644 --- a/osu.Game/Rulesets/UI/DrawableRulesetDependencies.cs +++ b/osu.Game/Rulesets/UI/DrawableRulesetDependencies.cs @@ -64,7 +64,7 @@ namespace osu.Game.Rulesets.UI CacheAs(ShaderManager = new FallbackShaderManager(ShaderManager, parent.Get())); } - RulesetConfigManager = parent.Get().GetConfigFor(ruleset); + RulesetConfigManager = parent.Get().GetConfigFor(ruleset); if (RulesetConfigManager != null) Cache(RulesetConfigManager); } From 77da1e12d5be37a1eacd09182d2e7124c2a2eb90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 23 Dec 2021 19:18:26 +0100 Subject: [PATCH 3/5] Add test implementation of the ruleset config cache --- .../Tests/Rulesets/TestRulesetConfigCache.cs | 19 +++++++++++++++++++ osu.Game/Tests/Visual/OsuTestScene.cs | 8 ++++++++ 2 files changed, 27 insertions(+) create mode 100644 osu.Game/Tests/Rulesets/TestRulesetConfigCache.cs diff --git a/osu.Game/Tests/Rulesets/TestRulesetConfigCache.cs b/osu.Game/Tests/Rulesets/TestRulesetConfigCache.cs new file mode 100644 index 0000000000..537bee6824 --- /dev/null +++ b/osu.Game/Tests/Rulesets/TestRulesetConfigCache.cs @@ -0,0 +1,19 @@ +// 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.Concurrent; +using osu.Game.Rulesets; +using osu.Game.Rulesets.Configuration; + +namespace osu.Game.Tests.Rulesets +{ + /// + /// Test implementation of a , which ensures isolation between test scenes. + /// + public class TestRulesetConfigCache : IRulesetConfigCache + { + private readonly ConcurrentDictionary configCache = new ConcurrentDictionary(); + + public IRulesetConfigManager GetConfigFor(Ruleset ruleset) => configCache.GetOrAdd(ruleset.ShortName, _ => ruleset.CreateConfig(null)); + } +} diff --git a/osu.Game/Tests/Visual/OsuTestScene.cs b/osu.Game/Tests/Visual/OsuTestScene.cs index 1b2c1e72c4..df8c9f22db 100644 --- a/osu.Game/Tests/Visual/OsuTestScene.cs +++ b/osu.Game/Tests/Visual/OsuTestScene.cs @@ -31,6 +31,7 @@ using osu.Game.Rulesets.UI; using osu.Game.Screens; using osu.Game.Storyboards; using osu.Game.Tests.Beatmaps; +using osu.Game.Tests.Rulesets; namespace osu.Game.Tests.Visual { @@ -119,6 +120,13 @@ namespace osu.Game.Tests.Visual var baseDependencies = base.CreateChildDependencies(parent); + // to isolate ruleset configs in tests from the actual database and avoid state pollution problems, + // as well as problems due to the implementation details of the "real" implementation (the configs only being available at `LoadComplete()`), + // cache a test implementation of the ruleset config cache over the "real" one. + var isolatedBaseDependencies = new DependencyContainer(baseDependencies); + isolatedBaseDependencies.CacheAs(new TestRulesetConfigCache()); + baseDependencies = isolatedBaseDependencies; + var providedRuleset = CreateRuleset(); if (providedRuleset != null) baseDependencies = rulesetDependencies = new DrawableRulesetDependencies(providedRuleset, baseDependencies); From 8094b502cb104dad68cb7b1df77e7c1841307e21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 23 Dec 2021 19:27:27 +0100 Subject: [PATCH 4/5] Remove test-specific logic from `RulesetConfigCache` --- osu.Game/Rulesets/RulesetConfigCache.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/osu.Game/Rulesets/RulesetConfigCache.cs b/osu.Game/Rulesets/RulesetConfigCache.cs index 3d7560a99a..dee13e74a5 100644 --- a/osu.Game/Rulesets/RulesetConfigCache.cs +++ b/osu.Game/Rulesets/RulesetConfigCache.cs @@ -1,10 +1,12 @@ // 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 osu.Framework.Graphics; using osu.Game.Configuration; using osu.Game.Database; +using osu.Game.Extensions; using osu.Game.Rulesets.Configuration; namespace osu.Game.Rulesets @@ -40,10 +42,11 @@ namespace osu.Game.Rulesets public IRulesetConfigManager GetConfigFor(Ruleset ruleset) { + if (!IsLoaded) + throw new InvalidOperationException($@"Cannot retrieve {nameof(IRulesetConfigManager)} before {nameof(RulesetConfigCache)} has loaded"); + if (!configCache.TryGetValue(ruleset.RulesetInfo.ShortName, out var config)) - // any ruleset request which wasn't initialised on startup should not be stored to realm. - // this should only be used by tests. - return ruleset.CreateConfig(null); + throw new InvalidOperationException($@"Attempted to retrieve {nameof(IRulesetConfigManager)} for an unavailable ruleset {ruleset.GetDisplayString()}"); return config; } From c429c74d8984bc17001ad93d0521b0b662b4ff6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 23 Dec 2021 19:34:41 +0100 Subject: [PATCH 5/5] Expose and use test ruleset config cache in test scenes --- .../Editor/TestSceneEditor.cs | 4 ++-- .../TestSceneTimingBasedNoteColouring.cs | 6 +----- .../TestSceneSliderSnaking.cs | 4 ++-- osu.Game/Tests/Visual/OsuTestScene.cs | 12 +++++++++++- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneEditor.cs b/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneEditor.cs index f186537875..5300747633 100644 --- a/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneEditor.cs +++ b/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneEditor.cs @@ -25,9 +25,9 @@ namespace osu.Game.Rulesets.Mania.Tests.Editor } [BackgroundDependencyLoader] - private void load(IRulesetConfigCache configCache) + private void load() { - var config = (ManiaRulesetConfigManager)configCache.GetConfigFor(Ruleset.Value.CreateInstance()).AsNonNull(); + var config = (ManiaRulesetConfigManager)RulesetConfigs.GetConfigFor(Ruleset.Value.CreateInstance()).AsNonNull(); config.BindWith(ManiaRulesetSetting.ScrollDirection, direction); } } diff --git a/osu.Game.Rulesets.Mania.Tests/TestSceneTimingBasedNoteColouring.cs b/osu.Game.Rulesets.Mania.Tests/TestSceneTimingBasedNoteColouring.cs index 94b199c5d5..4c688520ef 100644 --- a/osu.Game.Rulesets.Mania.Tests/TestSceneTimingBasedNoteColouring.cs +++ b/osu.Game.Rulesets.Mania.Tests/TestSceneTimingBasedNoteColouring.cs @@ -4,7 +4,6 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Graphics; -using osu.Framework.Allocation; using osu.Framework.Graphics.Containers; using osu.Game.Beatmaps; using osu.Game.Beatmaps.ControlPoints; @@ -25,9 +24,6 @@ namespace osu.Game.Rulesets.Mania.Tests [TestFixture] public class TestSceneTimingBasedNoteColouring : OsuTestScene { - [Resolved] - private IRulesetConfigCache configCache { get; set; } - private Bindable configTimingBasedNoteColouring; private ManualClock clock; @@ -49,7 +45,7 @@ namespace osu.Game.Rulesets.Mania.Tests }); AddStep("retrieve config bindable", () => { - var config = (ManiaRulesetConfigManager)configCache.GetConfigFor(Ruleset.Value.CreateInstance()).AsNonNull(); + var config = (ManiaRulesetConfigManager)RulesetConfigs.GetConfigFor(Ruleset.Value.CreateInstance()).AsNonNull(); configTimingBasedNoteColouring = config.GetBindable(ManiaRulesetSetting.TimingBasedNoteColouring); }); } diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderSnaking.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderSnaking.cs index 23d2d650bf..a3aa84d0e7 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderSnaking.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderSnaking.cs @@ -47,9 +47,9 @@ namespace osu.Game.Rulesets.Osu.Tests => new ClockBackedTestWorkingBeatmap(this.beatmap = beatmap, storyboard, new FramedClock(new ManualClock { Rate = 1 }), audioManager); [BackgroundDependencyLoader] - private void load(IRulesetConfigCache configCache) + private void load() { - var config = (OsuRulesetConfigManager)configCache.GetConfigFor(Ruleset.Value.CreateInstance()).AsNonNull(); + var config = (OsuRulesetConfigManager)RulesetConfigs.GetConfigFor(Ruleset.Value.CreateInstance()).AsNonNull(); config.BindWith(OsuRulesetSetting.SnakingInSliders, snakingIn); config.BindWith(OsuRulesetSetting.SnakingOutSliders, snakingOut); } diff --git a/osu.Game/Tests/Visual/OsuTestScene.cs b/osu.Game/Tests/Visual/OsuTestScene.cs index df8c9f22db..6b029729ea 100644 --- a/osu.Game/Tests/Visual/OsuTestScene.cs +++ b/osu.Game/Tests/Visual/OsuTestScene.cs @@ -95,6 +95,16 @@ namespace osu.Game.Tests.Visual /// protected Storage LocalStorage => localStorage.Value; + /// + /// A cache for ruleset configurations to be used in this test scene. + /// + /// + /// This instance is provided to the children of this test scene via DI. + /// It is only exposed so that test scenes themselves can access the ruleset config cache in a safe manner + /// (s cannot use DI themselves, as they will end up accessing the real cached instance from ). + /// + protected IRulesetConfigCache RulesetConfigs { get; private set; } + private Lazy localStorage; private Storage headlessHostStorage; @@ -124,7 +134,7 @@ namespace osu.Game.Tests.Visual // as well as problems due to the implementation details of the "real" implementation (the configs only being available at `LoadComplete()`), // cache a test implementation of the ruleset config cache over the "real" one. var isolatedBaseDependencies = new DependencyContainer(baseDependencies); - isolatedBaseDependencies.CacheAs(new TestRulesetConfigCache()); + isolatedBaseDependencies.CacheAs(RulesetConfigs = new TestRulesetConfigCache()); baseDependencies = isolatedBaseDependencies; var providedRuleset = CreateRuleset();