From 2d2e27086868453187148d57bdf5aeb894bb3692 Mon Sep 17 00:00:00 2001 From: MrGadget <9826063+MrGadget1024@users.noreply.github.com> Date: Fri, 5 Jul 2024 05:58:43 -0400 Subject: [PATCH] feat(Transport): Added OnServerConnectedWithAddress (#3855) * feat(Transport): Added OnServerConnectedWithAddress Transports can now pass the remote client address directly to NetworkServer Action - Facliltates ThreadedTransport passing the client Address - KCP Transport Updated accordingly - Original OnServerConnected passes ServerGetClientAddress result to NetworkConnectionToClient consrtuctor - Saves round trips back to the transport for client address whenever it's needed * formatting * Simplified * Cleanup --- Assets/Mirror/Core/LocalConnectionToClient.cs | 2 - .../Mirror/Core/NetworkConnectionToClient.cs | 6 ++- Assets/Mirror/Core/NetworkServer.cs | 44 ++++++++++++------- Assets/Mirror/Core/Transport.cs | 1 + .../Editor/NetworkServer/NetworkServerTest.cs | 4 ++ .../Edgegap/EdgegapRelay/EdgegapKcpServer.cs | 2 +- .../EdgegapRelay/EdgegapKcpTransport.cs | 2 +- Assets/Mirror/Transports/KCP/KcpTransport.cs | 2 +- .../KCP/kcp2k/highlevel/KcpServer.cs | 7 +-- .../Transports/Threaded/ThreadedTransport.cs | 15 ++++--- 10 files changed, 52 insertions(+), 33 deletions(-) diff --git a/Assets/Mirror/Core/LocalConnectionToClient.cs b/Assets/Mirror/Core/LocalConnectionToClient.cs index c41c6ae29..3ceba2993 100644 --- a/Assets/Mirror/Core/LocalConnectionToClient.cs +++ b/Assets/Mirror/Core/LocalConnectionToClient.cs @@ -14,8 +14,6 @@ public class LocalConnectionToClient : NetworkConnectionToClient public LocalConnectionToClient() : base(LocalConnectionId) {} - public override string address => "localhost"; - internal override void Send(ArraySegment segment, int channelId = Channels.Reliable) { // instead of invoking it directly, we enqueue and process next update. diff --git a/Assets/Mirror/Core/NetworkConnectionToClient.cs b/Assets/Mirror/Core/NetworkConnectionToClient.cs index 68e205fd3..802e46874 100644 --- a/Assets/Mirror/Core/NetworkConnectionToClient.cs +++ b/Assets/Mirror/Core/NetworkConnectionToClient.cs @@ -14,7 +14,7 @@ public class NetworkConnectionToClient : NetworkConnection readonly NetworkWriter reliableRpcs = new NetworkWriter(); readonly NetworkWriter unreliableRpcs = new NetworkWriter(); - public virtual string address => Transport.active.ServerGetClientAddress(connectionId); + public virtual string address { get; private set; } /// NetworkIdentities that this connection can see // TODO move to server's NetworkConnectionToClient? @@ -50,9 +50,11 @@ public class NetworkConnectionToClient : NetworkConnection /// Round trip time (in seconds) that it takes a message to go server->client->server. public double rtt => _rtt.Value; - public NetworkConnectionToClient(int networkConnectionId) + public NetworkConnectionToClient(int networkConnectionId, string clientAddress = "localhost") : base(networkConnectionId) { + address = clientAddress; + // initialize EMA with 'emaDuration' seconds worth of history. // 1 second holds 'sendRate' worth of values. // multiplied by emaDuration gives n-seconds. diff --git a/Assets/Mirror/Core/NetworkServer.cs b/Assets/Mirror/Core/NetworkServer.cs index 716a0fe47..0833a4e33 100644 --- a/Assets/Mirror/Core/NetworkServer.cs +++ b/Assets/Mirror/Core/NetworkServer.cs @@ -192,6 +192,7 @@ static void AddTransportHandlers() { // += so that other systems can also hook into it (i.e. statistics) Transport.active.OnServerConnected += OnTransportConnected; + Transport.active.OnServerConnectedWithAddress += OnTransportConnectedWithAddress; Transport.active.OnServerDataReceived += OnTransportData; Transport.active.OnServerDisconnected += OnTransportDisconnected; Transport.active.OnServerError += OnTransportError; @@ -636,25 +637,39 @@ public static void SendToReadyObservers(NetworkIdentity identity, T message, // transport events //////////////////////////////////////////////////// // called by transport static void OnTransportConnected(int connectionId) - { - // Debug.Log($"Server accepted client:{connectionId}"); + => OnTransportConnectedWithAddress(connectionId, Transport.active.ServerGetClientAddress(connectionId)); + static void OnTransportConnectedWithAddress(int connectionId, string clientAddress) + { + if (IsConnectionAllowed(connectionId)) + { + // create a connection + NetworkConnectionToClient conn = new NetworkConnectionToClient(connectionId, clientAddress); + OnConnected(conn); + } + else + { + // kick the client immediately + Transport.active.ServerDisconnect(connectionId); + } + } + + static bool IsConnectionAllowed(int connectionId) + { // connectionId needs to be != 0 because 0 is reserved for local player // note that some transports like kcp generate connectionId by // hashing which can be < 0 as well, so we need to allow < 0! if (connectionId == 0) { Debug.LogError($"Server.HandleConnect: invalid connectionId: {connectionId} . Needs to be != 0, because 0 is reserved for local player."); - Transport.active.ServerDisconnect(connectionId); - return; + return false; } // connectionId not in use yet? if (connections.ContainsKey(connectionId)) { - Transport.active.ServerDisconnect(connectionId); - // Debug.Log($"Server connectionId {connectionId} already in use...kicked client"); - return; + Debug.LogError($"Server connectionId {connectionId} already in use...client will be kicked"); + return false; } // are more connections allowed? if not, kick @@ -662,18 +677,13 @@ static void OnTransportConnected(int connectionId) // less code and third party transport might not do that anyway) // (this way we could also send a custom 'tooFull' message later, // Transport can't do that) - if (connections.Count < maxConnections) + if (connections.Count >= maxConnections) { - // add connection - NetworkConnectionToClient conn = new NetworkConnectionToClient(connectionId); - OnConnected(conn); - } - else - { - // kick - Transport.active.ServerDisconnect(connectionId); - // Debug.Log($"Server full, kicked client {connectionId}"); + Debug.LogError($"Server full, client {connectionId} will be kicked"); + return false; } + + return true; } internal static void OnConnected(NetworkConnectionToClient conn) diff --git a/Assets/Mirror/Core/Transport.cs b/Assets/Mirror/Core/Transport.cs index a20d84f78..6d4f70703 100644 --- a/Assets/Mirror/Core/Transport.cs +++ b/Assets/Mirror/Core/Transport.cs @@ -68,6 +68,7 @@ public abstract class Transport : MonoBehaviour // server ////////////////////////////////////////////////////////////// /// Called by Transport when a new client connected to the server. public Action OnServerConnected; + public Action OnServerConnectedWithAddress; /// Called by Transport when the server received a message from a client. public Action, int> OnServerDataReceived; diff --git a/Assets/Mirror/Tests/Editor/NetworkServer/NetworkServerTest.cs b/Assets/Mirror/Tests/Editor/NetworkServer/NetworkServerTest.cs index 0c8b77788..939fe010d 100644 --- a/Assets/Mirror/Tests/Editor/NetworkServer/NetworkServerTest.cs +++ b/Assets/Mirror/Tests/Editor/NetworkServer/NetworkServerTest.cs @@ -71,7 +71,9 @@ public void MaxConnections() Assert.That(NetworkServer.connections.Count, Is.EqualTo(1)); // connect second: should fail + LogAssert.ignoreFailingMessages = true; transport.OnServerConnected.Invoke(43); + LogAssert.ignoreFailingMessages = false; Assert.That(NetworkServer.connections.Count, Is.EqualTo(1)); } @@ -161,7 +163,9 @@ public void ConnectDuplicateConnectionIds() NetworkConnectionToClient original = NetworkServer.connections[42]; // connect duplicate - shouldn't overwrite first one + LogAssert.ignoreFailingMessages = true; transport.OnServerConnected.Invoke(42); + LogAssert.ignoreFailingMessages = false; Assert.That(NetworkServer.connections.Count, Is.EqualTo(1)); Assert.That(NetworkServer.connections[42], Is.EqualTo(original)); } diff --git a/Assets/Mirror/Transports/Edgegap/EdgegapRelay/EdgegapKcpServer.cs b/Assets/Mirror/Transports/Edgegap/EdgegapRelay/EdgegapKcpServer.cs index 9aaf75f75..cc87b14f3 100644 --- a/Assets/Mirror/Transports/Edgegap/EdgegapRelay/EdgegapKcpServer.cs +++ b/Assets/Mirror/Transports/Edgegap/EdgegapRelay/EdgegapKcpServer.cs @@ -28,7 +28,7 @@ public class EdgegapKcpServer : KcpServer bool relayActive; public EdgegapKcpServer( - Action OnConnected, + Action OnConnected, Action, KcpChannel> OnData, Action OnDisconnected, Action OnError, diff --git a/Assets/Mirror/Transports/Edgegap/EdgegapRelay/EdgegapKcpTransport.cs b/Assets/Mirror/Transports/Edgegap/EdgegapRelay/EdgegapKcpTransport.cs index a7e2ff352..e0a410033 100644 --- a/Assets/Mirror/Transports/Edgegap/EdgegapRelay/EdgegapKcpTransport.cs +++ b/Assets/Mirror/Transports/Edgegap/EdgegapRelay/EdgegapKcpTransport.cs @@ -60,7 +60,7 @@ protected override void Awake() // server server = new EdgegapKcpServer( - (connectionId) => OnServerConnected.Invoke(connectionId), + (connectionId, endPoint) => OnServerConnectedWithAddress.Invoke(connectionId, endPoint.PrettyAddress()), (connectionId, message, channel) => OnServerDataReceived.Invoke(connectionId, message, FromKcpChannel(channel)), (connectionId) => OnServerDisconnected.Invoke(connectionId), (connectionId, error, reason) => OnServerError.Invoke(connectionId, ToTransportError(error), reason), diff --git a/Assets/Mirror/Transports/KCP/KcpTransport.cs b/Assets/Mirror/Transports/KCP/KcpTransport.cs index bec9c1381..2be24af8a 100644 --- a/Assets/Mirror/Transports/KCP/KcpTransport.cs +++ b/Assets/Mirror/Transports/KCP/KcpTransport.cs @@ -122,7 +122,7 @@ protected virtual void Awake() // server server = new KcpServer( - (connectionId) => OnServerConnected.Invoke(connectionId), + (connectionId, endPoint) => OnServerConnectedWithAddress.Invoke(connectionId, endPoint.PrettyAddress()), (connectionId, message, channel) => OnServerDataReceived.Invoke(connectionId, message, FromKcpChannel(channel)), (connectionId) => OnServerDisconnected.Invoke(connectionId), (connectionId, error, reason) => OnServerError.Invoke(connectionId, ToTransportError(error), reason), diff --git a/Assets/Mirror/Transports/KCP/kcp2k/highlevel/KcpServer.cs b/Assets/Mirror/Transports/KCP/kcp2k/highlevel/KcpServer.cs index 5b8ce4d56..25648cf3f 100644 --- a/Assets/Mirror/Transports/KCP/kcp2k/highlevel/KcpServer.cs +++ b/Assets/Mirror/Transports/KCP/kcp2k/highlevel/KcpServer.cs @@ -18,7 +18,7 @@ public class KcpServer // events are readonly, set in constructor. // this ensures they are always initialized when used. // fixes https://github.com/MirrorNetworking/Mirror/issues/3337 and more - protected readonly Action OnConnected; + protected readonly Action OnConnected; // connectionId, address protected readonly Action, KcpChannel> OnData; protected readonly Action OnDisconnected; protected readonly Action OnError; @@ -43,7 +43,7 @@ public class KcpServer public Dictionary connections = new Dictionary(); - public KcpServer(Action OnConnected, + public KcpServer(Action OnConnected, Action, KcpChannel> OnData, Action OnDisconnected, Action OnError, @@ -285,7 +285,8 @@ void OnConnectedCallback(KcpServerConnection conn) // finally, call mirror OnConnected event Log.Info($"[KCP] Server: OnConnected({connectionId})"); - OnConnected(connectionId); + IPEndPoint endPoint = conn.remoteEndPoint as IPEndPoint; + OnConnected(connectionId, endPoint); } void OnDisconnectedCallback() diff --git a/Assets/Mirror/Transports/Threaded/ThreadedTransport.cs b/Assets/Mirror/Transports/Threaded/ThreadedTransport.cs index a931d161d..e760b8464 100644 --- a/Assets/Mirror/Transports/Threaded/ThreadedTransport.cs +++ b/Assets/Mirror/Transports/Threaded/ThreadedTransport.cs @@ -6,6 +6,7 @@ using System; using System.Collections.Concurrent; using System.Runtime.CompilerServices; +using System.Net; using System.Threading; using UnityEngine; @@ -317,9 +318,11 @@ protected void OnThreadedClientDisconnected() EnqueueClientMain(ClientMainEventType.OnClientDisconnected, null, null, null); } - protected void OnThreadedServerConnected(int connectionId) + protected void OnThreadedServerConnected(int connectionId, IPEndPoint endPoint) { - EnqueueServerMain(ServerMainEventType.OnServerConnected, null, connectionId, null, null); + // create string copy of address immediately before sending to another thread + string address = endPoint.PrettyAddress(); + EnqueueServerMain(ServerMainEventType.OnServerConnected, address, connectionId, null, null); } protected void OnThreadedServerSend(int connectionId, ArraySegment message, int channelId) @@ -515,9 +518,9 @@ public override void ServerEarlyUpdate() // SERVER EVENTS /////////////////////////////////////////// case ServerMainEventType.OnServerConnected: { - // call original transport event - // TODO pass client address in OnConnect here later - OnServerConnected?.Invoke(elem.connectionId.Value);//, (string)elem.param); + // call original transport event with connectionId, address + string address = (string)elem.param; + OnServerConnectedWithAddress?.Invoke(elem.connectionId.Value, address); break; } case ServerMainEventType.OnServerSent: @@ -612,7 +615,7 @@ public override void ServerDisconnect(int connectionId) // querying this at runtime won't work for threaded transports. public override string ServerGetClientAddress(int connectionId) { - throw new NotImplementedException(); + throw new NotImplementedException("ThreadedTransport passes each connection's address in OnServerConnectedThreaded. Don't use ServerGetClientAddress."); } public override void ServerStop()