From 04bf6c3689391a8e02df2617cc10bce2de517ee1 Mon Sep 17 00:00:00 2001 From: puvvadar Date: Tue, 15 Jun 2021 10:14:02 -0700 Subject: [PATCH 1/5] Separate out session validation into its own packet --- .../AutoGen/Multiplayer.AutoPackets.xml | 5 +- .../Source/MultiplayerSystemComponent.cpp | 51 +++++++++++-------- .../Code/Source/MultiplayerSystemComponent.h | 1 + 3 files changed, 34 insertions(+), 23 deletions(-) diff --git a/Gems/Multiplayer/Code/Source/AutoGen/Multiplayer.AutoPackets.xml b/Gems/Multiplayer/Code/Source/AutoGen/Multiplayer.AutoPackets.xml index ce8931107f..3abd2a86d9 100644 --- a/Gems/Multiplayer/Code/Source/AutoGen/Multiplayer.AutoPackets.xml +++ b/Gems/Multiplayer/Code/Source/AutoGen/Multiplayer.AutoPackets.xml @@ -9,13 +9,16 @@ - + + + + diff --git a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp index e4256a875c..dc86a07945 100644 --- a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp +++ b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp @@ -213,6 +213,8 @@ namespace Multiplayer reinterpret_cast(connection->GetUserData())->SetProviderTicket(config.m_playerSessionId); } + connection->SendReliablePacket(MultiplayerPackets::ValidateSession(config.m_playerSessionId.c_str())); + return true; } @@ -414,22 +416,6 @@ namespace Multiplayer [[maybe_unused]] MultiplayerPackets::Connect& packet ) { - // Validate our session with the provider if any - if (AZ::Interface::Get() != nullptr) - { - AzFramework::PlayerConnectionConfig config; - config.m_playerConnectionId = aznumeric_cast(connection->GetConnectionId()); - config.m_playerSessionId = packet.GetTicket(); - if(!AZ::Interface::Get()->ValidatePlayerJoinSession(config)) - { - 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()); - } - if (connection->SendReliablePacket(MultiplayerPackets::Accept(InvalidHostId, sv_map))) { // Sync our console @@ -455,6 +441,32 @@ namespace Multiplayer return true; } + bool MultiplayerSystemComponent::HandleRequest + ( + [[maybe_unused]] AzNetworking::IConnection* connection, + [[maybe_unused]] const IPacketHeader& packetHeader, + [[maybe_unused]] MultiplayerPackets::ValidateSession& packet + ) + { + // Validate our session with the provider if any + if (AZ::Interface::Get() != nullptr) + { + AzFramework::PlayerConnectionConfig config; + config.m_playerConnectionId = aznumeric_cast(connection->GetConnectionId()); + config.m_playerSessionId = packet.GetTicket(); + if(!AZ::Interface::Get()->ValidatePlayerJoinSession(config)) + { + auto visitor = [](IConnection& connection) { connection.Disconnect(DisconnectReason::TerminatedByUser, TerminationEndpoint::Local); }; + m_networkInterface->GetConnectionSet().VisitConnections(visitor); + return false; + } + + reinterpret_cast(connection->GetUserData())->SetProviderTicket(packet.GetTicket().c_str()); + } + + return true; + } + bool MultiplayerSystemComponent::HandleRequest ( AzNetworking::IConnection* connection, @@ -587,12 +599,7 @@ namespace Multiplayer if (connection->GetConnectionRole() == ConnectionRole::Connector) { AZLOG_INFO("New outgoing connection to remote address: %s", connection->GetRemoteAddress().GetString().c_str()); - AZ::CVarFixedString providerTicket; - if (connection->GetUserData() != nullptr) - { - providerTicket = reinterpret_cast(connection->GetUserData())->GetProviderTicket(); - } - connection->SendReliablePacket(MultiplayerPackets::Connect(0, providerTicket)); + connection->SendReliablePacket(MultiplayerPackets::Connect(0)); } else { diff --git a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h index 0efef3ebe4..2a3fe73ff4 100644 --- a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h +++ b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h @@ -82,6 +82,7 @@ namespace Multiplayer bool HandleRequest(AzNetworking::IConnection* connection, const AzNetworking::IPacketHeader& packetHeader, MultiplayerPackets::Connect& packet); bool HandleRequest(AzNetworking::IConnection* connection, const AzNetworking::IPacketHeader& packetHeader, MultiplayerPackets::Accept& packet); + bool HandleRequest(AzNetworking::IConnection* connection, const AzNetworking::IPacketHeader& packetHeader, MultiplayerPackets::ValidateSession& packet); bool HandleRequest(AzNetworking::IConnection* connection, const AzNetworking::IPacketHeader& packetHeader, MultiplayerPackets::ReadyForEntityUpdates& packet); bool HandleRequest(AzNetworking::IConnection* connection, const AzNetworking::IPacketHeader& packetHeader, MultiplayerPackets::SyncConsole& packet); bool HandleRequest(AzNetworking::IConnection* connection, const AzNetworking::IPacketHeader& packetHeader, MultiplayerPackets::ConsoleCommand& packet); From adc09435f48ac2b6adfbfb6b288b9ca9d0134ff4 Mon Sep 17 00:00:00 2001 From: puvvadar Date: Tue, 15 Jun 2021 11:08:59 -0700 Subject: [PATCH 2/5] Cleanup validation failure logic --- Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp index dc86a07945..aa5e0d7467 100644 --- a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp +++ b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp @@ -456,8 +456,7 @@ namespace Multiplayer config.m_playerSessionId = packet.GetTicket(); if(!AZ::Interface::Get()->ValidatePlayerJoinSession(config)) { - auto visitor = [](IConnection& connection) { connection.Disconnect(DisconnectReason::TerminatedByUser, TerminationEndpoint::Local); }; - m_networkInterface->GetConnectionSet().VisitConnections(visitor); + connection->Disconnect(DisconnectReason::TerminatedByServer, TerminationEndpoint::Local); return false; } From 2eca923694423a6714baf0464d1b527866cbe716 Mon Sep 17 00:00:00 2001 From: puvvadar Date: Tue, 15 Jun 2021 14:34:24 -0700 Subject: [PATCH 3/5] Rework session validation to work through Connect packet --- .../AutoGen/Multiplayer.AutoPackets.xml | 5 +- .../Source/MultiplayerSystemComponent.cpp | 81 ++++++++----------- .../Code/Source/MultiplayerSystemComponent.h | 3 +- 3 files changed, 36 insertions(+), 53 deletions(-) diff --git a/Gems/Multiplayer/Code/Source/AutoGen/Multiplayer.AutoPackets.xml b/Gems/Multiplayer/Code/Source/AutoGen/Multiplayer.AutoPackets.xml index 3abd2a86d9..ce8931107f 100644 --- a/Gems/Multiplayer/Code/Source/AutoGen/Multiplayer.AutoPackets.xml +++ b/Gems/Multiplayer/Code/Source/AutoGen/Multiplayer.AutoPackets.xml @@ -9,16 +9,13 @@ + - - - - diff --git a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp index aa5e0d7467..5f44a2cccb 100644 --- a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp +++ b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp @@ -199,21 +199,10 @@ namespace Multiplayer { 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()); - ConnectionId connectionId = m_networkInterface->Connect(ipAddress); - - AzNetworking::IConnection* connection = m_networkInterface->GetConnectionSet().GetConnection(connectionId); - if (connection->GetUserData() == nullptr) // Only add user data if the connect event handler has not already done so - { - connection->SetUserData(new ClientToServerConnectionData(connection, *this, config.m_playerSessionId)); - } - else - { - reinterpret_cast(connection->GetUserData())->SetProviderTicket(config.m_playerSessionId); - } - - connection->SendReliablePacket(MultiplayerPackets::ValidateSession(config.m_playerSessionId.c_str())); + m_networkInterface->Connect(ipAddress); return true; } @@ -416,6 +405,22 @@ namespace Multiplayer [[maybe_unused]] MultiplayerPackets::Connect& packet ) { + // Validate our session with the provider if any + if (AZ::Interface::Get() != nullptr) + { + AzFramework::PlayerConnectionConfig config; + config.m_playerConnectionId = aznumeric_cast(connection->GetConnectionId()); + config.m_playerSessionId = packet.GetTicket(); + if(!AZ::Interface::Get()->ValidatePlayerJoinSession(config)) + { + 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()); + } + if (connection->SendReliablePacket(MultiplayerPackets::Accept(InvalidHostId, sv_map))) { // Sync our console @@ -441,31 +446,6 @@ namespace Multiplayer return true; } - bool MultiplayerSystemComponent::HandleRequest - ( - [[maybe_unused]] AzNetworking::IConnection* connection, - [[maybe_unused]] const IPacketHeader& packetHeader, - [[maybe_unused]] MultiplayerPackets::ValidateSession& packet - ) - { - // Validate our session with the provider if any - if (AZ::Interface::Get() != nullptr) - { - AzFramework::PlayerConnectionConfig config; - config.m_playerConnectionId = aznumeric_cast(connection->GetConnectionId()); - config.m_playerSessionId = packet.GetTicket(); - if(!AZ::Interface::Get()->ValidatePlayerJoinSession(config)) - { - connection->Disconnect(DisconnectReason::TerminatedByServer, TerminationEndpoint::Local); - return false; - } - - reinterpret_cast(connection->GetUserData())->SetProviderTicket(packet.GetTicket().c_str()); - } - - return true; - } - bool MultiplayerSystemComponent::HandleRequest ( AzNetworking::IConnection* connection, @@ -595,10 +575,16 @@ namespace Multiplayer datum.m_isInvited = false; datum.m_agentType = MultiplayerAgentType::Client; + AZStd::string providerTicket; if (connection->GetConnectionRole() == ConnectionRole::Connector) { AZLOG_INFO("New outgoing connection to remote address: %s", connection->GetRemoteAddress().GetString().c_str()); - connection->SendReliablePacket(MultiplayerPackets::Connect(0)); + if (!m_pendingConnectionTickets.empty()) + { + providerTicket = m_pendingConnectionTickets.front(); + m_pendingConnectionTickets.pop(); + } + connection->SendReliablePacket(MultiplayerPackets::Connect(0, providerTicket.c_str())); } else { @@ -628,7 +614,11 @@ namespace Multiplayer { if (connection->GetUserData() == nullptr) // Only add user data if the connect event handler has not already done so { - connection->SetUserData(new ClientToServerConnectionData(connection, *this)); + connection->SetUserData(new ClientToServerConnectionData(connection, *this, providerTicket)); + } + else + { + reinterpret_cast(connection->GetUserData())->SetProviderTicket(providerTicket); } AZStd::unique_ptr window = AZStd::make_unique(); @@ -652,12 +642,7 @@ namespace Multiplayer AZStd::string reasonString = ToString(reason); AZLOG_INFO("%s due to %s from remote address: %s", endpointString, reasonString.c_str(), connection->GetRemoteAddress().GetString().c_str()); - if (connection->GetConnectionRole() == ConnectionRole::Acceptor) - { - // The authority is shutting down its connection - m_shutdownEvent.Signal(m_networkInterface); - } - else if (GetAgentType() == MultiplayerAgentType::Client && connection->GetConnectionRole() == ConnectionRole::Connector) + if (GetAgentType() == MultiplayerAgentType::Client && connection->GetConnectionRole() == ConnectionRole::Connector) { // The client is disconnecting m_clientDisconnectedEvent.Signal(); @@ -675,7 +660,7 @@ namespace Multiplayer if (m_agentType == MultiplayerAgentType::DedicatedServer || m_agentType == MultiplayerAgentType::ClientServer) { if (AZ::Interface::Get() != nullptr && - connection->GetConnectionRole() == ConnectionRole::Connector) + connection->GetConnectionRole() == ConnectionRole::Acceptor) { AzFramework::PlayerConnectionConfig config; config.m_playerConnectionId = aznumeric_cast(connection->GetConnectionId()); @@ -686,7 +671,7 @@ namespace Multiplayer // 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::Connector) + if (m_agentType == MultiplayerAgentType::DedicatedServer && connection->GetConnectionRole() == ConnectionRole::Acceptor) { if (AZ::Interface::Get() != nullptr && m_networkInterface->GetConnectionSet().GetConnectionCount() == 0) diff --git a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h index 2a3fe73ff4..00313c6b65 100644 --- a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h +++ b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h @@ -82,7 +82,6 @@ namespace Multiplayer bool HandleRequest(AzNetworking::IConnection* connection, const AzNetworking::IPacketHeader& packetHeader, MultiplayerPackets::Connect& packet); bool HandleRequest(AzNetworking::IConnection* connection, const AzNetworking::IPacketHeader& packetHeader, MultiplayerPackets::Accept& packet); - bool HandleRequest(AzNetworking::IConnection* connection, const AzNetworking::IPacketHeader& packetHeader, MultiplayerPackets::ValidateSession& packet); bool HandleRequest(AzNetworking::IConnection* connection, const AzNetworking::IPacketHeader& packetHeader, MultiplayerPackets::ReadyForEntityUpdates& packet); bool HandleRequest(AzNetworking::IConnection* connection, const AzNetworking::IPacketHeader& packetHeader, MultiplayerPackets::SyncConsole& packet); bool HandleRequest(AzNetworking::IConnection* connection, const AzNetworking::IPacketHeader& packetHeader, MultiplayerPackets::ConsoleCommand& packet); @@ -147,6 +146,8 @@ namespace Multiplayer ConnectionAcquiredEvent m_connAcquiredEvent; ClientDisconnectedEvent m_clientDisconnectedEvent; + AZStd::queue m_pendingConnectionTickets; + AZ::TimeMs m_lastReplicatedHostTimeMs = AZ::TimeMs{ 0 }; HostFrameId m_lastReplicatedHostFrameId = InvalidHostFrameId; From cf5641a211ba15264d68604b420a223586fc00f2 Mon Sep 17 00:00:00 2001 From: puvvadar Date: Tue, 15 Jun 2021 14:53:23 -0700 Subject: [PATCH 4/5] Cleanup shutdown, activate and deactivate for MPSystemComponent --- .../Code/Source/MultiplayerSystemComponent.cpp | 16 ++++++++++++---- .../Code/Tests/MultiplayerSystemTests.cpp | 5 ++++- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp index 5f44a2cccb..3564c1e91e 100644 --- a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp +++ b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp @@ -179,7 +179,10 @@ namespace Multiplayer AZ::TickBus::Handler::BusConnect(); AzFramework::SessionNotificationBus::Handler::BusConnect(); m_networkInterface = AZ::Interface::Get()->CreateNetworkInterface(AZ::Name(MPNetworkInterfaceName), sv_protocol, TrustZone::ExternalClientToServer, *this); - m_consoleCommandHandler.Connect(AZ::Interface::Get()->GetConsoleCommandInvokedEvent()); + if (AZ::Interface::Get()) + { + m_consoleCommandHandler.Connect(AZ::Interface::Get()->GetConsoleCommandInvokedEvent()); + } AZ::Interface::Register(this); AZ::Interface::Register(this); @@ -191,6 +194,8 @@ namespace Multiplayer { AZ::Interface::Unregister(this); AZ::Interface::Unregister(this); + m_consoleCommandHandler.Disconnect(); + AZ::Interface::Get()->DestroyNetworkInterface(AZ::Name(MPNetworkInterfaceName)); AzFramework::SessionNotificationBus::Handler::BusDisconnect(); AZ::TickBus::Handler::BusDisconnect(); } @@ -673,10 +678,13 @@ namespace Multiplayer // We avoid this for client server as the host itself is a user if (m_agentType == MultiplayerAgentType::DedicatedServer && connection->GetConnectionRole() == ConnectionRole::Acceptor) { - if (AZ::Interface::Get() != nullptr - && m_networkInterface->GetConnectionSet().GetConnectionCount() == 0) + if (m_networkInterface->GetConnectionSet().GetConnectionCount() == 0) { - AZ::Interface::Get()->HandleDestroySession(); + m_shutdownEvent.Signal(m_networkInterface); + if (AZ::Interface::Get() != nullptr) + { + AZ::Interface::Get()->HandleDestroySession(); + } } } } diff --git a/Gems/Multiplayer/Code/Tests/MultiplayerSystemTests.cpp b/Gems/Multiplayer/Code/Tests/MultiplayerSystemTests.cpp index 7b09b65de4..9579c84fc1 100644 --- a/Gems/Multiplayer/Code/Tests/MultiplayerSystemTests.cpp +++ b/Gems/Multiplayer/Code/Tests/MultiplayerSystemTests.cpp @@ -35,14 +35,16 @@ namespace UnitTest m_initHandler = Multiplayer::SessionInitEvent::Handler([this](AzNetworking::INetworkInterface* value) { TestInitEvent(value); }); m_mpComponent->AddSessionInitHandler(m_initHandler); - m_shutdownHandler = Multiplayer::SessionInitEvent::Handler([this](AzNetworking::INetworkInterface* value) { TestShutdownEvent(value); }); + m_shutdownHandler = Multiplayer::SessionShutdownEvent::Handler([this](AzNetworking::INetworkInterface* value) { TestShutdownEvent(value); }); m_mpComponent->AddSessionShutdownHandler(m_shutdownHandler); m_connAcquiredHandler = Multiplayer::ConnectionAcquiredEvent::Handler([this](Multiplayer::MultiplayerAgentDatum value) { TestConnectionAcquiredEvent(value); }); m_mpComponent->AddConnectionAcquiredHandler(m_connAcquiredHandler); + m_mpComponent->Activate(); } void TearDown() override { + m_mpComponent->Deactivate(); delete m_mpComponent; delete m_netComponent; AZ::NameDictionary::Destroy(); @@ -86,6 +88,7 @@ namespace UnitTest TEST_F(MultiplayerSystemTests, TestShutdownEvent) { + m_mpComponent->InitializeMultiplayer(Multiplayer::MultiplayerAgentType::DedicatedServer); IMultiplayerConnectionMock connMock1 = IMultiplayerConnectionMock(AzNetworking::ConnectionId(), AzNetworking::IpAddress(), AzNetworking::ConnectionRole::Acceptor); IMultiplayerConnectionMock connMock2 = IMultiplayerConnectionMock(AzNetworking::ConnectionId(), AzNetworking::IpAddress(), AzNetworking::ConnectionRole::Connector); m_mpComponent->OnDisconnect(&connMock1, AzNetworking::DisconnectReason::None, AzNetworking::TerminationEndpoint::Local); From 2a6009c94b9796abbe0222abc67c36e5886e2745 Mon Sep 17 00:00:00 2001 From: puvvadar Date: Tue, 15 Jun 2021 17:46:51 -0700 Subject: [PATCH 5/5] Cleanup OnDisconnect client case slightly --- Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp index 3564c1e91e..c43d136cd8 100644 --- a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp +++ b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp @@ -647,9 +647,10 @@ namespace Multiplayer AZStd::string reasonString = ToString(reason); AZLOG_INFO("%s due to %s from remote address: %s", endpointString, reasonString.c_str(), connection->GetRemoteAddress().GetString().c_str()); - if (GetAgentType() == MultiplayerAgentType::Client && connection->GetConnectionRole() == ConnectionRole::Connector) + // The client is disconnecting + if (GetAgentType() == MultiplayerAgentType::Client) { - // The client is disconnecting + AZ_Assert(connection->GetConnectionRole() == ConnectionRole::Connector, "Client connection role should only ever be Connector"); m_clientDisconnectedEvent.Signal(); }