Write new name to skin.ini when renaming skin via settings

[bdach]

Reported at https://osu.ppy.sh/comments/3681620, with appropriate levels of rage bait (DID ANYONE TEST THIS?!?!?!?!?!?!?!?!?!111!!)

Reasoning for this is that without this, users' skin names can be dropped after an external edit because they're never persisted anywhere outside of realm.

The only other choice I see is to stop re-populating skin metadata from the .ini upon completing an external edit, which is very doable but seems worse than this. Dunno.

[peppy]

This seeems to work, but I am curious whether it may make more sense to write the full metadata out before performing the external edit operation. Something like this (note: this doesn't work properly for some reason):

diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs
index 764f5fdfb6..eef8030121 100644
--- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs
+++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs
@@ -310,11 +310,11 @@ protected override void PopIn()
                 base.PopIn();
             }
 
-            private void rename()
+            private void rename() => skins.CurrentSkinInfo.Value.PerformWrite(skin =>
             {
-                skins.Rename(skins.CurrentSkinInfo.Value, textBox.Text);
+                skin.Name = textBox.Text;
                 PopOut();
-            }
+            });
         }
     }
 }
diff --git a/osu.Game/Overlays/SkinEditor/SkinEditor.cs b/osu.Game/Overlays/SkinEditor/SkinEditor.cs
index f4a1bb7562..076a0f2400 100644
--- a/osu.Game/Overlays/SkinEditor/SkinEditor.cs
+++ b/osu.Game/Overlays/SkinEditor/SkinEditor.cs
@@ -290,6 +290,8 @@ private async Task editExternally()
 
             var skin = currentSkin.Value.SkinInfo.PerformRead(s => s.Detach());
 
+            skins.UpdateMetadata(skins.CurrentSkinInfo.Value, skin.Name);
+
             externalEditOperation = await externalEditOverlay!.Begin(skin).ConfigureAwait(false);
         }
 
diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs
index 1be6f1bc4a..73224b5767 100644
--- a/osu.Game/Skinning/SkinManager.cs
+++ b/osu.Game/Skinning/SkinManager.cs
@@ -349,14 +349,8 @@ public void Delete([CanBeNull] Expression<Func<SkinInfo, bool>> filter = null, b
             });
         }
 
-        public void Rename(Live<SkinInfo> skin, string newName)
-        {
-            skin.PerformWrite(s =>
-            {
-                s.Name = newName;
-                skinImporter.UpdateSkinIniMetadata(s, s.Realm!);
-            });
-        }
+        public void UpdateMetadata(Live<SkinInfo> skin, string newName) =>
+            skin.PerformWrite(s => skinImporter.UpdateSkinIniMetadata(s, s.Realm!));
 
         public void SetSkinFromConfiguration(string guidString)
         {

I'm just wondering if there's going to be similar operations in the future which will also require this handling and it's best to have it in a central place. Though, I think your current implementation may work better with the "Export" button in settings as well.

---

Also, in testing I noticed this section gets written twice to the skin.ini when modifying a default skin... a bit weird.

2025-07-12.03.02.17.mp4

// The following content was automatically added by osu! in order to use metadata that more closely matches user expectations.
[General]
Name: osu! "classic" (2013) (modified)
Author: Unknown
Version: 2.7

// The following content was automatically added by osu! in order to use metadata that more closely matches user expectations.
[General]
Name: wang wang 2
Author: Unknown

[peppy]

I'm fine with what is in this diff, just raising a few discussion points.

[bdach]

I'm just wondering if there's going to be similar operations in the future which will also require this handling and it's best to have it in a central place. Though, I think your current implementation may work better with the "Export" button in settings as well.

The export thing is why I decided to go with this version to begin with. This way the user doesn't lose the skin rename on export & re-import.

Also, in testing I noticed this section gets written twice to the skin.ini when modifying a default skin... a bit weird.

The first time it gets written is when the skin becomes skinnable, e.g. on the "import" of the mutable skin when skin editor is opened. The second is when the skin is renamed.

I agree in principle that it's "weird" but without proper support for re-encoding skin.ini it's the best that I can do.