Merge pull request ppy#5969 from peppy/skin-configuration-refactor

Refactor skin configuration lookups to be more flexible

Co-authored-by: Dan Balasescu <smoogipoo@smgi.me>

osu.Game.Rulesets.Catch.Tests/TestSceneCatcher.cs

@@ -14,6 +14,7 @@
 using osu.Framework.Graphics.Sprites;
 using osuTK.Graphics;
 using osu.Framework.Audio.Sample;
+using osu.Framework.Bindables;
 using osu.Framework.Graphics.Textures;
 using osu.Game.Audio;
 
@@ -99,8 +100,7 @@ public Drawable GetDrawableComponent(ISkinComponent component)
             public Texture GetTexture(string componentName) =>
                 throw new NotImplementedException();
 
-            public TValue GetValue<TConfiguration, TValue>(Func<TConfiguration, TValue> query) where TConfiguration : SkinConfiguration =>
-                throw new NotImplementedException();
+            public IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup) => throw new NotImplementedException();
         }
     }
 }

osu.Game.Rulesets.Osu.Tests/TestSceneSkinFallbacks.cs

@@ -7,6 +7,7 @@
 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.Sprites;
 using osu.Framework.Graphics.Textures;
@@ -135,6 +136,7 @@ public Drawable GetDrawableComponent(ISkinComponent component)
             public SampleChannel GetSample(ISampleInfo sampleInfo) => null;
 
             public TValue GetValue<TConfiguration, TValue>(Func<TConfiguration, TValue> query) where TConfiguration : SkinConfiguration => default;
+            public IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup) => null;
 
             public event Action SourceChanged;
 

osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs

@@ -12,6 +12,7 @@
 using osu.Framework.Graphics.Containers;
 using osu.Game.Rulesets.Objects;
 using osu.Game.Rulesets.Osu.Configuration;
+using osu.Game.Rulesets.Osu.Skinning;
 using osu.Game.Rulesets.Scoring;
 using osuTK.Graphics;
 using osu.Game.Skinning;
@@ -166,12 +167,12 @@ protected override void SkinChanged(ISkinSource skin, bool allowFallback)
         {
             base.SkinChanged(skin, allowFallback);
 
-            Body.BorderSize = skin.GetValue<SkinConfiguration, float?>(s => s.SliderBorderSize) ?? SliderBody.DEFAULT_BORDER_SIZE;
-            sliderPathRadius = skin.GetValue<SkinConfiguration, float?>(s => s.SliderPathRadius) ?? OsuHitObject.OBJECT_RADIUS;
+            Body.BorderSize = skin.GetConfig<OsuSkinConfiguration, float>(OsuSkinConfiguration.SliderBorderSize)?.Value ?? SliderBody.DEFAULT_BORDER_SIZE;
+            sliderPathRadius = skin.GetConfig<OsuSkinConfiguration, float>(OsuSkinConfiguration.SliderPathRadius)?.Value ?? OsuHitObject.OBJECT_RADIUS;
             updatePathRadius();
 
-            Body.AccentColour = skin.GetValue<SkinConfiguration, Color4?>(s => s.CustomColours.ContainsKey("SliderTrackOverride") ? s.CustomColours["SliderTrackOverride"] : (Color4?)null) ?? AccentColour.Value;
-            Body.BorderColour = skin.GetValue<SkinConfiguration, Color4?>(s => s.CustomColours.ContainsKey("SliderBorder") ? s.CustomColours["SliderBorder"] : (Color4?)null) ?? Color4.White;
+            Body.AccentColour = skin.GetConfig<OsuSkinColour, Color4>(OsuSkinColour.SliderTrackOverride)?.Value ?? AccentColour.Value;
+            Body.BorderColour = skin.GetConfig<OsuSkinColour, Color4>(OsuSkinColour.SliderBorder)?.Value ?? Color4.White;
         }
 
         private void updatePathRadius() => Body.PathRadius = slider.Scale * sliderPathRadius;

osu.Game.Rulesets.Osu/Objects/Drawables/Pieces/SliderBall.cs

@@ -11,6 +11,7 @@
 using osu.Framework.Input.Events;
 using osu.Game.Rulesets.Objects.Drawables;
 using osu.Game.Rulesets.Objects.Types;
