diff --git a/AutomatedTesting/Gem/PythonTests/editor/EditorScripts/EntityOutliner_EntityOrdering.py b/AutomatedTesting/Gem/PythonTests/editor/EditorScripts/EntityOutliner_EntityOrdering.py new file mode 100644 index 0000000000..8b1e4763db --- /dev/null +++ b/AutomatedTesting/Gem/PythonTests/editor/EditorScripts/EntityOutliner_EntityOrdering.py @@ -0,0 +1,145 @@ +""" +Copyright (c) Contributors to the Open 3D Engine Project. +For complete copyright and license terms please see the LICENSE at the root of this distribution. + +SPDX-License-Identifier: Apache-2.0 OR MIT +""" + + +class Tests: + entities_sorted = ( + "Entities sorted in the expected order", + "Entities sorted in an incorrect order", + ) + + +def EntityOutliner_EntityOrdering(): + """ + Summary: + Verify that manual entity ordering in the entity outliner works and is stable. + + Expected Behavior: + Several entities are created, some are manually ordered, and their order + is maintained, even when new entities are added. + + Test Steps: + 1) Open the empty Prefab Base level + 2) Add 5 entities to the outliner + 3) Move "Entity1" to the top of the order + 4) Move "Entity4" to the bottom of the order + 5) Add another new entity, ensure the rest of the order is unchanged + """ + + import editor_python_test_tools.pyside_utils as pyside_utils + import azlmbr.legacy.general as general + from editor_python_test_tools.utils import Report + from editor_python_test_tools.utils import TestHelper as helper + from PySide2 import QtCore, QtWidgets, QtGui, QtTest + + # Grab the Editor, Entity Outliner, and Outliner Model + editor_window = pyside_utils.get_editor_main_window() + entity_outliner = pyside_utils.find_child_by_hierarchy( + editor_window, ..., "EntityOutlinerWidgetUI", ..., "m_objectTree" + ) + entity_outliner_model = entity_outliner.model() + + # Get the outliner index for the root prefab container entity + def get_prefab_container_index(): + return entity_outliner_model.index(0, 0) + + # Get the outlienr index for the top level entity of a given name + def index_for_name(name): + root_index = get_prefab_container_index() + for row in range(entity_outliner_model.rowCount(root_index)): + row_index = entity_outliner_model.index(row, 0, root_index) + if row_index.data() == name: + return row_index + return None + + # Validate that the outliner top level entity order matches the expected order + def verify_entities_sorted(expected_order): + actual_order = [] + root_index = get_prefab_container_index() + for row in range(entity_outliner_model.rowCount(root_index)): + row_index = entity_outliner_model.index(row, 0, root_index) + actual_order.append(row_index.data()) + + sorted_correctly = actual_order == expected_order + Report.result(Tests.entities_sorted, sorted_correctly) + if not sorted_correctly: + print(f"Expected entity order: {expected_order}") + print(f"Actual entity order: {actual_order}") + + # Creates an entity from the outliner context menu + def create_entity(): + pyside_utils.trigger_context_menu_entry( + entity_outliner, "Create entity", index=get_prefab_container_index() + ) + general.idle_wait(0.0) + + # Moves an entity (wrapped by move_entity_before and move_entity_after) + def _move_entity(source_name, target_name, move_after=False): + source_index = index_for_name(source_name) + target_index = index_for_name(target_name) + + target_row = target_index.row() + if move_after: + target_row += 1 + + # Generate MIME data and directly inject it into the model instead of + # generating mouse click operations, as it's more reliable and we're + # testing the underlying drag & drop logic as opposed to Qt's mouse + # handling here + mime_data = entity_outliner_model.mimeData([source_index]) + entity_outliner_model.dropMimeData( + mime_data, QtCore.Qt.MoveAction, target_row, 0, target_index.parent() + ) + QtWidgets.QApplication.processEvents() + + # Move an entity before another entity in the order by dragging the source above the target + move_entity_before = lambda source_name, target_name: _move_entity( + source_name, target_name, move_after=False + ) + # Move an entity after another entity in the order by dragging the source beloew the target + move_entity_after = lambda source_name, target_name: _move_entity( + source_name, target_name, move_after=True + ) + + expected_order = [] + + # 1) Open the empty Prefab Base level + helper.init_idle() + helper.open_level("Prefab", "Base") + + # 2) Add 5 entities to the outliner + ENTITIES_TO_ADD = 5 + for i in range(ENTITIES_TO_ADD): + create_entity() + + # Our new entity should be given a name with a number automatically + new_entity = f"Entity{i+1}" + # The new entity should be added to the top of its parent entity + expected_order = [new_entity] + expected_order + + verify_entities_sorted(expected_order) + + # 3) Move "Entity1" to the top of the order + move_entity_before("Entity1", "Entity5") + expected_order = ["Entity1", "Entity5", "Entity4", "Entity3", "Entity2"] + verify_entities_sorted(expected_order) + + # 4) Move "Entity4" to the bottom of the order + move_entity_after("Entity4", "Entity2") + expected_order = ["Entity1", "Entity5", "Entity3", "Entity2", "Entity4"] + verify_entities_sorted(expected_order) + + # 5) Add another new entity, ensure the rest of the order is unchanged + create_entity() + expected_order = ["Entity6", "Entity1", "Entity5", "Entity3", "Entity2", "Entity4"] + verify_entities_sorted(expected_order) + + +if __name__ == "__main__": + from editor_python_test_tools.utils import Report + + Report.start_test(EntityOutliner_EntityOrdering) diff --git a/AutomatedTesting/Gem/PythonTests/editor/TestSuite_Main.py b/AutomatedTesting/Gem/PythonTests/editor/TestSuite_Main.py index 26b254ae71..49069569eb 100644 --- a/AutomatedTesting/Gem/PythonTests/editor/TestSuite_Main.py +++ b/AutomatedTesting/Gem/PythonTests/editor/TestSuite_Main.py @@ -41,3 +41,15 @@ class TestAutomation(TestAutomationBase): from .EditorScripts import BasicEditorWorkflows_LevelEntityComponentCRUD as test_module self._run_test(request, workspace, editor, test_module, batch_mode=False, autotest_mode=False, use_null_renderer=False) + + def test_EntityOutlienr_EntityOrdering(self, request, workspace, editor, launcher_platform): + from .EditorScripts import EntityOutliner_EntityOrdering as test_module + self._run_test( + request, + workspace, + editor, + test_module, + batch_mode=False, + autotest_mode=True, + extra_cmdline_args=["--regset=/Amazon/Preferences/EnablePrefabSystem=true"] + ) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/EditorEntityHelpers.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/EditorEntityHelpers.cpp index 28d6e2bc08..511f9dd580 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/EditorEntityHelpers.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/EditorEntityHelpers.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -468,6 +469,17 @@ namespace AzToolsFramework EntityIdList children; EditorEntityInfoRequestBus::EventResult(children, parentId, &EditorEntityInfoRequestBus::Events::GetChildren); + // If Prefabs are enabled, don't check the order for an invalid parent, just return its children (i.e. the root container entity) + if (!parentId.IsValid()) + { + bool isPrefabEnabled = false; + AzFramework::ApplicationRequests::Bus::BroadcastResult(isPrefabEnabled, &AzFramework::ApplicationRequests::IsPrefabSystemEnabled); + if (isPrefabEnabled) + { + return children; + } + } + EntityIdList entityChildOrder; AZ::EntityId sortEntityId = GetEntityIdForSortInfo(parentId); EditorEntitySortRequestBus::EventResult(entityChildOrder, sortEntityId, &EditorEntitySortRequestBus::Events::GetChildEntityOrderArray); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/EditorEntitySortComponent.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/EditorEntitySortComponent.cpp index b747469f4d..6936397187 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/EditorEntitySortComponent.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/EditorEntitySortComponent.cpp @@ -11,6 +11,8 @@ #include #include #include +#include +#include static_assert(sizeof(AZ::u64) == sizeof(AZ::EntityId), "We use AZ::EntityId for Persistent ID, which is a u64 under the hood. These must be the same size otherwise the persistent id will have to be rewritten"); @@ -144,6 +146,12 @@ namespace AzToolsFramework bool EditorEntitySortComponent::AddChildEntityInternal(const AZ::EntityId& entityId, bool addToBack, EntityOrderArray::iterator insertPosition) { AZ_PROFILE_FUNCTION(AzToolsFramework); + + if (m_ignoreIncomingOrderChanges) + { + return true; + } + auto entityItr = m_childEntityOrderCache.find(entityId); if (entityItr == m_childEntityOrderCache.end()) { @@ -198,6 +206,12 @@ namespace AzToolsFramework bool EditorEntitySortComponent::RemoveChildEntity(const AZ::EntityId& entityId) { AZ_PROFILE_FUNCTION(AzToolsFramework); + + if (m_ignoreIncomingOrderChanges) + { + return true; + } + auto entityItr = m_childEntityOrderCache.find(entityId); if (entityItr != m_childEntityOrderCache.end()) { @@ -250,11 +264,30 @@ namespace AzToolsFramework } } + void EditorEntitySortComponent::OnPrefabInstancePropagationBegin() + { + m_ignoreIncomingOrderChanges = true; + } + + void EditorEntitySortComponent::OnPrefabInstancePropagationEnd() + { + m_ignoreIncomingOrderChanges = false; + } + void EditorEntitySortComponent::MarkDirtyAndSendChangedEvent() { // mark the order as dirty before sending the ChildEntityOrderArrayUpdated event in order for PrepareSave to be properly handled in the case // one of the event listeners needs to build the InstanceDataHierarchy m_entityOrderIsDirty = true; + + // Force an immediate update for prefabs, which won't receive PrepareSave + bool isPrefabEnabled = false; + AzFramework::ApplicationRequests::Bus::BroadcastResult( + isPrefabEnabled, &AzFramework::ApplicationRequests::IsPrefabSystemEnabled); + if (isPrefabEnabled) + { + PrepareSave(); + } EditorEntitySortNotificationBus::Event(GetEntityId(), &EditorEntitySortNotificationBus::Events::ChildEntityOrderArrayUpdated); } @@ -264,10 +297,20 @@ namespace AzToolsFramework // This is a special case for certain EditorComponents only! EditorEntitySortRequestBus::Handler::BusConnect(GetEntityId()); EditorEntityContextNotificationBus::Handler::BusConnect(); + AzToolsFramework::Prefab::PrefabPublicNotificationBus::Handler::BusConnect(); } void EditorEntitySortComponent::Activate() { + // Run the post-serialize handler if prefabs are enabled because PostLoad won't be called automatically + bool isPrefabEnabled = false; + AzFramework::ApplicationRequests::Bus::BroadcastResult( + isPrefabEnabled, &AzFramework::ApplicationRequests::IsPrefabSystemEnabled); + if (isPrefabEnabled) + { + PostLoad(); + } + // Send out that the order for our entity is now updated EditorEntitySortNotificationBus::Event(GetEntityId(), &EditorEntitySortNotificationBus::Events::ChildEntityOrderArrayUpdated); } diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/EditorEntitySortComponent.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/EditorEntitySortComponent.h index d728191c64..806e903c96 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/EditorEntitySortComponent.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/EditorEntitySortComponent.h @@ -10,6 +10,7 @@ #include "EditorEntitySortBus.h" #include #include +#include #include namespace AzToolsFramework @@ -20,6 +21,7 @@ namespace AzToolsFramework : public AzToolsFramework::Components::EditorComponentBase , public EditorEntitySortRequestBus::Handler , public EditorEntityContextNotificationBus::Handler + , public AzToolsFramework::Prefab::PrefabPublicNotificationBus::Handler { public: AZ_COMPONENT(EditorEntitySortComponent, "{6EA1E03D-68B2-466D-97F7-83998C8C27F0}", EditorComponentBase); @@ -45,6 +47,9 @@ namespace AzToolsFramework // EditorEntityContextNotificationBus::Handler void OnEntityStreamLoadSuccess() override; ////////////////////////////////////////////////////////////////////////// + + void OnPrefabInstancePropagationBegin() override; + void OnPrefabInstancePropagationEnd() override; private: void MarkDirtyAndSendChangedEvent(); bool AddChildEntityInternal(const AZ::EntityId& entityId, bool addToBack, EntityOrderArray::iterator insertPosition); @@ -106,6 +111,7 @@ namespace AzToolsFramework EntityOrderCache m_childEntityOrderCache; ///< The map of entity id to index for quick look up bool m_entityOrderIsDirty = true; ///< This flag indicates our stored serialization order data is out of date and must be rebuilt before serialization occurs + bool m_ignoreIncomingOrderChanges = false; ///< This is set when prefab propagation occurs so that non-authored order changes can be ignored }; } } // namespace AzToolsFramework