Add migration logic for legacy health displays

osu.Game.Tests/Visual/Skinning/TestSceneSkinMigration.cs

@@ -44,7 +44,7 @@ namespace osu.Game.Tests.Visual.Skinning
         {
             LegacySkin skin = null!;
 
-            AddStep("load skin with some configuration", () =>
+            AddStep("load skin", () =>
             {
                 skin = loadSkin<LegacySkin>(new Dictionary<GlobalSkinnableContainers, SkinLayoutInfo>
                 {
@@ -120,16 +120,195 @@ namespace osu.Game.Tests.Visual.Skinning
 
         #endregion
 
+        #region Version 2
+
+        [Test]
+        public void TestMigration_2()
+        {
+            LegacySkin skin = null!;
+
+            AddStep("load skin", () =>
+            {
+                skin = loadSkin<LegacySkin>(new Dictionary<GlobalSkinnableContainers, SkinLayoutInfo>
+                {
+                    {
+                        GlobalSkinnableContainers.MainHUDComponents,
+                        createLayout(1, [nameof(LegacyHealthDisplay)])
+                    },
+                    {
+                        GlobalSkinnableContainers.Playfield,
+                        createLayout(1)
+                    }
+                });
+            });
+
+            // HUD
+            AddAssert("health display removed from global HUD", () =>
+            {
+                var dict = skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].DrawableInfo;
+                return dict.Single(kvp => kvp.Key == @"global").Value.Single().Type.Name == nameof(BigBlackBox);
+            });
+            AddAssert("health display moved to each ruleset except mania", () =>
+            {
+                var dict = skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].DrawableInfo.ToArray();
+                dict = dict.Where(kvp => kvp.Key != @"global" && kvp.Key != @"mania").ToArray();
+
+                return dict.All(kvp => kvp.Value.Select(d => d.Type.Name).SequenceEqual([nameof(BigBlackBox), nameof(LegacyHealthDisplay)])) &&
+                       dict.All(kvp => kvp.Value.Single(d => d.Type.Name == nameof(LegacyHealthDisplay)).Rotation == 0f);
+            });
+            AddAssert("no health display in mania HUD", () =>
+            {
+                var dict = skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].DrawableInfo;
+                return dict.Single(kvp => kvp.Key == @"mania").Value.Single().Type.Name == nameof(BigBlackBox);
+            });
+
+            // Playfield
+            AddAssert("health display in mania moved to playfield", () =>
+            {
+                var dict = skin.LayoutInfos[GlobalSkinnableContainers.Playfield].DrawableInfo;
+                return dict.Single(kvp => kvp.Key == @"mania").Value.Select(d => d.Type.Name).SequenceEqual([nameof(BigBlackBox), nameof(LegacyHealthDisplay)]) &&
+                       dict.Single(kvp => kvp.Key == @"mania").Value.Single(d => d.Type.Name == nameof(LegacyHealthDisplay)).Rotation == -90f;
+            });
+            AddAssert("rest is unaffected", () =>
+            {
+                var dict = skin.LayoutInfos[GlobalSkinnableContainers.Playfield].DrawableInfo;
+                return dict.Where(kvp => kvp.Key != @"mania").All(kvp => kvp.Value.Single().Type.Name == nameof(BigBlackBox));
+            });
+        }
+
+        [Test]
+        public void TestMigration_2_NonLegacySkin()
+        {
+            ArgonSkin skin = null!;
+
+            AddStep("load argon skin", () =>
+            {
+                skin = loadSkin<ArgonSkin>(new Dictionary<GlobalSkinnableContainers, SkinLayoutInfo>
+                {
+                    {
+                        GlobalSkinnableContainers.MainHUDComponents,
+                        createLayout(1, [nameof(LegacyHealthDisplay)])
+                    },
+                    {
+                        GlobalSkinnableContainers.Playfield,
+                        createLayout(1)
+                    }
+                });
+            });
+
+            // One may argue that if a LegacyHealthDisplay exists in a non-legacy skin,
+            // then it should be swapped with the mania variant similar to legacy skins.
+            // This is not simple to achieve as we have to be aware of the presence of
+            // the health display in the HUD layout while migrating the Playfield layout,
+            // which is impossible with the current structure of skin layout migration.
+            // Instead, don't touch any non-legacy skin and call it a day.
+
+            // HUD
+            AddAssert("health display still in global HUD", () =>
+            {
+                var dict = skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].DrawableInfo;
+
+                return dict.Single(kvp => kvp.Key == @"global").Value.Select(d => d.Type.Name).SequenceEqual([nameof(BigBlackBox), nameof(LegacyHealthDisplay)]) &&
+                       dict.Single(kvp => kvp.Key == @"global").Value.Single(d => d.Type.Name == nameof(LegacyHealthDisplay)).Rotation == 0f;
+            });
+            AddAssert("ruleset HUDs unaffected", () =>
+            {
+                var dict = skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].DrawableInfo.ToArray();
+                return dict.Where(kvp => kvp.Key != @"global").All(kvp => kvp.Value.Single().Type.Name == nameof(BigBlackBox));
+            });
+
+            // Playfield
+            AddAssert("playfield unaffected", () =>
+            {
+                var dict = skin.LayoutInfos[GlobalSkinnableContainers.Playfield].DrawableInfo.ToArray();
+                return dict.All(kvp => kvp.Value.Single().Type.Name == nameof(BigBlackBox));
+            });
+        }
+
+        [Test]
+        public void TestMigration_2_NoHUD()
+        {
+            LegacySkin skin = null!;
+
+            AddStep("load skin", () =>
+            {
+                skin = loadSkin<LegacySkin>(new Dictionary<GlobalSkinnableContainers, SkinLayoutInfo>
+                {
+                    {
+                        GlobalSkinnableContainers.Playfield,
+                        createLayout(1)
+                    },
+                });
+            });
+
+            // In this case, we must add a health display to the Playfield target,
+            // otherwise on mania the user will not see a health display anymore.
+
+            // HUD
+            AddAssert("HUD not configured", () => !skin.LayoutInfos.ContainsKey(GlobalSkinnableContainers.MainHUDComponents));
+
+            // Playfield
+            AddAssert("health display in mania moved to playfield", () =>
+            {
+                var dict = skin.LayoutInfos[GlobalSkinnableContainers.Playfield].DrawableInfo;
+                return dict.Single(kvp => kvp.Key == @"mania").Value.Select(d => d.Type.Name).SequenceEqual([nameof(BigBlackBox), nameof(LegacyHealthDisplay)]) &&
+                       dict.Single(kvp => kvp.Key == @"mania").Value.Single(d => d.Type.Name == nameof(LegacyHealthDisplay)).Rotation == -90f;
+            });
+            AddAssert("rest is unaffected", () =>
+            {
+                var dict = skin.LayoutInfos[GlobalSkinnableContainers.Playfield].DrawableInfo;
+                return dict.Where(kvp => kvp.Key != @"mania").All(d => d.Value.Single().Type.Name == nameof(BigBlackBox));
+            });
+        }
+
+        [Test]
+        public void TestMigration_2_NoPlayfield()
+        {
+            LegacySkin skin = null!;
+
+            AddStep("load skin", () =>
+            {
+                skin = loadSkin<LegacySkin>(new Dictionary<GlobalSkinnableContainers, SkinLayoutInfo>
+                {
+                    {
+                        GlobalSkinnableContainers.MainHUDComponents,
+                        createLayout(1, [nameof(LegacyHealthDisplay)])
+                    }
+                });
+            });
+
+            // HUD
+            AddAssert("health display removed from global HUD", () =>
+            {
+                var dict = skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].DrawableInfo;
+                return dict.Single(kvp => kvp.Key == @"global").Value.Single().Type.Name == nameof(BigBlackBox);
+            });
+            AddAssert("health display moved to each ruleset except mania", () =>
+            {
+                var dict = skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].DrawableInfo.ToArray();
+                dict = dict.Where(kvp => kvp.Key != @"global" && kvp.Key != @"mania").ToArray();
+
+                return dict.All(kvp => kvp.Value.Select(d => d.Type.Name).SequenceEqual([nameof(BigBlackBox), nameof(LegacyHealthDisplay)])) &&
+                       dict.All(kvp => kvp.Value.Single(d => d.Type.Name == nameof(LegacyHealthDisplay)).Rotation == 0f);
+            });
+            AddAssert("no health display in mania HUD", () =>
+            {
+                var dict = skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].DrawableInfo;
+                return dict.Single(kvp => kvp.Key == @"mania").Value.Single().Type.Name == nameof(BigBlackBox);
+            });
+
+            // Playfield
+            AddAssert("playfield not configured", () => !skin.LayoutInfos.ContainsKey(GlobalSkinnableContainers.Playfield));
+        }
+
+        #endregion
+
         private SkinLayoutInfo createLayout(int version, string[]? globalComponents = null, string? ruleset = null, string[]? rulesetComponents = null)
         {
-            var info = new SkinLayoutInfo
+            var info = new SkinLayoutInfo { Version = version };
-            {
+
-                Version = version,
+            if (globalComponents != null)
-                DrawableInfo =
+                info.DrawableInfo.Add(@"global", globalComponents.Select(c => resolveComponent(c).CreateSerialisedInfo()).ToArray());
-                {
-                    { "global", globalComponents?.Select(c => resolveComponent(c).CreateSerialisedInfo()).ToArray() ?? Array.Empty<SerialisedDrawableInfo>() },
-                }
-            };
 
             if (ruleset != null && rulesetComponents != null)
                 info.DrawableInfo.Add(ruleset, rulesetComponents.Select(c => resolveComponent(c).CreateSerialisedInfo()).ToArray());

osu.Game/Skinning/LegacySkin.cs

@@ -376,7 +376,8 @@ namespace osu.Game.Skinning
                                     }
                                 })
                                 {
-                                    new LegacyDefaultComboCounter()
+                                    new LegacyDefaultComboCounter(),
+                                    new LegacyHealthDisplay(),
                                 };
                             }
 
