fix: Update NetworkBehaviour on session owner promotion (#3817)

* fix: Update NetworkBehaviour on Session Owner promotion

* Update CHANGELOG

* Add a test for resetting extended ownership flags

* Update CHANGELOG

Author: EmandM

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -49,6 +49,8 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Ensure `NetworkBehaviour.IsSessionOwner` is correctly set when a new session owner is promoted. (#3817)
+- Reset extended ownership flags on `NetworkObject` despawn. (#3817)
 - Fixed issues with the "Client-server quickstart for Netcode for GameObjects" script having static methods and properties. (#3787)
 - Fixed issue where a warning message was being logged upon a client disconnecting from a server when the log level is set to developer. (#3786)
 - Fixed issue where the server or host would no longer have access to the transport id to client id table when processing a transport level client disconnect event. (#3786)

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

@@ -515,7 +515,7 @@ public NetworkManager NetworkManager
         /// <summary>
         /// Gets whether the client is the distributed authority mode session owner.
         /// </summary>
-        public bool IsSessionOwner { get; private set; }
+        public bool IsSessionOwner { get; internal set; }
 
         /// <summary>
         /// Gets whether the server (local or remote) is a host.

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

@@ -216,19 +216,21 @@ internal void HandleRedistributionToClients()
 
         internal void SetSessionOwner(ulong sessionOwner)
         {
-            var previousSessionOwner = CurrentSessionOwner;
             CurrentSessionOwner = sessionOwner;
-            LocalClient.IsSessionOwner = LocalClientId == sessionOwner;
-            if (LocalClient.IsSessionOwner)
+            var isSessionOwner = LocalClientId == sessionOwner;
+            LocalClient.IsSessionOwner = isSessionOwner;
+
+            foreach (var networkObject in SpawnManager.SpawnedObjects.Values)
             {
-                foreach (var networkObjectEntry in SpawnManager.SpawnedObjects)
+                if (isSessionOwner)
                 {
-                    var networkObject = networkObjectEntry.Value;
                     if (networkObject.IsOwnershipSessionOwner && networkObject.OwnerClientId != LocalClientId)
                     {
                         SpawnManager.ChangeOwnership(networkObject, LocalClientId, true);
                     }
                 }
+
+                networkObject.InvokeSessionOwnerPromoted(isSessionOwner);
             }
 
             OnSessionOwnerPromoted?.Invoke(sessionOwner);

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

@@ -2002,6 +2002,7 @@ internal void ResetOnDespawn()
             IsSpawned = false;
             DeferredDespawnTick = 0;
             m_LatestParent = null;
+            RemoveOwnershipExtended(OwnershipStatusExtended.Locked | OwnershipStatusExtended.Requested);
         }
 
         /// <summary>
@@ -2078,6 +2079,19 @@ internal void InvokeOwnershipChanged(ulong previous, ulong next)
             }
         }
 
+        internal void InvokeSessionOwnerPromoted(bool isSessionOwner)
+        {
+            if (!IsSpawned)
+            {
+                return;
+            }
+
+            foreach (var childBehaviour in ChildNetworkBehaviours)
+            {
+                childBehaviour.IsSessionOwner = isSessionOwner;
+            }
+        }
+
         internal void InvokeBehaviourOnNetworkObjectParentChanged(NetworkObject parentNetworkObject)
         {
             for (int i = 0; i < ChildNetworkBehaviours.Count; i++)

com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/ExtendedNetworkShowAndHideTests.cs

@@ -1,33 +1,33 @@
 using System.Collections;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
 using NUnit.Framework;
 using Unity.Netcode.TestHelpers.Runtime;
 using UnityEngine;
 using UnityEngine.TestTools;
 
 namespace Unity.Netcode.RuntimeTests
 {
-    [TestFixture(HostOrServer.DAHost, true)]
-    [TestFixture(HostOrServer.DAHost, false)]
+    internal enum SceneManagementSetting
+    {
+        EnableSceneManagement,
+        DisableSceneManagement
+    }
+
+    [TestFixture(SceneManagementSetting.EnableSceneManagement)]
+    [TestFixture(SceneManagementSetting.DisableSceneManagement)]
     internal class ExtendedNetworkShowAndHideTests : NetcodeIntegrationTest
     {
         protected override int NumberOfClients => 3;
-        private bool m_EnableSceneManagement;
+        private readonly bool m_EnableSceneManagement;
         private GameObject m_ObjectToSpawn;
-        private NetworkObject m_SpawnedObject;
-        private NetworkManager m_ClientToHideFrom;
-        private NetworkManager m_LateJoinClient;
-        private NetworkManager m_SpawnOwner;
-
-        // TODO: [CmbServiceTests] Adapt to run with the service (can't control who becomes the session owner, needs a logic rework)
-        /// <inheritdoc/>
-        protected override bool UseCMBService()
-        {
-            return false;
-        }
+        private List<NetworkObject> m_SpawnedObjects = new();
+        private Dictionary<NetworkManager, NetworkObject> m_ObjectHiddenFromClient = new();
 
-        public ExtendedNetworkShowAndHideTests(HostOrServer hostOrServer, bool enableSceneManagement) : base(hostOrServer)
+        public ExtendedNetworkShowAndHideTests(SceneManagementSetting sceneManagement) : base(HostOrServer.DAHost)
         {
-            m_EnableSceneManagement = enableSceneManagement;
+            m_EnableSceneManagement = sceneManagement == SceneManagementSetting.EnableSceneManagement;
         }
 
         protected override void OnServerAndClientsCreated()
@@ -38,48 +38,93 @@ protected override void OnServerAndClientsCreated()
             }
 
             m_ObjectToSpawn = CreateNetworkObjectPrefab("TestObject");
+            m_ObjectToSpawn.AddComponent<EmptyBehaviour>();
             m_ObjectToSpawn.SetActive(false);
 
             base.OnServerAndClientsCreated();
         }
 
-        private bool AllClientsSpawnedObject()
+        private bool AllObjectsHiddenFromTheirClient(StringBuilder errorLog)
         {
-            foreach (var client in m_NetworkManagers)
+            var allHidden = true;
+            foreach (var (clientToHideFrom, objectToHide) in m_ObjectHiddenFromClient)
             {
-                if (!s_GlobalNetworkObjects.ContainsKey(client.LocalClientId))
+                if (clientToHideFrom.SpawnManager.SpawnedObjects.ContainsKey(objectToHide.NetworkObjectId))
                 {
-                    return false;
-                }
-                if (!s_GlobalNetworkObjects[client.LocalClientId].ContainsKey(m_SpawnedObject.NetworkObjectId))
-                {
-                    return false;
+                    errorLog.AppendLine($"Object-{objectToHide.NetworkObjectId} is still visible to Client-{clientToHideFrom.LocalClientId}");
+                    allHidden = false;
                 }
             }
-            return true;
+
+            return allHidden;
         }
 
-        private bool IsClientPromotedToSessionOwner()
+        private bool IsClientPromotedToSessionOwner(StringBuilder errorLog)
         {
+            var valid = true;
+            var currentSessionOwner = GetAuthorityNetworkManager();
             foreach (var client in m_NetworkManagers)
             {
-                if (!client.IsConnectedClient)
+                if (currentSessionOwner == client)
                 {
-                    continue;
+                    if (!client.LocalClient.IsSessionOwner)
+                    {
+                        errorLog.AppendLine($"Session owner is Client-{client.LocalClientId} but they are not marked as session owner.");
+                        valid = false;
+                    }
                 }
-                if (client.CurrentSessionOwner != m_ClientToHideFrom.LocalClientId)
+                else if (client.LocalClient.IsSessionOwner)
                 {
-                    return false;
+                    errorLog.AppendLine($"Client-{client.LocalClientId} is incorrectly marked as Session Owner.");
+                    valid = false;
+                }
+
+                if (client.CurrentSessionOwner != currentSessionOwner.LocalClientId)
+                {
+                    errorLog.AppendLine($"Client-{client.LocalClientId} has the incorrect session owner id (Has: {client.CurrentSessionOwner}, expected: {currentSessionOwner.LocalClientId}).");
+                    valid = false;
                 }
             }
-            return true;
+            return valid;
         }
 
-        protected override void OnNewClientCreated(NetworkManager networkManager)
+        private bool AllObjectsSpawnedExceptForHidden(StringBuilder errorLog)
         {
-            m_LateJoinClient = networkManager;
-            networkManager.NetworkConfig.EnableSceneManagement = m_EnableSceneManagement;
-            base.OnNewClientCreated(networkManager);
+            var valid = true;
+            foreach (var client in m_NetworkManagers)
+            {
+                foreach (var spawnedObject in m_SpawnedObjects)
+                {
+                    var isVisible = client.SpawnManager.SpawnedObjects.ContainsKey(spawnedObject.NetworkObjectId);
+
+                    if (m_ObjectHiddenFromClient.TryGetValue(client, out var hiddenObject) && hiddenObject == spawnedObject)
+                    {
+                        if (isVisible)
+                        {
+                            errorLog.AppendLine($"Client-{client.LocalClientId} can see object-{spawnedObject.NetworkObjectId} that should be hidden.");
+                            valid = false;
+                            continue;
+                        }
+                    }
+                    else if (!isVisible)
+                    {
+                        errorLog.AppendLine($"Client-{client.LocalClientId} hasn't spawned object-{spawnedObject.NetworkObjectId}");
+                        valid = false;
+                    }
+
+                    if (isVisible)
+                    {
+                        var clientObject = client.SpawnManager.SpawnedObjects[spawnedObject.NetworkObjectId];
+                        var behaviour = clientObject.GetComponent<EmptyBehaviour>();
+                        if (behaviour.IsSessionOwner != client.LocalClient.IsSessionOwner)
+                        {
+                            errorLog.AppendLine($"Client-{client.LocalClientId} network behaviour has incorrect session owner value. Should be {client.LocalClient.IsSessionOwner}, is {behaviour.IsSessionOwner}");
+                            valid = false;
+                        }
+                    }
+                }
+            }
+            return valid;
         }
 
         /// <summary>
@@ -95,41 +140,89 @@ public IEnumerator HiddenObjectPromotedSessionOwnerNewClientSynchronizes()
         {
             // Get the test relative session owner
             var sessionOwner = GetAuthorityNetworkManager();
-            m_SpawnOwner = m_UseCmbService ? m_ClientNetworkManagers[1] : m_ClientNetworkManagers[0];
-            m_ClientToHideFrom = m_UseCmbService ? m_ClientNetworkManagers[NumberOfClients - 1] : m_ClientNetworkManagers[1];
+
+            // Spawn objects for the non-session owner clients
             m_ObjectToSpawn.SetActive(true);
+            foreach (var client in m_NetworkManagers)
+            {
+                if (client == sessionOwner)
+                {
+                    Assert.IsTrue(client.LocalClient.IsSessionOwner);
+                    continue;
+                }
+
+                var instance = SpawnObject(m_ObjectToSpawn, client).GetComponent<NetworkObject>();
+                m_SpawnedObjects.Add(instance);
+            }
+
+            yield return WaitForSpawnedOnAllOrTimeOut(m_SpawnedObjects);
+            AssertOnTimeout("[InitialSpawn] Not all clients spawned all objects");
+
+            // Hide one spawned object from each client
+            var setOfInstances = m_SpawnedObjects.ToHashSet();
+            foreach (var clientToHideFrom in m_NetworkManagers)
+            {
+                // Session owner doesn't need to have an object hidden from them
+                if (clientToHideFrom == sessionOwner)
+                {
+                    continue;
+                }
+
+                // Find an object that this client doesn't own that isn't already hidden from another client
+                var toHide = setOfInstances.Last(obj => obj.OwnerClientId != clientToHideFrom.LocalClientId);
+                toHide.NetworkHide(clientToHideFrom.LocalClientId);
+                setOfInstances.Remove(toHide);
+                m_ObjectHiddenFromClient.Add(clientToHideFrom, toHide);
 
-            // Spawn the object with a non-session owner client
-            m_SpawnedObject = SpawnObject(m_ObjectToSpawn, m_SpawnOwner).GetComponent<NetworkObject>();
-            yield return WaitForConditionOrTimeOut(AllClientsSpawnedObject);
-            AssertOnTimeout($"Not all clients spawned and instance of {m_SpawnedObject.name}");
+                Assert.IsFalse(toHide.GetComponent<EmptyBehaviour>().IsSessionOwner, "No object should have been spawned owned by the session owner");
+            }
 
-            // Hide the spawned object from the to be promoted session owner
-            m_SpawnedObject.NetworkHide(m_ClientToHideFrom.LocalClientId);
+            Assert.That(m_ObjectHiddenFromClient.Count, Is.EqualTo(NumberOfClients), "Test should hide one object per non-authority client");
+            Assert.That(setOfInstances, Is.Empty, "Not all objects have been hidden frm someone.");
 
-            yield return WaitForConditionOrTimeOut(() => !m_ClientToHideFrom.SpawnManager.SpawnedObjects.ContainsKey(m_SpawnedObject.NetworkObjectId));
-            AssertOnTimeout($"{m_SpawnedObject.name} was not hidden from Client-{m_ClientToHideFrom.LocalClientId}!");
+            yield return WaitForConditionOrTimeOut(AllObjectsHiddenFromTheirClient);
+            AssertOnTimeout("Not all objects have been hidden from someone!");
 
             // Promoted a new session owner (DAHost promotes while CMB Session we disconnect the current session owner)
             if (!m_UseCmbService)
             {
-                m_ServerNetworkManager.PromoteSessionOwner(m_ClientToHideFrom.LocalClientId);
+                var nonAuthority = GetNonAuthorityNetworkManager();
+                m_ServerNetworkManager.PromoteSessionOwner(nonAuthority.LocalClientId);
+                yield return s_DefaultWaitForTick;
             }
             else
             {
-                sessionOwner.Shutdown();
+                yield return StopOneClient(sessionOwner);
             }
 
             // Wait for the new session owner to be promoted and for all clients to acknowledge the promotion
             yield return WaitForConditionOrTimeOut(IsClientPromotedToSessionOwner);
-            AssertOnTimeout($"Client-{m_ClientToHideFrom.LocalClientId} was not promoted as session owner on all client instances!");
+            AssertOnTimeout($"No client was promoted as session owner on all client instances!");
+
+            var newSessionOwner = GetAuthorityNetworkManager();
+            VerboseDebug($"Client-{newSessionOwner.LocalClientId} was promoted as session owner on all client instances!");
+            Assert.That(newSessionOwner, Is.Not.EqualTo(sessionOwner), "The current session owner be different from the original session owner.");
+            Assert.That(m_ObjectHiddenFromClient, Does.ContainKey(newSessionOwner), "An object should be hidden from the newly promoted session owner");
 
             // Connect a new client instance
-            yield return CreateAndStartNewClient();
+            var newClient = CreateNewClient();
+            newClient.NetworkConfig.EnableSceneManagement = m_EnableSceneManagement;
+            yield return StartClient(newClient);
 
             // Assure the newly connected client is synchronized with the NetworkObject hidden from the newly promoted session owner
-            yield return WaitForConditionOrTimeOut(() => m_LateJoinClient.SpawnManager.SpawnedObjects.ContainsKey(m_SpawnedObject.NetworkObjectId));
-            AssertOnTimeout($"Client-{m_LateJoinClient.LocalClientId} never spawned {nameof(NetworkObject)} {m_SpawnedObject.name}!");
+            yield return WaitForConditionOrTimeOut(AllObjectsSpawnedExceptForHidden);
+            AssertOnTimeout("[LateJoinClient] Not all objects spawned correctly");
+        }
+
+        protected override IEnumerator OnTearDown()
+        {
+            m_SpawnedObjects.Clear();
+            m_ObjectHiddenFromClient.Clear();
+            return base.OnTearDown();
+        }
+
+        private class EmptyBehaviour : NetworkBehaviour
+        {
         }
     }
 }