diff --git a/Assets/Mirror/Core/SyncDictionary.cs b/Assets/Mirror/Core/SyncDictionary.cs index f0f05215618..1ffd5ae6227 100644 --- a/Assets/Mirror/Core/SyncDictionary.cs +++ b/Assets/Mirror/Core/SyncDictionary.cs @@ -28,12 +28,82 @@ struct Change internal TValue item; } - // list of changes. + abstract class ChangesTracker + { + protected readonly List changes = new List(); + public uint Count => (uint)changes.Count; + public List CurrentChanges => changes; + public virtual void ClearChanges() => changes.Clear(); + public virtual void AddChange(Change changeToAdd) => changes.Add(changeToAdd); + public virtual void ResetTrackedFields() { } + } + + // Traditional list of changes // -> insert/delete/clear is only ONE change - // -> changing the same slot 10x caues 10 changes. + // -> changing the same slot 10x causes 10 changes. // -> note that this grows until next sync(!) - // TODO Dictionary to avoid ever growing changes / redundant changes! - readonly List changes = new List(); + // -> may still be useful for some code that relies on knowing about every change that happens, including redundant changes + class AllChangesTracker : ChangesTracker { /* Nothing to override here */ } + + // Map of changes + // -> insert/delete/set of any field results in AT MOST one change + // -> changing the same slot 10x causes 1 change + // -> clear operation deletes all tracked changes and replaces them with just 1 single tracked change + // -> Things only get a little trickier with tracking "changes ahead", aka "which changes were previously sent/received via SerializeAll/DeserializeAll when Deltas are being set/received." + // -> ResetTrackedFields is important for "changes head" as it means the next time any new slot gets changed, it results in at most 1 more change being added. + class SingleChangePerKeyTracker : ChangesTracker + { + readonly Dictionary changeMap = new Dictionary(); + int changeCountDuringLastChangeMapReset = 0; + + public override void ClearChanges() + { + changeMap.Clear(); + changeCountDuringLastChangeMapReset = 0; + base.ClearChanges(); + } + + public override void AddChange(Change changeToAdd) + { + if(changeToAdd.operation == Operation.OP_CLEAR) + { + changeMap.Clear(); + if(changeCountDuringLastChangeMapReset == 0) + { + // ResetTrackedFields was never called so just clear the whole changes list + changes.Clear(); + } + if(changes.Count > changeCountDuringLastChangeMapReset) + { + // Remove everything from the changes list that was added after the last call to ResetTrackedFields + int removeCount = changes.Count - (changeCountDuringLastChangeMapReset + 1); + changes.RemoveRange(changeCountDuringLastChangeMapReset, removeCount); + } + base.AddChange(changeToAdd); + } + else if(changeMap.TryGetValue(changeToAdd.key, out int changeIndex)) + { + // changeMap should never contain an index that does not exist in the changes list + // changeMap should always provide the index pointing to the last recorded change in the changes list for the given key + changes[changeIndex] = changeToAdd; + } + else + { + base.AddChange(changeToAdd); + changeMap[changeToAdd.key] = changes.Count - 1; + } + } + + public override void ResetTrackedFields() + { + // This has to do with the 'changes ahead' system. By clearning changeMap, it means that up to 1 new change per slot + // this point will get added to the changes list even if the changes list itself already contains a change for said slot. + changeMap.Clear(); + changeCountDuringLastChangeMapReset = changes.Count; + } + } + + readonly ChangesTracker changeTracker; // how many changes we need to ignore // this is needed because when we initialize the list, @@ -43,7 +113,7 @@ struct Change public override void Reset() { - changes.Clear(); + changeTracker.ClearChanges(); changesAhead = 0; objects.Clear(); } @@ -58,11 +128,22 @@ public override void Reset() // throw away all the changes // this should be called after a successful sync - public override void ClearChanges() => changes.Clear(); + public override void ClearChanges() => changeTracker.ClearChanges(); - public SyncIDictionary(IDictionary objects) + public SyncIDictionary(IDictionary objects, bool efficientChangeTrackingAndSync = false) { this.objects = objects; + + if (efficientChangeTrackingAndSync) + { + // Nearly all of the time, we should just sync 1 change per key to save bandwidth by not sending redundant changes + this.changeTracker = new SingleChangePerKeyTracker(); + } + else + { + // Some applications may need to sync all changes in great detail, even if those changes are redundant + this.changeTracker = new AllChangesTracker(); + } } void AddOperation(Operation op, TKey key, TValue item, bool checkAccess) @@ -72,16 +153,14 @@ void AddOperation(Operation op, TKey key, TValue item, bool checkAccess) throw new System.InvalidOperationException("SyncDictionaries can only be modified by the owner."); } - Change change = new Change - { - operation = op, - key = key, - item = item - }; - if (IsRecording()) { - changes.Add(change); + changeTracker.AddChange(new Change() + { + operation = op, + key = key, + item = item + }); OnDirty?.Invoke(); } @@ -103,19 +182,19 @@ public override void OnSerializeAll(NetworkWriter writer) // thus the client will need to skip all the pending changes // or they would be applied again. // So we write how many changes are pending - writer.WriteUInt((uint)changes.Count); + changeTracker.ResetTrackedFields(); + writer.WriteUInt(changeTracker.Count); } public override void OnSerializeDelta(NetworkWriter writer) { // write all the queued up changes - writer.WriteUInt((uint)changes.Count); + writer.WriteUInt(changeTracker.Count); - for (int i = 0; i < changes.Count; i++) + List changes = changeTracker.CurrentChanges; + foreach(Change change in changes) { - Change change = changes[i]; writer.WriteByte((byte)change.operation); - switch (change.operation) { case Operation.OP_ADD: @@ -134,11 +213,11 @@ public override void OnSerializeDelta(NetworkWriter writer) public override void OnDeserializeAll(NetworkReader reader) { - // if init, write the full list content + // if init, write the full list content int count = (int)reader.ReadUInt(); objects.Clear(); - changes.Clear(); + changeTracker.ClearChanges(); for (int i = 0; i < count; i++) { @@ -159,14 +238,13 @@ public override void OnDeserializeDelta(NetworkReader reader) for (int i = 0; i < changesCount; i++) { - Operation operation = (Operation)reader.ReadByte(); - // apply the operation only if it is a new change // that we have not applied yet bool apply = changesAhead == 0; TKey key = default; TValue item = default; + Operation operation = (Operation)reader.ReadByte(); switch (operation) { case Operation.OP_ADD: @@ -316,9 +394,9 @@ public bool Remove(KeyValuePair item) public class SyncDictionary : SyncIDictionary { - public SyncDictionary() : base(new Dictionary()) {} - public SyncDictionary(IEqualityComparer eq) : base(new Dictionary(eq)) {} - public SyncDictionary(IDictionary d) : base(new Dictionary(d)) {} + public SyncDictionary(bool efficientChangeTrackingAndSync = false) : base(new Dictionary(), efficientChangeTrackingAndSync) {} + public SyncDictionary(IEqualityComparer eq, bool efficientChangeTrackingAndSync = false) : base(new Dictionary(eq), efficientChangeTrackingAndSync) {} + public SyncDictionary(IDictionary d, bool efficientChangeTrackingAndSync = false) : base(new Dictionary(d), efficientChangeTrackingAndSync) {} public new Dictionary.ValueCollection Values => ((Dictionary)objects).Values; public new Dictionary.KeyCollection Keys => ((Dictionary)objects).Keys; public new Dictionary.Enumerator GetEnumerator() => ((Dictionary)objects).GetEnumerator(); diff --git a/Assets/Mirror/Tests/Editor/SyncCollections/SyncDictionaryTest.cs b/Assets/Mirror/Tests/Editor/SyncCollections/SyncDictionaryTest.cs index 61a93ebd83b..f85a0bae5fc 100644 --- a/Assets/Mirror/Tests/Editor/SyncCollections/SyncDictionaryTest.cs +++ b/Assets/Mirror/Tests/Editor/SyncCollections/SyncDictionaryTest.cs @@ -4,13 +4,15 @@ namespace Mirror.Tests.SyncCollections { - [TestFixture] + [TestFixture(true)] + [TestFixture(false)] public class SyncDictionaryTest { SyncDictionary serverSyncDictionary; SyncDictionary clientSyncDictionary; int serverSyncDictionaryDirtyCalled; int clientSyncDictionaryDirtyCalled; + bool optimized; void SerializeAllTo(T fromList, T toList) where T : SyncObject { @@ -29,11 +31,16 @@ void SerializeDeltaTo(T fromList, T toList) where T : SyncObject fromList.ClearChanges(); } + public SyncDictionaryTest(bool optimized) + { + this.optimized = optimized; + } + [SetUp] public void SetUp() { - serverSyncDictionary = new SyncDictionary(); - clientSyncDictionary = new SyncDictionary(); + serverSyncDictionary = new SyncDictionary(optimized); + clientSyncDictionary = new SyncDictionary(optimized); // set writable serverSyncDictionary.IsWritable = () => true;