birkeh/o3de
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix Entity Outliner sort order with Prefabs enabled

Browse Source 
The EditorEntitySortComponent was relying on serialization callbacks not exposed to the JSON serializer to marshal data between its serialized state and its runtime state, which led to the outliner sort order becoming disrupted every time a prefab propagation occurs (and the component is subsequently serialized and deserialized). This change:
- Forces `PrepareSave` and `PostLoad` for `EditorEntitySortComponent` to be called at update (direct descendant added or removed) and activation time respectively while prefabs are enabled. While this could be optimized, and this logic stands to be refactored once slices are fully removed, I was unable to gather any samples in which `PrepareSave` are called in a sampling profiler in a scene with 1000 top-level entities, so I don't anticipate this introducing a meaningful performance regression in the short term.
- Disables updates in `EditorEntitySortComponent` during prefab propagation, as any detected changes do not signal authored intent at this time
- Made `GetEntityChildOrder` in `EditorEntityHelpers` work with prefabs enabled, which restores the existing "Sort: A -> Z" and "Sort: Z -> A" behaviors (which have some preexisting issues this does not fix)
- Adds a Python regression test to the Editor suite to validate this behavior - the test is currently in TestSuite_Main and not TestSuite_Main_Optimized because it requires an Editor launch with prefabs enabled, this can be fixed once AutomatedTesting is further migrated away from slices

@AMZN-daimini has a larger change that improves the JSON serialization format (https://github.com/o3de/o3de/pull/1292) which we should absolutely bring in in the future to improve the legibility of the Prefab format, but this change fixes the functionality (including saving & reloading a level and keeping a consistent order) without altering the Prefab format - this lower impact radius fix is my preference for our stabilization period.

Signed-off-by: nvsickle <nvsickle@amazon.com>
This commit is contained in:
nvsickle
2021-11-03 19:57:51 -07:00
parent 47b7ad1ec4
commit c9fb41d0e5
5 changed files with 218 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Code/Framework/AzToolsFramework/AzToolsFramework/Entity/EditorEntityHelpers.cpp
+12
View File
@@ -15,6 +15,7 @@
#include <AzCore/std/sort.h>
#include <AzCore/RTTI/AttributeReader.h>
#include <AzCore/Serialization/SerializeContext.h>
#include <AzFramework/API/ApplicationAPI.h>
#include <AzToolsFramework/Commands/EntityStateCommand.h>
#include <AzToolsFramework/ContainerEntity/ContainerEntityInterface.h>
#include <AzToolsFramework/Entity/EditorEntityInfoBus.h>
@@ -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);
Code/Framework/AzToolsFramework/AzToolsFramework/Entity/EditorEntitySortComponent.cpp
+43
View File
@@ -11,6 +11,8 @@
#include <AzCore/Debug/Profiler.h>
#include <AzCore/Serialization/EditContext.h>
#include <AzCore/std/sort.h>
#include <AzFramework/API/ApplicationAPI.h>
#include <AzToolsFramework/Prefab/PrefabPublicInterface.h>
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);
}
Code/Framework/AzToolsFramework/AzToolsFramework/Entity/EditorEntitySortComponent.h
+6
View File
@@ -10,6 +10,7 @@
#include "EditorEntitySortBus.h"
#include <AzToolsFramework/ToolsComponents/EditorComponentBase.h>
#include <AzToolsFramework/Entity/EditorEntityContextBus.h>
#include <AzToolsFramework/Prefab/PrefabPublicNotificationBus.h>
#include <AzCore/Serialization/SerializeContext.h>
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