Ensure default skins are copied before modifying

osu.Game/Skinning/Editor/SkinEditor.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Linq;
 using osu.Framework.Allocation;
+using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Input.Events;
@@ -30,6 +31,8 @@ namespace osu.Game.Skinning.Editor
         [Resolved]
         private SkinManager skins { get; set; }
 
+        private Bindable<Skin> currentSkin;
+
         public SkinEditor(Drawable targetScreen)
         {
             this.targetScreen = targetScreen;
@@ -119,23 +122,25 @@ namespace osu.Game.Skinning.Editor
         protected override void LoadComplete()
         {
             base.LoadComplete();
+
             Show();
+
+            // as long as the skin editor is loaded, let's make sure we can modify the current skin.
+            currentSkin = skins.CurrentSkin.GetBoundCopy();
+
+            // schedule ensures this only happens when the skin editor is visible.
+            // also avoid some weird endless recursion / bindable feedback loop (something to do with tracking skins across three different bindable types).
+            // probably something which will be factored out in a future database refactor so not too concerning for now.
+            currentSkin.BindValueChanged(skin => Scheduler.AddOnce(skins.EnsureMutableSkin), true);
         }
 
         private void revert()
         {
-            var currentSkin = skins.CurrentSkin.Value;
-
-            var legacySkin = currentSkin as LegacySkin;
-
-            if (legacySkin == null)
-                return;
-
             SkinnableElementTargetContainer[] targetContainers = targetScreen.ChildrenOfType<SkinnableElementTargetContainer>().ToArray();
 
             foreach (var t in targetContainers)
             {
-                legacySkin.ResetDrawableTarget(t);
+                currentSkin.Value.ResetDrawableTarget(t);
 
                 // add back default components
                 getTarget(t.Target).Reload();
@@ -144,17 +149,10 @@ namespace osu.Game.Skinning.Editor
 
         private void save()
         {
-            var currentSkin = skins.CurrentSkin.Value;
-
-            var legacySkin = currentSkin as LegacySkin;
-
-            if (legacySkin == null)
-                return;
-
             SkinnableElementTargetContainer[] targetContainers = targetScreen.ChildrenOfType<SkinnableElementTargetContainer>().ToArray();
 
             foreach (var t in targetContainers)
-                legacySkin.UpdateDrawableTarget(t);
+                currentSkin.Value.UpdateDrawableTarget(t);
 
             skins.Save(skins.CurrentSkin.Value);
         }

osu.Game/Skinning/SkinManager.cs

@@ -153,22 +153,30 @@ namespace osu.Game.Skinning
         /// <param name="skinInfo">The skin to lookup.</param>
         /// <returns>A <see cref="Skin"/> instance correlating to the provided <see cref="SkinInfo"/>.</returns>
         public Skin GetSkin(SkinInfo skinInfo) => skinInfo.CreateInstance(legacyDefaultResources, this);
+
+        /// <summary>
+        /// Ensure that the current skin is in a state it can accept user modifications.
+        /// This will create a copy of any internal skin and being tracking in the database if not already.
+        /// </summary>
+        public void EnsureMutableSkin()
         {
-            if (skinInfo == SkinInfo.Default)
+            if (CurrentSkinInfo.Value.ID >= 1) return;
-                return new DefaultSkin();
 
-            if (skinInfo == DefaultLegacySkin.Info)
+            var skin = CurrentSkin.Value;
-                return new DefaultLegacySkin(legacyDefaultResources, this);
 
-            return new LegacySkin(skinInfo, this);
+            // if the user is attempting to save one of the default skin implementations, create a copy first.
+            CurrentSkinInfo.Value = Import(new SkinInfo
+            {
+                Name = skin.SkinInfo.Name + " (modified)",
+                Creator = skin.SkinInfo.Creator,
+                InstantiationInfo = skin.SkinInfo.InstantiationInfo,
+            }).Result;
         }
 
         public void Save(Skin skin)
         {
-            // some skins don't support saving just yet.
+            if (skin.SkinInfo.ID <= 0)
-            // eventually we will want to create a copy of the skin to allow for customisation.
+                throw new InvalidOperationException($"Attempting to save a skin which is not yet tracked. Call {nameof(EnsureMutableSkin)} first.");
-            if (skin.SkinInfo.Files == null)
-                return;
 
             foreach (var drawableInfo in skin.DrawableComponentInfo)
             {