Merge pull request #15898 from peppy/skin-export-instntiation-info

Serialise and deserialise `SkinInfo.InstantiationInfo` to allow for more correct imports

osu.Game.Tests/Skins/IO/ImportSkinTest.cs

@@ -164,6 +164,74 @@ namespace osu.Game.Tests.Skins.IO
             assertCorrectMetadata(import2, "name 1 [my custom skin 2]", "author 1", osu);
         });
 
+        [Test]
+        public Task TestExportThenImportDefaultSkin() => runSkinTest(osu =>
+        {
+            var skinManager = osu.Dependencies.Get<SkinManager>();
+
+            skinManager.EnsureMutableSkin();
+
+            MemoryStream exportStream = new MemoryStream();
+
+            Guid originalSkinId = skinManager.CurrentSkinInfo.Value.ID;
+
+            skinManager.CurrentSkinInfo.Value.PerformRead(s =>
+            {
+                Assert.IsFalse(s.Protected);
+                Assert.AreEqual(typeof(DefaultSkin), s.CreateInstance(skinManager).GetType());
+
+                new LegacySkinExporter(osu.Dependencies.Get<Storage>()).ExportModelTo(s, exportStream);
+
+                Assert.Greater(exportStream.Length, 0);
+            });
+
+            var imported = skinManager.Import(new ImportTask(exportStream, "exported.osk"));
+
+            imported.Result.PerformRead(s =>
+            {
+                Assert.IsFalse(s.Protected);
+                Assert.AreNotEqual(originalSkinId, s.ID);
+                Assert.AreEqual(typeof(DefaultSkin), s.CreateInstance(skinManager).GetType());
+            });
+
+            return Task.CompletedTask;
+        });
+
+        [Test]
+        public Task TestExportThenImportClassicSkin() => runSkinTest(osu =>
+        {
+            var skinManager = osu.Dependencies.Get<SkinManager>();
+
+            skinManager.CurrentSkinInfo.Value = skinManager.DefaultLegacySkin.SkinInfo;
+
+            skinManager.EnsureMutableSkin();
+
+            MemoryStream exportStream = new MemoryStream();
+
+            Guid originalSkinId = skinManager.CurrentSkinInfo.Value.ID;
+
+            skinManager.CurrentSkinInfo.Value.PerformRead(s =>
+            {
+                Assert.IsFalse(s.Protected);
+                Assert.AreEqual(typeof(DefaultLegacySkin), s.CreateInstance(skinManager).GetType());
+
+                new LegacySkinExporter(osu.Dependencies.Get<Storage>()).ExportModelTo(s, exportStream);
+
+                Assert.Greater(exportStream.Length, 0);
+            });
+
+            var imported = skinManager.Import(new ImportTask(exportStream, "exported.osk"));
+
+            imported.Result.PerformRead(s =>
+            {
+                Assert.IsFalse(s.Protected);
+                Assert.AreNotEqual(originalSkinId, s.ID);
+                Assert.AreEqual(typeof(DefaultLegacySkin), s.CreateInstance(skinManager).GetType());
+            });
+
+            return Task.CompletedTask;
+        });
+
         #endregion
 
         private void assertCorrectMetadata(ILive<SkinInfo> import1, string name, string creator, OsuGameBase osu)

osu.Game/Database/IModelFileManager.cs

@@ -25,7 +25,7 @@ namespace osu.Game.Database
         void DeleteFile(TModel model, TFileModel file);
 
         /// <summary>
-        /// Add a new file.
+        /// Add a new file. If the file already exists, it is overwritten.
         /// </summary>
         /// <param name="model">The item to operate on.</param>
         /// <param name="contents">The new file contents.</param>

osu.Game/Skinning/SkinInfo.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using Newtonsoft.Json;
 using osu.Framework.Extensions.ObjectExtensions;
 using osu.Framework.Testing;
 using osu.Game.Database;
