1
0
mirror of https://github.com/ppy/osu.git synced 2024-11-11 12:57:36 +08:00

Merge pull request #16231 from bdach/ruleset-config-cache-overwriting-3

Fix ruleset config cache potentially not working correctly in test contexts
This commit is contained in:
Dean Herbert 2021-12-24 17:18:42 +09:00 committed by GitHub
commit 83a1d39f80
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 79 additions and 28 deletions

View File

@ -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()
{
var config = (ManiaRulesetConfigManager)configCache.GetConfigFor(Ruleset.Value.CreateInstance());
var config = (ManiaRulesetConfigManager)RulesetConfigs.GetConfigFor(Ruleset.Value.CreateInstance()).AsNonNull();
config.BindWith(ManiaRulesetSetting.ScrollDirection, direction);
}
}

View File

@ -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;
@ -14,6 +13,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;
@ -24,9 +24,6 @@ namespace osu.Game.Rulesets.Mania.Tests
[TestFixture]
public class TestSceneTimingBasedNoteColouring : OsuTestScene
{
[Resolved]
private RulesetConfigCache configCache { get; set; }
private Bindable<bool> configTimingBasedNoteColouring;
private ManualClock clock;
@ -48,7 +45,7 @@ namespace osu.Game.Rulesets.Mania.Tests
});
AddStep("retrieve config bindable", () =>
{
var config = (ManiaRulesetConfigManager)configCache.GetConfigFor(Ruleset.Value.CreateInstance());
var config = (ManiaRulesetConfigManager)RulesetConfigs.GetConfigFor(Ruleset.Value.CreateInstance()).AsNonNull();
configTimingBasedNoteColouring = config.GetBindable<bool>(ManiaRulesetSetting.TimingBasedNoteColouring);
});
}

View File

@ -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()
{
var config = (OsuRulesetConfigManager)configCache.GetConfigFor(Ruleset.Value.CreateInstance());
var config = (OsuRulesetConfigManager)RulesetConfigs.GetConfigFor(Ruleset.Value.CreateInstance()).AsNonNull();
config.BindWith(OsuRulesetSetting.SnakingInSliders, snakingIn);
config.BindWith(OsuRulesetSetting.SnakingOutSliders, snakingOut);
}

View File

@ -72,7 +72,7 @@ namespace osu.Game.Tests.Visual.Navigation
typeof(FileStore),
typeof(ScoreManager),
typeof(BeatmapManager),
typeof(RulesetConfigCache),
typeof(IRulesetConfigCache),
typeof(OsuColour),
typeof(IBindable<WorkingBeatmap>),
typeof(Bindable<WorkingBeatmap>),

View File

@ -268,7 +268,7 @@ namespace osu.Game
dependencies.Cache(scorePerformanceManager);
AddInternal(scorePerformanceManager);
dependencies.Cache(rulesetConfigCache = new RulesetConfigCache(realmFactory, RulesetStore));
dependencies.CacheAs<IRulesetConfigCache>(rulesetConfigCache = new RulesetConfigCache(realmFactory, RulesetStore));
var powerStatus = CreateBatteryInfo();
if (powerStatus != null)

View File

