From 95705a1545d7d8288ebf92a48c8652d38c7096b0 Mon Sep 17 00:00:00 2001
From: James Frowen <jamesfrowendev@gmail.com>
Date: Fri, 10 Apr 2020 16:15:24 +0100
Subject: [PATCH] Moving error check to when cache is created (#1682)

* Moving error check to when cache is created

* updating tests for new code behaviour

* Update Assets/Mirror/Runtime/NetworkIdentity.cs

Co-authored-by: vis2k <info@noobtuts.com>
---
 Assets/Mirror/Runtime/NetworkIdentity.cs      | 28 ++++++++---
 .../Tests/Editor/NetworkIdentityTests.cs      | 48 +++++++++++++------
 2 files changed, 55 insertions(+), 21 deletions(-)

diff --git a/Assets/Mirror/Runtime/NetworkIdentity.cs b/Assets/Mirror/Runtime/NetworkIdentity.cs
index 5bb4e6cfa..6a0659fb3 100644
--- a/Assets/Mirror/Runtime/NetworkIdentity.cs
+++ b/Assets/Mirror/Runtime/NetworkIdentity.cs
@@ -145,7 +145,28 @@ internal set
         /// </summary>
         public static readonly Dictionary<uint, NetworkIdentity> spawned = new Dictionary<uint, NetworkIdentity>();
 
-        public NetworkBehaviour[] NetworkBehaviours => networkBehavioursCache = networkBehavioursCache ?? GetComponents<NetworkBehaviour>();
+        public NetworkBehaviour[] NetworkBehaviours
+        {
+            get
+            {
+                if (networkBehavioursCache == null)
+                {
+                    CreateNetworkBehavioursCache();
+                }
+                return networkBehavioursCache;
+            }
+        }
+
+        void CreateNetworkBehavioursCache()
+        {
+            networkBehavioursCache = GetComponents<NetworkBehaviour>();
+            if (NetworkBehaviours.Length > 64)
+            {
+                Debug.LogError($"Only 64 NetworkBehaviour components are allowed for NetworkIdentity: {name} because of the dirtyComponentMask", this);
+                // Log error once then resize array so that NetworkIdentity does not throw exceptions later
+                Array.Resize(ref networkBehavioursCache, 64);
+            }
+        }
 
         [SerializeField, HideInInspector] string m_AssetId;
 
@@ -763,11 +784,6 @@ internal void OnSerializeAllSafely(bool initialState, NetworkWriter ownerWriter,
             // clear 'written' variables
             ownerWritten = observersWritten = 0;
 
-            if (NetworkBehaviours.Length > 64)
-            {
-                Debug.LogError("Only 64 NetworkBehaviour components are allowed for NetworkIdentity: " + name + " because of the dirtyComponentMask");
-                return;
-            }
             ulong dirtyComponentsMask = GetDirtyMask(initialState);
 
             if (dirtyComponentsMask == 0L)
diff --git a/Assets/Mirror/Tests/Editor/NetworkIdentityTests.cs b/Assets/Mirror/Tests/Editor/NetworkIdentityTests.cs
index 352ef1efa..64ac4e752 100644
--- a/Assets/Mirror/Tests/Editor/NetworkIdentityTests.cs
+++ b/Assets/Mirror/Tests/Editor/NetworkIdentityTests.cs
@@ -1,5 +1,6 @@
-using System;
+using System;
 using System.Collections.Generic;
+using System.Text.RegularExpressions;
 using NSubstitute;
 using NUnit.Framework;
 using UnityEngine;
@@ -901,7 +902,34 @@ public void OnSerializeAndDeserializeAllSafely()
         // OnSerializeAllSafely supports at max 64 components, because our
         // dirty mask is ulong and can only handle so many bits.
         [Test]
-        public void OnSerializeAllSafelyShouldDetectTooManyComponents()
+        public void OnSerializeAllSafelyShouldNotLogErrorsForTooManyComponents()
+        {
+            // add 65 components
+            for (int i = 0; i < 65; ++i)
+            {
+                gameObject.AddComponent<SerializeTest1NetworkBehaviour>();
+            }
+            // ingore error from creating cache (has its own test)
+            LogAssert.ignoreFailingMessages = true;
+            _ = identity.NetworkBehaviours;
+            LogAssert.ignoreFailingMessages = false;
+
+
+            // try to serialize
+            NetworkWriter ownerWriter = new NetworkWriter();
+            NetworkWriter observersWriter = new NetworkWriter();
+
+            identity.OnSerializeAllSafely(true, ownerWriter, out int ownerWritten, observersWriter, out int observersWritten);
+
+            // Should still write with too mnay Components because NetworkBehavioursCache should handle the error
+            Assert.That(ownerWriter.Position, Is.GreaterThan(0));
+            Assert.That(observersWriter.Position, Is.GreaterThan(0));
+            Assert.That(ownerWritten, Is.GreaterThan(0));
+            Assert.That(observersWritten, Is.GreaterThan(0));
+        }
+
+        [Test]
+        public void CreatingNetworkBehavioursCacheShouldLogErrorForTooComponents()
         {
             // add 65 components
             for (int i = 0; i < 65; ++i)
@@ -909,19 +937,9 @@ public void OnSerializeAllSafelyShouldDetectTooManyComponents()
                 gameObject.AddComponent<SerializeTest1NetworkBehaviour>();
             }
 
-            // try to serialize
-            NetworkWriter ownerWriter = new NetworkWriter();
-            NetworkWriter observersWriter = new NetworkWriter();
-            // error log is expected because of too many components
-            LogAssert.ignoreFailingMessages = true;
-            identity.OnSerializeAllSafely(true, ownerWriter, out int ownerWritten, observersWriter, out int observersWritten);
-            LogAssert.ignoreFailingMessages = false;
-
-            // shouldn't have written anything because too many components
-            Assert.That(ownerWriter.Position, Is.EqualTo(0));
-            Assert.That(observersWriter.Position, Is.EqualTo(0));
-            Assert.That(ownerWritten, Is.EqualTo(0));
-            Assert.That(observersWritten, Is.EqualTo(0));
+            // call NetworkBehaviours property to create the cache
+            LogAssert.Expect(LogType.Error, new Regex("Only 64 NetworkBehaviour components are allowed for NetworkIdentity.+"));
+            _ = identity.NetworkBehaviours;
         }
 
         // OnDeserializeSafely should be able to detect and handle serialization