Merge pull request #1521 from andy840119/make-karaoke-skin-element-co…

…nvertor-inherit-generic-class

Make karaoke skin element convertor inherit generic convertor.

osu.Game.Rulesets.Karaoke.Tests/IO/Serialization/Converters/KaraokeSkinElementConvertorTest.cs

@@ -4,13 +4,23 @@
 using Newtonsoft.Json;
 using NUnit.Framework;
 using osu.Framework.Graphics;
+using osu.Framework.IO.Serialization;
 using osu.Game.Rulesets.Karaoke.IO.Serialization.Converters;
 using osu.Game.Rulesets.Karaoke.Skinning.Elements;
 
 namespace osu.Game.Rulesets.Karaoke.Tests.IO.Serialization.Converters
 {
     public class KaraokeSkinElementConvertorTest : BaseSingleConverterTest<KaraokeSkinElementConvertor>
     {
+        protected override JsonConverter[] CreateExtraConverts()
+            => new JsonConverter[]
+            {
+                new ColourConvertor(),
+                new Vector2Converter(),
+                new ShaderConvertor(),
+                new FontUsageConvertor()
+            };
+
         [Test]
         public void TestLyricConfigSerializer()
         {

osu.Game.Rulesets.Karaoke/IO/Serialization/Converters/GenericTypeConvertor.cs

@@ -10,7 +10,13 @@
 
 namespace osu.Game.Rulesets.Karaoke.IO.Serialization.Converters
 {
-    public abstract class GenericTypeConvertor<TType> : JsonConverter<TType>
+    public abstract class GenericTypeConvertor<TType> : GenericTypeConvertor<TType, string>
+    {
+        protected override string GetNameByType(MemberInfo type)
+            => type.Name;
+    }
+
+    public abstract class GenericTypeConvertor<TType, TTypeName> : JsonConverter<TType> where TTypeName : notnull
     {
         public sealed override TType ReadJson(JsonReader reader, Type objectType, TType? existingValue, bool hasExistingValue, JsonSerializer serializer)
         {
@@ -27,9 +33,13 @@ public sealed override TType ReadJson(JsonReader reader, Type objectType, TType?
 
             Type getTypeByProperties(IEnumerable<JProperty> properties)
             {
-                string? elementType = properties.FirstOrDefault(x => x.Name == "$type")?.Value.ToObject<string>();
+                var value = properties.FirstOrDefault(x => x.Name == "$type")?.Value;
+                if (value == null)
+                    throw new ArgumentNullException(nameof(value));
+
+                TTypeName? elementType = value.ToObject<TTypeName>();
                 if (elementType == null)
-                    throw new ArgumentNullException(nameof(elementType));
+                    throw new InvalidCastException(nameof(elementType));
 
                 return GetTypeByName(elementType);
             }
@@ -47,7 +57,7 @@ public sealed override void WriteJson(JsonWriter writer, TType? value, JsonSeria
             // follow: https://stackoverflow.com/a/59329703
             // not a good way but seems there's no better choice.
             serializer.Converters.Remove(this);
-            serializer.ContractResolver = new KaraokeSkinContractResolver();
+            serializer.ContractResolver = new WritablePropertiesOnlyResolver();
 
             var jObject = JObject.FromObject(value, serializer);
 
@@ -61,9 +71,8 @@ public sealed override void WriteJson(JsonWriter writer, TType? value, JsonSeria
 
         protected virtual void InteractWithJObject(JObject jObject, JsonWriter writer, TType value, JsonSerializer serializer) { }
 
-        protected abstract Type GetTypeByName(string name);
+        protected abstract Type GetTypeByName(TTypeName name);
 
-        protected virtual string GetNameByType(MemberInfo type)
-            => type.Name;
+        protected abstract TTypeName GetNameByType(MemberInfo type);
     }
 }

osu.Game.Rulesets.Karaoke/IO/Serialization/Converters/KaraokeSkinElementConvertor.cs

@@ -2,64 +2,20 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System;
-using System.Collections.Generic;
-using System.Linq;
-using Newtonsoft.Json;
-using Newtonsoft.Json.Linq;
-using osu.Framework.IO.Serialization;
-using osu.Game.IO.Serialization;
+using System.Reflection;
 using osu.Game.Rulesets.Karaoke.Skinning.Elements;
 
 namespace osu.Game.Rulesets.Karaoke.IO.Serialization.Converters
 {
-    public class KaraokeSkinElementConvertor : JsonConverter<IKaraokeSkinElement>
+    public class KaraokeSkinElementConvertor : GenericTypeConvertor<IKaraokeSkinElement, ElementType>
     {
-        // because we wants serializer that containers some common convertors except this one, so make a local one.
-        private readonly JsonSerializer localSerializer;
+        protected override Type GetTypeByName(ElementType name)
+            => GetObjectType(name);
 
-        public KaraokeSkinElementConvertor()
-        {
-            var settings = JsonSerializableExtensions.CreateGlobalSettings();
-            settings.ContractResolver = new KaraokeSkinContractResolver();
-            settings.Converters.Add(new ColourConvertor());
-            settings.Converters.Add(new Vector2Converter());
-            settings.Converters.Add(new ShaderConvertor());
-            settings.Converters.Add(new FontUsageConvertor());
-            localSerializer = JsonSerializer.Create(settings);
-        }
+        protected override ElementType GetNameByType(MemberInfo type)
+            => GetElementType(type);
 
-        public override IKaraokeSkinElement? ReadJson(JsonReader reader, Type objectType, IKaraokeSkinElement? existingValue, bool hasExistingValue, JsonSerializer serializer)
-        {
-            var jObject = JObject.Load(reader);
-            var properties = jObject.Children().OfType<JProperty>().ToArray();
-
-            var type = objectType != typeof(IKaraokeSkinElement) ? objectType : getTypeByProperties(properties);
-            var newReader = jObject.CreateReader();
-            return localSerializer.Deserialize(newReader, type) as IKaraokeSkinElement;
-
-            static Type getTypeByProperties(IEnumerable<JProperty> properties)
-            {
-                var elementType = properties.FirstOrDefault(x => x.Name == "$type")?.Value.ToObject<ElementType>();
-                if (elementType == null)
-                    throw new ArgumentNullException(nameof(elementType));
-
-                return GetObjectType(elementType.Value);
-            }
-        }
-
-        public override void WriteJson(JsonWriter writer, IKaraokeSkinElement? value, JsonSerializer serializer)
-        {
-            if (value == null)
-                throw new ArgumentNullException(nameof(value));
-
-            var jObject = JObject.FromObject(value, localSerializer);
-
-            // should get type from enum instead of class type because change class name might cause resource not found.
-            jObject.AddFirst(new JProperty("$type", GetElementType(value.GetType())));
-            jObject.WriteTo(writer);
-        }
-
-        public static ElementType GetElementType(Type elementType) =>
+        public static ElementType GetElementType(MemberInfo elementType) =>
             elementType switch
             {
                 var type when type == typeof(LyricConfig) => ElementType.LyricConfig,

osu.Game.Rulesets.Karaoke/IO/Serialization/KaraokeSkinContractResolver.cs → osu.Game.Rulesets.Karaoke/IO/Serialization/WritablePropertiesOnlyResolver.cs

@@ -14,7 +14,7 @@ namespace osu.Game.Rulesets.Karaoke.IO.Serialization
     /// <summary>
     /// This contract resolver is for save and load data from <see cref="KaraokeSkin"/>
     /// </summary>
-    public class KaraokeSkinContractResolver : SnakeCaseKeyContractResolver
+    public class WritablePropertiesOnlyResolver : SnakeCaseKeyContractResolver
     {
         // we only wants to save properties that only writable.
         protected override IList<JsonProperty> CreateProperties(Type type, MemberSerialization memberSerialization)