Refactor into shared logic

Author: EmandM

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -15,14 +15,14 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed `NullReferenceException` on `NetworkList` when used without a NetworkManager in scene. (#2539)
 - Fixed inconsistencies in the `OnSceneEvent` callback. (#3487)
 - Fixed issue where `NetworkClient` could persist some settings if re-using the same `NetworkManager` instance. (#3494)
 - Fixed issue where a pooled `NetworkObject` was not resetting the internal latest parent property when despawned. (#3494)
 - Fixed issue where the initial client synchronization pre-serialization process was not excluding spawned `NetworkObjects` that already had pending visibility for the client being synchronized. (#3493)
 - Fixed issue where invoking `NetworkObject.NetworkShow` and `NetworkObject.ChangeOwnership` consecutively within the same call stack location could result in an unnecessary change in ownership error message generated on the target client side. (#3493)
 - Fixed issue where `NetworkVariable`s on a `NetworkBehaviour` could fail to synchronize changes if one has `NetworkVariableUpdateTraits` set and is dirty but is not ready to send. (#3465)
 - Fixed issue where when a client changes ownership via RPC the `NetworkBehaviour.OnOwnershipChanged` can result in identical previous and current owner identifiers. (#3434)
-- Fixed `NullReferenceException` on `NetworkList` when used without a NetworkManager in scene. (#2539)
 
 ### Changed
 

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

@@ -425,7 +425,7 @@ public IEnumerator<T> GetEnumerator()
         public void Add(T item)
         {
             // check write permissions
-            if (m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId))
+            if (LocalClientCannotWrite())
             {
                 LogWritePermissionError();
                 return;
@@ -452,7 +452,7 @@ public void Add(T item)
         public void Clear()
         {
             // check write permissions
-            if (m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId))
+            if (LocalClientCannotWrite())
             {
                 LogWritePermissionError();
                 return;
@@ -490,7 +490,7 @@ public bool Contains(T item)
         public bool Remove(T item)
         {
             // check write permissions
-            if (m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId))
+            if (LocalClientCannotWrite())
             {
                 LogWritePermissionError();
                 return false;
@@ -539,7 +539,7 @@ public int IndexOf(T item)
         public void Insert(int index, T item)
         {
             // check write permissions
-            if (m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId))
+            if (LocalClientCannotWrite())
             {
                 LogWritePermissionError();
                 return;
@@ -575,7 +575,7 @@ public void Insert(int index, T item)
         public void RemoveAt(int index)
         {
             // check write permissions
-            if (m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId))
+            if (LocalClientCannotWrite())
             {
                 LogWritePermissionError();
                 return;
@@ -608,7 +608,7 @@ public T this[int index]
             set
             {
                 // check write permissions
-                if (m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId))
+                if (LocalClientCannotWrite())
                 {
                     LogWritePermissionError();
                     return;

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

@@ -90,7 +90,7 @@ public void Reset(T value = default)
         // The introduction of standard .NET collections caused an issue with permissions since there is no way to detect changes in the
         // collection without doing a full comparison. While this approach does consume more memory per collection instance, it is the
         // lowest risk approach to resolving the issue where a client with no write permissions could make changes to a collection locally
-        // which can cause a myriad of issues. 
+        // which can cause a myriad of issues.
         private protected T m_InternalOriginalValue;
 
         private protected T m_PreviousValue;
@@ -112,7 +112,7 @@ public virtual T Value
             get => m_InternalValue;
             set
             {
-                if (m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId))
+                if (LocalClientCannotWrite())
                 {
                     LogWritePermissionError();
                     return;
@@ -145,7 +145,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 (m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId))
+            if (LocalClientCannotWrite())
             {
                 // If modifications are detected, then revert back to the last known current value
                 if (!NetworkVariableSerialization<T>.AreEqual(ref m_InternalValue, ref m_InternalOriginalValue))
@@ -221,7 +221,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 && m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId) && !NetworkVariableSerialization<T>.AreEqual(ref m_InternalValue, ref m_InternalOriginalValue))
+            if (!NetworkUpdaterCheck && LocalClientCannotWrite() && !NetworkVariableSerialization<T>.AreEqual(ref m_InternalValue, ref m_InternalOriginalValue))
             {
                 NetworkVariableSerialization<T>.Duplicate(m_InternalOriginalValue, ref m_InternalValue);
                 return true;
@@ -297,7 +297,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 (m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId) && !NetworkVariableSerialization<T>.AreEqual(ref m_InternalOriginalValue, ref m_InternalValue))
+            if (LocalCLientCannotWrite() && !NetworkVariableSerialization<T>.AreEqual(ref m_InternalOriginalValue, ref m_InternalValue))
             {
                 NetworkVariableSerialization<T>.Duplicate(m_InternalOriginalValue, ref m_InternalValue);
             }
@@ -320,7 +320,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta)
         /// This should be always invoked (client & server) to assure the previous values are set
         /// !! IMPORTANT !!
         /// When a server forwards delta updates to connected clients, it needs to preserve the previous dirty value(s)
-        /// until it is done serializing all valid NetworkVariable field deltas (relative to each client). This is invoked 
+        /// until it is done serializing all valid NetworkVariable field deltas (relative to each client). This is invoked
         /// after it is done forwarding the deltas at the end of the <see cref="NetworkVariableDeltaMessage.Handle(ref NetworkContext)"/> method.
         /// </summary>
         internal override void PostDeltaRead()
@@ -338,7 +338,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 (m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId) && !NetworkVariableSerialization<T>.AreEqual(ref m_InternalOriginalValue, ref m_InternalValue))
+            if (LocalClientCannotWrite() && !NetworkVariableSerialization<T>.AreEqual(ref m_InternalOriginalValue, ref m_InternalValue))
             {
                 NetworkVariableSerialization<T>.Duplicate(m_InternalOriginalValue, ref m_InternalValue);
             }

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

@@ -337,6 +337,11 @@ public bool CanClientWrite(ulong clientId)
             }
         }
 
+        internal bool LocalClientCannotWrite()
+        {
+            return m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId);
+        }
+
         /// <summary>
         /// Returns the ClientId of the owning client
         /// </summary>