Merge branch 'develop-2.0.0' into renovate/develop-2.0.0-package-ci-ubuntu-22.04-4.x

Author: michalChrobot

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -24,8 +24,10 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed issue when using a client-server topology where a `NetworkList` with owner write permissions was resetting sent time and dirty flags after having been spawned on owning clients that were not the spawn authority. (#3850)
 - Fixed an integer overflow that occurred when configuring a large disconnect timeout with Unity Transport. (#3810)
 
+
 ### Security
 
 

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

@@ -76,6 +76,16 @@ public uint PrefabIdHash
         /// </remarks>
         public List<NetworkTransform> NetworkTransforms { get; private set; }
 
+        /// <summary>
+        /// Set to true if this instance is the original/first instance created and spawned which
+        /// means the <see cref="CreateObjectMessage"/> was generated from this instance and sent
+        /// to all other clients.
+        ///
+        /// Client-Server: This will be true for all instances on the server or host.
+        /// Distributed Authority: This will be true on the client that created the first instance
+        /// and spawned it (even if spawning with ownership being assigned to a different client).
+        /// </summary>
+        internal bool IsSpawnAuthority;
 
 #if COM_UNITY_MODULES_PHYSICS || COM_UNITY_MODULES_PHYSICS2D
         /// <summary>
@@ -2009,6 +2019,7 @@ internal void ResetOnDespawn()
         {
             // Always clear out the observers list when despawned
             Observers.Clear();
+            IsSpawnAuthority = false;
             IsSpawned = false;
             DeferredDespawnTick = 0;
             m_LatestParent = null;

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

@@ -60,11 +60,18 @@ public NetworkList(IEnumerable<T> values = default,
 
         internal override void OnSpawned()
         {
-            // If we are dirty and have write permissions by the time the NetworkObject
-            // is finished spawning (same frame), then go ahead and reset the dirty related
-            // properties for NetworkList in the event user script has made changes when
-            // spawning to prevent duplicate entries.
-            if (IsDirty() && CanSend())
+            // If the NetworkList is:
+            // - Dirty
+            // - State updates can be sent:
+            // -- The instance has write permissions.
+            // -- The last sent time plus the max send time period is less than the current time.
+            // - User script has modified the list during spawn.
+            // - This instance is on the spawn authority side.
+            // When the NetworkObject is finished spawning (on the same frame), go ahead and reset
+            // the dirty related properties and last sent time to prevent duplicate entries from
+            // being sent (i.e. CreateObjectMessage will contain the changes so we don't need to
+            // send a proceeding NetworkVariableDeltaMessage).
+            if (IsDirty() && CanSend() && m_NetworkObject.IsSpawnAuthority)
             {
                 UpdateLastSentTime();
                 ResetDirty();

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

@@ -1054,6 +1054,9 @@ internal void AuthorityLocalSpawn([NotNull] NetworkObject networkObject, ulong n
                     Debug.LogError("Spawning NetworkObjects with nested NetworkObjects is only supported for scene objects. Child NetworkObjects will not be spawned over the network!");
                 }
             }
+
+            networkObject.IsSpawnAuthority = true;
+
             // Invoke NetworkBehaviour.OnPreSpawn methods
             networkObject.NetworkManagerOwner = NetworkManager;
             networkObject.InvokeBehaviourNetworkPreSpawn();

com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkListTests.cs

@@ -27,6 +27,12 @@ public NetworkListTests(HostOrServer host) : base(host) { }
 
         private ulong m_TestObjectId;
 
+        protected override IEnumerator OnSetup()
+        {
+            IsOwnerWriteTest = false;
+            return base.OnSetup();
+        }
+
         protected override void OnServerAndClientsCreated()
         {
             m_ListObjectPrefab = CreateNetworkObjectPrefab("ListObject");
@@ -285,13 +291,169 @@ private int[] Shuffle(List<int> list)
             // This will do a shuffle of the list
             return list.OrderBy(_ => rng.Next()).ToArray();
         }
+
+        private List<NetworkObject> m_SpawnedObjects = new List<NetworkObject>();
+        internal const int ValueCount = 10;
+        internal static bool IsOwnerWriteTest;
+        internal NetworkManager LateJoinedClient;
+        internal static List<int> OwnerWriteExpectedValues = new List<int>();
+
+        protected override void OnNewClientCreated(NetworkManager networkManager)
+        {
+            if (IsOwnerWriteTest)
+            {
+                LateJoinedClient = networkManager;
+            }
+            else
+            {
+                LateJoinedClient = null;
+            }
+            base.OnNewClientCreated(networkManager);
+        }
+
+        [UnityTest]
+        public IEnumerator OwnerWriteTests()
+        {
+            IsOwnerWriteTest = true;
+            var authorityNetworkManager = GetAuthorityNetworkManager();
+            m_SpawnedObjects.Clear();
+            OwnerWriteExpectedValues.Clear();
+            // Set our initial expected values as 0 - 9
+            for (int i = 0; i < ValueCount; i++)
+            {
+                OwnerWriteExpectedValues.Add(i);
+            }
+
+            // Each spawned instance will be owned by each NetworkManager instance in order
+            // to validate owner write NetworkLists.
+            foreach (var networkManager in m_NetworkManagers)
+            {
+                m_SpawnedObjects.Add(SpawnObject(m_ListObjectPrefab, networkManager).GetComponent<NetworkObject>());
+            }
+
+            // Verify all NetworkManager instances spawned the objects
+            yield return WaitForSpawnedOnAllOrTimeOut(m_SpawnedObjects);
+            AssertOnTimeout("Not all instances were spawned on all clients!");
+
+            // Verify all spawned object instances have the expected owner write NetworkList values
+            yield return WaitForConditionOrTimeOut(OnVerifyOwnerWriteData);
+            AssertOnTimeout("Detected invalid count or value on one of the spawned instances!");
+
+            // Late join a client
+            yield return CreateAndStartNewClient();
+
+            // Spawn an instance with the new client being the owner
+            m_SpawnedObjects.Add(SpawnObject(m_ListObjectPrefab, LateJoinedClient).GetComponent<NetworkObject>());
+
+            // Verify all NetworkManager instances spawned the objects
+            yield return WaitForSpawnedOnAllOrTimeOut(m_SpawnedObjects);
+            AssertOnTimeout("Not all instances were spawned on all clients!");
+
+            // Verify all spawned object instances have the expected owner write NetworkList values
+            yield return WaitForConditionOrTimeOut(OnVerifyOwnerWriteData);
+            AssertOnTimeout("Detected invalid count or value on one of the spawned instances!");
+
+            // Now have all of the clients update their list values to randomly assigned values
+            // in order to verify changes to owner write NetworkLists are synchronized properly.
+            OwnerWriteExpectedValues.Clear();
+            for (int i = 0; i < ValueCount; i++)
+            {
+                OwnerWriteExpectedValues.Add(Random.Range(10, 100));
+            }
+            UpdateOwnerWriteValues();
+
+            // Verify all spawned object instances have the expected owner write NetworkList values
+            yield return WaitForConditionOrTimeOut(OnVerifyOwnerWriteData);
+            AssertOnTimeout("Detected invalid count or value on one of the spawned instances!");
+
+            // Verifies that spawning with ownership in distributed authority mode work properly.
+            // Where:
+            // Client-A spawns with ownership assigned to Client-B
+            // Client-B applies values at spawn.
+            // All clients then should be updated with those new values applied.
+            if (m_DistributedAuthority)
+            {
+                var prefabNetworkObject = m_ListObjectPrefab.GetComponent<NetworkObject>();
+                foreach (var networkManager in m_NetworkManagers)
+                {
+                    var instance = Object.Instantiate(m_ListObjectPrefab).GetComponent<NetworkObject>();
+                    SpawnInstanceWithOwnership(instance, authorityNetworkManager, networkManager.LocalClientId);
+                    m_SpawnedObjects.Add(instance);
+                }
+
+                // Verify all NetworkManager instances spawned the objects
+                yield return WaitForSpawnedOnAllOrTimeOut(m_SpawnedObjects);
+                AssertOnTimeout("Not all instances were spawned on all clients!");
+
+                // Verify all spawned object instances have the expected owner write NetworkList values
+                yield return WaitForConditionOrTimeOut(OnVerifyOwnerWriteData);
+                AssertOnTimeout("Detected invalid count or value on one of the spawned instances!");
+            }
+        }
+
+        private void UpdateOwnerWriteValues()
+        {
+            foreach (var spawnedObject in m_SpawnedObjects)
+            {
+                var owningNetworkManager = m_NetworkManagers.Where((c) => c.LocalClientId == spawnedObject.OwnerClientId).First();
+                var networkObjectId = spawnedObject.NetworkObjectId;
+                var listComponent = owningNetworkManager.SpawnManager.SpawnedObjects[networkObjectId].GetComponent<NetworkListTest>();
+                for (int i = 0; i < ValueCount; i++)
+                {
+                    listComponent.OwnerWriteList[i] = OwnerWriteExpectedValues[i];
+                }
+            }
+        }
+
+        private bool OnVerifyOwnerWriteData(StringBuilder errorLog)
+        {
+            foreach (var spawnedObject in m_SpawnedObjects)
+            {
+                var networkObjectId = spawnedObject.NetworkObjectId;
+                foreach (var networkManager in m_NetworkManagers)
+                {
+                    if (!networkManager.SpawnManager.SpawnedObjects.ContainsKey(networkObjectId))
+                    {
+                        errorLog.Append($"[Client-{networkManager.LocalClientId}] Does not have an instance of spawned object NetworkObjectId: {networkObjectId}");
+                        return false;
+                    }
+                    var listComponent = networkManager.SpawnManager.SpawnedObjects[networkObjectId].GetComponent<NetworkListTest>();
+
+                    if (listComponent == null)
+                    {
+                        errorLog.Append($"[Client-{networkManager.LocalClientId}] List component was not found");
+                        return false;
+                    }
+
+                    if (listComponent.OwnerWriteList.Count != ValueCount)
+                    {
+                        errorLog.Append($"[Client-{networkManager.LocalClientId}] List component has the incorrect number of items. Expected: {ValueCount}, Have: {listComponent.TheList.Count}");
+                        return false;
+                    }
+
+                    for (int i = 0; i < ValueCount; i++)
+                    {
+                        var actual = listComponent.OwnerWriteList[i];
+                        var expected = OwnerWriteExpectedValues[i];
+                        if (expected != actual)
+                        {
+                            errorLog.Append($"[Client-{networkManager.LocalClientId}] Incorrect value at index {i}, expected: {expected}, actual: {actual}");
+                            return false;
+                        }
+                    }
+                }
+            }
+
+            return true;
+        }
     }
 
     internal class NetworkListTest : NetworkBehaviour
     {
         public readonly NetworkList<int> TheList = new();
         public readonly NetworkList<StructUsedOnlyInNetworkList> TheStructList = new();
         public readonly NetworkList<FixedString128Bytes> TheLargeList = new();
+        public readonly NetworkList<int> OwnerWriteList = new NetworkList<int>(default, NetworkVariableReadPermission.Everyone, NetworkVariableWritePermission.Owner);
 
         private void ListChanged(NetworkListEvent<int> e)
         {
@@ -309,6 +471,18 @@ public override void OnDestroy()
             base.OnDestroy();
         }
 
+        public override void OnNetworkSpawn()
+        {
+            if (NetworkListTests.IsOwnerWriteTest && IsOwner)
+            {
+                for (int i = 0; i < NetworkListTests.ValueCount; i++)
+                {
+                    OwnerWriteList.Add(NetworkListTests.OwnerWriteExpectedValues[i]);
+                }
+            }
+            base.OnNetworkSpawn();
+        }
+
         public bool ListDelegateTriggered;
     }
 
@@ -325,8 +499,6 @@ internal class NetworkListTestPredicate : ConditionalPredicateBase
 
         private readonly NetworkListTest m_NonAuthorityInstance;
 
-        private string m_TestStageFailedMessage;
-
         /// <summary>
         /// Determines if the condition has been reached for the current NetworkListTestState
         /// </summary>