Merge branch 'develop-2.0.0' into fix/bufferedlinearinterpolator-jitter

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -18,6 +18,9 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed issue when using a distributed authority network topology and many clients attempt to connect simultaneously the session owner could max-out the maximum in-flight reliable messages allowed, start dropping packets, and some of the connecting clients would fail to fully synchronize. (#3350)
+- Fixed issue when using a distributed authority network topology and scene management was disabled clients would not be able to spawn any new network prefab instances until synchronization was complete. (#3350)
+- Fixed issue where an owner that changes ownership, when using a distributed authority network topology, could yield identical previous and current owner identifiers. This could also cause `NetworkTransform` to fail to change ownership which would leave the previous owner still subscribed to network tick events. (#3347)
 - Fixed issue where the `MaximumInterpolationTime` could not be modified from within the inspector view or runtime. (#3337)
 - Fixed `ChangeOwnership` changing ownership to clients that are not observers. This also happened with automated object distribution. (#3323)
 - Fixed issue where `AnticipatedNetworkVariable` previous value returned by `AnticipatedNetworkVariable.OnAuthoritativeValueChanged` is updated correctly on the non-authoritative side. (#3306)

com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs

@@ -1185,14 +1185,23 @@ internal void MarkVariablesDirty(bool dirty)
             }
         }
 
-        internal void MarkOwnerReadVariablesDirty()
+        /// <summary>
+        /// For owner read permissions, when changing ownership we need to do a full synchronization
+        /// of all NetworkVariables that are owner read permission based since the owner is the only
+        /// instance that knows what the most current values are.
+        /// </summary>
+        internal void MarkOwnerReadDirtyAndCheckOwnerWriteIsDirty()
         {
             for (int j = 0; j < NetworkVariableFields.Count; j++)
             {
                 if (NetworkVariableFields[j].ReadPerm == NetworkVariableReadPermission.Owner)
                 {
                     NetworkVariableFields[j].SetDirty(true);
                 }
+                if (NetworkVariableFields[j].WritePerm == NetworkVariableWritePermission.Owner)
+                {
+                    NetworkVariableFields[j].OnCheckIsDirtyState();
+                }
             }
         }
 

com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviourUpdater.cs

@@ -108,8 +108,6 @@ internal void NetworkBehaviourUpdate(bool forceSend = false)
                 foreach (var dirtyobj in m_DirtyNetworkObjects)
                 {
                     dirtyobj.PostNetworkVariableWrite(forceSend);
-                    // Once done processing, we set the previous owner id to the current owner id
-                    dirtyobj.PreviousOwnerId = dirtyobj.OwnerClientId;
                 }
                 m_DirtyNetworkObjects.Clear();
             }

com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs

@@ -2567,12 +2567,39 @@ internal void MarkVariablesDirty(bool dirty)
             }
         }
 
-        internal void MarkOwnerReadVariablesDirty()
+        /// <summary>
+        /// Used when changing ownership, this will mark any owner read permission base NetworkVariables as dirty
+        /// and will check if any owner write permission NetworkVariables are dirty (primarily for collections) so
+        /// the new owner will get a full state update prior to changing ownership.
+        /// </summary>
+        /// <remarks>
+        /// We have to pass in the original owner and previous owner to "reset" back to the current state of this
+        /// NetworkObject in order to preserve the same ownership change flow. By the time this is invoked, the
+        /// new and previous owner ids have already been set.
+        /// </remarks>
+        /// <param name="originalOwnerId">the owner prior to beginning the change in ownership change.</param>
+        /// <param name="originalPreviousOwnerId">the previous owner prior to beginning the change in ownership change.</param>
+        internal void SynchronizeOwnerNetworkVariables(ulong originalOwnerId, ulong originalPreviousOwnerId)
         {
+            var currentOwnerId = OwnerClientId;
+            OwnerClientId = originalOwnerId;
+            PreviousOwnerId = originalPreviousOwnerId;
             for (int i = 0; i < ChildNetworkBehaviours.Count; i++)
             {
-                ChildNetworkBehaviours[i].MarkOwnerReadVariablesDirty();
+                ChildNetworkBehaviours[i].MarkOwnerReadDirtyAndCheckOwnerWriteIsDirty();
             }
+
+            // Now set the new owner and previous owner identifiers back to their original new values
+            // before we run the NetworkBehaviourUpdate. For owner read only permissions this order of
+            // operations is **particularly important** as we need to first (above) mark things as dirty
+            // from the context of the original owner and then second (below) we need to send the messages
+            // which requires the new owner to be set for owner read permission NetworkVariables.
+            OwnerClientId = currentOwnerId;
+            PreviousOwnerId = originalOwnerId;
+
+            // Force send a state update for all owner read NetworkVariables  and any currently dirty
+            // owner write NetworkVariables.
+            NetworkManager.BehaviourUpdater.NetworkBehaviourUpdate(true);
         }
 
         // NGO currently guarantees that the client will receive spawn data for all objects in one network tick.

com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ChangeOwnershipMessage.cs

@@ -381,11 +381,8 @@ private void HandleOwnershipChange(ref NetworkContext context)
 
             if (originalOwner == networkManager.LocalClientId && !networkManager.DistributedAuthorityMode)
             {
-                // 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);
+                // Fully synchronize NetworkVariables with either read or write ownership permissions. 
+                networkObject.SynchronizeOwnerNetworkVariables(originalOwner, networkObject.PreviousOwnerId);
             }
 
             // Always invoke ownership change notifications

com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ClientConnectedMessage.cs

@@ -73,6 +73,8 @@ public void Handle(ref NetworkContext context)
                     /// DANGO-TODO: Determine if this needs to be removed once the service handles object distribution
                     networkManager.RedistributeToClients = true;
                     networkManager.ClientsToRedistribute.Add(ClientId);
+
+                    // TODO: We need a client synchronized message or something like that here
                 }
             }
         }

