Move combo counter to ruleset-specific HUD components container

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 @@ public partial class TestSceneCatchPlayerLegacySkin : LegacySkinPlayerTestScene
         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 @@ public void TestLegacyHUDComboCounterHidden([Values] bool withModifiedSkin)
                 CreateTest();
             }
 
-            AddAssert("legacy HUD combo counter hidden", () =>
-            {
-                return Player.ChildrenOfType<LegacyComboCounter>().All(c => c.ChildrenOfType<Container>().Single().Alpha == 0f);
-            });
+            AddAssert("legacy HUD combo counter not added", () => !Player.ChildrenOfType<LegacyComboCounter>().Any());
         }
     }
 }

osu.Game.Rulesets.Catch/Skinning/Argon/CatchArgonSkinTransformer.cs

@@ -6,7 +6,7 @@
 
 namespace osu.Game.Rulesets.Catch.Skinning.Argon
 {
-    public class CatchArgonSkinTransformer : SkinTransformer
+    public class CatchArgonSkinTransformer : ArgonSkinTransformer
     {
         public CatchArgonSkinTransformer(ISkin skin)
             : base(skin)

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

@@ -1,10 +1,9 @@
 // Copyright (c) ppy Pty Ltd <[email protected]>. 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 @@ public CatchLegacySkinTransformer(ISkin skin)
 
         public override Drawable? GetDrawableComponent(ISkinComponentLookup lookup)
         {
-            if (lookup is SkinComponentsContainerLookup containerLookup)
+            switch (lookup)
             {
-                switch (containerLookup.Target)
-                {
-                    case SkinComponentsContainerLookup.TargetArea.MainHUDComponents:
-                        var components = base.GetDrawableComponent(lookup) as Container;
-
-                        if (providesComboCounter && components != null)
-                        {
-                            // 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 SkinComponentsContainerLookup containerLookup:
+                    switch (containerLookup.Target)
+                    {
+                        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 (lookup is CatchSkinComponentLookup catchSkinComponent)
-            {
-                switch (catchSkinComponent.Component)
-                {
-                    case CatchSkinComponents.Fruit:
-                        if (hasPear)
-                            return new LegacyFruitPiece();
+                    break;
 
-                        return null;
+                case CatchSkinComponentLookup catchSkinComponent:
+                    switch (catchSkinComponent.Component)
+                    {
+                        case CatchSkinComponents.Fruit:
+                            if (hasPear)
+                                return new LegacyFruitPiece();
 
-                    case CatchSkinComponents.Banana:
-                        if (GetTexture("fruit-bananas") != null)
-                            return new LegacyBananaPiece();
+                            return null;
 
-                        return null;
+                        case CatchSkinComponents.Banana:
+                            if (GetTexture("fruit-bananas") != null)
+                                return new LegacyBananaPiece();
 
-                    case CatchSkinComponents.Droplet:
-                        if (GetTexture("fruit-drop") != null)
-                            return new LegacyDropletPiece();
+                            return null;
 
-                        return null;
+                        case CatchSkinComponents.Droplet:
+                            if (GetTexture("fruit-drop") != null)
+                                return new LegacyDropletPiece();
 
-                    case CatchSkinComponents.Catcher:
-                        decimal version = GetConfig<SkinConfiguration.LegacySetting, decimal>(SkinConfiguration.LegacySetting.Version)?.Value ?? 1;
+                            return null;
 
-                        if (version < 2.3m)
-                        {
-                            if (hasOldStyleCatcherSprite())
-                                return new LegacyCatcherOld();
-                        }
+                        case CatchSkinComponents.Catcher:
+                            decimal version = GetConfig<SkinConfiguration.LegacySetting, decimal>(SkinConfiguration.LegacySetting.Version)?.Value ?? 1;
 
-                        if (hasNewStyleCatcherSprite())
-                            return new LegacyCatcherNew();
+                            if (version < 2.3m)
+                            {
+                                if (hasOldStyleCatcherSprite())
+                                    return new LegacyCatcherOld();
+                            }
 
-                        return null;
+                            if (hasNewStyleCatcherSprite())
+                                return new LegacyCatcherNew();
 
-                    case CatchSkinComponents.CatchComboCounter:
-                        if (providesComboCounter)
-                            return new LegacyCatchComboCounter();
+                            return null;
 
-                        return null;
+                        case CatchSkinComponents.CatchComboCounter:
+                            if (providesComboCounter)
+                                return new LegacyCatchComboCounter();
 
-                    case CatchSkinComponents.HitExplosion:
-                        if (hasOldStyleCatcherSprite() || hasNewStyleCatcherSprite())
-                            return new LegacyHitExplosion();
+                            return null;
 
-                        return null;
+                        case CatchSkinComponents.HitExplosion:
+                            if (hasOldStyleCatcherSprite() || hasNewStyleCatcherSprite())
+                                return new LegacyHitExplosion();
+
+                            return null;
 
-                    default:
-                        throw new UnsupportedSkinComponentException(lookup);
-                }
+                        default:
+                            throw new UnsupportedSkinComponentException(lookup);
+                    }
             }
 
             return base.GetDrawableComponent(lookup);

osu.Game.Rulesets.Osu/Skinning/Argon/OsuArgonSkinTransformer.cs

@@ -7,7 +7,7 @@
 
 namespace osu.Game.Rulesets.Osu.Skinning.Argon
 {
-    public class OsuArgonSkinTransformer : SkinTransformer
+    public class OsuArgonSkinTransformer : ArgonSkinTransformer
     {
         public OsuArgonSkinTransformer(ISkin skin)
             : base(skin)

osu.Game.Rulesets.Taiko/Skinning/Argon/TaikoArgonSkinTransformer.cs

@@ -7,16 +7,16 @@
 
 namespace osu.Game.Rulesets.Taiko.Skinning.Argon
 {
-    public class TaikoArgonSkinTransformer : SkinTransformer
+    public class TaikoArgonSkinTransformer : ArgonSkinTransformer
     {
         public TaikoArgonSkinTransformer(ISkin skin)
             : base(skin)
         {
         }
 
-        public override Drawable? GetDrawableComponent(ISkinComponentLookup component)
+        public override Drawable? GetDrawableComponent(ISkinComponentLookup lookup)
         {
-            switch (component)
+            switch (lookup)
             {
                 case GameplaySkinComponentLookup<HitResult> resultComponent:
                     // This should eventually be moved to a skin setting, when supported.
@@ -75,7 +75,7 @@ public TaikoArgonSkinTransformer(ISkin skin)
                     break;
             }
 
-            return base.GetDrawableComponent(component);
+            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 @@ public void TestComboCounterIncrementing()
 
             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/Rulesets/Ruleset.cs

@@ -212,7 +212,19 @@ public virtual LegacyMods ConvertToLegacyMods(Mod[] mods)
         /// <param name="skin">The source skin.</param>
         /// <param name="beatmap">The current beatmap.</param>
         /// <returns>A skin with a transformer applied, or null if no transformation is provided by this ruleset.</returns>
-        public virtual ISkin? CreateSkinTransformer(ISkin skin, IBeatmap beatmap) => null;
+        public virtual ISkin? CreateSkinTransformer(ISkin skin, IBeatmap beatmap)
+        {
+            switch (skin)
+            {
+                case LegacySkin:
+                    return new LegacySkinTransformer(skin);
+
+                case ArgonSkin:
+                    return new ArgonSkinTransformer(skin);
+            }
+
+            return null;
+        }
 
         protected Ruleset()
         {

osu.Game/Skinning/ArgonSkin.cs

@@ -114,15 +114,14 @@ public ArgonSkin(SkinInfo skin, IStorageResourceProvider resources)
                             return songSelectComponents;
 
                         case SkinComponentsContainerLookup.TargetArea.MainHUDComponents:
-                            var skinnableTargetWrapper = new DefaultSkinComponentsContainer(container =>
+                            var mainHUDComponents = new DefaultSkinComponentsContainer(container =>
                             {
                                 var health = container.OfType<ArgonHealthDisplay>().FirstOrDefault();
                                 var healthLine = container.OfType<BoxElement>().FirstOrDefault();
                                 var wedgePieces = container.OfType<ArgonWedgePiece>().ToArray();
                                 var score = container.OfType<ArgonScoreCounter>().FirstOrDefault();
                                 var accuracy = container.OfType<ArgonAccuracyCounter>().FirstOrDefault();
                                 var performancePoints = container.OfType<ArgonPerformancePointsCounter>().FirstOrDefault();
-                                var combo = container.OfType<ArgonComboCounter>().FirstOrDefault();
                                 var songProgress = container.OfType<ArgonSongProgress>().FirstOrDefault();
                                 var keyCounter = container.OfType<ArgonKeyCounterDisplay>().FirstOrDefault();
 
@@ -203,13 +202,6 @@ public ArgonSkin(SkinInfo skin, IStorageResourceProvider resources)
                                             keyCounter.Origin = Anchor.BottomRight;
                                             keyCounter.Position = new Vector2(-(hitError.Width + padding), -(padding * 2 + song_progress_offset_height));
                                         }
-
-                                        if (combo != null && hitError != null)
-                                        {
-                                            combo.Anchor = Anchor.BottomLeft;
-                                            combo.Origin = Anchor.BottomLeft;
-                                            combo.Position = new Vector2((hitError.Width + padding), -(padding * 2 + song_progress_offset_height));
-                                        }
                                     }
                                 }
                             })
@@ -239,18 +231,14 @@ public ArgonSkin(SkinInfo skin, IStorageResourceProvider resources)
                                     {
                                         Scale = new Vector2(0.8f),
                                     },
-                                    new ArgonComboCounter
-                                    {
-                                        Scale = new Vector2(1.3f)
-                                    },
                                     new BarHitErrorMeter(),
                                     new BarHitErrorMeter(),
                                     new ArgonSongProgress(),
                                     new ArgonKeyCounterDisplay(),
                                 }
                             };
 
-                            return skinnableTargetWrapper;
+                            return mainHUDComponents;
                     }
 
                     return null;

osu.Game/Skinning/ArgonSkinTransformer.cs

@@ -0,0 +1,53 @@
+// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System.Linq;
+using osu.Framework.Graphics;
+using osu.Game.Screens.Play.HUD;
+using osuTK;
+
+namespace osu.Game.Skinning
+{
+    public class ArgonSkinTransformer : SkinTransformer
+    {
+        public ArgonSkinTransformer(ISkin skin)
+            : base(skin)
+        {
+        }
+
+        public override Drawable? GetDrawableComponent(ISkinComponentLookup lookup)
+        {
+            switch (lookup)
+            {
+                case SkinComponentsContainerLookup containerLookup:
+                    switch (containerLookup.Target)
+                    {
+                        case SkinComponentsContainerLookup.TargetArea.MainHUDComponents when containerLookup.Ruleset != null:
+                            var rulesetHUDComponents = Skin.GetDrawableComponent(lookup);
+
+                            rulesetHUDComponents ??= new DefaultSkinComponentsContainer(container =>
+                            {
+                                var combo = container.OfType<ArgonComboCounter>().FirstOrDefault();
+
+                                if (combo != null)
+                                {
+                                    combo.Anchor = Anchor.BottomLeft;
+                                    combo.Origin = Anchor.BottomLeft;
+                                    combo.Position = new Vector2(36, -66);
+                                    combo.Scale = new Vector2(1.3f);
+                                }
+                            })
+                            {
+                                new ArgonComboCounter(),
+                            };
+
+                            return rulesetHUDComponents;
+                    }
+
+                    break;
+            }
+
+            return base.GetDrawableComponent(lookup);
+        }
+    }
+}

osu.Game/Skinning/LegacyComboCounter.cs

@@ -43,18 +43,6 @@ public partial class LegacyComboCounter : CompositeDrawable, ISerialisableDrawab
 
         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()

osu.Game/Skinning/LegacySkin.cs

@@ -402,7 +402,6 @@ protected override void ParseConfigurationStream(Stream stream)
                             {
                                 Children = new Drawable[]
                                 {
-                                    new LegacyComboCounter(),
                                     new LegacyScoreCounter(),
                                     new LegacyAccuracyCounter(),
                                     new LegacySongProgress(),