From e6b2e3b0ed1f4059a9fef74053b7ed5d6ec39d9d Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 26 Mar 2020 05:18:12 +0300 Subject: [PATCH 01/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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 3daacbc2d202b3d42ac84e7242e571a045d8fa09 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 17 Apr 2020 13:34:20 +0900 Subject: [PATCH 24/35] Initial inefficient refactor of hitobject enumeration --- osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs | 76 ++++++++------------ osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs | 2 +- 2 files changed, 29 insertions(+), 49 deletions(-) diff --git a/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs b/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs index dfca2aff7b..171ce6fe61 100644 --- a/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs +++ b/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.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.Collections.Generic; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Drawables; -using osu.Game.Rulesets.Osu.Objects; using osu.Game.Rulesets.Osu.Objects.Drawables; using osu.Game.Rulesets.UI; @@ -37,12 +37,9 @@ namespace osu.Game.Rulesets.Osu.UI DrawableHitObject blockingObject = null; // Find the last hitobject which blocks future hits. - foreach (var obj in hitObjectContainer.AliveObjects) + foreach (var obj in enumerateHitObjectsUpTo(hitObject)) { - if (obj == hitObject) - break; - - if (drawableCanBlockFutureHits(obj)) + if (hitObjectCanBlockFutureHits(obj)) blockingObject = obj; } @@ -64,64 +61,47 @@ namespace osu.Game.Rulesets.Osu.UI /// Handles a being hit to potentially miss all earlier s. /// /// The that was hit. - public void HandleHit(HitObject hitObject) + public void HandleHit(DrawableHitObject hitObject) { // Hitobjects which themselves don't block future hitobjects don't cause misses (e.g. slider ticks, spinners). if (!hitObjectCanBlockFutureHits(hitObject)) return; - double maximumTime = hitObject.StartTime; - - // Iterate through and apply miss results to all top-level and nested hitobjects which block future hits. - foreach (var obj in hitObjectContainer.AliveObjects) + foreach (var obj in enumerateHitObjectsUpTo(hitObject)) { - if (obj.Judged || obj.HitObject.StartTime >= maximumTime) + if (obj.Judged) continue; - if (hitObjectCanBlockFutureHits(obj.HitObject)) - applyMiss(obj); - - foreach (var nested in obj.NestedHitObjects) - { - if (nested.Judged || nested.HitObject.StartTime >= maximumTime) - continue; - - if (hitObjectCanBlockFutureHits(nested.HitObject)) - applyMiss(nested); - } + if (hitObjectCanBlockFutureHits(obj)) + ((DrawableOsuHitObject)obj).MissForcefully(); } - - static void applyMiss(DrawableHitObject obj) => ((DrawableOsuHitObject)obj).MissForcefully(); - } - - /// - /// Whether a blocks hits on future s until its start time is reached. - /// - /// - /// This will ONLY match on top-most s. - /// - /// The to test. - private static bool drawableCanBlockFutureHits(DrawableHitObject hitObject) - { - // Special considerations for slider tails aren't required since only top-most drawable hitobjects are being iterated over. - return hitObject is DrawableHitCircle || hitObject is DrawableSlider; } /// /// Whether a blocks hits on future s until its start time is reached. /// - /// - /// This is more rigorous and may not match on top-most s as does. - /// /// The to test. - private static bool hitObjectCanBlockFutureHits(HitObject hitObject) - { - // Unlike the above we will receive slider tails, but they do not block future hits. - if (hitObject is SliderTailCircle) - return false; + private static bool hitObjectCanBlockFutureHits(DrawableHitObject hitObject) + => hitObject is DrawableHitCircle; - // All other hitcircles continue to block future hits. - return hitObject is HitCircle; + // Todo: Inefficient + private IEnumerable enumerateHitObjectsUpTo(DrawableHitObject hitObject) + { + return enumerate(hitObjectContainer.AliveObjects); + + IEnumerable enumerate(IEnumerable list) + { + foreach (var obj in list) + { + if (obj.HitObject.StartTime >= hitObject.HitObject.StartTime) + yield break; + + yield return obj; + + foreach (var nested in enumerate(obj.NestedHitObjects)) + yield return nested; + } + } } } } diff --git a/osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs b/osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs index 2f222f59b4..4b1a2ce43c 100644 --- a/osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs +++ b/osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs @@ -86,7 +86,7 @@ namespace osu.Game.Rulesets.Osu.UI private void onNewResult(DrawableHitObject judgedObject, JudgementResult result) { // Hitobjects that block future hits should miss previous hitobjects if they're hit out-of-order. - hitPolicy.HandleHit(result.HitObject); + hitPolicy.HandleHit(judgedObject); if (!judgedObject.DisplayResult || !DisplayJudgements.Value) return; From 62f77a05befb156ac6cda6411f2dda85ffcc8b44 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 17 Apr 2020 14:00:00 +0900 Subject: [PATCH 25/35] Optimise by removing state machine --- osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs | 115 +++++++++++++++---- 1 file changed, 95 insertions(+), 20 deletions(-) diff --git a/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs b/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs index 171ce6fe61..b55e04ec4c 100644 --- a/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs +++ b/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.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 System.Collections; using System.Collections.Generic; +using System.Diagnostics; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Drawables; using osu.Game.Rulesets.Osu.Objects.Drawables; @@ -36,11 +38,14 @@ namespace osu.Game.Rulesets.Osu.UI { DrawableHitObject blockingObject = null; - // Find the last hitobject which blocks future hits. - foreach (var obj in enumerateHitObjectsUpTo(hitObject)) + var enumerator = new HitObjectEnumerator(hitObjectContainer, hitObject.HitObject.StartTime); + + while (enumerator.MoveNext()) { - if (hitObjectCanBlockFutureHits(obj)) - blockingObject = obj; + Debug.Assert(enumerator.Current != null); + + if (hitObjectCanBlockFutureHits(enumerator.Current)) + blockingObject = enumerator.Current; } // If there is no previous hitobject, allow the hit. @@ -67,13 +72,17 @@ namespace osu.Game.Rulesets.Osu.UI if (!hitObjectCanBlockFutureHits(hitObject)) return; - foreach (var obj in enumerateHitObjectsUpTo(hitObject)) + var enumerator = new HitObjectEnumerator(hitObjectContainer, hitObject.HitObject.StartTime); + + while (enumerator.MoveNext()) { - if (obj.Judged) + Debug.Assert(enumerator.Current != null); + + if (enumerator.Current.Judged) continue; - if (hitObjectCanBlockFutureHits(obj)) - ((DrawableOsuHitObject)obj).MissForcefully(); + if (hitObjectCanBlockFutureHits(enumerator.Current)) + ((DrawableOsuHitObject)enumerator.Current).MissForcefully(); } } @@ -84,23 +93,89 @@ namespace osu.Game.Rulesets.Osu.UI private static bool hitObjectCanBlockFutureHits(DrawableHitObject hitObject) => hitObject is DrawableHitCircle; - // Todo: Inefficient - private IEnumerable enumerateHitObjectsUpTo(DrawableHitObject hitObject) + private struct HitObjectEnumerator : IEnumerator { - return enumerate(hitObjectContainer.AliveObjects); + private readonly IEnumerator hitObjectEnumerator; + private readonly double targetTime; - IEnumerable enumerate(IEnumerable list) + private DrawableHitObject currentTopLevel; + private int currentNestedIndex; + + public HitObjectEnumerator(HitObjectContainer hitObjectContainer, double targetTime) { - foreach (var obj in list) - { - if (obj.HitObject.StartTime >= hitObject.HitObject.StartTime) - yield break; + hitObjectEnumerator = hitObjectContainer.AliveObjects.GetEnumerator(); + this.targetTime = targetTime; - yield return obj; + currentTopLevel = null; + currentNestedIndex = -1; + Current = null; + } - foreach (var nested in enumerate(obj.NestedHitObjects)) - yield return nested; - } + /// + /// Attempts to move to the next top-level or nested hitobject. + /// Stops when no such hitobject is found or until the hitobject start time reaches . + /// + /// Whether a new hitobject was moved to. + public bool MoveNext() + { + // If we don't already have a top-level hitobject, try to get one. + if (currentTopLevel == null) + return moveNextTopLevel(); + + // If we have a top-level hitobject, try to move to the next nested hitobject or otherwise move to the next top-level hitobject. + if (!moveNextNested()) + return moveNextTopLevel(); + + // Guaranteed by moveNextNested() to have a hitobject. + return true; + } + + /// + /// Attempts to move to the next top-level hitobject. + /// + /// Whether a new top-level hitobject was found. + private bool moveNextTopLevel() + { + currentNestedIndex = -1; + + hitObjectEnumerator.MoveNext(); + currentTopLevel = hitObjectEnumerator.Current; + + Current = currentTopLevel; + + return Current?.HitObject.StartTime < targetTime; + } + + /// + /// Attempts to move to the next nested hitobject in the current top-level hitobject. + /// + /// Whether a new nested hitobject was moved to. + private bool moveNextNested() + { + currentNestedIndex++; + if (currentNestedIndex >= currentTopLevel.NestedHitObjects.Count) + return false; + + Current = currentTopLevel.NestedHitObjects[currentNestedIndex]; + Debug.Assert(Current != null); + + return Current?.HitObject.StartTime < targetTime; + } + + public void Reset() + { + hitObjectEnumerator.Reset(); + currentTopLevel = null; + currentNestedIndex = -1; + Current = null; + } + + public DrawableHitObject Current { get; set; } + + object IEnumerator.Current => Current; + + public void Dispose() + { } } } From ee5301b887a78a3bd0cabab17c306857133da794 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 17 Apr 2020 14:12:38 +0900 Subject: [PATCH 26/35] Fix head/tail circles not getting correct hit windows --- osu.Game.Rulesets.Osu.Tests/TestSceneOutOfOrderHits.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneOutOfOrderHits.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneOutOfOrderHits.cs index d6858f831e..3df51be600 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneOutOfOrderHits.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneOutOfOrderHits.cs @@ -371,6 +371,9 @@ namespace osu.Game.Rulesets.Osu.Tests { HeadCircle.HitWindows = new TestHitWindows(); TailCircle.HitWindows = new TestHitWindows(); + + HeadCircle.HitWindows.SetDifficulty(0); + TailCircle.HitWindows.SetDifficulty(0); }; } } From 08df9d49e52a968691b77a76fe360c3111eb3436 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 17 Apr 2020 14:12:43 +0900 Subject: [PATCH 27/35] Add failing test --- .../TestSceneOutOfOrderHits.cs | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneOutOfOrderHits.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneOutOfOrderHits.cs index 3df51be600..40ee53e8f2 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneOutOfOrderHits.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneOutOfOrderHits.cs @@ -296,6 +296,44 @@ namespace osu.Game.Rulesets.Osu.Tests addJudgementAssert(hitObjects[1], HitResult.Great); } + [Test] + public void TestHitSliderHeadBeforeHitCircle() + { + const double time_circle = 1000; + const double time_slider = 1200; + Vector2 positionCircle = Vector2.Zero; + Vector2 positionSlider = new Vector2(80); + + var hitObjects = new List + { + new TestHitCircle + { + StartTime = time_circle, + Position = positionCircle + }, + new TestSlider + { + StartTime = time_slider, + Position = positionSlider, + Path = new SliderPath(PathType.Linear, new[] + { + Vector2.Zero, + new Vector2(25, 0), + }) + } + }; + + performTest(hitObjects, new List + { + new OsuReplayFrame { Time = time_circle - 100, Position = positionSlider, Actions = { OsuAction.LeftButton } }, + new OsuReplayFrame { Time = time_circle, Position = positionCircle, Actions = { OsuAction.RightButton } }, + new OsuReplayFrame { Time = time_slider, Position = positionSlider, Actions = { OsuAction.LeftButton } }, + }); + + addJudgementAssert(hitObjects[0], HitResult.Great); + addJudgementAssert(hitObjects[1], HitResult.Great); + } + private void addJudgementAssert(OsuHitObject hitObject, HitResult result) { AddAssert($"({hitObject.GetType().ReadableName()} @ {hitObject.StartTime}) judgement is {result}", From a4a782381797f927bc80f108cbbf94f410faef99 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 17 Apr 2020 14:22:03 +0900 Subject: [PATCH 28/35] Add fail-safe to ensure hittability after a hit --- osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs b/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs index b55e04ec4c..31edefea83 100644 --- a/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs +++ b/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs @@ -1,6 +1,7 @@ // 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; using System.Collections.Generic; using System.Diagnostics; @@ -72,6 +73,9 @@ namespace osu.Game.Rulesets.Osu.UI if (!hitObjectCanBlockFutureHits(hitObject)) return; + if (!IsHittable(hitObject, hitObject.HitObject.StartTime + hitObject.Result.TimeOffset)) + throw new InvalidOperationException($"A {hitObject} was hit before it become hittable!"); + var enumerator = new HitObjectEnumerator(hitObjectContainer, hitObject.HitObject.StartTime); while (enumerator.MoveNext()) From 4e4fe5cc904107ac647ffaa57d5ac17361ee073e Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 17 Apr 2020 14:33:29 +0900 Subject: [PATCH 29/35] Fix slider heads not being blocked when hit out of order --- osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs index 522217a916..72502c02cd 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs @@ -125,7 +125,11 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables return new DrawableSliderTail(slider, tail); case SliderHeadCircle head: - return new DrawableSliderHead(slider, head) { OnShake = Shake }; + return new DrawableSliderHead(slider, head) + { + OnShake = Shake, + CheckHittable = (d, t) => CheckHittable?.Invoke(d, t) ?? true + }; case SliderTick tick: return new DrawableSliderTick(tick) { Position = tick.Position - slider.Position }; From 2dee5e03e30f158880a09aaa04ded47879d6f74d Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 17 Apr 2020 14:40:29 +0900 Subject: [PATCH 30/35] Dispose enumerators for safety --- osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs | 31 +++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs b/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs index 31edefea83..4bc7da4794 100644 --- a/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs +++ b/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs @@ -39,14 +39,15 @@ namespace osu.Game.Rulesets.Osu.UI { DrawableHitObject blockingObject = null; - var enumerator = new HitObjectEnumerator(hitObjectContainer, hitObject.HitObject.StartTime); - - while (enumerator.MoveNext()) + using (var enumerator = new HitObjectEnumerator(hitObjectContainer, hitObject.HitObject.StartTime)) { - Debug.Assert(enumerator.Current != null); + while (enumerator.MoveNext()) + { + Debug.Assert(enumerator.Current != null); - if (hitObjectCanBlockFutureHits(enumerator.Current)) - blockingObject = enumerator.Current; + if (hitObjectCanBlockFutureHits(enumerator.Current)) + blockingObject = enumerator.Current; + } } // If there is no previous hitobject, allow the hit. @@ -76,17 +77,18 @@ namespace osu.Game.Rulesets.Osu.UI if (!IsHittable(hitObject, hitObject.HitObject.StartTime + hitObject.Result.TimeOffset)) throw new InvalidOperationException($"A {hitObject} was hit before it become hittable!"); - var enumerator = new HitObjectEnumerator(hitObjectContainer, hitObject.HitObject.StartTime); - - while (enumerator.MoveNext()) + using (var enumerator = new HitObjectEnumerator(hitObjectContainer, hitObject.HitObject.StartTime)) { - Debug.Assert(enumerator.Current != null); + while (enumerator.MoveNext()) + { + Debug.Assert(enumerator.Current != null); - if (enumerator.Current.Judged) - continue; + if (enumerator.Current.Judged) + continue; - if (hitObjectCanBlockFutureHits(enumerator.Current)) - ((DrawableOsuHitObject)enumerator.Current).MissForcefully(); + if (hitObjectCanBlockFutureHits(enumerator.Current)) + ((DrawableOsuHitObject)enumerator.Current).MissForcefully(); + } } } @@ -180,6 +182,7 @@ namespace osu.Game.Rulesets.Osu.UI public void Dispose() { + hitObjectEnumerator?.Dispose(); } } } From bbcbd7e3fbc790e91415bc96ffcfb93c63bcffc6 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 17 Apr 2020 14:48:12 +0900 Subject: [PATCH 31/35] Simplify by removing custom enumerator --- osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs | 118 +++---------------- 1 file changed, 19 insertions(+), 99 deletions(-) diff --git a/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs b/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs index 4bc7da4794..cd9838e7bf 100644 --- a/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs +++ b/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs @@ -2,9 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; -using System.Collections; using System.Collections.Generic; -using System.Diagnostics; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Drawables; using osu.Game.Rulesets.Osu.Objects.Drawables; @@ -39,15 +37,10 @@ namespace osu.Game.Rulesets.Osu.UI { DrawableHitObject blockingObject = null; - using (var enumerator = new HitObjectEnumerator(hitObjectContainer, hitObject.HitObject.StartTime)) + foreach (var obj in enumerateHitObjectsUpTo(hitObject.HitObject.StartTime)) { - while (enumerator.MoveNext()) - { - Debug.Assert(enumerator.Current != null); - - if (hitObjectCanBlockFutureHits(enumerator.Current)) - blockingObject = enumerator.Current; - } + if (hitObjectCanBlockFutureHits(obj)) + blockingObject = obj; } // If there is no previous hitobject, allow the hit. @@ -77,18 +70,13 @@ namespace osu.Game.Rulesets.Osu.UI if (!IsHittable(hitObject, hitObject.HitObject.StartTime + hitObject.Result.TimeOffset)) throw new InvalidOperationException($"A {hitObject} was hit before it become hittable!"); - using (var enumerator = new HitObjectEnumerator(hitObjectContainer, hitObject.HitObject.StartTime)) + foreach (var obj in enumerateHitObjectsUpTo(hitObject.HitObject.StartTime)) { - while (enumerator.MoveNext()) - { - Debug.Assert(enumerator.Current != null); + if (obj.Judged) + continue; - if (enumerator.Current.Judged) - continue; - - if (hitObjectCanBlockFutureHits(enumerator.Current)) - ((DrawableOsuHitObject)enumerator.Current).MissForcefully(); - } + if (hitObjectCanBlockFutureHits(obj)) + ((DrawableOsuHitObject)obj).MissForcefully(); } } @@ -99,90 +87,22 @@ namespace osu.Game.Rulesets.Osu.UI private static bool hitObjectCanBlockFutureHits(DrawableHitObject hitObject) => hitObject is DrawableHitCircle; - private struct HitObjectEnumerator : IEnumerator + private IEnumerable enumerateHitObjectsUpTo(double targetTime) { - private readonly IEnumerator hitObjectEnumerator; - private readonly double targetTime; - - private DrawableHitObject currentTopLevel; - private int currentNestedIndex; - - public HitObjectEnumerator(HitObjectContainer hitObjectContainer, double targetTime) + foreach (var obj in hitObjectContainer.AliveObjects) { - hitObjectEnumerator = hitObjectContainer.AliveObjects.GetEnumerator(); - this.targetTime = targetTime; + if (obj.HitObject.StartTime >= targetTime) + yield break; - currentTopLevel = null; - currentNestedIndex = -1; - Current = null; - } + yield return obj; - /// - /// Attempts to move to the next top-level or nested hitobject. - /// Stops when no such hitobject is found or until the hitobject start time reaches . - /// - /// Whether a new hitobject was moved to. - public bool MoveNext() - { - // If we don't already have a top-level hitobject, try to get one. - if (currentTopLevel == null) - return moveNextTopLevel(); + for (int i = 0; i < obj.NestedHitObjects.Count; i++) + { + if (obj.NestedHitObjects[i].HitObject.StartTime >= targetTime) + break; - // If we have a top-level hitobject, try to move to the next nested hitobject or otherwise move to the next top-level hitobject. - if (!moveNextNested()) - return moveNextTopLevel(); - - // Guaranteed by moveNextNested() to have a hitobject. - return true; - } - - /// - /// Attempts to move to the next top-level hitobject. - /// - /// Whether a new top-level hitobject was found. - private bool moveNextTopLevel() - { - currentNestedIndex = -1; - - hitObjectEnumerator.MoveNext(); - currentTopLevel = hitObjectEnumerator.Current; - - Current = currentTopLevel; - - return Current?.HitObject.StartTime < targetTime; - } - - /// - /// Attempts to move to the next nested hitobject in the current top-level hitobject. - /// - /// Whether a new nested hitobject was moved to. - private bool moveNextNested() - { - currentNestedIndex++; - if (currentNestedIndex >= currentTopLevel.NestedHitObjects.Count) - return false; - - Current = currentTopLevel.NestedHitObjects[currentNestedIndex]; - Debug.Assert(Current != null); - - return Current?.HitObject.StartTime < targetTime; - } - - public void Reset() - { - hitObjectEnumerator.Reset(); - currentTopLevel = null; - currentNestedIndex = -1; - Current = null; - } - - public DrawableHitObject Current { get; set; } - - object IEnumerator.Current => Current; - - public void Dispose() - { - hitObjectEnumerator?.Dispose(); + yield return obj.NestedHitObjects[i]; + } } } } 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 32/35] 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; } } } From 28318a0140a4e3854d81896e7a762a451f8d7637 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 20 Apr 2020 10:59:08 +0900 Subject: [PATCH 33/35] Add mention of notelock in xmldoc (potentially easier to find class) --- osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs b/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs index cd9838e7bf..176402c831 100644 --- a/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs +++ b/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs @@ -11,7 +11,7 @@ using osu.Game.Rulesets.UI; namespace osu.Game.Rulesets.Osu.UI { /// - /// Ensures that s are hit in-order. + /// Ensures that s are hit in-order. Affectionately known as "note lock". /// If a is hit out of order: /// /// The hit is blocked if it occurred earlier than the previous 's start time. From e1acfd26a6849be6cee96edc66ae58c17e9a1fac Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 20 Apr 2020 10:59:44 +0900 Subject: [PATCH 34/35] Simplify return logic --- osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs b/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs index 176402c831..1f027e9726 100644 --- a/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs +++ b/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs @@ -51,10 +51,7 @@ namespace osu.Game.Rulesets.Osu.UI // 1. The last blocking hitobject has been judged. // 2. The current time is after the last hitobject's start time. // Hits at exactly the same time as the blocking hitobject are allowed for maps that contain simultaneous hitobjects (e.g. /b/372245). - if (blockingObject.Judged || time >= blockingObject.HitObject.StartTime) - return true; - - return false; + return blockingObject.Judged || time >= blockingObject.HitObject.StartTime; } /// From 8c85602ad013001281d4148b6109d606d6885c8c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 20 Apr 2020 11:00:42 +0900 Subject: [PATCH 35/35] Use foreach for conformity --- osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs b/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs index 1f027e9726..53dd1127d6 100644 --- a/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs +++ b/osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs @@ -93,12 +93,12 @@ namespace osu.Game.Rulesets.Osu.UI yield return obj; - for (int i = 0; i < obj.NestedHitObjects.Count; i++) + foreach (var nestedObj in obj.NestedHitObjects) { - if (obj.NestedHitObjects[i].HitObject.StartTime >= targetTime) + if (nestedObj.HitObject.StartTime >= targetTime) break; - yield return obj.NestedHitObjects[i]; + yield return nestedObj; } } }