SyncList: keep old OnChange parameters to not break projects

Assets/Mirror/Core/SyncList.cs

@@ -33,8 +33,19 @@ public enum Operation : byte
         /// <para>For OP_SET and OP_REMOVE, T is the OLD value of the entry.</para>
         /// <para>For OP_CLEAR, T is default.</para>
         /// </summary>
+        // DEPRECATED 2024-10-14
+        // TODO deprecate in favor of explicit Callback, later rename Callback to OnChange for consistency with other SyncCollections.
+        [Obsolete("SyncList.OnChange(op, index, item) is obsolete. Use SyncList.Callback(op, index, oldItem, newItem) instead.")]
         public Action<Operation, int, T> OnChange;
 
+        /// <summary>
+        /// This is called for all changes to the List.
+        /// Parameters: Operation, index, oldItem, newItem.
+        /// Sometimes we need both oldItem and newItem.
+        /// Keep for compatibility since 10 years of projects use this.
+        /// </summary>
+        public Action<Operation, int, T, T> Callback;
+
         /// <summary>This is called before the list is cleared so the list can be iterated</summary>
         public Action OnClear;
 
@@ -110,23 +121,38 @@ void AddOperation(Operation op, int itemIndex, T oldItem, T newItem, bool checkA
             {
                 case Operation.OP_ADD:
                     OnAdd?.Invoke(itemIndex);
+ #pragma warning disable CS0618
                     OnChange?.Invoke(op, itemIndex, newItem);
+ #pragma warning restore CS0618
+                    Callback?.Invoke(op, itemIndex, oldItem, newItem);
                     break;
                 case Operation.OP_INSERT:
                     OnInsert?.Invoke(itemIndex);
+ #pragma warning disable CS0618
                     OnChange?.Invoke(op, itemIndex, newItem);
+ #pragma warning restore CS0618
+                    Callback?.Invoke(op, itemIndex, oldItem, newItem);
                     break;
                 case Operation.OP_SET:
                     OnSet?.Invoke(itemIndex, oldItem);
+ #pragma warning disable CS0618
                     OnChange?.Invoke(op, itemIndex, oldItem);
+ #pragma warning restore CS0618
+                    Callback?.Invoke(op, itemIndex, oldItem, newItem);
                     break;
                 case Operation.OP_REMOVEAT:
                     OnRemove?.Invoke(itemIndex, oldItem);
+ #pragma warning disable CS0618
                     OnChange?.Invoke(op, itemIndex, oldItem);
+ #pragma warning restore CS0618
+                    Callback?.Invoke(op, itemIndex, oldItem, newItem);
                     break;
                 case Operation.OP_CLEAR:
                     OnClear?.Invoke();
+ #pragma warning disable CS0618
                     OnChange?.Invoke(op, itemIndex, default);
+ #pragma warning restore CS0618
+                    Callback?.Invoke(op, itemIndex, default, default);
                     break;
             }
         }

Assets/Mirror/Tests/Editor/SyncCollections/SyncListStructTest.cs

@@ -56,6 +56,14 @@ public void OldValueShouldNotBeNewValue()
                 Assert.That(clientList[itemIndex].item.price, Is.EqualTo(15));
                 callbackCalled = true;
             };
+            clientList.Callback = (SyncList<TestPlayer>.Operation op, int itemIndex, TestPlayer oldItem, TestPlayer newItem) =>
+            {
+                Assert.That(op == SyncList<TestPlayer>.Operation.OP_SET, Is.True);
+                Assert.That(itemIndex, Is.EqualTo(0));
+                Assert.That(oldItem.item.price, Is.EqualTo(10));
+                Assert.That(newItem.item.price, Is.EqualTo(15));
+                callbackCalled = true;
+            };
 
             SyncListTest.SerializeDeltaTo(serverList, clientList);
             Assert.IsTrue(callbackCalled);

Assets/Mirror/Tests/Editor/SyncCollections/SyncListTest.cs

@@ -105,6 +105,13 @@ public void TestClear()
                 Assert.That(op, Is.EqualTo(SyncList<string>.Operation.OP_CLEAR));
                 Assert.That(clientSyncList.Count, Is.EqualTo(3));
             };
+            clientSyncList.Callback = (op, index, oldItem, newItem) =>
+            {
+                called = true;
+
+                Assert.That(op, Is.EqualTo(SyncList<string>.Operation.OP_CLEAR));
+                Assert.That(clientSyncList.Count, Is.EqualTo(3));
+            };
 
             bool actionCalled = false;
             clientSyncList.OnClear = () =>
@@ -132,6 +139,14 @@ public void TestInsert()
                 Assert.That(index, Is.EqualTo(0));
                 Assert.That(clientSyncList[index], Is.EqualTo("yay"));
             };
+            clientSyncList.Callback = (op, index, oldItem, newItem) =>
+            {
+                called = true;
+
+                Assert.That(op, Is.EqualTo(SyncList<string>.Operation.OP_INSERT));
+                Assert.That(index, Is.EqualTo(0));
+                Assert.That(newItem, Is.EqualTo("yay"));
+            };
 
             bool actionCalled = false;
             clientSyncList.OnInsert = (index) =>
@@ -168,6 +183,15 @@ public void TestSet()
                 Assert.That(oldItem, Is.EqualTo("World"));
                 Assert.That(clientSyncList[index], Is.EqualTo("yay"));
             };
+            clientSyncList.Callback = (op, index, oldItem, newItem) =>
+            {
+                called = true;
+
+                Assert.That(op, Is.EqualTo(SyncList<string>.Operation.OP_SET));
+                Assert.That(index, Is.EqualTo(1));
+                Assert.That(oldItem, Is.EqualTo("World"));
+                Assert.That(newItem, Is.EqualTo("yay"));
+            };
 
             bool actionCalled = false;
             clientSyncList.OnSet = (index, oldItem) =>
