Fix issue #297 : Our replacement for ConfigNode.ReadObject() now always return true to align with the stock API behavior, preventing issues for mods (uselessly) checking the ConfigNode.LoadObjectFromConfig() return value. Also changed the verbosity level from error to warning when loading objects that don't have any persistent fields, as this is a valid (albeit useless and wasteful) pattern.

Author: gotmachine

KSPCommunityFixes/Library/Extensions.cs

@@ -17,6 +17,14 @@ public static string AssemblyQualifiedName(this object obj)
             return $"{type.Assembly.GetName().Name}:{type.Name}";
         }
 
+        /// <summary>
+        /// Get an assembly qualified type name in the "assemblyName:typeName" format
+        /// </summary>
+        public static string AssemblyQualifiedName(this Type type)
+        {
+            return $"{type.Assembly.GetName().Name}:{type.Name}";
+        }
+
         public static bool IsPAWOpen(this Part part)
         {
             return part.PartActionWindow.IsNotNullOrDestroyed() && part.PartActionWindow.isActiveAndEnabled;

KSPCommunityFixes/Modding/PersistentIConfigNode.cs

@@ -1,15 +1,14 @@
 // - Add support for IConfigNode serialization
 // - Add support for Guid serialization
 // - Rewrite Object reading and writing for perf, clarity, and reuse
-//#define DEBUG_PERSISTENTICONFIGNODE
-using HarmonyLib;
+// #define DEBUG_PERSISTENTICONFIGNODE
+
 using System;
 using System.Collections;
 using System.Collections.Generic;
 using System.Reflection;
 using UnityEngine;
 using static ConfigNode;
-using Random = System.Random;
 
 namespace KSPCommunityFixes.Modding
 {
@@ -30,9 +29,9 @@ public LoadState(bool r, ReadLinkList l)
 
         protected override void ApplyPatches()
         {
-            AddPatch(PatchType.Prefix, typeof(ConfigNode), nameof(ConfigNode.ReadObject), new Type[] { typeof(object), typeof(ConfigNode) });
+            AddPatch(PatchType.Override, typeof(ConfigNode), nameof(ReadObject), new Type[] { typeof(object), typeof(ConfigNode) });
 
-            AddPatch(PatchType.Prefix, typeof(ConfigNode), nameof(ConfigNode.WriteObject), new Type[] { typeof(object), typeof(ConfigNode), typeof(int) });
+            AddPatch(PatchType.Override, typeof(ConfigNode), nameof(WriteObject), new Type[] { typeof(object), typeof(ConfigNode), typeof(int) });
 
             AddPatch(PatchType.Prefix, typeof(ConfigNode), nameof(CreateConfigFromObject), new Type[] { typeof(object), typeof(int), typeof(ConfigNode) });
 
@@ -68,52 +67,21 @@ private static void ConfigNode_LoadObjectFromConfig_Postfix(LoadState __state)
             readLinks = __state.links;
         }
 
-        // used by TypeCache since we can't make the below method return a value.
-        public static bool WriteSuccess;
-
-        public static bool ConfigNode_WriteObject_Prefix(object obj, ConfigNode node, int pass)
+        private static void ConfigNode_WriteObject_Override(object obj, ConfigNode node, int pass)
         {
-            if (obj is IPersistenceSave persistenceSave)
-                persistenceSave.PersistenceSave();
-
-            Type type = obj.GetType();
-            var typeCache = TypeCache.GetOrCreate(type);
-            if (typeCache == null)
-            {
-                WriteSuccess = false;
-                return false;
-            }
-
-            WriteSuccess = typeCache.Write(obj, node, pass);
-
-            return false;
+            TypeCache.TryWriteObject(obj, node, pass);
         }
 
 
-        public static bool ConfigNode_ReadObject_Prefix(object obj, ConfigNode node, out bool __result)
+        private static bool ConfigNode_ReadObject_Override(object obj, ConfigNode node)
         {
-            Type type = obj.GetType();
-            var typeCache = TypeCache.GetOrCreate(type);
-            if (typeCache == null)
-            {
-                __result = false;
-                return false;
-            }
-
-            bool readResult = typeCache.Read(obj, node);
-
-            if (obj is IPersistenceLoad persistenceLoad)
-            {
-                iPersistentLoaders.Add(persistenceLoad);
-            }
-
-            __result = readResult;
-            return false;
+            TypeCache.TryReadObject(obj, node);
+            return true;
         }
     }
 
     // This is an expanded version of System.TypeCode
-    public enum DataType : uint
+    internal enum DataType : uint
     {
         INVALID = 0,
         IConfigNode,
@@ -151,7 +119,7 @@ public enum DataType : uint
         LastValueType = ValueEnum,
     }
 
-    public class FieldData
+    internal class FieldData
     {
         private static readonly System.Globalization.CultureInfo _Invariant = System.Globalization.CultureInfo.InvariantCulture;
 
@@ -272,7 +240,7 @@ public bool Read(ConfigNode node, object host, int attribIndex)
                 }
                 else
                 {
-                    PersistentIConfigNode.ConfigNode_ReadObject_Prefix(existing, node, out bool success);
+                    bool success = TypeCache.TryReadObject(existing, node);
                     if (removeAfterUse)
                         node.name = string.Empty;
                     return success;
@@ -413,10 +381,10 @@ public bool Write(object value, ConfigNode node, int attribIndex, int pass)
 
                 case DataType.Object:
                 case DataType.Component:
-                    PersistentIConfigNode.ConfigNode_WriteObject_Prefix(value, subNode, pass);
+                    bool success = TypeCache.TryWriteObject(value, subNode, pass);
                     if (subNode.HasData)
                         node._nodes.nodes.Add(subNode);
-                    return PersistentIConfigNode.WriteSuccess;
+                    return success;
             }
             return false;
         }
