diff --git a/Assets/Mirror/Core/NetworkServer.cs b/Assets/Mirror/Core/NetworkServer.cs index 51cc4caec..716a0fe47 100644 --- a/Assets/Mirror/Core/NetworkServer.cs +++ b/Assets/Mirror/Core/NetworkServer.cs @@ -1592,14 +1592,12 @@ static void SpawnObject(GameObject obj, NetworkConnection ownerConnection) RebuildObservers(identity, true); } - /// This takes an object that has been spawned and un-spawns it. - // The object will be removed from clients that it was spawned on, or - // the custom spawn handler function on the client will be called for - // the object. - // Unlike when calling NetworkServer.Destroy(), on the server the object - // will NOT be destroyed. This allows the server to re-use the object, - // even spawn it again later. - public static void UnSpawn(GameObject obj) + // internal Unspawn function which has the 'resetState' parameter. + // resetState calls .ResetState() on the object after unspawning. + // this is necessary for scene objects, but not for prefabs since we + // don't want to reset their isServer flags etc. + // fixes: https://github.com/MirrorNetworking/Mirror/issues/3832 + static void UnSpawnInternal(GameObject obj, bool resetState) { // Debug.Log($"DestroyObject instance:{identity.netId}"); @@ -1673,10 +1671,22 @@ public static void UnSpawn(GameObject obj) identity.OnStopServer(); // finally reset the state and deactivate it - identity.ResetState(); - identity.gameObject.SetActive(false); + if (resetState) + { + identity.ResetState(); + identity.gameObject.SetActive(false); + } } + /// This takes an object that has been spawned and un-spawns it. + // The object will be removed from clients that it was spawned on, or + // the custom spawn handler function on the client will be called for + // the object. + // Unlike when calling NetworkServer.Destroy(), on the server the object + // will NOT be destroyed. This allows the server to re-use the object, + // even spawn it again later. + public static void UnSpawn(GameObject obj) => UnSpawnInternal(obj, resetState: true); + // destroy ///////////////////////////////////////////////////////////// /// Destroys this object and corresponding objects on all clients. // In some cases it is useful to remove an object but not delete it on @@ -1698,17 +1708,31 @@ public static void Destroy(GameObject obj) return; } - // first, we unspawn it on clients and server - UnSpawn(obj); + // get the NetworkIdentity component first + if (!GetNetworkIdentity(obj, out NetworkIdentity identity)) + { + Debug.LogWarning($"NetworkServer.Destroy() called on {obj.name} which doesn't have a NetworkIdentity component."); + return; + } - // additionally, if it's a prefab then we destroy it completely. + // is this a scene object? + // then we simply unspawn & reset it so it can still be spawned again. // we never destroy scene objects on server or on client, since once // they are gone, they are gone forever and can't be instantiate again. // for example, server may Destroy() a scene object and once a match // restarts, the scene objects would be gone from the new match. - if (GetNetworkIdentity(obj, out NetworkIdentity identity) && - identity.sceneId == 0) + if (identity.sceneId != 0) { + UnSpawnInternal(obj, resetState: true); + } + // is this a prefab? + // then we destroy it completely. + else + { + // unspawn without calling ResetState. + // otherwise isServer/isClient flags might be reset in OnDestroy. + // fixes: https://github.com/MirrorNetworking/Mirror/issues/3832 + UnSpawnInternal(obj, resetState: false); identity.destroyCalled = true; // Destroy if application is running diff --git a/Assets/Mirror/Tests/Editor/NetworkBehaviour/NetworkBehaviourTests.cs b/Assets/Mirror/Tests/Editor/NetworkBehaviour/NetworkBehaviourTests.cs index 181cdd4d8..b78702891 100644 --- a/Assets/Mirror/Tests/Editor/NetworkBehaviour/NetworkBehaviourTests.cs +++ b/Assets/Mirror/Tests/Editor/NetworkBehaviour/NetworkBehaviourTests.cs @@ -855,6 +855,28 @@ public void OnStopLocalPlayer() Assert.That(comp.onStopLocalPlayerCalled, Is.EqualTo(1)); } + // test to prevent: https://github.com/MirrorNetworking/Mirror/issues/3832 + class NetworkBehaviourOnDestroy : NetworkBehaviour + { + void OnDestroy() + { + Debug.LogWarning("OnDestroy called with isServer=" + isServer); + // on server, IsServer should still be false in OnDestroy. + Assert.That(isServer, Is.EqualTo(NetworkServer.active)); + } + } + [Test] + public void NetworkDestroy_OnDestroyFlags() + { + NetworkServer.Listen(1); + ConnectClientBlockingAuthenticatedAndReady(out _); + + CreateNetworked(out GameObject _, out NetworkIdentity identity, out NetworkBehaviourOnDestroy comp); + NetworkServer.Destroy(identity.gameObject); + + // the NetworkBehaviourOnDestroy component has the asset to check isServer.. + } + [Test] public void AuthorityIsFalseVariations() {