fix: networkvariable not synchronizing changes made during spawn or post-spawn. (#3878)

* fix

Fixing the issue with the missing `OnSpawned` override within `NetworkVariable`.

* test

Adding a general test for scenarios like this one.

* update

Adding changelog entry.

* style

Adding comments for clarity of what `OnSpawned` and the call to `PostNetworkVariableWrite` is for.
Removing trailing whitespaces.

* refactor

Just apply the changes to the NetworkVariable instance and not all NetworkVariables of the NetworkBehaviour.

* style

correcting the comment.

* fix

Very similar to NetworkList (we might collapse the OnSpawned logic into NetworkVariableBase.

* test-fix

OwnerModifiedTests was not properly spawning from the non-session owner client (it was spawning with ownership on the session owner side).

NetworkVariableCollectionTests is showing an issue with changes for only the host instance and only within the TestDictionaryCollections (has to do with trying to add and then reverting vs the tracked changes).

* test

Narrowed down the issue to the changes tracked (i.e. added, removed, changed, unchanged) not matching when running a host but when comparing the actual dictionaries that all passes...

* test

Removing developer logging.

* test - fix

Fixed the issue in the `TestDictionaryCollections` test where clients that did not have the initial added target changes for server changes when running a host. Now, upon clients spawning players locally on the clients, the server write dictionary that is already populated by the host will have the added target changes injected during spawn to assure the changes match.  (This is only for this specific test when running  the host `TestFixture` pass.

* style

removing unused debug related script.

* update

Based on peer review, folding the same logic for spawn authority resetting dirty once spawned to assure no duplicate changes are sent.

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -6,6 +6,13 @@ 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).
 
+## [2.8.2] - 2025-02-11
+
+### Fixed
+
+- Fixed issue where `NetworkVariable` was not properly synchronizing to changes made by the spawn and write authority during `OnNetworkSpawn` and `OnNetworkPostSpawn`. (#3878)
+
+
 ## [2.8.1] - 2025-01-23
 
 ### Fixed

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

@@ -836,7 +836,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();
             }
         }
 
@@ -889,7 +889,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()