Merge branch 'develop-2.0.0' into ci/turn-off-single-commit-validation-2.x

Author: michalChrobot

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -12,6 +12,8 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed `OnClientConnectedCallback` passing incorrect `clientId` when scene management is disabled. (#3312)
+- Fixed issue where the `NetworkObject.Ownership` custom editor did not take the default "Everything" flag into consideration. (#3305)
 - Fixed DestroyObject flow on non-authority game clients. (#3291)
 - Fixed exception being thrown when a `GameObject` with an associated `NetworkTransform` is disabled. (#3243)
 - Fixed issue where the scene migration synchronization table was not cleaned up if the `GameObject` of a `NetworkObject` is destroyed before it should have been. (#3230)
@@ -24,6 +26,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Changed
 
+- Changed root in-scene placed `NetworkObject` instances now will always have either the `Distributable` permission set unless the `SessionOwner` permission is set. (#3305)
 - Changed the `DestroyObject` message to reduce the serialized message size and remove the unnecessary message field. (#3304)
 - Changed the `NetworkTimeSystem.Sync` method to use half RTT to calculate the desired local time offset as opposed to the full RTT. (#3212)
 

com.unity.netcode.gameobjects/Editor/NetworkObjectEditor.cs

@@ -14,6 +14,9 @@ namespace Unity.Netcode.Editor
     [CanEditMultipleObjects]
     public class NetworkObjectEditor : UnityEditor.Editor
     {
+        private const NetworkObject.OwnershipStatus k_AllOwnershipFlags = NetworkObject.OwnershipStatus.RequestRequired | NetworkObject.OwnershipStatus.Transferable | NetworkObject.OwnershipStatus.Distributable;
+        private const int k_SessionOwnerFlagAsInt = (int)NetworkObject.OwnershipStatus.SessionOwner;
+
         private bool m_Initialized;
         private NetworkObject m_NetworkObject;
         private bool m_ShowObservers;
@@ -114,10 +117,38 @@ public override void OnInspectorGUI()
             {
                 EditorGUI.BeginChangeCheck();
                 serializedObject.UpdateIfRequiredOrScript();
+
+                // Get the current ownership property and precalculate values in order to handle
+                // the exclusion or inclusion of "all" or just the session owner flags.
+                var ownershipProperty = serializedObject.FindProperty(nameof(NetworkObject.Ownership));
+                var previousOwnership = (NetworkObject.OwnershipStatus)ownershipProperty.intValue;
+                var hadAll = previousOwnership == k_AllOwnershipFlags;
+                var hadSessionOwner = ownershipProperty.intValue == k_SessionOwnerFlagAsInt;
+
                 DrawPropertiesExcluding(serializedObject, k_HiddenFields);
-                if (m_NetworkObject.IsOwnershipSessionOwner)
+
+                // If the ownership flags were changed
+                var currentOwnership = (NetworkObject.OwnershipStatus)ownershipProperty.intValue;
+                if (currentOwnership != previousOwnership)
                 {
-                    m_NetworkObject.Ownership = NetworkObject.OwnershipStatus.SessionOwner;
+                    // Determine if we need to handle setting or removing the session owner flag specifically
+                    // when a user selects the "All" enum flag value.
+                    var hasSessionOwner = currentOwnership.HasFlag(NetworkObject.OwnershipStatus.SessionOwner);
+                    if (hasSessionOwner)
+                    {
+                        if (ownershipProperty.intValue == -1 && !hadAll)
+                        {
+                            ownershipProperty.intValue = (int)k_AllOwnershipFlags;
+                        }
+                        else if ((hadAll && !hadSessionOwner) || (!hadAll && !hadSessionOwner))
+                        {
+                            ownershipProperty.intValue = k_SessionOwnerFlagAsInt;
+                        }
+                        else if (hadSessionOwner && hasSessionOwner)
+                        {
+                            ownershipProperty.intValue &= ~k_SessionOwnerFlagAsInt;
+                        }
+                    }
                 }
                 serializedObject.ApplyModifiedProperties();
                 EditorGUI.EndChangeCheck();

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

@@ -322,6 +322,15 @@ private void CheckForInScenePlaced()
 
                 // Default scene migration synchronization to false for in-scene placed NetworkObjects
                 SceneMigrationSynchronization = false;
+
+                // Root In-scene placed NetworkObjects have to either have the SessionOwner or Distributable permission flag set.
+                if (transform.parent == null)
+                {
+                    if (!Ownership.HasFlag(OwnershipStatus.SessionOwner) && !Ownership.HasFlag(OwnershipStatus.Distributable))
+                    {
+                        Ownership |= OwnershipStatus.Distributable;
+                    }
+                }
             }
         }
 #endif // UNITY_EDITOR
@@ -493,16 +502,36 @@ public void DeferDespawn(int tickOffset, bool destroy = true)
         /// <see cref="Transferable"/>: When set, a non-owner can obtain ownership immediately (without requesting and as long as it is not locked).
         /// <see cref="RequestRequired"/>: When set, a non-owner must request ownership from the owner (will always get locked once ownership is transferred).
         /// <see cref="SessionOwner"/>: When set, only the current session owner may have ownership over this object.
+        /// <see cref="All"/>: Used within the inspector view only. When selected it will set the Distributable, Transferable, and RequestRequired flags or if those flags are already set it will select the SessionOwner flag by itself.
         /// </summary>
         // Ranges from 1 to 8 bits
         [Flags]
         public enum OwnershipStatus
         {
+            /// <summary>
+            ///  When set, this instance will have no permissions (i.e. cannot distribute, transfer, etc).
+            /// </summary>
             None = 0,
+            /// <summary>
+            ///  When set, this instance will be automatically redistributed when a client joins (if not locked or no request is pending) or leaves.
+            /// </summary>
             Distributable = 1 << 0,
+            /// <summary>
+            /// When set, a non-owner can obtain ownership immediately (without requesting and as long as it is not locked).
+            /// </summary>
             Transferable = 1 << 1,
+            /// <summary>
+            /// When set, a non-owner must request ownership from the owner (will always get locked once ownership is transferred).
+            /// </summary>
             RequestRequired = 1 << 2,
+            /// <summary>
+            /// When set, only the current session owner may have ownership over this object.
+            /// </summary>
             SessionOwner = 1 << 3,
+            /// <summary>
+            /// Used within the inspector view only. When selected it will set the Distributable, Transferable, and RequestRequired flags or if those flags are already set it will select the SessionOwner flag by itself.
+            /// </summary>
+            All = ~0,
         }
 
         /// <summary>

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

@@ -305,7 +305,7 @@ public void Handle(ref NetworkContext context)
                     NetworkLog.LogInfo($"[Client-{OwnerClientId}][Scene Management Disabled] Synchronization complete!");
                 }
                 // When scene management is disabled we notify after everything is synchronized
-                networkManager.ConnectionManager.InvokeOnClientConnectedCallback(context.SenderId);
+                networkManager.ConnectionManager.InvokeOnClientConnectedCallback(OwnerClientId);
 
                 // For convenience, notify all NetworkBehaviours that synchronization is complete.
                 networkManager.SpawnManager.NotifyNetworkObjectsSynchronized();

com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs

@@ -1967,9 +1967,12 @@ internal void DistributeNetworkObjects(ulong clientId)
                                 {
                                     continue;
                                 }
-                                if ((!child.IsOwnershipDistributable || !child.IsOwnershipTransferable) && NetworkManager.LogLevel == LogLevel.Developer)
+                                if (!child.IsOwnershipDistributable || !child.IsOwnershipTransferable)
                                 {
-                                    NetworkLog.LogWarning($"Sibling {child.name} of root parent {ownerList.Value[i].name} is neither transferrable or distributable! Object distribution skipped and could lead to a potentially un-owned or owner-mismatched {nameof(NetworkObject)}!");
+                                    if (NetworkManager.LogLevel == LogLevel.Developer)
+                                    {
+                                        NetworkLog.LogWarning($"Sibling {child.name} of root parent {ownerList.Value[i].name} is neither transferrable or distributable! Object distribution skipped and could lead to a potentially un-owned or owner-mismatched {nameof(NetworkObject)}!");
+                                    }
                                     continue;
                                 }
                                 // Transfer ownership of all distributable =or= transferrable children with the same owner to the same client to preserve the sibling ownership tree.

com.unity.netcode.gameobjects/Tests/Runtime/Connection.meta

@@ -0,0 +1,82 @@
+using System.Collections;
+using System.Collections.Generic;
+using NUnit.Framework;
+using Unity.Netcode.TestHelpers.Runtime;
+using UnityEngine.TestTools;
+
+namespace Unity.Netcode.RuntimeTests
+{
+    [TestFixture(SceneManagementState.SceneManagementEnabled, NetworkTopologyTypes.DistributedAuthority)]
+    [TestFixture(SceneManagementState.SceneManagementDisabled, NetworkTopologyTypes.DistributedAuthority)]
+    [TestFixture(SceneManagementState.SceneManagementEnabled, NetworkTopologyTypes.ClientServer)]
+    [TestFixture(SceneManagementState.SceneManagementDisabled, NetworkTopologyTypes.ClientServer)]
+    internal class ClientConnectionTests : IntegrationTestWithApproximation
+    {
+        protected override int NumberOfClients => 3;
+        private readonly bool m_SceneManagementEnabled;
+        private HashSet<ulong> m_ServerCallbackCalled = new HashSet<ulong>();
+        private HashSet<ulong> m_ClientCallbackCalled = new HashSet<ulong>();
+
+        public ClientConnectionTests(SceneManagementState sceneManagementState, NetworkTopologyTypes networkTopologyType) : base(networkTopologyType)
+        {
+            m_SceneManagementEnabled = sceneManagementState == SceneManagementState.SceneManagementEnabled;
+        }
+
+        protected override void OnServerAndClientsCreated()
+        {
+            m_ServerNetworkManager.NetworkConfig.EnableSceneManagement = m_SceneManagementEnabled;
+            m_ServerNetworkManager.OnClientConnectedCallback += Server_OnClientConnectedCallback;
+
+            foreach (var client in m_ClientNetworkManagers)
+            {
+                client.NetworkConfig.EnableSceneManagement = m_SceneManagementEnabled;
+                client.OnClientConnectedCallback += Client_OnClientConnectedCallback;
+            }
+
+            base.OnServerAndClientsCreated();
+        }
+
+        [UnityTest]
+        public IEnumerator VerifyOnClientConnectedCallback()
+        {
+            yield return WaitForConditionOrTimeOut(AllCallbacksCalled);
+            AssertOnTimeout("Timed out waiting for all clients to be connected!");
+
+            // The client callbacks should have been called once per client (called once on self)
+            Assert.True(m_ClientCallbackCalled.Count == NumberOfClients);
+
+            // The server callback should be called for self, and then once per client
+            Assert.True(m_ServerCallbackCalled.Count == 1 + NumberOfClients);
+        }
+
+        private void Server_OnClientConnectedCallback(ulong clientId)
+        {
+            if (!m_ServerCallbackCalled.Add(clientId))
+            {
+                Assert.Fail($"Client already connected: {clientId}");
+            }
+        }
+
+        private void Client_OnClientConnectedCallback(ulong clientId)
+        {
+            if (!m_ClientCallbackCalled.Add(clientId))
+            {
+                Assert.Fail($"Client already connected: {clientId}");
+            }
+        }
+
+        private bool AllCallbacksCalled()
+        {
+            foreach (var client in m_ClientNetworkManagers)
+            {
+                if (!m_ClientCallbackCalled.Contains(client.LocalClientId) || !m_ServerCallbackCalled.Contains(client.LocalClientId))
+                {
+                    return false;
+                }
+            }
+
+            return m_ServerCallbackCalled.Contains(m_ServerNetworkManager.LocalClientId);
+        }
+    }
+}
+