From 4769a95b4994b032f64e446272ffed0051230f43 Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Tue, 11 May 2021 11:56:14 +0900
Subject: [PATCH] Fix encapsulation and remove target lookup overhead

---
 .../HUD/SkinnableElementTargetContainer.cs    | 23 ++++++++++++++-----
 osu.Game/Screens/Play/HUD/SkinnableInfo.cs    |  9 --------
 osu.Game/Skinning/IDefaultSkinnableTarget.cs  | 12 ----------
 osu.Game/Skinning/ISkinnableTarget.cs         | 12 +++++++++-
 osu.Game/Skinning/LegacySkin.cs               |  3 +--
 5 files changed, 29 insertions(+), 30 deletions(-)
 delete mode 100644 osu.Game/Skinning/IDefaultSkinnableTarget.cs

diff --git a/osu.Game/Screens/Play/HUD/SkinnableElementTargetContainer.cs b/osu.Game/Screens/Play/HUD/SkinnableElementTargetContainer.cs
index f1b282119b..79a0db1751 100644
--- a/osu.Game/Screens/Play/HUD/SkinnableElementTargetContainer.cs
+++ b/osu.Game/Screens/Play/HUD/SkinnableElementTargetContainer.cs
@@ -3,6 +3,7 @@
 
 using System.Collections.Generic;
 using System.Linq;
+using osu.Framework.Graphics;
 using osu.Game.Extensions;
 using osu.Game.Skinning;
 
@@ -19,6 +20,21 @@ namespace osu.Game.Screens.Play.HUD
             Target = target;
         }
 
+        public void Reload()
+        {
+            content = CurrentSkin.GetDrawableComponent(new SkinnableTargetComponent(Target)) as SkinnableTargetWrapper;
+
+            ClearInternal();
+
+            if (content != null)
+                LoadComponentAsync(content, AddInternal);
+        }
+
+        public void Add(Drawable drawable)
+        {
+            content.Add(drawable);
+        }
+
         public IEnumerable<SkinnableInfo> CreateSerialisedChildren() =>
             content.Select(d => d.CreateSerialisedInformation());
 
@@ -26,12 +42,7 @@ namespace osu.Game.Screens.Play.HUD
         {
             base.SkinChanged(skin, allowFallback);
 
-            content = skin.GetDrawableComponent(new SkinnableTargetComponent(Target)) as SkinnableTargetWrapper;
-
-            ClearInternal();
-
-            if (content != null)
-                LoadComponentAsync(content, AddInternal);
+            Reload();
         }
     }
 }
diff --git a/osu.Game/Screens/Play/HUD/SkinnableInfo.cs b/osu.Game/Screens/Play/HUD/SkinnableInfo.cs
index ec4eaa0e59..abeb96f647 100644
--- a/osu.Game/Screens/Play/HUD/SkinnableInfo.cs
+++ b/osu.Game/Screens/Play/HUD/SkinnableInfo.cs
@@ -4,8 +4,6 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
-using Newtonsoft.Json;
-using Newtonsoft.Json.Converters;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Game.Extensions;
@@ -23,9 +21,6 @@ namespace osu.Game.Screens.Play.HUD
     {
         public Type Type { get; set; }
 
-        [JsonConverter(typeof(StringEnumConverter))]
-        public SkinnableTarget? Target { get; set; }
-
         public Vector2 Position { get; set; }
 
         public float Rotation { get; set; }
@@ -44,10 +39,6 @@ namespace osu.Game.Screens.Play.HUD
         {
             Type = component.GetType();
 
-            ISkinnableTarget target = component.Parent as ISkinnableTarget;
-
-            Target = target?.Target;
-
             Position = component.Position;
             Rotation = component.Rotation;
             Scale = component.Scale;
diff --git a/osu.Game/Skinning/IDefaultSkinnableTarget.cs b/osu.Game/Skinning/IDefaultSkinnableTarget.cs
deleted file mode 100644
index 24fb454af8..0000000000
--- a/osu.Game/Skinning/IDefaultSkinnableTarget.cs
+++ /dev/null
@@ -1,12 +0,0 @@
-// 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.
-
-namespace osu.Game.Skinning
-{
-    /// <summary>
-    /// The default placement location for new <see cref="ISkinnableComponent"/>s.
-    /// </summary>
-    public interface IDefaultSkinnableTarget : ISkinnableTarget
-    {
-    }
-}
diff --git a/osu.Game/Skinning/ISkinnableTarget.cs b/osu.Game/Skinning/ISkinnableTarget.cs
index 90e48c8bba..2379fd4247 100644
--- a/osu.Game/Skinning/ISkinnableTarget.cs
+++ b/osu.Game/Skinning/ISkinnableTarget.cs
@@ -8,8 +8,18 @@ namespace osu.Game.Skinning
     /// <summary>
     /// Denotes a container which can house <see cref="ISkinnableComponent"/>s.
     /// </summary>
-    public interface ISkinnableTarget : IDrawable
+    public interface ISkinnableTarget
     {
         public SkinnableTarget Target { get; }
+
+        /// <summary>
+        /// Reload this target from the current skin.
+        /// </summary>
+        public void Reload();
+
+        /// <summary>
+        /// Add the provided item to this target.
+        /// </summary>
+        public void Add(Drawable drawable);
     }
 }
diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs
index fc03ce909c..3ffd2245c2 100644
--- a/osu.Game/Skinning/LegacySkin.cs
+++ b/osu.Game/Skinning/LegacySkin.cs
@@ -331,9 +331,8 @@ namespace osu.Game.Skinning
                     switch (target.Target)
                     {
                         case SkinnableTarget.MainHUDComponents:
-                            return new SkinnableTargetWrapper(target.Target)
+                            return new SkinnableTargetWrapper
                             {
-                                RelativeSizeAxes = Axes.Both,
                                 Children = new[]
                                 {
                                     // TODO: these should fallback to the osu!classic skin.