fix: Moving tick writing out of NetworkVariable and into NetworkBehav… (#590)

* fix: Moving tick writing out of NetworkVariable and into NetworkBehaviour. Allows proper handling of INetworkVariable that are not plain NetworkVariable. Forces extension of the interface. issues/573, MTT-532

* test[NetworkList]: Adding manual test for NetworkList

* test[NetworkList]: Adding manual test for NetworkDictionary and NetworkSet

Author: jeffreyrainy

com.unity.multiplayer.mlapi/Runtime/Core/NetworkBehaviour.cs

@@ -621,6 +621,10 @@ private void NetworkVariableUpdate(ulong clientId)
                             {
                                 writtenAny = true;
 
+                                // write the network tick at which this NetworkVariable was modified remotely
+                                // this will allow lag-compensation
+                                writer.WriteUInt16Packed(NetworkVariableFields[k].RemoteTick);
+
                                 if (NetworkManager.Singleton.NetworkConfig.EnsureNetworkVariableLengthSafety)
                                 {
                                     using (var varBuffer = PooledNetworkBuffer.Get())

com.unity.multiplayer.mlapi/Runtime/NetworkVariable/Collections/NetworkDictionary.cs

@@ -531,6 +531,15 @@ private void HandleAddDictionaryEvent(NetworkDictionaryEvent<TKey, TValue> dicti
                 m_DirtyEvents.Add(dictionaryEvent);
             }
         }
+
+        public ushort RemoteTick
+        {
+            get
+            {
+                // todo: implement proper network tick for NetworkDictionary
+                return NetworkTickSystem.NoTick;
+            }
+        }
     }
 
     /// <summary>
@@ -586,4 +595,4 @@ public enum EventType
         /// </summary>
         public TValue Value;
     }
-}
+}

com.unity.multiplayer.mlapi/Runtime/NetworkVariable/Collections/NetworkList.cs

@@ -531,6 +531,15 @@ private void HandleAddListEvent(NetworkListEvent<T> listEvent)
                 m_DirtyEvents.Add(listEvent);
             }
         }
+
+        public ushort RemoteTick
+        {
+            get
+            {
+                // todo: implement proper network tick for NetworkList
+                return NetworkTickSystem.NoTick;
+            }
+        }
     }
 
     /// <summary>
@@ -590,4 +599,4 @@ public enum EventType
         /// </summary>
         public int Index;
     }
-}
+}

com.unity.multiplayer.mlapi/Runtime/NetworkVariable/Collections/NetworkSet.cs

@@ -496,6 +496,15 @@ public bool Remove(T item)
 
         /// <inheritdoc />
         public bool IsReadOnly => m_Set.IsReadOnly;
+
+        public ushort RemoteTick
+        {
+            get
+            {
+                // todo: implement proper network tick for NetworkSet
+                return NetworkTickSystem.NoTick;
+            }
+        }
     }
 
     /// <summary>
@@ -536,4 +545,4 @@ public enum EventType
         public T Value;
     }
 }
-#endif
+#endif

com.unity.multiplayer.mlapi/Runtime/NetworkVariable/INetworkVariable.cs

@@ -73,5 +73,10 @@ public interface INetworkVariable
         /// </summary>
         /// <param name="behaviour">The behaviour the container behaves to</param>
         void SetNetworkBehaviour(NetworkBehaviour behaviour);
+
+        /// <summary>
+        /// Accessor for the RemoteTick stored in the networkVariable, list, set or dictionary
+        /// </summary>
+        ushort RemoteTick { get; }
     }
-}
+}

com.unity.multiplayer.mlapi/Runtime/NetworkVariable/NetworkVariable.cs

@@ -145,14 +145,6 @@ public bool CanClientRead(ulong clientId)
         /// <param name="stream">The stream to write the value to</param>
         public void WriteDelta(Stream stream)
         {
-            using (var writer = PooledNetworkWriter.Get(stream))
-            {
-                // write the network tick at which this NetworkVariable was modified remotely
-                // this will allow lag-compensation
-                // todo: this is currently only done on delta updates. Consider whether it should be done in WriteField
-                writer.WriteUInt16Packed(RemoteTick);
-            }
-
             WriteField(stream);
         }
 

testproject/Assets/Scripts/Testing/ManualNetworkVariableTest.cs

@@ -1,6 +1,8 @@
 using System;
+using System.Collections.Generic;
 using UnityEngine;
 using MLAPI.NetworkVariable;
