fix!: fixing spawning for unauthenticated code

OnlySpawnOnAuthenticated field removed because the previous authentication changes mean that spawnMessage can only be received when authenticated

BREAKING CHANGE: removing ServerObjectManager.OnlySpawnOnAuthenticated field

Assets/Mirage/Runtime/INetworkVisibility.cs

@@ -13,13 +13,11 @@ public interface INetworkVisibility
     /// </summary>
     internal class AlwaysVisible : INetworkVisibility
     {
-        private readonly ServerObjectManager _objectManager;
         private readonly NetworkServer _server;
 
-        public AlwaysVisible(ServerObjectManager serverObjectManager)
+        public AlwaysVisible(NetworkServer server)
         {
-            _objectManager = serverObjectManager;
-            _server = serverObjectManager.Server;
+            _server = server;
         }
 
         public bool OnCheckObserver(INetworkPlayer player) => true;
@@ -29,21 +27,17 @@ public void OnRebuildObservers(HashSet<INetworkPlayer> observers, bool initializ
             // add all server connections
             foreach (var player in _server.Players)
             {
-                if (!player.SceneIsReady)
+                // skip players that have not authenticated
+                // this is needed because Message by default (including scene message) check IsAuthenticated 
+                if (!player.IsAuthenticated)
                     continue;
 
-                // todo replace this with a better visibility system (where default checks auth/scene ready)
-                if (_objectManager.OnlySpawnOnAuthenticated && !player.IsAuthenticated)
+                // skip players that are loading a scene
+                if (!player.SceneIsReady)
                     continue;
 
                 observers.Add(player);
             }
-
-            // add local host connection (if any)
-            if (_server.LocalPlayer != null && _server.LocalPlayer.SceneIsReady)
-            {
-                observers.Add(_server.LocalPlayer);
-            }
         }
     }
 }

Assets/Mirage/Runtime/NetworkServer.cs

@@ -131,7 +131,7 @@ public class NetworkServer : MonoBehaviour
         public bool IsHost => LocalClient != null && LocalClient.Active;
 
         /// <summary>
-        /// A list of local connections on the server.
+        /// A list of connections on the server.
         /// </summary>
         public IReadOnlyCollection<INetworkPlayer> Players => _connections.Values;
 

Assets/Mirage/Runtime/ServerObjectManager.cs

@@ -33,10 +33,6 @@ public class ServerObjectManager : MonoBehaviour
         private NetworkServer _server;
         public NetworkServer Server => _server;
 
-        [Header("Authentication")]
-        [Tooltip("Will only send spawn message to Players who are Authenticated. Checks the Player.IsAuthenticated property")]
-        public bool OnlySpawnOnAuthenticated;
-
         public INetIdGenerator NetIdGenerator;
         private uint _nextNetworkId = 1;
 
@@ -52,7 +48,7 @@ internal void ServerStarted(NetworkServer server)
             _server = server;
             _server.Stopped.AddListener(OnServerStopped);
 
-            DefaultVisibility = new AlwaysVisible(this);
+            DefaultVisibility = new AlwaysVisible(server);
 
             _rpcHandler = new RpcHandler(_server.MessageHandler, _server.World, RpcInvokeType.ServerRpc);
         }
@@ -383,8 +379,8 @@ public void Spawn(NetworkIdentity identity)
 
         internal void SendSpawnMessage(NetworkIdentity identity, INetworkPlayer player)
         {
-            logger.Assert(!OnlySpawnOnAuthenticated || player.IsAuthenticated || identity.Visibility != DefaultVisibility,
-                "SendSpawnMessage should only be called if OnlySpawnOnAuthenticated is false, player is authenticated, or there is custom visibility");
+            logger.Assert(player.IsAuthenticated || identity.Visibility is not AlwaysVisible,
+                "SendSpawnMessage should only be called if player is authenticated, or there is custom visibility");
             if (logger.LogEnabled()) logger.Log($"Server SendSpawnMessage: name={identity.name} sceneId={identity.SceneId:X} netId={identity.NetId}");
 
             // one writer for owner, one for observers

Assets/Tests/Common/TestExtensions.cs

@@ -16,14 +16,21 @@ public static void SetSceneId(this NetworkIdentity identity, int id, int hash =
 
     public static class NetworkServerTestExtensions
     {
-        public static void AddTestPlayer(this NetworkServer server, INetworkPlayer player)
+        public static void AddTestPlayer(this NetworkServer server, INetworkPlayer player, bool authenticated = true)
         {
             var info = typeof(NetworkServer).GetField("_connections", BindingFlags.Instance | BindingFlags.NonPublic);
             var connections = (Dictionary<IConnection, INetworkPlayer>)info.GetValue(server);
 
-            var connectiion = Substitute.For<IConnection>();
-            player.Connection.Returns(connectiion);
-            connections.Add(connectiion, player);
+            var connection = Substitute.For<IConnection>();
+            player.Connection.Returns(connection);
+            if (authenticated)
+            {
+                player.IsAuthenticated.Returns(true);
+                var auth = new Authentication.PlayerAuthentication(null, null);
+                player.Authentication.Returns(auth);
+            }
+
+            connections.Add(connection, player);
         }
     }
 }

Assets/Tests/Runtime/NetworkIdentityCallbackTests.cs

@@ -22,6 +22,7 @@ public override void OnRebuildObservers(HashSet<INetworkPlayer> observers, bool
         private NetworkIdentity identity;
         private INetworkPlayer player1;
         private INetworkPlayer player2;
+        private INetworkPlayer player3;
 
         protected override async UniTask ExtraSetup()
         {
@@ -33,32 +34,38 @@ protected override async UniTask ExtraSetup()
 
             player1 = Substitute.For<INetworkPlayer>();
             player2 = Substitute.For<INetworkPlayer>();
+            player3 = Substitute.For<INetworkPlayer>();
         }
 
         [Test]
         public void AlwaysVisibleAddsAllReadyPlayers()
         {
             player1.SceneIsReady.Returns(true);
             player2.SceneIsReady.Returns(false);
+            player3.SceneIsReady.Returns(true);
 
             // add some server connections
             server.AddTestPlayer(player1);
             server.AddTestPlayer(player2);
+            server.AddTestPlayer(player3, false);
 
             // add a host connection
             server.AddLocalConnection(client, Substitute.For<SocketLayer.IConnection>());
             server.Connected.Invoke(server.LocalPlayer);
             server.LocalPlayer.SceneIsReady = true;
+            server.LocalPlayer.SetAuthentication(new Mirage.Authentication.PlayerAuthentication(null, null));
 
             // call OnStartServer so that observers dict is created
             identity.StartServer();
 
-            var alwaysVisible = new AlwaysVisible(serverObjectManager);
+            var alwaysVisible = new AlwaysVisible(server);
 
             // add all to observers. should have the two ready connections then.
             var newObservers = new HashSet<INetworkPlayer>();
             alwaysVisible.OnRebuildObservers(newObservers, true);
 
+            // player2 should be missing, because not ready
+            // player3 should be missing, because not authenticated
             Assert.That(newObservers, Is.EquivalentTo(new[] { player1, server.LocalPlayer, serverPlayer }));
         }