From 2811a8418774bb0002fb51333259878360547377 Mon Sep 17 00:00:00 2001 From: puvvadar Date: Thu, 30 Sep 2021 19:59:34 -0700 Subject: [PATCH 1/5] Move did handshake logic to connection data plus an optimization Signed-off-by: puvvadar --- .../AutoGen/AutoPacketDispatcher_Inline.jinja | 11 ++++++- .../ConnectionData/IConnectionData.h | 9 ++++++ .../ClientToServerConnectionData.h | 3 ++ .../ClientToServerConnectionData.inl | 10 ++++++ .../ServerToClientConnectionData.h | 3 ++ .../ServerToClientConnectionData.inl | 10 ++++++ .../Editor/MultiplayerEditorConnection.h | 1 - .../Source/MultiplayerSystemComponent.cpp | 31 ++++++++++++------- .../Code/Source/MultiplayerSystemComponent.h | 3 +- 9 files changed, 66 insertions(+), 15 deletions(-) diff --git a/Code/Framework/AzNetworking/AzNetworking/AutoGen/AutoPacketDispatcher_Inline.jinja b/Code/Framework/AzNetworking/AzNetworking/AutoGen/AutoPacketDispatcher_Inline.jinja index c6f1a23402..73c3227cd5 100644 --- a/Code/Framework/AzNetworking/AzNetworking/AutoGen/AutoPacketDispatcher_Inline.jinja +++ b/Code/Framework/AzNetworking/AzNetworking/AutoGen/AutoPacketDispatcher_Inline.jinja @@ -6,15 +6,24 @@ namespace {{ xml.attrib['Name'] }} { switch (aznumeric_cast(packetHeader.GetPacketType())) { +{% set hs = namespace(handshake=false) %} +{% for Packet in xml.iter('Packet') %} +{% if ('HandshakePacket' in Packet.attrib) and (Packet.attrib['HandshakePacket'] == 'true') %} +{% set hs.handshake = True %} +{% endif %} +{% endfor %} + {% for Packet in xml.iter('Packet') %} case aznumeric_cast({{ Packet.attrib['Name'] }}::Type): { AZLOG(Debug_DispatchPackets, "Received packet %s", "{{ Packet.attrib['Name'] }}"); +{% if hs.handshake %} {% if ('HandshakePacket' not in Packet.attrib) or (Packet.attrib['HandshakePacket'] == 'false') %} - if (!handler.IsHandshakeComplete()) + if (!handler.IsHandshakeComplete(connection)) { return AzNetworking::PacketDispatchResult::Skipped; } +{% endif %} {% endif %} {{ Packet.attrib['Name'] }} packet; diff --git a/Gems/Multiplayer/Code/Include/Multiplayer/ConnectionData/IConnectionData.h b/Gems/Multiplayer/Code/Include/Multiplayer/ConnectionData/IConnectionData.h index 66a7a1175a..01914f55ae 100644 --- a/Gems/Multiplayer/Code/Include/Multiplayer/ConnectionData/IConnectionData.h +++ b/Gems/Multiplayer/Code/Include/Multiplayer/ConnectionData/IConnectionData.h @@ -50,5 +50,14 @@ namespace Multiplayer //! Sets the state of connection whether update messages can be sent or not. //! @param canSendUpdates the state value virtual void SetCanSendUpdates(bool canSendUpdates) = 0; + + + //! Fetches the state of connection whether handshake logic has completed + //! @return true if handshake has completed + virtual bool DidHandshake() const = 0; + + //! Sets the state of connection whether handshake logic has completed + //! @param didHandshake if handshake logic has completed + virtual void SetDidHandshake(bool didHandshake) = 0; }; } diff --git a/Gems/Multiplayer/Code/Source/ConnectionData/ClientToServerConnectionData.h b/Gems/Multiplayer/Code/Source/ConnectionData/ClientToServerConnectionData.h index e5f1d0edfc..27826baf88 100644 --- a/Gems/Multiplayer/Code/Source/ConnectionData/ClientToServerConnectionData.h +++ b/Gems/Multiplayer/Code/Source/ConnectionData/ClientToServerConnectionData.h @@ -33,6 +33,8 @@ namespace Multiplayer void Update(AZ::TimeMs hostTimeMs) override; bool CanSendUpdates() const override; void SetCanSendUpdates(bool canSendUpdates) override; + bool DidHandshake() const override; + void SetDidHandshake(bool didHandshake) override; //! @} const AZStd::string& GetProviderTicket() const; @@ -43,6 +45,7 @@ namespace Multiplayer AZStd::string m_providerTicket; AzNetworking::IConnection* m_connection = nullptr; bool m_canSendUpdates = true; + bool m_didHandshake = false; }; } diff --git a/Gems/Multiplayer/Code/Source/ConnectionData/ClientToServerConnectionData.inl b/Gems/Multiplayer/Code/Source/ConnectionData/ClientToServerConnectionData.inl index 9d6a3ca744..8874fbbabc 100644 --- a/Gems/Multiplayer/Code/Source/ConnectionData/ClientToServerConnectionData.inl +++ b/Gems/Multiplayer/Code/Source/ConnectionData/ClientToServerConnectionData.inl @@ -27,4 +27,14 @@ namespace Multiplayer { m_providerTicket = ticket; } + + inline bool ClientToServerConnectionData::DidHandshake() const + { + return m_didHandshake; + } + + inline void ClientToServerConnectionData::SetDidHandshake(bool didHandshake) + { + m_didHandshake = didHandshake; + } } diff --git a/Gems/Multiplayer/Code/Source/ConnectionData/ServerToClientConnectionData.h b/Gems/Multiplayer/Code/Source/ConnectionData/ServerToClientConnectionData.h index 9e9afc4413..4e588570ec 100644 --- a/Gems/Multiplayer/Code/Source/ConnectionData/ServerToClientConnectionData.h +++ b/Gems/Multiplayer/Code/Source/ConnectionData/ServerToClientConnectionData.h @@ -33,6 +33,8 @@ namespace Multiplayer void Update(AZ::TimeMs hostTimeMs) override; bool CanSendUpdates() const override; void SetCanSendUpdates(bool canSendUpdates) override; + bool DidHandshake() const override; + void SetDidHandshake(bool didHandshake) override; //! @} NetworkEntityHandle GetPrimaryPlayerEntity(); @@ -52,6 +54,7 @@ namespace Multiplayer AZStd::string m_providerTicket; AzNetworking::IConnection* m_connection = nullptr; bool m_canSendUpdates = false; + bool m_didHandshake = false; }; } diff --git a/Gems/Multiplayer/Code/Source/ConnectionData/ServerToClientConnectionData.inl b/Gems/Multiplayer/Code/Source/ConnectionData/ServerToClientConnectionData.inl index 53ba51f36a..e4348fe539 100644 --- a/Gems/Multiplayer/Code/Source/ConnectionData/ServerToClientConnectionData.inl +++ b/Gems/Multiplayer/Code/Source/ConnectionData/ServerToClientConnectionData.inl @@ -38,4 +38,14 @@ namespace Multiplayer { m_providerTicket = ticket; } + + inline bool ServerToClientConnectionData::DidHandshake() const + { + return m_didHandshake; + } + + inline void ServerToClientConnectionData::SetDidHandshake(bool didHandshake) + { + m_didHandshake = didHandshake; + } } diff --git a/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorConnection.h b/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorConnection.h index ca815d5c48..b93c830cd0 100644 --- a/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorConnection.h +++ b/Gems/Multiplayer/Code/Source/Editor/MultiplayerEditorConnection.h @@ -33,7 +33,6 @@ namespace Multiplayer MultiplayerEditorConnection(); ~MultiplayerEditorConnection() = default; - bool IsHandshakeComplete() const { return true; }; bool HandleRequest(AzNetworking::IConnection* connection, const AzNetworking::IPacketHeader& packetHeader, MultiplayerEditorPackets::EditorServerInit& packet); bool HandleRequest(AzNetworking::IConnection* connection, const AzNetworking::IPacketHeader& packetHeader, MultiplayerEditorPackets::EditorServerReady& packet); diff --git a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp index 023c5ea3af..b1d80a87b5 100644 --- a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp +++ b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp @@ -442,9 +442,9 @@ namespace Multiplayer MultiplayerPackets::SyncConsole m_syncPacket; }; - bool MultiplayerSystemComponent::IsHandshakeComplete() const + bool MultiplayerSystemComponent::IsHandshakeComplete(AzNetworking::IConnection* connection) const { - return m_didHandshake; + return reinterpret_cast(connection->GetUserData())->DidHandshake(); } bool MultiplayerSystemComponent::HandleRequest @@ -471,7 +471,7 @@ namespace Multiplayer if (connection->SendReliablePacket(MultiplayerPackets::Accept(InvalidHostId, sv_map))) { - m_didHandshake = true; + reinterpret_cast(connection->GetUserData())->SetDidHandshake(true); // Sync our console ConsoleReplicator consoleReplicator(connection); @@ -488,7 +488,7 @@ namespace Multiplayer [[maybe_unused]] MultiplayerPackets::Accept& packet ) { - m_didHandshake = true; + reinterpret_cast(connection->GetUserData())->SetDidHandshake(true); AZ::CVarFixedString commandString = "sv_map " + packet.GetMap(); AZ::Interface::Get()->PerformCommand(commandString.c_str()); @@ -903,8 +903,9 @@ namespace Multiplayer // Unfortunately necessary, as NotifyPreRender can update transforms and thus cause a deadlock inside the vis system AZStd::vector gatheredEntities; + INetworkEntityManager* netEntityManager = GetNetworkEntityManager(); AZ::Interface::Get()->GetDefaultVisibilityScene()->Enumerate(viewFrustum, - [&gatheredEntities](const AzFramework::IVisibilityScene::NodeData& nodeData) + [netEntityManager, &gatheredEntities](const AzFramework::IVisibilityScene::NodeData& nodeData) { gatheredEntities.reserve(gatheredEntities.size() + nodeData.m_entries.size()); for (AzFramework::VisibilityEntry* visEntry : nodeData.m_entries) @@ -912,10 +913,14 @@ namespace Multiplayer if (visEntry->m_typeFlags & AzFramework::VisibilityEntry::TypeFlags::TYPE_Entity) { AZ::Entity* entity = static_cast(visEntry->m_userData); - NetBindComponent* netBindComponent = entity->FindComponent(); - if (netBindComponent != nullptr) + NetEntityId netEntitydId = netEntityManager->GetNetEntityIdById(entity->GetId()); + if (netEntitydId != InvalidNetEntityId) { - gatheredEntities.push_back(netBindComponent); + NetBindComponent* netBindComponent = netEntityManager->GetEntity(netEntitydId).GetNetBindComponent(); + if (netBindComponent != nullptr) + { + gatheredEntities.push_back(netBindComponent); + } } } } @@ -932,10 +937,14 @@ namespace Multiplayer for (auto& iter : *(m_networkEntityManager.GetNetworkEntityTracker())) { AZ::Entity* entity = iter.second; - NetBindComponent* netBindComponent = entity->FindComponent(); - if (netBindComponent != nullptr) + NetEntityId netEntitydId = GetNetworkEntityManager()->GetNetEntityIdById(entity->GetId()); + if (netEntitydId != InvalidNetEntityId) { - netBindComponent->NotifyPreRender(deltaTime); + NetBindComponent* netBindComponent = GetNetworkEntityManager()->GetEntity(netEntitydId).GetNetBindComponent(); + if (netBindComponent != nullptr) + { + netBindComponent->NotifyPreRender(deltaTime); + } } } } diff --git a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h index c467ed9ad9..ad9f22bc5b 100644 --- a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h +++ b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.h @@ -76,7 +76,7 @@ namespace Multiplayer int GetTickOrder() override; //! @} - bool IsHandshakeComplete() const; + bool IsHandshakeComplete(AzNetworking::IConnection* connection) const; 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::ReadyForEntityUpdates& packet); @@ -159,7 +159,6 @@ namespace Multiplayer double m_serverSendAccumulator = 0.0; float m_renderBlendFactor = 0.0f; float m_tickFactor = 0.0f; - bool m_didHandshake = false; #if !defined(AZ_RELEASE_BUILD) MultiplayerEditorConnection m_editorConnectionListener; From f6638420f09fb6ca97b0b47962c201d08aeb020f Mon Sep 17 00:00:00 2001 From: puvvadar Date: Thu, 30 Sep 2021 20:02:38 -0700 Subject: [PATCH 2/5] Formatting fix up Signed-off-by: puvvadar --- .../AutoGen/AutoPacketDispatcher_Inline.jinja | 16 ++++++++-------- .../Multiplayer/ConnectionData/IConnectionData.h | 1 - 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/Code/Framework/AzNetworking/AzNetworking/AutoGen/AutoPacketDispatcher_Inline.jinja b/Code/Framework/AzNetworking/AzNetworking/AutoGen/AutoPacketDispatcher_Inline.jinja index 73c3227cd5..a621af92cb 100644 --- a/Code/Framework/AzNetworking/AzNetworking/AutoGen/AutoPacketDispatcher_Inline.jinja +++ b/Code/Framework/AzNetworking/AzNetworking/AutoGen/AutoPacketDispatcher_Inline.jinja @@ -6,25 +6,25 @@ namespace {{ xml.attrib['Name'] }} { switch (aznumeric_cast(packetHeader.GetPacketType())) { -{% set hs = namespace(handshake=false) %} +{% set packet_ns = namespace(handshake=false) %} {% for Packet in xml.iter('Packet') %} -{% if ('HandshakePacket' in Packet.attrib) and (Packet.attrib['HandshakePacket'] == 'true') %} -{% set hs.handshake = True %} -{% endif %} +{% if ('HandshakePacket' in Packet.attrib) and (Packet.attrib['HandshakePacket'] == 'true') %} +{% set packet_ns.handshake = True %} +{% endif %} {% endfor %} {% for Packet in xml.iter('Packet') %} case aznumeric_cast({{ Packet.attrib['Name'] }}::Type): { AZLOG(Debug_DispatchPackets, "Received packet %s", "{{ Packet.attrib['Name'] }}"); -{% if hs.handshake %} -{% if ('HandshakePacket' not in Packet.attrib) or (Packet.attrib['HandshakePacket'] == 'false') %} +{% if packet_ns.handshake %} +{% if ('HandshakePacket' not in Packet.attrib) or (Packet.attrib['HandshakePacket'] == 'false') %} if (!handler.IsHandshakeComplete(connection)) { return AzNetworking::PacketDispatchResult::Skipped; } -{% endif %} -{% endif %} +{% endif %} +{% endif %} {{ Packet.attrib['Name'] }} packet; if (!serializer.Serialize(packet, "Packet")) diff --git a/Gems/Multiplayer/Code/Include/Multiplayer/ConnectionData/IConnectionData.h b/Gems/Multiplayer/Code/Include/Multiplayer/ConnectionData/IConnectionData.h index 01914f55ae..90dfcc16bd 100644 --- a/Gems/Multiplayer/Code/Include/Multiplayer/ConnectionData/IConnectionData.h +++ b/Gems/Multiplayer/Code/Include/Multiplayer/ConnectionData/IConnectionData.h @@ -51,7 +51,6 @@ namespace Multiplayer //! @param canSendUpdates the state value virtual void SetCanSendUpdates(bool canSendUpdates) = 0; - //! Fetches the state of connection whether handshake logic has completed //! @return true if handshake has completed virtual bool DidHandshake() const = 0; From 0a6ddfab5efd24ab232105927f04200a393debeb Mon Sep 17 00:00:00 2001 From: puvvadar Date: Fri, 1 Oct 2021 11:02:12 -0700 Subject: [PATCH 3/5] Revert optimization change in favor of another PR Signed-off-by: puvvadar --- .../Source/MultiplayerSystemComponent.cpp | 23 ++++++------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp index b1d80a87b5..71e8b04d85 100644 --- a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp +++ b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp @@ -903,9 +903,8 @@ namespace Multiplayer // Unfortunately necessary, as NotifyPreRender can update transforms and thus cause a deadlock inside the vis system AZStd::vector gatheredEntities; - INetworkEntityManager* netEntityManager = GetNetworkEntityManager(); AZ::Interface::Get()->GetDefaultVisibilityScene()->Enumerate(viewFrustum, - [netEntityManager, &gatheredEntities](const AzFramework::IVisibilityScene::NodeData& nodeData) + [&gatheredEntities](const AzFramework::IVisibilityScene::NodeData& nodeData) { gatheredEntities.reserve(gatheredEntities.size() + nodeData.m_entries.size()); for (AzFramework::VisibilityEntry* visEntry : nodeData.m_entries) @@ -913,14 +912,10 @@ namespace Multiplayer if (visEntry->m_typeFlags & AzFramework::VisibilityEntry::TypeFlags::TYPE_Entity) { AZ::Entity* entity = static_cast(visEntry->m_userData); - NetEntityId netEntitydId = netEntityManager->GetNetEntityIdById(entity->GetId()); - if (netEntitydId != InvalidNetEntityId) + NetBindComponent* netBindComponent = entity->FindComponent(); + if (netBindComponent != nullptr) { - NetBindComponent* netBindComponent = netEntityManager->GetEntity(netEntitydId).GetNetBindComponent(); - if (netBindComponent != nullptr) - { - gatheredEntities.push_back(netBindComponent); - } + gatheredEntities.push_back(netBindComponent); } } } @@ -937,14 +932,10 @@ namespace Multiplayer for (auto& iter : *(m_networkEntityManager.GetNetworkEntityTracker())) { AZ::Entity* entity = iter.second; - NetEntityId netEntitydId = GetNetworkEntityManager()->GetNetEntityIdById(entity->GetId()); - if (netEntitydId != InvalidNetEntityId) + NetBindComponent* netBindComponent = entity->FindComponent(); + if (netBindComponent != nullptr) { - NetBindComponent* netBindComponent = GetNetworkEntityManager()->GetEntity(netEntitydId).GetNetBindComponent(); - if (netBindComponent != nullptr) - { - netBindComponent->NotifyPreRender(deltaTime); - } + netBindComponent->NotifyPreRender(deltaTime); } } } From a18655e8f7305f05844170adfce2ee0a574cf832 Mon Sep 17 00:00:00 2001 From: puvvadar Date: Fri, 1 Oct 2021 11:43:22 -0700 Subject: [PATCH 4/5] Update jinja change to use booleanTrue filter Signed-off-by: puvvadar --- .../AzNetworking/AutoGen/AutoPacketDispatcher_Inline.jinja | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Code/Framework/AzNetworking/AzNetworking/AutoGen/AutoPacketDispatcher_Inline.jinja b/Code/Framework/AzNetworking/AzNetworking/AutoGen/AutoPacketDispatcher_Inline.jinja index a621af92cb..145f15bb19 100644 --- a/Code/Framework/AzNetworking/AzNetworking/AutoGen/AutoPacketDispatcher_Inline.jinja +++ b/Code/Framework/AzNetworking/AzNetworking/AutoGen/AutoPacketDispatcher_Inline.jinja @@ -8,7 +8,7 @@ namespace {{ xml.attrib['Name'] }} { {% set packet_ns = namespace(handshake=false) %} {% for Packet in xml.iter('Packet') %} -{% if ('HandshakePacket' in Packet.attrib) and (Packet.attrib['HandshakePacket'] == 'true') %} +{% if ('HandshakePacket' in Packet.attrib) and (Packet.attrib['HandshakePacket']|booleanTrue == true) %} {% set packet_ns.handshake = True %} {% endif %} {% endfor %} From 91aafdcddef85d0adb128d10165d55fc622de112 Mon Sep 17 00:00:00 2001 From: puvvadar Date: Mon, 4 Oct 2021 15:17:01 -0700 Subject: [PATCH 5/5] Fix bad include in NetworkTransformTests Signed-off-by: puvvadar --- Gems/Multiplayer/Code/Tests/NetworkTransformTests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gems/Multiplayer/Code/Tests/NetworkTransformTests.cpp b/Gems/Multiplayer/Code/Tests/NetworkTransformTests.cpp index 94cb94e000..359c8003a4 100644 --- a/Gems/Multiplayer/Code/Tests/NetworkTransformTests.cpp +++ b/Gems/Multiplayer/Code/Tests/NetworkTransformTests.cpp @@ -15,7 +15,7 @@ #include #include #include -#include +#include namespace Multiplayer {