Merge branch 'develop' into chore/add_auto_labeler

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -6,7 +6,19 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 Additional documentation and release notes are available at [Multiplayer Documentation](https://docs-multiplayer.unity3d.com).
 
-[Unreleased]
+## [Unreleased]
+
+### Added
+
+### Fixed
+
+- Fixed issue where a spawned `NetworkObject` that was registered with a prefab handler and owned by a client would invoke destroy more than once on the host-server side if the client disconnected while the `NetworkObject` was still spawned. (#3202)
+- Fixed issue where `NetworkRigidBody2D` was still using the deprecated `isKinematic` property in Unity versions 2022.3 and newer. (#3199)
+- Fixed issue where an exception was thrown when calling `NetworkManager.Shutdown` after calling `UnityTransport.Shutdown`. (#3118)
+
+### Changed
+
+## [1.12.0] - 2024-11-19
 
 ### Added
 
@@ -18,7 +30,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
-- Fixed issue where an exception was thrown when calling `NetworkManager.Shutdown` after calling `UnityTransport.Shutdown`. (#3118)
+- Fixed issue where `NetworkList` properties on in-scene placed `NetworkObject`s could cause small memory leaks when entering playmode. (#3148)
 - Fixed issue where a newly synchronizing client would be synchronized with the current `NetworkVariable` values always which could cause issues with collections if there were any pending state updates. Now, when initially synchronizing a client, if a `NetworkVariable` has a pending state update it will serialize the previously known value(s) to the synchronizing client so when the pending updates are sent they aren't duplicate values on the newly connected client side. (#3126)
 - Fixed issue where changing ownership would mark every `NetworkVariable` dirty. Now, it will only mark any `NetworkVariable` with owner read permissions as dirty and will send/flush any pending updates to all clients prior to sending the change in ownership message.  (#3126)
 - Fixed issue with `NetworkVariable` collections where transferring ownership to another client would not update the new owner's previous value to the most current value which could cause the last/previous added value to be detected as a change when adding or removing an entry (as long as the entry removed was not the last/previously added value).  (#3126)

com.unity.netcode.gameobjects/Components/NetworkRigidbody2D.cs

@@ -52,7 +52,16 @@ private void SetupRigidBody()
             // Turn off physics for the rigid body until spawned, otherwise
             // clients can run fixed update before the first full
             // NetworkTransform update
-            m_Rigidbody.isKinematic = true;
+            SetIsKinematic(true);
+        }
+
+        private void SetIsKinematic(bool isKinematic)
+        {
+#if UNITY_2022_3_OR_NEWER
+            m_Rigidbody.bodyType = isKinematic ? RigidbodyType2D.Kinematic : RigidbodyType2D.Dynamic;
+#else
+            m_Rigidbody.isKinematic = isKinematic;
+#endif
         }
 
         /// <summary>
@@ -89,7 +98,7 @@ private void UpdateOwnershipAuthority()
             }
 
             // If you have authority then you are not kinematic
-            m_Rigidbody.isKinematic = !m_IsAuthority;
+            SetIsKinematic(!m_IsAuthority);
 
             // Set interpolation of the Rigidbody2D based on authority
             // With authority: let local transform handle interpolation

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

@@ -967,6 +967,11 @@ internal void OnClientDisconnectFromServer(ulong clientId)
                     {
                         if (NetworkManager.PrefabHandler.ContainsHandler(ConnectedClients[clientId].PlayerObject.GlobalObjectIdHash))
                         {
+                            // If the player is spawned, then despawn before invoking HandleNetworkPrefabDestroy
+                            if (playerObject.IsSpawned)
+                            {
+                                NetworkManager.SpawnManager.DespawnObject(ConnectedClients[clientId].PlayerObject, false);
+                            }
                             NetworkManager.PrefabHandler.HandleNetworkPrefabDestroy(ConnectedClients[clientId].PlayerObject);
                         }
                         else if (playerObject.IsSpawned)
@@ -984,40 +989,34 @@ internal void OnClientDisconnectFromServer(ulong clientId)
 
                 // Get the NetworkObjects owned by the disconnected client
                 var clientOwnedObjects = NetworkManager.SpawnManager.GetClientOwnedObjects(clientId);
-                if (clientOwnedObjects == null)
+
+                // Handle despawn & destroy or change ownership
+                for (int i = clientOwnedObjects.Count - 1; i >= 0; i--)
                 {
-                    // This could happen if a client is never assigned a player object and is disconnected
-                    // Only log this in verbose/developer mode
-                    if (NetworkManager.LogLevel == LogLevel.Developer)
+                    var ownedObject = clientOwnedObjects[i];
+                    if (!ownedObject)
                     {
-                        NetworkLog.LogWarning($"ClientID {clientId} disconnected with (0) zero owned objects!  Was a player prefab not assigned?");
+                        continue;
                     }
-                }
-                else
-                {
-                    // Handle changing ownership and prefab handlers
-                    for (int i = clientOwnedObjects.Count - 1; i >= 0; i--)
+                    if (!ownedObject.DontDestroyWithOwner)
                     {
-                        var ownedObject = clientOwnedObjects[i];
-                        if (ownedObject != null)
+                        if (NetworkManager.PrefabHandler.ContainsHandler(clientOwnedObjects[i].GlobalObjectIdHash))
                         {
-                            if (!ownedObject.DontDestroyWithOwner)
+                            if (ownedObject.IsSpawned)
                             {
-                                if (NetworkManager.PrefabHandler.ContainsHandler(clientOwnedObjects[i].GlobalObjectIdHash))
-                                {
-                                    NetworkManager.PrefabHandler.HandleNetworkPrefabDestroy(clientOwnedObjects[i]);
-                                }
-                                else
-                                {
-                                    Object.Destroy(ownedObject.gameObject);
-                                }
-                            }
-                            else if (!NetworkManager.ShutdownInProgress)
-                            {
-                                ownedObject.RemoveOwnership();
+                                NetworkManager.SpawnManager.DespawnObject(ownedObject, false);
                             }
+                            NetworkManager.PrefabHandler.HandleNetworkPrefabDestroy(clientOwnedObjects[i]);
+                        }
+                        else
+                        {
+                            Object.Destroy(ownedObject.gameObject);
                         }
                     }
+                    else if (!NetworkManager.ShutdownInProgress)
+                    {
+                        ownedObject.RemoveOwnership();
+                    }
                 }
 
                 // TODO: Could(should?) be replaced with more memory per client, by storing the visibility

com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs

@@ -49,6 +49,11 @@ public NetworkList(IEnumerable<T> values = default,
             }
         }
 
+        ~NetworkList()
+        {
+            Dispose();
+        }
+
         /// <inheritdoc />
         public override void ResetDirty()
         {
@@ -599,8 +604,17 @@ public int LastModifiedTick
         /// </summary>
         public override void Dispose()
         {
-            m_List.Dispose();
-            m_DirtyEvents.Dispose();
+            if (m_List.IsCreated)
+            {
+                m_List.Dispose();
+            }
+
+            if (m_DirtyEvents.IsCreated)
+            {
+                m_DirtyEvents.Dispose();
+            }
+
+            base.Dispose();
         }
     }
 

com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs

@@ -435,10 +435,15 @@ internal NetworkObject InstantiateAndSpawnNoParameterChecks(NetworkObject networ
 
             var networkObject = networkPrefab;
             // Host spawns the ovveride and server spawns the original prefab unless forceOverride is set to true where both server or host will spawn the override.
-            if (forceOverride || NetworkManager.IsHost)
+            if (forceOverride || NetworkManager.IsHost || NetworkManager.PrefabHandler.ContainsHandler(networkPrefab.GlobalObjectIdHash))
             {
                 networkObject = GetNetworkObjectToSpawn(networkPrefab.GlobalObjectIdHash, ownerClientId, position, rotation);
             }
+            else // Under this case, server instantiate the prefab passed in.
+            {
+                networkObject = InstantiateNetworkPrefab(networkPrefab.gameObject, networkPrefab.GlobalObjectIdHash, position, rotation);
+            }
+
             if (networkObject == null)
             {
                 Debug.LogError($"Failed to instantiate and spawn {networkPrefab.name}!");
@@ -447,7 +452,15 @@ internal NetworkObject InstantiateAndSpawnNoParameterChecks(NetworkObject networ
             networkObject.IsPlayerObject = isPlayerObject;
             networkObject.transform.position = position;
             networkObject.transform.rotation = rotation;
-            networkObject.SpawnWithOwnership(ownerClientId, destroyWithScene);
+            // If spawning as a player, then invoke SpawnAsPlayerObject
+            if (isPlayerObject)
+            {
+                networkObject.SpawnAsPlayerObject(ownerClientId, destroyWithScene);
+            }
+            else // Otherwise just spawn with ownership
+            {
+                networkObject.SpawnWithOwnership(ownerClientId, destroyWithScene);
+            }
             return networkObject;
         }
 
@@ -512,17 +525,38 @@ internal NetworkObject GetNetworkObjectToSpawn(uint globalObjectIdHash, ulong ow
                 }
                 else
                 {
-                    // Create prefab instance
-                    networkObject = UnityEngine.Object.Instantiate(networkPrefabReference).GetComponent<NetworkObject>();
-                    networkObject.transform.position = position ?? networkObject.transform.position;
-                    networkObject.transform.rotation = rotation ?? networkObject.transform.rotation;
-                    networkObject.NetworkManagerOwner = NetworkManager;
-                    networkObject.PrefabGlobalObjectIdHash = globalObjectIdHash;
+                    // Create prefab instance while applying any pre-assigned position and rotation values
+                    networkObject = InstantiateNetworkPrefab(networkPrefabReference, globalObjectIdHash, position, rotation);
                 }
             }
             return networkObject;
         }
 
+        /// <summary>
+        /// Instantiates a network prefab instance, assigns the base prefab <see cref="NetworkObject.GlobalObjectIdHash"/>, positions, and orients
+        /// the instance.
+        /// !!! Should only be invoked by <see cref="GetNetworkObjectToSpawn"/> unless used by an integration test !!!
+        /// </summary>
+        /// <remarks>
+        /// <param name="prefabGlobalObjectIdHash"> should be the base prefab <see cref="NetworkObject.GlobalObjectIdHash"/> value and not the
+        /// overrided value.
+        /// (Can be used for integration testing)
+        /// </remarks>
+        /// <param name="networkPrefab">prefab to instantiate</param>
+        /// <param name="prefabGlobalObjectIdHash"><see cref="NetworkObject.GlobalObjectIdHash"/> of the base prefab instance</param>
+        /// <param name="position">conditional position in place of the network prefab's default position</param>
+        /// <param name="rotation">conditional rotation in place of the network prefab's default rotation</param>
+        /// <returns>the instance of the <see cref="NetworkObject"/></returns>
+        internal NetworkObject InstantiateNetworkPrefab(GameObject networkPrefab, uint prefabGlobalObjectIdHash, Vector3? position, Quaternion? rotation)
+        {
+            var networkObject = UnityEngine.Object.Instantiate(networkPrefab).GetComponent<NetworkObject>();
+            networkObject.transform.position = position ?? networkObject.transform.position;
+            networkObject.transform.rotation = rotation ?? networkObject.transform.rotation;
+            networkObject.NetworkManagerOwner = NetworkManager;
+            networkObject.PrefabGlobalObjectIdHash = prefabGlobalObjectIdHash;
+            return networkObject;
+        }
+
         /// <summary>
         /// Creates a local NetowrkObject to be spawned.
         /// </summary>

com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTestHelpers.cs

@@ -527,6 +527,29 @@ public static void MakeNetworkObjectTestPrefab(NetworkObject networkObject, uint
             }
         }
 
+        /// <summary>
+        /// Creates a <see cref="NetworkObject"/> to be used with integration testing
+        /// </summary>
+        /// <param name="baseName">namr of the object</param>
+        /// <param name="owner">owner of the object</param>
+        /// <param name="moveToDDOL">when true, the instance is automatically migrated into the DDOL</param>
+        /// <returns></returns>
+        internal static GameObject CreateNetworkObject(string baseName, NetworkManager owner, bool moveToDDOL = false)
+        {
+            var gameObject = new GameObject
+            {
+                name = baseName
+            };
+            var networkObject = gameObject.AddComponent<NetworkObject>();
+            networkObject.NetworkManagerOwner = owner;
+            MakeNetworkObjectTestPrefab(networkObject);
+            if (moveToDDOL)
+            {
+                Object.DontDestroyOnLoad(gameObject);
+            }
+            return gameObject;
+        }
+
         public static GameObject CreateNetworkObjectPrefab(string baseName, NetworkManager server, params NetworkManager[] clients)
         {
             void AddNetworkPrefab(NetworkConfig config, NetworkPrefab prefab)
@@ -538,13 +561,7 @@ void AddNetworkPrefab(NetworkConfig config, NetworkPrefab prefab)
             Assert.IsNotNull(server, prefabCreateAssertError);
             Assert.IsFalse(server.IsListening, prefabCreateAssertError);
 
-            var gameObject = new GameObject
-            {
-                name = baseName
-            };
-            var networkObject = gameObject.AddComponent<NetworkObject>();
-            networkObject.NetworkManagerOwner = server;
-            MakeNetworkObjectTestPrefab(networkObject);
+            var gameObject = CreateNetworkObject(baseName, server);
             var networkPrefab = new NetworkPrefab() { Prefab = gameObject };
 
             // We could refactor this test framework to share a NetworkPrefabList instance, but at this point it's