Refactor CatchLegacySkinTransformer logic and remove HiddenByRulesetImplementation entirely

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

@@ -4,7 +4,6 @@
 using System.Linq;
 using NUnit.Framework;
 using osu.Framework.Extensions.IEnumerableExtensions;
-using osu.Framework.Graphics.Containers;
 using osu.Framework.Screens;
 using osu.Framework.Testing;
 using osu.Game.Skinning;
@@ -19,7 +18,7 @@ namespace osu.Game.Rulesets.Catch.Tests
         protected override Ruleset CreatePlayerRuleset() => new CatchRuleset();
 
         [Test]
-        public void TestLegacyHUDComboCounterHidden([Values] bool withModifiedSkin)
+        public void TestLegacyHUDComboCounterNotExistent([Values] bool withModifiedSkin)
         {
             if (withModifiedSkin)
             {
@@ -29,10 +28,7 @@ namespace osu.Game.Rulesets.Catch.Tests
                 CreateTest();
             }
 
-            AddAssert("legacy HUD combo counter hidden", () =>
+            AddAssert("legacy HUD combo counter not added", () => !Player.ChildrenOfType<LegacyComboCounter>().Any());
-            {
-                return Player.ChildrenOfType<LegacyComboCounter>().All(c => c.ChildrenOfType<Container>().Single().Alpha == 0f);
-            });
         }
     }
 }

osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs

@@ -1,10 +1,9 @@
 // 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.
 
-using System.Linq;
+using System.Diagnostics;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
-using osu.Framework.Graphics.Containers;
 using osu.Game.Skinning;
 using osuTK.Graphics;
 