com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionApprovedMessage.cs

@@ -270,6 +270,13 @@ public void Handle(ref NetworkContext context)
             // Only if scene management is disabled do we handle NetworkObject synchronization at this point
             if (!networkManager.NetworkConfig.EnableSceneManagement)
             {
+                /// Mark the client being connected before running through the spawning synchronization so we
+                /// can assure that if a user attempts to spawn something when an already spawned NetworkObject
+                /// is spawned (during the initial synchronization just below) it will not error out complaining
+                /// about the player not being connected.
+                /// The check for this is done within <see cref="NetworkObject.SpawnInternal(bool, ulong, bool)"/>
+                networkManager.IsConnectedClient = true;
+
                 // DANGO-TODO: This is a temporary fix for no DA CMB scene event handling.
                 // We will either use this same concept or provide some way for the CMB state plugin to handle it.
                 if (networkManager.DistributedAuthorityMode && networkManager.LocalClient.IsSessionOwner)
@@ -292,9 +299,6 @@ public void Handle(ref NetworkContext context)
                     NetworkObject.AddSceneObject(sceneObject, m_ReceivedSceneObjectData, networkManager);
                 }
 
-                // Mark the client being connected
-                networkManager.IsConnectedClient = true;
-
                 if (networkManager.AutoSpawnPlayerPrefabClientSide)
                 {
                     networkManager.ConnectionManager.CreateAndSpawnPlayer(OwnerClientId);
@@ -315,14 +319,14 @@ public void Handle(ref NetworkContext context)
                 if (networkManager.DistributedAuthorityMode && networkManager.CMBServiceConnection && networkManager.LocalClient.IsSessionOwner && networkManager.NetworkConfig.EnableSceneManagement)
                 {
                     // Mark the client being connected
-                    networkManager.IsConnectedClient = true;
+                    networkManager.IsConnectedClient = networkManager.ConnectionManager.LocalClient.IsApproved;
 
                     networkManager.SceneManager.IsRestoringSession = GetIsSessionRestor();
 
                     if (!networkManager.SceneManager.IsRestoringSession)
                     {
                         // Synchronize the service with the initial session owner's loaded scenes and spawned objects
-                        networkManager.SceneManager.SynchronizeNetworkObjects(NetworkManager.ServerClientId);
+                        networkManager.SceneManager.SynchronizeNetworkObjects(NetworkManager.ServerClientId, true);
 
                         // Spawn any in-scene placed NetworkObjects
                         networkManager.SpawnManager.ServerSpawnSceneObjectsOnStartSweep();
@@ -334,9 +338,9 @@ public void Handle(ref NetworkContext context)
                         }
 
                         // Synchronize the service with the initial session owner's loaded scenes and spawned objects
-                        networkManager.SceneManager.SynchronizeNetworkObjects(NetworkManager.ServerClientId);
+                        networkManager.SceneManager.SynchronizeNetworkObjects(NetworkManager.ServerClientId, true);
 
-                        // With scene management enabled and since the session owner doesn't send a Synchronize scene event synchronize itself,
+                        // With scene management enabled and since the session owner doesn't send a scene event synchronize to itself,
                         // we need to notify the session owner that everything should be synchronized/spawned at this time.
                         networkManager.SpawnManager.NotifyNetworkObjectsSynchronized();
 

com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariable.cs

@@ -166,6 +166,13 @@ public bool CheckDirtyState(bool forceCheck = false)
             return isDirty;
         }
 
+        /// <inheritdoc/>
+        internal override void OnCheckIsDirtyState()
+        {
+            CheckDirtyState();
+            base.OnCheckIsDirtyState();
+        }
+
         internal ref T RefValue()
         {
             return ref m_InternalValue;

com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs

@@ -373,6 +373,15 @@ internal ulong OwnerClientId()
             return m_NetworkBehaviour.NetworkObject.OwnerClientId;
         }
 
+        /// <summary>
+        /// Primarily to check for collections dirty states when doing
+        /// a fully owner read/write NetworkVariable update.
+        /// </summary>
+        internal virtual void OnCheckIsDirtyState()
+        {
+
+        }
+
         /// <summary>
         /// Writes the dirty changes, that is, the changes since the variable was last dirty, to the writer
         /// </summary>

com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs

@@ -1986,6 +1986,14 @@ private void OnClientLoadedScene(uint sceneEventId, Scene scene)
         /// </summary>
         internal Func<Scene, bool> ExcludeSceneFromSychronization;
 
+        /// <summary>
+        /// This is used for distributed authority sessions only and assures that
+        /// when many clients attempt to connect at the same time they will be
+        /// handled sequentially so as to not saturate the session owner's maximum
+        /// reliable messages.
+        /// </summary>
+        internal List<ulong> ClientConnectionQueue = new List<ulong>();
+
         /// <summary>
         /// Server Side:
         /// This is used for players that have just had their connection approved and will assure they are synchronized
@@ -1994,8 +2002,30 @@ private void OnClientLoadedScene(uint sceneEventId, Scene scene)
         /// synchronized.
         /// </summary>
         /// <param name="clientId">newly joined client identifier</param>
-        internal void SynchronizeNetworkObjects(ulong clientId)
+        /// <param name="synchronizingService">true only when invoked on a newly connected and approved client.</param>
+        internal void SynchronizeNetworkObjects(ulong clientId, bool synchronizingService = false)
         {
+            // If we are connected to a live service hosted session and we are not doing the initial synchronization for the service...
+            if (NetworkManager.CMBServiceConnection && !synchronizingService)
+            {
+                // then as long as this is a newly connecting client add it to the connecting client queue.
+                // Otherwise, if this is not a newly connecting client (i.e. it is already in the queue), then go ahead and synchronize
+                // that client.
+                if (!ClientConnectionQueue.Contains(clientId))
+                {
+                    ClientConnectionQueue.Add(clientId);
+                    // If we are already synchronizing one or more clients, exit early. This client will be synchronized later.
+                    if (ClientConnectionQueue.Count > 1)
+                    {
+                        if (NetworkManager.LogLevel <= LogLevel.Developer)
+                        {
+                            Debug.Log($"Deferring Client-{clientId} synchrnization.");
+                        }
+                        return;
+                    }
+                }
+            }
+
             // Update the clients
             NetworkManager.SpawnManager.UpdateObservedNetworkObjects(clientId);
 
@@ -2623,6 +2653,44 @@ private void HandleSessionOwnerEvent(uint sceneEventId, ulong clientId)
                         // DANGO-EXP TODO: Remove this once service distributes objects
                         NetworkManager.SpawnManager.DistributeNetworkObjects(clientId);
                         EndSceneEvent(sceneEventId);
+
+                        // Exit early if not a distributed authority session or this is a DAHost
+                        // (DAHost has a unique connection per client, so no need to queue synchronization)
+                        if (!NetworkManager.DistributedAuthorityMode || NetworkManager.DAHost)
+                        {
+                            return;
+                        }
+
+                        // Otherwise, this is a session owner that could have pending clients to synchronize
+                        if (NetworkManager.DistributedAuthorityMode && NetworkManager.CMBServiceConnection)
+                        {
+                            // Remove the client that just synchronized
+                            ClientConnectionQueue.Remove(clientId);
+
+                            // If we have pending clients to synchronize, then make sure they are still connected
+                            while (ClientConnectionQueue.Count > 0)
+                            {
+                                // If the next client is no longer connected then remove it from the list
+                                if (!NetworkManager.ConnectedClientsIds.Contains(ClientConnectionQueue[0]))
+                                {
+                                    ClientConnectionQueue.RemoveAt(0);
+                                }
+                                else
+                                {
+                                    break;
+                                }
+                            }
+
+                            // If we still have any pending clients waiting, then synchronize the next one
+                            if (ClientConnectionQueue.Count > 0)
+                            {
+                                if (NetworkManager.LogLevel <= LogLevel.Developer)
+                                {
+                                    Debug.Log($"Synchronizing Client-{ClientConnectionQueue[0]}...");
+                                }
+                                SynchronizeNetworkObjects(ClientConnectionQueue[0]);
+                            }
+                        }
                         break;
                     }
                 default: