Merge branch 'develop-2.0.0' into fix/3559/using-unreliable-deltas-should-disable-certain-features

Author: EmandM

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -25,6 +25,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 ### Fixed
 
 - Fixed `NetworkTransform` issue where a user could enable UseUnreliableDeltas while SwitchTransformSpaceWhenParented was also enabled (and vice versa). (#3875)
+- Fixed issue where `NetworkVariable` was not properly synchronizing to changes made by the spawn and write authority during `OnNetworkSpawn` and `OnNetworkPostSpawn`. (#3878)
 - Fixed issue where `NetworkManager` was not cleaning itself up if an exception was thrown while starting. (#3864)
 - Prevented a `NullReferenceException` in `UnityTransport` when using a custom `INetworkStreamDriverConstructor` that doesn't use all the default pipelines and the multiplayer tools package is installed. (#3853)
 

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

@@ -838,7 +838,7 @@ internal void NetworkPostSpawn()
             // all spawn related methods have been invoked.
             for (int i = 0; i < NetworkVariableFields.Count; i++)
             {
-                NetworkVariableFields[i].OnSpawned();
+                NetworkVariableFields[i].InternalOnSpawned();
             }
         }
 
@@ -891,7 +891,7 @@ internal void InternalOnNetworkPreDespawn()
             // all spawn related methods have been invoked.
             for (int i = 0; i < NetworkVariableFields.Count; i++)
             {
-                NetworkVariableFields[i].OnPreDespawn();
+                NetworkVariableFields[i].InternalOnPreDespawn();
             }
         }
 

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

@@ -58,28 +58,6 @@ public NetworkList(IEnumerable<T> values = default,
             Dispose();
         }
 
-        internal override void OnSpawned()
-        {
-            // 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();
-                SetDirty(false);
-            }
-            base.OnSpawned();
-        }
-
         /// <inheritdoc cref="NetworkVariable{T}.ResetDirty"/>
         public override void ResetDirty()
         {

com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs

@@ -140,27 +140,37 @@ public void Initialize(NetworkBehaviour networkBehaviour)
             }
         }
 
-        /// TODO-API: After further vetting and alignment on these, we might make them part of the public API.
-        /// Could actually be like an interface that gets automatically registered for these kinds of notifications
-        /// without having to be a NetworkBehaviour.
-        #region OnSpawn and OnPreDespawn (ETC)
-
         /// <summary>
         /// Invoked after the associated <see cref="NetworkBehaviour.OnNetworkPostSpawn"/> has been invoked.
         /// </summary>
-        internal virtual void OnSpawned()
+        internal void InternalOnSpawned()
         {
-
+            // If the NetworkVariableBase derived class is:
+            // - On the spawn authority side.
+            // - 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.
+            // 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 updates from
+            // being sent (i.e. CreateObjectMessage will contain the changes so we don't need to
+            // send a proceeding NetworkVariableDeltaMessage).
+            if (m_NetworkObject.IsSpawnAuthority && IsDirty() && CanWrite() && CanSend())
+            {
+                UpdateLastSentTime();
+                ResetDirty();
+                SetDirty(false);
+            }
         }
 
         /// <summary>
         /// Invoked after the associated <see cref="NetworkBehaviour.OnNetworkPreDespawn"/> has been invoked.
         /// </summary>
-        internal virtual void OnPreDespawn()
+        internal void InternalOnPreDespawn()
         {
 
         }
-        #endregion
 
         /// <summary>
         /// Deinitialize is invoked when a NetworkObject is despawned.

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

