fix: adding method to remove character without destroying the object

fixes: #883

BREAKING CHANGE: RemovePlayerForConnection removed, use RemoveCharacter or DestroyCharacter instead. Note for RemoveCharacter destroyServerObject now defaults to true

Assets/Mirage/Runtime/ClientObjectManager.cs

@@ -127,6 +127,7 @@ internal void RegisterHostHandlers()
             Client.MessageHandler.RegisterHandler<ObjectHideMessage>(msg => { });
             Client.MessageHandler.RegisterHandler<SpawnMessage>(OnHostClientSpawn);
             Client.MessageHandler.RegisterHandler<RemoveAuthorityMessage>(msg => { });
+            Client.MessageHandler.RegisterHandler<RemoveCharacterMessage>(OnRemoveCharacter);
             Client.MessageHandler.RegisterHandler<ServerRpcReply>(msg => { });
             Client.MessageHandler.RegisterHandler<RpcMessage>(msg => { });
         }
@@ -137,6 +138,7 @@ internal void RegisterMessageHandlers()
             Client.MessageHandler.RegisterHandler<ObjectHideMessage>(OnObjectHide);
             Client.MessageHandler.RegisterHandler<SpawnMessage>(OnSpawn);
             Client.MessageHandler.RegisterHandler<RemoveAuthorityMessage>(OnRemoveAuthority);
+            Client.MessageHandler.RegisterHandler<RemoveCharacterMessage>(OnRemoveCharacter);
             Client.MessageHandler.RegisterHandler<ServerRpcReply>(OnServerRpcReply);
             Client.MessageHandler.RegisterHandler<RpcMessage>(OnRpcMessage);
         }
@@ -493,7 +495,25 @@ internal void OnRemoveAuthority(RemoveAuthorityMessage msg)
 
             identity.HasAuthority = false;
 
-            // objects spawned as part of initial state are started on a second pass
+            identity.NotifyAuthority();
+        }
+
+        internal void OnRemoveCharacter(RemoveCharacterMessage msg)
+        {
+            if (logger.LogEnabled()) logger.Log($"Client remove character handler");
+
+            INetworkPlayer player = Client.Player;
+            NetworkIdentity identity = player.Identity;
+
+            if (identity == null)
+            {
+                logger.LogWarning($"Could not find player's character");
+                return;
+            }
+
+            player.Identity = null;
+            identity.HasAuthority = msg.keepAuthority;
+
             identity.NotifyAuthority();
         }
 

Assets/Mirage/Runtime/IServerObjectManager.cs

@@ -12,6 +12,9 @@ public interface IServerObjectManager
         void ReplaceCharacter(INetworkPlayer player, GameObject character, int prefabHash, bool keepAuthority = false);
         void ReplaceCharacter(INetworkPlayer player, NetworkIdentity identity, bool keepAuthority = false);
 
+        void RemoveCharacter(INetworkPlayer player, bool keepAuthority = false);
+        void DestroyCharacter(INetworkPlayer player, bool destroyServerObject = true);
+
         void Spawn(GameObject obj, INetworkPlayer owner = null);
         void Spawn(GameObject obj, GameObject ownerObject);
         void Spawn(GameObject obj, int prefabHash, INetworkPlayer owner = null);

Assets/Mirage/Runtime/Messages.cs

@@ -120,6 +120,12 @@ public struct RemoveAuthorityMessage
         public uint netId;
     }
 
+    [NetworkMessage]
+    public struct RemoveCharacterMessage
+    {
+        public bool keepAuthority;
+    }
+
     [NetworkMessage]
     public struct ObjectDestroyMessage
     {

Assets/Mirage/Runtime/ServerObjectManager.cs

@@ -344,22 +344,50 @@ internal void HideToPlayer(NetworkIdentity identity, INetworkPlayer player)
         }
 
         /// <summary>
-        /// Removes the player object from the connection
+        /// Removes the character from a player, with the option to keep the player as the owner of the object
         /// </summary>
-        /// <param name="player">The connection of the client to remove from</param>
-        /// <param name="destroyServerObject">Indicates whether the server object should be destroyed</param>
-        /// <exception cref="InvalidOperationException">Received remove player message but connection has no player</exception>
-        public void RemovePlayerForConnection(INetworkPlayer player, bool destroyServerObject = false)
+        /// <param name="player"></param>
+        /// <param name="keepAuthority"></param>
+        /// <exception cref="InvalidOperationException">Throws when player does not have a character</exception>
+        public void RemoveCharacter(INetworkPlayer player, bool keepAuthority = false)
         {
-            if (!player.HasCharacter)
+            ThrowIfNoCharacter(player);
+
+            NetworkIdentity identity = player.Identity;
+            player.Identity = null;
+            if (!keepAuthority)
             {
-                throw new InvalidOperationException("Received remove player message but connection has no player");
+                logger.Assert(identity.Owner == player, "Owner should be player that is being removed");
+                identity.Owner = null;
             }
 
+            player.Send(new RemoveCharacterMessage { keepAuthority = keepAuthority });
+        }
+
+        /// <summary>
+        /// Removes and destroys the character from a player
+        /// </summary>
+        /// <param name="player"></param>
+        /// <param name="destroyServerObject"></param>
+        /// <exception cref="InvalidOperationException">Throws when player does not have a character</exception>
+        public void DestroyCharacter(INetworkPlayer player, bool destroyServerObject = true)
+        {
+            ThrowIfNoCharacter(player);
+
             Destroy(player.Identity.gameObject, destroyServerObject);
             player.Identity = null;
         }
 
