Ensure NetworkManagerOwner is always correctly set

Author: EmandM

com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs

@@ -805,6 +805,7 @@ internal void HandleConnectionApproval(ulong ownerClientId, NetworkManager.Conne
                     // Spawn the player NetworkObject locally
                     NetworkManager.SpawnManager.SpawnNetworkObjectLocally(
                         playerObject,
+                        NetworkManager,
                         NetworkManager.SpawnManager.GetNetworkObjectId(),
                         sceneObject: false,
                         playerObject: true,
@@ -954,6 +955,7 @@ internal void CreateAndSpawnPlayer(ulong ownerId)
                     var globalObjectIdHash = playerPrefab.GetComponent<NetworkObject>().GlobalObjectIdHash;
                     var networkObject = NetworkManager.SpawnManager.GetNetworkObjectToSpawn(globalObjectIdHash, ownerId, playerPrefab.transform.position, playerPrefab.transform.rotation);
                     networkObject.IsSceneObject = false;
+                    networkObject.NetworkManagerOwner = NetworkManager;
                     networkObject.SpawnAsPlayerObject(ownerId, networkObject.DestroyWithScene);
                 }
             }

com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs

@@ -546,9 +546,10 @@ internal bool IsBehaviourEditable()
             return HasAuthority;
         }
 
-        internal void SetNetworkObject(NetworkObject networkObject)
+        internal void SetNetworkObject(NetworkObject networkObject, ushort behaviourId)
         {
             m_NetworkObject = networkObject;
+            NetworkBehaviourId = behaviourId;
         }
 
         //  TODO: this needs an overhaul.  It's expensive, it's ja little naive in how it looks for networkObject in
@@ -613,11 +614,6 @@ public NetworkObject NetworkObject
         /// </summary>
         public ushort NetworkBehaviourId { get; internal set; }
 
-        /// <summary>
-        /// Internally caches the Id of this behaviour in a NetworkObject. Makes look-up faster
-        /// </summary>
-        internal ushort NetworkBehaviourIdCache = 0;
-
         /// <summary>
         /// Returns the NetworkBehaviour with a given BehaviourId for the current NetworkObject.
         /// </summary>
@@ -647,11 +643,6 @@ internal void UpdateNetworkProperties()
             NetworkObjectId = networkObject.NetworkObjectId;
             IsLocalPlayer = networkObject.IsLocalPlayer;
 
-            // This is "OK" because GetNetworkBehaviourOrderIndex uses the order of
-            // NetworkObject.ChildNetworkBehaviours which is set once when first
-            // accessed.
-            NetworkBehaviourId = networkObject.GetNetworkBehaviourOrderIndex(this);
-
             // Set ownership related properties
             IsOwnedByServer = networkObject.IsOwnedByServer;
             IsOwner = networkObject.IsOwner;
@@ -669,35 +660,6 @@ internal void UpdateNetworkProperties()
             }
         }
 
-        private void ResetAllFields()
-        {
-            m_NetworkObject = null;
-            m_NetworkManager = null;
-            RpcTarget = null;
-
-            // Set identification related properties
-            NetworkObjectId = default;
-            IsLocalPlayer = false;
-
-            // This is "OK" because GetNetworkBehaviourOrderIndex uses the order of
-            // NetworkObject.ChildNetworkBehaviours which is set once when first
-            // accessed.
-            NetworkBehaviourId = default;
-
-            // Set ownership related properties
-            IsOwnedByServer = false;
-            IsOwner = false;
-            OwnerClientId = default;
-
-            // Set NetworkManager dependent properties
-            IsHost = false;
-            IsClient = false;
-            IsServer = false;
-            IsSessionOwner = false;
-            HasAuthority = false;
-            ServerIsHost = false;
-        }
-
         /// <summary>
         /// Only for use in distributed authority mode.
         /// Invoked only on the authority instance when a <see cref="NetworkObject"/> is deferring its despawn on non-authoritative instances.
@@ -890,8 +852,6 @@ internal void InternalOnNetworkDespawn()
             {
                 NetworkVariableFields[i].Deinitialize();
             }
-
-            ResetAllFields();
         }
 
         /// <summary>
