chore: Optimize NetworkVariable property accessors (#3689)

## Purpose of this PR

Optimize property accessors around `NetworkVariable`

1. Keep a self reference to `m_NetworkBehaviour`, `m_NetworkObject` and
`m_NetworkManager`
2. Swap `CanWrite` and `CannotWrite` to aggressively inlined functions
3. Store a `m_UseServerTime` boolean that can be calculated once rather
than on every check on the NetworkVar
4. Add early returns in the `NetowrkVariableDeltaMessage` to reduce
indentation and increase readability.

### Jira ticket

Needs a JIRA ticket

### Changelog

- Changed: Improved performance of NetworkVariable property accessors.

## Documentation

- No documentation changes or additions were necessary.

## Testing & QA (How your changes can be verified during release
Playtest)

These changes are less risky and should be covered by the automated
testing.

### Functional Testing
[//]: # (If checked, List manual tests that have been performed.)
_Manual testing :_
- [ ] `Manual testing done`

_Automated tests:_
- [x] `Covered by existing automated tests`
- [ ] `Covered by new automated tests`

_Does the change require QA team to:_

- [ ] `Review automated tests`?
- [ ] `Execute manual tests`?
- [ ] `Provide feedback about the PR`?

If any boxes above are checked the QA team will be automatically added
as a PR reviewer.

## Backports

This is a performance improvement so backporting this fix isn't
necessary.

Author: EmandM

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -15,6 +15,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Changed
 
+- Improved performance of the NetworkVariable. (#3683)
 - Improved performance around the NetworkBehaviour component. (#3687)
 
 ### Deprecated

com.unity.netcode.gameobjects/Runtime/Messaging/Messages/NetworkVariableDeltaMessage.cs

@@ -217,7 +217,7 @@ public void Anticipate(T value)
             m_LastAnticipationCounter = m_NetworkBehaviour.NetworkManager.AnticipationSystem.AnticipationCounter;
             m_AnticipatedValue = value;
             NetworkVariableSerialization<T>.Duplicate(m_AnticipatedValue, ref m_PreviousAnticipatedValue);
-            if (CanWrite)
+            if (CanWrite())
             {
                 AuthoritativeValue = value;
             }

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

@@ -429,7 +429,7 @@ public IEnumerator<T> GetEnumerator()
         public void Add(T item)
         {
             // check write permissions
-            if (CannotWrite)
+            if (CannotWrite())
             {
                 LogWritePermissionError();
                 return;
@@ -456,7 +456,7 @@ public void Add(T item)
         public void Clear()
         {
             // check write permissions
-            if (CannotWrite)
+            if (CannotWrite())
             {
                 LogWritePermissionError();
                 return;
@@ -494,7 +494,7 @@ public bool Contains(T item)
         public bool Remove(T item)
         {
             // check write permissions
-            if (CannotWrite)
+            if (CannotWrite())
             {
                 LogWritePermissionError();
                 return false;
@@ -543,7 +543,7 @@ public int IndexOf(T item)
         public void Insert(int index, T item)
         {
             // check write permissions
-            if (CannotWrite)
+            if (CannotWrite())
             {
                 LogWritePermissionError();
                 return;
@@ -579,7 +579,7 @@ public void Insert(int index, T item)
         public void RemoveAt(int index)
         {
             // check write permissions
-            if (CannotWrite)
+            if (CannotWrite())
             {
                 throw new InvalidOperationException("Client is not allowed to write to this NetworkList");
             }
@@ -613,7 +613,7 @@ public void RemoveAt(int index)
         public void Set(int index, T value, bool forceUpdate = false)
         {
             // check write permissions
-            if (CannotWrite)
+            if (CannotWrite())
             {
                 LogWritePermissionError();
                 return;

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

@@ -89,7 +89,7 @@ public NetworkVariable(T value = default,
         /// <param name="value">the value to reset the NetworkVariable to (if none specified it resets to the default)</param>
         public void Reset(T value = default)
         {
-            if (m_NetworkBehaviour == null || m_NetworkBehaviour != null && !m_NetworkBehaviour.NetworkObject.IsSpawned)
+            if (m_NetworkBehaviour == null || m_NetworkObject == null || !m_NetworkObject.IsSpawned)
             {
                 m_InternalValue = value;
                 NetworkVariableSerialization<T>.Duplicate(m_InternalValue, ref m_LastInternalValue);
@@ -148,7 +148,7 @@ public virtual T Value
             get => m_InternalValue;
             set
             {
-                if (CannotWrite)
+                if (CannotWrite())
                 {
                     LogWritePermissionError();
                     return;
@@ -182,7 +182,7 @@ public bool CheckDirtyState(bool forceCheck = false)
             var isDirty = base.IsDirty();
 
             // A client without permissions invoking this method should only check to assure the current value is equal to the last known current value
-            if (CannotWrite)
+            if (CannotWrite())
             {
                 // If modifications are detected, then revert back to the last known current value
                 if (!NetworkVariableSerialization<T>.AreEqual(ref m_InternalValue, ref m_LastInternalValue))
@@ -266,7 +266,7 @@ public override bool IsDirty()
         {
             // If the client does not have write permissions but the internal value is determined to be locally modified and we are applying updates, then we should revert
             // to the original collection value prior to applying updates (primarily for collections).
-            if (!NetworkUpdaterCheck && CannotWrite && !NetworkVariableSerialization<T>.AreEqual(ref m_InternalValue, ref m_LastInternalValue))
+            if (!NetworkUpdaterCheck && CannotWrite() && !NetworkVariableSerialization<T>.AreEqual(ref m_InternalValue, ref m_LastInternalValue))
             {
                 NetworkVariableSerialization<T>.Duplicate(m_LastInternalValue, ref m_InternalValue);
                 return true;
@@ -328,7 +328,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta)
         {
             // If the client does not have write permissions but the internal value is determined to be locally modified and we are applying updates, then we should revert
             // to the original collection value prior to applying updates (primarily for collections).
-            if (CannotWrite && !NetworkVariableSerialization<T>.AreEqual(ref m_LastInternalValue, ref m_InternalValue))
+            if (CannotWrite() && !NetworkVariableSerialization<T>.AreEqual(ref m_LastInternalValue, ref m_InternalValue))
             {
                 NetworkVariableSerialization<T>.Duplicate(m_LastInternalValue, ref m_InternalValue);
             }
@@ -370,7 +370,7 @@ public override void ReadField(FastBufferReader reader)
         {
             // If the client does not have write permissions but the internal value is determined to be locally modified and we are applying updates, then we should revert
             // to the original collection value prior to applying updates (primarily for collections).
-            if (CannotWrite && !NetworkVariableSerialization<T>.AreEqual(ref m_LastInternalValue, ref m_InternalValue))
+            if (CannotWrite() && !NetworkVariableSerialization<T>.AreEqual(ref m_LastInternalValue, ref m_InternalValue))
             {
                 NetworkVariableSerialization<T>.Duplicate(m_LastInternalValue, ref m_InternalValue);
             }

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

@@ -1,4 +1,5 @@
 using System;
+using System.Runtime.CompilerServices;
 using UnityEngine;
 
 namespace Unity.Netcode
@@ -42,7 +43,11 @@ public abstract class NetworkVariableBase : IDisposable
         /// </summary>
         private protected NetworkBehaviour m_NetworkBehaviour;
 
-        private NetworkManager m_InternalNetworkManager;
+        private protected NetworkManager m_NetworkManager;
+
+        private protected NetworkObject m_NetworkObject;
+
+        private bool m_UseServerTime;
 
         // 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
@@ -60,8 +65,6 @@ internal void LogWritePermissionError()
             Debug.LogError(GetWritePermissionError());
         }
 
-        private protected NetworkManager m_NetworkManager => m_InternalNetworkManager;
-
         /// <summary>
         /// Gets the NetworkBehaviour instance associated with this network variable
         /// </summary>
@@ -108,31 +111,35 @@ public void Initialize(NetworkBehaviour networkBehaviour)
                 return;
             }
 
-            if (!m_NetworkBehaviour.NetworkObject.NetworkManagerOwner)
+            m_NetworkObject = m_NetworkBehaviour.NetworkObject;
+
+            if (!m_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;
+            m_NetworkManager = m_NetworkObject.NetworkManagerOwner;
+
+            m_UseServerTime = m_NetworkManager.CMBServiceConnection || !m_NetworkManager.IsServer;
 
             // When in distributed authority mode, there is no such thing as server write permissions
-            InternalWritePerm = m_InternalNetworkManager.DistributedAuthorityMode ? NetworkVariableWritePermission.Owner : InternalWritePerm;
+            InternalWritePerm = m_NetworkManager.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)
+            if (m_NetworkManager.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)
+            else if (m_NetworkManager.LogLevel == LogLevel.Developer)
             {
                 Debug.LogWarning($"[{m_NetworkBehaviour.name}][{m_NetworkBehaviour.GetType().Name}][{GetType().Name}][Initialize] {nameof(NetworkManager)} has no {nameof(NetworkTimeSystem)} assigned!");
             }
@@ -251,7 +258,7 @@ public virtual void SetDirty(bool isDirty)
         internal bool CanSend()
         {
             // When connected to a service or not the server, always use the synchronized server time as opposed to the local time
-            var time = m_InternalNetworkManager.CMBServiceConnection || !m_InternalNetworkManager.IsServer ? m_NetworkBehaviour.NetworkManager.ServerTime.Time : m_NetworkBehaviour.NetworkManager.NetworkTimeSystem.LocalTime;
+            var time = m_UseServerTime ? m_NetworkManager.ServerTime.Time : m_NetworkManager.NetworkTimeSystem.LocalTime;
             var timeSinceLastUpdate = time - LastUpdateSent;
             return
                 (
@@ -267,7 +274,7 @@ internal bool CanSend()
         internal void UpdateLastSentTime()
         {
             // When connected to a service or not the server, always use the synchronized server time as opposed to the local time
-            LastUpdateSent = m_InternalNetworkManager.CMBServiceConnection || !m_InternalNetworkManager.IsServer ? m_NetworkBehaviour.NetworkManager.ServerTime.Time : m_NetworkBehaviour.NetworkManager.NetworkTimeSystem.LocalTime;
+            LastUpdateSent = m_UseServerTime ? m_NetworkManager.ServerTime.Time : m_NetworkManager.NetworkTimeSystem.LocalTime;
         }
 
         internal static bool IgnoreInitializeWarning;
@@ -286,27 +293,27 @@ protected void MarkNetworkBehaviourDirty()
                 }
                 return;
             }
-            if (m_NetworkBehaviour.NetworkManager.ShutdownInProgress)
+            if (m_NetworkManager.ShutdownInProgress)
             {
-                if (m_NetworkBehaviour.NetworkManager.LogLevel <= LogLevel.Developer)
+                if (m_NetworkManager.LogLevel <= LogLevel.Developer)
                 {
                     Debug.LogWarning($"NetworkVariable is written to during the NetworkManager shutdown! " +
                  "Are you modifying a NetworkVariable within a NetworkBehaviour.OnDestroy or NetworkBehaviour.OnDespawn method?");
                 }
                 return;
             }
 
-            if (!m_NetworkBehaviour.NetworkManager.IsListening)
+            if (!m_NetworkManager.IsListening)
             {
-                if (m_NetworkBehaviour.NetworkManager.LogLevel <= LogLevel.Developer)
+                if (m_NetworkManager.LogLevel <= LogLevel.Developer)
                 {
                     Debug.LogWarning($"NetworkVariable is written to after the NetworkManager has already shutdown! " +
                      "Are you modifying a NetworkVariable within a NetworkBehaviour.OnDestroy or NetworkBehaviour.OnDespawn method?");
                 }
                 return;
             }
 
-            m_NetworkBehaviour.NetworkManager.BehaviourUpdater?.AddForUpdate(m_NetworkBehaviour.NetworkObject);
+            m_NetworkManager.BehaviourUpdater?.AddForUpdate(m_NetworkObject);
         }
 
         /// <summary>
@@ -345,7 +352,7 @@ public bool CanClientRead(ulong clientId)
             }
 
             // When in distributed authority mode, everyone can read (but only the owner can write)
-            if (m_NetworkManager != null && m_NetworkManager.DistributedAuthorityMode)
+            if (m_NetworkManager.DistributedAuthorityMode)
             {
                 return true;
             }
@@ -355,7 +362,7 @@ public bool CanClientRead(ulong clientId)
                 case NetworkVariableReadPermission.Everyone:
                     return true;
                 case NetworkVariableReadPermission.Owner:
-                    return clientId == m_NetworkBehaviour.NetworkObject.OwnerClientId || NetworkManager.ServerClientId == clientId;
+                    return clientId == m_NetworkObject.OwnerClientId || NetworkManager.ServerClientId == clientId;
             }
         }
 
@@ -377,26 +384,26 @@ public bool CanClientWrite(ulong clientId)
                 case NetworkVariableWritePermission.Server:
                     return clientId == NetworkManager.ServerClientId;
                 case NetworkVariableWritePermission.Owner:
-                    return clientId == m_NetworkBehaviour.NetworkObject.OwnerClientId;
+                    return clientId == m_NetworkObject.OwnerClientId;
             }
         }
 
         /// <summary>
         /// Returns true if the current <see cref="NetworkManager.LocalClientId"/> can write to this variable; otherwise false.
         /// </summary>
-        internal bool CanWrite => m_NetworkManager && CanClientWrite(m_NetworkManager.LocalClientId);
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        internal bool CanWrite()
+        {
+            return m_NetworkManager && CanClientWrite(m_NetworkManager.LocalClientId);
+        }
 
         /// <summary>
         /// Returns false if the current <see cref="NetworkManager.LocalClientId"/> can write to this variable; otherwise true.
         /// </summary>
-        internal bool CannotWrite => m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId);
-
-        /// <summary>
-        /// Returns the ClientId of the owning client
-        /// </summary>
-        internal ulong OwnerClientId()
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        internal bool CannotWrite()
         {
-            return m_NetworkBehaviour.NetworkObject.OwnerClientId;
+            return m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId);
         }
 
         /// <summary>
@@ -467,7 +474,10 @@ internal virtual void WriteFieldSynchronization(FastBufferWriter writer)
         /// </summary>
         public virtual void Dispose()
         {
-            m_InternalNetworkManager = null;
+            HasBeenInitialized = false;
+            m_NetworkBehaviour = null;
+            m_NetworkObject = null;
+            m_NetworkManager = null;
         }
     }
 }