+using osu.Game.Rulesets.Osu.Skinning;
 using osuTK.Graphics;
 using osu.Game.Skinning;
 using osuTK;
@@ -218,7 +219,7 @@ private void load(DrawableHitObject drawableObject, ISkinSource skin)
             {
                 RelativeSizeAxes = Axes.Both;
 
-                float radius = skin.GetValue<SkinConfiguration, float?>(s => s.SliderPathRadius) ?? OsuHitObject.OBJECT_RADIUS;
+                float radius = skin.GetConfig<OsuSkinConfiguration, float>(OsuSkinConfiguration.SliderPathRadius)?.Value ?? OsuHitObject.OBJECT_RADIUS;
 
                 InternalChild = new CircularContainer
                 {

osu.Game.Rulesets.Osu/OsuRuleset.cs

@@ -167,7 +167,7 @@ public override IEnumerable<Mod> GetModsFor(ModType type)
 
         public override RulesetSettingsSubsection CreateSettings() => new OsuSettingsSubsection(this);
 
-        public override ISkin CreateLegacySkinProvider(ISkinSource source) => new OsuLegacySkin(source);
+        public override ISkin CreateLegacySkinProvider(ISkinSource source) => new OsuLegacySkinTransformer(source);
 
         public override int? LegacyID => 0;
 

osu.Game.Rulesets.Osu/Skinning/LegacySliderBall.cs

@@ -23,7 +23,7 @@ public LegacySliderBall(Drawable animationContent)
         [BackgroundDependencyLoader]
         private void load(ISkinSource skin, DrawableHitObject drawableObject)
         {
-            animationContent.Colour = skin.GetValue<SkinConfiguration, Color4?>(s => s.CustomColours.ContainsKey("SliderBall") ? s.CustomColours["SliderBall"] : (Color4?)null) ?? Color4.White;
+            animationContent.Colour = skin.GetConfig<OsuSkinColour, Color4>(OsuSkinColour.SliderBall)?.Value ?? Color4.White;
 
             InternalChildren = new[]
             {

osu.Game.Rulesets.Osu/Skinning/OsuLegacySkin.cs → osu.Game.Rulesets.Osu/Skinning/OsuLegacySkinTransformer.cs

@@ -3,21 +3,19 @@
 
 using System;
 using osu.Framework.Audio.Sample;
+using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Textures;
 using osu.Game.Audio;
 using osu.Game.Skinning;
 using osuTK;
-using osuTK.Graphics;
 
 namespace osu.Game.Rulesets.Osu.Skinning
 {
-    public class OsuLegacySkin : ISkin
+    public class OsuLegacySkinTransformer : ISkin
     {
         private readonly ISkin source;
 
-        private Lazy<SkinConfiguration> configuration;
-
         private Lazy<bool> hasHitCircle;
 
         /// <summary>
@@ -27,7 +25,7 @@ public class OsuLegacySkin : ISkin
         /// </summary>
         private const float legacy_circle_radius = 64 - 5;
 
-        public OsuLegacySkin(ISkinSource source)
+        public OsuLegacySkinTransformer(ISkinSource source)
         {
             this.source = source;
 
@@ -37,21 +35,6 @@ public OsuLegacySkin(ISkinSource source)
 
         private void sourceChanged()
         {
-            // these need to be lazy in order to ensure they aren't called before the dependencies have been loaded into our source.
-            configuration = new Lazy<SkinConfiguration>(() =>
-            {
-                var config = new SkinConfiguration();
-                if (hasHitCircle.Value)
-                    config.SliderPathRadius = legacy_circle_radius;
-
-                // defaults should only be applied for non-beatmap skins (which are parsed via this constructor).
-                config.CustomColours["SliderBall"] =
-                    source.GetValue<SkinConfiguration, Color4?>(s => s.CustomColours.TryGetValue("SliderBall", out var val) ? val : (Color4?)null)
-                    ?? new Color4(2, 170, 255, 255);
-
-                return config;
-            });
-
             hasHitCircle = new Lazy<bool>(() => source.GetTexture("hitcircle") != null);
         }
 
@@ -96,8 +79,8 @@ public Drawable GetDrawableComponent(ISkinComponent component)
                     return null;
 
                 case OsuSkinComponents.HitCircleText:
-                    string font = GetValue<SkinConfiguration, string>(config => config.HitCircleFont);
-                    var overlap = GetValue<SkinConfiguration, float>(config => config.HitCircleOverlap);
+                    var font = GetConfig<OsuSkinConfiguration, string>(OsuSkinConfiguration.HitCircleFont)?.Value ?? "default";
+                    var overlap = GetConfig<OsuSkinConfiguration, float>(OsuSkinConfiguration.HitCircleOverlap)?.Value ?? 0;
 
                     return !hasFont(font)
                         ? null
@@ -116,13 +99,27 @@ public Drawable GetDrawableComponent(ISkinComponent component)
 
         public SampleChannel GetSample(ISampleInfo sample) => source.GetSample(sample);
 
-        public TValue GetValue<TConfiguration, TValue>(Func<TConfiguration, TValue> query) where TConfiguration : SkinConfiguration
+        public IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup)
         {
-            TValue val;
-            if (configuration.Value is TConfiguration conf && (val = query.Invoke(conf)) != null)
-                return val;
+            switch (lookup)
+            {
+                case OsuSkinColour colour:
+                    return source.GetConfig<SkinCustomColourLookup, TValue>(new SkinCustomColourLookup(colour));
+
+                case OsuSkinConfiguration osuLookup:
+                    switch (osuLookup)
+                    {
+                        case OsuSkinConfiguration.SliderPathRadius:
+                            if (hasHitCircle.Value)
+                                return SkinUtils.As<TValue>(new BindableFloat(legacy_circle_radius));
+
+                            break;
+                    }
+
+                    break;
+            }
 
-            return source.GetValue(query);
+            return source.GetConfig<TLookup, TValue>(lookup);
         }
 
         private bool hasFont(string fontName) => source.GetTexture($"{fontName}-0") != null;

osu.Game.Rulesets.Osu/Skinning/OsuSkinColour.cs

@@ -0,0 +1,12 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+namespace osu.Game.Rulesets.Osu.Skinning
+{
+    public enum OsuSkinColour
+    {
+        SliderTrackOverride,
+        SliderBorder,
+        SliderBall
+    }
+}

osu.Game.Rulesets.Osu/Skinning/OsuSkinConfiguration.cs

@@ -0,0 +1,14 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+namespace osu.Game.Rulesets.Osu.Skinning
+{
+    public enum OsuSkinConfiguration
+    {
+        HitCircleFont,
+        HitCircleOverlap,
+        SliderBorderSize,
+        SliderPathRadius,
+        CursorExpand,
+    }
+}

osu.Game.Rulesets.Osu/UI/Cursor/OsuCursor.cs

@@ -10,6 +10,7 @@
 using osu.Framework.Graphics.Shapes;
 using osu.Game.Beatmaps;
 using osu.Game.Configuration;
+using osu.Game.Rulesets.Osu.Skinning;
 using osu.Game.Skinning;
 using osuTK;
 using osuTK.Graphics;
@@ -38,7 +39,7 @@ public OsuCursor()
 
         protected override void SkinChanged(ISkinSource skin, bool allowFallback)
         {
-            cursorExpand = skin.GetValue<SkinConfiguration, bool>(s => s.CursorExpand ?? true);
+            cursorExpand = skin.GetConfig<OsuSkinConfiguration, bool>(OsuSkinConfiguration.CursorExpand)?.Value ?? true;
         }
 
         [BackgroundDependencyLoader]

osu.Game.Tests/Resources/skin.ini

@@ -1,5 +1,6 @@
 [General]
 Name: test skin
+TestLookup: TestValue
 
 [Colours]
 Combo1 : 142,199,255

osu.Game.Tests/Skins/LegacySkinDecoderTest.cs

@@ -41,5 +41,20 @@ public void TestDecodeSkinColours(bool hasColours)
                     Assert.AreEqual(expectedColors[i], comboColors[i]);
             }
         }
+
+        [Test]
+        public void TestDecodeGeneral()
+        {
+            var decoder = new LegacySkinDecoder();
+
+            using (var resStream = TestResources.OpenResource("skin.ini"))
+            using (var stream = new StreamReader(resStream))
+            {
+                var config = decoder.Decode(stream);
+
+                Assert.AreEqual("test skin", config.SkinInfo.Name);
+                Assert.AreEqual("TestValue", config.ConfigDictionary["TestLookup"]);
+            }
+        }
     }
 }