NetworkReader/Writer: all types of collections are now encoded via count-offsetting (null=0, []=1) for consistency and to prepare for VarUInt compression (this way we won't need zig zag VarInt) (#3869)

* better comments about count offsetting

* NetworkReader/Writer: all types of collections are now encoded via count-offsetting (null=0, []=1) for consistency and to prepare for VarUInt compression (this way we won't need zig zag VarInt)

---------

Co-authored-by: mischa <info@noobtuts.com>

Assets/Mirror/Core/NetworkReaderExtensions.cs

@@ -97,8 +97,9 @@ public static byte[] ReadBytes(this NetworkReader reader, int count)
         /// <exception cref="T:OverflowException">if count is invalid</exception>
         public static byte[] ReadBytesAndSize(this NetworkReader reader)
         {
-            // count = 0 means the array was null
+            // we offset count by '1' to easily support null without writing another byte.
-            // otherwise count -1 is the length of the array
+            // encoding null as '0' instead of '-1' also allows for better compression
+            // (ushort vs. short / varuint vs. varint) etc.
             uint count = reader.ReadUInt();
             // Use checked() to force it to throw OverflowException if data is invalid
             return count == 0 ? null : reader.ReadBytes(checked((int)(count - 1u)));
@@ -107,8 +108,9 @@ public static byte[] ReadBytesAndSize(this NetworkReader reader)
         /// <exception cref="T:OverflowException">if count is invalid</exception>
         public static ArraySegment<byte> ReadArraySegmentAndSize(this NetworkReader reader)
         {
-            // count = 0 means the array was null
+            // we offset count by '1' to easily support null without writing another byte.
-            // otherwise count - 1 is the length of the array
+            // encoding null as '0' instead of '-1' also allows for better compression
+            // (ushort vs. short / varuint vs. varint) etc.
             uint count = reader.ReadUInt();
             // Use checked() to force it to throw OverflowException if data is invalid
             return count == 0 ? default : reader.ReadBytesSegment(checked((int)(count - 1u)));
@@ -264,10 +266,12 @@ public static GameObject ReadGameObject(this NetworkReader reader)
         // note that Weaver/Readers/GenerateReader() handles this manually.
         public static List<T> ReadList<T>(this NetworkReader reader)
         {
-            int length = reader.ReadInt();
+            // we offset count by '1' to easily support null without writing another byte.
-
+            // encoding null as '0' instead of '-1' also allows for better compression
-            // 'null' is encoded as '-1'
+            // (ushort vs. short / varuint vs. varint) etc.
-            if (length < 0) return null;
+            uint length = reader.ReadUInt();
+            if (length == 0) return null;
+            length -= 1;
 
             // prevent allocation attacks with a reasonable limit.
             //   server shouldn't allocate too much on client devices.
@@ -278,7 +282,7 @@ public static List<T> ReadList<T>(this NetworkReader reader)
                 throw new EndOfStreamException($"NetworkReader attempted to allocate a List<{typeof(T)}> {length} elements, which is larger than the allowed limit of {NetworkReader.AllocationLimit}.");
             }
 
-            List<T> result = new List<T>(length);
+            List<T> result = new List<T>((checked((int)length)));
             for (int i = 0; i < length; i++)
             {
                 result.Add(reader.Read<T>());
@@ -294,9 +298,13 @@ public static List<T> ReadList<T>(this NetworkReader reader)
         /*
         public static HashSet<T> ReadHashSet<T>(this NetworkReader reader)
         {
-            int length = reader.ReadInt();
+            // we offset count by '1' to easily support null without writing another byte.
-            if (length < 0)
+            // encoding null as '0' instead of '-1' also allows for better compression
-                return null;
+            // (ushort vs. short / varuint vs. varint) etc.
+            uint length = reader.ReadUInt();
+            if (length == 0) return null;
+            length -= 1;
+
             HashSet<T> result = new HashSet<T>();
             for (int i = 0; i < length; i++)
             {
@@ -308,10 +316,12 @@ public static HashSet<T> ReadHashSet<T>(this NetworkReader reader)
 
         public static T[] ReadArray<T>(this NetworkReader reader)
         {
-            int length = reader.ReadInt();
+            // we offset count by '1' to easily support null without writing another byte.
-
+            // encoding null as '0' instead of '-1' also allows for better compression
-            // 'null' is encoded as '-1'
+            // (ushort vs. short / varuint vs. varint) etc.
-            if (length < 0) return null;
+            uint length = reader.ReadUInt();
+            if (length == 0) return null;
+            length -= 1;
 
             // prevent allocation attacks with a reasonable limit.
             //   server shouldn't allocate too much on client devices.

Assets/Mirror/Core/NetworkWriterExtensions.cs

@@ -51,10 +51,9 @@ public static class NetworkWriterExtensions
 
         public static void WriteString(this NetworkWriter writer, string value)
         {
-            // write 0 for null support, increment real size by 1
+            // we offset count by '1' to easily support null without writing another byte.
-            // (note: original HLAPI would write "" for null strings, but if a
+            // encoding null as '0' instead of '-1' also allows for better compression
-            //        string is null on the server then it should also be null
+            // (ushort vs. short / varuint vs. varint) etc.
-            //        on the client)
             if (value == null)
             {
                 writer.WriteUShort(0);
@@ -94,9 +93,10 @@ public static void WriteBytesAndSize(this NetworkWriter writer, byte[] buffer)
         // (like an inventory with different items etc.)
         public static void WriteBytesAndSize(this NetworkWriter writer, byte[] buffer, int offset, int count)
         {
-            // null is supported because [SyncVar]s might be structs with null byte[] arrays
+            // null is supported because [SyncVar]s might be structs with null byte[] arrays.
-            // write 0 for null array, increment normal size by 1 to save bandwidth
+            // we offset count by '1' to easily support null without writing another byte.
-            // (using size=-1 for null would limit max size to 32kb instead of 64kb)
+            // encoding null as '0' instead of '-1' also allows for better compression
+            // (ushort vs. short / varuint vs. varint) etc.
             if (buffer == null)
             {
                 writer.WriteUInt(0u);
@@ -115,9 +115,17 @@ public static void WriteArraySegmentAndSize(this NetworkWriter writer, ArraySegm
         // writes ArraySegment of any type, and size header
         public static void WriteArraySegment<T>(this NetworkWriter writer, ArraySegment<T> segment)
         {
-            int length = segment.Count;
+            // we offset count by '1' to easily support null without writing another byte.
-            writer.WriteInt(length);
+            // encoding null as '0' instead of '-1' also allows for better compression
-            for (int i = 0; i < length; i++)
+            // (ushort vs. short / varuint vs. varint) etc.
+            //
+            // ArraySegment technically can't be null, but users may call:
+            // - WriteArraySegment
+            // - ReadArray
+            // in which case ReadArray needs null support. both need to be compatible.
+            int count = segment.Count;
+            writer.WriteUInt(checked((uint)count) + 1u);
+            for (int i = 0; i < count; i++)
             {
                 writer.Write(segment.Array[segment.Offset + i]);
             }
@@ -315,10 +323,12 @@ public static void WriteGameObject(this NetworkWriter writer, GameObject value)
         // note that Weaver/Writers/GenerateWriter() handles this manually.
         public static void WriteList<T>(this NetworkWriter writer, List<T> list)
         {
-            // 'null' is encoded as '-1'
+            // we offset count by '1' to easily support null without writing another byte.
+            // encoding null as '0' instead of '-1' also allows for better compression
+            // (ushort vs. short / varuint vs. varint) etc.
             if (list is null)
             {
-                writer.WriteInt(-1);
+                writer.WriteUInt(0);
                 return;
             }
 
@@ -326,7 +336,7 @@ public static void WriteList<T>(this NetworkWriter writer, List<T> list)
             if (list.Count > NetworkReader.AllocationLimit)
                 throw new IndexOutOfRangeException($"NetworkWriter.WriteList - List<{typeof(T)}> too big: {list.Count} elements. Limit: {NetworkReader.AllocationLimit}");
 
-            writer.WriteInt(list.Count);
+            writer.WriteUInt(checked((uint)list.Count) + 1u);
             for (int i = 0; i < list.Count; i++)
                 writer.Write(list[i]);
         }
@@ -339,12 +349,15 @@ public static void WriteList<T>(this NetworkWriter writer, List<T> list)
         /*
         public static void WriteHashSet<T>(this NetworkWriter writer, HashSet<T> hashSet)
         {
+            // we offset count by '1' to easily support null without writing another byte.
+            // encoding null as '0' instead of '-1' also allows for better compression
+            // (ushort vs. short / varuint vs. varint) etc.
             if (hashSet is null)
             {
-                writer.WriteInt(-1);
+                writer.WriteUInt(0);
                 return;
             }
-            writer.WriteInt(hashSet.Count);
+            writer.WriteUInt(checked((uint)hashSet.Count) + 1u);
             foreach (T item in hashSet)
                 writer.Write(item);
         }
@@ -352,10 +365,12 @@ public static void WriteHashSet<T>(this NetworkWriter writer, HashSet<T> hashSet
 
         public static void WriteArray<T>(this NetworkWriter writer, T[] array)
         {
-            // 'null' is encoded as '-1'
+            // we offset count by '1' to easily support null without writing another byte.
+            // encoding null as '0' instead of '-1' also allows for better compression
+            // (ushort vs. short / varuint vs. varint) etc.
             if (array is null)
             {
-                writer.WriteInt(-1);
+                writer.WriteUInt(0);
                 return;
             }
 
@@ -363,7 +378,7 @@ public static void WriteArray<T>(this NetworkWriter writer, T[] array)
             if (array.Length > NetworkReader.AllocationLimit)
                 throw new IndexOutOfRangeException($"NetworkWriter.WriteArray - Array<{typeof(T)}> too big: {array.Length} elements. Limit: {NetworkReader.AllocationLimit}");
 
-            writer.WriteInt(array.Length);
+            writer.WriteUInt(checked((uint)array.Length) + 1u);
             for (int i = 0; i < array.Length; i++)
                 writer.Write(array[i]);
         }

Assets/Mirror/Tests/Editor/NetworkReaderWriter/NetworkWriterTest.cs

@@ -1404,7 +1404,8 @@ public void TestArrayThrowsIfLengthIsWrong(int badLength)
 
             void WriteBadArray()
             {
-                writer.WriteInt(badLength);
+                // Reader/Writer encode null as count=0 and [] as count=1 (+1 offset)
+                writer.WriteUInt((uint)(badLength+1));
                 int[] array = new int[testArraySize] { 1, 2, 3, 4 };
                 for (int i = 0; i < array.Length; i++)
                     writer.Write(array[i]);