From bcf3980de6295a1cb522f20e8482c3222625542c Mon Sep 17 00:00:00 2001 From: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com> Date: Thu, 14 Oct 2021 15:32:34 -0700 Subject: [PATCH] LYN-7191 + LYN-7194 | Adjust Prefab operations to conform with Prefab Focus/Edit workflows. (#4684) * Disable ability to delete container entity of focused prefab. Default entity creation to parent to container entity of focused prefab. Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com> * Disable detach and duplicate operations for the container of the focused prefab. Update the context menu accordingly. Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com> * Fix spacing Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com> * Address minor issues from PR (error message, optimization in RetrieveAndSortPrefabEntitiesAndInstances). Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com> --- .../Prefab/PrefabPublicHandler.cpp | 76 +++++++++++++------ .../Prefab/PrefabPublicHandler.h | 4 +- .../UI/Prefab/PrefabIntegrationManager.cpp | 56 +++++++++----- .../UI/Prefab/PrefabUiHandler.cpp | 6 -- 4 files changed, 91 insertions(+), 51 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp index b5f61b33bf..081655a166 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp @@ -43,6 +43,12 @@ namespace AzToolsFramework m_instanceToTemplateInterface = AZ::Interface::Get(); AZ_Assert(m_instanceToTemplateInterface, "PrefabPublicHandler - Could not retrieve instance of InstanceToTemplateInterface"); + m_prefabFocusInterface = AZ::Interface::Get(); + AZ_Assert(m_prefabFocusInterface, "Could not get PrefabFocusInterface on PrefabPublicHandler construction."); + + m_prefabFocusPublicInterface = AZ::Interface::Get(); + AZ_Assert(m_prefabFocusPublicInterface, "Could not get PrefabFocusPublicInterface on PrefabPublicHandler construction."); + m_prefabLoaderInterface = AZ::Interface::Get(); AZ_Assert(m_prefabLoaderInterface, "Could not get PrefabLoaderInterface on PrefabPublicHandler construction."); @@ -552,6 +558,13 @@ namespace AzToolsFramework PrefabEntityResult PrefabPublicHandler::CreateEntity(AZ::EntityId parentId, const AZ::Vector3& position) { + // If the parent is invalid, parent to the container of the currently focused prefab. + if (!parentId.IsValid()) + { + AzFramework::EntityContextId editorEntityContextId = AzToolsFramework::GetEntityContextId(); + parentId = m_prefabFocusPublicInterface->GetFocusedPrefabContainerEntityId(editorEntityContextId); + } + InstanceOptionalReference owningInstanceOfParentEntity = GetOwnerInstanceByEntityId(parentId); if (!owningInstanceOfParentEntity) { @@ -968,13 +981,13 @@ namespace AzToolsFramework return AZ::Failure(AZStd::string("No entities to duplicate.")); } - const EntityIdList entityIdsNoLevelInstance = GenerateEntityIdListWithoutLevelInstance(entityIds); - if (entityIdsNoLevelInstance.empty()) + const EntityIdList entityIdsNoFocusContainer = GenerateEntityIdListWithoutFocusedInstanceContainer(entityIds); + if (entityIdsNoFocusContainer.empty()) { - return AZ::Failure(AZStd::string("No entities to duplicate because only instance selected is the level instance.")); + return AZ::Failure(AZStd::string("No entities to duplicate because only instance selected is the container entity of the focused instance.")); } - if (!EntitiesBelongToSameInstance(entityIdsNoLevelInstance)) + if (!EntitiesBelongToSameInstance(entityIdsNoFocusContainer)) { 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.")); @@ -982,7 +995,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 = entityIdsNoLevelInstance[0]; + AZ::EntityId firstEntityIdToDuplicate = entityIdsNoFocusContainer[0]; InstanceOptionalReference commonOwningInstance = GetOwnerInstanceByEntityId(firstEntityIdToDuplicate); if (!commonOwningInstance.has_value()) { @@ -1002,7 +1015,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(entityIdsNoLevelInstance); + AzToolsFramework::EntityIdSet duplicationSet = AzToolsFramework::GetCulledEntityHierarchy(entityIdsNoFocusContainer); AZ_PROFILE_FUNCTION(AzToolsFramework); @@ -1106,19 +1119,21 @@ namespace AzToolsFramework PrefabOperationResult PrefabPublicHandler::DeleteFromInstance(const EntityIdList& entityIds, bool deleteDescendants) { - const EntityIdList entityIdsNoLevelInstance = GenerateEntityIdListWithoutLevelInstance(entityIds); + // Remove the container entity of the focused prefab from the list, if it is included. + const EntityIdList entityIdsNoFocusContainer = GenerateEntityIdListWithoutFocusedInstanceContainer(entityIds); - if (entityIdsNoLevelInstance.empty()) + if (entityIdsNoFocusContainer.empty()) { return AZ::Success(); } - if (!EntitiesBelongToSameInstance(entityIdsNoLevelInstance)) + // All entities in this list need to belong to the same prefab instance for the operation to be valid. + if (!EntitiesBelongToSameInstance(entityIdsNoFocusContainer)) { return AZ::Failure(AZStd::string("Cannot delete multiple entities belonging to different instances with one operation.")); } - AZ::EntityId firstEntityIdToDelete = entityIdsNoLevelInstance[0]; + AZ::EntityId firstEntityIdToDelete = entityIdsNoFocusContainer[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 @@ -1128,8 +1143,15 @@ namespace AzToolsFramework commonOwningInstance = commonOwningInstance->get().GetParentInstance(); } + // We only allow explicit deletions for entities inside the currently focused prefab. + AzFramework::EntityContextId editorEntityContextId = AzToolsFramework::GetEntityContextId(); + if (&m_prefabFocusInterface->GetFocusedPrefabInstance(editorEntityContextId)->get() != &commonOwningInstance->get()) + { + return AZ::Failure(AZStd::string("Cannot delete entities belonging to an instance that is not being edited.")); + } + // Retrieve entityList from entityIds - EntityList inputEntityList = EntityIdListToEntityList(entityIdsNoLevelInstance); + EntityList inputEntityList = EntityIdListToEntityList(entityIdsNoFocusContainer); AZ_PROFILE_FUNCTION(AzToolsFramework); @@ -1186,7 +1208,7 @@ namespace AzToolsFramework } else { - for (AZ::EntityId entityId : entityIdsNoLevelInstance) + for (AZ::EntityId entityId : entityIdsNoFocusContainer) { InstanceOptionalReference owningInstance = m_instanceEntityMapperInterface->FindOwningInstance(entityId); // If this is the container entity, it actually represents the instance so get its owner @@ -1227,9 +1249,12 @@ namespace AzToolsFramework return AZ::Failure(AZStd::string("Cannot detach Prefab Instance with invalid container entity.")); } - if (IsLevelInstanceContainerEntity(containerEntityId)) + auto editorEntityContextId = AzFramework::EntityContextId::CreateNull(); + EditorEntityContextRequestBus::BroadcastResult(editorEntityContextId, &EditorEntityContextRequests::GetEditorEntityContextId); + + if (containerEntityId == m_prefabFocusPublicInterface->GetFocusedPrefabContainerEntityId(editorEntityContextId)) { - return AZ::Failure(AZStd::string("Cannot detach level Prefab Instance.")); + return AZ::Failure(AZStd::string("Cannot detach focused Prefab Instance.")); } InstanceOptionalReference owningInstance = GetOwnerInstanceByEntityId(containerEntityId); @@ -1452,9 +1477,14 @@ namespace AzToolsFramework AZStd::queue entityQueue; + auto editorEntityContextId = AzFramework::EntityContextId::CreateNull(); + EditorEntityContextRequestBus::BroadcastResult(editorEntityContextId, &EditorEntityContextRequests::GetEditorEntityContextId); + + AZ::EntityId focusedPrefabContainerEntityId = + m_prefabFocusPublicInterface->GetFocusedPrefabContainerEntityId(editorEntityContextId); for (auto inputEntity : inputEntities) { - if (inputEntity && !IsLevelInstanceContainerEntity(inputEntity->GetId())) + if (inputEntity && inputEntity->GetId() != focusedPrefabContainerEntityId) { entityQueue.push(inputEntity); } @@ -1548,19 +1578,19 @@ namespace AzToolsFramework return AZ::Success(); } - EntityIdList PrefabPublicHandler::GenerateEntityIdListWithoutLevelInstance( + EntityIdList PrefabPublicHandler::GenerateEntityIdListWithoutFocusedInstanceContainer( const EntityIdList& entityIds) const { - EntityIdList outEntityIds; - outEntityIds.reserve(entityIds.size()); // Actual size could be smaller. + EntityIdList outEntityIds(entityIds); - for (const AZ::EntityId& entityId : entityIds) + AzFramework::EntityContextId editorEntityContextId = AzToolsFramework::GetEntityContextId(); + AZ::EntityId focusedInstanceContainerEntityId = m_prefabFocusPublicInterface->GetFocusedPrefabContainerEntityId(editorEntityContextId); + + if (auto iter = AZStd::find(outEntityIds.begin(), outEntityIds.end(), focusedInstanceContainerEntityId); iter != outEntityIds.end()) { - if (!IsLevelInstanceContainerEntity(entityId)) - { - outEntityIds.emplace_back(entityId); - } + outEntityIds.erase(iter); } + return outEntityIds; } diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h index f3c0e67d46..a9dadc3336 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h @@ -74,7 +74,7 @@ namespace AzToolsFramework Instance& commonRootEntityOwningInstance, EntityList& outEntities, AZStd::vector& outInstances) const; - EntityIdList GenerateEntityIdListWithoutLevelInstance(const EntityIdList& entityIds) const; + EntityIdList GenerateEntityIdListWithoutFocusedInstanceContainer(const EntityIdList& entityIds) const; InstanceOptionalReference GetOwnerInstanceByEntityId(AZ::EntityId entityId) const; bool EntitiesBelongToSameInstance(const EntityIdList& entityIds) const; @@ -187,6 +187,8 @@ namespace AzToolsFramework InstanceEntityMapperInterface* m_instanceEntityMapperInterface = nullptr; InstanceToTemplateInterface* m_instanceToTemplateInterface = nullptr; + PrefabFocusInterface* m_prefabFocusInterface = nullptr; + PrefabFocusPublicInterface* m_prefabFocusPublicInterface = nullptr; PrefabLoaderInterface* m_prefabLoaderInterface = nullptr; PrefabSystemComponentInterface* m_prefabSystemComponentInterface = nullptr; diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Prefab/PrefabIntegrationManager.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Prefab/PrefabIntegrationManager.cpp index 6ffa3aab69..16e3c047b5 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Prefab/PrefabIntegrationManager.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Prefab/PrefabIntegrationManager.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -175,12 +176,16 @@ namespace AzToolsFramework AzFramework::ApplicationRequests::Bus::BroadcastResult( prefabWipFeaturesEnabled, &AzFramework::ApplicationRequests::ArePrefabWipFeaturesEnabled); + auto editorEntityContextId = AzFramework::EntityContextId::CreateNull(); + EditorEntityContextRequestBus::BroadcastResult(editorEntityContextId, &EditorEntityContextRequests::GetEditorEntityContextId); + // Create Prefab { if (!selectedEntities.empty()) { - // Hide if the only selected entity is the Level Container - if (selectedEntities.size() > 1 || !s_prefabPublicInterface->IsLevelInstanceContainerEntity(selectedEntities[0])) + // Hide if the only selected entity is the Focused Instance Container + if (selectedEntities.size() > 1 || + selectedEntities[0] != s_prefabFocusPublicInterface->GetFocusedPrefabContainerEntityId(editorEntityContextId)) { bool layerInSelection = false; @@ -247,14 +252,14 @@ namespace AzToolsFramework // Edit Prefab if (prefabWipFeaturesEnabled && !s_prefabFocusPublicInterface->IsOwningPrefabBeingFocused(selectedEntity)) { - QAction* editAction = menu->addAction(QObject::tr("Edit Prefab")); - editAction->setToolTip(QObject::tr("Edit the prefab in focus mode.")); + QAction* editAction = menu->addAction(QObject::tr("Edit Prefab")); + editAction->setToolTip(QObject::tr("Edit the prefab in focus mode.")); - QObject::connect(editAction, &QAction::triggered, editAction, [selectedEntity] { - ContextMenu_EditPrefab(selectedEntity); - }); + QObject::connect(editAction, &QAction::triggered, editAction, [selectedEntity] { + ContextMenu_EditPrefab(selectedEntity); + }); - itemWasShown = true; + itemWasShown = true; } // Save Prefab @@ -283,8 +288,9 @@ namespace AzToolsFramework QAction* deleteAction = menu->addAction(QObject::tr("Delete")); QObject::connect(deleteAction, &QAction::triggered, deleteAction, [] { ContextMenu_DeleteSelected(); }); - if (selectedEntities.size() == 0 || - (selectedEntities.size() == 1 && s_prefabPublicInterface->IsLevelInstanceContainerEntity(selectedEntities[0]))) + + if (selectedEntities.empty() || + (selectedEntities.size() == 1 && selectedEntities[0] == s_prefabFocusPublicInterface->GetFocusedPrefabContainerEntityId(editorEntityContextId))) { deleteAction->setDisabled(true); } @@ -292,17 +298,17 @@ namespace AzToolsFramework // Detach Prefab if (selectedEntities.size() == 1) { - AZ::EntityId selectedEntity = selectedEntities[0]; + AZ::EntityId selectedEntityId = selectedEntities[0]; - if (s_prefabPublicInterface->IsInstanceContainerEntity(selectedEntity) && - !s_prefabPublicInterface->IsLevelInstanceContainerEntity(selectedEntity)) + if (s_prefabPublicInterface->IsInstanceContainerEntity(selectedEntityId) && + selectedEntityId != s_prefabFocusPublicInterface->GetFocusedPrefabContainerEntityId(editorEntityContextId)) { QAction* detachPrefabAction = menu->addAction(QObject::tr("Detach Prefab...")); QObject::connect( detachPrefabAction, &QAction::triggered, detachPrefabAction, - [selectedEntity] + [selectedEntityId] { - ContextMenu_DetachPrefab(selectedEntity); + ContextMenu_DetachPrefab(selectedEntityId); }); } } @@ -331,13 +337,21 @@ namespace AzToolsFramework QWidget* activeWindow = QApplication::activeWindow(); const AZStd::string prefabFilesPath = "@projectroot@/Prefabs"; - // Remove Level entity if it's part of the list - - auto levelContainerIter = - AZStd::find(selectedEntities.begin(), selectedEntities.end(), s_prefabPublicInterface->GetLevelInstanceContainerEntityId()); - if (levelContainerIter != selectedEntities.end()) + // Remove focused instance container entity if it's part of the list + auto editorEntityContextId = AzFramework::EntityContextId::CreateNull(); + EditorEntityContextRequestBus::BroadcastResult(editorEntityContextId, &EditorEntityContextRequests::GetEditorEntityContextId); + + auto focusedContainerIter = AZStd::find( + selectedEntities.begin(), selectedEntities.end(), + s_prefabFocusPublicInterface->GetFocusedPrefabContainerEntityId(editorEntityContextId)); + if (focusedContainerIter != selectedEntities.end()) { - selectedEntities.erase(levelContainerIter); + selectedEntities.erase(focusedContainerIter); + } + + if (selectedEntities.empty()) + { + return; } // Set default folder for prefabs diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Prefab/PrefabUiHandler.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Prefab/PrefabUiHandler.cpp index 00522b29dc..ccad85e32b 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Prefab/PrefabUiHandler.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Prefab/PrefabUiHandler.cpp @@ -178,12 +178,6 @@ namespace AzToolsFramework AZ::EntityId entityId(index.data(EntityOutlinerListModel::EntityIdRole).value()); - // We hide the root instance container entity from the Outliner, so avoid drawing its full container on children - if (m_prefabPublicInterface->IsLevelInstanceContainerEntity(entityId)) - { - return; - } - const QTreeView* outlinerTreeView(qobject_cast(option.widget)); const int ancestorLeft = outlinerTreeView->visualRect(index).left() + (m_prefabBorderThickness / 2) - 1; const int curveRectSize = m_prefabCapsuleRadius * 2;