@@ -1128,7 +1088,6 @@ internal void NetworkVariableUpdate(ulong targetClientId, bool forceSend = false
             // Getting these ahead of time actually improves performance
             var networkManager = m_NetworkManager;
             var networkObject = m_NetworkObject;
-            var behaviourIndex = networkObject.GetNetworkBehaviourOrderIndex(this);
             var messageManager = networkManager.MessageManager;
             var connectionManager = networkManager.ConnectionManager;
 
@@ -1168,7 +1127,7 @@ internal void NetworkVariableUpdate(ulong targetClientId, bool forceSend = false
                 var message = new NetworkVariableDeltaMessage
                 {
                     NetworkObjectId = NetworkObjectId,
-                    NetworkBehaviourIndex = behaviourIndex,
+                    NetworkBehaviourIndex = NetworkBehaviourId,
                     NetworkBehaviour = this,
                     TargetClientId = targetClientId,
                     DeliveryMappedNetworkVariableIndex = m_DeliveryMappedNetworkVariableIndices[j],
@@ -1606,8 +1565,6 @@ public virtual void OnDestroy()
             {
                 networkVar.Dispose();
             }
-
-            ResetAllFields();
         }
     }
 }

com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs

@@ -1119,8 +1119,7 @@ private bool InternalHasAuthority()
         }
 
         /// <summary>
-        /// The NetworkManager that owns this NetworkObject.
-        /// This property controls where this NetworkObject belongs.
+        /// The NetworkManager that is responsible for this NetworkObject instance.
         /// This property is null by default currently, which means that the above NetworkManager getter will return the Singleton.
         /// In the future this is the path where alternative NetworkManagers should be injected for running multi NetworkManagers
         /// </summary>
