From 3fd2f1305f2f8c6de2982570763efd37783264f2 Mon Sep 17 00:00:00 2001 From: rhongAMZ <69218254+rhongAMZ@users.noreply.github.com> Date: Wed, 9 Jun 2021 09:15:09 -0700 Subject: [PATCH] Selecting and deleting the level prefab root entity crashes editor (#1179) Early remove the level instance from the entity id list in the delete function and duplicate function. --- .../Prefab/PrefabPublicHandler.cpp | 40 +++++++++++++++---- .../Prefab/PrefabPublicHandler.h | 1 + 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp index fcdbc5ce07..690d408007 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp @@ -901,7 +901,13 @@ namespace AzToolsFramework return AZ::Failure(AZStd::string("No entities to duplicate.")); } - if (!EntitiesBelongToSameInstance(entityIds)) + const EntityIdList entityIdsNoLevelInstance = GenerateEntityIdListWithoutLevelInstance(entityIds); + if (entityIdsNoLevelInstance.empty()) + { + return AZ::Failure(AZStd::string("No entities to duplicate because only instance selected is the level instance.")); + } + + if (!EntitiesBelongToSameInstance(entityIdsNoLevelInstance)) { return AZ::Failure(AZStd::string("Cannot duplicate multiple entities belonging to different instances with one operation." "Change your selection to contain entities in the same instance.")); @@ -909,7 +915,7 @@ namespace AzToolsFramework // We've already verified the entities are all owned by the same instance, // so we can just retrieve our instance from the first entity in the list. - AZ::EntityId firstEntityIdToDuplicate = entityIds[0]; + AZ::EntityId firstEntityIdToDuplicate = entityIdsNoLevelInstance[0]; InstanceOptionalReference commonOwningInstance = GetOwnerInstanceByEntityId(firstEntityIdToDuplicate); if (!commonOwningInstance.has_value()) { @@ -929,7 +935,7 @@ namespace AzToolsFramework // This will cull out any entities that have ancestors in the list, since we will end up duplicating // the full nested hierarchy with what is returned from RetrieveAndSortPrefabEntitiesAndInstances - AzToolsFramework::EntityIdSet duplicationSet = AzToolsFramework::GetCulledEntityHierarchy(entityIds); + AzToolsFramework::EntityIdSet duplicationSet = AzToolsFramework::GetCulledEntityHierarchy(entityIdsNoLevelInstance); AZ_PROFILE_FUNCTION(AZ::Debug::ProfileCategory::AzToolsFramework); @@ -1004,17 +1010,19 @@ namespace AzToolsFramework PrefabOperationResult PrefabPublicHandler::DeleteFromInstance(const EntityIdList& entityIds, bool deleteDescendants) { - if (entityIds.empty()) + const EntityIdList entityIdsNoLevelInstance = GenerateEntityIdListWithoutLevelInstance(entityIds); + + if (entityIdsNoLevelInstance.empty()) { return AZ::Success(); } - if (!EntitiesBelongToSameInstance(entityIds)) + if (!EntitiesBelongToSameInstance(entityIdsNoLevelInstance)) { return AZ::Failure(AZStd::string("Cannot delete multiple entities belonging to different instances with one operation.")); } - AZ::EntityId firstEntityIdToDelete = entityIds[0]; + AZ::EntityId firstEntityIdToDelete = entityIdsNoLevelInstance[0]; InstanceOptionalReference commonOwningInstance = GetOwnerInstanceByEntityId(firstEntityIdToDelete); // If the first entity id is a container entity id, then we need to mark its parent as the common owning instance because you @@ -1025,7 +1033,7 @@ namespace AzToolsFramework } // Retrieve entityList from entityIds - EntityList inputEntityList = EntityIdListToEntityList(entityIds); + EntityList inputEntityList = EntityIdListToEntityList(entityIdsNoLevelInstance); AZ_PROFILE_FUNCTION(AZ::Debug::ProfileCategory::AzToolsFramework); @@ -1081,7 +1089,7 @@ namespace AzToolsFramework } else { - for (AZ::EntityId entityId : entityIds) + for (AZ::EntityId entityId : entityIdsNoLevelInstance) { InstanceOptionalReference owningInstance = m_instanceEntityMapperInterface->FindOwningInstance(entityId); // If this is the container entity, it actually represents the instance so get its owner @@ -1437,6 +1445,22 @@ namespace AzToolsFramework return (outEntities.size() + outInstances.size()) > 0; } + EntityIdList PrefabPublicHandler::GenerateEntityIdListWithoutLevelInstance( + const EntityIdList& entityIds) const + { + EntityIdList outEntityIds; + outEntityIds.reserve(entityIds.size()); // Actual size could be smaller. + + for (const AZ::EntityId& entityId : entityIds) + { + if (!IsLevelInstanceContainerEntity(entityId)) + { + outEntityIds.emplace_back(entityId); + } + } + return outEntityIds; + } + bool PrefabPublicHandler::EntitiesBelongToSameInstance(const EntityIdList& entityIds) const { if (entityIds.size() <= 1) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h index f3d778d8de..65e1391722 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h @@ -70,6 +70,7 @@ namespace AzToolsFramework PrefabOperationResult DeleteFromInstance(const EntityIdList& entityIds, bool deleteDescendants); bool RetrieveAndSortPrefabEntitiesAndInstances(const EntityList& inputEntities, Instance& commonRootEntityOwningInstance, EntityList& outEntities, AZStd::vector& outInstances) const; + EntityIdList GenerateEntityIdListWithoutLevelInstance(const EntityIdList& entityIds) const; InstanceOptionalReference GetOwnerInstanceByEntityId(AZ::EntityId entityId) const; bool EntitiesBelongToSameInstance(const EntityIdList& entityIds) const;