From 24a7968cc24aaef47cec31e7032b4f7dd01b1d26 Mon Sep 17 00:00:00 2001 From: Paul Pacheco Date: Fri, 25 Oct 2019 23:01:54 -0500 Subject: [PATCH] refactor: Split NC for clients and servers (#1175) NetworkConnection has been broken down into: NetworkConnection NetworkConnectionToClient NetworkConnectionToServer This way each class does one thing only, instead of trying to accommodate client and server functionality in the same class. I can't go any further than this without breaking a lot of code. Ideally we would move the connectionId down to NetworkConnectionToClient, but that will result in a lot of breaking changes. --- Assets/Mirror/Runtime/LocalConnections.cs | 12 +-- Assets/Mirror/Runtime/NetworkClient.cs | 2 +- Assets/Mirror/Runtime/NetworkConnection.cs | 74 ++----------------- .../Runtime/NetworkConnectionToClient.cs | 61 +++++++++++++++ .../Runtime/NetworkConnectionToClient.cs.meta | 11 +++ .../Runtime/NetworkConnectionToServer.cs | 36 +++++++++ .../Runtime/NetworkConnectionToServer.cs.meta | 11 +++ Assets/Mirror/Runtime/NetworkServer.cs | 31 ++++---- 8 files changed, 147 insertions(+), 91 deletions(-) create mode 100644 Assets/Mirror/Runtime/NetworkConnectionToClient.cs create mode 100644 Assets/Mirror/Runtime/NetworkConnectionToClient.cs.meta create mode 100644 Assets/Mirror/Runtime/NetworkConnectionToServer.cs create mode 100644 Assets/Mirror/Runtime/NetworkConnectionToServer.cs.meta diff --git a/Assets/Mirror/Runtime/LocalConnections.cs b/Assets/Mirror/Runtime/LocalConnections.cs index 5dfca4136..56fd349e1 100644 --- a/Assets/Mirror/Runtime/LocalConnections.cs +++ b/Assets/Mirror/Runtime/LocalConnections.cs @@ -5,12 +5,14 @@ namespace Mirror { // a server's connection TO a LocalClient. // sending messages on this connection causes the client's handler function to be invoked directly - class ULocalConnectionToClient : NetworkConnection + class ULocalConnectionToClient : NetworkConnectionToClient { - public ULocalConnectionToClient() : base ("localClient") + public ULocalConnectionToClient() : base (0) { } + public override string address => "localhost"; + internal override bool Send(ArraySegment segment, int channelId = Channels.DefaultReliable) { // LocalConnection doesn't support allocation-free sends yet. @@ -24,11 +26,9 @@ internal override bool Send(ArraySegment segment, int channelId = Channels // a localClient's connection TO a server. // send messages on this connection causes the server's handler function to be invoked directly. - internal class ULocalConnectionToServer : NetworkConnection + internal class ULocalConnectionToServer : NetworkConnectionToServer { - public ULocalConnectionToServer() : base("localServer") - { - } + public override string address => "localhost"; internal override bool Send(ArraySegment segment, int channelId = Channels.DefaultReliable) { diff --git a/Assets/Mirror/Runtime/NetworkClient.cs b/Assets/Mirror/Runtime/NetworkClient.cs index a58fcc469..eff3bdb45 100644 --- a/Assets/Mirror/Runtime/NetworkClient.cs +++ b/Assets/Mirror/Runtime/NetworkClient.cs @@ -91,7 +91,7 @@ public static void Connect(string address) Transport.activeTransport.ClientConnect(address); // setup all the handlers - connection = new NetworkConnection(address, 0); + connection = new NetworkConnectionToServer(); connection.SetHandlers(handlers); } diff --git a/Assets/Mirror/Runtime/NetworkConnection.cs b/Assets/Mirror/Runtime/NetworkConnection.cs index bce7453dd..cc0dad803 100644 --- a/Assets/Mirror/Runtime/NetworkConnection.cs +++ b/Assets/Mirror/Runtime/NetworkConnection.cs @@ -16,7 +16,7 @@ namespace Mirror /// NetworkConnection objects can "own" networked game objects. Owned objects will be destroyed on the server by default when the connection is destroyed. A connection owns the player objects created by its client, and other objects with client-authority assigned to the corresponding client. /// There are many virtual functions on NetworkConnection that allow its behaviour to be customized. NetworkClient and NetworkServer can both be made to instantiate custom classes derived from NetworkConnection by setting their networkConnectionClass member variable. /// - public class NetworkConnection : IDisposable + public abstract class NetworkConnection : IDisposable { public readonly HashSet visList = new HashSet(); @@ -54,7 +54,7 @@ public class NetworkConnection : IDisposable /// The IP address / URL / FQDN associated with the connection. /// Can be useful for a game master to do IP Bans etc. /// - public string address; + public abstract string address { get; } /// /// The last time that a message was received on this connection. @@ -114,9 +114,8 @@ internal set /// Creates a new NetworkConnection with the specified address /// /// - internal NetworkConnection(string networkAddress) + internal NetworkConnection() { - address = networkAddress; } /// @@ -124,9 +123,8 @@ internal NetworkConnection(string networkAddress) /// /// /// - internal NetworkConnection(string networkAddress, int networkConnectionId) + internal NetworkConnection(int networkConnectionId) { - address = networkAddress; connectionId = networkConnectionId; #pragma warning disable 618 isConnected = true; @@ -166,30 +164,7 @@ protected virtual void Dispose(bool disposing) /// /// Disconnects this connection. /// - public void Disconnect() - { - // don't clear address so we can still access it in NetworkManager.OnServerDisconnect - // => it's reset in Initialize anyway and there is no address empty check anywhere either - //address = ""; - - // set not ready and handle clientscene disconnect in any case - // (might be client or host mode here) - isReady = false; - ClientScene.HandleClientDisconnect(this); - - // server? then disconnect that client (not for host local player though) - if (Transport.activeTransport.ServerActive() && connectionId != 0) - { - Transport.activeTransport.ServerDisconnect(connectionId); - } - // not server and not host mode? then disconnect client - else - { - Transport.activeTransport.ClientDisconnect(); - } - - RemoveObservers(); - } + public abstract void Disconnect(); internal void SetHandlers(Dictionary handlers) { @@ -255,7 +230,7 @@ public bool Send(T msg, int channelId = Channels.DefaultReliable) where T : I // would check max size and show errors internally. best to do it // in one place in hlapi. // => it's important to log errors, so the user knows what went wrong. - static bool ValidatePacketSize(ArraySegment segment, int channelId) + protected internal static bool ValidatePacketSize(ArraySegment segment, int channelId) { if (segment.Count > Transport.activeTransport.GetMaxPacketSize(channelId)) { @@ -277,42 +252,7 @@ static bool ValidatePacketSize(ArraySegment segment, int channelId) // internal because no one except Mirror should send bytes directly to // the client. they would be detected as a message. send messages instead. List singleConnectionId = new List{-1}; - internal virtual bool Send(ArraySegment segment, int channelId = Channels.DefaultReliable) - { - if (logNetworkMessages) Debug.Log("ConnectionSend " + this + " bytes:" + BitConverter.ToString(segment.Array, segment.Offset, segment.Count)); - - // validate packet size first. - if (ValidatePacketSize(segment, channelId)) - { - // send to client or server - if (Transport.activeTransport.ClientConnected()) - { - return Transport.activeTransport.ClientSend(channelId, segment); - } - else if (Transport.activeTransport.ServerActive()) - { - singleConnectionId[0] = connectionId; - return Transport.activeTransport.ServerSend(singleConnectionId, channelId, segment); - } - } - return false; - } - - // Send to many. basically Transport.Send(connections) + checks. - internal static bool Send(List connectionIds, ArraySegment segment, int channelId = Channels.DefaultReliable) - { - // validate packet size first. - if (ValidatePacketSize(segment, channelId)) - { - // only the server sends to many, we don't have that function on - // a client. - if (Transport.activeTransport.ServerActive()) - { - return Transport.activeTransport.ServerSend(connectionIds, channelId, segment); - } - } - return false; - } + internal abstract bool Send(ArraySegment segment, int channelId = Channels.DefaultReliable); public override string ToString() { diff --git a/Assets/Mirror/Runtime/NetworkConnectionToClient.cs b/Assets/Mirror/Runtime/NetworkConnectionToClient.cs new file mode 100644 index 000000000..c3061dc1b --- /dev/null +++ b/Assets/Mirror/Runtime/NetworkConnectionToClient.cs @@ -0,0 +1,61 @@ +using System; +using System.Collections; +using System.Collections.Generic; +using UnityEngine; + +namespace Mirror +{ + public class NetworkConnectionToClient : NetworkConnection + { + public NetworkConnectionToClient(int networkConnectionId) : base(networkConnectionId) + { + } + + public override string address => Transport.activeTransport.ServerGetClientAddress(connectionId); + + // internal because no one except Mirror should send bytes directly to + // the client. they would be detected as a message. send messages instead. + List singleConnectionId = new List { -1 }; + + internal override bool Send(ArraySegment segment, int channelId = Channels.DefaultReliable) + { + if (logNetworkMessages) Debug.Log("ConnectionSend " + this + " bytes:" + BitConverter.ToString(segment.Array, segment.Offset, segment.Count)); + + // validate packet size first. + if (ValidatePacketSize(segment, channelId)) + { + singleConnectionId[0] = connectionId; + return Transport.activeTransport.ServerSend(singleConnectionId, channelId, segment); + } + return false; + } + + // Send to many. basically Transport.Send(connections) + checks. + internal static bool Send(List connectionIds, ArraySegment segment, int channelId = Channels.DefaultReliable) + { + // validate packet size first. + if (ValidatePacketSize(segment, channelId)) + { + // only the server sends to many, we don't have that function on + // a client. + if (Transport.activeTransport.ServerActive()) + { + return Transport.activeTransport.ServerSend(connectionIds, channelId, segment); + } + } + return false; + } + + /// + /// Disconnects this connection. + /// + public override void Disconnect() + { + // set not ready and handle clientscene disconnect in any case + // (might be client or host mode here) + isReady = false; + Transport.activeTransport.ServerDisconnect(connectionId); + RemoveObservers(); + } + } +} diff --git a/Assets/Mirror/Runtime/NetworkConnectionToClient.cs.meta b/Assets/Mirror/Runtime/NetworkConnectionToClient.cs.meta new file mode 100644 index 000000000..75bb7022f --- /dev/null +++ b/Assets/Mirror/Runtime/NetworkConnectionToClient.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: bb2195f8b29d24f0680a57fde2e9fd09 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Mirror/Runtime/NetworkConnectionToServer.cs b/Assets/Mirror/Runtime/NetworkConnectionToServer.cs new file mode 100644 index 000000000..5b7514cb1 --- /dev/null +++ b/Assets/Mirror/Runtime/NetworkConnectionToServer.cs @@ -0,0 +1,36 @@ +using System; +using System.Collections; +using System.Collections.Generic; +using UnityEngine; + +namespace Mirror +{ + public class NetworkConnectionToServer : NetworkConnection + { + public override string address => throw new NotImplementedException(); + + internal override bool Send(ArraySegment segment, int channelId = Channels.DefaultReliable) + { + if (logNetworkMessages) Debug.Log("ConnectionSend " + this + " bytes:" + BitConverter.ToString(segment.Array, segment.Offset, segment.Count)); + + // validate packet size first. + if (ValidatePacketSize(segment, channelId)) + { + return Transport.activeTransport.ClientSend(channelId, segment); + } + return false; + } + + /// + /// Disconnects this connection. + /// + public override void Disconnect() + { + // set not ready and handle clientscene disconnect in any case + // (might be client or host mode here) + isReady = false; + ClientScene.HandleClientDisconnect(this); + Transport.activeTransport.ClientDisconnect(); + } + } +} diff --git a/Assets/Mirror/Runtime/NetworkConnectionToServer.cs.meta b/Assets/Mirror/Runtime/NetworkConnectionToServer.cs.meta new file mode 100644 index 000000000..7e4a710b0 --- /dev/null +++ b/Assets/Mirror/Runtime/NetworkConnectionToServer.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 761977cbf38a34ded9dd89de45445675 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Mirror/Runtime/NetworkServer.cs b/Assets/Mirror/Runtime/NetworkServer.cs index 15e4eb741..850399944 100644 --- a/Assets/Mirror/Runtime/NetworkServer.cs +++ b/Assets/Mirror/Runtime/NetworkServer.cs @@ -23,12 +23,12 @@ public static class NetworkServer // original HLAPI has .localConnections list with only m_LocalConnection in it // (for backwards compatibility because they removed the real localConnections list a while ago) // => removed it for easier code. use .localConnection now! - public static NetworkConnection localConnection { get; private set; } + public static NetworkConnectionToClient localConnection { get; private set; } /// /// A list of local connections on the server. /// - public static Dictionary connections = new Dictionary(); + public static Dictionary connections = new Dictionary(); /// /// Dictionary of the message handlers registered with the server. @@ -148,7 +148,7 @@ public static bool Listen(int maxConns) /// /// Network connection to add. /// True if added. - public static bool AddConnection(NetworkConnection conn) + public static bool AddConnection(NetworkConnectionToClient conn) { if (!connections.ContainsKey(conn.connectionId)) { @@ -247,7 +247,7 @@ static bool SendToObservers(NetworkIdentity identity, T msg) where T: IMessag // send to all internet connections at once if (connectionIdsCache.Count > 0) - result &= NetworkConnection.Send(connectionIdsCache, segment); + result &= NetworkConnectionToClient.Send(connectionIdsCache, segment); NetworkDiagnostics.OnSend(msg, Channels.DefaultReliable, segment.Count, identity.observers.Count); // recycle writer and return @@ -270,7 +270,7 @@ public static bool SendToAll(int msgType, MessageBase msg, int channelId = Chann // send to all bool result = true; - foreach (KeyValuePair kvp in connections) + foreach (KeyValuePair kvp in connections) { result &= kvp.Value.Send(new ArraySegment(bytes), channelId); } @@ -302,7 +302,7 @@ public static bool SendToAll(T msg, int channelId = Channels.DefaultReliable) // avoid allocations, allow for multicast, etc. connectionIdsCache.Clear(); bool result = true; - foreach (KeyValuePair kvp in connections) + foreach (KeyValuePair kvp in connections) { // use local connection directly because it doesn't send via transport if (kvp.Value is ULocalConnectionToClient) @@ -314,7 +314,7 @@ public static bool SendToAll(T msg, int channelId = Channels.DefaultReliable) // send to all internet connections at once if (connectionIdsCache.Count > 0) - result &= NetworkConnection.Send(connectionIdsCache, segment); + result &= NetworkConnectionToClient.Send(connectionIdsCache, segment); NetworkDiagnostics.OnSend(msg, channelId, segment.Count, connections.Count); // recycle writer and return @@ -396,7 +396,7 @@ public static bool SendToReady(NetworkIdentity identity, T msg, bool includeO // send to all internet connections at once if (connectionIdsCache.Count > 0) - result &= NetworkConnection.Send(connectionIdsCache, segment); + result &= NetworkConnectionToClient.Send(connectionIdsCache, segment); NetworkDiagnostics.OnSend(msg, channelId, segment.Count, count); // recycle writer and return @@ -499,11 +499,8 @@ static void OnConnected(int connectionId) // Transport can't do that) if (connections.Count < maxConnections) { - // get ip address from connection - string address = Transport.activeTransport.ServerGetClientAddress(connectionId); - // add connection - NetworkConnection conn = new NetworkConnection(address, connectionId); + NetworkConnectionToClient conn = new NetworkConnectionToClient(connectionId); OnConnected(conn); } else @@ -514,7 +511,7 @@ static void OnConnected(int connectionId) } } - static void OnConnected(NetworkConnection conn) + static void OnConnected(NetworkConnectionToClient conn) { if (LogFilter.Debug) Debug.Log("Server accepted client:" + conn); @@ -527,7 +524,7 @@ static void OnDisconnected(int connectionId) { if (LogFilter.Debug) Debug.Log("Server disconnect client:" + connectionId); - if (connections.TryGetValue(connectionId, out NetworkConnection conn)) + if (connections.TryGetValue(connectionId, out NetworkConnectionToClient conn)) { conn.Disconnect(); RemoveConnection(connectionId); @@ -545,7 +542,7 @@ static void OnDisconnected(NetworkConnection conn) static void OnDataReceived(int connectionId, ArraySegment data, int channelId) { - if (connections.TryGetValue(connectionId, out NetworkConnection conn)) + if (connections.TryGetValue(connectionId, out NetworkConnectionToClient conn)) { conn.TransportReceive(data, channelId); } @@ -642,7 +639,7 @@ public static void ClearHandlers() [EditorBrowsable(EditorBrowsableState.Never), Obsolete("Use connection.Send(msg) instead.")] public static void SendToClient(int connectionId, int msgType, MessageBase msg) { - if (connections.TryGetValue(connectionId, out NetworkConnection conn)) + if (connections.TryGetValue(connectionId, out NetworkConnectionToClient conn)) { conn.Send(msgType, msg); return; @@ -660,7 +657,7 @@ public static void SendToClient(int connectionId, int msgType, MessageBase msg) [Obsolete("Use connection.Send(msg) instead")] public static void SendToClient(int connectionId, T msg) where T : IMessageBase { - if (connections.TryGetValue(connectionId, out NetworkConnection conn)) + if (connections.TryGetValue(connectionId, out NetworkConnectionToClient conn)) { conn.Send(msg); return;