From 1199a1b42d6fb900bb3b6251499e80f509079930 Mon Sep 17 00:00:00 2001
From: MrGadget <9826063+MrGadget1024@users.noreply.github.com>
Date: Sun, 14 Apr 2024 17:04:49 -0400
Subject: [PATCH] fix(SyncSet): AddOperation now properly handles oldItem

---
 Assets/Mirror/Core/SyncSet.cs | 70 ++++++++++++++++++++++++-----------
 1 file changed, 48 insertions(+), 22 deletions(-)

diff --git a/Assets/Mirror/Core/SyncSet.cs b/Assets/Mirror/Core/SyncSet.cs
index 5c8fe605a..5958fcd25 100644
--- a/Assets/Mirror/Core/SyncSet.cs
+++ b/Assets/Mirror/Core/SyncSet.cs
@@ -66,16 +66,36 @@ public override void Reset()
         // this should be called after a successful sync
         public override void ClearChanges() => changes.Clear();
 
-        void AddOperation(Operation op, T item, bool checkAccess)
+        void AddOperation(Operation op, T oldItem, T newItem, bool checkAccess)
         {
             if (checkAccess && IsReadOnly)
                 throw new InvalidOperationException("SyncSets can only be modified by the owner.");
 
-            Change change = new Change
+            Change change = default;
+            switch (op)
             {
-                operation = op,
-                item = item
-            };
+                case Operation.OP_ADD:
+                    change = new Change
+                    {
+                        operation = op,
+                        item = newItem
+                    };
+                    break;
+                case Operation.OP_REMOVE:
+                    change = new Change
+                    {
+                        operation = op,
+                        item = oldItem
+                    };
+                    break;
+                case Operation.OP_CLEAR:
+                    change = new Change
+                    {
+                        operation = op,
+                        item = default
+                    };
+                    break;
+            }
 
             if (IsRecording())
             {
@@ -86,22 +106,27 @@ void AddOperation(Operation op, T item, bool checkAccess)
             switch (op)
             {
                 case Operation.OP_ADD:
-                    OnAdd?.Invoke(item);
+                    OnAdd?.Invoke(newItem);
+#pragma warning disable CS0618 // Type or member is obsolete
+                    Callback?.Invoke(op, newItem);
+#pragma warning restore CS0618 // Type or member is obsolete
                     break;
                 case Operation.OP_REMOVE:
-                    OnRemove?.Invoke(item);
+                    OnRemove?.Invoke(oldItem);
+#pragma warning disable CS0618 // Type or member is obsolete
+                    Callback?.Invoke(op, oldItem);
+#pragma warning restore CS0618 // Type or member is obsolete
                     break;
                 case Operation.OP_CLEAR:
                     OnClear?.Invoke();
+#pragma warning disable CS0618 // Type or member is obsolete
+                    Callback?.Invoke(op, default);
+#pragma warning restore CS0618 // Type or member is obsolete
                     break;
             }
-
-#pragma warning disable CS0618 // Type or member is obsolete
-            Callback?.Invoke(op, item);
-#pragma warning restore CS0618 // Type or member is obsolete
         }
 
-        void AddOperation(Operation op, bool checkAccess) => AddOperation(op, default, checkAccess);
+        void AddOperation(Operation op, bool checkAccess) => AddOperation(op, default, default, checkAccess);
 
         public override void OnSerializeAll(NetworkWriter writer)
         {
@@ -173,33 +198,34 @@ public override void OnDeserializeDelta(NetworkReader reader)
                 // apply the operation only if it is a new change
                 // that we have not applied yet
                 bool apply = changesAhead == 0;
-                T item = default;
+                T oldItem = default;
+                T newItem = default;
 
                 switch (operation)
                 {
                     case Operation.OP_ADD:
-                        item = reader.Read<T>();
+                        newItem = reader.Read<T>();
                         if (apply)
                         {
-                            objects.Add(item);
+                            objects.Add(newItem);
                             // add dirty + changes.
                             // ClientToServer needs to set dirty in server OnDeserialize.
                             // no access check: server OnDeserialize can always
                             // write, even for ClientToServer (for broadcasting).
-                            AddOperation(Operation.OP_ADD, item, false);
+                            AddOperation(Operation.OP_ADD, default, newItem, false);
                         }
                         break;
 
                     case Operation.OP_REMOVE:
-                        item = reader.Read<T>();
+                        oldItem = reader.Read<T>();
                         if (apply)
                         {
-                            objects.Remove(item);
+                            objects.Remove(oldItem);
                             // add dirty + changes.
                             // ClientToServer needs to set dirty in server OnDeserialize.
                             // no access check: server OnDeserialize can always
                             // write, even for ClientToServer (for broadcasting).
-                            AddOperation(Operation.OP_REMOVE, item, false);
+                            AddOperation(Operation.OP_REMOVE, oldItem, default, false);
                         }
                         break;
 
@@ -230,7 +256,7 @@ public bool Add(T item)
         {
             if (objects.Add(item))
             {
-                AddOperation(Operation.OP_ADD, item, true);
+                AddOperation(Operation.OP_ADD, default, item, true);
                 return true;
             }
             return false;
@@ -239,7 +265,7 @@ public bool Add(T item)
         void ICollection<T>.Add(T item)
         {
             if (objects.Add(item))
-                AddOperation(Operation.OP_ADD, item, true);
+                AddOperation(Operation.OP_ADD, default, item, true);
         }
 
         public void Clear()
@@ -258,7 +284,7 @@ public bool Remove(T item)
         {
             if (objects.Remove(item))
             {
-                AddOperation(Operation.OP_REMOVE, item, true);
+                AddOperation(Operation.OP_REMOVE, item, default, true);
                 return true;
             }
             return false;