From 5bd06832d0a4e35bbbd52eea2ffb3970764e6d0e Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Fri, 28 Jul 2023 15:48:40 +0900
Subject: [PATCH] Fix skin component toolbox not working correctly for ruleset
 matching

Until now, the only usage of ruleset layers was where there is both a
ruleset specific and non-ruleset-specific layer present. The matching
code was making assumptions about this.

As I tried to add a new playfield layer which breaks this assumption,
non-ruleset-specifc components were not being displayed in the toolbox.
This turned out to be due to a `target` of `null` being provided due to
the weird `getTarget` matching (that happened to *just* do what we
wanted previously due to the equals implementation, but only because
there was a container without the ruleset present in the available
targets).

I've changed this to be a more appropriate lookup method, where the
target for dependency sourcing is provided separately from the ruleset
filter.
---
 .../Overlays/SkinEditor/SkinComponentToolbox.cs | 17 +++++++++++++----
 osu.Game/Overlays/SkinEditor/SkinEditor.cs      |  4 ++--
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/osu.Game/Overlays/SkinEditor/SkinComponentToolbox.cs b/osu.Game/Overlays/SkinEditor/SkinComponentToolbox.cs
index 1ce253d67c..a476fc1a6d 100644
--- a/osu.Game/Overlays/SkinEditor/SkinComponentToolbox.cs
+++ b/osu.Game/Overlays/SkinEditor/SkinComponentToolbox.cs
@@ -13,6 +13,7 @@ using osu.Framework.Threading;
 using osu.Game.Graphics.Sprites;
 using osu.Game.Graphics.UserInterface;
 using osu.Game.Localisation;
+using osu.Game.Rulesets;
 using osu.Game.Screens.Edit.Components;
 using osu.Game.Skinning;
 using osuTK;
@@ -23,14 +24,22 @@ namespace osu.Game.Overlays.SkinEditor
     {
         public Action<Type>? RequestPlacement;
 
-        private readonly SkinComponentsContainer? target;
+        private readonly SkinComponentsContainer target;
+
+        private readonly RulesetInfo? ruleset;
 
         private FillFlowContainer fill = null!;
 
-        public SkinComponentToolbox(SkinComponentsContainer? target = null)
-            : base(target?.Lookup.Ruleset == null ? SkinEditorStrings.Components : LocalisableString.Interpolate($"{SkinEditorStrings.Components} ({target.Lookup.Ruleset.Name})"))
+        /// <summary>
+        /// Create a new component toolbox for the specified taget.
+        /// </summary>
+        /// <param name="target">The target. This is mainly used as a dependency source to find candidate components.</param>
+        /// <param name="ruleset">A ruleset to filter components by. If null, only components which are not ruleset-specific will be included.</param>
+        public SkinComponentToolbox(SkinComponentsContainer target, RulesetInfo? ruleset)
+            : base(ruleset == null ? SkinEditorStrings.Components : LocalisableString.Interpolate($"{SkinEditorStrings.Components} ({ruleset.Name})"))
         {
             this.target = target;
+            this.ruleset = ruleset;
         }
 
         [BackgroundDependencyLoader]
@@ -51,7 +60,7 @@ namespace osu.Game.Overlays.SkinEditor
         {
             fill.Clear();
 
-            var skinnableTypes = SerialisedDrawableInfo.GetAllAvailableDrawables(target?.Lookup.Ruleset);
+            var skinnableTypes = SerialisedDrawableInfo.GetAllAvailableDrawables(ruleset);
             foreach (var type in skinnableTypes)
                 attemptAddComponent(type);
         }
diff --git a/osu.Game/Overlays/SkinEditor/SkinEditor.cs b/osu.Game/Overlays/SkinEditor/SkinEditor.cs
index 2b23ce290f..67cf0eab18 100644
--- a/osu.Game/Overlays/SkinEditor/SkinEditor.cs
+++ b/osu.Game/Overlays/SkinEditor/SkinEditor.cs
@@ -366,14 +366,14 @@ namespace osu.Game.Overlays.SkinEditor
             // If the new target has a ruleset, let's show ruleset-specific items at the top, and the rest below.
             if (target.NewValue.Ruleset != null)
             {
-                componentsSidebar.Add(new SkinComponentToolbox(skinComponentsContainer)
+                componentsSidebar.Add(new SkinComponentToolbox(skinComponentsContainer, target.NewValue.Ruleset)
                 {
                     RequestPlacement = requestPlacement
                 });
             }
 
             // Remove the ruleset from the lookup to get base components.
-            componentsSidebar.Add(new SkinComponentToolbox(getTarget(new SkinComponentsContainerLookup(target.NewValue.Target)))
+            componentsSidebar.Add(new SkinComponentToolbox(skinComponentsContainer, null)
             {
                 RequestPlacement = requestPlacement
             });