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,8 @@
 // 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.Bindables;
 using osu.Framework.Graphics;
-using osu.Framework.Graphics.Containers;
 using osu.Game.Skinning;
 using osuTK.Graphics;
 
@@ -28,76 +26,68 @@ 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:
+                            // 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/TestSceneSkinEditor.cs

@@ -7,21 +7,25 @@
 using System.Text;
 using NUnit.Framework;
 using osu.Framework.Allocation;
+using osu.Framework.Extensions;
 using osu.Framework.Extensions.IEnumerableExtensions;
 using osu.Framework.Extensions.ObjectExtensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.UserInterface;
 using osu.Framework.Input;
 using osu.Framework.Testing;
+using osu.Game.Database;
 using osu.Game.Overlays;
 using osu.Game.Overlays.Settings;
 using osu.Game.Overlays.SkinEditor;
 using osu.Game.Rulesets;
 using osu.Game.Rulesets.Osu;
 using osu.Game.Screens.Edit;
+using osu.Game.Screens.Play.HUD;
 using osu.Game.Screens.Play.HUD.HitErrorMeters;
 using osu.Game.Skinning;
 using osu.Game.Skinning.Components;
+using osu.Game.Tests.Resources;
 using osuTK;
 using osuTK.Input;
 
@@ -39,13 +43,17 @@ public partial class TestSceneSkinEditor : PlayerTestScene
         [Cached]
         public readonly EditorClipboard Clipboard = new EditorClipboard();
 
+        [Resolved]
+        private SkinManager skins { get; set; } = null!;
+
         private SkinComponentsContainer targetContainer => Player.ChildrenOfType<SkinComponentsContainer>().First();
 
         [SetUpSteps]
         public override void SetUpSteps()
         {
             base.SetUpSteps();
 
+            AddStep("reset skin", () => skins.CurrentSkinInfo.SetDefault());
             AddUntilStep("wait for hud load", () => targetContainer.ComponentsLoaded);
 
             AddStep("reload skin editor", () =>
@@ -369,6 +377,93 @@ public void TestCopyPaste()
                 () => Is.EqualTo(3));
         }
 
