From 281671a213f26a4d71bcc920205994b1f10c5db0 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Wed, 9 Oct 2019 21:04:26 +0300 Subject: [PATCH 01/29] Remove default combo colours usage from skins --- osu.Game/Skinning/LegacySkin.cs | 2 +- osu.Game/Skinning/LegacySkinDecoder.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index fea15458e4..3f0d4ca0d6 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -39,7 +39,7 @@ namespace osu.Game.Skinning using (LineBufferedReader reader = new LineBufferedReader(stream)) Configuration = new LegacySkinDecoder().Decode(reader); else - Configuration = new DefaultSkinConfiguration(); + Configuration = new SkinConfiguration(); if (storage != null) { diff --git a/osu.Game/Skinning/LegacySkinDecoder.cs b/osu.Game/Skinning/LegacySkinDecoder.cs index e97664e75e..ada2e075a7 100644 --- a/osu.Game/Skinning/LegacySkinDecoder.cs +++ b/osu.Game/Skinning/LegacySkinDecoder.cs @@ -5,14 +5,14 @@ using osu.Game.Beatmaps.Formats; namespace osu.Game.Skinning { - public class LegacySkinDecoder : LegacyDecoder + public class LegacySkinDecoder : LegacyDecoder { public LegacySkinDecoder() : base(1) { } - protected override void ParseLine(DefaultSkinConfiguration skin, Section section, string line) + protected override void ParseLine(SkinConfiguration skin, Section section, string line) { if (section != Section.Colours) { From 5e3f0f6c95f88845946bae4f96879628e42d9be7 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Wed, 9 Oct 2019 21:08:07 +0300 Subject: [PATCH 02/29] Return default combo colours if none provided --- osu.Game/Beatmaps/Formats/LegacyDecoder.cs | 9 --------- osu.Game/Skinning/LegacyBeatmapSkin.cs | 3 +++ osu.Game/Skinning/LegacySkin.cs | 12 +++++++++++- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/osu.Game/Beatmaps/Formats/LegacyDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyDecoder.cs index 83d20da458..4899a53cf1 100644 --- a/osu.Game/Beatmaps/Formats/LegacyDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyDecoder.cs @@ -77,8 +77,6 @@ namespace osu.Game.Beatmaps.Formats return line; } - private bool hasComboColours; - private void handleColours(T output, string line) { var pair = SplitKeyVal(line); @@ -105,13 +103,6 @@ namespace osu.Game.Beatmaps.Formats { if (!(output is IHasComboColours tHasComboColours)) return; - if (!hasComboColours) - { - // remove default colours. - tHasComboColours.ComboColours.Clear(); - hasComboColours = true; - } - tHasComboColours.ComboColours.Add(colour); } else diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index 6770da3c66..c9dec8a55e 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.cs @@ -9,6 +9,9 @@ namespace osu.Game.Skinning { public class LegacyBeatmapSkin : LegacySkin { + // Null should be returned in the case of no colours provided to fallback into current skin's colours. + protected override bool AllowDefaultColoursFallback => false; + public LegacyBeatmapSkin(BeatmapInfo beatmap, IResourceStore storage, AudioManager audioManager) : base(createSkinInfo(beatmap), new LegacySkinResourceStore(beatmap.BeatmapSet, storage), audioManager, beatmap.Path) { diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 3f0d4ca0d6..bc12c00cd8 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -26,6 +26,11 @@ namespace osu.Game.Skinning [CanBeNull] protected IResourceStore Samples; + /// + /// Whether to allow default combo colours as fallback if none provided in this skin. + /// + protected virtual bool AllowDefaultColoursFallback => true; + public LegacySkin(SkinInfo skin, IResourceStore storage, AudioManager audioManager) : this(skin, new LegacySkinResourceStore(skin, storage), audioManager, "skin.ini") { @@ -63,7 +68,12 @@ namespace osu.Game.Skinning switch (global) { case GlobalSkinConfiguration.ComboColours: - return SkinUtils.As(new Bindable>(Configuration.ComboColours)); + if (Configuration.ComboColours.Any()) + return SkinUtils.As(new Bindable>(Configuration.ComboColours)); + else if (AllowDefaultColoursFallback) + return SkinUtils.As(new Bindable>(new DefaultSkinConfiguration().ComboColours)); + + break; } break; From c2ada81c2313f77d5d7c19205da7dc92ee303e23 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Wed, 9 Oct 2019 21:08:54 +0300 Subject: [PATCH 03/29] Add tests ensuring correct behaviour --- .../Skins/TestSceneSkinConfigurationLookup.cs | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs b/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs index 578030748b..2ea66d8e41 100644 --- a/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs +++ b/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; +using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Audio.Sample; @@ -21,8 +22,8 @@ namespace osu.Game.Tests.Skins [HeadlessTest] public class TestSceneSkinConfigurationLookup : OsuTestScene { - private LegacySkin source1; - private LegacySkin source2; + private SkinSource source1; + private SkinSource source2; private SkinRequester requester; [SetUp] @@ -116,6 +117,26 @@ namespace osu.Game.Tests.Skins }); } + [TestCase(false)] + [TestCase(true)] + public void TestEmptyComboColours(bool allowFallback) + { + AddStep("Add custom combo colours to fallback source", () => source1.Configuration.ComboColours = new List + { + new Color4(100, 150, 200, 255), + new Color4(55, 110, 166, 255), + new Color4(75, 125, 175, 255), + }); + AddStep("Clear combo colours from source", () => source2.Configuration.ComboColours.Clear()); + AddStep("Disable default fallback in source", () => source2.AllowColoursFallback = allowFallback); + + AddAssert($"Check retrieved combo colours from {(allowFallback ? "default skin" : "fallback source")}", () => + { + var expectedColours = allowFallback ? new DefaultSkinConfiguration().ComboColours : source1.Configuration.ComboColours; + return requester.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value?.SequenceEqual(expectedColours) ?? false; + }); + } + public enum LookupType { Test @@ -130,6 +151,9 @@ namespace osu.Game.Tests.Skins public class SkinSource : LegacySkin { + public bool AllowColoursFallback = true; + protected override bool AllowDefaultColoursFallback => AllowColoursFallback; + public SkinSource() : base(new SkinInfo(), null, null, string.Empty) { From 8d40c1b733d9b1800474c7b0d8dba4e36684080c Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Wed, 9 Oct 2019 21:15:30 +0300 Subject: [PATCH 04/29] Remove default combo colours on empty skin test Checked differently in TestSceneSkinConfigurationLookup.TestEmptyComboColours() --- osu.Game.Tests/Resources/skin-empty.ini | 2 -- osu.Game.Tests/Skins/LegacySkinDecoderTest.cs | 25 ++++++++----------- 2 files changed, 10 insertions(+), 17 deletions(-) delete mode 100644 osu.Game.Tests/Resources/skin-empty.ini diff --git a/osu.Game.Tests/Resources/skin-empty.ini b/osu.Game.Tests/Resources/skin-empty.ini deleted file mode 100644 index b6c319fe3c..0000000000 --- a/osu.Game.Tests/Resources/skin-empty.ini +++ /dev/null @@ -1,2 +0,0 @@ -[General] -Name: test skin \ No newline at end of file diff --git a/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs b/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs index 0d96dd08da..76ce2331f5 100644 --- a/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs +++ b/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs @@ -13,28 +13,23 @@ namespace osu.Game.Tests.Skins [TestFixture] public class LegacySkinDecoderTest { - [TestCase(true)] - [TestCase(false)] - public void TestDecodeSkinColours(bool hasColours) + [Test] + public void TestDecodeSkinColours() { var decoder = new LegacySkinDecoder(); - using (var resStream = TestResources.OpenResource(hasColours ? "skin.ini" : "skin-empty.ini")) + using (var resStream = TestResources.OpenResource("skin.ini")) using (var stream = new LineBufferedReader(resStream)) { var comboColors = decoder.Decode(stream).ComboColours; - List expectedColors; - if (hasColours) - expectedColors = new List - { - new Color4(142, 199, 255, 255), - new Color4(255, 128, 128, 255), - new Color4(128, 255, 255, 255), - new Color4(100, 100, 100, 100), - }; - else - expectedColors = new DefaultSkin().Configuration.ComboColours; + List expectedColors = new List + { + new Color4(142, 199, 255, 255), + new Color4(255, 128, 128, 255), + new Color4(128, 255, 255, 255), + new Color4(100, 100, 100, 100), + }; Assert.AreEqual(expectedColors.Count, comboColors.Count); for (int i = 0; i < expectedColors.Count; i++) From cef6e2a26b49cdecd9663aa4af586292c8e8b4e6 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Wed, 6 Nov 2019 23:20:36 +0300 Subject: [PATCH 05/29] Move colours fallback logic to SkinConfiguration.ComboColours getter --- osu.Game/Skinning/DefaultSkinConfiguration.cs | 10 +------ osu.Game/Skinning/LegacySkin.cs | 9 ++++--- osu.Game/Skinning/SkinConfiguration.cs | 27 ++++++++++++++++++- 3 files changed, 32 insertions(+), 14 deletions(-) diff --git a/osu.Game/Skinning/DefaultSkinConfiguration.cs b/osu.Game/Skinning/DefaultSkinConfiguration.cs index cd5975edac..8e89ab25b2 100644 --- a/osu.Game/Skinning/DefaultSkinConfiguration.cs +++ b/osu.Game/Skinning/DefaultSkinConfiguration.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using osuTK.Graphics; - namespace osu.Game.Skinning { /// @@ -12,13 +10,7 @@ namespace osu.Game.Skinning { public DefaultSkinConfiguration() { - ComboColours.AddRange(new[] - { - new Color4(255, 192, 0, 255), - new Color4(0, 202, 0, 255), - new Color4(18, 124, 255, 255), - new Color4(242, 24, 57, 255), - }); + ComboColours = DefaultComboColours; } } } diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index bc12c00cd8..ff1e501a06 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -46,6 +46,8 @@ namespace osu.Game.Skinning else Configuration = new SkinConfiguration(); + Configuration.AllowDefaultColoursFallback = AllowDefaultColoursFallback; + if (storage != null) { Samples = audioManager?.GetSampleStore(storage); @@ -68,10 +70,9 @@ namespace osu.Game.Skinning switch (global) { case GlobalSkinConfiguration.ComboColours: - if (Configuration.ComboColours.Any()) - return SkinUtils.As(new Bindable>(Configuration.ComboColours)); - else if (AllowDefaultColoursFallback) - return SkinUtils.As(new Bindable>(new DefaultSkinConfiguration().ComboColours)); + var comboColours = Configuration.ComboColours; + if (comboColours != null) + return SkinUtils.As(new Bindable>(comboColours)); break; } diff --git a/osu.Game/Skinning/SkinConfiguration.cs b/osu.Game/Skinning/SkinConfiguration.cs index 54aac86e3c..57b969be33 100644 --- a/osu.Game/Skinning/SkinConfiguration.cs +++ b/osu.Game/Skinning/SkinConfiguration.cs @@ -14,7 +14,32 @@ namespace osu.Game.Skinning { public readonly SkinInfo SkinInfo = new SkinInfo(); - public List ComboColours { get; set; } = new List(); + internal bool AllowDefaultColoursFallback; + + public static List DefaultComboColours = new List + { + new Color4(255, 192, 0, 255), + new Color4(0, 202, 0, 255), + new Color4(18, 124, 255, 255), + new Color4(242, 24, 57, 255), + }; + + private List comboColours = new List(); + + public List ComboColours + { + get + { + if (comboColours.Count > 0) + return comboColours; + + if (AllowDefaultColoursFallback) + return DefaultComboColours; + + return null; + } + set => comboColours = value; + } public Dictionary CustomColours { get; set; } = new Dictionary(); From 61778232d8b76436458a87cd60a18d2443159d6d Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Wed, 6 Nov 2019 23:24:54 +0300 Subject: [PATCH 06/29] Rewrite inline comment --- osu.Game/Skinning/LegacyBeatmapSkin.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index c9dec8a55e..4e020083a7 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.cs @@ -9,7 +9,7 @@ namespace osu.Game.Skinning { public class LegacyBeatmapSkin : LegacySkin { - // Null should be returned in the case of no colours provided to fallback into current skin's colours. + // Disallow default colours fallback on beatmap skins to allow using parent skin combo colours. (via SkinProvidingContainer) protected override bool AllowDefaultColoursFallback => false; public LegacyBeatmapSkin(BeatmapInfo beatmap, IResourceStore storage, AudioManager audioManager) From cea5bb396358c675b9d80327cb849d4c47b6aca2 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Thu, 7 Nov 2019 15:50:26 +0300 Subject: [PATCH 07/29] Return skin-empty.ini back --- osu.Game.Tests/Resources/skin-empty.ini | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 osu.Game.Tests/Resources/skin-empty.ini diff --git a/osu.Game.Tests/Resources/skin-empty.ini b/osu.Game.Tests/Resources/skin-empty.ini new file mode 100644 index 0000000000..b6c319fe3c --- /dev/null +++ b/osu.Game.Tests/Resources/skin-empty.ini @@ -0,0 +1,2 @@ +[General] +Name: test skin \ No newline at end of file From 41515e9e6cd87857af9a7b1ccbb71195f3691d86 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Thu, 7 Nov 2019 15:51:03 +0300 Subject: [PATCH 08/29] Update current tests to match the expected behaviour --- osu.Game.Tests/Skins/LegacySkinDecoderTest.cs | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs b/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs index 76ce2331f5..202161a1b8 100644 --- a/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs +++ b/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs @@ -13,23 +13,38 @@ namespace osu.Game.Tests.Skins [TestFixture] public class LegacySkinDecoderTest { - [Test] - public void TestDecodeSkinColours() + [TestCase(true)] + [TestCase(false)] + [TestCase(false, false)] + public void TestDecodeSkinColours(bool hasColours, bool canFallback = true) { var decoder = new LegacySkinDecoder(); - using (var resStream = TestResources.OpenResource("skin.ini")) + using (var resStream = TestResources.OpenResource(hasColours ? "skin.ini" : "skin-empty.ini")) using (var stream = new LineBufferedReader(resStream)) { - var comboColors = decoder.Decode(stream).ComboColours; + var skinConfiguration = decoder.Decode(stream); + skinConfiguration.AllowDefaultComboColoursFallback = canFallback; - List expectedColors = new List + var comboColors = skinConfiguration.ComboColours; + + if (!canFallback && !hasColours) + { + Assert.IsNull(comboColors); + return; + } + + List expectedColors; + if (hasColours) + expectedColors = new List { new Color4(142, 199, 255, 255), new Color4(255, 128, 128, 255), new Color4(128, 255, 255, 255), new Color4(100, 100, 100, 100), }; + else + expectedColors = SkinConfiguration.DefaultComboColours; Assert.AreEqual(expectedColors.Count, comboColors.Count); for (int i = 0; i < expectedColors.Count; i++) From 808543885f34f26b995a6446d6b7a910fe1b0ea8 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Thu, 7 Nov 2019 15:54:30 +0300 Subject: [PATCH 09/29] Change ComboColours type to IReadOnlyList Also exposes functions to modify the internal list (AddComboColours, ClearComboColours) --- .../Skins/TestSceneSkinConfigurationLookup.cs | 17 +++++++---------- osu.Game/Beatmaps/Formats/IHasComboColours.cs | 15 ++++++++++++++- osu.Game/Beatmaps/Formats/LegacyDecoder.cs | 2 +- .../Objects/Drawables/DrawableHitObject.cs | 2 +- osu.Game/Skinning/DefaultLegacySkin.cs | 7 +++---- osu.Game/Skinning/DefaultSkin.cs | 2 +- osu.Game/Skinning/LegacySkin.cs | 2 +- osu.Game/Skinning/SkinConfiguration.cs | 10 ++++++++-- 8 files changed, 36 insertions(+), 21 deletions(-) diff --git a/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs b/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs index 2ea66d8e41..c945568753 100644 --- a/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs +++ b/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs @@ -95,7 +95,7 @@ namespace osu.Game.Tests.Skins [Test] public void TestGlobalLookup() { - AddAssert("Check combo colours", () => requester.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value?.Count > 0); + AddAssert("Check combo colours", () => requester.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value?.Count > 0); } [Test] @@ -121,19 +121,19 @@ namespace osu.Game.Tests.Skins [TestCase(true)] public void TestEmptyComboColours(bool allowFallback) { - AddStep("Add custom combo colours to fallback source", () => source1.Configuration.ComboColours = new List + AddStep("Add custom combo colours to source1", () => source1.Configuration.ComboColours = new List { new Color4(100, 150, 200, 255), new Color4(55, 110, 166, 255), new Color4(75, 125, 175, 255), }); - AddStep("Clear combo colours from source", () => source2.Configuration.ComboColours.Clear()); - AddStep("Disable default fallback in source", () => source2.AllowColoursFallback = allowFallback); + AddStep("Clear combo colours from source2", () => source2.Configuration.ClearComboColours()); + AddStep("Disallow default colours fallback in source2", () => source2.Configuration.AllowDefaultComboColoursFallback = allowFallback); - AddAssert($"Check retrieved combo colours from {(allowFallback ? "default skin" : "fallback source")}", () => + AddAssert($"Check retrieved combo colours from {(allowFallback ? "source1" : "fallback source")}", () => { - var expectedColours = allowFallback ? new DefaultSkinConfiguration().ComboColours : source1.Configuration.ComboColours; - return requester.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value?.SequenceEqual(expectedColours) ?? false; + var expectedColours = allowFallback ? SkinConfiguration.DefaultComboColours : source1.Configuration.ComboColours; + return requester.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value?.SequenceEqual(expectedColours) ?? false; }); } @@ -151,9 +151,6 @@ namespace osu.Game.Tests.Skins public class SkinSource : LegacySkin { - public bool AllowColoursFallback = true; - protected override bool AllowDefaultColoursFallback => AllowColoursFallback; - public SkinSource() : base(new SkinInfo(), null, null, string.Empty) { diff --git a/osu.Game/Beatmaps/Formats/IHasComboColours.cs b/osu.Game/Beatmaps/Formats/IHasComboColours.cs index 4c15cb96d1..5ef31c1904 100644 --- a/osu.Game/Beatmaps/Formats/IHasComboColours.cs +++ b/osu.Game/Beatmaps/Formats/IHasComboColours.cs @@ -8,6 +8,19 @@ namespace osu.Game.Beatmaps.Formats { public interface IHasComboColours { - List ComboColours { get; set; } + /// + /// Retrieves the list of combo colours for presentation only. + /// + IReadOnlyList ComboColours { get; set; } + + /// + /// Adds combo colours to the list. + /// + void AddComboColours(params Color4[] colours); + + /// + /// Clear current combo colours from the list. + /// + void ClearComboColours(); } } diff --git a/osu.Game/Beatmaps/Formats/LegacyDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyDecoder.cs index 4899a53cf1..ddaa04b657 100644 --- a/osu.Game/Beatmaps/Formats/LegacyDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyDecoder.cs @@ -103,7 +103,7 @@ namespace osu.Game.Beatmaps.Formats { if (!(output is IHasComboColours tHasComboColours)) return; - tHasComboColours.ComboColours.Add(colour); + tHasComboColours.AddComboColours(colour); } else { diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index f8bc74b2a6..09ff7bfded 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -265,7 +265,7 @@ namespace osu.Game.Rulesets.Objects.Drawables { if (HitObject is IHasComboInformation combo) { - var comboColours = CurrentSkin.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value; + var comboColours = CurrentSkin.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value; AccentColour.Value = comboColours?.Count > 0 ? comboColours[combo.ComboIndex % comboColours.Count] : Color4.White; } } diff --git a/osu.Game/Skinning/DefaultLegacySkin.cs b/osu.Game/Skinning/DefaultLegacySkin.cs index 4b6eea6b6e..6eda0dfb34 100644 --- a/osu.Game/Skinning/DefaultLegacySkin.cs +++ b/osu.Game/Skinning/DefaultLegacySkin.cs @@ -13,13 +13,12 @@ namespace osu.Game.Skinning : base(Info, storage, audioManager, string.Empty) { Configuration.CustomColours["SliderBall"] = new Color4(2, 170, 255, 255); - Configuration.ComboColours.AddRange(new[] - { + Configuration.AddComboColours( new Color4(255, 192, 0, 255), new Color4(0, 202, 0, 255), new Color4(18, 124, 255, 255), - new Color4(242, 24, 57, 255), - }); + new Color4(242, 24, 57, 255) + ); } public static SkinInfo Info { get; } = new SkinInfo diff --git a/osu.Game/Skinning/DefaultSkin.cs b/osu.Game/Skinning/DefaultSkin.cs index 529c1afca5..2a065ea3d7 100644 --- a/osu.Game/Skinning/DefaultSkin.cs +++ b/osu.Game/Skinning/DefaultSkin.cs @@ -35,7 +35,7 @@ namespace osu.Game.Skinning switch (global) { case GlobalSkinConfiguration.ComboColours: - return SkinUtils.As(new Bindable>(Configuration.ComboColours)); + return SkinUtils.As(new Bindable>(Configuration.ComboColours)); } break; diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index ff1e501a06..94c2c8668f 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -72,7 +72,7 @@ namespace osu.Game.Skinning case GlobalSkinConfiguration.ComboColours: var comboColours = Configuration.ComboColours; if (comboColours != null) - return SkinUtils.As(new Bindable>(comboColours)); + return SkinUtils.As(new Bindable>(comboColours)); break; } diff --git a/osu.Game/Skinning/SkinConfiguration.cs b/osu.Game/Skinning/SkinConfiguration.cs index 57b969be33..1fd781a5b3 100644 --- a/osu.Game/Skinning/SkinConfiguration.cs +++ b/osu.Game/Skinning/SkinConfiguration.cs @@ -2,6 +2,8 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; +using System.Linq; +using osu.Framework.Extensions.IEnumerableExtensions; using osu.Game.Beatmaps.Formats; using osuTK.Graphics; @@ -26,7 +28,7 @@ namespace osu.Game.Skinning private List comboColours = new List(); - public List ComboColours + public IReadOnlyList ComboColours { get { @@ -38,9 +40,13 @@ namespace osu.Game.Skinning return null; } - set => comboColours = value; + set => comboColours = value.ToList(); } + public void AddComboColours(params Color4[] colours) => colours.ForEach(c => comboColours.Add(c)); + + public void ClearComboColours() => comboColours.Clear(); + public Dictionary CustomColours { get; set; } = new Dictionary(); public readonly Dictionary ConfigDictionary = new Dictionary(); From 9874ce49ce6f81165862961ee6244b2267629c9a Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Thu, 7 Nov 2019 15:55:34 +0300 Subject: [PATCH 10/29] Move fallback allowance to the skin configuration only. --- osu.Game/Skinning/LegacyBeatmapSkin.cs | 5 ++--- osu.Game/Skinning/LegacySkin.cs | 7 ------- osu.Game/Skinning/SkinConfiguration.cs | 7 +++++-- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index 4e020083a7..fa7e895a28 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.cs @@ -9,12 +9,11 @@ namespace osu.Game.Skinning { public class LegacyBeatmapSkin : LegacySkin { - // Disallow default colours fallback on beatmap skins to allow using parent skin combo colours. (via SkinProvidingContainer) - protected override bool AllowDefaultColoursFallback => false; - public LegacyBeatmapSkin(BeatmapInfo beatmap, IResourceStore storage, AudioManager audioManager) : base(createSkinInfo(beatmap), new LegacySkinResourceStore(beatmap.BeatmapSet, storage), audioManager, beatmap.Path) { + // Disallow default colours fallback on beatmap skins to allow using parent skin combo colours. (via SkinProvidingContainer) + Configuration.AllowDefaultComboColoursFallback = false; } private static SkinInfo createSkinInfo(BeatmapInfo beatmap) => diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 94c2c8668f..359acc3a8f 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -26,11 +26,6 @@ namespace osu.Game.Skinning [CanBeNull] protected IResourceStore Samples; - /// - /// Whether to allow default combo colours as fallback if none provided in this skin. - /// - protected virtual bool AllowDefaultColoursFallback => true; - public LegacySkin(SkinInfo skin, IResourceStore storage, AudioManager audioManager) : this(skin, new LegacySkinResourceStore(skin, storage), audioManager, "skin.ini") { @@ -46,8 +41,6 @@ namespace osu.Game.Skinning else Configuration = new SkinConfiguration(); - Configuration.AllowDefaultColoursFallback = AllowDefaultColoursFallback; - if (storage != null) { Samples = audioManager?.GetSampleStore(storage); diff --git a/osu.Game/Skinning/SkinConfiguration.cs b/osu.Game/Skinning/SkinConfiguration.cs index 1fd781a5b3..da833acd98 100644 --- a/osu.Game/Skinning/SkinConfiguration.cs +++ b/osu.Game/Skinning/SkinConfiguration.cs @@ -16,7 +16,10 @@ namespace osu.Game.Skinning { public readonly SkinInfo SkinInfo = new SkinInfo(); - internal bool AllowDefaultColoursFallback; + /// + /// Whether to allow as a fallback list for when no combo colours are provided. + /// + internal bool AllowDefaultComboColoursFallback = true; public static List DefaultComboColours = new List { @@ -35,7 +38,7 @@ namespace osu.Game.Skinning if (comboColours.Count > 0) return comboColours; - if (AllowDefaultColoursFallback) + if (AllowDefaultComboColoursFallback) return DefaultComboColours; return null; From 164cb66f6aaa001204b2b2c9054d5e6c3fa47bf5 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Thu, 7 Nov 2019 16:13:10 +0300 Subject: [PATCH 11/29] Fix indention --- osu.Game.Tests/Skins/LegacySkinDecoderTest.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs b/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs index 202161a1b8..e2f556dcc3 100644 --- a/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs +++ b/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs @@ -35,7 +35,9 @@ namespace osu.Game.Tests.Skins } List expectedColors; + if (hasColours) + { expectedColors = new List { new Color4(142, 199, 255, 255), @@ -43,8 +45,11 @@ namespace osu.Game.Tests.Skins new Color4(128, 255, 255, 255), new Color4(100, 100, 100, 100), }; + } else + { expectedColors = SkinConfiguration.DefaultComboColours; + } Assert.AreEqual(expectedColors.Count, comboColors.Count); for (int i = 0; i < expectedColors.Count; i++) From 8ed7bc3f5366ec7b8a98b47439616280d6bc4f25 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Thu, 7 Nov 2019 18:42:17 +0300 Subject: [PATCH 12/29] Fix another indention --- osu.Game.Tests/Skins/LegacySkinDecoderTest.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs b/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs index e2f556dcc3..003b3f5e56 100644 --- a/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs +++ b/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs @@ -39,12 +39,12 @@ namespace osu.Game.Tests.Skins if (hasColours) { expectedColors = new List - { - new Color4(142, 199, 255, 255), - new Color4(255, 128, 128, 255), - new Color4(128, 255, 255, 255), - new Color4(100, 100, 100, 100), - }; + { + new Color4(142, 199, 255, 255), + new Color4(255, 128, 128, 255), + new Color4(128, 255, 255, 255), + new Color4(100, 100, 100, 100), + }; } else { From 517f547590462f6fe26b6cdbe1b55814f5847141 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Thu, 7 Nov 2019 19:12:18 +0300 Subject: [PATCH 13/29] Fix failing test --- osu.Game.Tests/Gameplay/TestSceneHitObjectAccentColour.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Gameplay/TestSceneHitObjectAccentColour.cs b/osu.Game.Tests/Gameplay/TestSceneHitObjectAccentColour.cs index 6d7159a825..c6d1f9da29 100644 --- a/osu.Game.Tests/Gameplay/TestSceneHitObjectAccentColour.cs +++ b/osu.Game.Tests/Gameplay/TestSceneHitObjectAccentColour.cs @@ -130,7 +130,7 @@ namespace osu.Game.Tests.Gameplay switch (global) { case GlobalSkinConfiguration.ComboColours: - return SkinUtils.As(new Bindable>(ComboColours)); + return SkinUtils.As(new Bindable>(ComboColours)); } break; From 1e24ee795613543c1d32756a374a6ec2168a7540 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Sun, 24 Nov 2019 02:07:56 +0300 Subject: [PATCH 14/29] Remove ClearComboColours() --- osu.Game/Beatmaps/Formats/IHasComboColours.cs | 5 ----- osu.Game/Skinning/SkinConfiguration.cs | 2 -- 2 files changed, 7 deletions(-) diff --git a/osu.Game/Beatmaps/Formats/IHasComboColours.cs b/osu.Game/Beatmaps/Formats/IHasComboColours.cs index 5ef31c1904..ac0d32dbaa 100644 --- a/osu.Game/Beatmaps/Formats/IHasComboColours.cs +++ b/osu.Game/Beatmaps/Formats/IHasComboColours.cs @@ -17,10 +17,5 @@ namespace osu.Game.Beatmaps.Formats /// Adds combo colours to the list. /// void AddComboColours(params Color4[] colours); - - /// - /// Clear current combo colours from the list. - /// - void ClearComboColours(); } } diff --git a/osu.Game/Skinning/SkinConfiguration.cs b/osu.Game/Skinning/SkinConfiguration.cs index da833acd98..94a1160b38 100644 --- a/osu.Game/Skinning/SkinConfiguration.cs +++ b/osu.Game/Skinning/SkinConfiguration.cs @@ -48,8 +48,6 @@ namespace osu.Game.Skinning public void AddComboColours(params Color4[] colours) => colours.ForEach(c => comboColours.Add(c)); - public void ClearComboColours() => comboColours.Clear(); - public Dictionary CustomColours { get; set; } = new Dictionary(); public readonly Dictionary ConfigDictionary = new Dictionary(); From 0f9978b34a8e250ba6458dd9ecee6eb90529a403 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Sun, 24 Nov 2019 02:08:36 +0300 Subject: [PATCH 15/29] Use AddRange instead --- osu.Game/Skinning/SkinConfiguration.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Skinning/SkinConfiguration.cs b/osu.Game/Skinning/SkinConfiguration.cs index 94a1160b38..0a21594f2a 100644 --- a/osu.Game/Skinning/SkinConfiguration.cs +++ b/osu.Game/Skinning/SkinConfiguration.cs @@ -46,7 +46,7 @@ namespace osu.Game.Skinning set => comboColours = value.ToList(); } - public void AddComboColours(params Color4[] colours) => colours.ForEach(c => comboColours.Add(c)); + public void AddComboColours(params Color4[] colours) => comboColours.AddRange(colours); public Dictionary CustomColours { get; set; } = new Dictionary(); From 5bf6e57eb0f19b4fedc09d06ca1e5d52cef5b129 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Sun, 24 Nov 2019 02:16:43 +0300 Subject: [PATCH 16/29] Remove unnecessary usage --- osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs | 1 - osu.Game/Skinning/SkinConfiguration.cs | 1 - 2 files changed, 2 deletions(-) diff --git a/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs b/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs index c945568753..c300799476 100644 --- a/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs +++ b/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs @@ -127,7 +127,6 @@ namespace osu.Game.Tests.Skins new Color4(55, 110, 166, 255), new Color4(75, 125, 175, 255), }); - AddStep("Clear combo colours from source2", () => source2.Configuration.ClearComboColours()); AddStep("Disallow default colours fallback in source2", () => source2.Configuration.AllowDefaultComboColoursFallback = allowFallback); AddAssert($"Check retrieved combo colours from {(allowFallback ? "source1" : "fallback source")}", () => diff --git a/osu.Game/Skinning/SkinConfiguration.cs b/osu.Game/Skinning/SkinConfiguration.cs index 0a21594f2a..3bfc941fd7 100644 --- a/osu.Game/Skinning/SkinConfiguration.cs +++ b/osu.Game/Skinning/SkinConfiguration.cs @@ -3,7 +3,6 @@ using System.Collections.Generic; using System.Linq; -using osu.Framework.Extensions.IEnumerableExtensions; using osu.Game.Beatmaps.Formats; using osuTK.Graphics; From 81c849303853cdcc328793b39a440cc76844f477 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 12 Dec 2019 12:16:19 +0300 Subject: [PATCH 17/29] Split combo colour decoding to small test cases --- osu.Game.Tests/Skins/LegacySkinDecoderTest.cs | 69 ++++++++++--------- 1 file changed, 38 insertions(+), 31 deletions(-) diff --git a/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs b/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs index 003b3f5e56..cb2af359b9 100644 --- a/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs +++ b/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs @@ -13,43 +13,21 @@ namespace osu.Game.Tests.Skins [TestFixture] public class LegacySkinDecoderTest { - [TestCase(true)] - [TestCase(false)] - [TestCase(false, false)] - public void TestDecodeSkinColours(bool hasColours, bool canFallback = true) + public void TestDecodeSkinColours() { var decoder = new LegacySkinDecoder(); - using (var resStream = TestResources.OpenResource(hasColours ? "skin.ini" : "skin-empty.ini")) + using (var resStream = TestResources.OpenResource("skin.ini")) using (var stream = new LineBufferedReader(resStream)) { - var skinConfiguration = decoder.Decode(stream); - skinConfiguration.AllowDefaultComboColoursFallback = canFallback; - - var comboColors = skinConfiguration.ComboColours; - - if (!canFallback && !hasColours) + var comboColors = decoder.Decode(stream).ComboColours; + var expectedColors = new List { - Assert.IsNull(comboColors); - return; - } - - List expectedColors; - - if (hasColours) - { - expectedColors = new List - { - new Color4(142, 199, 255, 255), - new Color4(255, 128, 128, 255), - new Color4(128, 255, 255, 255), - new Color4(100, 100, 100, 100), - }; - } - else - { - expectedColors = SkinConfiguration.DefaultComboColours; - } + new Color4(142, 199, 255, 255), + new Color4(255, 128, 128, 255), + new Color4(128, 255, 255, 255), + new Color4(100, 100, 100, 100), + }; Assert.AreEqual(expectedColors.Count, comboColors.Count); for (int i = 0; i < expectedColors.Count; i++) @@ -57,6 +35,35 @@ namespace osu.Game.Tests.Skins } } + public void TestDecodeEmptySkinColours() + { + var decoder = new LegacySkinDecoder(); + + using (var resStream = TestResources.OpenResource("skin-empty.ini")) + using (var stream = new LineBufferedReader(resStream)) + { + var comboColors = decoder.Decode(stream).ComboColours; + var expectedColors = SkinConfiguration.DefaultComboColours; + + Assert.AreEqual(expectedColors.Count, comboColors.Count); + for (int i = 0; i < expectedColors.Count; i++) + Assert.AreEqual(expectedColors[i], comboColors[i]); + } + } + + public void TestDecodeEmptySkinColoursNoFallback() + { + var decoder = new LegacySkinDecoder(); + + using (var resStream = TestResources.OpenResource("skin-empty.ini")) + using (var stream = new LineBufferedReader(resStream)) + { + var skinConfiguration = decoder.Decode(stream); + skinConfiguration.AllowDefaultComboColoursFallback = false; + Assert.IsNull(skinConfiguration.ComboColours); + } + } + [Test] public void TestDecodeGeneral() { From ebd778da2de92377a1e1c874f294d73d0b90494a Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 12 Dec 2019 12:48:07 +0300 Subject: [PATCH 18/29] More test splitting --- .../Skins/TestSceneSkinConfigurationLookup.cs | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs b/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs index fe96cbd633..8808cf2185 100644 --- a/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs +++ b/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs @@ -117,9 +117,8 @@ namespace osu.Game.Tests.Skins }); } - [TestCase(false)] - [TestCase(true)] - public void TestEmptyComboColours(bool allowFallback) + [Test] + public void TestEmptyComboColours() { AddStep("Add custom combo colours to source1", () => source1.Configuration.ComboColours = new List { @@ -127,11 +126,25 @@ namespace osu.Game.Tests.Skins new Color4(55, 110, 166, 255), new Color4(75, 125, 175, 255), }); - AddStep("Disallow default colours fallback in source2", () => source2.Configuration.AllowDefaultComboColoursFallback = allowFallback); - AddAssert($"Check retrieved combo colours from {(allowFallback ? "source1" : "fallback source")}", () => + AddAssert("Check retrieved combo colours is skin default colours", () => + requester.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value?.SequenceEqual(SkinConfiguration.DefaultComboColours) ?? false); + } + + [Test] + public void TestEmptyComboColoursNoFallback() + { + AddStep("Add custom combo colours to source1", () => source1.Configuration.ComboColours = new List { - var expectedColours = allowFallback ? SkinConfiguration.DefaultComboColours : source1.Configuration.ComboColours; + new Color4(100, 150, 200, 255), + new Color4(55, 110, 166, 255), + new Color4(75, 125, 175, 255), + }); + AddStep("Disallow default colours fallback in source2", () => source2.Configuration.AllowDefaultComboColoursFallback = false); + + AddAssert("Check retrieved combo colours from source1", () => + { + var expectedColours = source1.Configuration.ComboColours; return requester.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value?.SequenceEqual(expectedColours) ?? false; }); } From bf8caee53fdbf9fc392462fea7dfdb549af0e7cb Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 12 Dec 2019 14:05:24 +0300 Subject: [PATCH 19/29] Inherit SkinConfiguration directly --- osu.Game/Skinning/LegacySkinConfiguration.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Skinning/LegacySkinConfiguration.cs b/osu.Game/Skinning/LegacySkinConfiguration.cs index b1679bd464..027f5b8883 100644 --- a/osu.Game/Skinning/LegacySkinConfiguration.cs +++ b/osu.Game/Skinning/LegacySkinConfiguration.cs @@ -3,7 +3,7 @@ namespace osu.Game.Skinning { - public class LegacySkinConfiguration : DefaultSkinConfiguration + public class LegacySkinConfiguration : SkinConfiguration { public const decimal LATEST_VERSION = 2.7m; From 4406172eb4151a367cabc659456c44f5822631b8 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 12 Dec 2019 14:05:37 +0300 Subject: [PATCH 20/29] Add missing test attributes --- osu.Game.Tests/Skins/LegacySkinDecoderTest.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs b/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs index 8dbd894a0e..cef38bbbb8 100644 --- a/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs +++ b/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs @@ -13,6 +13,7 @@ namespace osu.Game.Tests.Skins [TestFixture] public class LegacySkinDecoderTest { + [Test] public void TestDecodeSkinColours() { var decoder = new LegacySkinDecoder(); @@ -35,6 +36,7 @@ namespace osu.Game.Tests.Skins } } + [Test] public void TestDecodeEmptySkinColours() { var decoder = new LegacySkinDecoder(); @@ -51,6 +53,7 @@ namespace osu.Game.Tests.Skins } } + [Test] public void TestDecodeEmptySkinColoursNoFallback() { var decoder = new LegacySkinDecoder(); From 7e1d21fa1658efcdf344f0ac632b7db83010847e Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 12 Dec 2019 14:08:35 +0300 Subject: [PATCH 21/29] Simplify combo colours lookup tests --- .../Skins/TestSceneSkinConfigurationLookup.cs | 20 +++++-------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs b/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs index 8808cf2185..ed54cc982d 100644 --- a/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs +++ b/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs @@ -120,13 +120,6 @@ namespace osu.Game.Tests.Skins [Test] public void TestEmptyComboColours() { - AddStep("Add custom combo colours to source1", () => source1.Configuration.ComboColours = new List - { - new Color4(100, 150, 200, 255), - new Color4(55, 110, 166, 255), - new Color4(75, 125, 175, 255), - }); - AddAssert("Check retrieved combo colours is skin default colours", () => requester.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value?.SequenceEqual(SkinConfiguration.DefaultComboColours) ?? false); } @@ -134,19 +127,16 @@ namespace osu.Game.Tests.Skins [Test] public void TestEmptyComboColoursNoFallback() { - AddStep("Add custom combo colours to source1", () => source1.Configuration.ComboColours = new List - { + AddStep("Add custom combo colours to source1", () => source1.Configuration.AddComboColours( new Color4(100, 150, 200, 255), new Color4(55, 110, 166, 255), - new Color4(75, 125, 175, 255), - }); + new Color4(75, 125, 175, 255) + )); + AddStep("Disallow default colours fallback in source2", () => source2.Configuration.AllowDefaultComboColoursFallback = false); AddAssert("Check retrieved combo colours from source1", () => - { - var expectedColours = source1.Configuration.ComboColours; - return requester.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value?.SequenceEqual(expectedColours) ?? false; - }); + requester.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value?.SequenceEqual(source1.Configuration.ComboColours) ?? false); } [Test] From 8d0dd1497b291898dc216754a7561e1c61e3b73c Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 21 Dec 2019 12:26:58 +0300 Subject: [PATCH 22/29] Add legacy beatmap skin test scene with combo colours tests --- .../TestSceneLegacyBeatmapSkin.cs | 173 ++++++++++++++++++ 1 file changed, 173 insertions(+) create mode 100644 osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs new file mode 100644 index 0000000000..8ad66d833f --- /dev/null +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs @@ -0,0 +1,173 @@ +// 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 System.Linq; +using NUnit.Framework; +using osu.Framework.Allocation; +using osu.Framework.Audio; +using osu.Framework.Audio.Sample; +using osu.Framework.Bindables; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Textures; +using osu.Framework.IO.Stores; +using osu.Game.Audio; +using osu.Game.Beatmaps; +using osu.Game.Graphics.Containers; +using osu.Game.Rulesets.Osu.Objects; +using osu.Game.Screens; +using osu.Game.Screens.Play; +using osu.Game.Skinning; +using osu.Game.Tests.Visual; +using osuTK; +using osuTK.Graphics; + +namespace osu.Game.Rulesets.Osu.Tests +{ + public class TestSceneLegacyBeatmapSkin : OsuTestScene + { + [Resolved] + private AudioManager audio { get; set; } + + [Test] + public void TestBeatmapComboColours() + { + ExposedPlayer player = null; + ISkin gameplaySkin = null; + IReadOnlyList colours = null; + + AddStep("load beatmap", () => player = loadBeatmap(false, true)); + AddUntilStep("wait for player", () => player.IsLoaded); + AddStep("attach skin requester", () => gameplaySkin = addSkinRequester(player)); + + AddStep("retrieve combo colours", () => colours = gameplaySkin.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value); + AddAssert("is beatmap colours", () => colours.SequenceEqual(TestBeatmapSkin.Colours)); + } + + [Test] + public void TestEmptyBeatmapComboColours() + { + ExposedPlayer player = null; + ISkin gameplaySkin = null; + IReadOnlyList colours = null; + + AddStep("load no-colour beatmap", () => player = loadBeatmap(false, false)); + AddUntilStep("wait for player", () => player.IsLoaded); + AddStep("attach skin requester", () => gameplaySkin = addSkinRequester(player)); + + AddStep("retrieve combo colours", () => colours = gameplaySkin.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value); + AddAssert("is default skin colours", () => colours.SequenceEqual(SkinConfiguration.DefaultComboColours)); + } + + [Test] + public void TestEmptyBeatmapCustomSkinColours() + { + ExposedPlayer player = null; + ISkin gameplaySkin = null; + IReadOnlyList colours = null; + + AddStep("load no-colour beatmap", () => player = loadBeatmap(true, false)); + AddUntilStep("wait for player", () => player.IsLoaded); + AddStep("attach skin requester", () => gameplaySkin = addSkinRequester(player)); + + AddStep("retrieve combo colours", () => colours = gameplaySkin.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value); + AddAssert("is custom skin colours", () => colours.SequenceEqual(TestSkin.Colours)); + } + + private ExposedPlayer loadBeatmap(bool skinHasCustomColours, bool beatmapHasCustomColours) + { + ExposedPlayer player; + + Beatmap.Value = new CustomSkinWorkingBeatmap(audio, beatmapHasCustomColours); + Child = new SkinProvidingContainer(new TestSkin(skinHasCustomColours)) + .WithChild(new OsuScreenStack(player = new ExposedPlayer()) { RelativeSizeAxes = Axes.Both }); + + return player; + } + + private ISkin addSkinRequester(ExposedPlayer player) + { + SkinRequester skin; + player.BeatmapSkinContainer.Add(skin = new SkinRequester()); + return skin; + } + + private class SkinRequester : Component, ISkin + { + [Resolved] + private ISkinSource skin { get; set; } + + public Drawable GetDrawableComponent(ISkinComponent component) => skin.GetDrawableComponent(component); + + public Texture GetTexture(string componentName) => skin.GetTexture(componentName); + + public SampleChannel GetSample(ISampleInfo sampleInfo) => skin.GetSample(sampleInfo); + + public IBindable GetConfig(TLookup lookup) => skin.GetConfig(lookup); + } + + private class ExposedPlayer : Player + { + public ExposedPlayer() + : base(false, false) + { + } + + public BeatmapSkinProvidingContainer BeatmapSkinContainer => GameplayClockContainer.OfType().First().OfType().First(); + } + + private class CustomSkinWorkingBeatmap : ClockBackedTestWorkingBeatmap + { + private readonly bool hasColours; + + public CustomSkinWorkingBeatmap(AudioManager audio, bool hasColours) + : base(new Beatmap + { + BeatmapInfo = + { + BeatmapSet = new BeatmapSetInfo(), + Ruleset = new OsuRuleset().RulesetInfo, + }, + HitObjects = { new HitCircle { Position = new Vector2(256, 192) } } + }, null, null, audio) + { + this.hasColours = hasColours; + } + + protected override ISkin GetSkin() => new TestBeatmapSkin(BeatmapInfo, hasColours); + } + + private class TestBeatmapSkin : LegacyBeatmapSkin + { + public static Color4[] Colours { get; } = + { + new Color4(50, 100, 150, 255), + new Color4(40, 80, 120, 255), + }; + + public TestBeatmapSkin(BeatmapInfo beatmap, bool hasColours) + : base(beatmap, new ResourceStore(), null) + { + if (hasColours) + Configuration.AddComboColours(Colours); + } + } + + private class TestSkin : LegacySkin + { + public static Color4[] Colours { get; } = + { + new Color4(10, 100, 150, 255), + new Color4(20, 20, 20, 255), + }; + + public TestSkin(bool hasCustomColours) + : base(new SkinInfo(), null, null, string.Empty) + { + if (hasCustomColours) + Configuration.AddComboColours(Colours); + } + } + } +} From ff5e6c0dcfeb8d9eb62ad5527486faa7af24af75 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 21 Dec 2019 12:36:34 +0300 Subject: [PATCH 23/29] Make DefaultComboColours a property --- osu.Game/Skinning/SkinConfiguration.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Skinning/SkinConfiguration.cs b/osu.Game/Skinning/SkinConfiguration.cs index 3bfc941fd7..e3a0ab23fa 100644 --- a/osu.Game/Skinning/SkinConfiguration.cs +++ b/osu.Game/Skinning/SkinConfiguration.cs @@ -20,7 +20,7 @@ namespace osu.Game.Skinning /// internal bool AllowDefaultComboColoursFallback = true; - public static List DefaultComboColours = new List + public static List DefaultComboColours { get; } = new List { new Color4(255, 192, 0, 255), new Color4(0, 202, 0, 255), From a6632cf1ef9708faeacbb35c61e812fc2a8bcc83 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 21 Dec 2019 12:53:05 +0300 Subject: [PATCH 24/29] Remove unnecessary skin requester and user-skin providing container --- .../TestSceneLegacyBeatmapSkin.cs | 74 ++++++++----------- 1 file changed, 29 insertions(+), 45 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs index 8ad66d833f..c26afefc41 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs @@ -1,18 +1,14 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System; using System.Collections.Generic; using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Audio; -using osu.Framework.Audio.Sample; -using osu.Framework.Bindables; using osu.Framework.Graphics; -using osu.Framework.Graphics.Containers; -using osu.Framework.Graphics.Textures; using osu.Framework.IO.Stores; -using osu.Game.Audio; using osu.Game.Beatmaps; using osu.Game.Graphics.Containers; using osu.Game.Rulesets.Osu.Objects; @@ -34,87 +30,69 @@ namespace osu.Game.Rulesets.Osu.Tests public void TestBeatmapComboColours() { ExposedPlayer player = null; - ISkin gameplaySkin = null; IReadOnlyList colours = null; AddStep("load beatmap", () => player = loadBeatmap(false, true)); AddUntilStep("wait for player", () => player.IsLoaded); - AddStep("attach skin requester", () => gameplaySkin = addSkinRequester(player)); - AddStep("retrieve combo colours", () => colours = gameplaySkin.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value); - AddAssert("is beatmap colours", () => colours.SequenceEqual(TestBeatmapSkin.Colours)); + AddStep("retrieve combo colours", () => colours = player.BeatmapSkin.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value); + AddAssert("is beatmap skin colours", () => colours.SequenceEqual(TestBeatmapSkin.Colours)); } [Test] public void TestEmptyBeatmapComboColours() { ExposedPlayer player = null; - ISkin gameplaySkin = null; IReadOnlyList colours = null; AddStep("load no-colour beatmap", () => player = loadBeatmap(false, false)); AddUntilStep("wait for player", () => player.IsLoaded); - AddStep("attach skin requester", () => gameplaySkin = addSkinRequester(player)); - AddStep("retrieve combo colours", () => colours = gameplaySkin.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value); - AddAssert("is default skin colours", () => colours.SequenceEqual(SkinConfiguration.DefaultComboColours)); + AddStep("retrieve combo colours", () => colours = player.BeatmapSkin.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value); + AddAssert("is default user skin colours", () => colours.SequenceEqual(SkinConfiguration.DefaultComboColours)); } [Test] public void TestEmptyBeatmapCustomSkinColours() { ExposedPlayer player = null; - ISkin gameplaySkin = null; IReadOnlyList colours = null; AddStep("load no-colour beatmap", () => player = loadBeatmap(true, false)); AddUntilStep("wait for player", () => player.IsLoaded); - AddStep("attach skin requester", () => gameplaySkin = addSkinRequester(player)); - AddStep("retrieve combo colours", () => colours = gameplaySkin.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value); - AddAssert("is custom skin colours", () => colours.SequenceEqual(TestSkin.Colours)); + AddStep("retrieve combo colours", () => colours = player.BeatmapSkin.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value); + AddAssert("is custom user skin colours", () => colours.SequenceEqual(TestSkin.Colours)); } - private ExposedPlayer loadBeatmap(bool skinHasCustomColours, bool beatmapHasCustomColours) + private ExposedPlayer loadBeatmap(bool userHasCustomColours, bool beatmapHasColours) { ExposedPlayer player; - Beatmap.Value = new CustomSkinWorkingBeatmap(audio, beatmapHasCustomColours); - Child = new SkinProvidingContainer(new TestSkin(skinHasCustomColours)) - .WithChild(new OsuScreenStack(player = new ExposedPlayer()) { RelativeSizeAxes = Axes.Both }); + Beatmap.Value = new CustomSkinWorkingBeatmap(audio, beatmapHasColours); + Child = new OsuScreenStack(player = new ExposedPlayer(userHasCustomColours)) { RelativeSizeAxes = Axes.Both }; return player; } - private ISkin addSkinRequester(ExposedPlayer player) - { - SkinRequester skin; - player.BeatmapSkinContainer.Add(skin = new SkinRequester()); - return skin; - } - - private class SkinRequester : Component, ISkin - { - [Resolved] - private ISkinSource skin { get; set; } - - public Drawable GetDrawableComponent(ISkinComponent component) => skin.GetDrawableComponent(component); - - public Texture GetTexture(string componentName) => skin.GetTexture(componentName); - - public SampleChannel GetSample(ISampleInfo sampleInfo) => skin.GetSample(sampleInfo); - - public IBindable GetConfig(TLookup lookup) => skin.GetConfig(lookup); - } - private class ExposedPlayer : Player { - public ExposedPlayer() + private readonly bool userHasCustomColours; + + public ExposedPlayer(bool userHasCustomColours) : base(false, false) { + this.userHasCustomColours = userHasCustomColours; } - public BeatmapSkinProvidingContainer BeatmapSkinContainer => GameplayClockContainer.OfType().First().OfType().First(); + protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) + { + var dependencies = new DependencyContainer(base.CreateChildDependencies(parent)); + dependencies.CacheAs(new TestSkin(userHasCustomColours)); + return dependencies; + } + + public BeatmapSkinProvidingContainer BeatmapSkin => GameplayClockContainer.OfType().First().OfType().First(); } private class CustomSkinWorkingBeatmap : ClockBackedTestWorkingBeatmap @@ -154,7 +132,7 @@ namespace osu.Game.Rulesets.Osu.Tests } } - private class TestSkin : LegacySkin + private class TestSkin : LegacySkin, ISkinSource { public static Color4[] Colours { get; } = { @@ -168,6 +146,12 @@ namespace osu.Game.Rulesets.Osu.Tests if (hasCustomColours) Configuration.AddComboColours(Colours); } + + public event Action SourceChanged + { + add { } + remove { } + } } } } From 145ac8e0b547c3f8472f4a0344dec7b6e3a4730a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 25 Dec 2019 14:55:05 +0900 Subject: [PATCH 25/29] Remove redundant setter --- osu.Game/Beatmaps/Formats/IHasComboColours.cs | 2 +- osu.Game/Skinning/SkinConfiguration.cs | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/osu.Game/Beatmaps/Formats/IHasComboColours.cs b/osu.Game/Beatmaps/Formats/IHasComboColours.cs index ac0d32dbaa..41c85db063 100644 --- a/osu.Game/Beatmaps/Formats/IHasComboColours.cs +++ b/osu.Game/Beatmaps/Formats/IHasComboColours.cs @@ -11,7 +11,7 @@ namespace osu.Game.Beatmaps.Formats /// /// Retrieves the list of combo colours for presentation only. /// - IReadOnlyList ComboColours { get; set; } + IReadOnlyList ComboColours { get; } /// /// Adds combo colours to the list. diff --git a/osu.Game/Skinning/SkinConfiguration.cs b/osu.Game/Skinning/SkinConfiguration.cs index e3a0ab23fa..a55870aa6d 100644 --- a/osu.Game/Skinning/SkinConfiguration.cs +++ b/osu.Game/Skinning/SkinConfiguration.cs @@ -2,7 +2,6 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; -using System.Linq; using osu.Game.Beatmaps.Formats; using osuTK.Graphics; @@ -28,7 +27,7 @@ namespace osu.Game.Skinning new Color4(242, 24, 57, 255), }; - private List comboColours = new List(); + private readonly List comboColours = new List(); public IReadOnlyList ComboColours { @@ -42,7 +41,6 @@ namespace osu.Game.Skinning return null; } - set => comboColours = value.ToList(); } public void AddComboColours(params Color4[] colours) => comboColours.AddRange(colours); From 04cbdd9c6c253f04a63b13fd9052cbe1cf181477 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 25 Dec 2019 14:55:13 +0900 Subject: [PATCH 26/29] Make test easier to visually examine --- osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs index c26afefc41..9adcf26529 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs @@ -136,7 +136,7 @@ namespace osu.Game.Rulesets.Osu.Tests { public static Color4[] Colours { get; } = { - new Color4(10, 100, 150, 255), + new Color4(150, 100, 50, 255), new Color4(20, 20, 20, 255), }; From 783159f7a06de9438552f0de2b9654662b78b6ac Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 25 Dec 2019 14:55:22 +0900 Subject: [PATCH 27/29] Remove redundant logic --- osu.Game/Skinning/DefaultSkinConfiguration.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/osu.Game/Skinning/DefaultSkinConfiguration.cs b/osu.Game/Skinning/DefaultSkinConfiguration.cs index 8e89ab25b2..5842ee82ee 100644 --- a/osu.Game/Skinning/DefaultSkinConfiguration.cs +++ b/osu.Game/Skinning/DefaultSkinConfiguration.cs @@ -8,9 +8,5 @@ namespace osu.Game.Skinning /// public class DefaultSkinConfiguration : SkinConfiguration { - public DefaultSkinConfiguration() - { - ComboColours = DefaultComboColours; - } } } From 9090d13e05ffe278e31c951f7f8ebf9d990da11c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 25 Dec 2019 15:01:05 +0900 Subject: [PATCH 28/29] Simplify tests --- .../TestSceneLegacyBeatmapSkin.cs | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs index 9adcf26529..e26b671d57 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs @@ -9,8 +9,8 @@ using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Graphics; using osu.Framework.IO.Stores; +using osu.Framework.Testing; using osu.Game.Beatmaps; -using osu.Game.Graphics.Containers; using osu.Game.Rulesets.Osu.Objects; using osu.Game.Screens; using osu.Game.Screens.Play; @@ -30,39 +30,33 @@ namespace osu.Game.Rulesets.Osu.Tests public void TestBeatmapComboColours() { ExposedPlayer player = null; - IReadOnlyList colours = null; - AddStep("load beatmap", () => player = loadBeatmap(false, true)); + AddStep("load coloured beatmap", () => player = loadBeatmap(false, true)); AddUntilStep("wait for player", () => player.IsLoaded); - AddStep("retrieve combo colours", () => colours = player.BeatmapSkin.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value); - AddAssert("is beatmap skin colours", () => colours.SequenceEqual(TestBeatmapSkin.Colours)); + AddAssert("is beatmap skin colours", () => player.UsableComboColours.SequenceEqual(TestBeatmapSkin.Colours)); } [Test] public void TestEmptyBeatmapComboColours() { ExposedPlayer player = null; - IReadOnlyList colours = null; AddStep("load no-colour beatmap", () => player = loadBeatmap(false, false)); AddUntilStep("wait for player", () => player.IsLoaded); - AddStep("retrieve combo colours", () => colours = player.BeatmapSkin.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value); - AddAssert("is default user skin colours", () => colours.SequenceEqual(SkinConfiguration.DefaultComboColours)); + AddAssert("is default user skin colours", () => player.UsableComboColours.SequenceEqual(SkinConfiguration.DefaultComboColours)); } [Test] public void TestEmptyBeatmapCustomSkinColours() { ExposedPlayer player = null; - IReadOnlyList colours = null; - AddStep("load no-colour beatmap", () => player = loadBeatmap(true, false)); + AddStep("load custom-skin colour", () => player = loadBeatmap(true, false)); AddUntilStep("wait for player", () => player.IsLoaded); - AddStep("retrieve combo colours", () => colours = player.BeatmapSkin.GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value); - AddAssert("is custom user skin colours", () => colours.SequenceEqual(TestSkin.Colours)); + AddAssert("is custom user skin colours", () => player.UsableComboColours.SequenceEqual(TestSkin.Colours)); } private ExposedPlayer loadBeatmap(bool userHasCustomColours, bool beatmapHasColours) @@ -92,7 +86,10 @@ namespace osu.Game.Rulesets.Osu.Tests return dependencies; } - public BeatmapSkinProvidingContainer BeatmapSkin => GameplayClockContainer.OfType().First().OfType().First(); + public IReadOnlyList UsableComboColours => + GameplayClockContainer.ChildrenOfType() + .First() + .GetConfig>(GlobalSkinConfiguration.ComboColours)?.Value; } private class CustomSkinWorkingBeatmap : ClockBackedTestWorkingBeatmap From 3485ef33a726b1caee8a0245188691d79da38674 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 25 Dec 2019 15:05:08 +0900 Subject: [PATCH 29/29] Rename tests and add missing coverage --- .../TestSceneLegacyBeatmapSkin.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs index e26b671d57..4676f14655 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs @@ -26,19 +26,20 @@ namespace osu.Game.Rulesets.Osu.Tests [Resolved] private AudioManager audio { get; set; } - [Test] - public void TestBeatmapComboColours() + [TestCase(true)] + [TestCase(false)] + public void TestBeatmapComboColours(bool customSkinColoursPresent) { ExposedPlayer player = null; - AddStep("load coloured beatmap", () => player = loadBeatmap(false, true)); + AddStep("load coloured beatmap", () => player = loadBeatmap(customSkinColoursPresent, true)); AddUntilStep("wait for player", () => player.IsLoaded); AddAssert("is beatmap skin colours", () => player.UsableComboColours.SequenceEqual(TestBeatmapSkin.Colours)); } [Test] - public void TestEmptyBeatmapComboColours() + public void TestBeatmapNoComboColours() { ExposedPlayer player = null; @@ -49,7 +50,7 @@ namespace osu.Game.Rulesets.Osu.Tests } [Test] - public void TestEmptyBeatmapCustomSkinColours() + public void TestBeatmapNoComboColoursSkinOverride() { ExposedPlayer player = null;