@ -29,7 +29,7 @@ namespace osu.Game.Overlays.Settings
{
dependencies = new DependencyContainer(base.CreateChildDependencies(parent));
Config = dependencies.Get<RulesetConfigCache>().GetConfigFor(ruleset);
Config = dependencies.Get<IRulesetConfigCache>().GetConfigFor(ruleset);
if (Config != null)
dependencies.Cache(Config);

View File

@ -80,7 +80,7 @@ namespace osu.Game.Rulesets.Edit
[BackgroundDependencyLoader]
private void load()
{
Config = Dependencies.Get<RulesetConfigCache>().GetConfigFor(Ruleset);
Config = Dependencies.Get<IRulesetConfigCache>().GetConfigFor(Ruleset);
try
{

View File

@ -0,0 +1,23 @@
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. 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
{
/// <summary>
/// A cache that provides a single <see cref="IRulesetConfigManager"/> per-ruleset.
/// This is done to support referring to and updating ruleset configs from multiple locations in the absence of inter-config bindings.
/// </summary>
public interface IRulesetConfigCache
{
/// <summary>
/// Retrieves the <see cref="IRulesetConfigManager"/> for a <see cref="Ruleset"/>.
/// </summary>
/// <param name="ruleset">The <see cref="Ruleset"/> to retrieve the <see cref="IRulesetConfigManager"/> for.</param>
/// <returns>The <see cref="IRulesetConfigManager"/> defined by <paramref name="ruleset"/>, null if <paramref name="ruleset"/> doesn't define one.</returns>
public IRulesetConfigManager? GetConfigFor(Ruleset ruleset);
}
}

View File

@ -6,15 +6,12 @@ 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
{
/// <summary>
/// A cache that provides a single <see cref="IRulesetConfigManager"/> per-ruleset.
/// This is done to support referring to and updating ruleset configs from multiple locations in the absence of inter-config bindings.
/// </summary>
public class RulesetConfigCache : Component
public class RulesetConfigCache : Component, IRulesetConfigCache
{
private readonly RealmContextFactory realmFactory;
private readonly RulesetStore rulesets;
@ -43,18 +40,13 @@ namespace osu.Game.Rulesets
}
}
/// <summary>
/// Retrieves the <see cref="IRulesetConfigManager"/> for a <see cref="Ruleset"/>.
/// </summary>
/// <param name="ruleset">The <see cref="Ruleset"/> to retrieve the <see cref="IRulesetConfigManager"/> for.</param>
/// <returns>The <see cref="IRulesetConfigManager"/> defined by <paramref name="ruleset"/>, null if <paramref name="ruleset"/> doesn't define one.</returns>
/// <exception cref="InvalidOperationException">If <paramref name="ruleset"/> doesn't have a valid <see cref="RulesetInfo.ID"/>.</exception>
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;
}

View File

@ -64,7 +64,7 @@ namespace osu.Game.Rulesets.UI
CacheAs(ShaderManager = new FallbackShaderManager(ShaderManager, parent.Get<ShaderManager>()));
}
RulesetConfigManager = parent.Get<RulesetConfigCache>().GetConfigFor(ruleset);
RulesetConfigManager = parent.Get<IRulesetConfigCache>().GetConfigFor(ruleset);
if (RulesetConfigManager != null)
Cache(RulesetConfigManager);
}

View File

@ -0,0 +1,19 @@
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. 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
{
/// <summary>
/// Test implementation of a <see cref="IRulesetConfigCache"/>, which ensures isolation between test scenes.
/// </summary>
public class TestRulesetConfigCache : IRulesetConfigCache
{
private readonly ConcurrentDictionary<string, IRulesetConfigManager> configCache = new ConcurrentDictionary<string, IRulesetConfigManager>();
public IRulesetConfigManager GetConfigFor(Ruleset ruleset) => configCache.GetOrAdd(ruleset.ShortName, _ => ruleset.CreateConfig(null));
}
}

View File

@ -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
{
@ -94,6 +95,16 @@ namespace osu.Game.Tests.Visual
/// </remarks>
protected Storage LocalStorage => localStorage.Value;
/// <summary>
/// A cache for ruleset configurations to be used in this test scene.
/// </summary>
/// <remarks>
/// This <see cref="IRulesetConfigCache"/> 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
/// (<see cref="OsuTestScene"/>s cannot use DI themselves, as they will end up accessing the real cached instance from <see cref="OsuGameBase"/>).
/// </remarks>
protected IRulesetConfigCache RulesetConfigs { get; private set; }
private Lazy<Storage> localStorage;
private Storage headlessHostStorage;
@ -119,6 +130,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(RulesetConfigs = new TestRulesetConfigCache());
baseDependencies = isolatedBaseDependencies;
var providedRuleset = CreateRuleset();
if (providedRuleset != null)
baseDependencies = rulesetDependencies = new DrawableRulesetDependencies(providedRuleset, baseDependencies);