@@ -16,6 +17,7 @@ namespace osu.Game.Skinning
 {
     [ExcludeFromDynamicCompile]
     [MapTo("Skin")]
+    [JsonObject(MemberSerialization.OptIn)]
     public class SkinInfo : RealmObject, IHasRealmFiles, IEquatable<SkinInfo>, IHasGuidPrimaryKey, ISoftDelete, IHasNamedFiles
     {
         internal static readonly Guid DEFAULT_SKIN = new Guid("2991CFD8-2140-469A-BCB9-2EC23FBCE4AD");
@@ -23,18 +25,22 @@ namespace osu.Game.Skinning
         internal static readonly Guid RANDOM_SKIN = new Guid("D39DFEFB-477C-4372-B1EA-2BCEA5FB8908");
 
         [PrimaryKey]
+        [JsonProperty]
         public Guid ID { get; set; } = Guid.NewGuid();
 
+        [JsonProperty]
         public string Name { get; set; } = string.Empty;
 
+        [JsonProperty]
         public string Creator { get; set; } = string.Empty;
 
+        [JsonProperty]
+        public string InstantiationInfo { get; set; } = string.Empty;
+
         public string Hash { get; set; } = string.Empty;
 
         public bool Protected { get; set; }
 
-        public string InstantiationInfo { get; set; } = string.Empty;
-
         public virtual Skin CreateInstance(IStorageResourceProvider resources)
         {
             var type = string.IsNullOrEmpty(InstantiationInfo)

osu.Game/Skinning/SkinManager.cs

@@ -156,7 +156,13 @@ namespace osu.Game.Skinning
                 }).Result;
 
                 if (result != null)
+                {
+                    // save once to ensure the required json content is populated.
+                    // currently this only happens on save.
+                    result.PerformRead(skin => Save(skin.CreateInstance(this)));
+
                     CurrentSkinInfo.Value = result;
+                }
             });
         }
 

osu.Game/Skinning/SkinModelManager.cs

