fix: ClientScene.localplayer is now set to null when it is destroyed (#2227)

* moving FakeNetworkConnection to common test folder

* adding test for local player destroy

* adding method to clear local player

* temp move test functions

* adding test for destroying non player

* moving asset to setup

* adding tests for destroy message and host mode

* adding ClearLocalPlayer to reset

Assets/Mirror/Runtime/ClientScene.cs

@@ -84,7 +84,10 @@ internal static void Shutdown()
             DestroyAllClientObjects();
         }
 
-        // this is called from message handler for Owner message
+        /// <summary>
+        /// this is called from message handler for Owner message
+        /// </summary>
+        /// <param name="identity"></param>
         internal static void InternalAddPlayer(NetworkIdentity identity)
         {
             logger.Log("ClientScene.InternalAddPlayer");
@@ -107,6 +110,17 @@ internal static void InternalAddPlayer(NetworkIdentity identity)
             }
         }
 
+        /// <summary>
+        /// Sets localPlayer to null
+        /// <para>Should be called when the local player object is destroyed</para>
+        /// </summary>
+        internal static void ClearLocalPlayer()
+        {
+            logger.Log("ClientScene.ClearLocalPlayer");
+
+            localPlayer = null;
+        }
+
         /// <summary>
         /// This adds a player GameObject for this client. This causes an AddPlayer message to be sent to the server, and NetworkManager.OnServerAddPlayer is called. If an extra message was passed to AddPlayer, then OnServerAddPlayer will be called with a NetworkReader that contains the contents of the message.
         /// <para>extraMessage can contain character selection, etc.</para>

Assets/Mirror/Runtime/NetworkIdentity.cs

@@ -721,6 +721,11 @@ void OnDestroy()
                 // Do not add logging to this (see above)
                 NetworkServer.Destroy(gameObject);
             }
+
+            if (isLocalPlayer)
+            {
+                ClientScene.ClearLocalPlayer();
+            }
         }
 
         internal void OnStartServer()
@@ -1546,6 +1551,11 @@ internal void Reset()
             networkBehavioursCache = null;
 
             ClearObservers();
+
+            if (isLocalPlayer)
+            {
+                ClientScene.ClearLocalPlayer();
+            }
         }
 
         /// <summary>

Assets/Mirror/Tests/Common/FakeNetworkConnection.cs

@@ -0,0 +1,23 @@
+using System;
+
+namespace Mirror.Tests
+{
+    public class FakeNetworkConnection : NetworkConnectionToClient
+    {
+        public FakeNetworkConnection() : base(1)
+        {
+        }
+
+        public override string address => "Test";
+
+        public override void Disconnect()
+        {
+            // nothing
+        }
+
+        internal override bool Send(ArraySegment<byte> segment, int channelId = 0)
+        {
+            return true;
+        }
+    }
+}

Assets/Mirror/Tests/Common/FakeNetworkConnection.cs.meta

@@ -0,0 +1,11 @@
+fileFormatVersion: 2
+guid: 935b1eb49c500674eaaf88982952a69e
+MonoImporter:
+  externalObjects: {}
+  serializedVersion: 2
+  defaultReferences: []
+  executionOrder: 0
+  icon: {instanceID: 0}
+  userData: 
+  assetBundleName: 
+  assetBundleVariant: 

Assets/Mirror/Tests/Editor/ClientSceneTests_OnSpawn.cs

