fix: networkvariablebase not being reinitialized if networkobject persists between sessions (#3181)

* fix

This will "re-initialize" NetworkVariableBase derived classes when the instance is not destroyed and repurposed.

* update

adding changelog entry

* update

Adding PR number to changelog entry

* fix

? This test needs more comments.

* update

Changing the approach to this fix while also cleaning up the initialize process so it can be invoked when the derived class is not yet fully spawned (or just recently instantiated) without potentially assigning the wrong NetworkManager instance. It is being invoked multiple times anyway, so this just gradually initializes all of the required elements until it is finally considered "initialized".
This also prevents OnInitialize from being invoked multiple times during this process and OnInitialize is only invoked as a last step to being considered "initialized".

Added a Deinitialize step to NetworkVariableBase when despawning a NetworkObject where the initialized status is reset. This allows for the next spawning of the NetworkObject (if in-scene placed or pooled) to run through the initialization process.

* test

reverting the changes to WhenServerChangesSmoothValue_ValuesAreLerped as the recent update resolves the issue from the previous fix.

* update

Removing exception.
Adjusting when we update the last time.
Logging warning if log level is developer and there is no NetworkTimeSystem.

* update

remove the try catch because evidently it is the only way we can catch certain exceptions during unit testing when duplicating the value (i.e. serializing).

* test

This validates the fix for resetting the NetworkVariableBase.LastUpdated property when a NetworkObject is persisted (i.e. recycled via in-scene placed NetworkObject or object pools).

Also minor update to prevent the integration test from logging a blank entry if there has been no logs added.

* test

The first iteration should ignore the time delta as that could be impacted by the test or VM running the test.

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -14,6 +14,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 - Changed the `NetworkTimeSystem.Sync` method to use half RTT to calculate the desired local time offset as opposed to the full RTT. (#3212)
 - 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. (#3200)
+- Fixed issue where `NetworkVariableBase` derived classes were not being re-initialized if the associated `NetworkObject` instance was not destroyed and re-spawned. (#3181)
 
 ### Changed
 

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

@@ -815,6 +815,13 @@ internal void InternalOnNetworkDespawn()
             {
                 Debug.LogException(e);
             }
+
+            // Deinitialize all NetworkVariables in the event the associated
+            // NetworkObject is recylced (in-scene placed or pooled).
+            for (int i = 0; i < NetworkVariableFields.Count; i++)
+            {
+                NetworkVariableFields[i].Deinitialize();
+            }
         }
 
         /// <summary>
@@ -918,10 +925,30 @@ internal void __nameNetworkVariable(NetworkVariableBase variable, string varName
             variable.Name = varName;
         }
 
+        /// <summary>
+        /// Does a first pass initialization for RPCs and NetworkVariables
+        /// If already initialized, then it just re-initializes the NetworkVariables.
+        /// </summary>
         internal void InitializeVariables()
         {
             if (m_VarInit)
             {
+                // If the primary initialization has already been done, then go ahead
+                // and re-initialize each NetworkVariable in the event it is an in-scene
+                // placed NetworkObject in an already loaded scene that has already been
+                // used within a network session =or= if this is a pooled NetworkObject
+                // that is being repurposed.
+                for (int i = 0; i < NetworkVariableFields.Count; i++)
+                {
+                    // If already initialized, then skip
+                    if (NetworkVariableFields[i].HasBeenInitialized)
+                    {
+                        continue;
+                    }
+                    NetworkVariableFields[i].Initialize(this);
+                }
+                // Exit early as we don't need to run through the rest of this initialization
+                // process
                 return;
             }
 

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

@@ -35,6 +35,12 @@ public abstract class NetworkVariableBase : IDisposable
 
         private NetworkManager m_InternalNetworkManager;
 
+        // Determines if this NetworkVariable has been "initialized" to prevent initializing more than once which can happen when first
+        // instantiated and spawned. If this NetworkVariable instance is on an in-scene placed NetworkObject =or= a pooled NetworkObject
+        // that can persist between sessions and/or be recycled we need to reset the LastUpdateSent value prior to spawning otherwise
+        // this NetworkVariableBase property instance will not update until the last session time used.
+        internal bool HasBeenInitialized { get; private set; }
+
         internal string GetWritePermissionError()
         {
             return $"|Client-{m_NetworkManager.LocalClientId}|{m_NetworkBehaviour.name}|{Name}| Write permissions ({WritePerm}) for this client instance is not allowed!";
@@ -45,17 +51,7 @@ internal void LogWritePermissionError()
             Debug.LogError(GetWritePermissionError());
         }
 
-        private protected NetworkManager m_NetworkManager
-        {
-            get
-            {
-                if (m_InternalNetworkManager == null && m_NetworkBehaviour && m_NetworkBehaviour.NetworkObject?.NetworkManager)
-                {
-                    m_InternalNetworkManager = m_NetworkBehaviour.NetworkObject?.NetworkManager;
-                }
-                return m_InternalNetworkManager;
-            }
-        }
+        private protected NetworkManager m_NetworkManager => m_InternalNetworkManager;
 
         public NetworkBehaviour GetBehaviour()
         {
@@ -68,21 +64,77 @@ public NetworkBehaviour GetBehaviour()
         /// <param name="networkBehaviour">The NetworkBehaviour the NetworkVariable belongs to</param>
         public void Initialize(NetworkBehaviour networkBehaviour)
         {
-            m_InternalNetworkManager = null;
+            // If we have already been initialized, then exit early.
+            // This can happen on the very first instantiation and spawning of the associated NetworkObject
+            if (HasBeenInitialized)
+            {
+                return;
+            }
+
+            // Throw an exception if there is an invalid NetworkBehaviour parameter
+            if (!networkBehaviour)
+            {
+                throw new Exception($"[{GetType().Name}][Initialize] {nameof(NetworkBehaviour)} parameter passed in is null!");
+            }
             m_NetworkBehaviour = networkBehaviour;
-            if (m_NetworkBehaviour && m_NetworkBehaviour.NetworkObject?.NetworkManager)
+
+            // Throw an exception if there is no NetworkManager available
+            if (!m_NetworkBehaviour.NetworkManager)
             {
-                m_InternalNetworkManager = m_NetworkBehaviour.NetworkObject?.NetworkManager;
-                // When in distributed authority mode, there is no such thing as server write permissions
-                InternalWritePerm = m_InternalNetworkManager.DistributedAuthorityMode ? NetworkVariableWritePermission.Owner : InternalWritePerm;
+                // Exit early if there has yet to be a NetworkManager assigned.
+                // This is ok because Initialize is invoked multiple times until
+                // it is considered "initialized".
+                return;
+            }
 
-                if (m_NetworkBehaviour.NetworkManager.NetworkTimeSystem != null)
-                {
-                    UpdateLastSentTime();
-                }
+            if (!m_NetworkBehaviour.NetworkObject)
+            {
+                // Exit early if there has yet to be a NetworkObject assigned.
+                // This is ok because Initialize is invoked multiple times until
+                // it is considered "initialized".
+                return;
+            }
+
+            if (!m_NetworkBehaviour.NetworkObject.NetworkManagerOwner)
+            {
+                // Exit early if there has yet to be a NetworkManagerOwner assigned
+                // to the NetworkObject. This is ok because Initialize is invoked 
+                // multiple times until it is considered "initialized".
+                return;
             }
+            m_InternalNetworkManager = m_NetworkBehaviour.NetworkObject.NetworkManagerOwner;
+
+            // When in distributed authority mode, there is no such thing as server write permissions
+            InternalWritePerm = m_InternalNetworkManager.DistributedAuthorityMode ? NetworkVariableWritePermission.Owner : InternalWritePerm;
 
             OnInitialize();
+
+            // Some unit tests don't operate with a running NetworkManager.
+            // Only update the last time if there is a NetworkTimeSystem.
+            if (m_InternalNetworkManager.NetworkTimeSystem != null)
+            {
+                // Update our last sent time relative to when this was initialized
+                UpdateLastSentTime();
+
+                // At this point, this instance is considered initialized
+                HasBeenInitialized = true;
+            }
+            else if (m_InternalNetworkManager.LogLevel == LogLevel.Developer)
+            {
+                Debug.LogWarning($"[{m_NetworkBehaviour.name}][{m_NetworkBehaviour.GetType().Name}][{GetType().Name}][Initialize] {nameof(NetworkManager)} has no {nameof(NetworkTimeSystem)} assigned!");
+            }
+        }
+
+        /// <summary>
+        /// Deinitialize is invoked when a NetworkObject is despawned.
+        /// This allows for a recyled NetworkObject (in-scene or pooled)
+        /// to be properly initialized upon the next use/spawn.
+        /// </summary>
+        internal void Deinitialize()
+        {
+            // When despawned, reset the HasBeenInitialized so if the associated NetworkObject instance
+            // is recylced (i.e. in-scene placed or pooled) it will re-initialize the LastUpdateSent time.
+            HasBeenInitialized = false;
         }
 
         /// <summary>

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

@@ -1821,8 +1821,12 @@ private void UnloadRemainingScenes()
 
         private void LogWaitForMessages()
         {
-            VerboseDebug(m_WaitForLog.ToString());
-            m_WaitForLog.Clear();
+            // If there is nothing to log, then don't log anything
+            if (m_WaitForLog.Length > 0)
+            {
+                VerboseDebug(m_WaitForLog.ToString());
+                m_WaitForLog.Clear();
+            }
         }
 
         private IEnumerator WaitForTickAndFrames(NetworkManager networkManager, int tickCount, float targetFrames)