Network Hierarchy optimizations for rebuilding hierarchies

- reworked recursive rebuilding to iterative breadth first method - some minor optimization here and there
2021-10-20 13:50:35 -04:00
parent 900aa4e5bc 9e4a39f9ed
commit 9e2eba2829
5 changed files with 124 additions and 100 deletions
@@ -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;
@@ -82,22 +82,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;

@@ -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<AzFramework::TransformComponent>())
        {
            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<NetworkHierarchyChildComponentController*>(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<NetworkHierarchyChildComponentController*>(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<NetworkHierarchyRootComponent>())
-            {
-                root->RebuildHierarchy();
-            }
-        }
-    }
-
    void NetworkHierarchyChildComponent::OnHierarchyRootNetIdChanged(NetEntityId rootNetId)
    {
        ConstNetworkEntityHandle rootHandle = GetNetworkEntityManager()->GetEntity(rootNetId);
@@ -202,11 +193,13 @@ namespace Multiplayer

    void NetworkHierarchyChildComponent::NotifyChildrenHierarchyDisbanded()
    {
+        AZ::ComponentApplicationRequests* componentApplication = AZ::Interface<AZ::ComponentApplicationRequests>::Get();
+
        AZStd::vector<AZ::EntityId> allChildren;
        AZ::TransformBus::EventResult(allChildren, GetEntityId(), &AZ::TransformBus::Events::GetChildren);
        for (const AZ::EntityId& childEntityId : allChildren)
        {
-            if (const AZ::Entity* childEntity = AZ::Interface<AZ::ComponentApplicationRequests>::Get()->FindEntity(childEntityId))
+            if (const AZ::Entity* childEntity = componentApplication->FindEntity(childEntityId))
            {
                if (auto* hierarchyChildComponent = childEntity->FindComponent<NetworkHierarchyChildComponent>())
                {
@@ -20,6 +20,8 @@
 AZ_CVAR(uint32_t, bg_hierarchyEntityMaxLimit, 16, nullptr, AZ::ConsoleFunctorFlags::Null,
    "Maximum allowed size of network entity hierarchies, including top level entity.");

+static constexpr int CommonHierarchyEntityMaxLimit = 16; // Should match @bg_hierarchyEntityMaxLimit
+
 namespace Multiplayer
 {
    void NetworkHierarchyRootComponent::Reflect(AZ::ReflectContext* context)
@@ -173,6 +175,29 @@ namespace Multiplayer
        }
    }

+    static AZStd::tuple<NetworkHierarchyRootComponent*, NetworkHierarchyChildComponent*> GetHierarchyComponents(const AZ::Entity* entity)
+    {
+        NetworkHierarchyChildComponent* childComponent = nullptr;
+        NetworkHierarchyRootComponent* rootComponent = nullptr;
+
+        for (AZ::Component* component : entity->GetComponents())
+        {
+            if (component->GetUnderlyingComponentType() == NetworkHierarchyChildComponent::TYPEINFO_Uuid())
+            {
+                childComponent = static_cast<NetworkHierarchyChildComponent*>(component);
+                break;
+            }
+
+            if (component->GetUnderlyingComponentType() == NetworkHierarchyRootComponent::TYPEINFO_Uuid())
+            {
+                rootComponent = static_cast<NetworkHierarchyRootComponent*>(component);
+                break;
+            }
+        }
+
+        return AZStd::tie(rootComponent, childComponent);
+    }
+
    void NetworkHierarchyRootComponent::OnParentChanged([[maybe_unused]] AZ::EntityId oldParent, AZ::EntityId newParent)
    {
        // If the parent is part of a hierarchy, it will detect this entity as a new child and rebuild hierarchy.
@@ -181,8 +206,8 @@ namespace Multiplayer

        if (AZ::Entity* parentEntity = AZ::Interface<AZ::ComponentApplicationRequests>::Get()->FindEntity(newParent))
        {
-            if (parentEntity->FindComponent<NetworkHierarchyRootComponent>() == nullptr &&
-                parentEntity->FindComponent<NetworkHierarchyChildComponent>() == nullptr)
+            auto [rootComponent, childComponent] = GetHierarchyComponents(parentEntity);
+            if (rootComponent == nullptr && childComponent == nullptr)
            {
                RebuildHierarchy();
            }
@@ -203,10 +228,9 @@ namespace Multiplayer
        AZStd::vector<AZ::Entity*> previousEntities;
        m_hierarchicalEntities.swap(previousEntities);

-        m_hierarchicalEntities.push_back(GetEntity()); // Add the root.
+        m_hierarchicalEntities.reserve(bg_hierarchyEntityMaxLimit);

-        uint32_t currentEntityCount = aznumeric_cast<uint32_t>(m_hierarchicalEntities.size());
-        RecursiveAttachHierarchicalEntities(GetEntityId(), currentEntityCount);
+        InternalBuildHierarchyList(GetEntity());

        bool hierarchyChanged = false;

@@ -244,62 +268,57 @@ namespace Multiplayer
        }
    }

-    void NetworkHierarchyRootComponent::SetRootForEntity(AZ::Entity* root, const AZ::Entity* childEntity)
+    void NetworkHierarchyRootComponent::InternalBuildHierarchyList(AZ::Entity* underEntity)
    {
-        if (auto* hierarchyChildComponent = childEntity->FindComponent<NetworkHierarchyChildComponent>())
-        {
-            hierarchyChildComponent->SetTopLevelHierarchyRootEntity(root);
-        }
-        else if (auto* hierarchyRootComponent = childEntity->FindComponent<NetworkHierarchyRootComponent>())
-        {
-            hierarchyRootComponent->SetTopLevelHierarchyRootEntity(root);
-        }
-    }
+        AZ::ComponentApplicationRequests* componentApplicationRequests = AZ::Interface<AZ::ComponentApplicationRequests>::Get();

-    bool NetworkHierarchyRootComponent::RecursiveAttachHierarchicalEntities(AZ::EntityId underEntity, uint32_t& currentEntityCount)
-    {
-        AZStd::vector<AZ::EntityId> allChildren;
-        AZ::TransformBus::EventResult(allChildren, underEntity, &AZ::TransformBus::Events::GetChildren);
+        AZStd::deque<AZ::Entity*, AZStd::allocator, CommonHierarchyEntityMaxLimit> candidates;
+        candidates.push_back(underEntity);

-        for (const AZ::EntityId& newChildId : allChildren)
+        while (!candidates.empty())
        {
-            if (!RecursiveAttachHierarchicalChild(newChildId, currentEntityCount))
+            AZ::Entity* candidate = candidates.front();
+            candidates.pop_front();
+
+            if (candidate)
            {
-                return false;
-            }
-        }
+                auto [hierarchyRootComponent, hierarchyChildComponent] = GetHierarchyComponents(candidate);

-        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<uint32_t>(bg_hierarchyEntityMaxLimit));
-            return false;
-        }
-
-        if (AZ::Entity* childEntity = AZ::Interface<AZ::ComponentApplicationRequests>::Get()->FindEntity(entity))
-        {
-            auto* hierarchyChildComponent = childEntity->FindComponent<NetworkHierarchyChildComponent>();
-            auto* hierarchyRootComponent = childEntity->FindComponent<NetworkHierarchyRootComponent>();
-
-            if ((hierarchyChildComponent && hierarchyChildComponent->IsHierarchyEnabled()) ||
-                (hierarchyRootComponent && hierarchyRootComponent->IsHierarchyEnabled()))
-            {
-                m_hierarchicalEntities.push_back(childEntity);
-                ++currentEntityCount;
-
-                if (!RecursiveAttachHierarchicalEntities(entity, currentEntityCount))
+                if ((hierarchyChildComponent && hierarchyChildComponent->IsHierarchyEnabled()) ||
+                    (hierarchyRootComponent && hierarchyRootComponent->IsHierarchyEnabled()))
                {
-                    return false;
+                    m_hierarchicalEntities.push_back(candidate);
+
+                    if (m_hierarchicalEntities.size() >= bg_hierarchyEntityMaxLimit)
+                    {
+                        AZLOG_WARN("Network hierarchy size exceeded, current limit is %d, root entity was %s",
+                            static_cast<int>(bg_hierarchyEntityMaxLimit),
+                            GetEntity()->GetName().c_str());
+                        return;
+                    }
+
+                    const AZStd::vector<AZ::EntityId> allChildren = candidate->GetTransform()->GetChildren();
+                    for (const AZ::EntityId& newChildId : allChildren)
+                    {
+                        candidates.push_back(componentApplicationRequests->FindEntity(newChildId));
+                    }
                }
            }
        }
+    }

-        return true;
+    void NetworkHierarchyRootComponent::SetRootForEntity(AZ::Entity* root, const AZ::Entity* childEntity)
+    {
+        auto [hierarchyRootComponent, hierarchyChildComponent] = GetHierarchyComponents(childEntity);
+
+        if (hierarchyChildComponent)
+        {
+            hierarchyChildComponent->SetTopLevelHierarchyRootEntity(root);
+        }
+        else if (hierarchyRootComponent)
+        {
+            hierarchyRootComponent->SetTopLevelHierarchyRootEntity(root);
+        }
    }

    void NetworkHierarchyRootComponent::SetTopLevelHierarchyRootEntity(AZ::Entity* hierarchyRoot)
@@ -394,8 +394,8 @@ namespace Multiplayer
        m_console->PerformCommand("bg_hierarchyEntityMaxLimit 2");

        // remake the hierarchy
-        m_root->m_entity->FindComponent<AzFramework::TransformComponent>()->SetParent(AZ::EntityId());
-        m_root->m_entity->FindComponent<AzFramework::TransformComponent>()->SetParent(m_root->m_entity->GetId());
+        m_child->m_entity->FindComponent<AzFramework::TransformComponent>()->SetParent(AZ::EntityId());
+        m_child->m_entity->FindComponent<AzFramework::TransformComponent>()->SetParent(m_root->m_entity->GetId());

        EXPECT_EQ(
            m_root->m_entity->FindComponent<NetworkHierarchyRootComponent>()->GetHierarchicalEntities().size(),
@@ -406,6 +406,17 @@ namespace Multiplayer
        m_console->GetCvarValue<uint32_t>("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<NetworkHierarchyRootComponent>()->BindNetworkHierarchyChangedEventHandler(mock.m_changedHandler);
+
+        m_child->m_entity->FindComponent<AzFramework::TransformComponent>()->SetParent(AZ::EntityId());
+        m_child->m_entity->FindComponent<AzFramework::TransformComponent>()->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<NetworkHierarchyRootComponent>()->GetHierarchicalEntities()[2],
-                m_childOfChild->m_entity.get()
+                m_child2->m_entity.get()
            );
            EXPECT_EQ(
                m_root->m_entity->FindComponent<NetworkHierarchyRootComponent>()->GetHierarchicalEntities()[3],
-                m_child2->m_entity.get()
+                m_childOfChild->m_entity.get()
            );
            EXPECT_EQ(
                m_root->m_entity->FindComponent<NetworkHierarchyRootComponent>()->GetHierarchicalEntities()[4],
@@ -1230,4 +1241,17 @@ namespace Multiplayer
            3
        );
    }
+
+    TEST_F(ServerHierarchyWithThreeRoots, ReattachMiddleChildWhileLastChildGetsLeaveEventOnce)
+    {
+        m_root2->m_entity->FindComponent<AzFramework::TransformComponent>()->SetParent(m_childOfChild->m_entity->GetId());
+        m_root3->m_entity->FindComponent<AzFramework::TransformComponent>()->SetParent(m_childOfChild->m_entity->GetId());
+
+        MockNetworkHierarchyCallbackHandler mock;
+        EXPECT_CALL(mock, OnNetworkHierarchyLeave());
+        
+        m_childOfChild3->m_entity->FindComponent<NetworkHierarchyChildComponent>()->BindNetworkHierarchyLeaveEventHandler(mock.m_leaveHandler);
+
+        m_child->m_entity->FindComponent<AzFramework::TransformComponent>()->SetParent(AZ::EntityId());
+    }
 }