+using MLAPI.NetworkVariable.Collections;
 
 namespace MLAPI
 {
@@ -10,7 +12,20 @@ namespace MLAPI
     [AddComponentMenu("MLAPI/ManualNetworkVariableTest")]
     public class ManualNetworkVariableTest : NetworkBehaviour
     {
-        private NetworkVariable<int> m_TestVar;
+        // testing NetworkList
+        private NetworkList<string> m_TestList = new NetworkList<string>();
+        private bool m_GotNetworkList = false;
+
+        // testing NetworkSet
+        private NetworkSet<string> m_TestSet = new NetworkSet<string>();
+        private bool m_GotNetworkSet = false;
+
+        // testing NetworkDictionary
+        private NetworkDictionary<int, string> m_TestDictionary = new NetworkDictionary<int, string>();
+        private bool m_GotNetworkDictionary = false;
+
+        // testing NetworkVariable, especially ticks
+        private NetworkVariable<int> m_TestVar = new NetworkVariable<int>();
         private int m_MinDelta = 0;
         private int m_MaxDelta = 0;
         private int m_LastRemoteTick = 0;
@@ -22,25 +37,65 @@ public class ManualNetworkVariableTest : NetworkBehaviour
 
         void Start()
         {
-            m_TestVar.OnValueChanged = ValueChanged;
+            m_TestVar.OnValueChanged += ValueChanged;
             m_TestVar.Settings.WritePermission = NetworkVariablePermission.Everyone;
 
+            m_TestList.OnListChanged += ListChanged;
+            m_TestList.Settings.WritePermission = NetworkVariablePermission.OwnerOnly;
+
+            m_TestSet.OnSetChanged += SetChanged;
+            m_TestSet.Settings.WritePermission = NetworkVariablePermission.OwnerOnly;
+
+            m_TestDictionary.OnDictionaryChanged += DictionaryChanged;
+            m_TestDictionary.Settings.WritePermission = NetworkVariablePermission.OwnerOnly;
+
             if (IsOwner)
             {
                 m_TestVar.Value = 0;
+                Debug.Log("We'll be sending " + MyMessage());
             }
         }
 
-        void Awake()
-        {
-            Debug.Log("Awake");
-        }
-
         private void FixedUpdate()
         {
             if (IsOwner)
             {
                 m_TestVar.Value = m_TestVar.Value + 1;
+                m_TestList.Add(MyMessage());
+                ((ICollection<string>)m_TestSet).Add(MyMessage());
+                m_TestDictionary[0] = MyMessage();
+            }
+        }
+
+        private string MyMessage()
+        {
+            return "Message from " + NetworkObjectId;
+        }
+
+        private void ListChanged(NetworkListEvent<string> listEvent)
+        {
+            if (!IsOwner && !m_GotNetworkList)
+            {
+                Debug.Log("Received: " + listEvent.Value);
+                m_GotNetworkList = true;
+            }
+        }
+
+        private void SetChanged(NetworkSetEvent<string> setEvent)
+        {
+            if (!IsOwner && !m_GotNetworkSet)
+            {
+                Debug.Log("Received: " + setEvent.Value);
+                m_GotNetworkSet = true;
+            }
+        }
+
+        private void DictionaryChanged(NetworkDictionaryEvent<int, string> dictionaryEvent)
+        {
+            if (!IsOwner && !m_GotNetworkSet)
+            {
+                Debug.Log("Received: " + dictionaryEvent.Key + ":" + dictionaryEvent.Value);
+                m_GotNetworkDictionary = true;
             }
         }
 
@@ -84,19 +139,35 @@ private void ValueChanged(int before, int after)
             {
                 // Let's be reasonable and allow a 5 tick difference
                 // that could be due to timing difference, lag, queueing
-                if (m_Problems == "" && Math.Abs(m_MaxDelta - m_MinDelta) < 5)
+
+                if (!m_GotNetworkList)
+                {
+                    m_Problems += "Didn't receive any NetworkList updates from other machines";
+                }
+
+                if (!m_GotNetworkSet)
+                {
+                    m_Problems += "Didn't receive any NetworkSet updates from other machines";
+                }
+
+                if (!m_GotNetworkDictionary)
+                {
+                    m_Problems += "Didn't receive any NetworkDictionary updates from other machines";
+                }
+
+                if (Math.Abs(m_MaxDelta - m_MinDelta) > 5)
+                {
+                    m_Problems += "Delta range: " + m_MinDelta + " + " + m_MaxDelta + "\n";
+                }
+
+                if (m_Problems == "")
                 {
                     Debug.Log("**** TEST PASSED ****");
                 }
                 else
                 {
                     Debug.Log("**** TEST FAILED ****");
-                    Debug.Log($"Delta range: {m_MinDelta}, {m_MaxDelta}");
-
-                    if (m_Problems != "")
-                    {
-                        Debug.Log(m_Problems);
-                    }
+                    Debug.Log(m_Problems);
                 }
                 enabled = false;
             }