More serialization fixes (#4224)

Robust.Shared/Serialization/Manager/SerializationManager.Validation.cs

@@ -193,7 +193,7 @@ public sealed partial class SerializationManager
         }
 
         return new ErrorNode(node,
-            $"Failed to get node validator. Type: {typeof(T).Name}. Node type: {node.GetType().Name}. Node: {node}");
+            $"Failed to get node validator. Type: {typeof(T).Name}. Node type: {node.GetType().Name}. Yaml:\n{node}");
     }
 
     #endregion

Robust.Shared/Serialization/Manager/SerializationManager.Writing.cs

@@ -56,7 +56,6 @@ public sealed partial class SerializationManager
                 alwaysWrite,
                 contextParam).Compile();
         }, this);
-
     }
 
     private WriteGenericDelegate<T> GetOrCreateWriteGenericDelegate<T>(T value, bool notNullableOverride)
@@ -125,8 +124,8 @@ public sealed partial class SerializationManager
                 call = Expression.Call(
                     instanceParam,
                     nameof(WriteArray),
-                    Type.EmptyTypes,
-                    Expression.Convert(objParam, typeof(Array)),
+                    new []{ actualType.GetElementType()! },
+                    Expression.Convert(objParam, actualType),
                     alwaysWriteParam,
                     contextParam);
             }
@@ -193,7 +192,7 @@ public sealed partial class SerializationManager
         }
 
         var type = typeof(T);
-        if (type.IsAbstract || type.IsInterface)
+        if (!type.IsSealed) // abstract classes, virtual classes, and interfaces.
         {
             return (WriteGenericDelegate<T>)_writeGenericBaseDelegates.GetOrAdd((type, value!.GetType(), notNullableOverride),
                 static (tuple, manager) => ValueFactory(tuple.baseType, tuple.actualType, tuple.Item3, manager), this);
@@ -213,13 +212,13 @@ public sealed partial class SerializationManager
         return new ValueDataNode(obj.Serialize());
     }
 
