From 2b8e87f674a3ab8315e966ff9ae608261ac04444 Mon Sep 17 00:00:00 2001 From: puvvadar Date: Wed, 16 Jun 2021 15:46:10 -0700 Subject: [PATCH 1/9] Begin cleaning MPSysComponent --- .../Source/MultiplayerSystemComponent.cpp | 48 ++++++++++++------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp index e4256a875c..06088ec0b5 100644 --- a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp +++ b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp @@ -82,6 +82,7 @@ namespace Multiplayer AZ_CVAR(AZ::CVarFixedString, sv_gamerules, "norules", nullptr, AZ::ConsoleFunctorFlags::DontReplicate, "GameRules server works with"); AZ_CVAR(ProtocolType, sv_protocol, ProtocolType::Udp, nullptr, AZ::ConsoleFunctorFlags::DontReplicate, "This flag controls whether we use TCP or UDP for game networking"); AZ_CVAR(bool, sv_isDedicated, true, nullptr, AZ::ConsoleFunctorFlags::DontReplicate, "Whether the host command creates an independent or client hosted server"); + AZ_CVAR(bool, sv_isTransient, true, nullptr, AZ::ConsoleFunctorFlags::DontReplicate, "Whether a dedicated server shuts down if all existing connections disconnect."); AZ_CVAR(AZ::TimeMs, cl_defaultNetworkEntityActivationTimeSliceMs, AZ::TimeMs{ 0 }, nullptr, AZ::ConsoleFunctorFlags::DontReplicate, "Max Ms to use to activate entities coming from the network, 0 means instantiate everything"); AZ_CVAR(AZ::TimeMs, sv_serverSendRateMs, AZ::TimeMs{ 50 }, nullptr, AZ::ConsoleFunctorFlags::Null, "Minimum number of milliseconds between each network update"); AZ_CVAR(AZ::CVarFixedString, sv_defaultPlayerSpawnAsset, "prefabs/player.network.spawnable", nullptr, AZ::ConsoleFunctorFlags::DontReplicate, "The default spawnable to use when a new player connects"); @@ -923,33 +924,43 @@ namespace Multiplayer } AZ_CONSOLEFREEFUNC(host, AZ::ConsoleFunctorFlags::DontReplicate, "Opens a multiplayer connection as a host for other clients to connect to"); + void stophost([[maybe_unused]] const AZ::ConsoleCommandContainer& arguments) + { + AZ::Interface::Get()->InitializeMultiplayer(MultiplayerAgentType::Uninitialized); + INetworkInterface* networkInterface = AZ::Interface::Get()->RetrieveNetworkInterface(AZ::Name(MPNetworkInterfaceName)); + networkInterface->StopListening(); + } + AZ_CONSOLEFREEFUNC(stophost, AZ::ConsoleFunctorFlags::DontReplicate, "Closes a multiplayer connection as a host for other clients to connect to"); + void connect([[maybe_unused]] const AZ::ConsoleCommandContainer& arguments) { - AZ::Interface::Get()->InitializeMultiplayer(MultiplayerAgentType::Client); - INetworkInterface* networkInterface = AZ::Interface::Get()->RetrieveNetworkInterface(AZ::Name(MPNetworkInterfaceName)); + AzFramework::SessionConnectionConfig config; if (arguments.size() < 1) { const AZ::CVarFixedString remoteAddress = cl_serveraddr; - const IpAddress ipAddress(remoteAddress.c_str(), cl_serverport, networkInterface->GetType()); - networkInterface->Connect(ipAddress); - return; + config.m_ipAddress = remoteAddress; + config.m_port = cl_serverport; } - - AZ::CVarFixedString remoteAddress{ arguments.front() }; - const AZStd::size_t portSeparator = remoteAddress.find_first_of(':'); - if (portSeparator == AZStd::string::npos) + else { - AZLOG_INFO("Remote address %s was malformed", remoteAddress.c_str()); - return; + AZ::CVarFixedString remoteAddress{ arguments.front() }; + const AZStd::size_t portSeparator = remoteAddress.find_first_of(':'); + if (portSeparator == AZStd::string::npos) + { + AZLOG_INFO("Remote address %s was malformed", remoteAddress.c_str()); + return; + } + char* mutableAddress = remoteAddress.data(); + mutableAddress[portSeparator] = '\0'; + const char* addressStr = mutableAddress; + const char* portStr = &(mutableAddress[portSeparator + 1]); + int32_t portNumber = atol(portStr); + config.m_ipAddress = addressStr; + config.m_port = portNumber; } - char* mutableAddress = remoteAddress.data(); - mutableAddress[portSeparator] = '\0'; - const char* addressStr = mutableAddress; - const char* portStr = &(mutableAddress[portSeparator + 1]); - int32_t portNumber = atol(portStr); - const IpAddress ipAddress(addressStr, aznumeric_cast(portNumber), networkInterface->GetType()); - networkInterface->Connect(ipAddress); + + AZ::Interface::Get()->RequestPlayerJoinSession(config); } AZ_CONSOLEFREEFUNC(connect, AZ::ConsoleFunctorFlags::DontReplicate, "Opens a multiplayer connection to a remote host"); @@ -959,6 +970,7 @@ namespace Multiplayer INetworkInterface* networkInterface = AZ::Interface::Get()->RetrieveNetworkInterface(AZ::Name(MPNetworkInterfaceName)); auto visitor = [](IConnection& connection) { connection.Disconnect(DisconnectReason::TerminatedByUser, TerminationEndpoint::Local); }; networkInterface->GetConnectionSet().VisitConnections(visitor); + networkInterface->StopListening(); } AZ_CONSOLEFREEFUNC(disconnect, AZ::ConsoleFunctorFlags::DontReplicate, "Disconnects any open multiplayer connections"); } From a748c2e152642ec7a68bded2659e5f2bfd6c518d Mon Sep 17 00:00:00 2001 From: puvvadar Date: Thu, 17 Jun 2021 16:17:36 -0700 Subject: [PATCH 2/9] Cleanup and consolidate MPSystemComponent and add StopListening to AzNetworking --- .../Framework/INetworkInterface.h | 5 + .../TcpTransport/TcpNetworkInterface.cpp | 6 + .../TcpTransport/TcpNetworkInterface.h | 1 + .../UdpTransport/UdpNetworkInterface.cpp | 15 +++ .../UdpTransport/UdpNetworkInterface.h | 1 + .../Code/Include/Multiplayer/IMultiplayer.h | 14 +++ .../Editor/MultiplayerEditorConnection.cpp | 9 +- .../Source/MultiplayerSystemComponent.cpp | 107 +++++++++--------- .../Code/Source/MultiplayerSystemComponent.h | 4 + 9 files changed, 103 insertions(+), 59 deletions(-) diff --git a/Code/Framework/AzNetworking/AzNetworking/Framework/INetworkInterface.h b/Code/Framework/AzNetworking/AzNetworking/Framework/INetworkInterface.h index e674640053..d16fda631f 100644 --- a/Code/Framework/AzNetworking/AzNetworking/Framework/INetworkInterface.h +++ b/Code/Framework/AzNetworking/AzNetworking/Framework/INetworkInterface.h @@ -88,6 +88,11 @@ namespace AzNetworking //! @return boolean true if the packet is confirmed acknowledged, false if the packet number is out of range, lost, or still pending acknowledgment virtual bool WasPacketAcked(ConnectionId connectionId, PacketId packetId) = 0; + //! Closes the network interface to stop accepting new incoming connections. + //! @param port the listen port number this network interface will potentially bind to, 0 if it's a don't care + //! @return boolean true if the operation was successful, false if it failed + virtual bool StopListening() = 0; + //! Disconnects the specified connection. //! @param connectionId identifier of the connection to terminate //! @param reason reason for the disconnect diff --git a/Code/Framework/AzNetworking/AzNetworking/TcpTransport/TcpNetworkInterface.cpp b/Code/Framework/AzNetworking/AzNetworking/TcpTransport/TcpNetworkInterface.cpp index 4c643fb698..ef3b9908a1 100644 --- a/Code/Framework/AzNetworking/AzNetworking/TcpTransport/TcpNetworkInterface.cpp +++ b/Code/Framework/AzNetworking/AzNetworking/TcpTransport/TcpNetworkInterface.cpp @@ -162,6 +162,12 @@ namespace AzNetworking return connection->WasPacketAcked(packetId); } + bool TcpNetworkInterface::StopListening() + { + m_port = 0; + return m_listenThread.StopListening(*this); + } + bool TcpNetworkInterface::Disconnect(ConnectionId connectionId, DisconnectReason reason) { IConnection* connection = m_connectionSet.GetConnection(connectionId); diff --git a/Code/Framework/AzNetworking/AzNetworking/TcpTransport/TcpNetworkInterface.h b/Code/Framework/AzNetworking/AzNetworking/TcpTransport/TcpNetworkInterface.h index f2d65eeb63..9ec687edc4 100644 --- a/Code/Framework/AzNetworking/AzNetworking/TcpTransport/TcpNetworkInterface.h +++ b/Code/Framework/AzNetworking/AzNetworking/TcpTransport/TcpNetworkInterface.h @@ -63,6 +63,7 @@ namespace AzNetworking bool SendReliablePacket(ConnectionId connectionId, const IPacket& packet) override; PacketId SendUnreliablePacket(ConnectionId connectionId, const IPacket& packet) override; bool WasPacketAcked(ConnectionId connectionId, PacketId packetId) override; + bool StopListening() override; bool Disconnect(ConnectionId connectionId, DisconnectReason reason) override; //! @} diff --git a/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpNetworkInterface.cpp b/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpNetworkInterface.cpp index 5676d48150..f2b5d667ad 100644 --- a/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpNetworkInterface.cpp +++ b/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpNetworkInterface.cpp @@ -359,6 +359,21 @@ namespace AzNetworking return connection->WasPacketAcked(packetId); } + bool UdpNetworkInterface::StopListening() + { + if (!m_socket->IsOpen()) + { + AZ_Assert(false, "StopListen cannot be invoked on an already closed network interface"); + return false; + } + + m_port = 0; + m_readerThread.UnregisterSocket(m_socket.get()); + m_allowIncomingConnections = false; + m_socket->Close(); + return true; + } + bool UdpNetworkInterface::Disconnect(ConnectionId connectionId, DisconnectReason reason) { IConnection* connection = m_connectionSet.GetConnection(connectionId); diff --git a/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpNetworkInterface.h b/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpNetworkInterface.h index dda15c421a..c2bc7da535 100644 --- a/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpNetworkInterface.h +++ b/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpNetworkInterface.h @@ -60,6 +60,7 @@ namespace AzNetworking bool SendReliablePacket(ConnectionId connectionId, const IPacket& packet) override; PacketId SendUnreliablePacket(ConnectionId connectionId, const IPacket& packet) override; bool WasPacketAcked(ConnectionId connectionId, PacketId packetId) override; + bool StopListening() override; bool Disconnect(ConnectionId connectionId, DisconnectReason reason) override; //! @} diff --git a/Gems/Multiplayer/Code/Include/Multiplayer/IMultiplayer.h b/Gems/Multiplayer/Code/Include/Multiplayer/IMultiplayer.h index 44472422c7..746c98d260 100644 --- a/Gems/Multiplayer/Code/Include/Multiplayer/IMultiplayer.h +++ b/Gems/Multiplayer/Code/Include/Multiplayer/IMultiplayer.h @@ -23,6 +23,7 @@ namespace AzNetworking { class INetworkInterface; + class IpAddress; } namespace Multiplayer @@ -66,6 +67,19 @@ namespace Multiplayer //! @param state The state of this connection virtual void InitializeMultiplayer(MultiplayerAgentType state) = 0; + //! Starts hosting a server + //! @param port The port to listen for connection on + //! @param isDedicated Whether the server is dedicated or client hosted + virtual void StartHost(uint16_t port, bool isDedicated = true) = 0; + + //! Connects to the specified IP as a Client + //! @param remoteAddress The domain or IP to connect to + //! @param port The port to connect to + virtual void Connect(AZStd::string remoteAddress, uint16_t port) = 0; + + // Disconnects all multiplayer connections, stops listening on the server and invokes handlers appropriate to network context + virtual void Terminate() = 0; + //! Adds a ClientDisconnectedEvent Handler which is invoked on the client when a disconnection occurs //! @param handler The ClientDisconnectedEvent Handler to add virtual void AddClientDisconnectedHandler(ClientDisconnectedEvent::Handler& handler) = 0; diff --git a/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorConnection.cpp b/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorConnection.cpp index 847d1caadf..3f7ac2712f 100644 --- a/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorConnection.cpp +++ b/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorConnection.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -143,13 +144,7 @@ namespace Multiplayer console->GetCvarValue("sv_port", remotePort) != AZ::GetValueResult::ConsoleVarNotFound) { // Connect the Editor to the editor server for Multiplayer simulation - AZ::Interface::Get()->InitializeMultiplayer(MultiplayerAgentType::Client); - INetworkInterface* networkInterface = - AZ::Interface::Get()->RetrieveNetworkInterface(AZ::Name(MPNetworkInterfaceName)); - - const IpAddress ipAddress(remoteAddress.c_str(), remotePort, networkInterface->GetType()); - networkInterface->Connect(ipAddress); - + AZ::Interface::Get()->Connect(remoteAddress.c_str(), remotePort); AZ::Interface::Get()->SendReadyForEntityUpdates(true); } } diff --git a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp index cd980456f3..56b30cd34f 100644 --- a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp +++ b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp @@ -201,14 +201,43 @@ namespace Multiplayer AZ::TickBus::Handler::BusDisconnect(); } + void MultiplayerSystemComponent::StartHost(uint16_t port, bool isDedicated) + { + InitializeMultiplayer(isDedicated ? MultiplayerAgentType::DedicatedServer : MultiplayerAgentType::ClientServer); + INetworkInterface* networkInterface = AZ::Interface::Get()->RetrieveNetworkInterface(AZ::Name(MPNetworkInterfaceName)); + networkInterface->Listen(port); + } + + void MultiplayerSystemComponent::Connect(AZStd::string remoteAddress, uint16_t port) + { + InitializeMultiplayer(MultiplayerAgentType::Client); + const IpAddress address(remoteAddress.c_str(), port, m_networkInterface->GetType()); + m_networkInterface->Connect(address); + } + + void MultiplayerSystemComponent::Terminate() + { + INetworkInterface* networkInterface = AZ::Interface::Get()->RetrieveNetworkInterface(AZ::Name(MPNetworkInterfaceName)); + + auto visitor = [](IConnection& connection) { connection.Disconnect(DisconnectReason::TerminatedByUser, TerminationEndpoint::Local); }; + networkInterface->GetConnectionSet().VisitConnections(visitor); + if (GetAgentType() == MultiplayerAgentType::DedicatedServer || GetAgentType() == MultiplayerAgentType::ClientServer) + { + networkInterface->StopListening(); + m_shutdownEvent.Signal(m_networkInterface); + if (AZ::Interface::Get() != nullptr) + { + AZ::Interface::Get()->HandleDestroySession(); + } + } + InitializeMultiplayer(MultiplayerAgentType::Uninitialized); + } + bool MultiplayerSystemComponent::RequestPlayerJoinSession(const AzFramework::SessionConnectionConfig& config) { - AZ::Interface::Get()->InitializeMultiplayer(MultiplayerAgentType::Client); - m_pendingConnectionTickets.push(config.m_playerSessionId); AZStd::string hostname = config.m_dnsName.empty() ? config.m_ipAddress : config.m_dnsName; - const IpAddress ipAddress(hostname.c_str(), config.m_port, m_networkInterface->GetType()); - m_networkInterface->Connect(ipAddress); + Connect(hostname.c_str(), config.m_port); return true; } @@ -217,12 +246,7 @@ namespace Multiplayer { if (GetAgentType() == MultiplayerAgentType::Client) { - AZ::Interface::Get()->InitializeMultiplayer(MultiplayerAgentType::Uninitialized); - auto visitor = [](IConnection& connection) - { - connection.Disconnect(DisconnectReason::TerminatedByUser, TerminationEndpoint::Local); - }; - m_networkInterface->GetConnectionSet().VisitConnections(visitor); + Terminate(); } } @@ -254,7 +278,7 @@ namespace Multiplayer } Multiplayer::MultiplayerAgentType serverType = sv_isDedicated ? MultiplayerAgentType::DedicatedServer : MultiplayerAgentType::ClientServer; - AZ::Interface::Get()->InitializeMultiplayer(serverType); + InitializeMultiplayer(serverType); return m_networkInterface->Listen(sessionConfig.m_port); } @@ -422,10 +446,9 @@ namespace Multiplayer auto visitor = [](IConnection& connection) { connection.Disconnect(DisconnectReason::TerminatedByUser, TerminationEndpoint::Local); }; m_networkInterface->GetConnectionSet().VisitConnections(visitor); return true; - } - - reinterpret_cast(connection->GetUserData())->SetProviderTicket(packet.GetTicket().c_str()); + } } + reinterpret_cast(connection->GetUserData())->SetProviderTicket(packet.GetTicket().c_str()); if (connection->SendReliablePacket(MultiplayerPackets::Accept(InvalidHostId, sv_map))) { @@ -655,17 +678,10 @@ namespace Multiplayer m_clientDisconnectedEvent.Signal(); } - // Clean up any multiplayer connection data we've bound to this connection instance - if (connection->GetUserData() != nullptr) - { - IConnectionData* connectionData = reinterpret_cast(connection->GetUserData()); - delete connectionData; - connection->SetUserData(nullptr); - } - // Signal to session management that a user has left the server if (m_agentType == MultiplayerAgentType::DedicatedServer || m_agentType == MultiplayerAgentType::ClientServer) { + if (AZ::Interface::Get() != nullptr && connection->GetConnectionRole() == ConnectionRole::Acceptor) { @@ -676,17 +692,21 @@ namespace Multiplayer } } + // Clean up any multiplayer connection data we've bound to this connection instance + if (connection->GetUserData() != nullptr) + { + IConnectionData* connectionData = reinterpret_cast(connection->GetUserData()); + delete connectionData; + connection->SetUserData(nullptr); + } + // Signal to session management when there are no remaining players in a dedicated server for potential cleanup - // We avoid this for client server as the host itself is a user - if (m_agentType == MultiplayerAgentType::DedicatedServer && connection->GetConnectionRole() == ConnectionRole::Acceptor) + // We avoid this for client server as the host itself is a user and non-transient dedicated servers + if (sv_isTransient && m_agentType == MultiplayerAgentType::DedicatedServer && connection->GetConnectionRole() == ConnectionRole::Acceptor) { if (m_networkInterface->GetConnectionSet().GetConnectionCount() == 0) { - m_shutdownEvent.Signal(m_networkInterface); - if (AZ::Interface::Get() != nullptr) - { - AZ::Interface::Get()->HandleDestroySession(); - } + Terminate(); } } } @@ -917,30 +937,18 @@ namespace Multiplayer void host([[maybe_unused]] const AZ::ConsoleCommandContainer& arguments) { - Multiplayer::MultiplayerAgentType serverType = sv_isDedicated ? MultiplayerAgentType::DedicatedServer : MultiplayerAgentType::ClientServer; - AZ::Interface::Get()->InitializeMultiplayer(serverType); - INetworkInterface* networkInterface = AZ::Interface::Get()->RetrieveNetworkInterface(AZ::Name(MPNetworkInterfaceName)); - networkInterface->Listen(sv_port); + AZ::Interface::Get()->StartHost(sv_port, sv_isDedicated); } AZ_CONSOLEFREEFUNC(host, AZ::ConsoleFunctorFlags::DontReplicate, "Opens a multiplayer connection as a host for other clients to connect to"); - void stophost([[maybe_unused]] const AZ::ConsoleCommandContainer& arguments) - { - AZ::Interface::Get()->InitializeMultiplayer(MultiplayerAgentType::Uninitialized); - INetworkInterface* networkInterface = AZ::Interface::Get()->RetrieveNetworkInterface(AZ::Name(MPNetworkInterfaceName)); - networkInterface->StopListening(); - } - AZ_CONSOLEFREEFUNC(stophost, AZ::ConsoleFunctorFlags::DontReplicate, "Closes a multiplayer connection as a host for other clients to connect to"); - void connect([[maybe_unused]] const AZ::ConsoleCommandContainer& arguments) { - AzFramework::SessionConnectionConfig config; + AzNetworking::IpAddress address; if (arguments.size() < 1) { const AZ::CVarFixedString remoteAddress = cl_serveraddr; - config.m_ipAddress = remoteAddress; - config.m_port = cl_serverport; + AZ::Interface::Get()->Connect(remoteAddress.c_str(), cl_serverport); } else { @@ -956,21 +964,16 @@ namespace Multiplayer const char* addressStr = mutableAddress; const char* portStr = &(mutableAddress[portSeparator + 1]); int32_t portNumber = atol(portStr); - config.m_ipAddress = addressStr; - config.m_port = portNumber; + AZ::Interface::Get()->Connect(addressStr, portNumber); } - AZ::Interface::Get()->RequestPlayerJoinSession(config); + } AZ_CONSOLEFREEFUNC(connect, AZ::ConsoleFunctorFlags::DontReplicate, "Opens a multiplayer connection to a remote host"); void disconnect([[maybe_unused]] const AZ::ConsoleCommandContainer& arguments) { - AZ::Interface::Get()->InitializeMultiplayer(MultiplayerAgentType::Uninitialized); - INetworkInterface* networkInterface = AZ::Interface::Get()->RetrieveNetworkInterface(AZ::Name(MPNetworkInterfaceName)); - auto visitor = [](IConnection& connection) { connection.Disconnect(DisconnectReason::TerminatedByUser, TerminationEndpoint::Local); }; - networkInterface->GetConnectionSet().VisitConnections(visitor); - networkInterface->StopListening(); + AZ::Interface::Get()->Terminate(); } AZ_CONSOLEFREEFUNC(disconnect, AZ::ConsoleFunctorFlags::DontReplicate, "Disconnects any open multiplayer connections"); } diff --git a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h index 00313c6b65..56cb347a51 100644 --- a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h +++ b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h @@ -37,6 +37,7 @@ namespace AzFramework namespace AzNetworking { class INetworkInterface; + class IpAddress; } namespace Multiplayer @@ -112,6 +113,9 @@ namespace Multiplayer void AddConnectionAcquiredHandler(ConnectionAcquiredEvent::Handler& handler) override; void AddSessionInitHandler(SessionInitEvent::Handler& handler) override; void AddSessionShutdownHandler(SessionShutdownEvent::Handler& handler) override; + void StartHost(uint16_t port, bool isDedicated = true) override; + void Connect(AZStd::string remoteAddress, uint16_t port) override; + void Terminate() override; void SendReadyForEntityUpdates(bool readyForEntityUpdates) override; AZ::TimeMs GetCurrentHostTimeMs() const override; INetworkTime* GetNetworkTime() override; From 04387be083471a4b9ce77c7dcb1806a657977500 Mon Sep 17 00:00:00 2001 From: puvvadar Date: Thu, 17 Jun 2021 16:25:44 -0700 Subject: [PATCH 3/9] Remove some redundant includes, forward declares and similar pieces --- .../AzNetworking/Framework/INetworkInterface.h | 1 - .../Code/Include/Multiplayer/IMultiplayer.h | 3 +-- .../Editor/MultiplayerEditorConnection.cpp | 1 - .../Code/Source/MultiplayerSystemComponent.cpp | 18 +++++------------- .../Code/Source/MultiplayerSystemComponent.h | 2 +- 5 files changed, 7 insertions(+), 18 deletions(-) diff --git a/Code/Framework/AzNetworking/AzNetworking/Framework/INetworkInterface.h b/Code/Framework/AzNetworking/AzNetworking/Framework/INetworkInterface.h index d16fda631f..c66f216eaa 100644 --- a/Code/Framework/AzNetworking/AzNetworking/Framework/INetworkInterface.h +++ b/Code/Framework/AzNetworking/AzNetworking/Framework/INetworkInterface.h @@ -89,7 +89,6 @@ namespace AzNetworking virtual bool WasPacketAcked(ConnectionId connectionId, PacketId packetId) = 0; //! Closes the network interface to stop accepting new incoming connections. - //! @param port the listen port number this network interface will potentially bind to, 0 if it's a don't care //! @return boolean true if the operation was successful, false if it failed virtual bool StopListening() = 0; diff --git a/Gems/Multiplayer/Code/Include/Multiplayer/IMultiplayer.h b/Gems/Multiplayer/Code/Include/Multiplayer/IMultiplayer.h index 746c98d260..539288041a 100644 --- a/Gems/Multiplayer/Code/Include/Multiplayer/IMultiplayer.h +++ b/Gems/Multiplayer/Code/Include/Multiplayer/IMultiplayer.h @@ -23,7 +23,6 @@ namespace AzNetworking { class INetworkInterface; - class IpAddress; } namespace Multiplayer @@ -70,7 +69,7 @@ namespace Multiplayer //! Starts hosting a server //! @param port The port to listen for connection on //! @param isDedicated Whether the server is dedicated or client hosted - virtual void StartHost(uint16_t port, bool isDedicated = true) = 0; + virtual void StartHosting(uint16_t port, bool isDedicated = true) = 0; //! Connects to the specified IP as a Client //! @param remoteAddress The domain or IP to connect to diff --git a/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorConnection.cpp b/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorConnection.cpp index 3f7ac2712f..b05d012ac8 100644 --- a/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorConnection.cpp +++ b/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorConnection.cpp @@ -20,7 +20,6 @@ #include #include #include -#include #include #include #include diff --git a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp index 56b30cd34f..99f632fa94 100644 --- a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp +++ b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp @@ -201,11 +201,10 @@ namespace Multiplayer AZ::TickBus::Handler::BusDisconnect(); } - void MultiplayerSystemComponent::StartHost(uint16_t port, bool isDedicated) + void MultiplayerSystemComponent::StartHosting(uint16_t port, bool isDedicated) { InitializeMultiplayer(isDedicated ? MultiplayerAgentType::DedicatedServer : MultiplayerAgentType::ClientServer); - INetworkInterface* networkInterface = AZ::Interface::Get()->RetrieveNetworkInterface(AZ::Name(MPNetworkInterfaceName)); - networkInterface->Listen(port); + m_networkInterface->Listen(port); } void MultiplayerSystemComponent::Connect(AZStd::string remoteAddress, uint16_t port) @@ -217,13 +216,11 @@ namespace Multiplayer void MultiplayerSystemComponent::Terminate() { - INetworkInterface* networkInterface = AZ::Interface::Get()->RetrieveNetworkInterface(AZ::Name(MPNetworkInterfaceName)); - auto visitor = [](IConnection& connection) { connection.Disconnect(DisconnectReason::TerminatedByUser, TerminationEndpoint::Local); }; - networkInterface->GetConnectionSet().VisitConnections(visitor); + m_networkInterface->GetConnectionSet().VisitConnections(visitor); if (GetAgentType() == MultiplayerAgentType::DedicatedServer || GetAgentType() == MultiplayerAgentType::ClientServer) { - networkInterface->StopListening(); + m_networkInterface->StopListening(); m_shutdownEvent.Signal(m_networkInterface); if (AZ::Interface::Get() != nullptr) { @@ -681,7 +678,6 @@ namespace Multiplayer // Signal to session management that a user has left the server if (m_agentType == MultiplayerAgentType::DedicatedServer || m_agentType == MultiplayerAgentType::ClientServer) { - if (AZ::Interface::Get() != nullptr && connection->GetConnectionRole() == ConnectionRole::Acceptor) { @@ -937,14 +933,12 @@ namespace Multiplayer void host([[maybe_unused]] const AZ::ConsoleCommandContainer& arguments) { - AZ::Interface::Get()->StartHost(sv_port, sv_isDedicated); + AZ::Interface::Get()->StartHosting(sv_port, sv_isDedicated); } AZ_CONSOLEFREEFUNC(host, AZ::ConsoleFunctorFlags::DontReplicate, "Opens a multiplayer connection as a host for other clients to connect to"); void connect([[maybe_unused]] const AZ::ConsoleCommandContainer& arguments) { - AzNetworking::IpAddress address; - if (arguments.size() < 1) { const AZ::CVarFixedString remoteAddress = cl_serveraddr; @@ -966,8 +960,6 @@ namespace Multiplayer int32_t portNumber = atol(portStr); AZ::Interface::Get()->Connect(addressStr, portNumber); } - - } AZ_CONSOLEFREEFUNC(connect, AZ::ConsoleFunctorFlags::DontReplicate, "Opens a multiplayer connection to a remote host"); diff --git a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h index 56cb347a51..ef114404a6 100644 --- a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h +++ b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h @@ -113,7 +113,7 @@ namespace Multiplayer void AddConnectionAcquiredHandler(ConnectionAcquiredEvent::Handler& handler) override; void AddSessionInitHandler(SessionInitEvent::Handler& handler) override; void AddSessionShutdownHandler(SessionShutdownEvent::Handler& handler) override; - void StartHost(uint16_t port, bool isDedicated = true) override; + void StartHosting(uint16_t port, bool isDedicated = true) override; void Connect(AZStd::string remoteAddress, uint16_t port) override; void Terminate() override; void SendReadyForEntityUpdates(bool readyForEntityUpdates) override; From 8f5681ffdb393e38e5e35b7ad0727b7e94728404 Mon Sep 17 00:00:00 2001 From: puvvadar Date: Mon, 21 Jun 2021 11:46:16 -0700 Subject: [PATCH 4/9] Add GetActiveConnectionCount to IConnectionSet --- .../ConnectionLayer/IConnectionSet.h | 4 ++++ .../TcpTransport/TcpConnectionSet.cpp | 18 ++++++++++++++++++ .../TcpTransport/TcpConnectionSet.h | 1 + .../UdpTransport/UdpConnectionSet.cpp | 18 ++++++++++++++++++ .../UdpTransport/UdpConnectionSet.h | 1 + .../Code/Source/MultiplayerSystemComponent.cpp | 2 +- 6 files changed, 43 insertions(+), 1 deletion(-) diff --git a/Code/Framework/AzNetworking/AzNetworking/ConnectionLayer/IConnectionSet.h b/Code/Framework/AzNetworking/AzNetworking/ConnectionLayer/IConnectionSet.h index 5b53efd951..1bcddb162b 100644 --- a/Code/Framework/AzNetworking/AzNetworking/ConnectionLayer/IConnectionSet.h +++ b/Code/Framework/AzNetworking/AzNetworking/ConnectionLayer/IConnectionSet.h @@ -47,5 +47,9 @@ namespace AzNetworking //! Returns the current total connection count for this connection set //! @return the current total connection count for this connection set virtual uint32_t GetConnectionCount() const = 0; + + //! Returns the current total count of connections not pending disconnect for this connection set + //! @return the current total count of connections not pending disconnect for this connection set + virtual uint32_t GetActiveConnectionCount() const = 0; }; } diff --git a/Code/Framework/AzNetworking/AzNetworking/TcpTransport/TcpConnectionSet.cpp b/Code/Framework/AzNetworking/AzNetworking/TcpTransport/TcpConnectionSet.cpp index be18fafff9..f3a3605ffa 100644 --- a/Code/Framework/AzNetworking/AzNetworking/TcpTransport/TcpConnectionSet.cpp +++ b/Code/Framework/AzNetworking/AzNetworking/TcpTransport/TcpConnectionSet.cpp @@ -114,6 +114,24 @@ namespace AzNetworking return aznumeric_cast(m_connectionIdMap.size()); } + uint32_t TcpConnectionSet::GetActiveConnectionCount() const + { + uint32_t activeConnections = 0; + for (auto iter = m_connectionIdMap.begin(); iter != m_connectionIdMap.end(); ++iter) + { + if (iter->second.get()) + { + ConnectionState state = iter->second.get()->GetConnectionState(); + if (state == ConnectionState::Connected || state == ConnectionState::Connecting) + { + ++activeConnections; + } + } + } + + return activeConnections; + } + TcpConnection* TcpConnectionSet::GetConnection(SocketFd socketFd) const { SocketFdMap::const_iterator lookup = m_socketFdMap.find(socketFd); diff --git a/Code/Framework/AzNetworking/AzNetworking/TcpTransport/TcpConnectionSet.h b/Code/Framework/AzNetworking/AzNetworking/TcpTransport/TcpConnectionSet.h index 25d42c2481..e3f950b457 100644 --- a/Code/Framework/AzNetworking/AzNetworking/TcpTransport/TcpConnectionSet.h +++ b/Code/Framework/AzNetworking/AzNetworking/TcpTransport/TcpConnectionSet.h @@ -49,6 +49,7 @@ namespace AzNetworking IConnection* GetConnection(ConnectionId connectionId) const override; ConnectionId GetNextConnectionId() override; uint32_t GetConnectionCount() const override; + uint32_t GetActiveConnectionCount() const override; //! @} //! Retrieves a connection from this connection list instance by socket fd. diff --git a/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpConnectionSet.cpp b/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpConnectionSet.cpp index 4e3347c080..8555bed86c 100644 --- a/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpConnectionSet.cpp +++ b/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpConnectionSet.cpp @@ -111,6 +111,24 @@ namespace AzNetworking return aznumeric_cast(m_connectionIdMap.size()); } + uint32_t UdpConnectionSet::GetActiveConnectionCount() const + { + uint32_t activeConnections = 0; + for (auto iter = m_connectionIdMap.begin(); iter != m_connectionIdMap.end(); ++iter) + { + if (iter->second.get()) + { + ConnectionState state = iter->second.get()->GetConnectionState(); + if (state == ConnectionState::Connected || state == ConnectionState::Connecting) + { + ++activeConnections; + } + } + } + + return activeConnections; + } + UdpConnection* UdpConnectionSet::GetConnection(const IpAddress& address) const { RemoteAddressMap::const_iterator lookup = m_remoteAddressMap.find(address); diff --git a/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpConnectionSet.h b/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpConnectionSet.h index 82752904fd..071a9776ec 100644 --- a/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpConnectionSet.h +++ b/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpConnectionSet.h @@ -50,6 +50,7 @@ namespace AzNetworking IConnection* GetConnection(ConnectionId connectionId) const override; ConnectionId GetNextConnectionId() override; uint32_t GetConnectionCount() const override; + uint32_t GetActiveConnectionCount() const override; //! @} //! Retrieves a connection from this connection list instance by endpoint remote address diff --git a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp index 99f632fa94..2e2e820e60 100644 --- a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp +++ b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp @@ -700,7 +700,7 @@ namespace Multiplayer // We avoid this for client server as the host itself is a user and non-transient dedicated servers if (sv_isTransient && m_agentType == MultiplayerAgentType::DedicatedServer && connection->GetConnectionRole() == ConnectionRole::Acceptor) { - if (m_networkInterface->GetConnectionSet().GetConnectionCount() == 0) + if (m_networkInterface->GetConnectionSet().GetActiveConnectionCount() == 0) { Terminate(); } From 3a7df983d949cf198ad6783561a9d718aac8f219 Mon Sep 17 00:00:00 2001 From: puvvadar Date: Mon, 21 Jun 2021 14:31:14 -0700 Subject: [PATCH 5/9] Cleanup listen and connect logic to prevent log spam --- .../UdpTransport/UdpNetworkInterface.cpp | 22 ++++++++++++++----- .../Code/Include/Multiplayer/IMultiplayer.h | 6 +++-- .../Source/MultiplayerSystemComponent.cpp | 8 +++---- .../Code/Source/MultiplayerSystemComponent.h | 4 ++-- 4 files changed, 27 insertions(+), 13 deletions(-) diff --git a/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpNetworkInterface.cpp b/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpNetworkInterface.cpp index f2b5d667ad..9f1b98f81b 100644 --- a/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpNetworkInterface.cpp +++ b/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpNetworkInterface.cpp @@ -116,17 +116,29 @@ namespace AzNetworking m_port = port; m_allowIncomingConnections = true; - m_socket->Open(m_port, UdpSocket::CanAcceptConnections::True, m_trustZone); - m_readerThread.RegisterSocket(m_socket.get()); - return true; + if (m_socket->Open(m_port, UdpSocket::CanAcceptConnections::True, m_trustZone)) + { + m_readerThread.RegisterSocket(m_socket.get()); + return true; + } + else + { + return false; + } } ConnectionId UdpNetworkInterface::Connect(const IpAddress& remoteAddress) { if (!m_socket->IsOpen()) { - m_socket->Open(m_port, UdpSocket::CanAcceptConnections::False, m_trustZone); - m_readerThread.RegisterSocket(m_socket.get()); + if (m_socket->Open(m_port, UdpSocket::CanAcceptConnections::False, m_trustZone)) + { + m_readerThread.RegisterSocket(m_socket.get()); + } + else + { + return InvalidConnectionId; + } } const ConnectionId connectionId = m_connectionSet.GetNextConnectionId(); diff --git a/Gems/Multiplayer/Code/Include/Multiplayer/IMultiplayer.h b/Gems/Multiplayer/Code/Include/Multiplayer/IMultiplayer.h index 539288041a..b1579a4ac0 100644 --- a/Gems/Multiplayer/Code/Include/Multiplayer/IMultiplayer.h +++ b/Gems/Multiplayer/Code/Include/Multiplayer/IMultiplayer.h @@ -69,12 +69,14 @@ namespace Multiplayer //! Starts hosting a server //! @param port The port to listen for connection on //! @param isDedicated Whether the server is dedicated or client hosted - virtual void StartHosting(uint16_t port, bool isDedicated = true) = 0; + //! @return if the application successfully started hosting + virtual bool StartHosting(uint16_t port, bool isDedicated = true) = 0; //! Connects to the specified IP as a Client //! @param remoteAddress The domain or IP to connect to //! @param port The port to connect to - virtual void Connect(AZStd::string remoteAddress, uint16_t port) = 0; + //! @result if a connection was successfully created + virtual bool Connect(AZStd::string remoteAddress, uint16_t port) = 0; // Disconnects all multiplayer connections, stops listening on the server and invokes handlers appropriate to network context virtual void Terminate() = 0; diff --git a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp index 2e2e820e60..32d8cfa600 100644 --- a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp +++ b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp @@ -201,17 +201,17 @@ namespace Multiplayer AZ::TickBus::Handler::BusDisconnect(); } - void MultiplayerSystemComponent::StartHosting(uint16_t port, bool isDedicated) + bool MultiplayerSystemComponent::StartHosting(uint16_t port, bool isDedicated) { InitializeMultiplayer(isDedicated ? MultiplayerAgentType::DedicatedServer : MultiplayerAgentType::ClientServer); - m_networkInterface->Listen(port); + return m_networkInterface->Listen(port); } - void MultiplayerSystemComponent::Connect(AZStd::string remoteAddress, uint16_t port) + bool MultiplayerSystemComponent::Connect(AZStd::string remoteAddress, uint16_t port) { InitializeMultiplayer(MultiplayerAgentType::Client); const IpAddress address(remoteAddress.c_str(), port, m_networkInterface->GetType()); - m_networkInterface->Connect(address); + return m_networkInterface->Connect(address) != InvalidConnectionId; } void MultiplayerSystemComponent::Terminate() diff --git a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h index ef114404a6..a17723632d 100644 --- a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h +++ b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h @@ -113,8 +113,8 @@ namespace Multiplayer void AddConnectionAcquiredHandler(ConnectionAcquiredEvent::Handler& handler) override; void AddSessionInitHandler(SessionInitEvent::Handler& handler) override; void AddSessionShutdownHandler(SessionShutdownEvent::Handler& handler) override; - void StartHosting(uint16_t port, bool isDedicated = true) override; - void Connect(AZStd::string remoteAddress, uint16_t port) override; + bool StartHosting(uint16_t port, bool isDedicated = true) override; + bool Connect(AZStd::string remoteAddress, uint16_t port) override; void Terminate() override; void SendReadyForEntityUpdates(bool readyForEntityUpdates) override; AZ::TimeMs GetCurrentHostTimeMs() const override; From f49e5cf64549f45168044655237f262acbffa304 Mon Sep 17 00:00:00 2001 From: puvvadar Date: Mon, 21 Jun 2021 14:49:49 -0700 Subject: [PATCH 6/9] Validate some base cases in InitializeMultiplayer --- .../Code/Source/MultiplayerSystemComponent.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp index 32d8cfa600..c2ff0a58a6 100644 --- a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp +++ b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp @@ -714,6 +714,17 @@ namespace Multiplayer void MultiplayerSystemComponent::InitializeMultiplayer(MultiplayerAgentType multiplayerType) { + if (m_agentType == multiplayerType) + { + // No-op + return; + } + + if (m_agentType != MultiplayerAgentType::Uninitialized && multiplayerType != MultiplayerAgentType::Uninitialized) + { + AZLOG_WARN("Attemping to InitializeMultiplayer from one initialized type to another. Your session may not have been properly torn down."); + } + if (m_agentType == MultiplayerAgentType::Uninitialized) { if (multiplayerType == MultiplayerAgentType::ClientServer || multiplayerType == MultiplayerAgentType::DedicatedServer) From f2bc80d103076dde878a8a5beedc50657084cb30 Mon Sep 17 00:00:00 2001 From: puvvadar Date: Mon, 21 Jun 2021 14:53:19 -0700 Subject: [PATCH 7/9] Remove obvious comment --- Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp index c2ff0a58a6..6ad23967cb 100644 --- a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp +++ b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp @@ -716,7 +716,6 @@ namespace Multiplayer { if (m_agentType == multiplayerType) { - // No-op return; } From 4d0d97b3fe97f3433e9f3c3a98955751300a6821 Mon Sep 17 00:00:00 2001 From: puvvadar Date: Mon, 21 Jun 2021 16:52:05 -0700 Subject: [PATCH 8/9] Remove excessive assert --- .../AzNetworking/UdpTransport/UdpNetworkInterface.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpNetworkInterface.cpp b/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpNetworkInterface.cpp index 9f1b98f81b..48f75b5a69 100644 --- a/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpNetworkInterface.cpp +++ b/Code/Framework/AzNetworking/AzNetworking/UdpTransport/UdpNetworkInterface.cpp @@ -375,7 +375,6 @@ namespace AzNetworking { if (!m_socket->IsOpen()) { - AZ_Assert(false, "StopListen cannot be invoked on an already closed network interface"); return false; } From 55f5e9f0ae9027c153e1d34894c90efd27de0794 Mon Sep 17 00:00:00 2001 From: puvvadar Date: Tue, 22 Jun 2021 09:25:29 -0700 Subject: [PATCH 9/9] Remove unnecessary forward declare --- Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h | 1 - 1 file changed, 1 deletion(-) diff --git a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h index a17723632d..cc5af38f04 100644 --- a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h +++ b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h @@ -37,7 +37,6 @@ namespace AzFramework namespace AzNetworking { class INetworkInterface; - class IpAddress; } namespace Multiplayer