feature: SyncObject IsRecording() to prepare for NetworkServer.Update not calling ClearAllComponentsDirtyBits on all spawned entities with no observers. (#2926)

* feature: SyncObject IsRecording() to prepare for NetworkServer.Update not calling ClearAllComponentsDirtyBits on all spawned entities with no observers.

* fix test

Assets/Mirror/Runtime/SyncDictionary.cs

@@ -17,6 +17,9 @@ public class SyncIDictionary<TKey, TValue> : IDictionary<TKey, TValue>, SyncObje
         // OnDirty sets owner NetworkBehaviour's dirty mask when changed.
         public Action OnDirty { get; set; }
 
+        // used to stop recording ever growing changes while we have no observers
+        public Func<bool> IsRecording { get; set; } = () => true;
+
         public enum Operation : byte
         {
             OP_ADD,
@@ -84,8 +87,11 @@ void AddOperation(Operation op, TKey key, TValue item)
                 item = item
             };
 
-            changes.Add(change);
-            OnDirty?.Invoke();
+            if (IsRecording())
+            {
+                changes.Add(change);
+                OnDirty?.Invoke();
+            }
 
             Callback?.Invoke(op, key, item);
         }

Assets/Mirror/Runtime/SyncList.cs

@@ -18,6 +18,9 @@ public class SyncList<T> : IList<T>, IReadOnlyList<T>, SyncObject
         // OnDirty sets owner NetworkBehaviour's dirty mask when changed.
         public Action OnDirty { get; set; }
 
+        // used to stop recording ever growing changes while we have no observers
+        public Func<bool> IsRecording { get; set; } = () => true;
+
         public enum Operation : byte
         {
             OP_ADD,
@@ -86,8 +89,11 @@ void AddOperation(Operation op, int itemIndex, T oldItem, T newItem)
                 item = newItem
             };
 
-            changes.Add(change);
-            OnDirty?.Invoke();
+            if (IsRecording())
+            {
+                changes.Add(change);
+                OnDirty?.Invoke();
+            }
 
             Callback?.Invoke(op, itemIndex, oldItem, newItem);
         }

Assets/Mirror/Runtime/SyncObject.cs

@@ -8,6 +8,15 @@ public interface SyncObject
         /// <summary>Used internally to set owner NetworkBehaviour's dirty mask bit when changed.</summary>
         Action OnDirty { get; set; }
 
+        /// <summary>Used internally to check if we are currently tracking changes.</summary>
+        // prevents ever growing .changes lists:
+        // if a monster has no observers but we keep modifing a SyncObject,
+        // then the changes would never be flushed and keep growing,
+        // because OnSerialize isn't called without observers.
+        // => Func so we can set it to () => observers.Count > 0
+        //    without depending on NetworkComponent/NetworkIdentity here.
+        Func<bool> IsRecording { get; set; }
+
         /// <summary>Discard all the queued changes</summary>
         // Consider the object fully synchronized with clients
         void ClearChanges();

Assets/Mirror/Runtime/SyncSet.cs

@@ -17,6 +17,9 @@ public class SyncSet<T> : ISet<T>, SyncObject
         // OnDirty sets owner NetworkBehaviour's dirty mask when changed.
         public Action OnDirty { get; set; }
 
+        // used to stop recording ever growing changes while we have no observers
+        public Func<bool> IsRecording { get; set; } = () => true;
+
         public enum Operation : byte
         {
             OP_ADD,
@@ -73,8 +76,11 @@ void AddOperation(Operation op, T item)
                 item = item
             };
 
-            changes.Add(change);
-            OnDirty?.Invoke();
+            if (IsRecording())
+            {
+                changes.Add(change);
+                OnDirty?.Invoke();
+            }
 
             Callback?.Invoke(op, item);
         }

Assets/Mirror/Tests/Editor/SyncDictionaryTest.cs

@@ -329,5 +329,15 @@ public void ResetShouldClearItems()
             serverSyncDictionary.Reset();
             Assert.That(serverSyncDictionary, Is.Empty);
         }
+
+        [Test]
+        public void IsRecording()
+        {
+            // shouldn't record changes if IsRecording() returns false
+            serverSyncDictionary.ClearChanges();
+            serverSyncDictionary.IsRecording = () => false;
+            serverSyncDictionary[42] = null;
+            Assert.That(serverSyncDictionary.GetChangeCount(), Is.EqualTo(0));
+        }
     }
 }

Assets/Mirror/Tests/Editor/SyncListTest.cs

@@ -394,6 +394,16 @@ public void ResetShouldClearItems()
 
             Assert.That(serverSyncList, Is.Empty);
         }
+
+        [Test]
+        public void IsRecording()
+        {
+            // shouldn't record changes if IsRecording() returns false
+            serverSyncList.ClearChanges();
+            serverSyncList.IsRecording = () => false;
+            serverSyncList.Add("42");
+            Assert.That(serverSyncList.GetChangeCount(), Is.EqualTo(0));
+        }
     }
 
     public static class SyncObjectTestMethods

Assets/Mirror/Tests/Editor/SyncSetTest.cs

@@ -298,5 +298,15 @@ public void ResetShouldClearItems()
             serverSyncSet.Reset();
             Assert.That(serverSyncSet, Is.Empty);
         }
+
+        [Test]
+        public void IsRecording()
+        {
+            // shouldn't record changes if IsRecording() returns false
+            serverSyncSet.ClearChanges();
+            serverSyncSet.IsRecording = () => false;
+            serverSyncSet.Add("42");
+            Assert.That(serverSyncSet.GetChangeCount(), Is.EqualTo(0));
+        }
     }
 }

Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarTests~/SyncVarsSyncList.cs

@@ -9,6 +9,7 @@ class SyncVarsSyncList : NetworkBehaviour
         public class SyncObjImplementer : SyncObject
         {
             public Action OnDirty { get; set; }
+            public Func<bool> IsRecording { get; set; } = () => true;
             public void ClearChanges() { }
             public void OnSerializeAll(NetworkWriter writer) { }
             public void OnSerializeDelta(NetworkWriter writer) { }