@@ -198,6 +222,15 @@ public void TestSetNull()
                 Assert.That(oldItem, Is.EqualTo("World"));
                 Assert.That(clientSyncList[index], Is.EqualTo(null));
             };
+            clientSyncList.Callback = (op, index, oldItem, newItem) =>
+            {
+                called = true;
+
+                Assert.That(op, Is.EqualTo(SyncList<string>.Operation.OP_SET));
+                Assert.That(index, Is.EqualTo(1));
+                Assert.That(oldItem, Is.EqualTo("World"));
+                Assert.That(newItem, Is.EqualTo(null));
+            };
 
             bool actionCalled = false;
             clientSyncList.OnSet = (index, oldItem) =>
@@ -216,6 +249,7 @@ public void TestSetNull()
 
             // clear handlers so we don't get called again
             clientSyncList.OnChange = null;
+            clientSyncList.Callback = null;
             clientSyncList.OnSet = null;
 
             serverSyncList[1] = "yay";
@@ -235,6 +269,14 @@ public void TestRemoveAll()
                 Assert.That(index, Is.EqualTo(0));
                 Assert.That(oldItem, Is.Not.EqualTo("!"));
             };
+            clientSyncList.Callback = (op, index, oldItem, newItem) =>
+            {
+                called = true;
+
+                Assert.That(op, Is.EqualTo(SyncList<string>.Operation.OP_REMOVEAT));
+                Assert.That(index, Is.EqualTo(0));
+                Assert.That(oldItem, Is.Not.EqualTo("!"));
+            };
 
             bool actionCalled = false;
             clientSyncList.OnRemove = (index, item) =>
@@ -272,6 +314,14 @@ public void TestRemoveAt()
                 Assert.That(index, Is.EqualTo(1));
                 Assert.That(oldItem, Is.EqualTo("World"));
             };
+            clientSyncList.Callback = (op, index, oldItem, newItem) =>
+            {
+                called = true;
+
+                Assert.That(op, Is.EqualTo(SyncList<string>.Operation.OP_REMOVEAT));
+                Assert.That(index, Is.EqualTo(1));
+                Assert.That(oldItem, Is.EqualTo("World"));
+            };
 
             bool actionCalled = false;
             clientSyncList.OnRemove = (index, oldItem) =>
@@ -300,6 +350,14 @@ public void TestRemove()
                 Assert.That(index, Is.EqualTo(1));
                 Assert.That(oldItem, Is.EqualTo("World"));
             };
+            clientSyncList.Callback = (op, index, oldItem, newItem) =>
+            {
+                called = true;
+
+                Assert.That(op, Is.EqualTo(SyncList<string>.Operation.OP_REMOVEAT));
+                Assert.That(index, Is.EqualTo(1));
+                Assert.That(oldItem, Is.EqualTo("World"));
+            };
 
             bool actionCalled = false;
             clientSyncList.OnRemove = (index, oldItem) =>
@@ -438,11 +496,21 @@ public void CallbackTest()
                 Assert.That(newItem, Is.EqualTo("yay"));
                 Assert.That(clientSyncList[index], Is.EqualTo("yay"));
             };
+            bool callbackActionCalled = false;
+            clientSyncList.Callback = (op, index, oldItem, newItem) =>
+            {
+                callbackActionCalled = true;
+                Assert.That(op, Is.EqualTo(SyncList<string>.Operation.OP_ADD));
+                Assert.That(index, Is.EqualTo(3));
+                Assert.That(oldItem, Is.Null);
+                Assert.That(newItem, Is.EqualTo("yay"));
+            };
 
             serverSyncList.Add("yay");
             SerializeDeltaTo(serverSyncList, clientSyncList);
             Assert.That(actionCalled, Is.True);
             Assert.That(changeActionCalled, Is.True);
+            Assert.That(callbackActionCalled, Is.True);
         }
 
         [Test]
@@ -463,11 +531,19 @@ public void CallbackRemoveTest()
                 Assert.That(op, Is.EqualTo(SyncList<string>.Operation.OP_REMOVEAT));
                 Assert.That(index, Is.EqualTo(1));
             };
+            bool callbackActionCalled = false;
+            clientSyncList.Callback = (op, index, oldItem, newItem) =>
+            {
+                callbackActionCalled = true;
+                Assert.That(op, Is.EqualTo(SyncList<string>.Operation.OP_REMOVEAT));
+                Assert.That(index, Is.EqualTo(1));
+            };
 
             serverSyncList.Remove("World");
             SerializeDeltaTo(serverSyncList, clientSyncList);
             Assert.That(actionCalled, Is.True);
             Assert.That(changeActionCalled, Is.True);
+            Assert.That(callbackActionCalled, Is.True);
         }
 
         [Test]
@@ -489,11 +565,21 @@ public void CallbackRemoveAtTest()
                 Assert.That(index, Is.EqualTo(1));
                 Assert.That(oldItem, Is.EqualTo("World"));
             };
+            bool callbackActionCalled = false;
+            clientSyncList.Callback = (op, index, oldItem, newItem) =>
+            {
+                callbackActionCalled = true;
+                Assert.That(op, Is.EqualTo(SyncList<string>.Operation.OP_REMOVEAT));
+                Assert.That(index, Is.EqualTo(1));
+                Assert.That(oldItem, Is.EqualTo("World"));
+                Assert.That(newItem, Is.Null);
+            };
 
             serverSyncList.RemoveAt(1);
             SerializeDeltaTo(serverSyncList, clientSyncList);
             Assert.That(actionCalled, Is.True);
             Assert.That(changeActionCalled, Is.True);
+            Assert.That(callbackActionCalled, Is.True);
         }
 
         [Test]