fix: fixing ReadNetworkBehaviour when NI is not found

If Ni is destroyed it will be null even if netID was not 0. This would have caused component Index to not be read which will then break the next read.

this now read component index if netid is not 0, even if NI is null

Assets/Mirage/Runtime/Serialization/MirageTypesExtensions.cs

@@ -48,27 +48,38 @@ public static NetworkIdentity ReadNetworkIdentity(this NetworkReader reader)
             if (netId == 0)
                 return null;
 
-            if (reader.ObjectLocator != null)
+            return FindNetworkIdentity(reader.ObjectLocator, netId);
+        }
+
+        private static NetworkIdentity FindNetworkIdentity(IObjectLocator objectLocator, uint netId)
+        {
+            if (objectLocator == null)
             {
-                // if not found return c# null
-                return reader.ObjectLocator.TryGetIdentity(netId, out NetworkIdentity identity)
-                    ? identity
-                    : null;
+                if (logger.WarnEnabled()) logger.LogWarning($"Could not find NetworkIdentity because ObjectLocator is null");
+                return null;
             }
 
-            if (logger.WarnEnabled()) logger.LogFormat(LogType.Warning, "ReadNetworkIdentity netId:{0} not found in spawned", netId);
-            return null;
+            // if not found return c# null
+            return objectLocator.TryGetIdentity(netId, out NetworkIdentity identity)
+                ? identity
+                : null;
         }
 
         public static NetworkBehaviour ReadNetworkBehaviour(this NetworkReader reader)
         {
-            NetworkIdentity identity = reader.ReadNetworkIdentity();
-            if (identity == null)
-            {
+            // we can't use ReadNetworkIdentity here, because we need to know if netid was 0 or not
+            // if it is not 0 we need to read component index even if NI is null, or it'll fail to deserilize next part
+            uint netId = reader.ReadPackedUInt32();
+            if (netId == 0)
                 return null;
-            }
 
+            // always read index if netid is not 0
             byte componentIndex = reader.ReadByte();
+
+            NetworkIdentity identity = FindNetworkIdentity(reader.ObjectLocator, netId);
+            if (identity is null)
+                return null;
+
             return identity.NetworkBehaviours[componentIndex];
         }
 

Assets/Tests/Runtime/Serialization/NetworkWriterTest.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using Mirage.Serialization;
+using NSubstitute;
 using NUnit.Framework;
 using UnityEngine;
 
@@ -12,6 +13,12 @@ public class NetworkWriterTest
         private readonly NetworkWriter writer = new NetworkWriter(1300);
         private readonly NetworkReader reader = new NetworkReader();
 
+        [SetUp]
+        public void Setup()
+        {
+            // set new ObjectLocator
+            reader.ObjectLocator = Substitute.For<IObjectLocator>();
+        }
         [TearDown]
         public void TearDown()
         {
@@ -975,18 +982,76 @@ public void TestNullList()
         }
 
         [Test]
-        public void TestWriteNetworkBehavior()
+        public void WriteNetworkBehaviorNull()
         {
             writer.WriteNetworkBehaviour(null);
 
             reader.Reset(writer.ToArraySegment());
-            NetworkBehaviour behavior = reader.ReadNetworkBehaviour();
+            NetworkBehaviour behaviour = reader.ReadNetworkBehaviour();
 
-            Assert.That(behavior, Is.Null);
+            Assert.That(behaviour, Is.Null);
 
             Assert.That(writer.ByteLength, Is.EqualTo(reader.BytePosition));
         }
 
+        [Test]
+        public void WriteNetworkBehaviorNotNull()
+        {
+            var go = new GameObject("TestWriteNetworkBehaviorNotNull", typeof(NetworkIdentity), typeof(MockComponent));
+            MockComponent mock = go.GetComponent<MockComponent>();
+            // init lazy props
+            _ = mock.Identity.NetworkBehaviours;
+            mock.Identity.NetId = 1;
+            // returns found id
+            reader.ObjectLocator.TryGetIdentity(1, out NetworkIdentity _).Returns(x => { x[1] = mock.Identity; return true; });
+
+            // try/fianlly so go is always destroyed
+            try
+            {
+                writer.WriteNetworkBehaviour(mock);
+
+                reader.Reset(writer.ToArraySegment());
+                MockComponent behaviour = reader.ReadNetworkBehaviour<MockComponent>();
+
+                Assert.That(behaviour == mock);
+                Assert.That(writer.ByteLength, Is.EqualTo(reader.BytePosition));
+            }
+            finally
+            {
+                GameObject.Destroy(go);
+            }
+        }
+
+        [Test]
+        public void WriteNetworkBehaviorDestroyed()
+        {
+            // setup
+            var go = new GameObject("TestWriteNetworkBehaviorNotNull", typeof(NetworkIdentity), typeof(MockComponent));
+            MockComponent mock = go.GetComponent<MockComponent>();
+            // init lazy props
+            _ = mock.Identity.NetworkBehaviours;
+            mock.Identity.NetId = 1;
+            // return not found
+            reader.ObjectLocator.TryGetIdentity(1, out NetworkIdentity _).Returns(x => { x[1] = null; return false; });
+
+            // try/fianlly so go is always destroyed
+            try
+            {
+                writer.WriteNetworkBehaviour(mock);
+
+                reader.Reset(writer.ToArraySegment());
+                MockComponent behaviour = reader.ReadNetworkBehaviour<MockComponent>();
+
+                Assert.That(behaviour, Is.Null);
+                // make sure read same as written (including compIndex for non-0 netid
+                Assert.That(writer.ByteLength, Is.EqualTo(reader.BytePosition));
+            }
+            finally
+            {
+                GameObject.Destroy(go);
+            }
+        }
+
         [Test]
         public void TestWriteGameObject()
         {