diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 89c4b4f86e..b6e524f537 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -17,6 +17,10 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Fixed +- Fixed issue where a newly synchronizing client would be synchronized with the current `NetworkVariable` values always which could cause issues with collections if there were any pending state updates. Now, when initially synchronizing a client, if a `NetworkVariable` has a pending state update it will serialize the previously known value(s) to the synchronizing client so when the pending updates are sent they aren't duplicate values on the newly connected client side. (#3126) +- Fixed issue where changing ownership would mark every `NetworkVariable` dirty. Now, it will only mark any `NetworkVariable` with owner read permissions as dirty and will send/flush any pending updates to all clients prior to sending the change in ownership message. (#3126) +- Fixed issue with `NetworkVariable` collections where transferring ownership to another client would not update the new owner's previous value to the most current value which could cause the last/previous added value to be detected as a change when adding or removing an entry (as long as the entry removed was not the last/previously added value). (#3126) +- Fixed issue where a client (or server) with no write permissions for a `NetworkVariable` using a standard .NET collection type could still modify the collection which could cause various issues depending upon the modification and collection type. (#3126) - Fixed issue where `NetworkAnimator` would statically allocate write buffer space for `Animator` parameters that could cause a write error if the number of parameters exceeded the space allocated. (#3124) - Fixed issue with the in-scene network prefab instance update menu tool where it was not properly updating scenes when invoked on the root prefab instance. (#3084) - Fixed issue where `NetworkAnimator` would send updates to non-observer clients. (#3058) @@ -28,6 +32,7 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Changed +- Changed `NetworkVariableDeltaMessage` so the server now forwards delta state updates (owner write permission based from a client) to other clients immediately as opposed to keeping a `NetworkVariable` or `NetworkList` dirty and processing them at the end of the frame or potentially on the next network tick. (#3126) - The Debug Simulator section of the Unity Transport component will now be hidden if Unity Transport 2.0 or later is installed. It was already non-functional in that situation and users should instead use the more featureful [Network Simulator](https://docs-multiplayer.unity3d.com/tools/current/tools-network-simulator/) tool from the Multiplayer Tools package. (#3120) diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs index 5e4ce503c0..25f134959a 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs @@ -765,6 +765,13 @@ public virtual void OnGainedOwnership() { } internal void InternalOnGainedOwnership() { UpdateNetworkProperties(); + // New owners need to assure any NetworkVariables they have write permissions + // to are updated so the previous and original values are aligned with the + // current value (primarily for collections). + if (OwnerClientId == NetworkManager.LocalClientId) + { + UpdateNetworkVariableOnOwnershipChanged(); + } OnGainedOwnership(); } @@ -946,20 +953,20 @@ internal void PreVariableUpdate() PreNetworkVariableWrite(); } - internal void VariableUpdate(ulong targetClientId) - { - NetworkVariableUpdate(targetClientId, NetworkBehaviourId); - } - internal readonly List NetworkVariableIndexesToReset = new List(); internal readonly HashSet NetworkVariableIndexesToResetSet = new HashSet(); - private void NetworkVariableUpdate(ulong targetClientId, int behaviourIndex) + internal void NetworkVariableUpdate(ulong targetClientId, int behaviourIndex, bool forceSend = false) { - if (!CouldHaveDirtyNetworkVariables()) + if (!forceSend && !CouldHaveDirtyNetworkVariables()) { return; } + // Getting these ahead of time actually improves performance + var networkManager = NetworkManager; + var networkObject = NetworkObject; + var messageManager = networkManager.MessageManager; + var connectionManager = networkManager.ConnectionManager; for (int j = 0; j < m_DeliveryMappedNetworkVariableIndices.Count; j++) { @@ -982,10 +989,14 @@ private void NetworkVariableUpdate(ulong targetClientId, int behaviourIndex) var message = new NetworkVariableDeltaMessage { NetworkObjectId = NetworkObjectId, - NetworkBehaviourIndex = NetworkObject.GetNetworkBehaviourOrderIndex(this), + NetworkBehaviourIndex = networkObject.GetNetworkBehaviourOrderIndex(this), NetworkBehaviour = this, TargetClientId = targetClientId, - DeliveryMappedNetworkVariableIndex = m_DeliveryMappedNetworkVariableIndices[j] + DeliveryMappedNetworkVariableIndex = m_DeliveryMappedNetworkVariableIndices[j], + // By sending the network delivery we can forward messages immediately as opposed to processing them + // at the end. While this will send updates to clients that cannot read, the handler will ignore anything + // sent to a client that does not have read permissions. + NetworkDelivery = m_DeliveryTypesForNetworkVariableGroups[j] }; // TODO: Serialization is where the IsDirty flag gets changed. // Messages don't get sent from the server to itself, so if we're host and sending to ourselves, @@ -994,7 +1005,7 @@ private void NetworkVariableUpdate(ulong targetClientId, int behaviourIndex) // so we don't have to do this serialization work if we're not going to use the result. if (IsServer && targetClientId == NetworkManager.ServerClientId) { - var tmpWriter = new FastBufferWriter(NetworkManager.MessageManager.NonFragmentedMessageMaxSize, Allocator.Temp, NetworkManager.MessageManager.FragmentedMessageMaxSize); + var tmpWriter = new FastBufferWriter(messageManager.NonFragmentedMessageMaxSize, Allocator.Temp, messageManager.FragmentedMessageMaxSize); using (tmpWriter) { message.Serialize(tmpWriter, message.Version); @@ -1002,7 +1013,7 @@ private void NetworkVariableUpdate(ulong targetClientId, int behaviourIndex) } else { - NetworkManager.ConnectionManager.SendMessage(ref message, m_DeliveryTypesForNetworkVariableGroups[j], targetClientId); + connectionManager.SendMessage(ref message, m_DeliveryTypesForNetworkVariableGroups[j], targetClientId); } } } @@ -1029,6 +1040,26 @@ private bool CouldHaveDirtyNetworkVariables() return false; } + /// + /// Invoked on a new client to assure the previous and original values + /// are synchronized with the current known value. + /// + /// + /// Primarily for collections to assure the previous value(s) is/are the + /// same as the current value(s) in order to not re-send already known entries. + /// + internal void UpdateNetworkVariableOnOwnershipChanged() + { + for (int j = 0; j < NetworkVariableFields.Count; j++) + { + // Only invoke OnInitialize on NetworkVariables the owner can write to + if (NetworkVariableFields[j].CanClientWrite(OwnerClientId)) + { + NetworkVariableFields[j].OnInitialize(); + } + } + } + internal void MarkVariablesDirty(bool dirty) { for (int j = 0; j < NetworkVariableFields.Count; j++) @@ -1037,6 +1068,17 @@ internal void MarkVariablesDirty(bool dirty) } } + internal void MarkOwnerReadVariablesDirty() + { + for (int j = 0; j < NetworkVariableFields.Count; j++) + { + if (NetworkVariableFields[j].ReadPerm == NetworkVariableReadPermission.Owner) + { + NetworkVariableFields[j].SetDirty(true); + } + } + } + /// /// Synchronizes by setting only the NetworkVariable field values that the client has permission to read. /// Note: This is only invoked when first synchronizing a NetworkBehaviour (i.e. late join or spawned NetworkObject) @@ -1067,7 +1109,11 @@ internal void WriteNetworkVariableData(FastBufferWriter writer, ulong targetClie // The way we do packing, any value > 63 in a ushort will use the full 2 bytes to represent. writer.WriteValueSafe((ushort)0); var startPos = writer.Position; - NetworkVariableFields[j].WriteField(writer); + // Write the NetworkVariable field value + // WriteFieldSynchronization will write the current value only if there are no pending changes. + // Otherwise, it will write the previous value if there are pending changes since the pending + // changes will be sent shortly after the client's synchronization. + NetworkVariableFields[j].WriteFieldSynchronization(writer); var size = writer.Position - startPos; writer.Seek(writePos); writer.WriteValueSafe((ushort)size); @@ -1075,7 +1121,11 @@ internal void WriteNetworkVariableData(FastBufferWriter writer, ulong targetClie } else { - NetworkVariableFields[j].WriteField(writer); + // Write the NetworkVariable field value + // WriteFieldSynchronization will write the current value only if there are no pending changes. + // Otherwise, it will write the previous value if there are pending changes since the pending + // changes will be sent shortly after the client's synchronization. + NetworkVariableFields[j].WriteFieldSynchronization(writer); } } else // Only if EnsureNetworkVariableLengthSafety, otherwise just skip diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviourUpdater.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviourUpdater.cs index 67487ce0b6..1453871247 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviourUpdater.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviourUpdater.cs @@ -19,10 +19,15 @@ public class NetworkBehaviourUpdater internal void AddForUpdate(NetworkObject networkObject) { + // Since this is a HashSet, we don't need to worry about duplicate entries m_PendingDirtyNetworkObjects.Add(networkObject); } - internal void NetworkBehaviourUpdate() + /// + /// Sends NetworkVariable deltas + /// + /// internal only, when changing ownership we want to send this before the change in ownership message + internal void NetworkBehaviourUpdate(bool forceSend = false) { #if DEVELOPMENT_BUILD || UNITY_EDITOR m_NetworkBehaviourUpdate.Begin(); @@ -54,7 +59,7 @@ internal void NetworkBehaviourUpdate() // Sync just the variables for just the objects this client sees for (int k = 0; k < dirtyObj.ChildNetworkBehaviours.Count; k++) { - dirtyObj.ChildNetworkBehaviours[k].VariableUpdate(client.ClientId); + dirtyObj.ChildNetworkBehaviours[k].NetworkVariableUpdate(client.ClientId, k, forceSend); } } } @@ -73,7 +78,7 @@ internal void NetworkBehaviourUpdate() } for (int k = 0; k < sobj.ChildNetworkBehaviours.Count; k++) { - sobj.ChildNetworkBehaviours[k].VariableUpdate(NetworkManager.ServerClientId); + sobj.ChildNetworkBehaviours[k].NetworkVariableUpdate(NetworkManager.ServerClientId, k, forceSend); } } } @@ -86,19 +91,28 @@ internal void NetworkBehaviourUpdate() var behaviour = dirtyObj.ChildNetworkBehaviours[k]; for (int i = 0; i < behaviour.NetworkVariableFields.Count; i++) { + // Set to true for NetworkVariable to ignore duplication of the + // "internal original value" for collections support. + behaviour.NetworkVariableFields[i].NetworkUpdaterCheck = true; if (behaviour.NetworkVariableFields[i].IsDirty() && !behaviour.NetworkVariableIndexesToResetSet.Contains(i)) { behaviour.NetworkVariableIndexesToResetSet.Add(i); behaviour.NetworkVariableIndexesToReset.Add(i); } + // Set to true for NetworkVariable to ignore duplication of the + // "internal original value" for collections support. + behaviour.NetworkVariableFields[i].NetworkUpdaterCheck = false; } } } + // Now, reset all the no-longer-dirty variables foreach (var dirtyobj in m_DirtyNetworkObjects) { - dirtyobj.PostNetworkVariableWrite(); + dirtyobj.PostNetworkVariableWrite(forceSend); + // Once done processing, we set the previous owner id to the current owner id + dirtyobj.PreviousOwnerId = dirtyobj.OwnerClientId; } m_DirtyNetworkObjects.Clear(); } diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs index 9dcfcfd51f..3f62c0960b 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs @@ -272,6 +272,8 @@ private GlobalObjectId GetGlobalId() /// public ulong OwnerClientId { get; internal set; } + internal ulong PreviousOwnerId; + /// /// If true, the object will always be replicated as root on clients and the parent will be ignored. /// @@ -1484,6 +1486,14 @@ internal void MarkVariablesDirty(bool dirty) } } + internal void MarkOwnerReadVariablesDirty() + { + for (int i = 0; i < ChildNetworkBehaviours.Count; i++) + { + ChildNetworkBehaviours[i].MarkOwnerReadVariablesDirty(); + } + } + // NGO currently guarantees that the client will receive spawn data for all objects in one network tick. // Children may arrive before their parents; when they do they are stored in OrphanedChildren and then // resolved when their parents arrived. Because we don't send a partial list of spawns (yet), something @@ -1725,11 +1735,11 @@ public void Deserialize(FastBufferReader reader) } } - internal void PostNetworkVariableWrite() + internal void PostNetworkVariableWrite(bool forceSend) { for (int k = 0; k < ChildNetworkBehaviours.Count; k++) { - ChildNetworkBehaviours[k].PostNetworkVariableWrite(); + ChildNetworkBehaviours[k].PostNetworkVariableWrite(forceSend); } } diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ChangeOwnershipMessage.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ChangeOwnershipMessage.cs index b0d1ca7c4d..43949baedc 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ChangeOwnershipMessage.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ChangeOwnershipMessage.cs @@ -45,12 +45,6 @@ public void Handle(ref NetworkContext context) networkObject.InvokeBehaviourOnLostOwnership(); } - // We are new owner. - if (OwnerClientId == networkManager.LocalClientId) - { - networkObject.InvokeBehaviourOnGainedOwnership(); - } - // For all other clients that are neither the former or current owner, update the behaviours' properties if (OwnerClientId != networkManager.LocalClientId && originalOwner != networkManager.LocalClientId) { @@ -60,6 +54,21 @@ public void Handle(ref NetworkContext context) } } + // We are new owner. + if (OwnerClientId == networkManager.LocalClientId) + { + networkObject.InvokeBehaviourOnGainedOwnership(); + } + + if (originalOwner == networkManager.LocalClientId) + { + // Mark any owner read variables as dirty + networkObject.MarkOwnerReadVariablesDirty(); + // Immediately queue any pending deltas and order the message before the + // change in ownership message. + networkManager.BehaviourUpdater.NetworkBehaviourUpdate(true); + } + networkObject.InvokeOwnershipChanged(originalOwner, OwnerClientId); networkManager.NetworkMetrics.TrackOwnershipChangeReceived(context.SenderId, networkObject, context.MessageSize); diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/NetworkVariableDeltaMessage.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/NetworkVariableDeltaMessage.cs index 965286998a..fed46a1337 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/NetworkVariableDeltaMessage.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/NetworkVariableDeltaMessage.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Runtime.CompilerServices; using Unity.Collections; namespace Unity.Netcode @@ -10,9 +11,22 @@ namespace Unity.Netcode /// serialization. This is due to the generally amorphous nature of network variable /// deltas, since they're all driven by custom virtual method overloads. /// + /// + /// Version 1: + /// This version -does not- use the "KeepDirty" approach. Instead, the server will forward any state updates + /// to the connected clients that are not the sender or the server itself. Each NetworkVariable state update + /// included, on a per client basis, is first validated that the client can read the NetworkVariable before + /// being added to the m_ForwardUpdates table. + /// Version 0: + /// The original version uses the "KeepDirty" approach in a client-server network topology where the server + /// proxies state updates by "keeping the NetworkVariable(s) dirty" so it will send state updates + /// at the end of the frame (but could delay until the next tick). + /// internal struct NetworkVariableDeltaMessage : INetworkMessage { - public int Version => 0; + private const int k_ServerDeltaForwardingAndNetworkDelivery = 1; + public int Version => k_ServerDeltaForwardingAndNetworkDelivery; + public ulong NetworkObjectId; public ushort NetworkBehaviourIndex; @@ -21,8 +35,42 @@ internal struct NetworkVariableDeltaMessage : INetworkMessage public ulong TargetClientId; public NetworkBehaviour NetworkBehaviour; + public NetworkDelivery NetworkDelivery; + private FastBufferReader m_ReceivedNetworkVariableData; + private bool m_ForwardingMessage; + + private int m_ReceivedMessageVersion; + + private const string k_Name = "NetworkVariableDeltaMessage"; + + private Dictionary> m_ForwardUpdates; + + private List m_UpdatedNetworkVariables; + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private void WriteNetworkVariable(ref FastBufferWriter writer, ref NetworkVariableBase networkVariable, bool ensureNetworkVariableLengthSafety, int nonfragmentedSize, int fragmentedSize) + { + if (ensureNetworkVariableLengthSafety) + { + var tempWriter = new FastBufferWriter(nonfragmentedSize, Allocator.Temp, fragmentedSize); + networkVariable.WriteDelta(tempWriter); + BytePacker.WriteValueBitPacked(writer, tempWriter.Length); + + if (!writer.TryBeginWrite(tempWriter.Length)) + { + throw new OverflowException($"Not enough space in the buffer to write {nameof(NetworkVariableDeltaMessage)}"); + } + + tempWriter.CopyTo(writer); + } + else + { + networkVariable.WriteDelta(writer); + } + } + public void Serialize(FastBufferWriter writer, int targetVersion) { if (!writer.TryBeginWrite(FastBufferWriter.GetWriteSize(NetworkObjectId) + FastBufferWriter.GetWriteSize(NetworkBehaviourIndex))) @@ -32,16 +80,56 @@ public void Serialize(FastBufferWriter writer, int targetVersion) var obj = NetworkBehaviour.NetworkObject; var networkManager = obj.NetworkManagerOwner; + var typeName = NetworkBehaviour.__getTypeName(); + var nonFragmentedMessageMaxSize = networkManager.MessageManager.NonFragmentedMessageMaxSize; + var fragmentedMessageMaxSize = networkManager.MessageManager.FragmentedMessageMaxSize; + var ensureNetworkVariableLengthSafety = networkManager.NetworkConfig.EnsureNetworkVariableLengthSafety; BytePacker.WriteValueBitPacked(writer, NetworkObjectId); BytePacker.WriteValueBitPacked(writer, NetworkBehaviourIndex); + // If using k_IncludeNetworkDelivery version, then we want to write the network delivery used and if we + // are forwarding state updates then serialize any NetworkVariable states specific to this client. + if (targetVersion >= k_ServerDeltaForwardingAndNetworkDelivery) + { + writer.WriteValueSafe(NetworkDelivery); + // If we are forwarding the message, then proceed to forward state updates specific to the targeted client + if (m_ForwardingMessage) + { + for (int i = 0; i < NetworkBehaviour.NetworkVariableFields.Count; i++) + { + var startingSize = writer.Length; + var networkVariable = NetworkBehaviour.NetworkVariableFields[i]; + var shouldWrite = m_ForwardUpdates[TargetClientId].Contains(i); + + // This var does not belong to the currently iterating delivery group. + if (ensureNetworkVariableLengthSafety) + { + if (!shouldWrite) + { + BytePacker.WriteValueBitPacked(writer, (ushort)0); + } + } + else + { + writer.WriteValueSafe(shouldWrite); + } + + if (shouldWrite) + { + WriteNetworkVariable(ref writer, ref networkVariable, ensureNetworkVariableLengthSafety, nonFragmentedMessageMaxSize, fragmentedMessageMaxSize); + networkManager.NetworkMetrics.TrackNetworkVariableDeltaSent(TargetClientId, obj, networkVariable.Name, typeName, writer.Length - startingSize); + } + } + return; + } + } + for (int i = 0; i < NetworkBehaviour.NetworkVariableFields.Count; i++) { if (!DeliveryMappedNetworkVariableIndex.Contains(i)) { - // This var does not belong to the currently iterating delivery group. - if (networkManager.NetworkConfig.EnsureNetworkVariableLengthSafety) + if (ensureNetworkVariableLengthSafety) { BytePacker.WriteValueBitPacked(writer, (ushort)0); } @@ -55,7 +143,6 @@ public void Serialize(FastBufferWriter writer, int targetVersion) var startingSize = writer.Length; var networkVariable = NetworkBehaviour.NetworkVariableFields[i]; - var shouldWrite = networkVariable.IsDirty() && networkVariable.CanClientRead(TargetClientId) && (networkManager.IsServer || networkVariable.CanClientWrite(networkManager.LocalClientId)) && @@ -79,7 +166,7 @@ public void Serialize(FastBufferWriter writer, int targetVersion) shouldWrite = false; } - if (networkManager.NetworkConfig.EnsureNetworkVariableLengthSafety) + if (ensureNetworkVariableLengthSafety) { if (!shouldWrite) { @@ -93,38 +180,22 @@ public void Serialize(FastBufferWriter writer, int targetVersion) if (shouldWrite) { - if (networkManager.NetworkConfig.EnsureNetworkVariableLengthSafety) - { - var tempWriter = new FastBufferWriter(networkManager.MessageManager.NonFragmentedMessageMaxSize, Allocator.Temp, networkManager.MessageManager.FragmentedMessageMaxSize); - NetworkBehaviour.NetworkVariableFields[i].WriteDelta(tempWriter); - BytePacker.WriteValueBitPacked(writer, tempWriter.Length); - - if (!writer.TryBeginWrite(tempWriter.Length)) - { - throw new OverflowException($"Not enough space in the buffer to write {nameof(NetworkVariableDeltaMessage)}"); - } - - tempWriter.CopyTo(writer); - } - else - { - networkVariable.WriteDelta(writer); - } - networkManager.NetworkMetrics.TrackNetworkVariableDeltaSent( - TargetClientId, - obj, - networkVariable.Name, - NetworkBehaviour.__getTypeName(), - writer.Length - startingSize); + WriteNetworkVariable(ref writer, ref networkVariable, ensureNetworkVariableLengthSafety, nonFragmentedMessageMaxSize, fragmentedMessageMaxSize); + networkManager.NetworkMetrics.TrackNetworkVariableDeltaSent(TargetClientId, obj, networkVariable.Name, typeName, writer.Length - startingSize); } } } public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int receivedMessageVersion) { + m_ReceivedMessageVersion = receivedMessageVersion; ByteUnpacker.ReadValueBitPacked(reader, out NetworkObjectId); ByteUnpacker.ReadValueBitPacked(reader, out NetworkBehaviourIndex); - + // If we are using the k_IncludeNetworkDelivery message version, then read the NetworkDelivery used + if (receivedMessageVersion >= k_ServerDeltaForwardingAndNetworkDelivery) + { + reader.ReadValueSafe(out NetworkDelivery); + } m_ReceivedNetworkVariableData = reader; return true; @@ -136,7 +207,11 @@ public void Handle(ref NetworkContext context) if (networkManager.SpawnManager.SpawnedObjects.TryGetValue(NetworkObjectId, out NetworkObject networkObject)) { + var ensureNetworkVariableLengthSafety = networkManager.NetworkConfig.EnsureNetworkVariableLengthSafety; var networkBehaviour = networkObject.GetNetworkBehaviourAtOrderIndex(NetworkBehaviourIndex); + var isServerAndDeltaForwarding = m_ReceivedMessageVersion >= k_ServerDeltaForwardingAndNetworkDelivery && networkManager.IsServer; + var markNetworkVariableDirty = m_ReceivedMessageVersion >= k_ServerDeltaForwardingAndNetworkDelivery ? false : networkManager.IsServer; + m_UpdatedNetworkVariables = new List(); if (networkBehaviour == null) { @@ -147,13 +222,31 @@ public void Handle(ref NetworkContext context) } else { + // (For client-server) As opposed to worrying about adding additional processing on the server to send NetworkVariable + // updates at the end of the frame, we now track all NetworkVariable state updates, per client, that need to be forwarded + // to the client. This creates a list of all remaining connected clients that could have updates applied. + if (isServerAndDeltaForwarding) + { + m_ForwardUpdates = new Dictionary>(); + foreach (var clientId in networkManager.ConnectedClientsIds) + { + if (clientId == context.SenderId || clientId == networkManager.LocalClientId || !networkObject.Observers.Contains(clientId)) + { + continue; + } + m_ForwardUpdates.Add(clientId, new List()); + } + } + + // Update NetworkVariable Fields for (int i = 0; i < networkBehaviour.NetworkVariableFields.Count; i++) { int varSize = 0; - if (networkManager.NetworkConfig.EnsureNetworkVariableLengthSafety) + var networkVariable = networkBehaviour.NetworkVariableFields[i]; + + if (ensureNetworkVariableLengthSafety) { ByteUnpacker.ReadValueBitPacked(m_ReceivedNetworkVariableData, out varSize); - if (varSize == 0) { continue; @@ -168,8 +261,6 @@ public void Handle(ref NetworkContext context) } } - var networkVariable = networkBehaviour.NetworkVariableFields[i]; - if (networkManager.IsServer && !networkVariable.CanClientWrite(context.SenderId)) { // we are choosing not to fire an exception here, because otherwise a malicious client could use this to crash the server @@ -197,12 +288,57 @@ public void Handle(ref NetworkContext context) NetworkLog.LogError($"Client wrote to {typeof(NetworkVariable<>).Name} without permission. No more variables can be read. This is critical. => {nameof(NetworkObjectId)}: {NetworkObjectId} - {nameof(NetworkObject.GetNetworkBehaviourOrderIndex)}(): {networkObject.GetNetworkBehaviourOrderIndex(networkBehaviour)} - VariableIndex: {i}"); NetworkLog.LogError($"[{networkVariable.GetType().Name}]"); } - return; } int readStartPos = m_ReceivedNetworkVariableData.Position; - networkVariable.ReadDelta(m_ReceivedNetworkVariableData, networkManager.IsServer); + if (ensureNetworkVariableLengthSafety) + { + var remainingBufferSize = m_ReceivedNetworkVariableData.Length - m_ReceivedNetworkVariableData.Position; + if (varSize > (remainingBufferSize)) + { + UnityEngine.Debug.LogError($"[{networkBehaviour.name}][Delta State Read Error] Expecting to read {varSize} but only {remainingBufferSize} remains!"); + return; + } + } + + // Added a try catch here to assure any failure will only fail on this one message and not disrupt the stack + try + { + // Read the delta + networkVariable.ReadDelta(m_ReceivedNetworkVariableData, markNetworkVariableDirty); + + // Add the NetworkVariable field index so we can invoke the PostDeltaRead + m_UpdatedNetworkVariables.Add(i); + } + catch (Exception ex) + { + UnityEngine.Debug.LogException(ex); + return; + } + + // (For client-server) As opposed to worrying about adding additional processing on the server to send NetworkVariable + // updates at the end of the frame, we now track all NetworkVariable state updates, per client, that need to be forwarded + // to the client. This happens once the server is finished processing all state updates for this message. + if (isServerAndDeltaForwarding) + { + foreach (var forwardEntry in m_ForwardUpdates) + { + // Only track things that the client can read + if (networkVariable.CanClientRead(forwardEntry.Key)) + { + // If the object is about to be shown to the client then don't send an update as it will + // send a full update when shown. + if (networkManager.SpawnManager.ObjectsToShowToClient.ContainsKey(forwardEntry.Key) && + networkManager.SpawnManager.ObjectsToShowToClient[forwardEntry.Key] + .Contains(networkObject)) + { + continue; + } + forwardEntry.Value.Add(i); + } + } + } networkManager.NetworkMetrics.TrackNetworkVariableDeltaReceived( context.SenderId, @@ -211,7 +347,7 @@ public void Handle(ref NetworkContext context) networkBehaviour.__getTypeName(), context.MessageSize); - if (networkManager.NetworkConfig.EnsureNetworkVariableLengthSafety) + if (ensureNetworkVariableLengthSafety) { if (m_ReceivedNetworkVariableData.Position > (readStartPos + varSize)) { @@ -233,6 +369,40 @@ public void Handle(ref NetworkContext context) } } } + + // If we are using the version of this message that includes network delivery, then + // forward this update to all connected clients (other than the sender and the server). + if (isServerAndDeltaForwarding) + { + var message = new NetworkVariableDeltaMessage() + { + NetworkBehaviour = networkBehaviour, + NetworkBehaviourIndex = NetworkBehaviourIndex, + NetworkObjectId = NetworkObjectId, + m_ForwardingMessage = true, + m_ForwardUpdates = m_ForwardUpdates, + }; + + foreach (var forwardEntry in m_ForwardUpdates) + { + // Only forward updates to any client that has visibility to the state updates included in this message + if (forwardEntry.Value.Count > 0) + { + message.TargetClientId = forwardEntry.Key; + networkManager.ConnectionManager.SendMessage(ref message, NetworkDelivery, forwardEntry.Key); + } + } + } + + // This should be always invoked (client & server) to assure the previous values are set + // !! IMPORTANT ORDER OF OPERATIONS !! (Has to happen after forwarding deltas) + // When a server forwards delta updates to connected clients, it needs to preserve the previous value + // until it is done serializing all valid NetworkVariable field deltas (relative to each client). This + // is invoked after it is done forwarding the deltas. + foreach (var fieldIndex in m_UpdatedNetworkVariables) + { + networkBehaviour.NetworkVariableFields[fieldIndex].PostDeltaRead(); + } } } else diff --git a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs index a489e9dbff..7a6d24bb7c 100644 --- a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs +++ b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs @@ -153,6 +153,13 @@ public override void ReadField(FastBufferReader reader) /// public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta) { + /// This is only invoked by and the only time + /// keepDirtyDelta is set is when it is the server processing. To be able to handle previous + /// versions, we use IsServer to keep the dirty states received and the keepDirtyDelta to + /// actually mark this as dirty and add it to the list of s to + /// be updated. With the forwarding of deltas being handled by , + /// once all clients have been forwarded the dirty events, we clear them by invoking . + var isServer = m_NetworkManager.IsServer; reader.ReadValueSafe(out ushort deltaCount); for (int i = 0; i < deltaCount; i++) { @@ -175,7 +182,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta) }); } - if (keepDirtyDelta) + if (isServer) { m_DirtyEvents.Add(new NetworkListEvent() { @@ -183,7 +190,11 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta) Index = m_List.Length - 1, Value = m_List[m_List.Length - 1] }); - MarkNetworkObjectDirty(); + // Preserve the legacy way of handling this + if (keepDirtyDelta) + { + MarkNetworkObjectDirty(); + } } } break; @@ -213,7 +224,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta) }); } - if (keepDirtyDelta) + if (isServer) { m_DirtyEvents.Add(new NetworkListEvent() { @@ -221,7 +232,11 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta) Index = index, Value = m_List[index] }); - MarkNetworkObjectDirty(); + // Preserve the legacy way of handling this + if (keepDirtyDelta) + { + MarkNetworkObjectDirty(); + } } } break; @@ -247,7 +262,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta) }); } - if (keepDirtyDelta) + if (isServer) { m_DirtyEvents.Add(new NetworkListEvent() { @@ -255,7 +270,11 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta) Index = index, Value = value }); - MarkNetworkObjectDirty(); + // Preserve the legacy way of handling this + if (keepDirtyDelta) + { + MarkNetworkObjectDirty(); + } } } break; @@ -275,7 +294,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta) }); } - if (keepDirtyDelta) + if (isServer) { m_DirtyEvents.Add(new NetworkListEvent() { @@ -283,7 +302,11 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta) Index = index, Value = value }); - MarkNetworkObjectDirty(); + // Preserve the legacy way of handling this + if (keepDirtyDelta) + { + MarkNetworkObjectDirty(); + } } } break; @@ -311,7 +334,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta) }); } - if (keepDirtyDelta) + if (isServer) { m_DirtyEvents.Add(new NetworkListEvent() { @@ -320,7 +343,11 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta) Value = value, PreviousValue = previousValue }); - MarkNetworkObjectDirty(); + // Preserve the legacy way of handling this + if (keepDirtyDelta) + { + MarkNetworkObjectDirty(); + } } } break; @@ -337,13 +364,18 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta) }); } - if (keepDirtyDelta) + if (isServer) { m_DirtyEvents.Add(new NetworkListEvent() { Type = eventType }); - MarkNetworkObjectDirty(); + + // Preserve the legacy way of handling this + if (keepDirtyDelta) + { + MarkNetworkObjectDirty(); + } } } break; @@ -357,6 +389,18 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta) } } + /// + /// + /// For NetworkList, we just need to reset dirty if a server has read deltas + /// + internal override void PostDeltaRead() + { + if (m_NetworkManager.IsServer) + { + ResetDirty(); + } + } + /// public IEnumerator GetEnumerator() { diff --git a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariable.cs b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariable.cs index 938d53401b..6469c9ee73 100644 --- a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariable.cs +++ b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariable.cs @@ -41,6 +41,7 @@ public override void OnInitialize() base.OnInitialize(); m_HasPreviousValue = true; + NetworkVariableSerialization.Duplicate(m_InternalValue, ref m_InternalOriginalValue); NetworkVariableSerialization.Duplicate(m_InternalValue, ref m_PreviousValue); } @@ -56,6 +57,7 @@ public NetworkVariable(T value = default, : base(readPerm, writePerm) { m_InternalValue = value; + m_InternalOriginalValue = default; // Since we start with IsDirty = true, this doesn't need to be duplicated // right away. It won't get read until after ResetDirty() is called, and // the duplicate will be made there. Avoiding calling @@ -65,12 +67,32 @@ public NetworkVariable(T value = default, m_PreviousValue = default; } + /// + /// Resets the NetworkVariable when the associated NetworkObject is not spawned + /// + /// the value to reset the NetworkVariable to (if none specified it resets to the default) + public void Reset(T value = default) + { + if (m_NetworkBehaviour == null || m_NetworkBehaviour != null && !m_NetworkBehaviour.NetworkObject.IsSpawned) + { + m_InternalValue = value; + NetworkVariableSerialization.Duplicate(m_InternalValue, ref m_InternalOriginalValue); + m_PreviousValue = default; + } + } + /// /// The internal value of the NetworkVariable /// [SerializeField] private protected T m_InternalValue; + // The introduction of standard .NET collections caused an issue with permissions since there is no way to detect changes in the + // collection without doing a full comparison. While this approach does consume more memory per collection instance, it is the + // lowest risk approach to resolving the issue where a client with no write permissions could make changes to a collection locally + // which can cause a myriad of issues. + private protected T m_InternalOriginalValue; + private protected T m_PreviousValue; private bool m_HasPreviousValue; @@ -101,6 +123,7 @@ public virtual T Value { T previousValue = m_InternalValue; m_InternalValue = value; + NetworkVariableSerialization.Duplicate(m_InternalValue, ref m_InternalOriginalValue); SetDirty(true); m_IsDisposed = false; OnValueChanged?.Invoke(previousValue, m_InternalValue); @@ -121,6 +144,17 @@ public bool CheckDirtyState(bool forceCheck = false) { var isDirty = base.IsDirty(); + // A client without permissions invoking this method should only check to assure the current value is equal to the last known current value + if (m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId)) + { + // If modifications are detected, then revert back to the last known current value + if (!NetworkVariableSerialization.AreEqual(ref m_InternalValue, ref m_InternalOriginalValue)) + { + NetworkVariableSerialization.Duplicate(m_InternalOriginalValue, ref m_InternalValue); + } + return false; + } + // Compare the previous with the current if not dirty or forcing a check. if ((!isDirty || forceCheck) && !NetworkVariableSerialization.AreEqual(ref m_PreviousValue, ref m_InternalValue)) { @@ -151,6 +185,13 @@ public override void Dispose() } m_InternalValue = default; + + if (m_InternalOriginalValue is IDisposable internalOriginalValueDisposable) + { + internalOriginalValueDisposable.Dispose(); + } + m_InternalOriginalValue = default; + if (m_HasPreviousValue && m_PreviousValue is IDisposable previousValueDisposable) { m_HasPreviousValue = false; @@ -171,6 +212,14 @@ public override void Dispose() /// Whether or not the container is dirty public override bool IsDirty() { + // If the client does not have write permissions but the internal value is determined to be locally modified and we are applying updates, then we should revert + // to the original collection value prior to applying updates (primarily for collections). + if (!NetworkUpdaterCheck && m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId) && !NetworkVariableSerialization.AreEqual(ref m_InternalValue, ref m_InternalOriginalValue)) + { + NetworkVariableSerialization.Duplicate(m_InternalOriginalValue, ref m_InternalValue); + return true; + } + // For most cases we can use the dirty flag. // This doesn't work for cases where we're wrapping more complex types // like INetworkSerializable, NativeList, NativeArray, etc. @@ -182,11 +231,12 @@ public override bool IsDirty() return true; } + var dirty = !NetworkVariableSerialization.AreEqual(ref m_PreviousValue, ref m_InternalValue); + // Cache the dirty value so we don't perform this again if we already know we're dirty // Unfortunately we can't cache the NOT dirty state, because that might change // in between to checks... but the DIRTY state won't change until ResetDirty() // is called. - var dirty = !NetworkVariableSerialization.AreEqual(ref m_PreviousValue, ref m_InternalValue); SetDirty(dirty); return dirty; } @@ -204,6 +254,8 @@ public override void ResetDirty() { m_HasPreviousValue = true; NetworkVariableSerialization.Duplicate(m_InternalValue, ref m_PreviousValue); + // Once updated, assure the original current value is updated for future comparison purposes + NetworkVariableSerialization.Duplicate(m_InternalValue, ref m_InternalOriginalValue); } base.ResetDirty(); } @@ -236,28 +288,64 @@ public override void WriteDelta(FastBufferWriter writer) /// Whether or not the container should keep the dirty delta, or mark the delta as consumed public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta) { - // In order to get managed collections to properly have a previous and current value, we have to - // duplicate the collection at this point before making any modifications to the current. - m_HasPreviousValue = true; - NetworkVariableSerialization.Duplicate(m_InternalValue, ref m_PreviousValue); + // If the client does not have write permissions but the internal value is determined to be locally modified and we are applying updates, then we should revert + // to the original collection value prior to applying updates (primarily for collections). + if (m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId) && !NetworkVariableSerialization.AreEqual(ref m_InternalOriginalValue, ref m_InternalValue)) + { + NetworkVariableSerialization.Duplicate(m_InternalOriginalValue, ref m_InternalValue); + } + NetworkVariableSerialization.ReadDelta(reader, ref m_InternalValue); - // todo: // keepDirtyDelta marks a variable received as dirty and causes the server to send the value to clients // In a prefect world, whether a variable was A) modified locally or B) received and needs retransmit // would be stored in different fields + // LEGACY NOTE: This is only to handle NetworkVariableDeltaMessage Version 0 connections. The updated + // NetworkVariableDeltaMessage no longer uses this approach. if (keepDirtyDelta) { SetDirty(true); } - OnValueChanged?.Invoke(m_PreviousValue, m_InternalValue); } + /// + /// This should be always invoked (client & server) to assure the previous values are set + /// !! IMPORTANT !! + /// When a server forwards delta updates to connected clients, it needs to preserve the previous dirty value(s) + /// until it is done serializing all valid NetworkVariable field deltas (relative to each client). This is invoked + /// after it is done forwarding the deltas at the end of the method. + /// + internal override void PostDeltaRead() + { + // In order to get managed collections to properly have a previous and current value, we have to + // duplicate the collection at this point before making any modifications to the current. + m_HasPreviousValue = true; + NetworkVariableSerialization.Duplicate(m_InternalValue, ref m_PreviousValue); + // Once updated, assure the original current value is updated for future comparison purposes + NetworkVariableSerialization.Duplicate(m_InternalValue, ref m_InternalOriginalValue); + } + /// public override void ReadField(FastBufferReader reader) { + // If the client does not have write permissions but the internal value is determined to be locally modified and we are applying updates, then we should revert + // to the original collection value prior to applying updates (primarily for collections). + if (m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId) && !NetworkVariableSerialization.AreEqual(ref m_InternalOriginalValue, ref m_InternalValue)) + { + NetworkVariableSerialization.Duplicate(m_InternalOriginalValue, ref m_InternalValue); + } + NetworkVariableSerialization.Read(reader, ref m_InternalValue); + // In order to get managed collections to properly have a previous and current value, we have to + // duplicate the collection at this point before making any modifications to the current. + // We duplicate the final value after the read (for ReadField ONLY) so the previous value is at par + // with the current value (since this is only invoked when initially synchronizing). + m_HasPreviousValue = true; + NetworkVariableSerialization.Duplicate(m_InternalValue, ref m_PreviousValue); + + // Once updated, assure the original current value is updated for future comparison purposes + NetworkVariableSerialization.Duplicate(m_InternalValue, ref m_InternalOriginalValue); } /// @@ -265,5 +353,20 @@ public override void WriteField(FastBufferWriter writer) { NetworkVariableSerialization.Write(writer, ref m_InternalValue); } + + internal override void WriteFieldSynchronization(FastBufferWriter writer) + { + // If we have a pending update, then synchronize the client with the previously known + // value since the updated version will be sent on the next tick or next time it is + // set to be updated + if (base.IsDirty() && m_HasPreviousValue) + { + NetworkVariableSerialization.Write(writer, ref m_PreviousValue); + } + else + { + base.WriteFieldSynchronization(writer); + } + } } } diff --git a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs index 8c7db22a24..f35a6c25f7 100644 --- a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs +++ b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs @@ -227,6 +227,12 @@ public virtual void ResetDirty() m_IsDirty = false; } + /// + /// Only used during the NetworkBehaviourUpdater pass and only used for NetworkVariable. + /// This is to bypass duplication of the "original internal value" for collections. + /// + internal bool NetworkUpdaterCheck; + /// /// Gets Whether or not the container is dirty /// @@ -313,6 +319,32 @@ internal ulong OwnerClientId() /// Whether or not the delta should be kept as dirty or consumed public abstract void ReadDelta(FastBufferReader reader, bool keepDirtyDelta); + /// + /// This should be always invoked (client & server) to assure the previous values are set + /// !! IMPORTANT !! + /// When a server forwards delta updates to connected clients, it needs to preserve the previous dirty value(s) + /// until it is done serializing all valid NetworkVariable field deltas (relative to each client). This is invoked + /// after it is done forwarding the deltas at the end of the method. + /// + internal virtual void PostDeltaRead() + { + } + + /// + /// There are scenarios, specifically with collections, where a client could be synchronizing and + /// some NetworkVariables have pending updates. To avoid duplicating entries, this is invoked only + /// when sending the full synchronization information. + /// + /// + /// Derrived classes should send the previous value for synchronization so when the updated value + /// is sent (after synchronizing the client) it will apply the updates. + /// + /// + internal virtual void WriteFieldSynchronization(FastBufferWriter writer) + { + WriteField(writer); + } + /// /// Virtual implementation /// diff --git a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs index 8c1c3ac3d2..ca7c6d2478 100644 --- a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs @@ -245,8 +245,30 @@ internal void RemoveOwnership(NetworkObject networkObject) ChangeOwnership(networkObject, NetworkManager.ServerClientId); } + private Dictionary m_LastChangeInOwnership = new Dictionary(); + private const int k_MaximumTickOwnershipChangeMultiplier = 6; + internal void ChangeOwnership(NetworkObject networkObject, ulong clientId) { + // If ownership changes faster than the latency between the client-server and there are NetworkVariables being updated during ownership changes, + // then notify the user they could potentially lose state updates if developer logging is enabled. + if (m_LastChangeInOwnership.ContainsKey(networkObject.NetworkObjectId) && m_LastChangeInOwnership[networkObject.NetworkObjectId] > Time.realtimeSinceStartup) + { + var hasNetworkVariables = false; + for (int i = 0; i < networkObject.ChildNetworkBehaviours.Count; i++) + { + hasNetworkVariables = networkObject.ChildNetworkBehaviours[i].NetworkVariableFields.Count > 0; + if (hasNetworkVariables) + { + break; + } + } + if (hasNetworkVariables && NetworkManager.LogLevel == LogLevel.Developer) + { + NetworkLog.LogWarningServer($"[Rapid Ownership Change Detected][Potential Loss in State] Detected a rapid change in ownership that exceeds a frequency less than {k_MaximumTickOwnershipChangeMultiplier}x the current network tick rate! Provide at least {k_MaximumTickOwnershipChangeMultiplier}x the current network tick rate between ownership changes to avoid NetworkVariable state loss."); + } + } + if (!NetworkManager.IsServer) { throw new NotServerException("Only the server can change ownership"); @@ -257,22 +279,30 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId) throw new SpawnStateException("Object is not spawned"); } - var previous = networkObject.OwnerClientId; + // Used to distinguish whether a new owner should receive any currently dirty NetworkVariable updates + networkObject.PreviousOwnerId = networkObject.OwnerClientId; + // Assign the new owner networkObject.OwnerClientId = clientId; // Always notify locally on the server when ownership is lost networkObject.InvokeBehaviourOnLostOwnership(); - networkObject.MarkVariablesDirty(true); - NetworkManager.BehaviourUpdater.AddForUpdate(networkObject); - // Server adds entries for all client ownership UpdateOwnershipTable(networkObject, networkObject.OwnerClientId); // Always notify locally on the server when a new owner is assigned networkObject.InvokeBehaviourOnGainedOwnership(); + if (networkObject.PreviousOwnerId == NetworkManager.LocalClientId) + { + // Mark any owner read variables as dirty + networkObject.MarkOwnerReadVariablesDirty(); + // Immediately queue any pending deltas and order the message before the + // change in ownership message. + NetworkManager.BehaviourUpdater.NetworkBehaviourUpdate(true); + } + var message = new ChangeOwnershipMessage { NetworkObjectId = networkObject.NetworkObjectId, @@ -292,7 +322,15 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId) /// !!Important!! /// This gets called specifically *after* sending the ownership message so any additional messages that need to proceed an ownership /// change can be sent from NetworkBehaviours that override the - networkObject.InvokeOwnershipChanged(previous, clientId); + networkObject.InvokeOwnershipChanged(networkObject.PreviousOwnerId, clientId); + + // Keep track of the ownership change frequency to assure a user is not exceeding changes faster than 2x the current Tick Rate. + if (!m_LastChangeInOwnership.ContainsKey(networkObject.NetworkObjectId)) + { + m_LastChangeInOwnership.Add(networkObject.NetworkObjectId, 0.0f); + } + var tickFrequency = 1.0f / NetworkManager.NetworkConfig.TickRate; + m_LastChangeInOwnership[networkObject.NetworkObjectId] = Time.realtimeSinceStartup + (tickFrequency * k_MaximumTickOwnershipChangeMultiplier); } internal bool HasPrefab(NetworkObject.SceneObject sceneObject) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/DeferredMessagingTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/DeferredMessagingTests.cs index f85a9cbd41..1cccee03fe 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/DeferredMessagingTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/DeferredMessagingTests.cs @@ -667,7 +667,7 @@ public void WhenMultipleSpawnTriggeredMessagesAreDeferred_TheyAreAllProcessedOnS serverObject.GetComponent().ChangeOwnership(m_ClientNetworkManagers[0].LocalClientId); - WaitForAllClientsToReceive(); + WaitForAllClientsToReceive(); foreach (var client in m_ClientNetworkManagers) { @@ -675,9 +675,9 @@ public void WhenMultipleSpawnTriggeredMessagesAreDeferred_TheyAreAllProcessedOnS Assert.IsTrue(manager.DeferMessageCalled); Assert.IsFalse(manager.ProcessTriggersCalled); - Assert.AreEqual(4, manager.DeferredMessageCountTotal()); - Assert.AreEqual(4, manager.DeferredMessageCountForType(IDeferredNetworkMessageManager.TriggerType.OnSpawn)); - Assert.AreEqual(4, manager.DeferredMessageCountForKey(IDeferredNetworkMessageManager.TriggerType.OnSpawn, serverObject.GetComponent().NetworkObjectId)); + Assert.AreEqual(3, manager.DeferredMessageCountTotal()); + Assert.AreEqual(3, manager.DeferredMessageCountForType(IDeferredNetworkMessageManager.TriggerType.OnSpawn)); + Assert.AreEqual(3, manager.DeferredMessageCountForKey(IDeferredNetworkMessageManager.TriggerType.OnSpawn, serverObject.GetComponent().NetworkObjectId)); Assert.AreEqual(0, manager.DeferredMessageCountForType(IDeferredNetworkMessageManager.TriggerType.OnAddPrefab)); AddPrefabsToClient(client); } @@ -809,7 +809,7 @@ public void WhenSpawnTriggeredMessagesAreDeferredBeforeThePrefabIsAdded_AddingTh serverObject.GetComponent().ChangeOwnership(m_ClientNetworkManagers[0].LocalClientId); - WaitForAllClientsToReceive(); + WaitForAllClientsToReceive(); // Validate messages are deferred and pending foreach (var client in m_ClientNetworkManagers) @@ -818,10 +818,10 @@ public void WhenSpawnTriggeredMessagesAreDeferredBeforeThePrefabIsAdded_AddingTh Assert.IsTrue(manager.DeferMessageCalled); Assert.IsFalse(manager.ProcessTriggersCalled); - Assert.AreEqual(5, manager.DeferredMessageCountTotal()); + Assert.AreEqual(4, manager.DeferredMessageCountTotal()); - Assert.AreEqual(4, manager.DeferredMessageCountForType(IDeferredNetworkMessageManager.TriggerType.OnSpawn)); - Assert.AreEqual(4, manager.DeferredMessageCountForKey(IDeferredNetworkMessageManager.TriggerType.OnSpawn, serverObject.GetComponent().NetworkObjectId)); + Assert.AreEqual(3, manager.DeferredMessageCountForType(IDeferredNetworkMessageManager.TriggerType.OnSpawn)); + Assert.AreEqual(3, manager.DeferredMessageCountForKey(IDeferredNetworkMessageManager.TriggerType.OnSpawn, serverObject.GetComponent().NetworkObjectId)); Assert.AreEqual(1, manager.DeferredMessageCountForType(IDeferredNetworkMessageManager.TriggerType.OnAddPrefab)); Assert.AreEqual(1, manager.DeferredMessageCountForKey(IDeferredNetworkMessageManager.TriggerType.OnAddPrefab, serverObject.GetComponent().GlobalObjectIdHash)); AddPrefabsToClient(client); diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/HiddenVariableTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/HiddenVariableTests.cs index 83d159714a..e545b6d046 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/HiddenVariableTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/HiddenVariableTests.cs @@ -173,11 +173,11 @@ public IEnumerator HiddenVariableTest() var otherClient = m_ServerNetworkManager.ConnectedClientsList[2]; m_NetSpawnedObject = SpawnObject(m_TestNetworkPrefab, m_ClientNetworkManagers[1]).GetComponent(); - yield return RefreshGameObects(4); + yield return RefreshGameObects(NumberOfClients); // === Check spawn occurred yield return WaitForSpawnCount(NumberOfClients + 1); - Debug.Assert(HiddenVariableObject.SpawnCount == NumberOfClients + 1); + AssertOnTimeout($"Timed out waiting for all clients to spawn {m_NetSpawnedObject.name}"); Debug.Log("Objects spawned"); // ==== Set the NetworkVariable value to 2 @@ -210,7 +210,7 @@ public IEnumerator HiddenVariableTest() Debug.Log("Object spawned"); // ==== We need a refresh for the newly re-spawned object - yield return RefreshGameObects(4); + yield return RefreshGameObects(NumberOfClients); currentValueSet = 4; m_NetSpawnedObject.GetComponent().MyNetworkVariable.Value = currentValueSet; diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariableCollectionsTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariableCollectionsTests.cs index 7d7e785575..c0db8f67c8 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariableCollectionsTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariableCollectionsTests.cs @@ -5,12 +5,14 @@ using System.Text; using NUnit.Framework; using Unity.Netcode.TestHelpers.Runtime; +using UnityEngine; using UnityEngine.TestTools; using Random = UnityEngine.Random; namespace Unity.Netcode.RuntimeTests { /// + /// Client-Server only test /// Validates using managed collections with NetworkVariable. /// Managed Collections Tested: /// - List @@ -18,24 +20,23 @@ namespace Unity.Netcode.RuntimeTests /// - HashSet /// This also does some testing on nested collections, but does /// not test every possible combination. - /// - [TestFixture(HostOrServer.Host, CollectionTypes.List)] - [TestFixture(HostOrServer.Server, CollectionTypes.List)] + /// + [TestFixture(HostOrServer.Host)] + [TestFixture(HostOrServer.Server)] public class NetworkVariableCollectionsTests : NetcodeIntegrationTest { - public enum CollectionTypes - { - Dictionary, - List, - } - protected override int NumberOfClients => 2; - private CollectionTypes m_CollectionType; + private bool m_EnableDebug; - public NetworkVariableCollectionsTests(HostOrServer hostOrServer, CollectionTypes collectionType) : base(hostOrServer) + public NetworkVariableCollectionsTests(HostOrServer hostOrServer) : base(hostOrServer) { - m_CollectionType = collectionType; + m_EnableDebug = false; + } + + protected override bool OnSetVerboseDebug() + { + return m_EnableDebug; } protected override IEnumerator OnSetup() @@ -50,15 +51,21 @@ protected override IEnumerator OnSetup() return base.OnSetup(); } + private void AddPlayerComponent() where T : ListTestHelperBase + { + var component = m_PlayerPrefab.AddComponent(); + component.SetDebugMode(m_EnableDebug); + } + protected override void OnCreatePlayerPrefab() { - m_PlayerPrefab.AddComponent(); - m_PlayerPrefab.AddComponent(); - m_PlayerPrefab.AddComponent(); - m_PlayerPrefab.AddComponent(); - m_PlayerPrefab.AddComponent(); - m_PlayerPrefab.AddComponent(); - m_PlayerPrefab.AddComponent(); + AddPlayerComponent(); + AddPlayerComponent(); + AddPlayerComponent(); + AddPlayerComponent(); + AddPlayerComponent(); + AddPlayerComponent(); + AddPlayerComponent(); base.OnCreatePlayerPrefab(); } @@ -90,6 +97,7 @@ public IEnumerator TestListBuiltInTypeCollections() { /////////////////////////////////////////////////////////////////////////// // List Single dimension list + compInt = client.LocalClient.PlayerObject.GetComponent(); compIntServer = m_PlayerNetworkObjects[NetworkManager.ServerClientId][client.LocalClientId].GetComponent(); yield return WaitForConditionOrTimeOut(() => compInt.ValidateInstances()); @@ -99,16 +107,34 @@ public IEnumerator TestListBuiltInTypeCollections() AssertOnTimeout($"[Server] Not all instances of client-{compIntServer.OwnerClientId}'s {nameof(ListTestHelperInt)} {compIntServer.name} component match!"); var randomInt = Random.Range(int.MinValue, int.MaxValue); + // Only test restore on non-host clients (otherwise a host is both server and client/owner) + if (!client.IsServer) + { + ////////////////////////////////// + // No Write Owner Add Int + compIntServer.Add(randomInt, ListTestHelperBase.Targets.Owner); + } + ////////////////////////////////// // Owner Add int compInt.Add(randomInt, ListTestHelperBase.Targets.Owner); yield return WaitForConditionOrTimeOut(() => compInt.CompareTrackedChanges(ListTestHelperBase.Targets.Owner)); AssertOnTimeout($"Client-{client.LocalClientId} add failed to synchronize on {nameof(ListTestHelperInt)} {compInt.name}!"); + + // Only test restore on non-host clients (otherwise a host is both server and client/owner) + if (!client.IsServer) + { + ////////////////////////////////// + // No Write Server Add Int + compInt.Add(randomInt, ListTestHelperBase.Targets.Server); + } + ////////////////////////////////// // Server Add int compIntServer.Add(randomInt, ListTestHelperBase.Targets.Server); yield return WaitForConditionOrTimeOut(() => compIntServer.CompareTrackedChanges(ListTestHelperBase.Targets.Server)); AssertOnTimeout($"Server add failed to synchronize on {nameof(ListTestHelperInt)} {compIntServer.name}!"); + ////////////////////////////////// // Owner Remove int var index = Random.Range(0, compInt.ListCollectionOwner.Value.Count - 1); @@ -131,12 +157,39 @@ public IEnumerator TestListBuiltInTypeCollections() //////////////////////////////////// // Owner Change int var valueIntChange = Random.Range(int.MinValue, int.MaxValue); + + // Only test restore on non-host clients (otherwise a host is both server and client/owner) + if (!client.IsServer) + { + // No Write Server Change int with IsDirty restore + compIntServer.ListCollectionOwner.Value[index] = valueIntChange; + compIntServer.ListCollectionOwner.IsDirty(); + yield return WaitForConditionOrTimeOut(() => compIntServer.CompareTrackedChanges(ListTestHelperBase.Targets.Owner)); + AssertOnTimeout($"Server change failed to restore on {nameof(ListTestHelperInt)} {compInt.name}!"); + + // No Write Server Change int with owner state update override + compIntServer.ListCollectionOwner.Value[index] = valueIntChange; + } compInt.ListCollectionOwner.Value[index] = valueIntChange; compInt.ListCollectionOwner.CheckDirtyState(); yield return WaitForConditionOrTimeOut(() => compInt.CompareTrackedChanges(ListTestHelperBase.Targets.Owner)); AssertOnTimeout($"Client-{client.LocalClientId} change failed to synchronize on {nameof(ListTestHelperInt)} {compInt.name}!"); + ////////////////////////////////// // Server Change int + + // Only test restore on non-host clients (otherwise a host is both server and client/owner) + if (!client.IsServer) + { + // No Write Client Change int with IsDirty restore + compInt.ListCollectionServer.Value[index] = valueIntChange; + compInt.ListCollectionServer.IsDirty(); + yield return WaitForConditionOrTimeOut(() => compInt.CompareTrackedChanges(ListTestHelperBase.Targets.Server)); + AssertOnTimeout($"Client-{client.LocalClientId} change failed to restore on {nameof(ListTestHelperInt)} {compInt.name}!"); + + // No Write Client Change int with owner state update override + compInt.ListCollectionServer.Value[index] = valueIntChange; + } compIntServer.ListCollectionServer.Value[index] = valueIntChange; compIntServer.ListCollectionServer.CheckDirtyState(); yield return WaitForConditionOrTimeOut(() => compIntServer.CompareTrackedChanges(ListTestHelperBase.Targets.Server)); @@ -211,13 +264,36 @@ public IEnumerator TestListBuiltInTypeCollections() ////////////////////////////////// // Owner Remove List item index = Random.Range(0, compListInt.ListCollectionOwner.Value.Count - 1); + + // Only test restore on non-host clients (otherwise a host is both server and client/owner) + if (!client.IsServer) + { + compListIntServer.ListCollectionOwner.Value.Remove(compListIntServer.ListCollectionOwner.Value[index]); + compListIntServer.ListCollectionOwner.IsDirty(); + yield return WaitForConditionOrTimeOut(() => compIntServer.CompareTrackedChanges(ListTestHelperBase.Targets.Owner)); + AssertOnTimeout($"Server remove failed to restore on {nameof(ListTestHelperListInt)} {compListIntServer.name}! {compListIntServer.GetLog()}"); + // No Write Server Remove List item with update restore + compListIntServer.ListCollectionOwner.Value.Remove(compListIntServer.ListCollectionOwner.Value[index]); + } compListInt.Remove(compListInt.ListCollectionOwner.Value[index], ListTestHelperBase.Targets.Owner); yield return WaitForConditionOrTimeOut(() => compInt.CompareTrackedChanges(ListTestHelperBase.Targets.Owner)); AssertOnTimeout($"Client-{client.LocalClientId} remove failed to synchronize on {nameof(ListTestHelperListInt)} {compListInt.name}! {compListInt.GetLog()}"); + ////////////////////////////////// // Server Remove List item index = Random.Range(0, compListIntServer.ListCollectionServer.Value.Count - 1); - compListIntServer.Remove(compListIntServer.ListCollectionServer.Value[index], ListTestHelperBase.Targets.Owner); + // Only test restore on non-host clients (otherwise a host is both server and client/owner) + if (!client.IsServer) + { + // No Write Client Remove List item with CheckDirtyState restore + compListInt.Remove(compListInt.ListCollectionServer.Value[index], ListTestHelperBase.Targets.Server); + yield return WaitForConditionOrTimeOut(() => compListInt.CompareTrackedChanges(ListTestHelperBase.Targets.Server)); + AssertOnTimeout($"Client-{client.LocalClientId} remove failed to restore on {nameof(ListTestHelperListInt)} {compListIntServer.name}! {compListIntServer.GetLog()}"); + + // No Write Client Remove List item with update restore + compListInt.Remove(compListInt.ListCollectionServer.Value[index], ListTestHelperBase.Targets.Server); + } + compListIntServer.Remove(compListIntServer.ListCollectionServer.Value[index], ListTestHelperBase.Targets.Server); yield return WaitForConditionOrTimeOut(() => compListIntServer.CompareTrackedChanges(ListTestHelperBase.Targets.Server)); AssertOnTimeout($"Server remove failed to synchronize on {nameof(ListTestHelperListInt)} {compListIntServer.name}! {compListIntServer.GetLog()}"); @@ -370,12 +446,37 @@ public IEnumerator TestListSerializableObjectCollections() //////////////////////////////////// // Owner Change SerializableObject + + // Only test restore on non-host clients (otherwise a host is both server and client/owner) + if (!client.IsServer) + { + // No Write Server Remove Serializable item with IsDirty restore + compObjectServer.ListCollectionOwner.Value[index] = SerializableObject.GetRandomObject(); + compObjectServer.ListCollectionOwner.IsDirty(); + yield return WaitForConditionOrTimeOut(() => compObjectServer.CompareTrackedChanges(ListTestHelperBase.Targets.Owner)); + AssertOnTimeout($"Server change failed to restore on {nameof(ListTestHelperSerializableObject)} {compObjectServer.name}!"); + + // No Write Server Remove Serializable item with owner state update restore + compObjectServer.ListCollectionOwner.Value[index] = SerializableObject.GetRandomObject(); + } compObject.ListCollectionOwner.Value[index] = SerializableObject.GetRandomObject(); compObject.ListCollectionOwner.CheckDirtyState(); yield return WaitForConditionOrTimeOut(() => compObject.CompareTrackedChanges(ListTestHelperBase.Targets.Owner)); AssertOnTimeout($"Client-{client.LocalClientId} change failed to synchronize on {nameof(ListTestHelperSerializableObject)} {compObject.name}!"); ////////////////////////////////// // Server Change SerializableObject + // Only test restore on non-host clients (otherwise a host is both server and client/owner) + if (!client.IsServer) + { + // No Write Client Remove Serializable item with IsDirty restore + compObject.ListCollectionServer.Value[index] = SerializableObject.GetRandomObject(); + compObject.ListCollectionServer.IsDirty(); + yield return WaitForConditionOrTimeOut(() => compObjectServer.CompareTrackedChanges(ListTestHelperBase.Targets.Server)); + AssertOnTimeout($"Client-{client.LocalClientId} change failed to restore on {nameof(ListTestHelperSerializableObject)} {compObjectServer.name}!"); + + // No Write Client Remove Serializable item with owner state update restore + compObject.ListCollectionServer.Value[index] = SerializableObject.GetRandomObject(); + } compObjectServer.ListCollectionServer.Value[index] = SerializableObject.GetRandomObject(); compObjectServer.ListCollectionServer.CheckDirtyState(); yield return WaitForConditionOrTimeOut(() => compObjectServer.CompareTrackedChanges(ListTestHelperBase.Targets.Server)); @@ -427,7 +528,7 @@ public IEnumerator TestListSerializableObjectCollections() AssertOnTimeout($"[Server] Not all instances of client-{compObjectServer.OwnerClientId}'s {nameof(ListTestHelperSerializableObject)} {compObjectServer.name} component match!"); /////////////////////////////////////////////////////////////////////////// - // List> Nested List Validation + // List> Nested List Validation compListObject = client.LocalClient.PlayerObject.GetComponent(); compListObjectServer = m_PlayerNetworkObjects[NetworkManager.ServerClientId][client.LocalClientId].GetComponent(); yield return WaitForConditionOrTimeOut(() => compListObject.ValidateInstances()); @@ -437,24 +538,24 @@ public IEnumerator TestListSerializableObjectCollections() AssertOnTimeout($"[Server] Not all instances of client-{compListObjectServer.OwnerClientId}'s {nameof(ListTestHelperListSerializableObject)} {compListObjectServer.name} component match! {compListObjectServer.GetLog()}"); ////////////////////////////////// - // Owner Add List item + // Owner Add List item compListObject.Add(SerializableObject.GetListOfRandomObjects(5), ListTestHelperBase.Targets.Owner); yield return WaitForConditionOrTimeOut(() => compListObject.CompareTrackedChanges(ListTestHelperBase.Targets.Owner)); AssertOnTimeout($"Client-{client.LocalClientId} add failed to synchronize on {nameof(ListTestHelperListSerializableObject)} {compListObject.name}! {compListObject.GetLog()}"); ////////////////////////////////// - // Server Add List item + // Server Add List item compListObjectServer.Add(SerializableObject.GetListOfRandomObjects(5), ListTestHelperBase.Targets.Server); yield return WaitForConditionOrTimeOut(() => compListObjectServer.CompareTrackedChanges(ListTestHelperBase.Targets.Server)); AssertOnTimeout($"Server add failed to synchronize on {nameof(ListTestHelperListSerializableObject)} {compListObjectServer.name}! {compListObjectServer.GetLog()}"); ////////////////////////////////// - // Owner Remove List item + // Owner Remove List item index = Random.Range(0, compListObject.ListCollectionOwner.Value.Count - 1); compListObject.Remove(compListObject.ListCollectionOwner.Value[index], ListTestHelperBase.Targets.Owner); yield return WaitForConditionOrTimeOut(() => compListObject.CompareTrackedChanges(ListTestHelperBase.Targets.Owner)); AssertOnTimeout($"Client-{client.LocalClientId} remove failed to synchronize on {nameof(ListTestHelperListSerializableObject)} {compListObject.name}! {compListObject.GetLog()}"); ////////////////////////////////// - // Server Remove List item + // Server Remove List item index = Random.Range(0, compListObjectServer.ListCollectionServer.Value.Count - 1); compListObjectServer.Remove(compListObjectServer.ListCollectionServer.Value[index], ListTestHelperBase.Targets.Owner); yield return WaitForConditionOrTimeOut(() => compListObjectServer.CompareTrackedChanges(ListTestHelperBase.Targets.Server)); @@ -468,7 +569,7 @@ public IEnumerator TestListSerializableObjectCollections() AssertOnTimeout($"[Server] Not all instances of client-{compListObjectServer.OwnerClientId}'s {nameof(ListTestHelperListSerializableObject)} {compListObjectServer.name} component match! {compListObjectServer.GetLog()}"); //////////////////////////////////// - // Owner Change List item + // Owner Change List item index = Random.Range(0, compListObject.ListCollectionOwner.Value.Count - 1); compListObject.ListCollectionOwner.Value[index] = SerializableObject.GetListOfRandomObjects(5); compListObject.ListCollectionOwner.CheckDirtyState(); @@ -477,7 +578,7 @@ public IEnumerator TestListSerializableObjectCollections() AssertOnTimeout($"Client-{client.LocalClientId} change index ({index}) failed to synchronize on {nameof(ListTestHelperListSerializableObject)} {compListObject.name}! {compListObject.GetLog()}"); ////////////////////////////////// - // Server Change List item + // Server Change List item index = Random.Range(0, compListObjectServer.ListCollectionServer.Value.Count - 1); compListObjectServer.ListCollectionServer.Value[index] = SerializableObject.GetListOfRandomObjects(5); compListObjectServer.ListCollectionServer.CheckDirtyState(); @@ -486,12 +587,12 @@ public IEnumerator TestListSerializableObjectCollections() AssertOnTimeout($"Server change failed to synchronize on {nameof(ListTestHelperListSerializableObject)} {compListObjectServer.name}! {compListObjectServer.GetLog()}"); //////////////////////////////////// - // Owner Add Range of List items + // Owner Add Range of List items compListObject.AddRange(SerializableObject.GetListOfListOfRandomObjects(5, 5), ListTestHelperBase.Targets.Owner); yield return WaitForConditionOrTimeOut(() => compListObject.CompareTrackedChanges(ListTestHelperBase.Targets.Owner)); AssertOnTimeout($"Client-{client.LocalClientId} add range failed to synchronize on {nameof(ListTestHelperListSerializableObject)} {compListObject.name}! {compListObject.GetLog()}"); ////////////////////////////////// - // Server Add Range of List items + // Server Add Range of List items compListObjectServer.AddRange(SerializableObject.GetListOfListOfRandomObjects(5, 5), ListTestHelperBase.Targets.Server); yield return WaitForConditionOrTimeOut(() => compListObjectServer.CompareTrackedChanges(ListTestHelperBase.Targets.Server)); AssertOnTimeout($"Server add range failed to synchronize on {nameof(ListTestHelperListSerializableObject)} {compListObjectServer.name}! {compListObjectServer.GetLog()}"); @@ -503,23 +604,46 @@ public IEnumerator TestListSerializableObjectCollections() AssertOnTimeout($"[Server] Not all instances of client-{compListObjectServer.OwnerClientId}'s {nameof(ListTestHelperListSerializableObject)} {compListObjectServer.name} component match!"); //////////////////////////////////// - // Owner Full Set List> + // Owner Full Set List> compListObject.FullSet(SerializableObject.GetListOfListOfRandomObjects(5, 5), ListTestHelperBase.Targets.Owner); yield return WaitForConditionOrTimeOut(() => compListObject.CompareTrackedChanges(ListTestHelperBase.Targets.Owner)); AssertOnTimeout($"Client-{client.LocalClientId} full set failed to synchronize on {nameof(ListTestHelperListSerializableObject)} {compListObject.name}!"); ////////////////////////////////// - // Server Full Set List> + // Server Full Set List> compListObjectServer.FullSet(SerializableObject.GetListOfListOfRandomObjects(5, 5), ListTestHelperBase.Targets.Server); yield return WaitForConditionOrTimeOut(() => compListObjectServer.CompareTrackedChanges(ListTestHelperBase.Targets.Server)); AssertOnTimeout($"Server full set failed to synchronize on {nameof(ListTestHelperListSerializableObject)} {compListObjectServer.name}!"); //////////////////////////////////// - // Owner Clear List> + // Owner Clear List> + // Only test restore on non-host clients (otherwise a host is both server and client/owner) + if (!client.IsServer) + { + // Server Clear List> with IsDirty restore + compListObjectServer.ListCollectionOwner.Value.Clear(); + compListObjectServer.ListCollectionOwner.IsDirty(); + yield return WaitForConditionOrTimeOut(() => compListObjectServer.CompareTrackedChanges(ListTestHelperBase.Targets.Owner)); + AssertOnTimeout($"Server clear owner collection failed to restore back to last known valid state on {nameof(ListTestHelperListSerializableObject)} {compListObject.name}!"); + // Server Clear List> with update state restore + compListObjectServer.ListCollectionOwner.Value.Clear(); + } compListObject.Clear(ListTestHelperBase.Targets.Owner); yield return WaitForConditionOrTimeOut(() => compListObject.CompareTrackedChanges(ListTestHelperBase.Targets.Owner)); AssertOnTimeout($"Client-{client.LocalClientId} clear failed to synchronize on {nameof(ListTestHelperListSerializableObject)} {compListObject.name}!"); ////////////////////////////////// - // Server Clear List> + // Server Clear List> + // Only test restore on non-host clients (otherwise a host is both server and client/owner) + if (!client.IsServer) + { + // Client Clear List> with IsDirty restore + compListObject.ListCollectionServer.Value.Clear(); + compListObject.ListCollectionServer.IsDirty(); + yield return WaitForConditionOrTimeOut(() => compListObject.CompareTrackedChanges(ListTestHelperBase.Targets.Server)); + AssertOnTimeout($"Client clear owner collection failed to restore back to last known valid state on {nameof(ListTestHelperListSerializableObject)} {compListObject.name}!"); + + // Client Clear List> with update state restore + compListObject.ListCollectionServer.Value.Clear(); + } compListObjectServer.Clear(ListTestHelperBase.Targets.Server); yield return WaitForConditionOrTimeOut(() => compListObjectServer.CompareTrackedChanges(ListTestHelperBase.Targets.Server)); AssertOnTimeout($"Server clear failed to synchronize on {nameof(ListTestHelperListSerializableObject)} {compListObjectServer.name}!"); @@ -539,6 +663,111 @@ private int GetNextKey() return m_CurrentKey; } + private int m_Stage; + + private List m_Clients; + + private bool m_IsInitialized = false; + private StringBuilder m_InitializedStatus = new StringBuilder(); + + private IEnumerator ValidateClients(NetworkManager clientBeingTested, bool initialize = false) + { + VerboseDebug($">>>>>>>>>>>>>>>>>>>>>>>>>[Client-{clientBeingTested.LocalClientId}][{m_Stage}][Validation]<<<<<<<<<<<<<<<<<<<<<<<<< "); + m_Stage++; + var compDictionary = (DictionaryTestHelper)null; + var compDictionaryServer = (DictionaryTestHelper)null; + var className = $"{nameof(DictionaryTestHelper)}"; + var clientsInitialized = new Dictionary(); + + var validateTimeout = new TimeoutHelper(0.25f); + + foreach (var client in m_Clients) + { + var ownerInitialized = false; + var serverInitialized = false; + /////////////////////////////////////////////////////////////////////////// + // Dictionary> nested dictionaries + compDictionary = client.LocalClient.PlayerObject.GetComponent(); + compDictionaryServer = m_PlayerNetworkObjects[NetworkManager.ServerClientId][client.LocalClientId].GetComponent(); + yield return WaitForConditionOrTimeOut(() => compDictionary.ValidateInstances(), validateTimeout); + if (initialize) + { + if (validateTimeout.HasTimedOut()) + { + m_InitializedStatus.AppendLine($"[Client -{client.LocalClientId}][Owner] Failed validation: {compDictionary.GetLog()}"); + } + else + { + m_InitializedStatus.AppendLine($"[Client -{client.LocalClientId}][Owner] Passed validation!"); + } + ownerInitialized = !validateTimeout.HasTimedOut(); + } + else + { + AssertOnTimeout($"[Owner] Not all instances of client-{compDictionary.OwnerClientId}'s {className} {compDictionary.name} component match! {compDictionary.GetLog()}"); + } + + yield return WaitForConditionOrTimeOut(() => compDictionaryServer.ValidateInstances(), validateTimeout); + if (initialize) + { + if (validateTimeout.HasTimedOut()) + { + m_InitializedStatus.AppendLine($"[Client -{client.LocalClientId}][Server] Failed validation: {compDictionaryServer.GetLog()}"); + } + else + { + m_InitializedStatus.AppendLine($"[Client -{client.LocalClientId}][Server] Passed validation!"); + } + serverInitialized = !validateTimeout.HasTimedOut(); + } + else + { + AssertOnTimeout($"[Server] Not all instances of client-{compDictionaryServer.OwnerClientId}'s {className} {compDictionaryServer.name} component match! {compDictionaryServer.GetLog()}"); + } + + if (initialize) + { + clientsInitialized.Add(client.LocalClientId, ownerInitialized & serverInitialized); + } + } + + if (initialize) + { + m_IsInitialized = true; + foreach (var entry in clientsInitialized) + { + if (!entry.Value) + { + m_IsInitialized = false; + break; + } + } + } + } + + private void ValidateClientsFlat(NetworkManager clientBeingTested) + { + if (!m_EnableDebug) + { + return; + } + VerboseDebug($">>>>>>>>>>>>>>>>>>>>>>>>>[{clientBeingTested.name}][{m_Stage}][Validation]<<<<<<<<<<<<<<<<<<<<<<<<< "); + m_Stage++; + var compDictionary = (DictionaryTestHelper)null; + var compDictionaryServer = (DictionaryTestHelper)null; + var className = $"{nameof(DictionaryTestHelper)}"; + foreach (var client in m_Clients) + { + /////////////////////////////////////////////////////////////////////////// + // Dictionary> nested dictionaries + compDictionary = client.LocalClient.PlayerObject.GetComponent(); + compDictionaryServer = m_PlayerNetworkObjects[NetworkManager.ServerClientId][client.LocalClientId].GetComponent(); + Assert.True(compDictionary.ValidateInstances(), $"[Owner] Not all instances of client-{compDictionary.OwnerClientId}'s {className} {compDictionary.name} component match! {compDictionary.GetLog()}"); + Assert.True(compDictionaryServer.ValidateInstances(), $"[Server] Not all instances of client-{compDictionaryServer.OwnerClientId}'s {className} {compDictionaryServer.name} component match! {compDictionaryServer.GetLog()}"); + } + } + + [UnityTest] public IEnumerator TestDictionaryCollections() { @@ -546,15 +775,47 @@ public IEnumerator TestDictionaryCollections() var compDictionaryServer = (DictionaryTestHelper)null; var className = $"{nameof(DictionaryTestHelper)}"; - var clientList = m_ClientNetworkManagers.ToList(); + m_Clients = m_ClientNetworkManagers.ToList(); if (m_ServerNetworkManager.IsHost) { - clientList.Insert(0, m_ServerNetworkManager); + m_Clients.Insert(0, m_ServerNetworkManager); } m_CurrentKey = 1000; - foreach (var client in clientList) + if (m_EnableDebug) + { + VerboseDebug(">>>>>>>>>>>>>>>>>>>>>>>>>>>>> Init Values <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<"); + foreach (var client in m_Clients) + { + compDictionary = client.LocalClient.PlayerObject.GetComponent(); + compDictionary.InitValues(); + compDictionaryServer = m_PlayerNetworkObjects[NetworkManager.ServerClientId][client.LocalClientId].GetComponent(); + compDictionaryServer.InitValues(); + } + VerboseDebug(">>>>>>>>>>>>>>>>>>>>>>>>>>>>> Init Check <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<"); + var count = 0; + while (count < 3) + { + m_InitializedStatus.Clear(); + foreach (var client in m_Clients) + { + yield return ValidateClients(client, true); + } + if (m_IsInitialized) + { + break; + } + count++; + m_Stage = 0; + } + + Assert.IsTrue(m_IsInitialized, $"Not all clients synchronized properly!\n {m_InitializedStatus.ToString()}"); + VerboseDebug(m_InitializedStatus.ToString()); + } + + VerboseDebug(">>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> BEGIN <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<"); + foreach (var client in m_Clients) { /////////////////////////////////////////////////////////////////////////// // Dictionary> nested dictionaries @@ -562,18 +823,55 @@ public IEnumerator TestDictionaryCollections() compDictionaryServer = m_PlayerNetworkObjects[NetworkManager.ServerClientId][client.LocalClientId].GetComponent(); yield return WaitForConditionOrTimeOut(() => compDictionary.ValidateInstances()); AssertOnTimeout($"[Owner] Not all instances of client-{compDictionary.OwnerClientId}'s {className} {compDictionary.name} component match! {compDictionary.GetLog()}"); - yield return WaitForConditionOrTimeOut(() => compDictionaryServer.ValidateInstances()); AssertOnTimeout($"[Server] Not all instances of client-{compDictionaryServer.OwnerClientId}'s {className} {compDictionaryServer.name} component match! {compDictionaryServer.GetLog()}"); ////////////////////////////////// // Owner Add SerializableObject Entry - compDictionary.Add((GetNextKey(), SerializableObject.GetRandomObject()), ListTestHelperBase.Targets.Owner); + var newEntry = (GetNextKey(), SerializableObject.GetRandomObject()); + // Only test restore on non-host clients (otherwise a host is both server and client/owner) + if (!client.IsServer) + { + // Server-side add same key and SerializableObject prior to being added to the owner side + compDictionaryServer.ListCollectionOwner.Value.Add(newEntry.Item1, newEntry.Item2); + // Checking if dirty on server side should revert back to origina known current dictionary state + compDictionaryServer.ListCollectionOwner.IsDirty(); + yield return WaitForConditionOrTimeOut(() => compDictionaryServer.CompareTrackedChanges(ListTestHelperBase.Targets.Owner)); + AssertOnTimeout($"Server add to owner write collection property failed to restore on {className} {compDictionaryServer.name}! {compDictionaryServer.GetLog()}"); + // Server-side add the same key and SerializableObject to owner write permission (would throw key exists exception too if previous failed) + compDictionaryServer.ListCollectionOwner.Value.Add(newEntry.Item1, newEntry.Item2); + // Server-side add a completely new key and SerializableObject to to owner write permission property + compDictionaryServer.ListCollectionOwner.Value.Add(GetNextKey(), SerializableObject.GetRandomObject()); + // Both should be overridden by the owner-side update + + } + VerboseDebug($"[{compDictionary.name}][Owner] Adding Key: {newEntry.Item1}"); + // Add key and SerializableObject to owner side + compDictionary.Add(newEntry, ListTestHelperBase.Targets.Owner); yield return WaitForConditionOrTimeOut(() => compDictionary.CompareTrackedChanges(ListTestHelperBase.Targets.Owner)); AssertOnTimeout($"Client-{client.LocalClientId} add failed to synchronize on {className} {compDictionary.name}! {compDictionary.GetLog()}"); + + ValidateClientsFlat(client); ////////////////////////////////// // Server Add SerializableObject Entry - compDictionaryServer.Add((GetNextKey(), SerializableObject.GetRandomObject()), ListTestHelperBase.Targets.Server); + newEntry = (GetNextKey(), SerializableObject.GetRandomObject()); + // Only test restore on non-host clients (otherwise a host is both server and client/owner) + if (!client.IsServer) + { + // Client-side add same key and SerializableObject to server write permission property + compDictionary.ListCollectionServer.Value.Add(newEntry.Item1, newEntry.Item2); + // Checking if dirty on client side should revert back to origina known current dictionary state + compDictionary.ListCollectionServer.IsDirty(); + yield return WaitForConditionOrTimeOut(() => compDictionary.CompareTrackedChanges(ListTestHelperBase.Targets.Server)); + AssertOnTimeout($"Client-{client.LocalClientId} add to server write collection property failed to restore on {className} {compDictionary.name}! {compDictionary.GetLog()}"); + // Client-side add the same key and SerializableObject to server write permission property (would throw key exists exception too if previous failed) + compDictionary.ListCollectionServer.Value.Add(newEntry.Item1, newEntry.Item2); + // Client-side add a completely new key and SerializableObject to to server write permission property + compDictionary.ListCollectionServer.Value.Add(GetNextKey(), SerializableObject.GetRandomObject()); + // Both should be overridden by the server-side update + } + VerboseDebug($"[{compDictionaryServer.name}][Server] Adding Key: {newEntry.Item1}"); + compDictionaryServer.Add(newEntry, ListTestHelperBase.Targets.Server); yield return WaitForConditionOrTimeOut(() => compDictionaryServer.CompareTrackedChanges(ListTestHelperBase.Targets.Server)); AssertOnTimeout($"Server add failed to synchronize on {className} {compDictionaryServer.name}! {compDictionaryServer.GetLog()}"); ////////////////////////////////// @@ -583,10 +881,11 @@ public IEnumerator TestDictionaryCollections() compDictionary.Remove(valueInt, ListTestHelperBase.Targets.Owner); yield return WaitForConditionOrTimeOut(() => compDictionary.CompareTrackedChanges(ListTestHelperBase.Targets.Owner)); AssertOnTimeout($"Client-{client.LocalClientId} remove failed to synchronize on {className} {compDictionary.name}! {compDictionary.GetLog()}"); + ////////////////////////////////// // Server Remove SerializableObject Entry - index = Random.Range(0, compDictionary.ListCollectionOwner.Value.Keys.Count - 1); - valueInt = compDictionary.ListCollectionOwner.Value.Keys.ToList()[index]; + index = Random.Range(0, compDictionary.ListCollectionServer.Value.Keys.Count - 1); + valueInt = compDictionary.ListCollectionServer.Value.Keys.ToList()[index]; compDictionaryServer.Remove(valueInt, ListTestHelperBase.Targets.Server); yield return WaitForConditionOrTimeOut(() => compDictionaryServer.CompareTrackedChanges(ListTestHelperBase.Targets.Server)); AssertOnTimeout($"Server remove failed to synchronize on {className} {compDictionaryServer.name}! {compDictionaryServer.GetLog()}"); @@ -597,41 +896,101 @@ public IEnumerator TestDictionaryCollections() yield return WaitForConditionOrTimeOut(() => compDictionaryServer.ValidateInstances()); AssertOnTimeout($"[Server] Not all instances of client-{compDictionaryServer.OwnerClientId}'s {className} {compDictionaryServer.name} component match! {compDictionaryServer.GetLog()}"); + ValidateClientsFlat(client); //////////////////////////////////// // Owner Change SerializableObject Entry - index = Random.Range(0, compDictionary.ListCollectionOwner.Value.Keys.Count - 1); - valueInt = compDictionary.ListCollectionOwner.Value.Keys.ToList()[index]; - compDictionary.ListCollectionOwner.Value[valueInt] = SerializableObject.GetRandomObject(); - compDictionary.ListCollectionOwner.CheckDirtyState(); - yield return WaitForConditionOrTimeOut(() => compDictionary.CompareTrackedChanges(ListTestHelperBase.Targets.Owner)); - AssertOnTimeout($"Client-{client.LocalClientId} change failed to synchronize on {className} {compDictionary.name}! {compDictionary.GetLog()}"); + var randomObject = SerializableObject.GetRandomObject(); + if (compDictionary.ListCollectionOwner.Value.Keys.Count != 0) + { + if (compDictionary.ListCollectionOwner.Value.Keys.Count == 1) + { + index = 0; + valueInt = compDictionary.ListCollectionOwner.Value.Keys.ToList()[0]; + } + else + { + index = Random.Range(0, compDictionary.ListCollectionOwner.Value.Keys.Count - 1); + valueInt = compDictionary.ListCollectionOwner.Value.Keys.ToList()[index]; + } + + // Only test restore on non-host clients (otherwise a host is both server and client/owner) + if (!client.IsServer) + { + // Server-side update same key value prior to being updated to the owner side + compDictionaryServer.ListCollectionOwner.Value[valueInt] = randomObject; + // Checking if dirty on server side should revert back to origina known current dictionary state + compDictionaryServer.ListCollectionOwner.IsDirty(); + yield return WaitForConditionOrTimeOut(() => compDictionaryServer.CompareTrackedChanges(ListTestHelperBase.Targets.Owner)); + AssertOnTimeout($"Server update collection entry value to local owner write collection property failed to restore on {className} {compDictionaryServer.name}! {compDictionaryServer.GetLog()}"); + + // Server-side update same key but with different value prior to being updated to the owner side + compDictionaryServer.ListCollectionOwner.Value[valueInt] = SerializableObject.GetRandomObject(); + if (compDictionaryServer.ListCollectionOwner.Value.Keys.Count > 1) + { + // Server-side update different key with different value prior to being updated to the owner side + compDictionaryServer.ListCollectionOwner.Value[compDictionaryServer.ListCollectionOwner.Value.Keys.ToList()[(index + 1) % compDictionaryServer.ListCollectionOwner.Value.Keys.Count]] = SerializableObject.GetRandomObject(); + } + // Owner-side update should force restore to current known value before updating to the owner's state update of the original index and SerializableObject + } + + compDictionary.ListCollectionOwner.Value[valueInt] = randomObject; + compDictionary.ListCollectionOwner.CheckDirtyState(); + yield return WaitForConditionOrTimeOut(() => compDictionary.CompareTrackedChanges(ListTestHelperBase.Targets.Owner)); + AssertOnTimeout($"Client-{client.LocalClientId} change failed to synchronize on {className} {compDictionary.name}! {compDictionary.GetLog()}"); + } + ////////////////////////////////// // Server Change SerializableObject - index = Random.Range(0, compDictionaryServer.ListCollectionOwner.Value.Keys.Count - 1); - valueInt = compDictionaryServer.ListCollectionOwner.Value.Keys.ToList()[index]; - compDictionaryServer.ListCollectionServer.Value[valueInt] = SerializableObject.GetRandomObject(); - compDictionaryServer.ListCollectionServer.CheckDirtyState(); - yield return WaitForConditionOrTimeOut(() => compDictionaryServer.CompareTrackedChanges(ListTestHelperBase.Targets.Server)); - AssertOnTimeout($"Server change failed to synchronize on {className} {compDictionaryServer.name}! {compDictionaryServer.GetLog()}"); + if (compDictionaryServer.ListCollectionServer.Value.Keys.Count != 0) + { + if (compDictionaryServer.ListCollectionServer.Value.Keys.Count == 1) + { + index = 0; + valueInt = compDictionaryServer.ListCollectionServer.Value.Keys.ToList()[0]; + } + else + { + index = Random.Range(0, compDictionaryServer.ListCollectionServer.Value.Keys.Count - 1); + valueInt = compDictionaryServer.ListCollectionServer.Value.Keys.ToList()[index]; + } - //////////////////////////////////// - // Owner Full Set Dictionary - compDictionary.FullSet(DictionaryTestHelper.GetDictionaryValues(), ListTestHelperBase.Targets.Owner); - yield return WaitForConditionOrTimeOut(() => compDictionary.CompareTrackedChanges(ListTestHelperBase.Targets.Owner)); - AssertOnTimeout($"Client-{client.LocalClientId} full set failed to synchronize on {className} {compDictionary.name}! {compDictionary.GetLog()}"); - ////////////////////////////////// - // Server Full Set Dictionary - compDictionaryServer.FullSet(DictionaryTestHelper.GetDictionaryValues(), ListTestHelperBase.Targets.Server); - yield return WaitForConditionOrTimeOut(() => compDictionaryServer.CompareTrackedChanges(ListTestHelperBase.Targets.Server)); - AssertOnTimeout($"Server full set failed to synchronize on {className} {compDictionaryServer.name}! {compDictionaryServer.GetLog()}"); + // Only test restore on non-host clients (otherwise a host is both server and client/owner) + if (!client.IsServer) + { + // Owner-side update same key value prior to being updated to the server side + compDictionary.ListCollectionServer.Value[valueInt] = randomObject; + // Checking if dirty on owner side should revert back to origina known current dictionary state + compDictionary.ListCollectionServer.IsDirty(); + yield return WaitForConditionOrTimeOut(() => compDictionary.CompareTrackedChanges(ListTestHelperBase.Targets.Server)); + AssertOnTimeout($"Client-{client.LocalClientId} update collection entry value to local server write collection property failed to restore on {className} {compDictionary.name}! {compDictionary.GetLog()}"); + + // Owner-side update same key but with different value prior to being updated to the server side + compDictionary.ListCollectionServer.Value[valueInt] = SerializableObject.GetRandomObject(); + + if (compDictionary.ListCollectionServer.Value.Keys.Count > 1) + { + // Owner-side update different key with different value prior to being updated to the server side + compDictionary.ListCollectionServer.Value[compDictionary.ListCollectionServer.Value.Keys.ToList()[(index + 1) % compDictionary.ListCollectionServer.Value.Keys.Count]] = SerializableObject.GetRandomObject(); + } + // Server-side update should force restore to current known value before updating to the server's state update of the original index and SerializableObject + } + + compDictionaryServer.ListCollectionServer.Value[valueInt] = SerializableObject.GetRandomObject(); + compDictionaryServer.ListCollectionServer.CheckDirtyState(); + yield return WaitForConditionOrTimeOut(() => compDictionaryServer.CompareTrackedChanges(ListTestHelperBase.Targets.Server)); + AssertOnTimeout($"Server change failed to synchronize on {className} {compDictionaryServer.name}! {compDictionaryServer.GetLog()}"); + } + ValidateClientsFlat(client); //////////////////////////////////// // Owner Clear compDictionary.Clear(ListTestHelperBase.Targets.Owner); + VerboseDebug($"[{compDictionary.name}] Clearing dictionary.."); yield return WaitForConditionOrTimeOut(() => compDictionary.CompareTrackedChanges(ListTestHelperBase.Targets.Owner)); AssertOnTimeout($"Client-{client.LocalClientId} clear failed to synchronize on {className} {compDictionary.name}! {compDictionary.GetLog()}"); ////////////////////////////////// // Server Clear + VerboseDebug($"[{compDictionaryServer.name}] Clearing dictionary.."); compDictionaryServer.Clear(ListTestHelperBase.Targets.Server); yield return WaitForConditionOrTimeOut(() => compDictionaryServer.CompareTrackedChanges(ListTestHelperBase.Targets.Server)); AssertOnTimeout($"Server clear failed to synchronize on {className} {compDictionaryServer.name}! {compDictionaryServer.GetLog()}"); @@ -641,6 +1000,22 @@ public IEnumerator TestDictionaryCollections() yield return WaitForConditionOrTimeOut(() => compDictionaryServer.ValidateInstances()); AssertOnTimeout($"[Server] Not all instances of client-{compDictionaryServer.OwnerClientId}'s {className} {compDictionaryServer.name} component match! {compDictionaryServer.GetLog()}"); + + //////////////////////////////////// + // Owner Full Set Dictionary + compDictionary.FullSet(DictionaryTestHelper.GetDictionaryValues(), ListTestHelperBase.Targets.Owner); + yield return WaitForConditionOrTimeOut(() => compDictionary.CompareTrackedChanges(ListTestHelperBase.Targets.Owner)); + AssertOnTimeout($"Client-{client.LocalClientId} full set failed to synchronize on {className} {compDictionary.name}! {compDictionary.GetLog()}"); + ////////////////////////////////// + // Server Full Set Dictionary + compDictionaryServer.FullSet(DictionaryTestHelper.GetDictionaryValues(), ListTestHelperBase.Targets.Server); + yield return WaitForConditionOrTimeOut(() => compDictionaryServer.CompareTrackedChanges(ListTestHelperBase.Targets.Server)); + AssertOnTimeout($"Server full set failed to synchronize on {className} {compDictionaryServer.name}! {compDictionaryServer.GetLog()}"); + if (m_EnableDebug) + { + yield return ValidateClients(client); + m_Stage = 0; + } } } @@ -837,6 +1212,487 @@ public IEnumerator TestHashSetBuiltInTypeCollections() } } + [TestFixture(HostOrServer.Host, CollectionTypes.List)] + [TestFixture(HostOrServer.Host, CollectionTypes.Dictionary)] + [TestFixture(HostOrServer.Server, CollectionTypes.List)] + [TestFixture(HostOrServer.Server, CollectionTypes.Dictionary)] + public class NetworkVariableCollectionsChangingTests : NetcodeIntegrationTest + { + protected override int NumberOfClients => 2; + public enum CollectionTypes + { + Dictionary, + List, + } + private StringBuilder m_ErrorLog = new StringBuilder(); + private CollectionTypes m_CollectionType; + private GameObject m_TestPrefab; + private NetworkObject m_Instance; + + public NetworkVariableCollectionsChangingTests(HostOrServer hostOrServer, CollectionTypes collectionType) : base(hostOrServer) + { + m_CollectionType = collectionType; + } + + protected override void OnServerAndClientsCreated() + { + m_TestPrefab = CreateNetworkObjectPrefab("TestObject"); + if (m_CollectionType == CollectionTypes.Dictionary) + { + m_TestPrefab.AddComponent(); + } + else + { + m_TestPrefab.AddComponent(); + } + base.OnServerAndClientsCreated(); + } + + private bool AllInstancesSpawned() + { + if (!m_ServerNetworkManager.SpawnManager.SpawnedObjects.ContainsKey(m_Instance.NetworkObjectId)) + { + return false; + } + + foreach (var client in m_ClientNetworkManagers) + { + if (!client.SpawnManager.SpawnedObjects.ContainsKey(m_Instance.NetworkObjectId)) + { + return false; + } + } + return true; + } + + private Dictionary m_NetworkManagers = new Dictionary(); + + private bool ValidateAllInstances() + { + if (!m_NetworkManagers.ContainsKey(m_Instance.OwnerClientId)) + { + return false; + } + + if (!m_NetworkManagers[m_Instance.OwnerClientId].SpawnManager.SpawnedObjects.ContainsKey(m_Instance.NetworkObjectId)) + { + return false; + } + + var ownerNetworkManager = m_NetworkManagers[m_Instance.OwnerClientId]; + + var ownerClientInstance = m_NetworkManagers[m_Instance.OwnerClientId].SpawnManager.SpawnedObjects[m_Instance.NetworkObjectId].GetComponent(); + + foreach (var client in m_NetworkManagers) + { + if (client.Value == ownerNetworkManager) + { + continue; + } + + var otherInstance = client.Value.SpawnManager.SpawnedObjects[m_Instance.NetworkObjectId].GetComponent(); + if (!ownerClientInstance.ValidateAgainst(otherInstance)) + { + return false; + } + } + return true; + } + + private bool OwnershipChangedOnAllClients(ulong expectedOwner) + { + m_ErrorLog.Clear(); + foreach (var client in m_NetworkManagers) + { + var otherInstance = client.Value.SpawnManager.SpawnedObjects[m_Instance.NetworkObjectId].GetComponent(); + if (otherInstance.OwnerClientId != expectedOwner) + { + m_ErrorLog.AppendLine($"Client-{client.Value.LocalClientId} instance of {m_Instance.name} still shows the owner is Client-{otherInstance.OwnerClientId} when it should be Client-{expectedOwner}!"); + return false; + } + } + return true; + } + + private BaseCollectionUpdateHelper GetOwnerInstance() + { + var ownerNetworkManager = m_NetworkManagers[m_Instance.OwnerClientId]; + return m_NetworkManagers[m_Instance.OwnerClientId].SpawnManager.SpawnedObjects[m_Instance.NetworkObjectId].GetComponent(); + } + + /// + /// Gets the authority instance. + /// Client-Server: will always return the server-side instance + /// Distributed Authority: will always return the owner + /// + /// authority instance + private BaseCollectionUpdateHelper GetAuthorityInstance() + { + return m_ServerNetworkManager.SpawnManager.SpawnedObjects[m_Instance.NetworkObjectId].GetComponent(); + } + + [UnityTest] + public IEnumerator CollectionAndOwnershipChangingTest() + { + BaseCollectionUpdateHelper.VerboseMode = m_EnableVerboseDebug; + var runWaitPeriod = new WaitForSeconds(0.5f); + m_NetworkManagers.Clear(); + if (m_UseHost) + { + m_NetworkManagers.Add(m_ServerNetworkManager.LocalClientId, m_ServerNetworkManager); + } + foreach (var client in m_ClientNetworkManagers) + { + m_NetworkManagers.Add(client.LocalClientId, client); + } + + var authorityNetworkManager = !m_UseHost ? m_ClientNetworkManagers[0] : m_ServerNetworkManager; + + var instance = SpawnObject(m_TestPrefab, authorityNetworkManager); + m_Instance = instance.GetComponent(); + var helper = instance.GetComponent(); + var currentOwner = helper.OwnerClientId; + yield return WaitForConditionOrTimeOut(AllInstancesSpawned); + AssertOnTimeout($"[Pre][1st Phase] Timed out waiting for all clients to spawn {m_Instance.name}!"); + helper.SetState(BaseCollectionUpdateHelper.HelperStates.Start); + yield return runWaitPeriod; + + // Update values, validate values, change owner, updates values, and repeat until all clients have been the owner at least once + for (int i = 0; i < 4; i++) + { + helper.SetState(BaseCollectionUpdateHelper.HelperStates.Pause); + yield return WaitForConditionOrTimeOut(ValidateAllInstances); + AssertOnTimeout($"[1st Phase] Timed out waiting for all clients to validdate their values!"); + helper.SetState(BaseCollectionUpdateHelper.HelperStates.Start); + yield return s_DefaultWaitForTick; + + currentOwner = GetAuthorityInstance().ChangeOwner(); + Assert.IsFalse(currentOwner == ulong.MaxValue, "A non-authority instance attempted to change ownership!"); + + yield return WaitForConditionOrTimeOut(() => OwnershipChangedOnAllClients(currentOwner)); + AssertOnTimeout($"[1st Phase] Timed out waiting for all clients to change ownership!\n {m_ErrorLog.ToString()}"); + helper = GetOwnerInstance(); + yield return runWaitPeriod; + } + + // Now reset the values + helper.SetState(BaseCollectionUpdateHelper.HelperStates.Pause); + helper.Clear(); + + // Validate all instances are reset + yield return WaitForConditionOrTimeOut(ValidateAllInstances); + AssertOnTimeout($"[Pre][2nd Phase]Timed out waiting for all clients to validdate their values!"); + helper.SetState(BaseCollectionUpdateHelper.HelperStates.Start); + + // Update, change ownership, and repeat until all clients have been the owner at least once + for (int i = 0; i < 4; i++) + { + yield return runWaitPeriod; + currentOwner = GetAuthorityInstance().ChangeOwner(); + Assert.IsFalse(currentOwner == ulong.MaxValue, "A non-authority instance attempted to change ownership!"); + yield return WaitForConditionOrTimeOut(() => OwnershipChangedOnAllClients(currentOwner)); + AssertOnTimeout($"[2nd Phase] Timed out waiting for all clients to change ownership!"); + helper = GetOwnerInstance(); + } + + helper.SetState(BaseCollectionUpdateHelper.HelperStates.Pause); + yield return WaitForConditionOrTimeOut(ValidateAllInstances); + AssertOnTimeout($"[Last Validate] Timed out waiting for all clients to validdate their values!"); + } + } + + #region COLLECTION CHANGING COMPONENTS + /// + /// Helper class to test adding dictionary entries rapidly with frequent ownership changes. + /// This includes a companion integer that is continually incremented and used as the key value for each entry. + /// + public class DictionaryCollectionUpdateHelper : BaseCollectionUpdateHelper + { + private NetworkVariable> m_DictionaryCollection = new NetworkVariable>(new Dictionary(), NetworkVariableReadPermission.Everyone, NetworkVariableWritePermission.Owner); + private NetworkVariable m_CurrentKeyValue = new NetworkVariable(0, NetworkVariableReadPermission.Everyone, NetworkVariableWritePermission.Owner); + + protected override bool OnValidateAgainst(BaseCollectionUpdateHelper otherHelper) + { + var otherListHelper = otherHelper as DictionaryCollectionUpdateHelper; + var localValues = m_DictionaryCollection.Value; + var otherValues = otherListHelper.m_DictionaryCollection.Value; + + if (localValues.Count != otherValues.Count) + { + return false; + } + + foreach (var entry in m_DictionaryCollection.Value) + { + if (!otherValues.ContainsKey(entry.Key)) + { + return false; + } + + if (entry.Value != otherValues[entry.Key]) + { + return false; + } + } + return true; + } + protected override void OnClear() + { + m_DictionaryCollection.Value.Clear(); + m_DictionaryCollection.CheckDirtyState(); + base.OnClear(); + } + + protected override void AddItem() + { + m_DictionaryCollection.Value.Add(m_CurrentKeyValue.Value, m_CurrentKeyValue.Value); + m_DictionaryCollection.CheckDirtyState(); + m_CurrentKeyValue.Value++; + } + } + + /// + /// Helper class to test adding list entries rapidly with frequent ownership changes + /// + public class ListCollectionUpdateHelper : BaseCollectionUpdateHelper + { + private NetworkVariable> m_ListCollection = new NetworkVariable>(new List(), NetworkVariableReadPermission.Everyone, NetworkVariableWritePermission.Owner); + + + protected override bool OnValidateAgainst(BaseCollectionUpdateHelper otherHelper) + { + var otherListHelper = otherHelper as ListCollectionUpdateHelper; + var localValues = m_ListCollection.Value; + var otherValues = otherListHelper.m_ListCollection.Value; + + if (localValues.Count != otherValues.Count) + { + return false; + } + + for (int i = 0; i < localValues.Count - 1; i++) + { + if (localValues[i] != i) + { + return false; + } + + if (localValues[i] != otherValues[i]) + { + return false; + } + } + return true; + } + + protected override void OnClear() + { + m_ListCollection.Value.Clear(); + m_ListCollection.CheckDirtyState(); + base.OnClear(); + } + + protected override void AddItem() + { + m_ListCollection.Value.Add(m_ListCollection.Value.Count); + m_ListCollection.CheckDirtyState(); + } + } + + /// + /// The base class to test rapidly adding items to a collection type + /// + public class BaseCollectionUpdateHelper : NetworkBehaviour + { + public static bool VerboseMode; + private const int k_OwnershipTickDelay = 1; + + public enum HelperStates + { + Stop, + Start, + Pause, + ClearToChangeOwner, + ChangingOwner + } + public HelperStates HelperState { get; private set; } + + private int m_SendClearForOwnershipOnTick; + private ulong m_NextClient = 0; + private ulong m_ClientToSendClear = 0; + + public void SetState(HelperStates helperState) + { + HelperState = helperState; + } + + protected virtual bool OnValidateAgainst(BaseCollectionUpdateHelper otherHelper) + { + return true; + } + + public bool ValidateAgainst(BaseCollectionUpdateHelper otherHelper) + { + return OnValidateAgainst(otherHelper); + } + + public override void OnNetworkSpawn() + { + // Register for tick updates + NetworkManager.NetworkTickSystem.Tick += OnNetworkTick; + + base.OnNetworkSpawn(); + } + public override void OnNetworkDespawn() + { + NetworkManager.NetworkTickSystem.Tick -= OnNetworkTick; + base.OnNetworkDespawn(); + } + + protected virtual void OnClear() + { + } + + public void Clear() + { + OnClear(); + } + + protected virtual void AddItem() + { + } + + private bool CanUpdate() + { + return HelperState == HelperStates.Start; + } + + private void Update() + { + // Exit early if not spawn, updating is not enabled, or is not the owner + if (!IsSpawned || !CanUpdate() || !IsOwner) + { + return; + } + + AddItem(); + } + + protected override void OnOwnershipChanged(ulong previous, ulong current) + { + // When the ownership changes and the client is the owner, then immediately add an item to the collection + if (NetworkManager.LocalClientId == current) + { + AddItem(); + } + base.OnOwnershipChanged(previous, current); + } + + + /// + /// Sets the tick delay period of time to provide all in-flight deltas to be processed. + /// + private void SetTickDelay() + { + m_SendClearForOwnershipOnTick = NetworkManager.ServerTime.Tick + k_OwnershipTickDelay; + } + + /// + /// Changes the ownership + /// + /// next owner or ulong.MaxValue that means the authority did not invoke this method + public ulong ChangeOwner() + { + if (IsServer && !IsOwnershipChanging()) + { + var index = NetworkManager.ConnectedClientsIds.ToList().IndexOf(OwnerClientId); + index++; + index = index % NetworkManager.ConnectedClientsIds.Count; + m_NextClient = NetworkManager.ConnectedClientsIds[index]; + + if (IsOwnedByServer && NetworkManager.IsServer) + { + HelperState = HelperStates.ChangingOwner; + SetTickDelay(); + Log($"Locally changing ownership to Client-{m_NextClient}"); + } + + if (NetworkManager.IsServer && !IsOwnedByServer) + { + // If we are transitioning between a client to the host or client to client, + // send a "heads-up" Rpc to the client prior to changing ownership. The client + // will stop updating for the tick delay period and then send a confirmation + // to the host that it is clear to change ownership. + ChangingOwnershipRpc(RpcTarget.Single(OwnerClientId, RpcTargetUse.Temp)); + Log($"Remotely changing ownership to Client-{m_NextClient}"); + } + + return m_NextClient; + } + + return ulong.MaxValue; + } + + /// + /// Sent by the host to a client when ownership is transitioning from a client to + /// the host or to another client. + /// + [Rpc(SendTo.SpecifiedInParams)] + private void ChangingOwnershipRpc(RpcParams rpcParams = default) + { + // The sender is who we respond to that it is clear to change ownership + m_ClientToSendClear = rpcParams.Receive.SenderClientId; + HelperState = HelperStates.ClearToChangeOwner; + SetTickDelay(); + } + + /// + /// Notification that the current owner has stopped updating and ownership + /// updates can occur without missed updates. + /// + /// + [Rpc(SendTo.SpecifiedInParams)] + private void ChangingOwnershipClearRpc(RpcParams rpcParams = default) + { + HelperState = HelperStates.ChangingOwner; + SetTickDelay(); + Log($"Changing ownership to Client-{m_NextClient} based on ready request."); + } + + private bool IsOwnershipChanging() + { + return HelperState == HelperStates.ClearToChangeOwner || HelperState == HelperStates.ChangingOwner; + } + + private void OnNetworkTick() + { + if (!IsSpawned || !IsOwnershipChanging() || m_SendClearForOwnershipOnTick > NetworkManager.ServerTime.Tick) + { + return; + } + + if (HelperState == HelperStates.ChangingOwner) + { + NetworkObject.ChangeOwnership(m_NextClient); + Log($"Local Change ownership to Client-{m_NextClient} complete! New Owner is {NetworkObject.OwnerClientId} | Expected {m_NextClient}"); + } + else + { + ChangingOwnershipClearRpc(RpcTarget.Single(m_ClientToSendClear, RpcTargetUse.Temp)); + } + HelperState = HelperStates.Stop; + } + + protected void Log(string msg) + { + if (VerboseMode) + { + Debug.Log($"[Client-{NetworkManager.LocalClientId}] {msg}"); + } + } + } + #endregion + #region HASHSET COMPONENT HELPERS public class HashSetBaseTypeTestHelper : ListTestHelperBase, IHashSetTestHelperBase { @@ -1649,6 +2505,14 @@ protected override void OnNetworkPostSpawn() ListCollectionServer.OnValueChanged += OnServerListValuesChanged; ListCollectionOwner.OnValueChanged += OnOwnerListValuesChanged; + if (!IsDebugMode) + { + InitValues(); + } + } + + public void InitValues() + { if (IsServer) { ListCollectionServer.Value = OnSetServerValues(); @@ -1660,8 +2524,8 @@ protected override void OnNetworkPostSpawn() ListCollectionOwner.Value = OnSetOwnerValues(); ListCollectionOwner.CheckDirtyState(); } - base.OnNetworkPostSpawn(); } + public override void OnNetworkDespawn() { ListCollectionServer.OnValueChanged -= OnServerListValuesChanged; @@ -1705,12 +2569,15 @@ public static List> GetListOfListOfRandomObjects(int nu return list; } - - public int IntValue; public long LongValue; public float FloatValue; + public override string ToString() + { + return $"{IntValue},{LongValue},{FloatValue}"; + } + public void NetworkSerialize(BufferSerializer serializer) where T : IReaderWriter { serializer.SerializeValue(ref IntValue); @@ -2602,7 +3469,6 @@ public static void ResetState() Instances.Clear(); } - public NetworkVariable> ListCollectionServer = new NetworkVariable>(new List(), NetworkVariableReadPermission.Everyone, NetworkVariableWritePermission.Server); public NetworkVariable> ListCollectionOwner = new NetworkVariable>(new List(), NetworkVariableReadPermission.Everyone, NetworkVariableWritePermission.Owner); // This tracks what has changed per instance which is used to compare to all other instances @@ -2865,6 +3731,8 @@ public override void OnNetworkDespawn() #region BASE TEST COMPONENT HELPERS public class ListTestHelperBase : NetworkBehaviour { + protected static bool IsDebugMode { get; private set; } + public enum Targets { Server, @@ -2897,6 +3765,10 @@ protected void LogStart() m_StringBuilder.AppendLine($"[Client-{NetworkManager.LocalClientId}][{name}] Log Started."); } + public void SetDebugMode(bool isDebug) + { + IsDebugMode = isDebug; + } public virtual bool CompareTrackedChanges(Targets target) {