[email protected]

The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). Additional documentation and release notes are available at [Multiplayer Documentation](https://docs-multiplayer.unity3d.com). ## [1.11.0] - 2024-08-20 ### Added - Added `NetworkVariable.CheckDirtyState` that is to be used in tandem with collections in order to detect whether the collection or an item within the collection has changed. (#3005) ### Fixed - Fixed issue by adding null checks in `NetworkVariableBase.CanClientRead` and `NetworkVariableBase.CanClientWrite` methods to ensure safe access to `NetworkBehaviour`. (#3011) - Fixed issue using collections within `NetworkVariable` where the collection would not detect changes to items or nested items. (#3005) - Fixed issue where `List`, `Dictionary`, and `HashSet` collections would not uniquely duplicate nested collections. (#3005) - Fixed Issue where a state with dual triggers, inbound and outbound, could cause a false layer to layer state transition message to be sent to non-authority `NetworkAnimator` instances and cause a warning message to be logged. (#2999) - Fixed issue where `FixedStringSerializer<T>` was using `NetworkVariableSerialization<byte>.AreEqual` to determine if two bytes were equal causes an exception to be thrown due to no byte serializer having been defined. (#2992) ### Changed - Changed permissions exception thrown in `NetworkList` to exiting early with a logged error that is now a unified permissions message within `NetworkVariableBase`. (#3005) - Changed permissions exception thrown in `NetworkVariable.Value` to exiting early with a logged error that is now a unified permissions message within `NetworkVariableBase`. (#3005)
needle-mirror · Aug 20, 2024 · c2664df · c2664df
1 parent 896943c
commit c2664df
Show file tree

Hide file tree

Showing 13 changed files with 3,179 additions and 94 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,27 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 Additional documentation and release notes are available at [Multiplayer Documentation](https://docs-multiplayer.unity3d.com).
+
+## [1.11.0] - 2024-08-20
+
+### Added
+
+- Added `NetworkVariable.CheckDirtyState` that is to be used in tandem with collections in order to detect whether the collection or an item within the collection has changed. (#3005)
+
+### Fixed
+
+- Fixed issue by adding null checks in `NetworkVariableBase.CanClientRead` and `NetworkVariableBase.CanClientWrite` methods to ensure safe access to `NetworkBehaviour`. (#3011)
+- Fixed issue using collections within `NetworkVariable` where the collection would not detect changes to items or nested items. (#3005)
+- Fixed issue where `List`, `Dictionary`, and `HashSet` collections would not uniquely duplicate nested collections. (#3005)
+- Fixed Issue where a state with dual triggers, inbound and outbound, could cause a false layer to layer state transition message to be sent to non-authority `NetworkAnimator` instances and cause a warning message to be logged. (#2999)
+- Fixed issue where `FixedStringSerializer<T>` was using `NetworkVariableSerialization<byte>.AreEqual` to determine if two bytes were equal causes an exception to be thrown due to no byte serializer having been defined. (#2992)
+
+### Changed
+
+- Changed permissions exception thrown in `NetworkList` to exiting early with a logged error that is now a unified permissions message within `NetworkVariableBase`. (#3005)
+- Changed permissions exception thrown in `NetworkVariable.Value` to exiting early with a logged error that is now a unified permissions message within `NetworkVariableBase`. (#3005)
+
+
 ## [1.10.0] - 2024-07-22
 
 ### Added

diff --git a/Components/NetworkAnimator.cs b/Components/NetworkAnimator.cs
@@ -832,7 +832,12 @@ private void CheckForStateChange(int layer)
                     stateChangeDetected = true;
                     //Debug.Log($"[Cross-Fade] To-Hash: {nt.fullPathHash} | TI-Duration: ({tt.duration}) | TI-Norm: ({tt.normalizedTime}) | From-Hash: ({m_AnimationHash[layer]}) | SI-FPHash: ({st.fullPathHash}) | SI-Norm: ({st.normalizedTime})");
                 }
-                else if (!tt.anyState && tt.fullPathHash != m_TransitionHash[layer])
+                // If we are not transitioned into the "any state" and the animator transition isn't a full path hash (layer to layer) and our pre-built destination state to transition does not contain the
+                // current layer (i.e. transitioning into a state from another layer) =or= we do contain the layer and the layer contains state to transition to is contained within our pre-built destination
+                // state then we can handle this transition as a non-cross fade state transition between layers.
+                // Otherwise, if we don't enter into this then this is a "trigger transition to some state that is now being transitioned back to the Idle state via trigger" or "Dual Triggers" IDLE<-->State.
+                else if (!tt.anyState && tt.fullPathHash != m_TransitionHash[layer] && (!m_DestinationStateToTransitioninfo.ContainsKey(layer) ||
+                    (m_DestinationStateToTransitioninfo.ContainsKey(layer) && m_DestinationStateToTransitioninfo[layer].ContainsKey(nt.fullPathHash))))
                 {
                     // first time in this transition for this layer
                     m_TransitionHash[layer] = tt.fullPathHash;
@@ -841,6 +846,10 @@ private void CheckForStateChange(int layer)
                     animState.CrossFade = false;
                     animState.Transition = true;
                     animState.NormalizedTime = tt.normalizedTime;
+                    if (m_DestinationStateToTransitioninfo.ContainsKey(layer) && m_DestinationStateToTransitioninfo[layer].ContainsKey(nt.fullPathHash))
+                    {
+                        animState.DestinationStateHash = nt.fullPathHash;
+                    }
                     stateChangeDetected = true;
                     //Debug.Log($"[Transition] TI-Duration: ({tt.duration}) | TI-Norm: ({tt.normalizedTime}) | From-Hash: ({m_AnimationHash[layer]}) |SI-FPHash: ({st.fullPathHash}) | SI-Norm: ({st.normalizedTime})");
                 }

diff --git a/Runtime/Core/NetworkBehaviour.cs b/Runtime/Core/NetworkBehaviour.cs
@@ -758,7 +758,7 @@ internal void InternalOnNetworkDespawn()
         }
 
         /// <summary>
-        /// Gets called when the local client gains ownership of this object
+        /// Invoked on both the server and the local client of the owner when <see cref="Netcode.NetworkObject"/> ownership is assigned.
         /// </summary>
         public virtual void OnGainedOwnership() { }
 
@@ -786,7 +786,8 @@ internal void InternalOnOwnershipChanged(ulong previous, ulong current)
         }
 
         /// <summary>
-        /// Gets called when we loose ownership of this object
+        /// Invoked on the local client when it loses ownership of the associated <see cref="Netcode.NetworkObject"/>.
+        /// This method is also invoked on the server when any client loses ownership.
         /// </summary>
         public virtual void OnLostOwnership() { }
 

diff --git a/Runtime/NetworkVariable/AnticipatedNetworkVariable.cs b/Runtime/NetworkVariable/AnticipatedNetworkVariable.cs
@@ -51,7 +51,6 @@ public enum StaleDataHandling
 #pragma warning restore IDE0001
     [Serializable]
     [GenerateSerializationForGenericParameter(0)]
-    [GenerateSerializationForType(typeof(byte))]
     public class AnticipatedNetworkVariable<T> : NetworkVariableBase
     {
         [SerializeField]

diff --git a/Runtime/NetworkVariable/Collections/NetworkList.cs b/Runtime/NetworkVariable/Collections/NetworkList.cs
@@ -369,7 +369,8 @@ public void Add(T item)
             // check write permissions
             if (!CanClientWrite(m_NetworkBehaviour.NetworkManager.LocalClientId))
             {
-                throw new InvalidOperationException("Client is not allowed to write to this NetworkList");
+                LogWritePermissionError();
+                return;
             }
 
             m_List.Add(item);
@@ -390,7 +391,8 @@ public void Clear()
             // check write permissions
             if (!CanClientWrite(m_NetworkBehaviour.NetworkManager.LocalClientId))
             {
-                throw new InvalidOperationException("Client is not allowed to write to this NetworkList");
+                LogWritePermissionError();
+                return;
             }
 
             m_List.Clear();
@@ -416,7 +418,8 @@ public bool Remove(T item)
             // check write permissions
             if (!CanClientWrite(m_NetworkBehaviour.NetworkManager.LocalClientId))
             {
-                throw new InvalidOperationException("Client is not allowed to write to this NetworkList");
+                LogWritePermissionError();
+                return false;
             }
 
             int index = m_List.IndexOf(item);
@@ -451,7 +454,8 @@ public void Insert(int index, T item)
             // check write permissions
             if (!CanClientWrite(m_NetworkBehaviour.NetworkManager.LocalClientId))
             {
-                throw new InvalidOperationException("Client is not allowed to write to this NetworkList");
+                LogWritePermissionError();
+                return;
             }
 
             if (index < m_List.Length)
@@ -480,7 +484,8 @@ public void RemoveAt(int index)
             // check write permissions
             if (!CanClientWrite(m_NetworkBehaviour.NetworkManager.LocalClientId))
             {
-                throw new InvalidOperationException("Client is not allowed to write to this NetworkList");
+                LogWritePermissionError();
+                return;
             }
 
             var value = m_List[index];
@@ -505,7 +510,8 @@ public T this[int index]
                 // check write permissions
                 if (!CanClientWrite(m_NetworkBehaviour.NetworkManager.LocalClientId))
                 {
-                    throw new InvalidOperationException("Client is not allowed to write to this NetworkList");
+                    LogWritePermissionError();
+                    return;
                 }
 
                 var previousValue = m_List[index];

diff --git a/Runtime/NetworkVariable/NetworkVariable.cs b/Runtime/NetworkVariable/NetworkVariable.cs
@@ -9,7 +9,6 @@ namespace Unity.Netcode
     /// <typeparam name="T">the unmanaged type for <see cref="NetworkVariable{T}"/> </typeparam>
     [Serializable]
     [GenerateSerializationForGenericParameter(0)]
-    [GenerateSerializationForType(typeof(byte))]
     public class NetworkVariable<T> : NetworkVariableBase
     {
         /// <summary>
@@ -80,25 +79,57 @@ public NetworkVariable(T value = default,
         /// <summary>
         /// The value of the NetworkVariable container
         /// </summary>
+        /// <remarks>
+        /// When assigning collections to <see cref="Value"/>, unless it is a completely new collection this will not
+        /// detect any deltas with most managed collection classes since assignment of one collection value to another
+        /// is actually just a reference to the collection itself. <br />
+        /// To detect deltas in a collection, you should invoke <see cref="CheckDirtyState"/> after making modifications to the collection.
+        /// </remarks>
         public virtual T Value
         {
             get => m_InternalValue;
             set
             {
-                // Compare bitwise
-                if (NetworkVariableSerialization<T>.AreEqual(ref m_InternalValue, ref value))
+                if (m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId))
                 {
+                    LogWritePermissionError();
                     return;
                 }
 
-                if (m_NetworkBehaviour && !CanClientWrite(m_NetworkBehaviour.NetworkManager.LocalClientId))
+                // Compare the Value being applied to the current value
+                if (!NetworkVariableSerialization<T>.AreEqual(ref m_InternalValue, ref value))
                 {
-                    throw new InvalidOperationException("Client is not allowed to write to this NetworkVariable");
+                    T previousValue = m_InternalValue;
+                    m_InternalValue = value;
+                    SetDirty(true);
+                    m_IsDisposed = false;
+                    OnValueChanged?.Invoke(previousValue, m_InternalValue);
                 }
+            }
+        }
+
+        /// <summary>
+        /// Invoke this method to check if a collection's items are dirty.
+        /// The default behavior is to exit early if the <see cref="NetworkVariable{T}"/> is already dirty.
+        /// </summary>
+        /// <param name="forceCheck"> when true, this check will force a full item collection check even if the NetworkVariable is already dirty</param>
+        /// <remarks>
+        /// This is to be used as a way to check if a <see cref="NetworkVariable{T}"/> containing a managed collection has any changees to the collection items.<br />
+        /// If you invoked this when a collection is dirty, it will not trigger the <see cref="OnValueChanged"/> unless you set <param name="forceCheck"/> to true. <br />
+        /// </remarks>
+        public bool CheckDirtyState(bool forceCheck = false)
+        {
+            var isDirty = base.IsDirty();
 
-                Set(value);
+            // Compare the previous with the current if not dirty or forcing a check.
+            if ((!isDirty || forceCheck) && !NetworkVariableSerialization<T>.AreEqual(ref m_PreviousValue, ref m_InternalValue))
+            {
+                SetDirty(true);
+                OnValueChanged?.Invoke(m_PreviousValue, m_InternalValue);
                 m_IsDisposed = false;
+                isDirty = true;
             }
+            return isDirty;
         }
 
         internal ref T RefValue()
@@ -185,9 +216,8 @@ public override void ResetDirty()
         private protected void Set(T value)
         {
             SetDirty(true);
-            T previousValue = m_InternalValue;
             m_InternalValue = value;
-            OnValueChanged?.Invoke(previousValue, m_InternalValue);
+            OnValueChanged?.Invoke(m_PreviousValue, m_InternalValue);
         }
 
         /// <summary>
@@ -206,20 +236,22 @@ public override void WriteDelta(FastBufferWriter writer)
         /// <param name="keepDirtyDelta">Whether or not the container should keep the dirty delta, or mark the delta as consumed</param>
         public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta)
         {
+            // In order to get managed collections to properly have a previous and current value, we have to
+            // duplicate the collection at this point before making any modifications to the current.
+            m_HasPreviousValue = true;
+            NetworkVariableSerialization<T>.Duplicate(m_InternalValue, ref m_PreviousValue);
+            NetworkVariableSerialization<T>.ReadDelta(reader, ref m_InternalValue);
+
             // todo:
             // keepDirtyDelta marks a variable received as dirty and causes the server to send the value to clients
             // In a prefect world, whether a variable was A) modified locally or B) received and needs retransmit
             // would be stored in different fields
-
-            T previousValue = m_InternalValue;
-            NetworkVariableSerialization<T>.ReadDelta(reader, ref m_InternalValue);
-
             if (keepDirtyDelta)
             {
                 SetDirty(true);
             }
 
-            OnValueChanged?.Invoke(previousValue, m_InternalValue);
+            OnValueChanged?.Invoke(m_PreviousValue, m_InternalValue);
         }
 
         /// <inheritdoc />

diff --git a/Runtime/NetworkVariable/NetworkVariableBase.cs b/Runtime/NetworkVariable/NetworkVariableBase.cs
@@ -32,21 +32,47 @@ public abstract class NetworkVariableBase : IDisposable
         /// Maintains a link to the associated NetworkBehaviour
         /// </summary>
         private protected NetworkBehaviour m_NetworkBehaviour;
+        private NetworkManager m_InternalNetworkManager;
 
         public NetworkBehaviour GetBehaviour()
         {
             return m_NetworkBehaviour;
         }
 
+        internal string GetWritePermissionError()
+        {
+            return $"|Client-{m_NetworkManager.LocalClientId}|{m_NetworkBehaviour.name}|{Name}| Write permissions ({WritePerm}) for this client instance is not allowed!";
+        }
+
+        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;
+            }
+        }
+
         /// <summary>
         /// Initializes the NetworkVariable
         /// </summary>
         /// <param name="networkBehaviour">The NetworkBehaviour the NetworkVariable belongs to</param>
         public void Initialize(NetworkBehaviour networkBehaviour)
         {
+            m_InternalNetworkManager = null;
             m_NetworkBehaviour = networkBehaviour;
-            if (m_NetworkBehaviour.NetworkManager)
+            if (m_NetworkBehaviour && m_NetworkBehaviour.NetworkObject?.NetworkManager)
             {
+                m_InternalNetworkManager = m_NetworkBehaviour.NetworkObject?.NetworkManager;
+
                 if (m_NetworkBehaviour.NetworkManager.NetworkTimeSystem != null)
                 {
                     UpdateLastSentTime();
@@ -217,6 +243,11 @@ public virtual bool IsDirty()
         /// <returns>Whether or not the client has permission to read</returns>
         public bool CanClientRead(ulong clientId)
         {
+            if (!m_NetworkBehaviour)
+            {
+                return false;
+            }
+
             switch (ReadPerm)
             {
                 default:
@@ -234,6 +265,11 @@ public bool CanClientRead(ulong clientId)
         /// <returns>Whether or not the client has permission to write</returns>
         public bool CanClientWrite(ulong clientId)
         {
+            if (!m_NetworkBehaviour)
+            {
+                return false;
+            }
+
             switch (WritePerm)
             {
                 default:

diff --git a/Runtime/NetworkVariable/NetworkVariableSerialization.cs b/Runtime/NetworkVariable/NetworkVariableSerialization.cs
@@ -349,7 +349,10 @@ public void Duplicate(in List<T> value, ref List<T> duplicatedValue)
             duplicatedValue.Clear();
             foreach (var item in value)
             {
-                duplicatedValue.Add(item);
+                // This handles the nested list scenario List<List<T>>
+                T subValue = default;
+                NetworkVariableSerialization<T>.Duplicate(item, ref subValue);
+                duplicatedValue.Add(subValue);
             }
         }
     }
@@ -421,6 +424,9 @@ public void Duplicate(in HashSet<T> value, ref HashSet<T> duplicatedValue)
             duplicatedValue.Clear();
             foreach (var item in value)
             {
+                // Handles nested HashSets
+                T subValue = default;
+                NetworkVariableSerialization<T>.Duplicate(item, ref subValue);
                 duplicatedValue.Add(item);
             }
         }
@@ -497,7 +503,12 @@ public void Duplicate(in Dictionary<TKey, TVal> value, ref Dictionary<TKey, TVal
             duplicatedValue.Clear();
             foreach (var item in value)
             {
-                duplicatedValue.Add(item.Key, item.Value);
+                // Handles nested dictionaries
+                TKey subKey = default;
+                TVal subValue = default;
+                NetworkVariableSerialization<TKey>.Duplicate(item.Key, ref subKey);
+                NetworkVariableSerialization<TVal>.Duplicate(item.Value, ref subValue);
+                duplicatedValue.Add(subKey, subValue);
             }
         }
     }
@@ -698,7 +709,7 @@ public unsafe void WriteDelta(FastBufferWriter writer, ref T value, ref T previo
             {
                 var val = value[i];
                 var prevVal = previousValue[i];
-                if (!NetworkVariableSerialization<byte>.AreEqual(ref val, ref prevVal))
+                if (val != prevVal)
                 {
                     ++numChanges;
                     changes.Set(i);
@@ -723,19 +734,11 @@ public unsafe void WriteDelta(FastBufferWriter writer, ref T value, ref T previo
             unsafe
             {
                 byte* ptr = value.GetUnsafePtr();
-                byte* prevPtr = previousValue.GetUnsafePtr();
                 for (int i = 0; i < value.Length; ++i)
                 {
                     if (changes.IsSet(i))
                     {
-                        if (i < previousValue.Length)
-                        {
-                            NetworkVariableSerialization<byte>.WriteDelta(writer, ref ptr[i], ref prevPtr[i]);
-                        }
-                        else
-                        {
-                            NetworkVariableSerialization<byte>.Write(writer, ref ptr[i]);
-                        }
+                        writer.WriteByteSafe(ptr[i]);
                     }
                 }
             }