@@ -831,7 +831,7 @@ public IEnumerator TestDictionaryCollections()
                 {
                     // Server-side add same key and SerializableObject prior to being added to the owner side
                     compDictionaryServer.ListCollectionOwner.Value.Add(newEntry.Item1, newEntry.Item2);
-                    // Checking if dirty on server side should revert back to origina known current dictionary state
+                    // Checking if dirty on server side should revert back to original known current dictionary state
                     compDictionaryServer.ListCollectionOwner.IsDirty();
                     yield return WaitForConditionOrTimeOut(() => compDictionaryServer.CompareTrackedChanges(ListTestHelperBase.Targets.Owner));
                     AssertOnTimeout($"Server add to owner write collection property failed to restore on {className} {compDictionaryServer.name}! {compDictionaryServer.GetLog()}");
@@ -840,7 +840,6 @@ public IEnumerator TestDictionaryCollections()
                     // Server-side add a completely new key and SerializableObject to to owner write permission property
                     compDictionaryServer.ListCollectionOwner.Value.Add(GetNextKey(), SerializableObject.GetRandomObject());
                     // Both should be overridden by the owner-side update
-
                 }
                 VerboseDebug($"[{compDictionary.name}][Owner] Adding Key: {newEntry.Item1}");
                 // Add key and SerializableObject to owner side
@@ -852,15 +851,17 @@ public IEnumerator TestDictionaryCollections()
                 //////////////////////////////////
                 // Server Add SerializableObject Entry
                 newEntry = (GetNextKey(), SerializableObject.GetRandomObject());
