refactor: spawnobjects throws exception instead of returning false (M…

…irageNet#639)

ServerObjectManager.SpawnObjects now throw InvalidOperationException is server is not set or is not active

BREAKING CHANGE: SpawnObjects throws Exception instead of returning false

Assets/Mirage/Runtime/IServerObjectManager.cs

@@ -34,6 +34,6 @@ public interface IServerObjectManager
 
         void UnSpawn(GameObject obj);
 
-        bool SpawnObjects();
+        void SpawnObjects();
     }
 }

Assets/Mirage/Runtime/ServerObjectManager.cs

@@ -699,15 +699,18 @@ public int Compare(NetworkIdentity x, NetworkIdentity y)
 
         /// <summary>
         /// This causes NetworkIdentity objects in a scene to be spawned on a server.
-        /// <para>NetworkIdentity objects in a scene are disabled by default. Calling SpawnObjects() causes these scene objects to be enabled and spawned. It is like calling NetworkServer.Spawn() for each of them.</para>
+        /// <para>
+        ///     NetworkIdentity objects in a scene are disabled by default.
+        ///     Calling SpawnObjects() causes these scene objects to be enabled and spawned.
+        ///     It is like calling NetworkServer.Spawn() for each of them.
+        /// </para>
         /// </summary>
-        /// <param name="client">The client associated to the objects.</param>
-        /// <returns>Success if objects where spawned.</returns>
-        public bool SpawnObjects()
+        /// <exception cref="T:InvalidOperationException">Thrown when server is not active</exception>
+        public void SpawnObjects()
         {
             // only if server active
             if (!Server || !Server.Active)
-                return false;
+                throw new InvalidOperationException("Server was not active");
 
             NetworkIdentity[] identities = Resources.FindObjectsOfTypeAll<NetworkIdentity>();
             Array.Sort(identities, new NetworkIdentityComparer());
@@ -722,8 +725,6 @@ public bool SpawnObjects()
                     Spawn(identity.gameObject);
                 }
             }
-
-            return true;
         }
 
         /// <summary>

Assets/Tests/Runtime/ClientServer/ServerObjectManagerTests.cs

@@ -87,7 +87,7 @@ public void SpawnSceneObject()
         {
             serverIdentity.sceneId = 42;
             serverIdentity.gameObject.SetActive(false);
-            Assert.That(serverObjectManager.SpawnObjects(), Is.True);
+            serverObjectManager.SpawnObjects();
             Assert.That(serverIdentity.gameObject.activeSelf, Is.True);
         }
 
@@ -96,7 +96,7 @@ public void SpawnPrefabObject()
         {
             serverIdentity.sceneId = 0;
             serverIdentity.gameObject.SetActive(false);
-            Assert.That(serverObjectManager.SpawnObjects(), Is.True);
+            serverObjectManager.SpawnObjects();
             Assert.That(serverIdentity.gameObject.activeSelf, Is.False);
         }
 
@@ -260,7 +260,12 @@ public IEnumerator SpawnObjectsFalseTest() => UniTask.ToCoroutine(async () =>
 
             await AsyncUtil.WaitUntilWithTimeout(() => !server.Active);
 
-            Assert.That(serverObjectManager.SpawnObjects(), Is.False);
+            InvalidOperationException exception = Assert.Throws<InvalidOperationException>(() =>
+            {
+                serverObjectManager.SpawnObjects();
+            });
+
+            Assert.That(exception, Has.Message.EqualTo("Server was not active"));
         });
     }
 }

Assets/Tests/Runtime/Host/HostSetup.cs

@@ -1,5 +1,6 @@
 using System.Collections;
 using Cysharp.Threading.Tasks;
+using NUnit.Framework;
 using UnityEngine;
 using UnityEngine.TestTools;
 
@@ -28,6 +29,9 @@ public virtual void ExtraSetup() { }
         public IEnumerator SetupHost() => UniTask.ToCoroutine(async () =>
         {
             networkManagerGo = new GameObject();
+            // set gameobject name to test name (helps with debugging)
+            networkManagerGo.name = TestContext.CurrentContext.Test.MethodName;
+
             networkManagerGo.AddComponent<MockTransport>();
             sceneManager = networkManagerGo.AddComponent<NetworkSceneManager>();
             serverObjectManager = networkManagerGo.AddComponent<ServerObjectManager>();

Assets/Tests/Runtime/Host/PlayerSpawnerTest.cs

@@ -1,5 +1,8 @@
+using System.Collections;
+using Cysharp.Threading.Tasks;
 using NUnit.Framework;
 using UnityEngine;
+using UnityEngine.TestTools;
 
 namespace Mirage.Tests.Host
 {
@@ -36,16 +39,19 @@ public override void ExtraTearDown()
             Object.Destroy(player);
         }
 
-        [Test]
-        public void DontAutoSpawnTest()
+        [UnityTest]
+        public IEnumerator DontAutoSpawnTest() => UniTask.ToCoroutine(async () =>
         {
             bool invokeAddPlayerMessage = false;
             server.LocalConnection.RegisterHandler<AddPlayerMessage>(msg => invokeAddPlayerMessage = true);
 
             sceneManager.ChangeServerScene("Assets/Mirror/Tests/Runtime/testScene.unity");
+            // wait for messages to be processed
+            await UniTask.Yield();
 
             Assert.That(invokeAddPlayerMessage, Is.False);
-        }
+
+        });
 
         [Test]
         public void ManualSpawnTest()