@@ -24,6 +24,8 @@ namespace osu.Game.Skinning
 {
     public class SkinModelManager : RealmArchiveModelManager<SkinInfo>
     {
+        private const string skin_info_file = "skininfo.json";
+
         private readonly IStorageResourceProvider skinResources;
 
         public SkinModelManager(Storage storage, RealmContextFactory contextFactory, GameHost host, IStorageResourceProvider skinResources)
@@ -49,8 +51,36 @@ namespace osu.Game.Skinning
 
         protected override Task Populate(SkinInfo model, ArchiveReader? archive, Realm realm, CancellationToken cancellationToken = default)
         {
-            if (string.IsNullOrEmpty(model.InstantiationInfo))
-                model.InstantiationInfo = createInstance(model).GetType().GetInvariantInstantiationInfo();
+            var skinInfoFile = model.Files.SingleOrDefault(f => f.Filename == skin_info_file);
+
+            if (skinInfoFile != null)
+            {
+                try
+                {
+                    using (var existingStream = Files.Storage.GetStream(skinInfoFile.File.GetStoragePath()))
+                    using (var reader = new StreamReader(existingStream))
+                    {
+                        var deserialisedSkinInfo = JsonConvert.DeserializeObject<SkinInfo>(reader.ReadToEnd());
+
+                        if (deserialisedSkinInfo != null)
+                        {
+                            // for now we only care about the instantiation info.
+                            // eventually we probably want to transfer everything across.
+                            model.InstantiationInfo = deserialisedSkinInfo.InstantiationInfo;
+                        }
+                    }
+                }
+                catch (Exception e)
+                {
+                    LogForModel(model, $"Error during {skin_info_file} parsing, falling back to default", e);
+
+                    // Not sure if we should still run the import in the case of failure here, but let's do so for now.
+                    model.InstantiationInfo = string.Empty;
+                }
+            }
+
+            // Always rewrite instantiation info (even after parsing in from the skin json) for sanity.
+            model.InstantiationInfo = createInstance(model).GetType().GetInvariantInstantiationInfo();
 
             checkSkinIniMetadata(model, realm);
 
@@ -128,7 +158,7 @@ namespace osu.Game.Skinning
                             sw.WriteLine(line);
                     }
 
-                    ReplaceFile(item, existingFile, stream, realm);
+                    ReplaceFile(existingFile, stream, realm);
 
                     // can be removed 20220502.
                     if (!ensureIniWasUpdated(item))
@@ -203,6 +233,15 @@ namespace osu.Game.Skinning
         {
             skin.SkinInfo.PerformWrite(s =>
             {
+                // Serialise out the SkinInfo itself.
+                string skinInfoJson = JsonConvert.SerializeObject(s, new JsonSerializerSettings { Formatting = Formatting.Indented });
+
+                using (var streamContent = new MemoryStream(Encoding.UTF8.GetBytes(skinInfoJson)))
+                {
+                    AddFile(s, streamContent, skin_info_file, s.Realm);
+                }
+
+                // Then serialise each of the drawable component groups into respective files.
                 foreach (var drawableInfo in skin.DrawableComponentInfo)
                 {
                     string json = JsonConvert.SerializeObject(drawableInfo.Value, new JsonSerializerSettings { Formatting = Formatting.Indented });
@@ -214,7 +253,7 @@ namespace osu.Game.Skinning
                         var oldFile = s.Files.FirstOrDefault(f => f.Filename == filename);
 
                         if (oldFile != null)
-                            ReplaceFile(s, oldFile, streamContent, s.Realm);
+                            ReplaceFile(oldFile, streamContent, s.Realm);
                         else
                             AddFile(s, streamContent, filename, s.Realm);
                     }

osu.Game/Stores/RealmArchiveModelManager.cs

@@ -52,10 +52,10 @@ namespace osu.Game.Stores
             item.Realm.Write(() => DeleteFile(item, file, item.Realm));
 
         public void ReplaceFile(TModel item, RealmNamedFileUsage file, Stream contents)
-            => item.Realm.Write(() => ReplaceFile(item, file, contents, item.Realm));
+            => item.Realm.Write(() => ReplaceFile(file, contents, item.Realm));
 
-        public void AddFile(TModel item, Stream stream, string filename)
-            => item.Realm.Write(() => AddFile(item, stream, filename, item.Realm));
+        public void AddFile(TModel item, Stream contents, string filename)
+            => item.Realm.Write(() => AddFile(item, contents, filename, item.Realm));
 
         /// <summary>
         /// Delete a file from within an ongoing realm transaction.
@@ -68,17 +68,25 @@ namespace osu.Game.Stores
         /// <summary>
         /// Replace a file from within an ongoing realm transaction.
         /// </summary>
-        protected void ReplaceFile(TModel model, RealmNamedFileUsage file, Stream contents, Realm realm)
+        protected void ReplaceFile(RealmNamedFileUsage file, Stream contents, Realm realm)
         {
             file.File = realmFileStore.Add(contents, realm);
         }
 
         /// <summary>
-        /// Add a file from within an ongoing realm transaction.
+        /// Add a file from within an ongoing realm transaction. If the file already exists, it is overwritten.
         /// </summary>
-        protected void AddFile(TModel item, Stream stream, string filename, Realm realm)
+        protected void AddFile(TModel item, Stream contents, string filename, Realm realm)
         {
-            var file = realmFileStore.Add(stream, realm);
+            var existing = item.Files.FirstOrDefault(f => string.Equals(f.Filename, filename, StringComparison.OrdinalIgnoreCase));
+
+            if (existing != null)
+            {
+                ReplaceFile(existing, contents, realm);
+                return;
+            }
+
+            var file = realmFileStore.Add(contents, realm);
             var namedUsage = new RealmNamedFileUsage(file, filename);
 
             item.Files.Add(namedUsage);