+
                 // Only test restore on non-host clients (otherwise a host is both server and client/owner)
                 if (!client.IsServer)
                 {
                     // Client-side add same key and SerializableObject to server write permission property
                     compDictionary.ListCollectionServer.Value.Add(newEntry.Item1, newEntry.Item2);
-                    // Checking if dirty on client side should revert back to origina known current dictionary state
+                    // Checking if dirty on client side should revert back to original known current dictionary state
                     compDictionary.ListCollectionServer.IsDirty();
                     yield return WaitForConditionOrTimeOut(() => compDictionary.CompareTrackedChanges(ListTestHelperBase.Targets.Server));
                     AssertOnTimeout($"Client-{client.LocalClientId} add to server write collection property failed to restore on {className} {compDictionary.name}! {compDictionary.GetLog()}");
+
                     // Client-side add the same key and SerializableObject to server write permission property (would throw key exists exception too if previous failed)
                     compDictionary.ListCollectionServer.Value.Add(newEntry.Item1, newEntry.Item2);
                     // Client-side add a completely new key and SerializableObject to to server write permission property
@@ -892,7 +893,6 @@ public IEnumerator TestDictionaryCollections()
 
                 yield return WaitForConditionOrTimeOut(() => compDictionaryServer.ValidateInstances());
                 AssertOnTimeout($"[Server] Not all instances of client-{compDictionaryServer.OwnerClientId}'s {className} {compDictionaryServer.name} component match! {compDictionaryServer.GetLog()}");
-
                 ValidateClientsFlat(client);
                 ////////////////////////////////////
                 // Owner Change SerializableObject Entry
@@ -915,7 +915,7 @@ public IEnumerator TestDictionaryCollections()
                     {
                         // Server-side update same key value prior to being updated to the owner side
                         compDictionaryServer.ListCollectionOwner.Value[valueInt] = randomObject;
-                        // Checking if dirty on server side should revert back to origina known current dictionary state
+                        // Checking if dirty on server side should revert back to original known current dictionary state
                         compDictionaryServer.ListCollectionOwner.IsDirty();
                         yield return WaitForConditionOrTimeOut(() => compDictionaryServer.CompareTrackedChanges(ListTestHelperBase.Targets.Owner));
                         AssertOnTimeout($"Server update collection entry value to local owner write collection property failed to restore on {className} {compDictionaryServer.name}! {compDictionaryServer.GetLog()}");
@@ -956,7 +956,7 @@ public IEnumerator TestDictionaryCollections()
                     {
                         // Owner-side update same key value prior to being updated to the server side
                         compDictionary.ListCollectionServer.Value[valueInt] = randomObject;
-                        // Checking if dirty on owner side should revert back to origina known current dictionary state
+                        // Checking if dirty on owner side should revert back to original known current dictionary state
                         compDictionary.ListCollectionServer.IsDirty();
                         yield return WaitForConditionOrTimeOut(() => compDictionary.CompareTrackedChanges(ListTestHelperBase.Targets.Server));
                         AssertOnTimeout($"Client-{client.LocalClientId} update collection entry value to local server write collection property failed to restore on {className} {compDictionary.name}! {compDictionary.GetLog()}");
@@ -1841,10 +1841,10 @@ private bool ChangesMatch(ulong clientId, Dictionary<DeltaTypes, Dictionary<int,
             var deltaTypes = Enum.GetValues(typeof(DeltaTypes)).OfType<DeltaTypes>().ToList();
             foreach (var deltaType in deltaTypes)
             {
-                LogMessage($"Comparing {deltaType}:");
+                LogMessage($"[Comparing {deltaType}] Local: {local[deltaType].Count} | Other: {other[deltaType].Count}");
                 if (local[deltaType].Count != other[deltaType].Count)
                 {
-                    LogMessage($"{deltaType}s count did not match!");
+                    LogMessage($"[Client-{clientId}] Local {deltaType}s count of {local[deltaType].Count} did not match the other's count of {other[deltaType].Count}!");
                     return false;
                 }
                 if (!CompareDictionaries(clientId, local[deltaType], other[deltaType]))
@@ -1970,6 +1970,18 @@ public void TrackChanges(Targets target, Dictionary<int, SerializableObject> pre
             contextTable[DeltaTypes.Removed] = whatWasRemoved;
             contextTable[DeltaTypes.Changed] = whatChanged;
             contextTable[DeltaTypes.UnChanged] = whatRemainedTheSame;
+
+            // Log all incoming changes when debug mode is enabled
+            if (!IsOwner && IsDebugMode)
+            {
+                LogMessage($"[{NetworkManager.name}][TrackChanges-> Client-{OwnerClientId}] Collection was updated!");
+                LogMessage($"Added: {whatWasAdded.Count} ");
+                LogMessage($"Removed: {whatWasRemoved.Count} ");
+                LogMessage($"Changed: {whatChanged.Count} ");
+                LogMessage($"UnChanged: {whatRemainedTheSame.Count} ");
+                UnityEngine.Debug.Log($"{GetLog()}");
+                LogStart();
+            }
         }
 
         public void OnServerListValuesChanged(Dictionary<int, SerializableObject> previous, Dictionary<int, SerializableObject> current)
@@ -2016,30 +2028,43 @@ public void ResetTrackedChanges()
 
         protected override void OnNetworkPostSpawn()
         {
-            TrackRelativeInstances();
 
+            TrackRelativeInstances();
             ListCollectionServer.OnValueChanged += OnServerListValuesChanged;
             ListCollectionOwner.OnValueChanged += OnOwnerListValuesChanged;
 
             if (!IsDebugMode)
             {
                 InitValues();
             }
+
+            base.OnNetworkPostSpawn();
         }
 
         public void InitValues()
         {
             if (IsServer)
             {
                 ListCollectionServer.Value = OnSetServerValues();
-                ListCollectionOwner.CheckDirtyState();
+                ListCollectionServer.CheckDirtyState();
             }
 
             if (IsOwner)
             {
                 ListCollectionOwner.Value = OnSetOwnerValues();
                 ListCollectionOwner.CheckDirtyState();
             }
+
+            // When running a host, the changes being tracked will not match because clients will be synchronized with changes
+            // already applied. This fixing this issue by injecting "added" server targeted changes during initialization on
+            // the connected clients' side.
+            if (!IsServer)
+            {
+                if (ListCollectionServer.Value.Count > 0 && NetworkVariableChanges[Targets.Server][DeltaTypes.Added].Count == 0)
+                {
+                    TrackChanges(Targets.Server, new Dictionary<int, SerializableObject>(), ListCollectionServer.Value);
+                }
+            }
         }
 
         public override void OnNetworkDespawn()