@@ -446,7 +414,7 @@ public static object ReadObject(ConfigNode node, object existing, DataType dataT
                         existing = TypeCache.GetNewInstance(fieldType);
                     if (existing == null)
                         return null;
-                    PersistentIConfigNode.ConfigNode_ReadObject_Prefix(existing, node, out success);
+                    success = TypeCache.TryReadObject(existing, node);
                     return existing;
             }
             return null;
@@ -491,7 +459,7 @@ public static MonoBehaviour CreateComponent(object host, Type fieldType, Persist
 
             // We're not reading directly here because the class might have
             // other interfaces/links/etc.
-            PersistentIConfigNode.ConfigNode_ReadObject_Prefix(child, node, out success);
+            success = TypeCache.TryReadObject(child, node);
             return child;
         }
 
@@ -825,7 +793,7 @@ private bool FindSetArrayType(Type seqType)
         }
     }
 
-    public class TypeCache
+    internal class TypeCache
     {
         private static readonly Dictionary<Type, TypeCache> cache = new Dictionary<Type, TypeCache>();
         //private static readonly Dictionary<Type, ConstructorInfo> _typeToConstrutor = new Dictionary<Type, ConstructorInfo>();
@@ -868,6 +836,34 @@ private int SeenFieldIndex(string fieldName)
             return numSeen;
         }
 
+        public static bool TryWriteObject(object obj, ConfigNode node, int pass)
+        {
+            if (obj is IPersistenceSave persistenceSave)
+                persistenceSave.PersistenceSave();
+
+            Type type = obj.GetType();
+            TypeCache typeCache = GetOrCreate(type);
+            if (typeCache == null)
+                return false;
+
+            return typeCache.Write(obj, node, pass);
+        }
+
+        public static bool TryReadObject(object obj, ConfigNode node)
+        {
+            Type type = obj.GetType();
+            TypeCache typeCache = GetOrCreate(type);
+            if (typeCache == null)
+                return false;
+
+            bool readResult = typeCache.Read(obj, node);
+
+            if (obj is IPersistenceLoad persistenceLoad)
+                iPersistentLoaders.Add(persistenceLoad);
+
+            return readResult;
+        }
+
         public bool Read(object host, ConfigNode node)
         {
             if (_isPersistentLinkable)
@@ -952,7 +948,7 @@ public static TypeCache CreateAndAdd(Type t)
             var tc = new TypeCache(t);
             if (tc._fields.Count == 0)
             {
-                Debug.LogError($"[KSPCommunityFixes]: No Persistent fields on object of type {t.Name} that is referenced in persistent field, adding as null to TypeCache.");
+                Debug.LogWarning($"[KSPCommunityFixes]: No Persistent fields on object of type {t.AssemblyQualifiedName()}, adding as null to TypeCache. That object likely shouldn't implement the IConfigNode interface.");
                 tc = null;
             }