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
This commit is contained in:
vis2k 2019-12-03 08:35:41 +01:00 committed by GitHub
parent 97e838b932
commit 13e2dcccd9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 34 additions and 29 deletions

View File

@ -47,7 +47,7 @@ public class SyncListSTRUCT<T> : SyncList<T> where T : struct
[EditorBrowsable(EditorBrowsableState.Never)] [EditorBrowsable(EditorBrowsableState.Never)]
public abstract class SyncList<T> : IList<T>, IReadOnlyList<T>, SyncObject public abstract class SyncList<T> : IList<T>, IReadOnlyList<T>, 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<T> objects; readonly IList<T> objects;
readonly IEqualityComparer<T> comparer; readonly IEqualityComparer<T> comparer;
@ -105,7 +105,7 @@ protected SyncList(IList<T> objects, IEqualityComparer<T> comparer = null)
// this should be called after a successfull sync // this should be called after a successfull sync
public void Flush() => changes.Clear(); 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) if (IsReadOnly)
{ {
@ -116,16 +116,14 @@ void AddOperation(Operation op, int itemIndex, T item)
{ {
operation = op, operation = op,
index = itemIndex, index = itemIndex,
item = item item = newItem
}; };
changes.Add(change); 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) public void OnSerializeAll(NetworkWriter writer)
{ {
// if init, write the full list content // if init, write the full list content
@ -214,16 +212,17 @@ public void OnDeserializeDelta(NetworkReader reader)
// that we have not applied yet // that we have not applied yet
bool apply = changesAhead == 0; bool apply = changesAhead == 0;
int index = 0; int index = 0;
T item = default; T oldItem = default;
T newItem = default;
switch (operation) switch (operation)
{ {
case Operation.OP_ADD: case Operation.OP_ADD:
item = DeserializeItem(reader); newItem = DeserializeItem(reader);
if (apply) if (apply)
{ {
index = objects.Count; index = objects.Count;
objects.Add(item); objects.Add(newItem);
} }
break; break;
@ -236,10 +235,10 @@ public void OnDeserializeDelta(NetworkReader reader)
case Operation.OP_INSERT: case Operation.OP_INSERT:
index = (int)reader.ReadPackedUInt32(); index = (int)reader.ReadPackedUInt32();
item = DeserializeItem(reader); newItem = DeserializeItem(reader);
if (apply) if (apply)
{ {
objects.Insert(index, item); objects.Insert(index, newItem);
} }
break; break;
@ -247,24 +246,25 @@ public void OnDeserializeDelta(NetworkReader reader)
index = (int)reader.ReadPackedUInt32(); index = (int)reader.ReadPackedUInt32();
if (apply) if (apply)
{ {
item = objects[index]; oldItem = objects[index];
objects.RemoveAt(index); objects.RemoveAt(index);
} }
break; break;
case Operation.OP_SET: case Operation.OP_SET:
index = (int)reader.ReadPackedUInt32(); index = (int)reader.ReadPackedUInt32();
item = DeserializeItem(reader); newItem = DeserializeItem(reader);
if (apply) if (apply)
{ {
objects[index] = item; oldItem = objects[index];
objects[index] = newItem;
} }
break; break;
} }
if (apply) if (apply)
{ {
Callback?.Invoke(operation, index, item); Callback?.Invoke(operation, index, oldItem, newItem);
} }
// we just skipped this change // we just skipped this change
else else
@ -277,13 +277,13 @@ public void OnDeserializeDelta(NetworkReader reader)
public void Add(T item) public void Add(T item)
{ {
objects.Add(item); objects.Add(item);
AddOperation(Operation.OP_ADD, objects.Count - 1, item); AddOperation(Operation.OP_ADD, objects.Count - 1, default, item);
} }
public void Clear() public void Clear()
{ {
objects.Clear(); objects.Clear();
AddOperation(Operation.OP_CLEAR, 0); AddOperation(Operation.OP_CLEAR, 0, default, default);
} }
public bool Contains(T item) => IndexOf(item) >= 0; public bool Contains(T item) => IndexOf(item) >= 0;
@ -309,7 +309,7 @@ public int FindIndex(Predicate<T> match)
public void Insert(int index, T item) public void Insert(int index, T item)
{ {
objects.Insert(index, item); objects.Insert(index, item);
AddOperation(Operation.OP_INSERT, index, item); AddOperation(Operation.OP_INSERT, index, default, item);
} }
public bool Remove(T item) public bool Remove(T item)
@ -325,8 +325,9 @@ public bool Remove(T item)
public void RemoveAt(int index) public void RemoveAt(int index)
{ {
T oldItem = objects[index];
objects.RemoveAt(index); objects.RemoveAt(index);
AddOperation(Operation.OP_REMOVEAT, index); AddOperation(Operation.OP_REMOVEAT, index, oldItem, default);
} }
public T this[int i] public T this[int i]
@ -336,8 +337,9 @@ public T this[int i]
{ {
if (!comparer.Equals(objects[i], value)) if (!comparer.Equals(objects[i], value))
{ {
T oldItem = objects[i];
objects[i] = value; objects[i] = value;
AddOperation(Operation.OP_SET, i, value); AddOperation(Operation.OP_SET, i, oldItem, value);
} }
} }
} }

View File

@ -185,13 +185,14 @@ public void CallbackTest()
{ {
bool called = false; bool called = false;
clientSyncList.Callback += (op, index, item) => clientSyncList.Callback += (op, index, oldItem, newItem) =>
{ {
called = true; called = true;
Assert.That(op, Is.EqualTo(SyncList<string>.Operation.OP_ADD)); Assert.That(op, Is.EqualTo(SyncList<string>.Operation.OP_ADD));
Assert.That(index, Is.EqualTo(3)); 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"); serverSyncList.Add("yay");
@ -206,12 +207,13 @@ public void CallbackRemoveTest()
{ {
bool called = false; bool called = false;
clientSyncList.Callback += (op, index, item) => clientSyncList.Callback += (op, index, oldItem, newItem) =>
{ {
called = true; called = true;
Assert.That(op, Is.EqualTo(SyncList<string>.Operation.OP_REMOVEAT)); Assert.That(op, Is.EqualTo(SyncList<string>.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"); serverSyncList.Remove("World");
SerializeDeltaTo(serverSyncList, clientSyncList); SerializeDeltaTo(serverSyncList, clientSyncList);
@ -224,13 +226,14 @@ public void CallbackRemoveAtTest()
{ {
bool called = false; bool called = false;
clientSyncList.Callback += (op, index, item) => clientSyncList.Callback += (op, index, oldItem, newItem) =>
{ {
called = true; called = true;
Assert.That(op, Is.EqualTo(SyncList<string>.Operation.OP_REMOVEAT)); Assert.That(op, Is.EqualTo(SyncList<string>.Operation.OP_REMOVEAT));
Assert.That(index, Is.EqualTo(1)); 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); serverSyncList.RemoveAt(1);

View File

@ -77,9 +77,9 @@ class Player : NetworkBehaviour {
inventory.Callback += OnInventoryUpdated; 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: case SyncListItem.Operation.OP_ADD:
// index is where it got added in the list // 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: 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 ```cs
class SyncListItem : SyncList<Item> class SyncListItem : SyncList<Item>
{ {
public SyncListItem() : base(new MyIList<Item>()) {} public SyncListItem() : base(new MyIList<Item>()) {}
} }