From d05d5d1e976f6b7c429c51c800070d1bdbf79676 Mon Sep 17 00:00:00 2001 From: AMZN-Olex <5432499+AMZN-Olex@users.noreply.github.com> Date: Tue, 5 Oct 2021 08:31:23 -0400 Subject: [PATCH] Hierarchy components optimizations Signed-off-by: AMZN-Olex <5432499+AMZN-Olex@users.noreply.github.com> --- .../NetworkHierarchyChildComponent.h | 2 - .../NetworkHierarchyRootComponent.h | 18 +--- .../NetworkHierarchyChildComponent.cpp | 68 +++++++------- .../NetworkHierarchyRootComponent.cpp | 88 ++++++++----------- .../Code/Tests/ServerHierarchyTests.cpp | 32 ++++++- 5 files changed, 103 insertions(+), 105 deletions(-) diff --git a/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkHierarchyChildComponent.h b/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkHierarchyChildComponent.h index 544fb3d6cb..a31cd90efd 100644 --- a/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkHierarchyChildComponent.h +++ b/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkHierarchyChildComponent.h @@ -64,10 +64,8 @@ namespace Multiplayer private: AZ::ChildChangedEvent::Handler m_childChangedHandler; - AZ::ParentChangedEvent::Handler m_parentChangedHandler; void OnChildChanged(AZ::ChildChangeType type, AZ::EntityId child); - void OnParentChanged(AZ::EntityId oldParent, AZ::EntityId parent); //! Points to the top level root. AZ::Entity* m_rootEntity = nullptr; diff --git a/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkHierarchyRootComponent.h b/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkHierarchyRootComponent.h index 674dc6114d..7c9b0d34cd 100644 --- a/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkHierarchyRootComponent.h +++ b/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkHierarchyRootComponent.h @@ -78,22 +78,12 @@ namespace Multiplayer //! Rebuilds hierarchy starting from this root component's entity. void RebuildHierarchy(); - //! @param underEntity Walk the child entities that belong to @underEntity and consider adding them to the hierarchy - //! @param currentEntityCount The total number of entities in the hierarchy prior to calling this method, - //! used to avoid adding too many entities to the hierarchy while walking recursively the relevant entities. - //! @currentEntityCount will be modified to reflect the total entity count upon completion of this method. - //! @returns false if an attempt was made to go beyond the maximum supported hierarchy size, true otherwise - bool RecursiveAttachHierarchicalEntities(AZ::EntityId underEntity, uint32_t& currentEntityCount); - - //! @param entity Add the child entity and any of its relevant children to the hierarchy - //! @param currentEntityCount The total number of entities in the hierarchy prior to calling this method, - //! used to avoid adding too many entities to the hierarchy while walking recursively the relevant entities. - //! @currentEntityCount will be modified to reflect the total entity count upon completion of this method. - //! @returns false if an attempt was made to go beyond the maximum supported hierarchy size, true otherwise - bool RecursiveAttachHierarchicalChild(AZ::EntityId entity, uint32_t& currentEntityCount); + //! @param underEntity Walk the child entities that belong to @underEntity and consider adding them to the hierarchy. + //! Builds the hierarchy using breadth-first iterative method. + void InternalBuildHierarchyList(AZ::Entity* underEntity); void SetRootForEntity(AZ::Entity* root, const AZ::Entity* childEntity); - + //! Set to false when deactivating or otherwise not to be included in hierarchy considerations. bool m_isHierarchyEnabled = true; diff --git a/Gems/Multiplayer/Code/Source/Components/NetworkHierarchyChildComponent.cpp b/Gems/Multiplayer/Code/Source/Components/NetworkHierarchyChildComponent.cpp index 3124eccaa8..5d9b4a40ad 100644 --- a/Gems/Multiplayer/Code/Source/Components/NetworkHierarchyChildComponent.cpp +++ b/Gems/Multiplayer/Code/Source/Components/NetworkHierarchyChildComponent.cpp @@ -55,7 +55,6 @@ namespace Multiplayer NetworkHierarchyChildComponent::NetworkHierarchyChildComponent() : m_childChangedHandler([this](AZ::ChildChangeType type, AZ::EntityId child) { OnChildChanged(type, child); }) - , m_parentChangedHandler([this](AZ::EntityId oldParent, AZ::EntityId parent) { OnParentChanged(oldParent, parent); }) , m_hierarchyRootNetIdChanged([this](NetEntityId rootNetId) {OnHierarchyRootNetIdChanged(rootNetId); }) { @@ -75,7 +74,6 @@ namespace Multiplayer if (AzFramework::TransformComponent* transformComponent = GetEntity()->FindComponent()) { transformComponent->BindChildChangedEventHandler(m_childChangedHandler); - transformComponent->BindParentChangedEventHandler(m_parentChangedHandler); } } @@ -133,29 +131,33 @@ namespace Multiplayer void NetworkHierarchyChildComponent::SetTopLevelHierarchyRootEntity(AZ::Entity* hierarchyRoot) { - m_rootEntity = hierarchyRoot; - if (HasController() && GetNetBindComponent()->GetNetEntityRole() == NetEntityRole::Authority) + if (m_rootEntity != hierarchyRoot) { - NetworkHierarchyChildComponentController* controller = static_cast(GetController()); - if (m_rootEntity) + m_rootEntity = hierarchyRoot; + + if (HasController() && GetNetBindComponent()->GetNetEntityRole() == NetEntityRole::Authority) { - const NetEntityId netRootId = GetNetworkEntityManager()->GetNetEntityIdById(m_rootEntity->GetId()); - controller->SetHierarchyRoot(netRootId); + NetworkHierarchyChildComponentController* controller = static_cast(GetController()); + if (m_rootEntity) + { + const NetEntityId netRootId = GetNetworkEntityManager()->GetNetEntityIdById(m_rootEntity->GetId()); + controller->SetHierarchyRoot(netRootId); - m_networkHierarchyChangedEvent.Signal(m_rootEntity->GetId()); + m_networkHierarchyChangedEvent.Signal(m_rootEntity->GetId()); + } + else + { + controller->SetHierarchyRoot(InvalidNetEntityId); + + m_networkHierarchyLeaveEvent.Signal(); + } } - else + + if (m_rootEntity == nullptr) { - controller->SetHierarchyRoot(InvalidNetEntityId); - - m_networkHierarchyLeaveEvent.Signal(); + NotifyChildrenHierarchyDisbanded(); } } - - if (m_rootEntity == nullptr) - { - NotifyChildrenHierarchyDisbanded(); - } } void NetworkHierarchyChildComponent::OnChildChanged([[maybe_unused]] AZ::ChildChangeType type, [[maybe_unused]] AZ::EntityId child) @@ -169,17 +171,6 @@ namespace Multiplayer } } - void NetworkHierarchyChildComponent::OnParentChanged([[maybe_unused]] AZ::EntityId oldParent, [[maybe_unused]] AZ::EntityId parent) - { - if (m_rootEntity) - { - if (NetworkHierarchyRootComponent* root = m_rootEntity->FindComponent()) - { - root->RebuildHierarchy(); - } - } - } - void NetworkHierarchyChildComponent::OnHierarchyRootNetIdChanged(NetEntityId rootNetId) { ConstNetworkEntityHandle rootHandle = GetNetworkEntityManager()->GetEntity(rootNetId); @@ -202,19 +193,24 @@ namespace Multiplayer void NetworkHierarchyChildComponent::NotifyChildrenHierarchyDisbanded() { + AZ::ComponentApplicationRequests* componentApplication = AZ::Interface::Get(); + AZStd::vector allChildren; AZ::TransformBus::EventResult(allChildren, GetEntityId(), &AZ::TransformBus::Events::GetChildren); for (const AZ::EntityId& childEntityId : allChildren) { - if (const AZ::Entity* childEntity = AZ::Interface::Get()->FindEntity(childEntityId)) + if (const AZ::Entity* childEntity = componentApplication->FindEntity(childEntityId)) { - if (auto* hierarchyChildComponent = childEntity->FindComponent()) + for (Component* component : childEntity->GetComponents()) { - hierarchyChildComponent->SetTopLevelHierarchyRootEntity(nullptr); - } - else if (auto* hierarchyRootComponent = childEntity->FindComponent()) - { - hierarchyRootComponent->SetTopLevelHierarchyRootEntity(nullptr); + if (component->GetUnderlyingComponentType() == NetworkHierarchyChildComponent::TYPEINFO_Uuid()) + { + static_cast(component)->SetTopLevelHierarchyRootEntity(nullptr); + } + else if (component->GetUnderlyingComponentType() == NetworkHierarchyRootComponent::TYPEINFO_Uuid()) + { + static_cast(component)->SetTopLevelHierarchyRootEntity(nullptr); + } } } } diff --git a/Gems/Multiplayer/Code/Source/Components/NetworkHierarchyRootComponent.cpp b/Gems/Multiplayer/Code/Source/Components/NetworkHierarchyRootComponent.cpp index 4a5e9ce8d2..5011e3f17b 100644 --- a/Gems/Multiplayer/Code/Source/Components/NetworkHierarchyRootComponent.cpp +++ b/Gems/Multiplayer/Code/Source/Components/NetworkHierarchyRootComponent.cpp @@ -203,10 +203,9 @@ namespace Multiplayer AZStd::vector previousEntities; m_hierarchicalEntities.swap(previousEntities); - m_hierarchicalEntities.push_back(GetEntity()); // Add the root. + m_hierarchicalEntities.reserve(bg_hierarchyEntityMaxLimit); - uint32_t currentEntityCount = aznumeric_cast(m_hierarchicalEntities.size()); - RecursiveAttachHierarchicalEntities(GetEntityId(), currentEntityCount); + InternalBuildHierarchyList(GetEntity()); bool hierarchyChanged = false; @@ -244,6 +243,43 @@ namespace Multiplayer } } + void NetworkHierarchyRootComponent::InternalBuildHierarchyList(AZ::Entity* underEntity) + { + AZ::ComponentApplicationRequests* componentApplicationRequests = AZ::Interface::Get(); + + AZStd::deque candidates; + candidates.push_back(underEntity); + + while (!candidates.empty()) + { + AZ::Entity* candidate = candidates.front(); + candidates.pop_front(); + + if (candidate) + { + auto* hierarchyChildComponent = candidate->FindComponent(); + auto* hierarchyRootComponent = candidate->FindComponent(); + + if ((hierarchyChildComponent && hierarchyChildComponent->IsHierarchyEnabled()) || + (hierarchyRootComponent && hierarchyRootComponent->IsHierarchyEnabled())) + { + m_hierarchicalEntities.push_back(candidate); + + if (m_hierarchicalEntities.size() >= bg_hierarchyEntityMaxLimit) + { + return; + } + + const AZStd::vector allChildren = candidate->GetTransform()->GetChildren(); + for (const AZ::EntityId& newChildId : allChildren) + { + candidates.push_back(componentApplicationRequests->FindEntity(newChildId)); + } + } + } + } + } + void NetworkHierarchyRootComponent::SetRootForEntity(AZ::Entity* root, const AZ::Entity* childEntity) { if (auto* hierarchyChildComponent = childEntity->FindComponent()) @@ -256,52 +292,6 @@ namespace Multiplayer } } - bool NetworkHierarchyRootComponent::RecursiveAttachHierarchicalEntities(AZ::EntityId underEntity, uint32_t& currentEntityCount) - { - AZStd::vector allChildren; - AZ::TransformBus::EventResult(allChildren, underEntity, &AZ::TransformBus::Events::GetChildren); - - for (const AZ::EntityId& newChildId : allChildren) - { - if (!RecursiveAttachHierarchicalChild(newChildId, currentEntityCount)) - { - return false; - } - } - - return true; - } - - bool NetworkHierarchyRootComponent::RecursiveAttachHierarchicalChild(AZ::EntityId entity, uint32_t& currentEntityCount) - { - if (currentEntityCount >= bg_hierarchyEntityMaxLimit) - { - AZLOG_WARN("Entity %s is trying to build a network hierarchy that is too large. bg_hierarchyEntityMaxLimit is currently set to (%u)", - GetEntity()->GetName().c_str(), static_cast(bg_hierarchyEntityMaxLimit)); - return false; - } - - if (AZ::Entity* childEntity = AZ::Interface::Get()->FindEntity(entity)) - { - auto* hierarchyChildComponent = childEntity->FindComponent(); - auto* hierarchyRootComponent = childEntity->FindComponent(); - - if ((hierarchyChildComponent && hierarchyChildComponent->IsHierarchyEnabled()) || - (hierarchyRootComponent && hierarchyRootComponent->IsHierarchyEnabled())) - { - m_hierarchicalEntities.push_back(childEntity); - ++currentEntityCount; - - if (!RecursiveAttachHierarchicalEntities(entity, currentEntityCount)) - { - return false; - } - } - } - - return true; - } - void NetworkHierarchyRootComponent::SetTopLevelHierarchyRootEntity(AZ::Entity* hierarchyRoot) { m_rootEntity = hierarchyRoot; diff --git a/Gems/Multiplayer/Code/Tests/ServerHierarchyTests.cpp b/Gems/Multiplayer/Code/Tests/ServerHierarchyTests.cpp index b8e892e71a..1b5aa31780 100644 --- a/Gems/Multiplayer/Code/Tests/ServerHierarchyTests.cpp +++ b/Gems/Multiplayer/Code/Tests/ServerHierarchyTests.cpp @@ -394,8 +394,8 @@ namespace Multiplayer m_console->PerformCommand("bg_hierarchyEntityMaxLimit 2"); // remake the hierarchy - m_root->m_entity->FindComponent()->SetParent(AZ::EntityId()); - m_root->m_entity->FindComponent()->SetParent(m_root->m_entity->GetId()); + m_child->m_entity->FindComponent()->SetParent(AZ::EntityId()); + m_child->m_entity->FindComponent()->SetParent(m_root->m_entity->GetId()); EXPECT_EQ( m_root->m_entity->FindComponent()->GetHierarchicalEntities().size(), @@ -406,6 +406,17 @@ namespace Multiplayer m_console->GetCvarValue("bg_hierarchyEntityMaxLimit", currentMaxLimit); } + TEST_F(ServerDeepHierarchyTests, ReattachMiddleChildRebuildInvokedTwice) + { + MockNetworkHierarchyCallbackHandler mock; + EXPECT_CALL(mock, OnNetworkHierarchyUpdated(m_root->m_entity->GetId())).Times(2); + + m_root->m_entity->FindComponent()->BindNetworkHierarchyChangedEventHandler(mock.m_changedHandler); + + m_child->m_entity->FindComponent()->SetParent(AZ::EntityId()); + m_child->m_entity->FindComponent()->SetParent(m_root->m_entity->GetId()); + } + /* * Parent -> Child -> Child Of Child * -> Child2 -> Child Of Child2 @@ -533,11 +544,11 @@ namespace Multiplayer ); EXPECT_EQ( m_root->m_entity->FindComponent()->GetHierarchicalEntities()[2], - m_childOfChild->m_entity.get() + m_child2->m_entity.get() ); EXPECT_EQ( m_root->m_entity->FindComponent()->GetHierarchicalEntities()[3], - m_child2->m_entity.get() + m_childOfChild->m_entity.get() ); EXPECT_EQ( m_root->m_entity->FindComponent()->GetHierarchicalEntities()[4], @@ -1230,4 +1241,17 @@ namespace Multiplayer 3 ); } + + TEST_F(ServerHierarchyWithThreeRoots, ReattachMiddleChildWhileLastChildGetsLeaveEventOnce) + { + m_root2->m_entity->FindComponent()->SetParent(m_childOfChild->m_entity->GetId()); + m_root3->m_entity->FindComponent()->SetParent(m_childOfChild->m_entity->GetId()); + + MockNetworkHierarchyCallbackHandler mock; + EXPECT_CALL(mock, OnNetworkHierarchyLeave()); + + m_childOfChild3->m_entity->FindComponent()->BindNetworkHierarchyLeaveEventHandler(mock.m_leaveHandler); + + m_child->m_entity->FindComponent()->SetParent(AZ::EntityId()); + } }