+        private SkinComponentsContainer globalHUDTarget => Player.ChildrenOfType<SkinComponentsContainer>()
+                                                                 .Single(c => c.Lookup.Target == SkinComponentsContainerLookup.TargetArea.MainHUDComponents && c.Lookup.Ruleset == null);
+
+        private SkinComponentsContainer rulesetHUDTarget => Player.ChildrenOfType<SkinComponentsContainer>()
+                                                                  .Single(c => c.Lookup.Target == SkinComponentsContainerLookup.TargetArea.MainHUDComponents && c.Lookup.Ruleset != null);
+
+        [Test]
+        public void TestMigrationArgon()
+        {
+            Live<SkinInfo> importedSkin = null!;
+
+            AddStep("import old argon skin", () => skins.CurrentSkinInfo.Value = importedSkin = importSkinFromArchives(@"argon-layout-version-0.osk").SkinInfo);
+            AddUntilStep("wait for load", () => globalHUDTarget.ComponentsLoaded && rulesetHUDTarget.ComponentsLoaded);
+            AddAssert("no combo in global target", () => !globalHUDTarget.Components.OfType<ArgonComboCounter>().Any());
+            AddAssert("combo placed in ruleset target", () => rulesetHUDTarget.Components.OfType<ArgonComboCounter>().Count() == 1);
+
+            AddStep("add combo to global target", () => globalHUDTarget.Add(new ArgonComboCounter
+            {
+                Anchor = Anchor.Centre,
+                Origin = Anchor.Centre,
+                Scale = new Vector2(2f),
+            }));
+            AddStep("save skin", () => skins.Save(skins.CurrentSkin.Value));
+
+            AddStep("select another skin", () => skins.CurrentSkinInfo.SetDefault());
+            AddStep("select skin again", () => skins.CurrentSkinInfo.Value = importedSkin);
+            AddUntilStep("wait for load", () => globalHUDTarget.ComponentsLoaded && rulesetHUDTarget.ComponentsLoaded);
+            AddAssert("combo placed in global target", () => globalHUDTarget.Components.OfType<ArgonComboCounter>().Count() == 1);
+            AddAssert("combo placed in ruleset target", () => rulesetHUDTarget.Components.OfType<ArgonComboCounter>().Count() == 1);
+        }
+
+        [Test]
+        public void TestMigrationTriangles()
+        {
+            Live<SkinInfo> importedSkin = null!;
+
+            AddStep("import old triangles skin", () => skins.CurrentSkinInfo.Value = importedSkin = importSkinFromArchives(@"triangles-layout-version-0.osk").SkinInfo);
+            AddUntilStep("wait for load", () => globalHUDTarget.ComponentsLoaded && rulesetHUDTarget.ComponentsLoaded);
+            AddAssert("no combo in global target", () => !globalHUDTarget.Components.OfType<DefaultComboCounter>().Any());
+            AddAssert("combo placed in ruleset target", () => rulesetHUDTarget.Components.OfType<DefaultComboCounter>().Count() == 1);
+
+            AddStep("add combo to global target", () => globalHUDTarget.Add(new DefaultComboCounter
+            {
+                Anchor = Anchor.Centre,
+                Origin = Anchor.Centre,
+                Scale = new Vector2(2f),
+            }));
+            AddStep("save skin", () => skins.Save(skins.CurrentSkin.Value));
+
+            AddStep("select another skin", () => skins.CurrentSkinInfo.SetDefault());
+            AddStep("select skin again", () => skins.CurrentSkinInfo.Value = importedSkin);
+            AddUntilStep("wait for load", () => globalHUDTarget.ComponentsLoaded && rulesetHUDTarget.ComponentsLoaded);
+            AddAssert("combo placed in global target", () => globalHUDTarget.Components.OfType<DefaultComboCounter>().Count() == 1);
+            AddAssert("combo placed in ruleset target", () => rulesetHUDTarget.Components.OfType<DefaultComboCounter>().Count() == 1);
+        }
+
+        [Test]
+        public void TestMigrationLegacy()
+        {
+            Live<SkinInfo> importedSkin = null!;
+
+            AddStep("import old classic skin", () => skins.CurrentSkinInfo.Value = importedSkin = importSkinFromArchives(@"classic-layout-version-0.osk").SkinInfo);
+            AddUntilStep("wait for load", () => globalHUDTarget.ComponentsLoaded && rulesetHUDTarget.ComponentsLoaded);
+            AddAssert("no combo in global target", () => !globalHUDTarget.Components.OfType<LegacyComboCounter>().Any());
+            AddAssert("combo placed in ruleset target", () => rulesetHUDTarget.Components.OfType<LegacyComboCounter>().Count() == 1);
+
+            AddStep("add combo to global target", () => globalHUDTarget.Add(new LegacyComboCounter
+            {
+                Anchor = Anchor.Centre,
+                Origin = Anchor.Centre,
+                Scale = new Vector2(2f),
+            }));
+            AddStep("save skin", () => skins.Save(skins.CurrentSkin.Value));
+
+            AddStep("select another skin", () => skins.CurrentSkinInfo.SetDefault());
+            AddStep("select skin again", () => skins.CurrentSkinInfo.Value = importedSkin);
+            AddUntilStep("wait for load", () => globalHUDTarget.ComponentsLoaded && rulesetHUDTarget.ComponentsLoaded);
+            AddAssert("combo placed in global target", () => globalHUDTarget.Components.OfType<LegacyComboCounter>().Count() == 1);
+            AddAssert("combo placed in ruleset target", () => rulesetHUDTarget.Components.OfType<LegacyComboCounter>().Count() == 1);
+        }
+
+        private Skin importSkinFromArchives(string filename)
+        {
+            var imported = skins.Import(new ImportTask(TestResources.OpenResource($@"Archives/{filename}"), filename)).GetResultSafely();
+            return imported.PerformRead(skinInfo => skins.GetSkin(skinInfo));
+        }
+
         protected override Ruleset CreatePlayerRuleset() => new OsuRuleset();
 
         private partial class TestSkinEditorChangeHandler : SkinEditorChangeHandler

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()
         {