From 680791301f70b8476bb40af982e6adba8254bcb5 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 10 Jun 2021 16:36:27 +0300 Subject: [PATCH] Consume new method rather than caching skin sources on top of `Player` --- .../TestSceneLegacyBeatmapSkin.cs | 7 +--- .../TestSceneSkinFallbacks.cs | 26 +----------- .../Tests/Beatmaps/HitObjectSampleTest.cs | 42 ++----------------- .../Beatmaps/LegacyBeatmapSkinColourTest.cs | 32 ++++---------- .../Tests/Visual/LegacySkinPlayerTestScene.cs | 17 +------- 5 files changed, 16 insertions(+), 108 deletions(-) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneLegacyBeatmapSkin.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneLegacyBeatmapSkin.cs index bc3daca16f..0077ff9e3c 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneLegacyBeatmapSkin.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneLegacyBeatmapSkin.cs @@ -101,15 +101,10 @@ public void TestBeatmapHyperDashColoursOverride(bool useBeatmapSkin) AddAssert("is custom hyper dash fruit colours", () => ((CatchExposedPlayer)TestPlayer).UsableHyperDashFruitColour == TestSkin.HYPER_DASH_FRUIT_COLOUR); } - protected override ExposedPlayer CreateTestPlayer(bool userHasCustomColours) => new CatchExposedPlayer(userHasCustomColours); + protected override ExposedPlayer CreateTestPlayer() => new CatchExposedPlayer(); private class CatchExposedPlayer : ExposedPlayer { - public CatchExposedPlayer(bool userHasCustomColours) - : base(userHasCustomColours) - { - } - public Color4 UsableHyperDashColour => GameplayClockContainer.ChildrenOfType() .First() diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSkinFallbacks.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSkinFallbacks.cs index fd523fffcb..2b45818aa9 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSkinFallbacks.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSkinFallbacks.cs @@ -21,7 +21,6 @@ using osu.Game.Rulesets.Osu.Objects.Drawables; using osu.Game.Skinning; using osu.Game.Storyboards; -using osu.Game.Tests.Visual; namespace osu.Game.Rulesets.Osu.Tests { @@ -99,7 +98,7 @@ private void checkNextHitObject(string skin) => [Resolved] private AudioManager audio { get; set; } - protected override TestPlayer CreatePlayer(Ruleset ruleset) => new SkinProvidingPlayer(testUserSkin); + protected override ISkin GetPlayerSkin() => testUserSkin; protected override WorkingBeatmap CreateWorkingBeatmap(IBeatmap beatmap, Storyboard storyboard = null) => new CustomSkinWorkingBeatmap(beatmap, storyboard, Clock, audio, testBeatmapSkin); @@ -116,27 +115,6 @@ public CustomSkinWorkingBeatmap(IBeatmap beatmap, Storyboard storyboard, IFrameB protected override ISkin GetSkin() => skin; } - public class SkinProvidingPlayer : TestPlayer - { - private readonly TestSource userSkin; - - public SkinProvidingPlayer(TestSource userSkin) - { - this.userSkin = userSkin; - } - - private DependencyContainer dependencies; - - protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) - { - dependencies = new DependencyContainer(base.CreateChildDependencies(parent)); - - dependencies.CacheAs(userSkin); - - return dependencies; - } - } - public class TestSource : ISkinSource { private readonly string identifier; @@ -164,8 +142,8 @@ public Drawable GetDrawableComponent(ISkinComponent component) public ISample GetSample(ISampleInfo sampleInfo) => null; - public TValue GetValue(Func query) where TConfiguration : SkinConfiguration => default; public IBindable GetConfig(TLookup lookup) => null; + public ISkin FindProvider(Func lookupFunction) => null; public event Action SourceChanged; diff --git a/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs b/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs index 7ee6c519b7..7af0397726 100644 --- a/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs +++ b/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs @@ -47,15 +47,11 @@ public abstract class HitObjectSampleTest : PlayerTestScene, IStorageResourcePro private readonly TestResourceStore userSkinResourceStore = new TestResourceStore(); private readonly TestResourceStore beatmapSkinResourceStore = new TestResourceStore(); - private SkinSourceDependencyContainer dependencies; private IBeatmap currentTestBeatmap; protected sealed override bool HasCustomSteps => true; protected override bool Autoplay => true; - protected sealed override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) - => new DependencyContainer(dependencies = new SkinSourceDependencyContainer(base.CreateChildDependencies(parent))); - protected sealed override IBeatmap CreateBeatmap(RulesetInfo ruleset) => currentTestBeatmap; protected sealed override WorkingBeatmap CreateWorkingBeatmap(IBeatmap beatmap, Storyboard storyboard = null) @@ -63,6 +59,8 @@ protected sealed override WorkingBeatmap CreateWorkingBeatmap(IBeatmap beatmap, protected override TestPlayer CreatePlayer(Ruleset ruleset) => new TestPlayer(false); + protected override ISkin GetPlayerSkin() => Skin; + protected void CreateTestWithBeatmap(string filename) { CreateTest(() => @@ -109,8 +107,7 @@ protected void SetupSkins(string beatmapFile, string userFile) } }; - // Need to refresh the cached skin source to refresh the skin resource store. - dependencies.SkinSource = new SkinProvidingContainer(Skin = new LegacySkin(userSkinInfo, this)); + Skin = new LegacySkin(userSkinInfo, this); }); } @@ -132,39 +129,6 @@ protected void AssertNoLookup(string name) => AddAssert($"\"{name}\" not looked #endregion - private class SkinSourceDependencyContainer : IReadOnlyDependencyContainer - { - public ISkinSource SkinSource; - - private readonly IReadOnlyDependencyContainer fallback; - - public SkinSourceDependencyContainer(IReadOnlyDependencyContainer fallback) - { - this.fallback = fallback; - } - - public object Get(Type type) - { - if (type == typeof(ISkinSource)) - return SkinSource; - - return fallback.Get(type); - } - - public object Get(Type type, CacheInfo info) - { - if (type == typeof(ISkinSource)) - return SkinSource; - - return fallback.Get(type, info); - } - - public void Inject(T instance) where T : class - { - // Never used directly - } - } - private class TestResourceStore : IResourceStore { public readonly List PerformedLookups = new List(); diff --git a/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs b/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs index 2540b6d7da..1feb3eebbf 100644 --- a/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs +++ b/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs @@ -4,13 +4,11 @@ using System; using System.Collections.Generic; using System.Linq; -using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Bindables; using osu.Framework.IO.Stores; using osu.Framework.Testing; using osu.Game.Beatmaps; -using osu.Game.Screens.Play; using osu.Game.Skinning; using osu.Game.Tests.Visual; using osuTK.Graphics; @@ -49,36 +47,24 @@ private void configureSettings(bool beatmapSkins, bool beatmapColours) protected virtual ExposedPlayer LoadBeatmap(bool userHasCustomColours) { - ExposedPlayer player; - Beatmap.Value = testBeatmap; - LoadScreen(player = CreateTestPlayer(userHasCustomColours)); + ExposedPlayer player = CreateTestPlayer(); + + player.Skin = new TestSkin(userHasCustomColours); + + LoadScreen(player); return player; } - protected virtual ExposedPlayer CreateTestPlayer(bool userHasCustomColours) => new ExposedPlayer(userHasCustomColours); + protected virtual ExposedPlayer CreateTestPlayer() => new ExposedPlayer(); - protected class ExposedPlayer : Player + protected class ExposedPlayer : TestPlayer { - protected readonly bool UserHasCustomColours; - - public ExposedPlayer(bool userHasCustomColours) - : base(new PlayerConfiguration - { - AllowPause = false, - ShowResults = false, - }) + public ExposedPlayer() + : base(false, false) { - UserHasCustomColours = userHasCustomColours; - } - - protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) - { - var dependencies = new DependencyContainer(base.CreateChildDependencies(parent)); - dependencies.CacheAs(new TestSkin(UserHasCustomColours)); - return dependencies; } public IReadOnlyList UsableComboColours => diff --git a/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs b/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs index b810bbf6ae..14a928d3c1 100644 --- a/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs +++ b/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs @@ -5,7 +5,6 @@ using osu.Framework.Allocation; using osu.Framework.Extensions.IEnumerableExtensions; using osu.Framework.Testing; -using osu.Game.Rulesets; using osu.Game.Skinning; namespace osu.Game.Tests.Visual @@ -15,15 +14,12 @@ public abstract class LegacySkinPlayerTestScene : PlayerTestScene { protected LegacySkin LegacySkin { get; private set; } - private ISkinSource legacySkinSource; - - protected override TestPlayer CreatePlayer(Ruleset ruleset) => new SkinProvidingPlayer(legacySkinSource); + protected override ISkin GetPlayerSkin() => LegacySkin; [BackgroundDependencyLoader] private void load(SkinManager skins) { LegacySkin = new DefaultLegacySkin(skins); - legacySkinSource = new SkinProvidingContainer(LegacySkin); } [SetUpSteps] @@ -48,16 +44,5 @@ private void addResetTargetsStep() t.Reload(); })); } - - public class SkinProvidingPlayer : TestPlayer - { - [Cached(typeof(ISkinSource))] - private readonly ISkinSource skinSource; - - public SkinProvidingPlayer(ISkinSource skinSource) - { - this.skinSource = skinSource; - } - } } }