Commit Graph

3 Commits (4ee2f341dc0dc709aedb446b57d5cca61b86160d)

Author SHA1 Message Date
Nicholas Van Sickle 4ee2f341dc
Fix several Prefab outliner ordering issues (#5747)
* Fix several Prefab outliner ordering issues

This change does a few things to address instability in entity order when prefabs are enabled:
- Changes ordering behavior on entity add to always be "insert after the last selected sibling of the new entity, or at the end if there is no selected sibling"
- Adds some logic to ensure selection stays frozen during prefab propagation to allow this behavior to be used
- Alters delta generation in `PrefabPublicHandler::CreateEntity` to use the template instead of reserializing the DOM - this avoids a whole bunch of patching issues caused by EditorEntitySortComponent doing post-hoc order fix-up and should generally be safer/faster as we're producing patches for the actual target for those patches
- Because the duplicate action is DOM-driven, and there's some thorniness around making changes that will affect the template during propagation, this adds `PrefabPublicHandler::AddNewEntityToSortOrder` to directly patch the DOM for the duplicate case

Two bits of this come from patches from the incredibly helpful @AMZN-daimini
- Alters `PrefabPublicHandler::GenerateUndoNodesForEntityChangeAndUpdateCache` behavior for determining what's an override: we now check to see if we're part of the current focused instance but *not* owned by the focus instance directly. This lets entity order for nested prefabs get saved to the owning prefab instead of as an override. I'm putting this up for discussion without a feature flag gating it, but we may wish to make this a toggle or disable it outright for stabilization (in which case entity order won't be saved to the owning prefab)
- Adds a custom serializer for EditorEntitySortComponent. This isn't strictly necessary now, but it was very useful for debugging and ended up receiving much more manual testing its migration path and save/load path more than I've tested without using the serializer.

Signed-off-by: Nicholas Van Sickle <nvsickle@amazon.com>

* Add missing serializers

Signed-off-by: Nicholas Van Sickle <nvsickle@amazon.com>

* Address some review feedback

Signed-off-by: Nicholas Van Sickle <nvsickle@amazon.com>

* Generate patch in `PrefabPublicHandler::CreateEntity` using the instance's fully evaluated template in-memory

Signed-off-by: Nicholas Van Sickle <nvsickle@amazon.com>

* Fix up comment

Signed-off-by: Nicholas Van Sickle <nvsickle@amazon.com>

* Fix Linux build

Signed-off-by: Nicholas Van Sickle <nvsickle@amazon.com>

* Try to make test less timing dependent (haven't been able to repro failure locally)

Signed-off-by: Nicholas Van Sickle <nvsickle@amazon.com>

* Fix another build issue

Signed-off-by: Nicholas Van Sickle <nvsickle@amazon.com>

* Fix unit test failures

Signed-off-by: Nicholas Van Sickle <nvsickle@amazon.com>

* Fix a duplicate issue with sanitization that was causing sporadic test failure (thanks, test!)

Signed-off-by: Nicholas Van Sickle <nvsickle@amazon.com>

* One more Linux fix...

Signed-off-by: Nicholas Van Sickle <nvsickle@amazon.com>
4 years ago
nvsickle 52604d79f6 Address some review feedback
Signed-off-by: nvsickle <nvsickle@amazon.com>
4 years ago
nvsickle c9fb41d0e5 Fix Entity Outliner sort order with Prefabs enabled
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>
4 years ago