@@ -1771,6 +1770,9 @@ internal void SpawnInternal(bool destroyWithScene, ulong ownerClientId, bool pla
         {
             if (NetworkManagerOwner == null)
             {
+#if TEST_NO_SINGLETON
+                Debug.LogError("NetworkObject has no owner client! setting as singleton owner");
+#endif
                 NetworkManagerOwner = NetworkManager.Singleton;
             }
             if (!NetworkManager.IsListening)
@@ -1825,7 +1827,7 @@ internal void SpawnInternal(bool destroyWithScene, ulong ownerClientId, bool pla
                 }
             }
 
-            NetworkManager.SpawnManager.SpawnNetworkObjectLocally(this, NetworkManager.SpawnManager.GetNetworkObjectId(), IsSceneObject.HasValue && IsSceneObject.Value, playerObject, ownerClientId, destroyWithScene);
+            NetworkManager.SpawnManager.SpawnNetworkObjectLocally(this, NetworkManagerOwner, NetworkManager.SpawnManager.GetNetworkObjectId(), IsSceneObject.HasValue && IsSceneObject.Value, playerObject, ownerClientId, destroyWithScene);
 
             if ((NetworkManager.DistributedAuthorityMode && NetworkManager.DAHost) || (!NetworkManager.DistributedAuthorityMode && NetworkManager.IsServer))
             {
@@ -2534,7 +2536,7 @@ internal static void CheckOrphanChildren()
 
         internal void InvokeBehaviourNetworkPreSpawn()
         {
-            var networkManager = NetworkManager;
+            var networkManager = NetworkManagerOwner;
             for (int i = 0; i < ChildNetworkBehaviours.Count; i++)
             {
                 if (ChildNetworkBehaviours[i].gameObject.activeInHierarchy)
@@ -2611,58 +2613,74 @@ internal void InvokeBehaviourNetworkDespawn()
             }
         }
 
-        private List<NetworkBehaviour> m_ChildNetworkBehaviours;
+        internal List<NetworkBehaviour> m_ChildNetworkBehaviours;
 
         internal List<NetworkBehaviour> ChildNetworkBehaviours
         {
             get
             {
-                if (m_ChildNetworkBehaviours != null)
+                if (m_ChildNetworkBehaviours == null)
                 {
-                    return m_ChildNetworkBehaviours;
+                    m_ChildNetworkBehaviours = BuildChildBehavioursList();
                 }
 
-                m_ChildNetworkBehaviours = new List<NetworkBehaviour>();
-                var networkBehaviours = GetComponentsInChildren<NetworkBehaviour>(true);
-                for (int i = 0; i < networkBehaviours.Length; i++)
+                return m_ChildNetworkBehaviours;
+            }
+        }
+
+        private List<NetworkBehaviour> BuildChildBehavioursList()
+        {
+#if UNITY_EDITOR
+            if (NetworkManagerOwner == null)
+            {
+                Debug.LogError("NetworkManagerOwner should be set! Setting owner to NetworkManager.Singleton");
+                NetworkManagerOwner = NetworkManager.Singleton;
+            }
+#endif
+
+            var networkBehaviours = GetComponentsInChildren<NetworkBehaviour>(true);
+            var childBehaviours = new List<NetworkBehaviour>(networkBehaviours.Length);
+
+            foreach (var behaviour in networkBehaviours)
+            {
+                // Find the first parent NetworkObject of this child
+                // if it's not ourselves, this childBehaviour belongs to a different NetworkObject.
+                var networkObj = behaviour.GetComponentInParent<NetworkObject>();
+                if (networkObj != this)
                 {
-                    // Find the first parent NetworkObject of this child
-                    // if it's not ourselves, this childBehaviour belongs to a different NetworkObject.
-                    var networkObj = networkBehaviours[i].GetComponentInParent<NetworkObject>();
-                    if (networkObj != this)
-                    {
-                        continue;
-                    }
+                    continue;
+                }
 
-                    // Set ourselves as the NetworkObject that this behaviour belongs to and add it to the child list
-                    networkBehaviours[i].SetNetworkObject(this);
-                    m_ChildNetworkBehaviours.Add(networkBehaviours[i]);
+                // Set ourselves as the NetworkObject that this behaviour belongs to and add it to the child list
+                var nextIndex = childBehaviours.Count;
+                childBehaviours.Add(behaviour);
+                behaviour.SetNetworkObject(this, (ushort)nextIndex);
 
-                    var type = networkBehaviours[i].GetType();
-                    if (type == typeof(NetworkTransform) || type.IsInstanceOfType(typeof(NetworkTransform)) || type.IsSubclassOf(typeof(NetworkTransform)))
+                var type = behaviour.GetType();
+                if (type == typeof(NetworkTransform) || type.IsAssignableFrom(typeof(NetworkTransform)) || type.IsSubclassOf(typeof(NetworkTransform)))
+                {
+                    if (NetworkTransforms == null)
                     {
-                        if (NetworkTransforms == null)
-                        {
-                            NetworkTransforms = new List<NetworkTransform>();
-                        }
-                        var networkTransform = networkBehaviours[i] as NetworkTransform;
-                        networkTransform.IsNested = i != 0 && networkTransform.gameObject != gameObject;
-                        NetworkTransforms.Add(networkTransform);
+                        NetworkTransforms = new List<NetworkTransform>();
                     }
+                    var networkTransform = behaviour as NetworkTransform;
+                    networkTransform.IsNested = networkTransform.gameObject != gameObject;
+                    NetworkTransforms.Add(networkTransform);
+                }
 #if COM_UNITY_MODULES_PHYSICS || COM_UNITY_MODULES_PHYSICS2D
-                    else if (type.IsSubclassOf(typeof(NetworkRigidbodyBase)))
+                else if (type.IsSubclassOf(typeof(NetworkRigidbodyBase)))
+                {
+                    if (NetworkRigidbodies == null)
                     {
-                        if (NetworkRigidbodies == null)
-                        {
-                            NetworkRigidbodies = new List<NetworkRigidbodyBase>();
-                        }
-                        NetworkRigidbodies.Add(networkBehaviours[i] as NetworkRigidbodyBase);
+                        NetworkRigidbodies = new List<NetworkRigidbodyBase>();
                     }
-#endif
+                    NetworkRigidbodies.Add(behaviour as NetworkRigidbodyBase);
                 }
-
-                return m_ChildNetworkBehaviours;
+#endif
             }
+
+            childBehaviours.TrimExcess();
+            return childBehaviours;
         }
 
         /// <summary>
@@ -2744,25 +2762,14 @@ internal static void VerifyParentingStatus()
         public ushort GetNetworkBehaviourOrderIndex(NetworkBehaviour instance)
         {
             // read the cached index, and verify it first
-            if (instance.NetworkBehaviourIdCache < ChildNetworkBehaviours.Count)
+            if (instance.NetworkBehaviourId < ChildNetworkBehaviours.Count)
             {
-                if (ChildNetworkBehaviours[instance.NetworkBehaviourIdCache] == instance)
+                if (ChildNetworkBehaviours[instance.NetworkBehaviourId] == instance)
                 {
-                    return instance.NetworkBehaviourIdCache;
+                    return instance.NetworkBehaviourId;
                 }
 
-                // invalid cached id reset
-                instance.NetworkBehaviourIdCache = default;
-            }
-
-            for (ushort i = 0; i < ChildNetworkBehaviours.Count; i++)
-            {
-                if (ChildNetworkBehaviours[i] == instance)
-                {
-                    // cache the id, for next query
-                    instance.NetworkBehaviourIdCache = i;
-                    return i;
-                }
+                Debug.LogError("Network behaviour at index has changed. This should not be possible.");
             }
 
             return 0;
@@ -3246,6 +3253,8 @@ internal static NetworkObject AddSceneObject(in SceneObject sceneObject, FastBuf
                 return null;
             }
 
+            networkObject.NetworkManagerOwner = networkManager;
+
             // This will get set again when the NetworkObject is spawned locally, but we set it here ahead of spawning
             // in order to be able to determine which NetworkVariables the client will be allowed to read.
             networkObject.OwnerClientId = sceneObject.OwnerClientId;

com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs

@@ -1822,7 +1822,7 @@ private void OnSessionOwnerLoadedScene(uint sceneEventId, Scene scene)
                     if (!keyValuePairBySceneHandle.Value.IsPlayerObject)
                     {
                         // All in-scene placed NetworkObjects default to being owned by the server
-                        NetworkManager.SpawnManager.SpawnNetworkObjectLocally(keyValuePairBySceneHandle.Value,
+                        NetworkManager.SpawnManager.SpawnNetworkObjectLocally(keyValuePairBySceneHandle.Value, NetworkManager,
                             NetworkManager.SpawnManager.GetNetworkObjectId(), true, false, NetworkManager.LocalClientId, true);
                     }
                 }

com.unity.netcode.gameobjects/Runtime/SceneManagement/SceneEventData.cs

@@ -376,8 +376,15 @@ internal void AddDespawnedInSceneNetworkObjects()
 #endif
             foreach (var sobj in inSceneNetworkObjects)
             {
+                // For integration tests, don't collect objects that don't belong to us.
+                if (sobj.NetworkManagerOwner != null && sobj.NetworkManagerOwner != m_NetworkManager)
+                {
+                    continue;
+                }
+
                 if (sobj.IsSceneObject.HasValue && sobj.IsSceneObject.Value && !sobj.IsSpawned)
                 {
+                    sobj.NetworkManagerOwner = m_NetworkManager;
                     m_DespawnedInSceneObjectsSync.Add(sobj);
                 }
             }
@@ -1083,19 +1090,22 @@ private void DeserializeDespawnedInScenePlacedNetworkObjects()
                 }
 
                 // Now find the in-scene NetworkObject with the current GlobalObjectIdHash we are looking for
-                if (sceneRelativeNetworkObjects.ContainsKey(globalObjectIdHash))
+                if (sceneRelativeNetworkObjects.TryGetValue(globalObjectIdHash, out var despawnedObject))
                 {
+                    // Set the owner of this network object
+                    despawnedObject.NetworkManagerOwner = m_NetworkManager;
+
                     // Since this is a NetworkObject that was never spawned, we just need to send a notification
                     // out that it was despawned so users can make adjustments
-                    sceneRelativeNetworkObjects[globalObjectIdHash].InvokeBehaviourNetworkDespawn();
+                    despawnedObject.InvokeBehaviourNetworkDespawn();
                     if (!m_NetworkManager.SceneManager.ScenePlacedObjects.ContainsKey(globalObjectIdHash))
                     {
                         m_NetworkManager.SceneManager.ScenePlacedObjects.Add(globalObjectIdHash, new Dictionary<NetworkSceneHandle, NetworkObject>());
                     }
 
-                    if (!m_NetworkManager.SceneManager.ScenePlacedObjects[globalObjectIdHash].ContainsKey(sceneRelativeNetworkObjects[globalObjectIdHash].GetSceneOriginHandle()))
+                    if (!m_NetworkManager.SceneManager.ScenePlacedObjects[globalObjectIdHash].ContainsKey(despawnedObject.GetSceneOriginHandle()))
                     {
-                        m_NetworkManager.SceneManager.ScenePlacedObjects[globalObjectIdHash].Add(sceneRelativeNetworkObjects[globalObjectIdHash].GetSceneOriginHandle(), sceneRelativeNetworkObjects[globalObjectIdHash]);
+                        m_NetworkManager.SceneManager.ScenePlacedObjects[globalObjectIdHash].Add(despawnedObject.GetSceneOriginHandle(), despawnedObject);
                     }
                 }
                 else