From e6b2e3b0ed1f4059a9fef74053b7ed5d6ec39d9d Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 26 Mar 2020 05:18:12 +0300 Subject: [PATCH 01/24] Add osu!catch skin configurations --- .../Skinning/CatchSkinConfiguration.cs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 osu.Game.Rulesets.Catch/Skinning/CatchSkinConfiguration.cs diff --git a/osu.Game.Rulesets.Catch/Skinning/CatchSkinConfiguration.cs b/osu.Game.Rulesets.Catch/Skinning/CatchSkinConfiguration.cs new file mode 100644 index 0000000000..aea5beaa6b --- /dev/null +++ b/osu.Game.Rulesets.Catch/Skinning/CatchSkinConfiguration.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. + +namespace osu.Game.Rulesets.Catch.Skinning +{ + public enum CatchSkinConfiguration + { + /// + /// The colour to be used for the catcher while on hyper-dashing state. + /// + HyperDash, + + /// + /// The colour to be used for hyper-dash fruits. + /// + HyperDashFruit, + + /// + /// The colour to be used for the "exploding" catcher sprite on beginning of hyper-dashing. + /// + HyperDashAfterImage, + } +} From aa162b1033caff83366debb191f58366561b6555 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 26 Mar 2020 05:30:59 +0300 Subject: [PATCH 02/24] Setup hyper-dash colouring test scene --- .../TestSceneHyperDashColouring.cs | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs new file mode 100644 index 0000000000..2041e365ea --- /dev/null +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs @@ -0,0 +1,112 @@ +// 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.Linq; +using NUnit.Framework; +using osu.Framework.Audio.Sample; +using osu.Framework.Bindables; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Shapes; +using osu.Framework.Graphics.Sprites; +using osu.Framework.Graphics.Textures; +using osu.Framework.Testing; +using osu.Game.Audio; +using osu.Game.Beatmaps; +using osu.Game.Beatmaps.ControlPoints; +using osu.Game.Rulesets.Catch.Objects; +using osu.Game.Rulesets.Catch.Objects.Drawables; +using osu.Game.Rulesets.Catch.Skinning; +using osu.Game.Rulesets.Catch.UI; +using osu.Game.Skinning; +using osu.Game.Tests.Visual; +using osuTK; +using osuTK.Graphics; + +namespace osu.Game.Rulesets.Catch.Tests +{ + public class TestSceneHyperDashColouring : OsuTestScene + { + private Drawable setupSkinHierarchy(Func getChild, bool customHyperDashCatcherColour = false, bool customHyperDashFruitColour = false, bool customHyperDashAfterColour = false) + { + var testSkinProvider = new SkinProvidingContainer(new TestLegacySkin(customHyperDashCatcherColour, customHyperDashFruitColour, customHyperDashAfterColour)); + + var legacySkinTransformer = new SkinProvidingContainer(new CatchLegacySkinTransformer(testSkinProvider)); + + return testSkinProvider + .WithChild(legacySkinTransformer + .WithChild(getChild.Invoke())); + } + + private bool checkFruitHyperDashColour(DrawableFruit fruit, Color4 expectedColour, bool isLegacyFruit) => + isLegacyFruit + ? fruit.ChildrenOfType().First().Drawable.ChildrenOfType().Any(c => c.Colour == expectedColour) + : fruit.ChildrenOfType().First().Drawable.ChildrenOfType().Single(c => c.BorderColour == expectedColour).Any(d => d.Colour == expectedColour); + + private class TestLegacySkin : ISkin + { + public static Color4 CustomHyperDashColour { get; } = Color4.Goldenrod; + public static Color4 CustomHyperDashFruitColour { get; } = Color4.Cyan; + public static Color4 CustomHyperDashAfterColour { get; } = Color4.Lime; + + private readonly bool customHyperDashCatcherColour; + private readonly bool customHyperDashFruitColour; + private readonly bool customHyperDashAfterColour; + + public TestLegacySkin(bool customHyperDashCatcherColour = false, bool customHyperDashFruitColour = false, bool customHyperDashAfterColour = false) + { + this.customHyperDashCatcherColour = customHyperDashCatcherColour; + this.customHyperDashFruitColour = customHyperDashFruitColour; + this.customHyperDashAfterColour = customHyperDashAfterColour; + } + + public Drawable GetDrawableComponent(ISkinComponent component) => null; + + public Texture GetTexture(string componentName) + { + if (componentName == "fruit-pear") + { + // convince CatchLegacySkinTransformer to use the LegacyFruitPiece for pear fruit. + var texture = new Texture(Texture.WhitePixel.TextureGL) + { + Width = 1, + Height = 1, + ScaleAdjust = 1 / 96f + }; + return texture; + } + + return null; + } + + public SampleChannel GetSample(ISampleInfo sampleInfo) => null; + + public IBindable GetConfig(TLookup lookup) + { + switch (lookup) + { + case CatchSkinConfiguration config when config == CatchSkinConfiguration.HyperDash: + if (customHyperDashCatcherColour) + return SkinUtils.As(new Bindable(CustomHyperDashColour)); + + return null; + + case CatchSkinConfiguration config when config == CatchSkinConfiguration.HyperDashFruit: + if (customHyperDashFruitColour) + return SkinUtils.As(new Bindable(CustomHyperDashFruitColour)); + + return null; + + case CatchSkinConfiguration config when config == CatchSkinConfiguration.HyperDashAfterImage: + if (customHyperDashAfterColour) + return SkinUtils.As(new Bindable(CustomHyperDashAfterColour)); + + return null; + } + + return null; + } + } + } +} From 0a368f13d99421d17c34fa48a75db4001115c95a Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 26 Mar 2020 05:37:26 +0300 Subject: [PATCH 03/24] Add default hyper-dash colour constant on Catcher --- osu.Game.Rulesets.Catch/UI/Catcher.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game.Rulesets.Catch/UI/Catcher.cs b/osu.Game.Rulesets.Catch/UI/Catcher.cs index e361b29a9d..f53e14a8c7 100644 --- a/osu.Game.Rulesets.Catch/UI/Catcher.cs +++ b/osu.Game.Rulesets.Catch/UI/Catcher.cs @@ -21,6 +21,8 @@ namespace osu.Game.Rulesets.Catch.UI { public class Catcher : Container, IKeyBindingHandler { + public static Color4 DefaultHyperDashColour { get; } = Color4.Red; + /// /// Whether we are hyper-dashing or not. /// From 6f2cc5471adabc4392fcf1f63a5de32266016c10 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 26 Mar 2020 05:38:41 +0300 Subject: [PATCH 04/24] Add support for custom hyper-dash fruit colouring --- .../Objects/Drawables/FruitPiece.cs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Objects/Drawables/FruitPiece.cs b/osu.Game.Rulesets.Catch/Objects/Drawables/FruitPiece.cs index 5797588ded..c8f7c4912e 100644 --- a/osu.Game.Rulesets.Catch/Objects/Drawables/FruitPiece.cs +++ b/osu.Game.Rulesets.Catch/Objects/Drawables/FruitPiece.cs @@ -7,7 +7,10 @@ using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Shapes; +using osu.Game.Rulesets.Catch.Skinning; +using osu.Game.Rulesets.Catch.UI; using osu.Game.Rulesets.Objects.Drawables; +using osu.Game.Skinning; using osuTK.Graphics; namespace osu.Game.Rulesets.Catch.Objects.Drawables @@ -31,7 +34,7 @@ namespace osu.Game.Rulesets.Catch.Objects.Drawables } [BackgroundDependencyLoader] - private void load(DrawableHitObject drawableObject) + private void load(DrawableHitObject drawableObject, ISkinSource skin) { DrawableCatchHitObject drawableCatchObject = (DrawableCatchHitObject)drawableObject; hitObject = drawableCatchObject.HitObject; @@ -60,6 +63,11 @@ namespace osu.Game.Rulesets.Catch.Objects.Drawables }, }); + var hyperDashColour = + skin.GetConfig(CatchSkinConfiguration.HyperDashFruit)?.Value ?? + skin.GetConfig(CatchSkinConfiguration.HyperDash)?.Value ?? + Catcher.DefaultHyperDashColour; + if (hitObject.HyperDash) { AddInternal(new Circle @@ -67,7 +75,7 @@ namespace osu.Game.Rulesets.Catch.Objects.Drawables RelativeSizeAxes = Axes.Both, Anchor = Anchor.Centre, Origin = Anchor.Centre, - BorderColour = Color4.Red, + BorderColour = hyperDashColour, BorderThickness = 12f * RADIUS_ADJUST, Children = new Drawable[] { @@ -77,7 +85,7 @@ namespace osu.Game.Rulesets.Catch.Objects.Drawables Alpha = 0.3f, Blending = BlendingParameters.Additive, RelativeSizeAxes = Axes.Both, - Colour = Color4.Red, + Colour = hyperDashColour, } } }); From d995f3e1cc7eff1604d4fa06ed4f85de7152f020 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 26 Mar 2020 05:39:32 +0300 Subject: [PATCH 05/24] Add support for custom hyper-dash legacy fruit colouring --- osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs b/osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs index 25ee0811d0..99ecf12fd3 100644 --- a/osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs +++ b/osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs @@ -7,6 +7,7 @@ using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Sprites; using osu.Game.Rulesets.Catch.Objects.Drawables; +using osu.Game.Rulesets.Catch.UI; using osu.Game.Rulesets.Objects.Drawables; using osu.Game.Skinning; using osuTK; @@ -53,10 +54,15 @@ namespace osu.Game.Rulesets.Catch.Skinning if (drawableCatchObject.HitObject.HyperDash) { + var hyperDashColour = + skin.GetConfig(CatchSkinConfiguration.HyperDashFruit)?.Value ?? + skin.GetConfig(CatchSkinConfiguration.HyperDash)?.Value ?? + Catcher.DefaultHyperDashColour; + var hyperDash = new Sprite { Texture = skin.GetTexture(lookupName), - Colour = Color4.Red, + Colour = hyperDashColour, Anchor = Anchor.Centre, Origin = Anchor.Centre, Blending = BlendingParameters.Additive, From 29274b004cfa1141d3a4c85ec97e8960ccdeca48 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 26 Mar 2020 05:40:38 +0300 Subject: [PATCH 06/24] Add hyper-dash fruit colouring test cases --- .../TestSceneHyperDashColouring.cs | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs index 2041e365ea..7fab961aa7 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs @@ -28,6 +28,77 @@ namespace osu.Game.Rulesets.Catch.Tests { public class TestSceneHyperDashColouring : OsuTestScene { + [TestCase(false)] + [TestCase(true)] + public void TestHyperDashFruitColour(bool legacyFruit) + { + DrawableFruit drawableFruit = null; + + AddStep("setup fruit", () => + { + var fruit = new Fruit { IndexInBeatmap = legacyFruit ? 0 : 1, HyperDashTarget = new Banana() }; + fruit.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty()); + + Child = setupSkinHierarchy(() => + drawableFruit = new DrawableFruit(fruit) + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Scale = new Vector2(4f), + }, false, false); + }); + + AddAssert("fruit colour default-hyperdash", () => checkFruitHyperDashColour(drawableFruit, Catcher.DefaultHyperDashColour, legacyFruit)); + } + + [TestCase(false, true)] + [TestCase(false, false)] + [TestCase(true, true)] + [TestCase(true, false)] + public void TestCustomHyperDashFruitColour(bool legacyFruit, bool customCatcherHyperDashColour) + { + DrawableFruit drawableFruit = null; + + AddStep("setup fruit", () => + { + var fruit = new Fruit { IndexInBeatmap = legacyFruit ? 0 : 1, HyperDashTarget = new Banana() }; + fruit.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty()); + + Child = setupSkinHierarchy(() => + drawableFruit = new DrawableFruit(fruit) + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Scale = new Vector2(4f), + }, customCatcherHyperDashColour, true); + }); + + AddAssert("fruit colour custom-hyperdash", () => checkFruitHyperDashColour(drawableFruit, TestLegacySkin.CustomHyperDashFruitColour, legacyFruit)); + } + + [TestCase(false)] + [TestCase(true)] + public void TestCustomHyperDashFruitColourFallback(bool legacyFruit) + { + DrawableFruit drawableFruit = null; + + AddStep("setup fruit", () => + { + var fruit = new Fruit { IndexInBeatmap = legacyFruit ? 0 : 1, HyperDashTarget = new Banana() }; + fruit.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty()); + + Child = setupSkinHierarchy(() => + drawableFruit = new DrawableFruit(fruit) + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Scale = new Vector2(4f), + }, true, false); + }); + + AddAssert("fruit colour catcher-custom-hyperdash", () => checkFruitHyperDashColour(drawableFruit, TestLegacySkin.CustomHyperDashColour, legacyFruit)); + } + private Drawable setupSkinHierarchy(Func getChild, bool customHyperDashCatcherColour = false, bool customHyperDashFruitColour = false, bool customHyperDashAfterColour = false) { var testSkinProvider = new SkinProvidingContainer(new TestLegacySkin(customHyperDashCatcherColour, customHyperDashFruitColour, customHyperDashAfterColour)); From 45eb03bfe2adc868729915defc057b09e5fb7f90 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 28 Mar 2020 07:43:47 +0300 Subject: [PATCH 07/24] Apply review suggestions --- .../TestSceneHyperDashColouring.cs | 32 +++++++------------ 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs index 7fab961aa7..9ab8cf9113 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs @@ -48,7 +48,7 @@ namespace osu.Game.Rulesets.Catch.Tests }, false, false); }); - AddAssert("fruit colour default-hyperdash", () => checkFruitHyperDashColour(drawableFruit, Catcher.DefaultHyperDashColour, legacyFruit)); + AddAssert("default colour", () => checkFruitHyperDashColour(drawableFruit, Catcher.DefaultHyperDashColour, legacyFruit)); } [TestCase(false, true)] @@ -73,7 +73,7 @@ namespace osu.Game.Rulesets.Catch.Tests }, customCatcherHyperDashColour, true); }); - AddAssert("fruit colour custom-hyperdash", () => checkFruitHyperDashColour(drawableFruit, TestLegacySkin.CustomHyperDashFruitColour, legacyFruit)); + AddAssert("custom colour", () => checkFruitHyperDashColour(drawableFruit, TestLegacySkin.CustomHyperDashFruitColour, legacyFruit)); } [TestCase(false)] @@ -96,7 +96,7 @@ namespace osu.Game.Rulesets.Catch.Tests }, true, false); }); - AddAssert("fruit colour catcher-custom-hyperdash", () => checkFruitHyperDashColour(drawableFruit, TestLegacySkin.CustomHyperDashColour, legacyFruit)); + AddAssert("catcher custom colour", () => checkFruitHyperDashColour(drawableFruit, TestLegacySkin.CustomHyperDashColour, legacyFruit)); } private Drawable setupSkinHierarchy(Func getChild, bool customHyperDashCatcherColour = false, bool customHyperDashFruitColour = false, bool customHyperDashAfterColour = false) @@ -139,13 +139,12 @@ namespace osu.Game.Rulesets.Catch.Tests if (componentName == "fruit-pear") { // convince CatchLegacySkinTransformer to use the LegacyFruitPiece for pear fruit. - var texture = new Texture(Texture.WhitePixel.TextureGL) + return new Texture(Texture.WhitePixel.TextureGL) { Width = 1, Height = 1, ScaleAdjust = 1 / 96f }; - return texture; } return null; @@ -155,25 +154,16 @@ namespace osu.Game.Rulesets.Catch.Tests public IBindable GetConfig(TLookup lookup) { - switch (lookup) + if (lookup is CatchSkinConfiguration config) { - case CatchSkinConfiguration config when config == CatchSkinConfiguration.HyperDash: - if (customHyperDashCatcherColour) - return SkinUtils.As(new Bindable(CustomHyperDashColour)); + if (config == CatchSkinConfiguration.HyperDash && customHyperDashCatcherColour) + return SkinUtils.As(new Bindable(CustomHyperDashColour)); - return null; + if (config == CatchSkinConfiguration.HyperDashFruit && customHyperDashFruitColour) + return SkinUtils.As(new Bindable(CustomHyperDashFruitColour)); - case CatchSkinConfiguration config when config == CatchSkinConfiguration.HyperDashFruit: - if (customHyperDashFruitColour) - return SkinUtils.As(new Bindable(CustomHyperDashFruitColour)); - - return null; - - case CatchSkinConfiguration config when config == CatchSkinConfiguration.HyperDashAfterImage: - if (customHyperDashAfterColour) - return SkinUtils.As(new Bindable(CustomHyperDashAfterColour)); - - return null; + if (config == CatchSkinConfiguration.HyperDashAfterImage && customHyperDashAfterColour) + return SkinUtils.As(new Bindable(CustomHyperDashAfterColour)); } return null; From e51097da9e7ee6f6128fd5e9b19e23117a85904b Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 30 Mar 2020 09:29:00 +0300 Subject: [PATCH 08/24] Add a legacy skin provider above the test skin --- .../TestSceneHyperDashColouring.cs | 122 +++++++++--------- 1 file changed, 63 insertions(+), 59 deletions(-) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs index 9ab8cf9113..fea2939eae 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs @@ -1,9 +1,9 @@ // 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.Linq; using NUnit.Framework; +using osu.Framework.Allocation; using osu.Framework.Audio.Sample; using osu.Framework.Bindables; using osu.Framework.Graphics; @@ -28,6 +28,9 @@ namespace osu.Game.Rulesets.Catch.Tests { public class TestSceneHyperDashColouring : OsuTestScene { + [Resolved] + private SkinManager skins { get; set; } + [TestCase(false)] [TestCase(true)] public void TestHyperDashFruitColour(bool legacyFruit) @@ -36,19 +39,21 @@ namespace osu.Game.Rulesets.Catch.Tests AddStep("setup fruit", () => { - var fruit = new Fruit { IndexInBeatmap = legacyFruit ? 0 : 1, HyperDashTarget = new Banana() }; + var fruit = new Fruit { HyperDashTarget = new Banana() }; fruit.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty()); - Child = setupSkinHierarchy(() => - drawableFruit = new DrawableFruit(fruit) - { - Anchor = Anchor.Centre, - Origin = Anchor.Centre, - Scale = new Vector2(4f), - }, false, false); + Child = setupSkinHierarchy(drawableFruit = new DrawableFruit(fruit) + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Scale = new Vector2(4f), + }, false, false, false, legacyFruit); }); - AddAssert("default colour", () => checkFruitHyperDashColour(drawableFruit, Catcher.DefaultHyperDashColour, legacyFruit)); + AddAssert("default colour", () => + legacyFruit + ? checkLegacyFruitHyperDashColour(drawableFruit, Catcher.DefaultHyperDashColour) + : checkFruitHyperDashColour(drawableFruit, Catcher.DefaultHyperDashColour)); } [TestCase(false, true)] @@ -61,19 +66,21 @@ namespace osu.Game.Rulesets.Catch.Tests AddStep("setup fruit", () => { - var fruit = new Fruit { IndexInBeatmap = legacyFruit ? 0 : 1, HyperDashTarget = new Banana() }; + var fruit = new Fruit { HyperDashTarget = new Banana() }; fruit.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty()); - Child = setupSkinHierarchy(() => - drawableFruit = new DrawableFruit(fruit) - { - Anchor = Anchor.Centre, - Origin = Anchor.Centre, - Scale = new Vector2(4f), - }, customCatcherHyperDashColour, true); + Child = setupSkinHierarchy(drawableFruit = new DrawableFruit(fruit) + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Scale = new Vector2(4f), + }, customCatcherHyperDashColour, false, true, legacyFruit); }); - AddAssert("custom colour", () => checkFruitHyperDashColour(drawableFruit, TestLegacySkin.CustomHyperDashFruitColour, legacyFruit)); + AddAssert("custom colour", () => + legacyFruit + ? checkLegacyFruitHyperDashColour(drawableFruit, TestSkin.CustomHyperDashFruitColour) + : checkFruitHyperDashColour(drawableFruit, TestSkin.CustomHyperDashFruitColour)); } [TestCase(false)] @@ -84,71 +91,68 @@ namespace osu.Game.Rulesets.Catch.Tests AddStep("setup fruit", () => { - var fruit = new Fruit { IndexInBeatmap = legacyFruit ? 0 : 1, HyperDashTarget = new Banana() }; + var fruit = new Fruit { HyperDashTarget = new Banana() }; fruit.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty()); - Child = setupSkinHierarchy(() => + Child = setupSkinHierarchy( drawableFruit = new DrawableFruit(fruit) { Anchor = Anchor.Centre, Origin = Anchor.Centre, Scale = new Vector2(4f), - }, true, false); + }, true, false, false, legacyFruit); }); - AddAssert("catcher custom colour", () => checkFruitHyperDashColour(drawableFruit, TestLegacySkin.CustomHyperDashColour, legacyFruit)); + AddAssert("catcher custom colour", () => + legacyFruit + ? checkLegacyFruitHyperDashColour(drawableFruit, TestSkin.CustomHyperDashColour) + : checkFruitHyperDashColour(drawableFruit, TestSkin.CustomHyperDashColour)); } - private Drawable setupSkinHierarchy(Func getChild, bool customHyperDashCatcherColour = false, bool customHyperDashFruitColour = false, bool customHyperDashAfterColour = false) + private Drawable setupSkinHierarchy(Drawable child, bool customCatcherColour = false, bool customAfterColour = false, bool customFruitColour = false, bool legacySkin = true) { - var testSkinProvider = new SkinProvidingContainer(new TestLegacySkin(customHyperDashCatcherColour, customHyperDashFruitColour, customHyperDashAfterColour)); + var testSkinProvider = new SkinProvidingContainer(new TestSkin(customCatcherColour, customAfterColour, customFruitColour)); - var legacySkinTransformer = new SkinProvidingContainer(new CatchLegacySkinTransformer(testSkinProvider)); + if (legacySkin) + { + var legacySkinProvider = new SkinProvidingContainer(skins.GetSkin(DefaultLegacySkin.Info)); + var legacySkinTransformer = new SkinProvidingContainer(new CatchLegacySkinTransformer(testSkinProvider)); - return testSkinProvider - .WithChild(legacySkinTransformer - .WithChild(getChild.Invoke())); + return legacySkinProvider + .WithChild(testSkinProvider + .WithChild(legacySkinTransformer + .WithChild(child))); + } + + return testSkinProvider.WithChild(child); } - private bool checkFruitHyperDashColour(DrawableFruit fruit, Color4 expectedColour, bool isLegacyFruit) => - isLegacyFruit - ? fruit.ChildrenOfType().First().Drawable.ChildrenOfType().Any(c => c.Colour == expectedColour) - : fruit.ChildrenOfType().First().Drawable.ChildrenOfType().Single(c => c.BorderColour == expectedColour).Any(d => d.Colour == expectedColour); + private bool checkFruitHyperDashColour(DrawableFruit fruit, Color4 expectedColour) => + fruit.ChildrenOfType().First().Drawable.ChildrenOfType().Single(c => c.BorderColour == expectedColour).Any(d => d.Colour == expectedColour); - private class TestLegacySkin : ISkin + private bool checkLegacyFruitHyperDashColour(DrawableFruit fruit, Color4 expectedColour) => + fruit.ChildrenOfType().First().Drawable.ChildrenOfType().Any(c => c.Colour == expectedColour); + + private class TestSkin : ISkin { public static Color4 CustomHyperDashColour { get; } = Color4.Goldenrod; public static Color4 CustomHyperDashFruitColour { get; } = Color4.Cyan; public static Color4 CustomHyperDashAfterColour { get; } = Color4.Lime; - private readonly bool customHyperDashCatcherColour; - private readonly bool customHyperDashFruitColour; - private readonly bool customHyperDashAfterColour; + private readonly bool customCatcherColour; + private readonly bool customAfterColour; + private readonly bool customFruitColour; - public TestLegacySkin(bool customHyperDashCatcherColour = false, bool customHyperDashFruitColour = false, bool customHyperDashAfterColour = false) + public TestSkin(bool customCatcherColour = false, bool customAfterColour = false, bool customFruitColour = false) { - this.customHyperDashCatcherColour = customHyperDashCatcherColour; - this.customHyperDashFruitColour = customHyperDashFruitColour; - this.customHyperDashAfterColour = customHyperDashAfterColour; + this.customCatcherColour = customCatcherColour; + this.customAfterColour = customAfterColour; + this.customFruitColour = customFruitColour; } public Drawable GetDrawableComponent(ISkinComponent component) => null; - public Texture GetTexture(string componentName) - { - if (componentName == "fruit-pear") - { - // convince CatchLegacySkinTransformer to use the LegacyFruitPiece for pear fruit. - return new Texture(Texture.WhitePixel.TextureGL) - { - Width = 1, - Height = 1, - ScaleAdjust = 1 / 96f - }; - } - - return null; - } + public Texture GetTexture(string componentName) => null; public SampleChannel GetSample(ISampleInfo sampleInfo) => null; @@ -156,13 +160,13 @@ namespace osu.Game.Rulesets.Catch.Tests { if (lookup is CatchSkinConfiguration config) { - if (config == CatchSkinConfiguration.HyperDash && customHyperDashCatcherColour) + if (config == CatchSkinConfiguration.HyperDash && customCatcherColour) return SkinUtils.As(new Bindable(CustomHyperDashColour)); - if (config == CatchSkinConfiguration.HyperDashFruit && customHyperDashFruitColour) + if (config == CatchSkinConfiguration.HyperDashFruit && customFruitColour) return SkinUtils.As(new Bindable(CustomHyperDashFruitColour)); - if (config == CatchSkinConfiguration.HyperDashAfterImage && customHyperDashAfterColour) + if (config == CatchSkinConfiguration.HyperDashAfterImage && customAfterColour) return SkinUtils.As(new Bindable(CustomHyperDashAfterColour)); } From 16a4525a9cdbe35aecc28579937a0606e919f5de Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 30 Mar 2020 09:33:47 +0300 Subject: [PATCH 09/24] CatchSkinConfiguration -> CatchSkinColour --- .../TestSceneHyperDashColouring.cs | 8 ++++---- osu.Game.Rulesets.Catch/Objects/Drawables/FruitPiece.cs | 4 ++-- .../{CatchSkinConfiguration.cs => CatchSkinColour.cs} | 2 +- osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) rename osu.Game.Rulesets.Catch/Skinning/{CatchSkinConfiguration.cs => CatchSkinColour.cs} (94%) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs index fea2939eae..ebc3d3bff1 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs @@ -158,15 +158,15 @@ namespace osu.Game.Rulesets.Catch.Tests public IBindable GetConfig(TLookup lookup) { - if (lookup is CatchSkinConfiguration config) + if (lookup is CatchSkinColour config) { - if (config == CatchSkinConfiguration.HyperDash && customCatcherColour) + if (config == CatchSkinColour.HyperDash && customCatcherColour) return SkinUtils.As(new Bindable(CustomHyperDashColour)); - if (config == CatchSkinConfiguration.HyperDashFruit && customFruitColour) + if (config == CatchSkinColour.HyperDashFruit && customFruitColour) return SkinUtils.As(new Bindable(CustomHyperDashFruitColour)); - if (config == CatchSkinConfiguration.HyperDashAfterImage && customAfterColour) + if (config == CatchSkinColour.HyperDashAfterImage && customAfterColour) return SkinUtils.As(new Bindable(CustomHyperDashAfterColour)); } diff --git a/osu.Game.Rulesets.Catch/Objects/Drawables/FruitPiece.cs b/osu.Game.Rulesets.Catch/Objects/Drawables/FruitPiece.cs index c8f7c4912e..16818746b5 100644 --- a/osu.Game.Rulesets.Catch/Objects/Drawables/FruitPiece.cs +++ b/osu.Game.Rulesets.Catch/Objects/Drawables/FruitPiece.cs @@ -64,8 +64,8 @@ namespace osu.Game.Rulesets.Catch.Objects.Drawables }); var hyperDashColour = - skin.GetConfig(CatchSkinConfiguration.HyperDashFruit)?.Value ?? - skin.GetConfig(CatchSkinConfiguration.HyperDash)?.Value ?? + skin.GetConfig(CatchSkinColour.HyperDashFruit)?.Value ?? + skin.GetConfig(CatchSkinColour.HyperDash)?.Value ?? Catcher.DefaultHyperDashColour; if (hitObject.HyperDash) diff --git a/osu.Game.Rulesets.Catch/Skinning/CatchSkinConfiguration.cs b/osu.Game.Rulesets.Catch/Skinning/CatchSkinColour.cs similarity index 94% rename from osu.Game.Rulesets.Catch/Skinning/CatchSkinConfiguration.cs rename to osu.Game.Rulesets.Catch/Skinning/CatchSkinColour.cs index aea5beaa6b..2ad8f89739 100644 --- a/osu.Game.Rulesets.Catch/Skinning/CatchSkinConfiguration.cs +++ b/osu.Game.Rulesets.Catch/Skinning/CatchSkinColour.cs @@ -3,7 +3,7 @@ namespace osu.Game.Rulesets.Catch.Skinning { - public enum CatchSkinConfiguration + public enum CatchSkinColour { /// /// The colour to be used for the catcher while on hyper-dashing state. diff --git a/osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs b/osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs index 99ecf12fd3..5235058c52 100644 --- a/osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs +++ b/osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs @@ -55,8 +55,8 @@ namespace osu.Game.Rulesets.Catch.Skinning if (drawableCatchObject.HitObject.HyperDash) { var hyperDashColour = - skin.GetConfig(CatchSkinConfiguration.HyperDashFruit)?.Value ?? - skin.GetConfig(CatchSkinConfiguration.HyperDash)?.Value ?? + skin.GetConfig(CatchSkinColour.HyperDashFruit)?.Value ?? + skin.GetConfig(CatchSkinColour.HyperDash)?.Value ?? Catcher.DefaultHyperDashColour; var hyperDash = new Sprite From 7e82f5740b0668e1f21321cd257be9928026ad54 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 3 Apr 2020 19:35:50 +0300 Subject: [PATCH 10/24] Add a skin extension for simplifying falling back on hyper-dash colours --- .../Objects/Drawables/FruitPiece.cs | 3 +-- .../Skinning/CatchSkinExtensions.cs | 16 ++++++++++++++++ .../Skinning/LegacyFruitPiece.cs | 7 +------ 3 files changed, 18 insertions(+), 8 deletions(-) create mode 100644 osu.Game.Rulesets.Catch/Skinning/CatchSkinExtensions.cs diff --git a/osu.Game.Rulesets.Catch/Objects/Drawables/FruitPiece.cs b/osu.Game.Rulesets.Catch/Objects/Drawables/FruitPiece.cs index 16818746b5..2437958916 100644 --- a/osu.Game.Rulesets.Catch/Objects/Drawables/FruitPiece.cs +++ b/osu.Game.Rulesets.Catch/Objects/Drawables/FruitPiece.cs @@ -64,8 +64,7 @@ namespace osu.Game.Rulesets.Catch.Objects.Drawables }); var hyperDashColour = - skin.GetConfig(CatchSkinColour.HyperDashFruit)?.Value ?? - skin.GetConfig(CatchSkinColour.HyperDash)?.Value ?? + skin.GetHyperDashFruitColour()?.Value ?? Catcher.DefaultHyperDashColour; if (hitObject.HyperDash) diff --git a/osu.Game.Rulesets.Catch/Skinning/CatchSkinExtensions.cs b/osu.Game.Rulesets.Catch/Skinning/CatchSkinExtensions.cs new file mode 100644 index 0000000000..8fc0831918 --- /dev/null +++ b/osu.Game.Rulesets.Catch/Skinning/CatchSkinExtensions.cs @@ -0,0 +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 osu.Framework.Bindables; +using osu.Game.Skinning; +using osuTK.Graphics; + +namespace osu.Game.Rulesets.Catch.Skinning +{ + internal static class CatchSkinExtensions + { + public static IBindable GetHyperDashFruitColour(this ISkin skin) + => skin.GetConfig(CatchSkinColour.HyperDashFruit) ?? + skin.GetConfig(CatchSkinColour.HyperDash); + } +} diff --git a/osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs b/osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs index 5235058c52..d8489399d2 100644 --- a/osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs +++ b/osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs @@ -54,15 +54,10 @@ namespace osu.Game.Rulesets.Catch.Skinning if (drawableCatchObject.HitObject.HyperDash) { - var hyperDashColour = - skin.GetConfig(CatchSkinColour.HyperDashFruit)?.Value ?? - skin.GetConfig(CatchSkinColour.HyperDash)?.Value ?? - Catcher.DefaultHyperDashColour; - var hyperDash = new Sprite { Texture = skin.GetTexture(lookupName), - Colour = hyperDashColour, + Colour = skin.GetHyperDashFruitColour()?.Value ?? Catcher.DefaultHyperDashColour, Anchor = Anchor.Centre, Origin = Anchor.Centre, Blending = BlendingParameters.Additive, From 0340b6db51f88120511ac243cc0872bc8527eb32 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 3 Apr 2020 19:50:32 +0300 Subject: [PATCH 11/24] Describe step names more --- .../TestSceneHyperDashColouring.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs index ebc3d3bff1..066b399f13 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs @@ -37,7 +37,7 @@ namespace osu.Game.Rulesets.Catch.Tests { DrawableFruit drawableFruit = null; - AddStep("setup fruit", () => + AddStep("setup hyper-dash fruit", () => { var fruit = new Fruit { HyperDashTarget = new Banana() }; fruit.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty()); @@ -50,7 +50,7 @@ namespace osu.Game.Rulesets.Catch.Tests }, false, false, false, legacyFruit); }); - AddAssert("default colour", () => + AddAssert("hyper-dash fruit has default colour", () => legacyFruit ? checkLegacyFruitHyperDashColour(drawableFruit, Catcher.DefaultHyperDashColour) : checkFruitHyperDashColour(drawableFruit, Catcher.DefaultHyperDashColour)); @@ -64,7 +64,7 @@ namespace osu.Game.Rulesets.Catch.Tests { DrawableFruit drawableFruit = null; - AddStep("setup fruit", () => + AddStep("setup hyper-dash fruit", () => { var fruit = new Fruit { HyperDashTarget = new Banana() }; fruit.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty()); @@ -77,7 +77,7 @@ namespace osu.Game.Rulesets.Catch.Tests }, customCatcherHyperDashColour, false, true, legacyFruit); }); - AddAssert("custom colour", () => + AddAssert("hyper-dash fruit use fruit colour from skin", () => legacyFruit ? checkLegacyFruitHyperDashColour(drawableFruit, TestSkin.CustomHyperDashFruitColour) : checkFruitHyperDashColour(drawableFruit, TestSkin.CustomHyperDashFruitColour)); @@ -89,7 +89,7 @@ namespace osu.Game.Rulesets.Catch.Tests { DrawableFruit drawableFruit = null; - AddStep("setup fruit", () => + AddStep("setup hyper-dash fruit", () => { var fruit = new Fruit { HyperDashTarget = new Banana() }; fruit.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty()); @@ -103,7 +103,7 @@ namespace osu.Game.Rulesets.Catch.Tests }, true, false, false, legacyFruit); }); - AddAssert("catcher custom colour", () => + AddAssert("hyper-dash fruit colour falls back to catcher colour from skin", () => legacyFruit ? checkLegacyFruitHyperDashColour(drawableFruit, TestSkin.CustomHyperDashColour) : checkFruitHyperDashColour(drawableFruit, TestSkin.CustomHyperDashColour)); From dd684b68d9cef47cc6e5a61a730343536428dc3b Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 3 Apr 2020 19:53:38 +0300 Subject: [PATCH 12/24] Make parameters required --- osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs index 066b399f13..2009099a61 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs @@ -109,7 +109,7 @@ namespace osu.Game.Rulesets.Catch.Tests : checkFruitHyperDashColour(drawableFruit, TestSkin.CustomHyperDashColour)); } - private Drawable setupSkinHierarchy(Drawable child, bool customCatcherColour = false, bool customAfterColour = false, bool customFruitColour = false, bool legacySkin = true) + private Drawable setupSkinHierarchy(Drawable child, bool customCatcherColour, bool customAfterColour, bool customFruitColour, bool legacySkin = true) { var testSkinProvider = new SkinProvidingContainer(new TestSkin(customCatcherColour, customAfterColour, customFruitColour)); @@ -143,7 +143,7 @@ namespace osu.Game.Rulesets.Catch.Tests private readonly bool customAfterColour; private readonly bool customFruitColour; - public TestSkin(bool customCatcherColour = false, bool customAfterColour = false, bool customFruitColour = false) + public TestSkin(bool customCatcherColour, bool customAfterColour, bool customFruitColour) { this.customCatcherColour = customCatcherColour; this.customAfterColour = customAfterColour; From c4f7b4576848da614959c8a427f7886e7a2f66f3 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sun, 5 Apr 2020 00:02:33 +0300 Subject: [PATCH 13/24] Revert "Add support for custom hyper-dash fruit colouring" This reverts commit 6f2cc5471adabc4392fcf1f63a5de32266016c10 and also its testing cases. This became dead code after actual correct osu!catch skin colouring, we don't support modern skinning (non-legacy skinning) at the moment, so for what it's worth this can be reverted to default red-coloured --- .../TestSceneHyperDashColouring.cs | 67 ++++++------------- .../Objects/Drawables/FruitPiece.cs | 12 +--- 2 files changed, 23 insertions(+), 56 deletions(-) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs index 2009099a61..10739a3131 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs @@ -4,15 +4,10 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; -using osu.Framework.Audio.Sample; -using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; -using osu.Framework.Graphics.Shapes; using osu.Framework.Graphics.Sprites; -using osu.Framework.Graphics.Textures; using osu.Framework.Testing; -using osu.Game.Audio; using osu.Game.Beatmaps; using osu.Game.Beatmaps.ControlPoints; using osu.Game.Rulesets.Catch.Objects; @@ -31,9 +26,8 @@ namespace osu.Game.Rulesets.Catch.Tests [Resolved] private SkinManager skins { get; set; } - [TestCase(false)] - [TestCase(true)] - public void TestHyperDashFruitColour(bool legacyFruit) + [Test] + public void TestHyperDashFruitColour() { DrawableFruit drawableFruit = null; @@ -47,20 +41,15 @@ namespace osu.Game.Rulesets.Catch.Tests Anchor = Anchor.Centre, Origin = Anchor.Centre, Scale = new Vector2(4f), - }, false, false, false, legacyFruit); + }, false, false, false); }); - AddAssert("hyper-dash fruit has default colour", () => - legacyFruit - ? checkLegacyFruitHyperDashColour(drawableFruit, Catcher.DefaultHyperDashColour) - : checkFruitHyperDashColour(drawableFruit, Catcher.DefaultHyperDashColour)); + AddAssert("hyper-dash fruit has default colour", () => checkLegacyFruitHyperDashColour(drawableFruit, Catcher.DefaultHyperDashColour)); } - [TestCase(false, true)] - [TestCase(false, false)] - [TestCase(true, true)] - [TestCase(true, false)] - public void TestCustomHyperDashFruitColour(bool legacyFruit, bool customCatcherHyperDashColour) + [TestCase(true)] + [TestCase(false)] + public void TestCustomHyperDashFruitColour(bool customCatcherHyperDashColour) { DrawableFruit drawableFruit = null; @@ -74,18 +63,14 @@ namespace osu.Game.Rulesets.Catch.Tests Anchor = Anchor.Centre, Origin = Anchor.Centre, Scale = new Vector2(4f), - }, customCatcherHyperDashColour, false, true, legacyFruit); + }, customCatcherHyperDashColour, false, true); }); - AddAssert("hyper-dash fruit use fruit colour from skin", () => - legacyFruit - ? checkLegacyFruitHyperDashColour(drawableFruit, TestSkin.CustomHyperDashFruitColour) - : checkFruitHyperDashColour(drawableFruit, TestSkin.CustomHyperDashFruitColour)); + AddAssert("hyper-dash fruit use fruit colour from skin", () => checkLegacyFruitHyperDashColour(drawableFruit, TestSkin.CustomHyperDashFruitColour)); } - [TestCase(false)] - [TestCase(true)] - public void TestCustomHyperDashFruitColourFallback(bool legacyFruit) + [Test] + public void TestCustomHyperDashFruitColourFallback() { DrawableFruit drawableFruit = null; @@ -100,36 +85,24 @@ namespace osu.Game.Rulesets.Catch.Tests Anchor = Anchor.Centre, Origin = Anchor.Centre, Scale = new Vector2(4f), - }, true, false, false, legacyFruit); + }, true, false, false); }); - AddAssert("hyper-dash fruit colour falls back to catcher colour from skin", () => - legacyFruit - ? checkLegacyFruitHyperDashColour(drawableFruit, TestSkin.CustomHyperDashColour) - : checkFruitHyperDashColour(drawableFruit, TestSkin.CustomHyperDashColour)); + AddAssert("hyper-dash fruit colour falls back to catcher colour from skin", () => checkLegacyFruitHyperDashColour(drawableFruit, TestSkin.CustomHyperDashColour)); } - private Drawable setupSkinHierarchy(Drawable child, bool customCatcherColour, bool customAfterColour, bool customFruitColour, bool legacySkin = true) + private Drawable setupSkinHierarchy(Drawable child, bool customCatcherColour, bool customAfterColour, bool customFruitColour) { + var legacySkinProvider = new SkinProvidingContainer(skins.GetSkin(DefaultLegacySkin.Info)); var testSkinProvider = new SkinProvidingContainer(new TestSkin(customCatcherColour, customAfterColour, customFruitColour)); + var legacySkinTransformer = new SkinProvidingContainer(new CatchLegacySkinTransformer(testSkinProvider)); - if (legacySkin) - { - var legacySkinProvider = new SkinProvidingContainer(skins.GetSkin(DefaultLegacySkin.Info)); - var legacySkinTransformer = new SkinProvidingContainer(new CatchLegacySkinTransformer(testSkinProvider)); - - return legacySkinProvider - .WithChild(testSkinProvider - .WithChild(legacySkinTransformer - .WithChild(child))); - } - - return testSkinProvider.WithChild(child); + return legacySkinProvider + .WithChild(testSkinProvider + .WithChild(legacySkinTransformer + .WithChild(child))); } - private bool checkFruitHyperDashColour(DrawableFruit fruit, Color4 expectedColour) => - fruit.ChildrenOfType().First().Drawable.ChildrenOfType().Single(c => c.BorderColour == expectedColour).Any(d => d.Colour == expectedColour); - private bool checkLegacyFruitHyperDashColour(DrawableFruit fruit, Color4 expectedColour) => fruit.ChildrenOfType().First().Drawable.ChildrenOfType().Any(c => c.Colour == expectedColour); diff --git a/osu.Game.Rulesets.Catch/Objects/Drawables/FruitPiece.cs b/osu.Game.Rulesets.Catch/Objects/Drawables/FruitPiece.cs index 2437958916..359329885c 100644 --- a/osu.Game.Rulesets.Catch/Objects/Drawables/FruitPiece.cs +++ b/osu.Game.Rulesets.Catch/Objects/Drawables/FruitPiece.cs @@ -7,10 +7,8 @@ using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Shapes; -using osu.Game.Rulesets.Catch.Skinning; using osu.Game.Rulesets.Catch.UI; using osu.Game.Rulesets.Objects.Drawables; -using osu.Game.Skinning; using osuTK.Graphics; namespace osu.Game.Rulesets.Catch.Objects.Drawables @@ -34,7 +32,7 @@ namespace osu.Game.Rulesets.Catch.Objects.Drawables } [BackgroundDependencyLoader] - private void load(DrawableHitObject drawableObject, ISkinSource skin) + private void load(DrawableHitObject drawableObject) { DrawableCatchHitObject drawableCatchObject = (DrawableCatchHitObject)drawableObject; hitObject = drawableCatchObject.HitObject; @@ -63,10 +61,6 @@ namespace osu.Game.Rulesets.Catch.Objects.Drawables }, }); - var hyperDashColour = - skin.GetHyperDashFruitColour()?.Value ?? - Catcher.DefaultHyperDashColour; - if (hitObject.HyperDash) { AddInternal(new Circle @@ -74,7 +68,7 @@ namespace osu.Game.Rulesets.Catch.Objects.Drawables RelativeSizeAxes = Axes.Both, Anchor = Anchor.Centre, Origin = Anchor.Centre, - BorderColour = hyperDashColour, + BorderColour = Catcher.DefaultHyperDashColour, BorderThickness = 12f * RADIUS_ADJUST, Children = new Drawable[] { @@ -84,7 +78,7 @@ namespace osu.Game.Rulesets.Catch.Objects.Drawables Alpha = 0.3f, Blending = BlendingParameters.Additive, RelativeSizeAxes = Axes.Both, - Colour = hyperDashColour, + Colour = Catcher.DefaultHyperDashColour, } } }); From 10e65c4f53929cf477042c58aa302fd0f6e3b076 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sun, 5 Apr 2020 00:10:12 +0300 Subject: [PATCH 14/24] Add handling for legacy CatchTheBeat section in LegacyDecoder --- osu.Game/Beatmaps/Formats/LegacyDecoder.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/Formats/LegacyDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyDecoder.cs index 561707f9ef..743a470e6e 100644 --- a/osu.Game/Beatmaps/Formats/LegacyDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyDecoder.cs @@ -73,6 +73,9 @@ namespace osu.Game.Beatmaps.Formats switch (section) { case Section.Colours: + // osu!catch section only has colour settings + // so no harm in handling the entire section + case Section.CatchTheBeat: HandleColours(output, line); return; } @@ -149,7 +152,8 @@ namespace osu.Game.Beatmaps.Formats HitObjects, Variables, Fonts, - Mania + CatchTheBeat, + Mania, } internal class LegacyDifficultyControlPoint : DifficultyControlPoint From 55d076d6f359ed50edd3cf371195b656effd9929 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sun, 5 Apr 2020 00:10:25 +0300 Subject: [PATCH 15/24] Transform CatchSkinColour lookup to skin configuration custom colours lookup --- .../Skinning/CatchLegacySkinTransformer.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Catch/Skinning/CatchLegacySkinTransformer.cs b/osu.Game.Rulesets.Catch/Skinning/CatchLegacySkinTransformer.cs index 65e6e6f209..4a87eb95e7 100644 --- a/osu.Game.Rulesets.Catch/Skinning/CatchLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Catch/Skinning/CatchLegacySkinTransformer.cs @@ -65,6 +65,15 @@ namespace osu.Game.Rulesets.Catch.Skinning public SampleChannel GetSample(ISampleInfo sample) => source.GetSample(sample); - public IBindable GetConfig(TLookup lookup) => source.GetConfig(lookup); + public IBindable GetConfig(TLookup lookup) + { + switch (lookup) + { + case CatchSkinColour colour: + return source.GetConfig(new SkinCustomColourLookup(colour)); + } + + return source.GetConfig(lookup); + } } } From b100230538707ffcbed2a70fc17b0981b618b3eb Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sun, 5 Apr 2020 00:13:23 +0300 Subject: [PATCH 16/24] Test CatchSkinColour transformation on colour retrieval implicitly --- .../TestSceneHyperDashColouring.cs | 37 ++++--------------- 1 file changed, 8 insertions(+), 29 deletions(-) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs index 10739a3131..c8d28dbaeb 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs @@ -106,44 +106,23 @@ namespace osu.Game.Rulesets.Catch.Tests private bool checkLegacyFruitHyperDashColour(DrawableFruit fruit, Color4 expectedColour) => fruit.ChildrenOfType().First().Drawable.ChildrenOfType().Any(c => c.Colour == expectedColour); - private class TestSkin : ISkin + private class TestSkin : LegacySkin { public static Color4 CustomHyperDashColour { get; } = Color4.Goldenrod; public static Color4 CustomHyperDashFruitColour { get; } = Color4.Cyan; public static Color4 CustomHyperDashAfterColour { get; } = Color4.Lime; - private readonly bool customCatcherColour; - private readonly bool customAfterColour; - private readonly bool customFruitColour; - public TestSkin(bool customCatcherColour, bool customAfterColour, bool customFruitColour) + : base(new SkinInfo(), null, null, string.Empty) { - this.customCatcherColour = customCatcherColour; - this.customAfterColour = customAfterColour; - this.customFruitColour = customFruitColour; - } + if (customCatcherColour) + Configuration.CustomColours[CatchSkinColour.HyperDash.ToString()] = CustomHyperDashColour; - public Drawable GetDrawableComponent(ISkinComponent component) => null; + if (customAfterColour) + Configuration.CustomColours[CatchSkinColour.HyperDashAfterImage.ToString()] = CustomHyperDashAfterColour; - public Texture GetTexture(string componentName) => null; - - public SampleChannel GetSample(ISampleInfo sampleInfo) => null; - - public IBindable GetConfig(TLookup lookup) - { - if (lookup is CatchSkinColour config) - { - if (config == CatchSkinColour.HyperDash && customCatcherColour) - return SkinUtils.As(new Bindable(CustomHyperDashColour)); - - if (config == CatchSkinColour.HyperDashFruit && customFruitColour) - return SkinUtils.As(new Bindable(CustomHyperDashFruitColour)); - - if (config == CatchSkinColour.HyperDashAfterImage && customAfterColour) - return SkinUtils.As(new Bindable(CustomHyperDashAfterColour)); - } - - return null; + if (customFruitColour) + Configuration.CustomColours[CatchSkinColour.HyperDashFruit.ToString()] = CustomHyperDashFruitColour; } } } From dfd86e643bd415e5653a942e2432d56d224c6a1b Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sun, 5 Apr 2020 00:14:07 +0300 Subject: [PATCH 17/24] Add custom-coloured osu!catch skin configuration to 'Resources/special-skin' --- osu.Game.Rulesets.Catch.Tests/Resources/special-skin/skin.ini | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 osu.Game.Rulesets.Catch.Tests/Resources/special-skin/skin.ini diff --git a/osu.Game.Rulesets.Catch.Tests/Resources/special-skin/skin.ini b/osu.Game.Rulesets.Catch.Tests/Resources/special-skin/skin.ini new file mode 100644 index 0000000000..36515f33c5 --- /dev/null +++ b/osu.Game.Rulesets.Catch.Tests/Resources/special-skin/skin.ini @@ -0,0 +1,4 @@ +[CatchTheBeat] +HyperDash: 232,185,35 +HyperDashFruit: 0,255,255 +HyperDashAfterImage: 232,74,35 \ No newline at end of file From f6bbec72bfd664f1a29f27de192496bba08df6ca Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sun, 5 Apr 2020 00:20:21 +0300 Subject: [PATCH 18/24] Revert "Add custom-coloured osu!catch skin configuration to 'Resources/special-skin'" This reverts commit dfd86e643bd415e5653a942e2432d56d224c6a1b. --- osu.Game.Rulesets.Catch.Tests/Resources/special-skin/skin.ini | 4 ---- 1 file changed, 4 deletions(-) delete mode 100644 osu.Game.Rulesets.Catch.Tests/Resources/special-skin/skin.ini diff --git a/osu.Game.Rulesets.Catch.Tests/Resources/special-skin/skin.ini b/osu.Game.Rulesets.Catch.Tests/Resources/special-skin/skin.ini deleted file mode 100644 index 36515f33c5..0000000000 --- a/osu.Game.Rulesets.Catch.Tests/Resources/special-skin/skin.ini +++ /dev/null @@ -1,4 +0,0 @@ -[CatchTheBeat] -HyperDash: 232,185,35 -HyperDashFruit: 0,255,255 -HyperDashAfterImage: 232,74,35 \ No newline at end of file From 42ac0c72eac12ea39415f9bc9926adcc5a1a6ff6 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sun, 5 Apr 2020 00:46:52 +0300 Subject: [PATCH 19/24] Fix grammer issue and more rewording MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bartłomiej Dach --- osu.Game.Rulesets.Catch/Skinning/CatchSkinColour.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Skinning/CatchSkinColour.cs b/osu.Game.Rulesets.Catch/Skinning/CatchSkinColour.cs index 2ad8f89739..4506111498 100644 --- a/osu.Game.Rulesets.Catch/Skinning/CatchSkinColour.cs +++ b/osu.Game.Rulesets.Catch/Skinning/CatchSkinColour.cs @@ -6,12 +6,12 @@ namespace osu.Game.Rulesets.Catch.Skinning public enum CatchSkinColour { /// - /// The colour to be used for the catcher while on hyper-dashing state. + /// The colour to be used for the catcher while in hyper-dashing state. /// HyperDash, /// - /// The colour to be used for hyper-dash fruits. + /// The colour to be used for fruits that grant the catcher the ability to hyper-dash. /// HyperDashFruit, From 1b76a53d329acbe4c3dc73800e867b69277e5a0a Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sun, 5 Apr 2020 22:10:35 +0300 Subject: [PATCH 20/24] Move CatchTheBeat section handling to LegacySkinDecoder Best place to reside at --- osu.Game/Beatmaps/Formats/LegacyDecoder.cs | 3 --- osu.Game/Skinning/LegacySkinDecoder.cs | 6 ++++++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/osu.Game/Beatmaps/Formats/LegacyDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyDecoder.cs index 743a470e6e..113526f9dd 100644 --- a/osu.Game/Beatmaps/Formats/LegacyDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyDecoder.cs @@ -73,9 +73,6 @@ namespace osu.Game.Beatmaps.Formats switch (section) { case Section.Colours: - // osu!catch section only has colour settings - // so no harm in handling the entire section - case Section.CatchTheBeat: HandleColours(output, line); return; } diff --git a/osu.Game/Skinning/LegacySkinDecoder.cs b/osu.Game/Skinning/LegacySkinDecoder.cs index 88ba7b23b7..b5734edacf 100644 --- a/osu.Game/Skinning/LegacySkinDecoder.cs +++ b/osu.Game/Skinning/LegacySkinDecoder.cs @@ -44,6 +44,12 @@ namespace osu.Game.Skinning } break; + + // osu!catch section only has colour settings + // so no harm in handling the entire section + case Section.CatchTheBeat: + HandleColours(skin, line); + return; } if (!string.IsNullOrEmpty(pair.Key)) From 7f3ad6d5be7b79358f6c1d9ed2c44c3e60a30dda Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sun, 5 Apr 2020 22:15:11 +0300 Subject: [PATCH 21/24] Move default colour fallback to the extension methods itself --- osu.Game.Rulesets.Catch/Skinning/CatchSkinExtensions.cs | 6 +++++- osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs | 3 +-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Skinning/CatchSkinExtensions.cs b/osu.Game.Rulesets.Catch/Skinning/CatchSkinExtensions.cs index 8fc0831918..623f87bf11 100644 --- a/osu.Game.Rulesets.Catch/Skinning/CatchSkinExtensions.cs +++ b/osu.Game.Rulesets.Catch/Skinning/CatchSkinExtensions.cs @@ -1,7 +1,9 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using JetBrains.Annotations; using osu.Framework.Bindables; +using osu.Game.Rulesets.Catch.UI; using osu.Game.Skinning; using osuTK.Graphics; @@ -9,8 +11,10 @@ namespace osu.Game.Rulesets.Catch.Skinning { internal static class CatchSkinExtensions { + [NotNull] public static IBindable GetHyperDashFruitColour(this ISkin skin) => skin.GetConfig(CatchSkinColour.HyperDashFruit) ?? - skin.GetConfig(CatchSkinColour.HyperDash); + skin.GetConfig(CatchSkinColour.HyperDash) ?? + new Bindable(Catcher.DefaultHyperDashColour); } } diff --git a/osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs b/osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs index d8489399d2..470c12559e 100644 --- a/osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs +++ b/osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs @@ -7,7 +7,6 @@ using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Sprites; using osu.Game.Rulesets.Catch.Objects.Drawables; -using osu.Game.Rulesets.Catch.UI; using osu.Game.Rulesets.Objects.Drawables; using osu.Game.Skinning; using osuTK; @@ -57,7 +56,7 @@ namespace osu.Game.Rulesets.Catch.Skinning var hyperDash = new Sprite { Texture = skin.GetTexture(lookupName), - Colour = skin.GetHyperDashFruitColour()?.Value ?? Catcher.DefaultHyperDashColour, + Colour = skin.GetHyperDashFruitColour().Value, Anchor = Anchor.Centre, Origin = Anchor.Centre, Blending = BlendingParameters.Additive, From d27d8671ab08e6a334f9859e46a295c68b3d5f01 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 8 Apr 2020 14:23:29 +0300 Subject: [PATCH 22/24] Convert all static getter-only properties to static readonly fields --- .../TestSceneHyperDashColouring.cs | 18 +++++++++--------- .../Objects/Drawables/FruitPiece.cs | 4 ++-- .../Skinning/CatchSkinExtensions.cs | 2 +- osu.Game.Rulesets.Catch/UI/Catcher.cs | 2 +- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs index c8d28dbaeb..846b17f324 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs @@ -44,7 +44,7 @@ namespace osu.Game.Rulesets.Catch.Tests }, false, false, false); }); - AddAssert("hyper-dash fruit has default colour", () => checkLegacyFruitHyperDashColour(drawableFruit, Catcher.DefaultHyperDashColour)); + AddAssert("hyper-dash fruit has default colour", () => checkLegacyFruitHyperDashColour(drawableFruit, Catcher.DEFAULT_HYPER_DASH_COLOUR)); } [TestCase(true)] @@ -66,7 +66,7 @@ namespace osu.Game.Rulesets.Catch.Tests }, customCatcherHyperDashColour, false, true); }); - AddAssert("hyper-dash fruit use fruit colour from skin", () => checkLegacyFruitHyperDashColour(drawableFruit, TestSkin.CustomHyperDashFruitColour)); + AddAssert("hyper-dash fruit use fruit colour from skin", () => checkLegacyFruitHyperDashColour(drawableFruit, TestSkin.CUSTOM_HYPER_DASH_FRUIT_COLOUR)); } [Test] @@ -88,7 +88,7 @@ namespace osu.Game.Rulesets.Catch.Tests }, true, false, false); }); - AddAssert("hyper-dash fruit colour falls back to catcher colour from skin", () => checkLegacyFruitHyperDashColour(drawableFruit, TestSkin.CustomHyperDashColour)); + AddAssert("hyper-dash fruit colour falls back to catcher colour from skin", () => checkLegacyFruitHyperDashColour(drawableFruit, TestSkin.CUSTOM_HYPER_DASH_COLOUR)); } private Drawable setupSkinHierarchy(Drawable child, bool customCatcherColour, bool customAfterColour, bool customFruitColour) @@ -108,21 +108,21 @@ namespace osu.Game.Rulesets.Catch.Tests private class TestSkin : LegacySkin { - public static Color4 CustomHyperDashColour { get; } = Color4.Goldenrod; - public static Color4 CustomHyperDashFruitColour { get; } = Color4.Cyan; - public static Color4 CustomHyperDashAfterColour { get; } = Color4.Lime; + public static readonly Color4 CUSTOM_HYPER_DASH_COLOUR = Color4.Goldenrod; + public static readonly Color4 CUSTOM_HYPER_DASH_AFTER_COLOUR = Color4.Lime; + public static readonly Color4 CUSTOM_HYPER_DASH_FRUIT_COLOUR = Color4.Cyan; public TestSkin(bool customCatcherColour, bool customAfterColour, bool customFruitColour) : base(new SkinInfo(), null, null, string.Empty) { if (customCatcherColour) - Configuration.CustomColours[CatchSkinColour.HyperDash.ToString()] = CustomHyperDashColour; + Configuration.CustomColours[CatchSkinColour.HyperDash.ToString()] = CUSTOM_HYPER_DASH_COLOUR; if (customAfterColour) - Configuration.CustomColours[CatchSkinColour.HyperDashAfterImage.ToString()] = CustomHyperDashAfterColour; + Configuration.CustomColours[CatchSkinColour.HyperDashAfterImage.ToString()] = CUSTOM_HYPER_DASH_AFTER_COLOUR; if (customFruitColour) - Configuration.CustomColours[CatchSkinColour.HyperDashFruit.ToString()] = CustomHyperDashFruitColour; + Configuration.CustomColours[CatchSkinColour.HyperDashFruit.ToString()] = CUSTOM_HYPER_DASH_FRUIT_COLOUR; } } } diff --git a/osu.Game.Rulesets.Catch/Objects/Drawables/FruitPiece.cs b/osu.Game.Rulesets.Catch/Objects/Drawables/FruitPiece.cs index 359329885c..7ac9f11ad6 100644 --- a/osu.Game.Rulesets.Catch/Objects/Drawables/FruitPiece.cs +++ b/osu.Game.Rulesets.Catch/Objects/Drawables/FruitPiece.cs @@ -68,7 +68,7 @@ namespace osu.Game.Rulesets.Catch.Objects.Drawables RelativeSizeAxes = Axes.Both, Anchor = Anchor.Centre, Origin = Anchor.Centre, - BorderColour = Catcher.DefaultHyperDashColour, + BorderColour = Catcher.DEFAULT_HYPER_DASH_COLOUR, BorderThickness = 12f * RADIUS_ADJUST, Children = new Drawable[] { @@ -78,7 +78,7 @@ namespace osu.Game.Rulesets.Catch.Objects.Drawables Alpha = 0.3f, Blending = BlendingParameters.Additive, RelativeSizeAxes = Axes.Both, - Colour = Catcher.DefaultHyperDashColour, + Colour = Catcher.DEFAULT_HYPER_DASH_COLOUR, } } }); diff --git a/osu.Game.Rulesets.Catch/Skinning/CatchSkinExtensions.cs b/osu.Game.Rulesets.Catch/Skinning/CatchSkinExtensions.cs index 623f87bf11..718b22a0fb 100644 --- a/osu.Game.Rulesets.Catch/Skinning/CatchSkinExtensions.cs +++ b/osu.Game.Rulesets.Catch/Skinning/CatchSkinExtensions.cs @@ -15,6 +15,6 @@ namespace osu.Game.Rulesets.Catch.Skinning public static IBindable GetHyperDashFruitColour(this ISkin skin) => skin.GetConfig(CatchSkinColour.HyperDashFruit) ?? skin.GetConfig(CatchSkinColour.HyperDash) ?? - new Bindable(Catcher.DefaultHyperDashColour); + new Bindable(Catcher.DEFAULT_HYPER_DASH_COLOUR); } } diff --git a/osu.Game.Rulesets.Catch/UI/Catcher.cs b/osu.Game.Rulesets.Catch/UI/Catcher.cs index 9bfff209d5..920d804e72 100644 --- a/osu.Game.Rulesets.Catch/UI/Catcher.cs +++ b/osu.Game.Rulesets.Catch/UI/Catcher.cs @@ -21,7 +21,7 @@ namespace osu.Game.Rulesets.Catch.UI { public class Catcher : Container, IKeyBindingHandler { - public static Color4 DefaultHyperDashColour { get; } = Color4.Red; + public static readonly Color4 DEFAULT_HYPER_DASH_COLOUR = Color4.Red; /// /// Whether we are hyper-dashing or not. From 5f13dc81bed4d90e9fd43c5a3e96573de00951dd Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 14 Apr 2020 04:38:18 +0300 Subject: [PATCH 23/24] Remove no longer necessary extensions --- .../Skinning/CatchSkinExtensions.cs | 20 ------------------- .../Skinning/LegacyFruitPiece.cs | 9 ++++++--- 2 files changed, 6 insertions(+), 23 deletions(-) delete mode 100644 osu.Game.Rulesets.Catch/Skinning/CatchSkinExtensions.cs diff --git a/osu.Game.Rulesets.Catch/Skinning/CatchSkinExtensions.cs b/osu.Game.Rulesets.Catch/Skinning/CatchSkinExtensions.cs deleted file mode 100644 index 718b22a0fb..0000000000 --- a/osu.Game.Rulesets.Catch/Skinning/CatchSkinExtensions.cs +++ /dev/null @@ -1,20 +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 JetBrains.Annotations; -using osu.Framework.Bindables; -using osu.Game.Rulesets.Catch.UI; -using osu.Game.Skinning; -using osuTK.Graphics; - -namespace osu.Game.Rulesets.Catch.Skinning -{ - internal static class CatchSkinExtensions - { - [NotNull] - public static IBindable GetHyperDashFruitColour(this ISkin skin) - => skin.GetConfig(CatchSkinColour.HyperDashFruit) ?? - skin.GetConfig(CatchSkinColour.HyperDash) ?? - new Bindable(Catcher.DEFAULT_HYPER_DASH_COLOUR); - } -} diff --git a/osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs b/osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs index 470c12559e..5be54d3882 100644 --- a/osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs +++ b/osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs @@ -7,6 +7,7 @@ using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Sprites; using osu.Game.Rulesets.Catch.Objects.Drawables; +using osu.Game.Rulesets.Catch.UI; using osu.Game.Rulesets.Objects.Drawables; using osu.Game.Skinning; using osuTK; @@ -55,14 +56,16 @@ namespace osu.Game.Rulesets.Catch.Skinning { var hyperDash = new Sprite { - Texture = skin.GetTexture(lookupName), - Colour = skin.GetHyperDashFruitColour().Value, Anchor = Anchor.Centre, Origin = Anchor.Centre, Blending = BlendingParameters.Additive, Depth = 1, Alpha = 0.7f, - Scale = new Vector2(1.2f) + Scale = new Vector2(1.2f), + Texture = skin.GetTexture(lookupName), + Colour = skin.GetConfig(CatchSkinColour.HyperDashFruit)?.Value ?? + skin.GetConfig(CatchSkinColour.HyperDash)?.Value ?? + Catcher.DEFAULT_HYPER_DASH_COLOUR, }; AddInternal(hyperDash); From e3e0cd149f94d033a36d7622c0a8bf91e029fde0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 19 Apr 2020 12:41:00 +0200 Subject: [PATCH 24/24] Refactor test code to eliminate boolean flags --- .../TestSceneHyperDashColouring.cs | 127 +++++++++--------- 1 file changed, 65 insertions(+), 62 deletions(-) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs index 846b17f324..a48ecb9b79 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs @@ -27,74 +27,71 @@ namespace osu.Game.Rulesets.Catch.Tests private SkinManager skins { get; set; } [Test] - public void TestHyperDashFruitColour() + public void TestDefaultFruitColour() { - DrawableFruit drawableFruit = null; + var skin = new TestSkin(); - AddStep("setup hyper-dash fruit", () => - { - var fruit = new Fruit { HyperDashTarget = new Banana() }; - fruit.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty()); - - Child = setupSkinHierarchy(drawableFruit = new DrawableFruit(fruit) - { - Anchor = Anchor.Centre, - Origin = Anchor.Centre, - Scale = new Vector2(4f), - }, false, false, false); - }); - - AddAssert("hyper-dash fruit has default colour", () => checkLegacyFruitHyperDashColour(drawableFruit, Catcher.DEFAULT_HYPER_DASH_COLOUR)); - } - - [TestCase(true)] - [TestCase(false)] - public void TestCustomHyperDashFruitColour(bool customCatcherHyperDashColour) - { - DrawableFruit drawableFruit = null; - - AddStep("setup hyper-dash fruit", () => - { - var fruit = new Fruit { HyperDashTarget = new Banana() }; - fruit.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty()); - - Child = setupSkinHierarchy(drawableFruit = new DrawableFruit(fruit) - { - Anchor = Anchor.Centre, - Origin = Anchor.Centre, - Scale = new Vector2(4f), - }, customCatcherHyperDashColour, false, true); - }); - - AddAssert("hyper-dash fruit use fruit colour from skin", () => checkLegacyFruitHyperDashColour(drawableFruit, TestSkin.CUSTOM_HYPER_DASH_FRUIT_COLOUR)); + checkHyperDashFruitColour(skin, Catcher.DEFAULT_HYPER_DASH_COLOUR); } [Test] - public void TestCustomHyperDashFruitColourFallback() + public void TestCustomFruitColour() + { + var skin = new TestSkin + { + HyperDashFruitColour = Color4.Cyan + }; + + checkHyperDashFruitColour(skin, skin.HyperDashFruitColour); + } + + [Test] + public void TestCustomFruitColourPriority() + { + var skin = new TestSkin + { + HyperDashColour = Color4.Goldenrod, + HyperDashFruitColour = Color4.Cyan + }; + + checkHyperDashFruitColour(skin, skin.HyperDashFruitColour); + } + + [Test] + public void TestFruitColourFallback() + { + var skin = new TestSkin + { + HyperDashColour = Color4.Goldenrod + }; + + checkHyperDashFruitColour(skin, skin.HyperDashColour); + } + + private void checkHyperDashFruitColour(ISkin skin, Color4 expectedColour) { DrawableFruit drawableFruit = null; - AddStep("setup hyper-dash fruit", () => + AddStep("create hyper-dash fruit", () => { var fruit = new Fruit { HyperDashTarget = new Banana() }; fruit.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty()); - Child = setupSkinHierarchy( - drawableFruit = new DrawableFruit(fruit) - { - Anchor = Anchor.Centre, - Origin = Anchor.Centre, - Scale = new Vector2(4f), - }, true, false, false); + Child = setupSkinHierarchy(drawableFruit = new DrawableFruit(fruit) + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Scale = new Vector2(4f), + }, skin); }); - AddAssert("hyper-dash fruit colour falls back to catcher colour from skin", () => checkLegacyFruitHyperDashColour(drawableFruit, TestSkin.CUSTOM_HYPER_DASH_COLOUR)); + AddAssert("hyper-dash colour is correct", () => checkLegacyFruitHyperDashColour(drawableFruit, expectedColour)); } - private Drawable setupSkinHierarchy(Drawable child, bool customCatcherColour, bool customAfterColour, bool customFruitColour) + private Drawable setupSkinHierarchy(Drawable child, ISkin skin) { var legacySkinProvider = new SkinProvidingContainer(skins.GetSkin(DefaultLegacySkin.Info)); - var testSkinProvider = new SkinProvidingContainer(new TestSkin(customCatcherColour, customAfterColour, customFruitColour)); + var testSkinProvider = new SkinProvidingContainer(skin); var legacySkinTransformer = new SkinProvidingContainer(new CatchLegacySkinTransformer(testSkinProvider)); return legacySkinProvider @@ -108,21 +105,27 @@ namespace osu.Game.Rulesets.Catch.Tests private class TestSkin : LegacySkin { - public static readonly Color4 CUSTOM_HYPER_DASH_COLOUR = Color4.Goldenrod; - public static readonly Color4 CUSTOM_HYPER_DASH_AFTER_COLOUR = Color4.Lime; - public static readonly Color4 CUSTOM_HYPER_DASH_FRUIT_COLOUR = Color4.Cyan; + public Color4 HyperDashColour + { + get => Configuration.CustomColours[CatchSkinColour.HyperDash.ToString()]; + set => Configuration.CustomColours[CatchSkinColour.HyperDash.ToString()] = value; + } - public TestSkin(bool customCatcherColour, bool customAfterColour, bool customFruitColour) + public Color4 HyperDashAfterImageColour + { + get => Configuration.CustomColours[CatchSkinColour.HyperDashAfterImage.ToString()]; + set => Configuration.CustomColours[CatchSkinColour.HyperDashAfterImage.ToString()] = value; + } + + public Color4 HyperDashFruitColour + { + get => Configuration.CustomColours[CatchSkinColour.HyperDashFruit.ToString()]; + set => Configuration.CustomColours[CatchSkinColour.HyperDashFruit.ToString()] = value; + } + + public TestSkin() : base(new SkinInfo(), null, null, string.Empty) { - if (customCatcherColour) - Configuration.CustomColours[CatchSkinColour.HyperDash.ToString()] = CUSTOM_HYPER_DASH_COLOUR; - - if (customAfterColour) - Configuration.CustomColours[CatchSkinColour.HyperDashAfterImage.ToString()] = CUSTOM_HYPER_DASH_AFTER_COLOUR; - - if (customFruitColour) - Configuration.CustomColours[CatchSkinColour.HyperDashFruit.ToString()] = CUSTOM_HYPER_DASH_FRUIT_COLOUR; } } }