+        /// <exception cref="InvalidOperationException">Throws when player does not have a character</exception>
+        private static void ThrowIfNoCharacter(INetworkPlayer player)
+        {
+            if (!player.HasCharacter)
+            {
+                throw new InvalidOperationException("Player did not have a character");
+            }
+        }
+
+
         /// <summary>
         /// Handle ServerRpc from specific player, this could be one of multiple players on a single client
         /// </summary>

Assets/Tests/Runtime/ClientServer/ServerObjectManagerTests.cs

@@ -212,38 +212,83 @@ public void AddPlayerForConnectionAssetIdTest()
         }
 
         [UnityTest]
-        public IEnumerator RemovePlayerForConnectionTest() => UniTask.ToCoroutine(async () =>
+        public IEnumerator DestroyCharacter() => UniTask.ToCoroutine(async () =>
         {
-            serverObjectManager.RemovePlayerForConnection(serverPlayer);
+            serverObjectManager.DestroyCharacter(serverPlayer);
 
-            await AsyncUtil.WaitUntilWithTimeout(() => !clientIdentity);
+            await UniTask.Yield();
+            await UniTask.Yield();
 
-            Assert.That(serverPlayerGO);
+            Assert.That(serverPlayerGO == null);
+            Assert.That(clientPlayerGO == null);
         });
 
         [UnityTest]
-        public IEnumerator RemovePlayerForConnectionExceptionTest() => UniTask.ToCoroutine(async () =>
+        public IEnumerator DestroyCharacterKeepServerObject() => UniTask.ToCoroutine(async () =>
         {
-            serverObjectManager.RemovePlayerForConnection(serverPlayer);
+            serverObjectManager.DestroyCharacter(serverPlayer, destroyServerObject: false);
 
-            await AsyncUtil.WaitUntilWithTimeout(() => !clientIdentity);
+            await UniTask.Yield();
+            await UniTask.Yield();
 
-            Assert.Throws<InvalidOperationException>(() =>
-            {
-                serverObjectManager.RemovePlayerForConnection(serverPlayer);
-            });
+            Assert.That(serverPlayerGO != null);
+            Assert.That(clientPlayerGO == null);
+        });
+
+        [UnityTest]
+        public IEnumerator RemoveCharacterKeepAuthority() => UniTask.ToCoroutine(async () =>
+        {
+            serverObjectManager.RemoveCharacter(serverPlayer, true);
+
+            await UniTask.Yield();
+            await UniTask.Yield();
+
+            Assert.That(serverPlayerGO != null);
+            Assert.That(serverIdentity.Owner == serverPlayer);
+
+            Assert.That(clientPlayerGO != null);
+            Assert.That(clientIdentity.HasAuthority, Is.True);
+            Assert.That(clientIdentity.IsLocalPlayer, Is.False);
         });
 
         [UnityTest]
-        public IEnumerator RemovePlayerForConnectionDestroyTest() => UniTask.ToCoroutine(async () =>
+        public IEnumerator RemoveCharacter() => UniTask.ToCoroutine(async () =>
         {
-            serverObjectManager.RemovePlayerForConnection(serverPlayer, true);
+            serverObjectManager.RemoveCharacter(serverPlayer);
+
+            await UniTask.Yield();
+            await UniTask.Yield();
 
-            await AsyncUtil.WaitUntilWithTimeout(() => !clientIdentity);
+            Assert.That(serverPlayerGO != null);
+            Assert.That(serverIdentity.Owner == null);
 
-            Assert.That(!serverPlayerGO);
+            Assert.That(clientPlayerGO != null);
+            Assert.That(clientIdentity.HasAuthority, Is.False);
+            Assert.That(clientIdentity.IsLocalPlayer, Is.False);
         });
 
+        [Test]
+        public void DestroyCharacterThrowsIfNoCharacter()
+        {
+            INetworkPlayer player = Substitute.For<INetworkPlayer>();
+
+            Assert.Throws<InvalidOperationException>(() =>
+            {
+                serverObjectManager.DestroyCharacter(player);
+            });
+        }
+
+        [Test]
+        public void RemoveCharacterThrowsIfNoCharacter()
+        {
+            INetworkPlayer player = Substitute.For<INetworkPlayer>();
+
+            Assert.Throws<InvalidOperationException>(() =>
+            {
+                serverObjectManager.RemoveCharacter(player, false);
+            });
+        }
+
         [Test]
         public void ThrowsIfSpawnedCalledWithoutANetworkIdentity()
         {