Fix deleting skin elements not saving out to skin

Closes https://github.com/ppy/osu/issues/12786.

osu.Game/Skinning/Editor/SkinEditor.cs

@@ -2,6 +2,7 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System;
+using System.Collections.Generic;
 using System.Linq;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
@@ -193,14 +194,16 @@ private void placeComponent(Type type)
             SelectedComponents.Add(component);
         }
 
+        private IEnumerable<ISkinnableTarget> availableTargets => targetScreen.ChildrenOfType<ISkinnableTarget>();
+
         private ISkinnableTarget getTarget(SkinnableTarget target)
         {
-            return targetScreen.ChildrenOfType<ISkinnableTarget>().FirstOrDefault(c => c.Target == target);
+            return availableTargets.FirstOrDefault(c => c.Target == target);
         }
 
         private void revert()
         {
-            SkinnableTargetContainer[] targetContainers = targetScreen.ChildrenOfType<SkinnableTargetContainer>().ToArray();
+            ISkinnableTarget[] targetContainers = availableTargets.ToArray();
 
             foreach (var t in targetContainers)
             {
@@ -216,7 +219,7 @@ public void Save()
             if (!hasBegunMutating)
                 return;
 
-            SkinnableTargetContainer[] targetContainers = targetScreen.ChildrenOfType<SkinnableTargetContainer>().ToArray();
+            ISkinnableTarget[] targetContainers = availableTargets.ToArray();
 
             foreach (var t in targetContainers)
                 currentSkin.Value.UpdateDrawableTarget(t);
@@ -237,5 +240,11 @@ protected override void PopOut()
         {
             this.FadeOut(TRANSITION_DURATION, Easing.OutQuint);
         }
+
+        public void DeleteItems(ISkinnableDrawable[] items)
+        {
+            foreach (var item in items.ToArray())
+                availableTargets.FirstOrDefault(t => t.Components.Contains(item))?.Remove(item);
+        }
     }
 }

osu.Game/Skinning/Editor/SkinSelectionHandler.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using osu.Framework.Allocation;
 using osu.Framework.Extensions.EnumExtensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.UserInterface;
@@ -17,6 +18,9 @@ namespace osu.Game.Skinning.Editor
 {
     public class SkinSelectionHandler : SelectionHandler<ISkinnableDrawable>
     {
+        [Resolved]
+        private SkinEditor skinEditor { get; set; }
+
         public override bool HandleRotation(float angle)
         {
             // TODO: this doesn't correctly account for origin/anchor specs being different in a multi-selection.
@@ -72,14 +76,8 @@ protected override void OnSelectionChanged()
             SelectionBox.CanReverse = false;
         }
 
-        protected override void DeleteItems(IEnumerable<ISkinnableDrawable> items)
-        {
-            foreach (var i in items)
-            {
-                ((Drawable)i).Expire();
-                SelectedItems.Remove(i);
-            }
-        }
+        protected override void DeleteItems(IEnumerable<ISkinnableDrawable> items) =>
+            skinEditor.DeleteItems(items.ToArray());
 
         protected override IEnumerable<MenuItem> GetContextMenuItemsForSelection(IEnumerable<SelectionBlueprint<ISkinnableDrawable>> selection)
         {

osu.Game/Skinning/ISkinnableTarget.cs

@@ -37,8 +37,15 @@ public interface ISkinnableTarget : IDrawable
         void Reload();
 
         /// <summary>
-        /// Add the provided item to this target.
+        /// Add a new skinnable component to this target.
         /// </summary>
+        /// <param name="drawable">The component to add.</param>
         void Add(ISkinnableDrawable drawable);
+
+        /// <summary>
+        /// Remove an existing skinnable component to this target.
+        /// </summary>
+        /// <param name="component">The component to add.</param>
+        public void Remove(ISkinnableDrawable component);
     }
 }

osu.Game/Skinning/SkinnableTargetContainer.cs

@@ -43,10 +43,7 @@ public void Reload()
             }
         }
 
-        /// <summary>
-        /// Add a new skinnable component to this target.
-        /// </summary>
-        /// <param name="component">The component to add.</param>
+        /// <inheritdoc cref="ISkinnableTarget"/>
         /// <exception cref="NotSupportedException">Thrown when attempting to add an element to a target which is not supported by the current skin.</exception>
         /// <exception cref="ArgumentException">Thrown if the provided instance is not a <see cref="Drawable"/>.</exception>
         public void Add(ISkinnableDrawable component)
@@ -61,6 +58,21 @@ public void Add(ISkinnableDrawable component)
             components.Add(component);
         }
 
+        /// <inheritdoc cref="ISkinnableTarget"/>
+        /// <exception cref="NotSupportedException">Thrown when attempting to add an element to a target which is not supported by the current skin.</exception>
+        /// <exception cref="ArgumentException">Thrown if the provided instance is not a <see cref="Drawable"/>.</exception>
+        public void Remove(ISkinnableDrawable component)
+        {
+            if (content == null)
+                throw new NotSupportedException("Attempting to add a new component to a target container which is not supported by the current skin.");
+
+            if (!(component is Drawable drawable))
+                throw new ArgumentException($"Provided argument must be of type {nameof(Drawable)}.", nameof(drawable));
+
+            content.Remove(drawable);
+            components.Remove(component);
+        }
+
         protected override void SkinChanged(ISkinSource skin, bool allowFallback)
         {
             base.SkinChanged(skin, allowFallback);