@@ -7,25 +7,6 @@
 
 namespace Mirror.Tests.ClientSceneTests
 {
-    public class FakeNetworkConnection : NetworkConnectionToClient
-    {
-        public FakeNetworkConnection() : base(1)
-        {
-        }
-
-        public override string address => "Test";
-
-        public override void Disconnect()
-        {
-            // nothing
-        }
-
-        internal override bool Send(ArraySegment<byte> segment, int channelId = 0)
-        {
-            return true;
-        }
-    }
-
     public class PayloadTestBehaviour : NetworkBehaviour
     {
         public int value;

Assets/Mirror/Tests/Runtime/ClientSceneTests_LocalPlayer.cs

@@ -0,0 +1,152 @@
+using System.Collections;
+using System.Reflection;
+using NUnit.Framework;
+using UnityEngine;
+using UnityEngine.TestTools;
+
+namespace Mirror.Tests.Runtime.ClientSceneTests
+{
+    public class ClientSceneTests_LocalPlayer : ClientSceneTestsBase
+    {
+        [SetUp]
+        public void Setup()
+        {
+            Debug.Assert(ClientScene.localPlayer == null, "LocalPlayer should be null before this test");
+
+            PropertyInfo readyConnProperty = typeof(ClientScene).GetProperty(nameof(ClientScene.readyConnection));
+            readyConnProperty.SetValue(null, new FakeNetworkConnection());
+        }
+
+        NetworkIdentity SpawnObject(bool localPlayer)
+        {
+            const uint netId = 1000;
+
+            GameObject go = new GameObject();
+            _createdObjects.Add(go);
+
+            NetworkIdentity identity = go.AddComponent<NetworkIdentity>();
+
+            SpawnMessage msg = new SpawnMessage
+            {
+                netId = netId,
+                isLocalPlayer = localPlayer,
+                isOwner = localPlayer,
+            };
+
+            ClientScene.ApplySpawnPayload(identity, msg);
+
+            if (localPlayer)
+            {
+                Assert.That(ClientScene.localPlayer, Is.EqualTo(identity));
+            }
+
+            return identity;
+        }
+
+        [UnityTest]
+        public IEnumerator LocalPlayerIsSetToNullAfterDestroy()
+        {
+            NetworkIdentity player = SpawnObject(true);
+
+            GameObject.Destroy(player);
+
+            // wait a frame for destroy to happen
+            yield return null;
+
+            // use "is null" here to avoid unity == check
+            Assert.IsTrue(ClientScene.localPlayer is null, "local player should be set to c# null");
+        }
+
+        [UnityTest]
+        public IEnumerator DestroyingOtherObjectDoesntEffectLocalPlayer()
+        {
+            NetworkIdentity player = SpawnObject(true);
+            NetworkIdentity notPlayer = SpawnObject(false);
+
+            GameObject.Destroy(notPlayer);
+
+            // wait a frame for destroy to happen
+            yield return null;
+
+            Assert.IsTrue(ClientScene.localPlayer != null, "local player should not be cleared");
+            Assert.IsTrue(ClientScene.localPlayer == player, "local player should still be equal to player");
+        }
+
+        [UnityTest]
+        public IEnumerator LocalPlayerIsSetToNullAfterDestroyMessage()
+        {
+            NetworkIdentity player = SpawnObject(true);
+
+            ClientScene.OnObjectDestroy(new ObjectDestroyMessage
+            {
+                netId = player.netId
+            });
+
+            // wait a frame for destroy to happen
+            yield return null;
+
+            // use "is null" here to avoid unity == check
+            Assert.IsTrue(ClientScene.localPlayer is null, "local player should be set to c# null");
+        }
+    }
+    public class ClientSceneTest_LocalPlayer_asHost : HostSetup
+    {
+        [UnityTest]
+        public IEnumerator LocalPlayerIsSetToNullAfterNetworkDestroy()
+        {
+            const uint netId = 1000;
+
+            GameObject go = new GameObject();
+
+            NetworkIdentity identity = go.AddComponent<NetworkIdentity>();
+
+            SpawnMessage msg = new SpawnMessage
+            {
+                netId = netId,
+                isLocalPlayer = true,
+                isOwner = true,
+            };
+
+
+            NetworkIdentity.spawned[msg.netId] = identity;
+            ClientScene.OnHostClientSpawn(msg);
+
+            NetworkServer.Destroy(identity.gameObject);
+
+            // wait a frame for destroy to happen
+            yield return null;
+
+            // use "is null" here to avoid unity == check
+            Assert.IsTrue(ClientScene.localPlayer is null, "local player should be set to c# null");
+        }
+
+        [UnityTest]
+        public IEnumerator LocalPlayerIsSetToNullAfterNetworkUnspawn()
+        {
+            const uint netId = 1000;
+
+            GameObject go = new GameObject();
+
+            NetworkIdentity identity = go.AddComponent<NetworkIdentity>();
+
+            SpawnMessage msg = new SpawnMessage
+            {
+                netId = netId,
+                isLocalPlayer = true,
+                isOwner = true,
+            };
+
+
+            NetworkIdentity.spawned[msg.netId] = identity;
+            ClientScene.OnHostClientSpawn(msg);
+
+            NetworkServer.UnSpawn(identity.gameObject);
+
+            // wait a frame for destroy to happen
+            yield return null;
+
+            // use "is null" here to avoid unity == check
+            Assert.IsTrue(ClientScene.localPlayer is null, "local player should be set to c# null");
+        }
+    }
+}

Assets/Mirror/Tests/Runtime/ClientSceneTests_LocalPlayer.cs.meta

@@ -0,0 +1,11 @@
+fileFormatVersion: 2
+guid: 7c087df10f51b674f94f84ba30a303f4
+MonoImporter:
+  externalObjects: {}
+  serializedVersion: 2
+  defaultReferences: []
+  executionOrder: 0
+  icon: {instanceID: 0}
+  userData: 
+  assetBundleName: 
+  assetBundleVariant: