From 13e2dcccd9ad2ea8c2d106724f798fae054db35c Mon Sep 17 00:00:00 2001 From: vis2k Date: Tue, 3 Dec 2019 08:35:41 +0100 Subject: [PATCH] breaking: SyncList.Callback passes old and new entries instead of only passing one entry which is sometimes the old, sometimes the new entry. This is more consistent and it's very useful to know the previous value in a hook for OP_SET and OP_DIRTY. (#1273) * feature: SyncList.Callback passes old and new entries instead of only passing one entry which is sometimes the old, sometimes the new entry. This is more consistent and it's very useful to know the previous value in a hook for OP_SET and OP_DIRTY. * update tests * update docs --- Assets/Mirror/Runtime/SyncList.cs | 42 +++++++++++++++-------------- Assets/Mirror/Tests/SyncListTest.cs | 15 ++++++----- doc/Guides/Sync/SyncLists.md | 6 ++--- 3 files changed, 34 insertions(+), 29 deletions(-) diff --git a/Assets/Mirror/Runtime/SyncList.cs b/Assets/Mirror/Runtime/SyncList.cs index 890a82466..ca43b34fc 100644 --- a/Assets/Mirror/Runtime/SyncList.cs +++ b/Assets/Mirror/Runtime/SyncList.cs @@ -47,7 +47,7 @@ public class SyncListSTRUCT : SyncList where T : struct [EditorBrowsable(EditorBrowsableState.Never)] public abstract class SyncList : IList, IReadOnlyList, SyncObject { - public delegate void SyncListChanged(Operation op, int itemIndex, T item); + public delegate void SyncListChanged(Operation op, int itemIndex, T oldItem, T newItem); readonly IList objects; readonly IEqualityComparer comparer; @@ -105,7 +105,7 @@ protected SyncList(IList objects, IEqualityComparer comparer = null) // this should be called after a successfull sync public void Flush() => changes.Clear(); - void AddOperation(Operation op, int itemIndex, T item) + void AddOperation(Operation op, int itemIndex, T oldItem, T newItem) { if (IsReadOnly) { @@ -116,16 +116,14 @@ void AddOperation(Operation op, int itemIndex, T item) { operation = op, index = itemIndex, - item = item + item = newItem }; changes.Add(change); - Callback?.Invoke(op, itemIndex, item); + Callback?.Invoke(op, itemIndex, oldItem, newItem); } - void AddOperation(Operation op, int itemIndex) => AddOperation(op, itemIndex, default); - public void OnSerializeAll(NetworkWriter writer) { // if init, write the full list content @@ -214,16 +212,17 @@ public void OnDeserializeDelta(NetworkReader reader) // that we have not applied yet bool apply = changesAhead == 0; int index = 0; - T item = default; + T oldItem = default; + T newItem = default; switch (operation) { case Operation.OP_ADD: - item = DeserializeItem(reader); + newItem = DeserializeItem(reader); if (apply) { index = objects.Count; - objects.Add(item); + objects.Add(newItem); } break; @@ -236,10 +235,10 @@ public void OnDeserializeDelta(NetworkReader reader) case Operation.OP_INSERT: index = (int)reader.ReadPackedUInt32(); - item = DeserializeItem(reader); + newItem = DeserializeItem(reader); if (apply) { - objects.Insert(index, item); + objects.Insert(index, newItem); } break; @@ -247,24 +246,25 @@ public void OnDeserializeDelta(NetworkReader reader) index = (int)reader.ReadPackedUInt32(); if (apply) { - item = objects[index]; + oldItem = objects[index]; objects.RemoveAt(index); } break; case Operation.OP_SET: index = (int)reader.ReadPackedUInt32(); - item = DeserializeItem(reader); + newItem = DeserializeItem(reader); if (apply) { - objects[index] = item; + oldItem = objects[index]; + objects[index] = newItem; } break; } if (apply) { - Callback?.Invoke(operation, index, item); + Callback?.Invoke(operation, index, oldItem, newItem); } // we just skipped this change else @@ -277,13 +277,13 @@ public void OnDeserializeDelta(NetworkReader reader) public void Add(T item) { objects.Add(item); - AddOperation(Operation.OP_ADD, objects.Count - 1, item); + AddOperation(Operation.OP_ADD, objects.Count - 1, default, item); } public void Clear() { objects.Clear(); - AddOperation(Operation.OP_CLEAR, 0); + AddOperation(Operation.OP_CLEAR, 0, default, default); } public bool Contains(T item) => IndexOf(item) >= 0; @@ -309,7 +309,7 @@ public int FindIndex(Predicate match) public void Insert(int index, T item) { objects.Insert(index, item); - AddOperation(Operation.OP_INSERT, index, item); + AddOperation(Operation.OP_INSERT, index, default, item); } public bool Remove(T item) @@ -325,8 +325,9 @@ public bool Remove(T item) public void RemoveAt(int index) { + T oldItem = objects[index]; objects.RemoveAt(index); - AddOperation(Operation.OP_REMOVEAT, index); + AddOperation(Operation.OP_REMOVEAT, index, oldItem, default); } public T this[int i] @@ -336,8 +337,9 @@ public T this[int i] { if (!comparer.Equals(objects[i], value)) { + T oldItem = objects[i]; objects[i] = value; - AddOperation(Operation.OP_SET, i, value); + AddOperation(Operation.OP_SET, i, oldItem, value); } } } diff --git a/Assets/Mirror/Tests/SyncListTest.cs b/Assets/Mirror/Tests/SyncListTest.cs index aa7c2a73b..e7d23ee9e 100644 --- a/Assets/Mirror/Tests/SyncListTest.cs +++ b/Assets/Mirror/Tests/SyncListTest.cs @@ -185,13 +185,14 @@ public void CallbackTest() { bool called = false; - clientSyncList.Callback += (op, index, item) => + clientSyncList.Callback += (op, index, oldItem, newItem) => { called = true; Assert.That(op, Is.EqualTo(SyncList.Operation.OP_ADD)); Assert.That(index, Is.EqualTo(3)); - Assert.That(item, Is.EqualTo("yay")); + Assert.That(oldItem, Is.EqualTo(default(string))); + Assert.That(newItem, Is.EqualTo("yay")); }; serverSyncList.Add("yay"); @@ -206,12 +207,13 @@ public void CallbackRemoveTest() { bool called = false; - clientSyncList.Callback += (op, index, item) => + clientSyncList.Callback += (op, index, oldItem, newItem) => { called = true; Assert.That(op, Is.EqualTo(SyncList.Operation.OP_REMOVEAT)); - Assert.That(item, Is.EqualTo("World")); + Assert.That(oldItem, Is.EqualTo("World")); + Assert.That(newItem, Is.EqualTo(default(string))); }; serverSyncList.Remove("World"); SerializeDeltaTo(serverSyncList, clientSyncList); @@ -224,13 +226,14 @@ public void CallbackRemoveAtTest() { bool called = false; - clientSyncList.Callback += (op, index, item) => + clientSyncList.Callback += (op, index, oldItem, newItem) => { called = true; Assert.That(op, Is.EqualTo(SyncList.Operation.OP_REMOVEAT)); Assert.That(index, Is.EqualTo(1)); - Assert.That(item, Is.EqualTo("World")); + Assert.That(oldItem, Is.EqualTo("World")); + Assert.That(newItem, Is.EqualTo(default(string))); }; serverSyncList.RemoveAt(1); diff --git a/doc/Guides/Sync/SyncLists.md b/doc/Guides/Sync/SyncLists.md index 8e35f2d4a..58e93ed23 100644 --- a/doc/Guides/Sync/SyncLists.md +++ b/doc/Guides/Sync/SyncLists.md @@ -77,9 +77,9 @@ class Player : NetworkBehaviour { inventory.Callback += OnInventoryUpdated; } - void OnInventoryUpdated(SyncListItem.Operation op, int index, Item item) + void OnInventoryUpdated(SyncListItem.Operation op, int index, Item oldItem, Item newItem) { - switch (op) + switch (op) { case SyncListItem.Operation.OP_ADD: // index is where it got added in the list @@ -116,7 +116,7 @@ class Player : NetworkBehaviour { By default, SyncList uses a List to store it's data. If you want to use a different list implementation, add a constructor and pass the list implementation to the parent constructor. For example: ```cs -class SyncListItem : SyncList +class SyncListItem : SyncList { public SyncListItem() : base(new MyIList()) {} }