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

@@ -37,25 +37,14 @@ public CatchLegacySkinTransformer(ISkin skin)
 
                     // Modifications for global components.
                     if (containerLookup.Ruleset == null)
-                    {
-                        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;
-                    }
+                        return base.GetDrawableComponent(lookup) as Container;
 
                     // Skin has configuration.
                     if (base.GetDrawableComponent(lookup) is Drawable d)
                         return d;
 
                     // Our own ruleset components default.
+                    // todo: remove CatchSkinComponents.CatchComboCounter and refactor LegacyCatchComboCounter to be added here instead.
                     return new DefaultSkinComponentsContainer(container =>
                     {
                         var keyCounter = container.OfType<LegacyKeyCounterDisplay>().FirstOrDefault();

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

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,40 @@
+// 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 osu.Framework.Graphics;
+using osu.Framework.Graphics.Containers;
+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)
+        {
+            if (lookup is SkinComponentsContainerLookup containerLookup
+                && containerLookup.Target == SkinComponentsContainerLookup.TargetArea.MainHUDComponents
+                && containerLookup.Ruleset != null)
+            {
+                return base.GetDrawableComponent(lookup) ?? new Container
+                {
+                    RelativeSizeAxes = Axes.Both,
+                    Child = new ArgonComboCounter
+                    {
+                        Anchor = Anchor.BottomLeft,
+                        Origin = Anchor.BottomLeft,
+                        Position = new Vector2(36, -66),
+                        Scale = new Vector2(1.3f),
+                    },
+                };
+            }
+
+            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()