-    private DataNode WriteArray(Array obj, bool alwaysWrite, ISerializationContext? context)
+    private DataNode WriteArray<TElement>(TElement[] obj, bool alwaysWrite, ISerializationContext? context)
     {
         var sequenceNode = new SequenceDataNode();
 
         foreach (var val in obj)
         {
-            var serializedVal = WriteValue(val.GetType(), val, alwaysWrite, context);
+            var serializedVal = WriteValue(val, alwaysWrite, context);
             sequenceNode.Add(serializedVal);
         }
 

Robust.Shared/Serialization/TypeSerializers/Implementations/Custom/FlagSerializer.cs

@@ -33,6 +33,17 @@ namespace Robust.Shared.Serialization.TypeSerializers.Implementations.Custom
             var sequenceNode = new SequenceDataNode();
             var flagType = serializationManager.GetFlagTypeFromTag(typeof(TTag));
 
+            // Special case for -1 to avoid InvalidOperationException errors.
+            if (value == -1)
+            {
+                var name = Enum.GetName(flagType, -1);
+                if (name != null)
+                {
+                    sequenceNode.Add(new ValueDataNode(name));
+                    return sequenceNode;
+                }
+            }
+
             // Assumption: a bitflag enum has a constructor for every bit value such that
             // that bit is set in some other constructor i.e. if a 1 appears somewhere in
             // the bits of one of the enum constructors, there is an enum constructor which

Robust.UnitTesting/Shared/Serialization/VirtualObjectArrayTest.cs

@@ -0,0 +1,121 @@
+using System.Collections.Generic;
+using NUnit.Framework;
+using Robust.Shared.Serialization.Manager.Attributes;
+using Robust.Shared.Serialization.Markdown.Mapping;
+using Robust.Shared.Serialization.Markdown.Sequence;
+
+namespace Robust.UnitTesting.Shared.Serialization;
+
+/// <summary>
+/// Tests that arrays and lists of virtual/abstract objects can be properly serialized and deserialized.
+/// </summary>
+public sealed class VirtualObjectArrayTest : SerializationTest
+{
+    [ImplicitDataDefinitionForInheritors]
+    private abstract class BaseTestDataDef { }
+
+    private sealed class SealedTestDataDef : BaseTestDataDef { }
+
+    [Virtual]
+    private class VirtualTestDataDef : BaseTestDataDef { }
+
+    private sealed class ChildTestDef : VirtualTestDataDef { }
+
+    [Test]
+    public void SerializeVirtualObjectArrayTest()
+    {
+        var sequence = new SequenceDataNode
+        {
+            new MappingDataNode {Tag = $"!type:SealedTestDataDef"},
+            new MappingDataNode {Tag = $"!type:VirtualTestDataDef"},
+            new MappingDataNode {Tag = $"!type:ChildTestDef"}
+        };
+
+        {
+            // Deserialize the above yaml
+            var arr = Serialization.Read<BaseTestDataDef[]>(sequence, notNullableOverride: true);
+
+            // Ensure that the !type: tags were properly parsed
+            Assert.That(arr[0], Is.TypeOf(typeof(SealedTestDataDef)));
+            Assert.That(arr[1], Is.TypeOf(typeof(VirtualTestDataDef)));
+            Assert.That(arr[2], Is.TypeOf(typeof(ChildTestDef)));
+
+            // Write the parsed object back to yaml
+            var newSquence = Serialization.WriteValue(arr, notNullableOverride: true);
+
+            // Check that the yaml doesn't differ in any way.
+            var diff = newSquence.Except(sequence);
+            Assert.IsNull(diff);
+
+            // And finally, double check that the serialized data can be re-deserialized (dataNode.Except isn't perfect).
+            arr = Serialization.Read<BaseTestDataDef[]>(newSquence, notNullableOverride: true);
+            Assert.That(arr[0], Is.TypeOf(typeof(SealedTestDataDef)));
+            Assert.That(arr[1], Is.TypeOf(typeof(VirtualTestDataDef)));
+            Assert.That(arr[2], Is.TypeOf(typeof(ChildTestDef)));
+        }
+
+
+        // Repeat the above, but using lists instead of arrays
+        {
+            var list = Serialization.Read<List<BaseTestDataDef>>(sequence, notNullableOverride: true);
+            Assert.That(list[0], Is.TypeOf(typeof(SealedTestDataDef)));
+            Assert.That(list[1], Is.TypeOf(typeof(VirtualTestDataDef)));
+            Assert.That(list[2], Is.TypeOf(typeof(ChildTestDef)));
+
+            var newSquence = Serialization.WriteValue(list, notNullableOverride: true);
+            var diff = newSquence.Except(sequence);
+            Assert.IsNull(diff);
+
+            list = Serialization.Read<List<BaseTestDataDef>>(sequence, notNullableOverride: true);
+            Assert.That(list[0], Is.TypeOf(typeof(SealedTestDataDef)));
+            Assert.That(list[1], Is.TypeOf(typeof(VirtualTestDataDef)));
+            Assert.That(list[2], Is.TypeOf(typeof(ChildTestDef)));
+        }
+
+        // remove the first entry -- leave only entries that inherit from VirtualTestDataDef
+        sequence.RemoveAt(0);
+
+        // When writing, this will skip the !type tag for the first entry
+        var expectedSequence = new SequenceDataNode
+        {
+            new MappingDataNode(),
+            new MappingDataNode {Tag = $"!type:ChildTestDef"}
+        };
+
+        {
+            var virtArr = Serialization.Read<VirtualTestDataDef[]>(sequence, notNullableOverride: true);
+            Assert.That(virtArr[0], Is.TypeOf(typeof(VirtualTestDataDef)));
+            Assert.That(virtArr[1], Is.TypeOf(typeof(ChildTestDef)));
+
+            // The old sequence will now differ as it should not write the redundant !type tag
+            var newSquence = Serialization.WriteValue(virtArr, notNullableOverride: true);
+            var diff = newSquence.Except(sequence);
+            Assert.NotNull(diff);
+
+            diff = newSquence.Except(expectedSequence);
+            Assert.IsNull(diff);
+
+            virtArr = Serialization.Read<VirtualTestDataDef[]>(newSquence, notNullableOverride: true);
+            Assert.That(virtArr[0], Is.TypeOf(typeof(VirtualTestDataDef)));
+            Assert.That(virtArr[1], Is.TypeOf(typeof(ChildTestDef)));
+        }
+
+        // And again, repeat for lists instead of arrays
+        {
+            var virtList = Serialization.Read<List<VirtualTestDataDef>>(sequence, notNullableOverride: true);
+            Assert.That(virtList[0], Is.TypeOf(typeof(VirtualTestDataDef)));
+            Assert.That(virtList[1], Is.TypeOf(typeof(ChildTestDef)));
+
+            var newSquence = Serialization.WriteValue(virtList, notNullableOverride: true);
+            var diff = newSquence.Except(sequence);
+            Assert.NotNull(diff);
+
+            diff = newSquence.Except(expectedSequence);
+            Assert.IsNull(diff);
+
+            virtList = Serialization.Read<List<VirtualTestDataDef>>(newSquence, notNullableOverride: true);
+            Assert.That(virtList[0], Is.TypeOf(typeof(VirtualTestDataDef)));
+            Assert.That(virtList[1], Is.TypeOf(typeof(ChildTestDef)));
+        }
+    }
+}