diff --git a/Assets/Mirage/Runtime/INetworkVisibility.cs b/Assets/Mirage/Runtime/INetworkVisibility.cs index bb5abc72bd..f31d1b8818 100644 --- a/Assets/Mirage/Runtime/INetworkVisibility.cs +++ b/Assets/Mirage/Runtime/INetworkVisibility.cs @@ -13,13 +13,11 @@ public interface INetworkVisibility /// 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 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); - } } } } diff --git a/Assets/Mirage/Runtime/NetworkServer.cs b/Assets/Mirage/Runtime/NetworkServer.cs index 2733ce2924..295c38fdbb 100644 --- a/Assets/Mirage/Runtime/NetworkServer.cs +++ b/Assets/Mirage/Runtime/NetworkServer.cs @@ -131,7 +131,7 @@ public class NetworkServer : MonoBehaviour public bool IsHost => LocalClient != null && LocalClient.Active; /// - /// A list of local connections on the server. + /// A list of connections on the server. /// public IReadOnlyCollection Players => _connections.Values; diff --git a/Assets/Mirage/Runtime/ServerObjectManager.cs b/Assets/Mirage/Runtime/ServerObjectManager.cs index 4f70e47e84..2ffcde6cc5 100644 --- a/Assets/Mirage/Runtime/ServerObjectManager.cs +++ b/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 diff --git a/Assets/Tests/Common/TestExtensions.cs b/Assets/Tests/Common/TestExtensions.cs index bed4c6fd78..aa9f1dd39c 100644 --- a/Assets/Tests/Common/TestExtensions.cs +++ b/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)info.GetValue(server); - var connectiion = Substitute.For(); - player.Connection.Returns(connectiion); - connections.Add(connectiion, player); + var connection = Substitute.For(); + 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); } } } diff --git a/Assets/Tests/Runtime/NetworkIdentityCallbackTests.cs b/Assets/Tests/Runtime/NetworkIdentityCallbackTests.cs index 70af7ee3cb..090d6440a4 100644 --- a/Assets/Tests/Runtime/NetworkIdentityCallbackTests.cs +++ b/Assets/Tests/Runtime/NetworkIdentityCallbackTests.cs @@ -22,6 +22,7 @@ public override void OnRebuildObservers(HashSet observers, bool private NetworkIdentity identity; private INetworkPlayer player1; private INetworkPlayer player2; + private INetworkPlayer player3; protected override async UniTask ExtraSetup() { @@ -33,6 +34,7 @@ protected override async UniTask ExtraSetup() player1 = Substitute.For(); player2 = Substitute.For(); + player3 = Substitute.For(); } [Test] @@ -40,25 +42,30 @@ 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()); 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(); 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 })); }