diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index fcd2b5311c..27bf78002a 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -27,6 +27,8 @@ Additional documentation and release notes are available at [Multiplayer Documen - Issue where a growable `FastBufferWriter` created with a size of 0 never returned from its first write or `TryBeginWrite` call that needed the buffer to grow. (#4224) - Issue where a `FastBufferReader` created with an offset and the default length of -1 read past the end of its source. The `ArraySegment` overload that takes an offset now applies it from the start of the segment. (#4222) +- Issue where a client synchronized while a `NetworkList` still had unsent changes applied those changes twice, resulting in duplicate entries and index-based events landing on the wrong elements on that client. (#4216) +- Issue where showing a `NetworkObject` with `NetworkShow` while its `NetworkVariable` updates were held back by `NetworkVariableUpdateTraits` stopped those pending changes from being sent until the variable was modified again. (#4216) - Issue where a client promoted to session owner could throw an exception when synchronizing a newly joining client. (#4214) - Issue where `NetworkManager.OnNetCodeDisconnect` was not invoked when a connection closed during a hybrid mode session, only when the session shut down. (#4209) - Issue where a rejected parent change, for example one made by a client without parenting authority, restored the `NetworkObject`'s parent but left it at the moved position when the change was made with `worldPositionStays` disabled. The original position, rotation and scale are now restored. (#4202) diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviourUpdater.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviourUpdater.cs index 871c8afb7d..06f87a5ca7 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviourUpdater.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviourUpdater.cs @@ -138,6 +138,19 @@ internal void ForceSendIfDirtyOnNetworkShow(NetworkObject networkObject) // Remove it from the pending and queued dirty objects lists m_PendingDirtyNetworkObjects.Remove(networkObject); m_DirtyNetworkObjects.Remove(networkObject); + + // Anything still dirty was held back by its update traits, keep it queued so a later tick sends it + foreach (var behaviour in networkObject.ChildNetworkBehaviours.Values) + { + foreach (var networkVariable in behaviour.NetworkVariableFields) + { + if (networkVariable.IsDirty()) + { + AddForUpdate(networkObject); + return; + } + } + } } /// diff --git a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs index 5bcd5cead3..03c1c79ad4 100644 --- a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs +++ b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs @@ -17,6 +17,10 @@ public class NetworkList : NetworkVariableBase where T : unmanaged, IEquatabl private NativeList m_List = new NativeList(64, Allocator.Persistent); private NativeList> m_DirtyEvents = new NativeList>(64, Allocator.Persistent); + // The list as it was before the first local change that is still pending in m_DirtyEvents + private NativeList m_LastSentList = new NativeList(64, Allocator.Persistent); + private bool m_HasLastSentList; + /// /// Delegate type for list changed event /// @@ -68,6 +72,7 @@ public override void ResetDirty() { m_DirtyEvents.Clear(); } + m_HasLastSentList = false; } /// @@ -150,10 +155,42 @@ private T ReadValue(FastBufferReader reader) /// public override void WriteField(FastBufferWriter writer) { - writer.WriteValueSafe((ushort)m_List.Length); - for (int i = 0; i < m_List.Length; i++) + WriteList(writer, m_List); + } + + internal override void WriteFieldSynchronization(FastBufferWriter writer) + { + // The pending events are sent to the synchronizing client with the next delta, so it must + // receive the list they apply to. Otherwise it would apply them twice. + if (m_HasLastSentList && m_DirtyEvents.Length > 0) + { + WriteList(writer, m_LastSentList); + } + else + { + base.WriteFieldSynchronization(writer); + } + } + + private static void WriteList(FastBufferWriter writer, NativeList list) + { + writer.WriteValueSafe((ushort)list.Length); + for (int i = 0; i < list.Length; i++) { - NetworkVariableSerialization.Serializer.Write(writer, ref m_List.ElementAt(i)); + NetworkVariableSerialization.Serializer.Write(writer, ref list.ElementAt(i)); + } + } + + /// + /// Invoked before a local change. Keeps a copy of the list as of the last sent delta + /// so can send it while changes are pending. + /// + private void CaptureLastSentList() + { + if (m_DirtyEvents.Length == 0) + { + m_LastSentList.CopyFrom(m_List); + m_HasLastSentList = true; } } @@ -440,6 +477,7 @@ public void Add(T item) return; } + CaptureLastSentList(); m_List.Add(item); var listEvent = new NetworkListEvent() @@ -467,6 +505,7 @@ public void Clear() return; } + CaptureLastSentList(); m_List.Clear(); var listEvent = new NetworkListEvent() @@ -511,6 +550,7 @@ public bool Remove(T item) return false; } + CaptureLastSentList(); m_List.RemoveAt(index); var listEvent = new NetworkListEvent() { @@ -554,6 +594,7 @@ public void Insert(int index, T item) return; } + CaptureLastSentList(); if (index < m_List.Length) { m_List.InsertRangeWithBeginEnd(index, index + 1); @@ -590,6 +631,7 @@ public void RemoveAt(int index) } var value = m_List[index]; + CaptureLastSentList(); m_List.RemoveAt(index); var listEvent = new NetworkListEvent() @@ -632,6 +674,7 @@ public void Set(int index, T value, bool forceUpdate = false) return; } + CaptureLastSentList(); m_List[index] = value; var listEvent = new NetworkListEvent() @@ -707,6 +750,11 @@ public override void Dispose() m_DirtyEvents.Dispose(); } + if (m_LastSentList.IsCreated) + { + m_LastSentList.Dispose(); + } + base.Dispose(); } } diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkListTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkListTests.cs index 4e4dfdade3..a903075e47 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkListTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkListTests.cs @@ -91,6 +91,91 @@ private bool OnVerifyData(StringBuilder errorLog) return true; } + /// + /// Validates that a client synchronized while the list still has unsent changes + /// does not apply those changes twice (snapshot + next delta). + /// + [UnityTest] + public IEnumerator LateJoinWithPendingChanges() + { + var authority = GetAuthorityNetworkManager(); + var instantiatedObject = SpawnObject(m_ListObjectPrefab, authority).GetComponent(); + m_TestObjectId = instantiatedObject.NetworkObjectId; + + yield return WaitForSpawnedOnAllOrTimeOut(instantiatedObject); + AssertOnTimeout("[Setup] Failed to spawn list object"); + + var list = instantiatedObject.GetComponent().TheList; + m_ExpectedValues.Clear(); + for (int i = 0; i < 5; i++) + { + m_ExpectedValues.Add(i); + list.Add(i); + } + + yield return WaitForConditionOrTimeOut(OnVerifyData); + AssertOnTimeout("[Setup] Failed to add items to list"); + + // Hold back deltas so these changes are still pending when the new client synchronizes + list.SetUpdateTraits(new NetworkVariableUpdateTraits { MinSecondsBetweenUpdates = 3600 }); + list.Add(100); + m_ExpectedValues.Add(100); + list.Insert(0, 101); + m_ExpectedValues.Insert(0, 101); + list.RemoveAt(2); + m_ExpectedValues.RemoveAt(2); + list[1] = 102; + m_ExpectedValues[1] = 102; + list.Remove(3); + m_ExpectedValues.Remove(3); + + yield return CreateAndStartNewClient(); + Assert.IsTrue(list.IsDirty(), "[Setup] Changes should still be pending after the new client synchronized"); + + list.SetUpdateTraits(default); + yield return WaitForConditionOrTimeOut(OnVerifyData); + AssertOnTimeout("Late joined client did not match the authority's list"); + } + + /// + /// Validates that showing an object while list updates are held back by update traits + /// still delivers the pending changes once they can be sent. + /// + [UnityTest] + public IEnumerator NetworkShowWithThrottledPendingChanges() + { + var authority = GetAuthorityNetworkManager(); + var nonAuthority = GetNonAuthorityNetworkManager(); + var instantiatedObject = SpawnObject(m_ListObjectPrefab, authority).GetComponent(); + m_TestObjectId = instantiatedObject.NetworkObjectId; + + yield return WaitForSpawnedOnAllOrTimeOut(instantiatedObject); + AssertOnTimeout("[Setup] Failed to spawn list object"); + + instantiatedObject.NetworkHide(nonAuthority.LocalClientId); + yield return WaitForConditionOrTimeOut(() => !nonAuthority.SpawnManager.SpawnedObjects.ContainsKey(m_TestObjectId)); + AssertOnTimeout("[Setup] Failed to hide list object"); + + // Hold back deltas so these changes are still pending when the object is shown + var list = instantiatedObject.GetComponent().TheList; + list.SetUpdateTraits(new NetworkVariableUpdateTraits { MinSecondsBetweenUpdates = 3600 }); + m_ExpectedValues.Clear(); + for (int i = 0; i < 5; i++) + { + m_ExpectedValues.Add(i); + list.Add(i); + } + + instantiatedObject.NetworkShow(nonAuthority.LocalClientId); + yield return WaitForConditionOrTimeOut(() => nonAuthority.SpawnManager.SpawnedObjects.ContainsKey(m_TestObjectId)); + AssertOnTimeout("[Setup] Failed to show list object"); + Assert.IsTrue(list.IsDirty(), "[Setup] Changes should still be pending after the object was shown"); + + list.SetUpdateTraits(default); + yield return WaitForConditionOrTimeOut(OnVerifyData); + AssertOnTimeout("Clients did not receive the changes that were pending when the object was shown"); + } + [UnityTest] public IEnumerator ValidateNetworkListSynchronization() {