@@ -415,7 +416,6 @@ namespace osu.Game.Skinning
                                     new LegacyScoreCounter(),
                                     new LegacyAccuracyCounter(),
                                     new LegacySongProgress(),
-                                    new LegacyHealthDisplay(),
                                     new BarHitErrorMeter(),
                                 }
                             };

osu.Game/Skinning/Skin.cs

@@ -23,6 +23,7 @@ using osu.Game.Database;
 using osu.Game.IO;
 using osu.Game.Rulesets;
 using osu.Game.Screens.Play.HUD;
+using osuTK;
 
 namespace osu.Game.Skinning
 {
@@ -277,6 +278,10 @@ namespace osu.Game.Skinning
 
         private void applyMigration(SkinLayoutInfo layout, GlobalSkinnableContainers target, int version)
         {
+            Debug.Assert(resources != null);
+
+            bool isLegacySkin = SkinInfo.PerformRead(s => s.GetInstanceType().IsAssignableTo(typeof(LegacySkin)));
+
             switch (version)
             {
                 case 1:
@@ -284,8 +289,7 @@ namespace osu.Game.Skinning
                     // Combo counters were moved out of the global HUD components into per-ruleset.
                     // This is to allow some rulesets to customise further (ie. mania and catch moving the combo to within their play area).
                     if (target != GlobalSkinnableContainers.MainHUDComponents ||
-                        !layout.TryGetDrawableInfo(null, out var globalHUDComponents) ||
+                        !layout.TryGetDrawableInfo(null, out var globalHUDComponents))
-                        resources == null)
                         break;
 
                     var comboCounters = globalHUDComponents.Where(c =>
@@ -307,6 +311,62 @@ namespace osu.Game.Skinning
 
                     break;
                 }
+
+                case 2:
+                {
+                    // Health displays are moved out of the global HUD components and into per-ruleset,
+                    // except for osu!mania, wherein the health display is moved to the Playfield target.
+                    switch (target)
+                    {
+                        case GlobalSkinnableContainers.MainHUDComponents:
+                            if (!isLegacySkin || !layout.TryGetDrawableInfo(null, out var globalHUDComponents))
+                                break;
+
+                            var healthDisplays = globalHUDComponents.Where(c => c.Type.Name == nameof(LegacyHealthDisplay)).ToArray();
+                            layout.Update(null, globalHUDComponents.Except(healthDisplays).ToArray());
+
+                            resources.RealmAccess.Run(r =>
+                            {
+                                foreach (var ruleset in r.All<RulesetInfo>())
+                                {
+                                    // for mania, the health display is moved from MainHUDComponents to Playfield.
+                                    if (ruleset.ShortName == @"mania")
+                                        continue;
+
+                                    layout.Update(ruleset, layout.TryGetDrawableInfo(ruleset, out var rulesetHUDComponents)
+                                        ? rulesetHUDComponents.Concat(healthDisplays).ToArray()
+                                        : healthDisplays);
+                                }
+                            });
+
+                            break;
+
+                        case GlobalSkinnableContainers.Playfield:
+                            if (!isLegacySkin)
+                                break;
+
+                            resources.RealmAccess.Run(r =>
+                            {
+                                var maniaRuleset = r.Find<RulesetInfo>(@"mania");
+
+                                if (!layout.TryGetDrawableInfo(maniaRuleset, out var maniaPlayfieldComponents))
+                                    maniaPlayfieldComponents = Array.Empty<SerialisedDrawableInfo>();
+
+                                layout.Update(maniaRuleset, maniaPlayfieldComponents.Append(new LegacyHealthDisplay
+                                {
+                                    Rotation = -90f,
+                                    Anchor = Anchor.BottomRight,
+                                    Origin = Anchor.TopLeft,
+                                    X = 1,
+                                    Scale = new Vector2(0.7f),
+                                }.CreateSerialisedInfo()).ToArray());
+                            });
+
+                            break;
+                    }
+
+                    break;
+                }
             }
         }
 