@@ -28,76 +27,69 @@ namespace osu.Game.Rulesets.Catch.Skinning.Legacy
 
         public override Drawable? GetDrawableComponent(ISkinComponentLookup lookup)
         {
-            if (lookup is SkinComponentsContainerLookup containerLookup)
+            switch (lookup)
             {
-                switch (containerLookup.Target)
+                case SkinComponentsContainerLookup containerLookup:
-                {
+                    switch (containerLookup.Target)
-                    case SkinComponentsContainerLookup.TargetArea.MainHUDComponents:
+                    {
-                        var components = base.GetDrawableComponent(lookup) as Container;
+                        case SkinComponentsContainerLookup.TargetArea.MainHUDComponents when containerLookup.Ruleset != null:
+                            Debug.Assert(containerLookup.Ruleset.ShortName == CatchRuleset.SHORT_NAME);
+                            // todo: remove CatchSkinComponents.CatchComboCounter and refactor LegacyCatchComboCounter to be added here instead.
+                            return Skin.GetDrawableComponent(lookup);
+                    }
 
-                        if (providesComboCounter && components != null)
+                    break;
-                        {
-                            // catch may provide its own combo counter; hide the default.
-                            // todo: this should be done in an elegant way per ruleset, defining which HUD skin components should be displayed.
-                            foreach (var legacyComboCounter in components.OfType<LegacyComboCounter>())
-                                legacyComboCounter.HiddenByRulesetImplementation = false;
-                        }
 
-                        return components;
+                case CatchSkinComponentLookup catchSkinComponent:
-                }
+                    switch (catchSkinComponent.Component)
-            }
+                    {
+                        case CatchSkinComponents.Fruit:
+                            if (hasPear)
+                                return new LegacyFruitPiece();
 
-            if (lookup is CatchSkinComponentLookup catchSkinComponent)
+                            return null;
-            {
-                switch (catchSkinComponent.Component)
-                {
-                    case CatchSkinComponents.Fruit:
-                        if (hasPear)
-                            return new LegacyFruitPiece();
 
-                        return null;
+                        case CatchSkinComponents.Banana:
+                            if (GetTexture("fruit-bananas") != null)
+                                return new LegacyBananaPiece();
 
-                    case CatchSkinComponents.Banana:
+                            return null;
-                        if (GetTexture("fruit-bananas") != null)
-                            return new LegacyBananaPiece();
 
-                        return null;
+                        case CatchSkinComponents.Droplet:
+                            if (GetTexture("fruit-drop") != null)
+                                return new LegacyDropletPiece();
 
-                    case CatchSkinComponents.Droplet:
+                            return null;
-                        if (GetTexture("fruit-drop") != null)
-                            return new LegacyDropletPiece();
 
-                        return null;
+                        case CatchSkinComponents.Catcher:
+                            decimal version = GetConfig<SkinConfiguration.LegacySetting, decimal>(SkinConfiguration.LegacySetting.Version)?.Value ?? 1;
 
-                    case CatchSkinComponents.Catcher:
+                            if (version < 2.3m)
-                        decimal version = GetConfig<SkinConfiguration.LegacySetting, decimal>(SkinConfiguration.LegacySetting.Version)?.Value ?? 1;
+                            {
+                                if (hasOldStyleCatcherSprite())
+                                    return new LegacyCatcherOld();
+                            }
 
-                        if (version < 2.3m)
+                            if (hasNewStyleCatcherSprite())
-                        {
+                                return new LegacyCatcherNew();
-                            if (hasOldStyleCatcherSprite())
-                                return new LegacyCatcherOld();
-                        }
 
-                        if (hasNewStyleCatcherSprite())
+                            return null;
-                            return new LegacyCatcherNew();
 
-                        return null;
+                        case CatchSkinComponents.CatchComboCounter:
+                            if (providesComboCounter)
+                                return new LegacyCatchComboCounter();
 
-                    case CatchSkinComponents.CatchComboCounter:
+                            return null;
-                        if (providesComboCounter)
-                            return new LegacyCatchComboCounter();
 
-                        return null;
+                        case CatchSkinComponents.HitExplosion:
+                            if (hasOldStyleCatcherSprite() || hasNewStyleCatcherSprite())
+                                return new LegacyHitExplosion();
 
-                    case CatchSkinComponents.HitExplosion:
+                            return null;
-                        if (hasOldStyleCatcherSprite() || hasNewStyleCatcherSprite())
-                            return new LegacyHitExplosion();
 
-                        return null;
+                        default:
-
+                            throw new UnsupportedSkinComponentException(lookup);
-                    default:
+                    }
-                        throw new UnsupportedSkinComponentException(lookup);
-                }
             }
 
             return base.GetDrawableComponent(lookup);

osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableComboCounter.cs

@@ -4,7 +4,6 @@
 using NUnit.Framework;
 using osu.Framework.Allocation;
 using osu.Framework.Graphics;
-using osu.Framework.Testing;
 using osu.Game.Rulesets.Osu;
 using osu.Game.Rulesets.Scoring;
 using osu.Game.Screens.Play.HUD;
@@ -28,17 +27,5 @@ namespace osu.Game.Tests.Visual.Gameplay
 
             AddStep("reset combo", () => scoreProcessor.Combo.Value = 0);
         }
-
-        [Test]
-        public void TestLegacyComboCounterHiddenByRulesetImplementation()
-        {
-            AddToggleStep("toggle legacy hidden by ruleset", visible =>
-            {
-                foreach (var legacyCounter in this.ChildrenOfType<LegacyComboCounter>())
-                    legacyCounter.HiddenByRulesetImplementation = visible;
-            });
-
-            AddRepeatStep("increase combo", () => scoreProcessor.Combo.Value++, 10);
-        }
     }
 }

osu.Game/Skinning/LegacyComboCounter.cs

@@ -43,18 +43,6 @@ namespace osu.Game.Skinning
 
         private readonly Container counterContainer;
 
-        /// <summary>
-        /// Hides the combo counter internally without affecting its <see cref="SerialisedDrawableInfo"/>.
-        /// </summary>
-        /// <remarks>
-        /// This is used for rulesets that provide their own combo counter and don't want this HUD one to be visible,
-        /// without potentially affecting the user's selected skin.
-        /// </remarks>
-        public bool HiddenByRulesetImplementation
-        {
-            set => counterContainer.Alpha = value ? 1 : 0;
-        }
-
         public bool UsesFixedAnchor { get; set; }
 
         public LegacyComboCounter()