osu.Game/Skinning/SkinInfo.cs

@@ -39,7 +39,7 @@ namespace osu.Game.Skinning
 
         public bool Protected { get; set; }
 
-        public virtual Skin CreateInstance(IStorageResourceProvider resources)
+        public Type GetInstanceType()
         {
             var type = string.IsNullOrEmpty(InstantiationInfo)
                 // handle the case of skins imported before InstantiationInfo was added.
@@ -52,12 +52,14 @@ namespace osu.Game.Skinning
                 // for user modified skins. This aims to amicably handle that.
                 // If we ever add more default skins in the future this will need some kind of proper migration rather than
                 // a single fallback.
-                return new TrianglesSkin(this, resources);
+                return typeof(TrianglesSkin);
             }
 
-            return (Skin)Activator.CreateInstance(type, this, resources)!;
+            return type;
         }
 
+        public virtual Skin CreateInstance(IStorageResourceProvider resources) => (Skin)Activator.CreateInstance(GetInstanceType(), this, resources)!;
+
         public virtual IList<RealmNamedFileUsage> Files { get; } = null!;
 
         public bool DeletePending { get; set; }

osu.Game/Skinning/SkinLayoutInfo.cs

@@ -26,9 +26,10 @@ namespace osu.Game.Skinning
         /// <list type="bullet">
         /// <item><description>0: Initial version of all skin layouts.</description></item>
         /// <item><description>1: Moves existing combo counters from global to per-ruleset HUD targets.</description></item>
+        /// <item><description>2: Moves existing legacy health bars from global to per-ruleset HUD targets, and to playfield target on mania.</description></item>
         /// </list>
         /// </remarks>
-        public const int LATEST_VERSION = 1;
+        public const int LATEST_VERSION = 2;
 
         [JsonProperty(DefaultValueHandling = DefaultValueHandling.Populate)]
         public int Version = LATEST_VERSION;