From eb8084d3f6c0e6973a8a250aeee9c231218c0691 Mon Sep 17 00:00:00 2001 From: srikappa Date: Fri, 7 May 2021 16:21:23 -0700 Subject: [PATCH 01/15] Initial draft for getting instantiation to work immediately after creation is undone --- .../PrefabEditorEntityOwnershipService.cpp | 4 +- .../Instance/InstanceToTemplatePropagator.cpp | 14 +++++- .../AzToolsFramework/Prefab/Link/Link.cpp | 3 ++ .../AzToolsFramework/Prefab/PrefabDomTypes.h | 1 + .../Prefab/PrefabPublicHandler.cpp | 47 +++++++++++++++---- .../Prefab/PrefabPublicHandler.h | 2 +- .../Prefab/PrefabSystemComponent.cpp | 6 +-- .../Prefab/PrefabSystemComponent.h | 2 +- .../Prefab/PrefabSystemComponentInterface.h | 2 +- .../AzToolsFramework/Prefab/PrefabUndo.cpp | 23 +++++---- .../AzToolsFramework/Prefab/PrefabUndo.h | 2 +- .../Prefab/PrefabUndoHelpers.cpp | 8 ++-- .../Prefab/PrefabUndoHelpers.h | 4 +- .../Tests/Prefab/PrefabUndoLinkTests.cpp | 4 +- 14 files changed, 88 insertions(+), 34 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp index 81233069a9..58aa245397 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp @@ -281,13 +281,15 @@ namespace AzToolsFramework containerEntity->AddComponent(aznew Prefab::EditorPrefabComponent()); HandleEntitiesAdded({containerEntity}); HandleEntitiesAdded(entities); - + + /* // Update the template of the instance since we modified the entities of the instance by calling HandleEntitiesAdded. Prefab::PrefabDom serializedInstance; if (Prefab::PrefabDomUtils::StoreInstanceInPrefabDom(addedInstance, serializedInstance)) { m_prefabSystemComponent->UpdatePrefabTemplate(addedInstance.GetTemplateId(), serializedInstance); } + */ return addedInstance; } diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.cpp index b1c5f64ef9..d298e1e2b2 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.cpp @@ -176,10 +176,15 @@ namespace AzToolsFramework { PrefabDom& templateDomReference = m_prefabSystemComponentInterface->FindTemplateDom(templateId); + PrefabDomUtils::PrintPrefabDomValue("patch is ", providedPatch); + PrefabDomUtils::PrintPrefabDomValue("template dom before is ", templateDomReference); + //apply patch to template AZ::JsonSerializationResult::ResultCode result = AZ::JsonSerialization::ApplyPatch(templateDomReference, templateDomReference.GetAllocator(), providedPatch, AZ::JsonMergeApproach::JsonPatch); + PrefabDomUtils::PrintPrefabDomValue("template dom after is ", templateDomReference); + //trigger propagation if (result.GetOutcome() == AZ::JsonSerializationResult::Outcomes::Success) { @@ -270,7 +275,7 @@ namespace AzToolsFramework return parentInstance; } - void InstanceToTemplatePropagator::AddPatchesToLink(PrefabDom& patches, Link& link) + void InstanceToTemplatePropagator::AddPatchesToLink(const PrefabDom& patches, Link& link) { PrefabDom& linkDom = link.GetLinkDom(); PrefabDomValueReference linkPatchesReference = @@ -279,7 +284,12 @@ namespace AzToolsFramework // This logic only covers addition of patches. If patches already exists, the given list of patches must be appended to them. if (!linkPatchesReference.has_value()) { - linkDom.AddMember(rapidjson::StringRef(PrefabDomUtils::PatchesName), patches, linkDom.GetAllocator()); + // If the original allocator the patches were created with gets destroyed, then the patches would become garbage in the + // linkDom. Since we cannot guarantee the lifecycle of the patch allocators, we are doing a copy of the patches here to + // associate them with the linkDom's allocator. This is a limitation with rapidjson. + PrefabDom patchesCopy; + patchesCopy.CopyFrom(patches, linkDom.GetAllocator()); + linkDom.AddMember(rapidjson::StringRef(PrefabDomUtils::PatchesName), patchesCopy, linkDom.GetAllocator()); } } } diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Link/Link.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Link/Link.cpp index e0834ed53b..e421e55a11 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Link/Link.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Link/Link.cpp @@ -181,6 +181,8 @@ namespace AzToolsFramework } else { + PrefabDomUtils::PrintPrefabDomValue("Patches are : ", m_linkDom); + PrefabDomUtils::PrintPrefabDomValue("Linked instance dom before is : ", linkedInstanceDom); AZ::JsonSerializationResult::ResultCode applyPatchResult = AZ::JsonSerialization::ApplyPatch( linkedInstanceDom, targetTemplatePrefabDom.GetAllocator(), @@ -193,6 +195,7 @@ namespace AzToolsFramework "Link::UpdateTarget - " "ApplyPatches failed for Prefab DOM from source Template '%u' and target Template '%u'.", m_sourceTemplateId, m_targetTemplateId); + PrefabDomUtils::PrintPrefabDomValue("Linked instance dom after is : ", linkedInstanceDom); return false; } } diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomTypes.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomTypes.h index e32f817227..e897630a91 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomTypes.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomTypes.h @@ -27,6 +27,7 @@ namespace AzToolsFramework using PrefabDomList = AZStd::vector; using PrefabDomReference = AZStd::optional>; + using PrefabDomConstReference = AZStd::optional>; using PrefabDomValueReference = AZStd::optional>; using PrefabDomValueConstReference = AZStd::optional>; diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp index 475510c52f..80e1b558e7 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp @@ -122,6 +122,24 @@ namespace AzToolsFramework AZ::EntityId containerEntityId = instanceToCreate->get().GetContainerEntityId(); + // Change top level entities to be parented to the container entity + // Mark them as dirty so this change is correctly applied to the template + for (AZ::Entity* topLevelEntity : entities) + { + //m_prefabUndoCache.UpdateCache(topLevelEntity->GetId()); + // undoBatch.MarkEntityDirty(topLevelEntity->GetId()); + AZ::TransformBus::Event(topLevelEntity->GetId(), &AZ::TransformBus::Events::SetParent, containerEntityId); + //ToolsApplicationRequests::Bus::Broadcast( + // &ToolsApplicationRequests::Bus::Events::RemoveDirtyEntity, topLevelEntity->GetId()); + } + + // Update the template of the instance since we modified the entities of the instance by calling HandleEntitiesAdded. + Prefab::PrefabDom serializedInstance; + if (Prefab::PrefabDomUtils::StoreInstanceInPrefabDom(instanceToCreate->get(), serializedInstance)) + { + m_prefabSystemComponentInterface->UpdatePrefabTemplate(instanceToCreate->get().GetTemplateId(), serializedInstance); + } + instanceToCreate->get().GetNestedInstances([&](AZStd::unique_ptr& nestedInstance) { AZ_Assert(nestedInstance, "Invalid nested instance found in the new prefab created."); EntityOptionalReference nestedInstanceContainerEntity = nestedInstance->GetContainerEntity(); @@ -129,7 +147,7 @@ namespace AzToolsFramework nestedInstanceContainerEntity, "Invalid container entity found for the nested instance used in prefab creation."); CreateLink( {&nestedInstanceContainerEntity->get()}, *nestedInstance, instanceToCreate->get().GetTemplateId(), - undoBatch.GetUndoBatch(), containerEntityId); + undoBatch.GetUndoBatch(), containerEntityId, false); }); CreateLink( @@ -141,10 +159,13 @@ namespace AzToolsFramework for (AZ::Entity* topLevelEntity : topLevelEntities) { m_prefabUndoCache.UpdateCache(topLevelEntity->GetId()); - undoBatch.MarkEntityDirty(topLevelEntity->GetId()); - AZ::TransformBus::Event(topLevelEntity->GetId(), &AZ::TransformBus::Events::SetParent, containerEntityId); + //undoBatch.MarkEntityDirty(topLevelEntity->GetId()); + //AZ::TransformBus::Event(topLevelEntity->GetId(), &AZ::TransformBus::Events::SetParent, containerEntityId); + ToolsApplicationRequests::Bus::Broadcast( + &ToolsApplicationRequests::Bus::Events::RemoveDirtyEntity, topLevelEntity->GetId()); } + // Select Container Entity { auto selectionUndo = aznew SelectionCommand({containerEntityId}, "Select Prefab Container Entity"); @@ -244,7 +265,7 @@ namespace AzToolsFramework void PrefabPublicHandler::CreateLink( const EntityList& topLevelEntities, Instance& sourceInstance, TemplateId targetTemplateId, - UndoSystem::URSequencePoint* undoBatch, AZ::EntityId commonRootEntityId) + UndoSystem::URSequencePoint* undoBatch, AZ::EntityId commonRootEntityId, const bool IsUndoRedoSupportNeeded) { AZ::EntityId containerEntityId = sourceInstance.GetContainerEntityId(); AZ::Entity* containerEntity = GetEntityById(containerEntityId); @@ -270,9 +291,19 @@ namespace AzToolsFramework m_instanceToTemplateInterface->GeneratePatch(patch, containerEntityDomBefore, containerEntityDomAfter); m_instanceToTemplateInterface->AppendEntityAliasToPatchPaths(patch, containerEntityId); - LinkId linkId = PrefabUndoHelpers::CreateLink( - sourceInstance.GetTemplateId(), targetTemplateId, patch, sourceInstance.GetInstanceAlias(), - undoBatch); + LinkId linkId; + if (IsUndoRedoSupportNeeded) + { + linkId = PrefabUndoHelpers::CreateLink( + sourceInstance.GetTemplateId(), targetTemplateId, AZStd::move(patch), sourceInstance.GetInstanceAlias(), undoBatch); + } + else + { + linkId = m_prefabSystemComponentInterface->CreateLink( + targetTemplateId, sourceInstance.GetTemplateId(), sourceInstance.GetInstanceAlias(), AZStd::move(patch), + InvalidLinkId); + m_prefabSystemComponentInterface->PropagateTemplateChanges(targetTemplateId); + } sourceInstance.SetLinkId(linkId); @@ -305,7 +336,7 @@ namespace AzToolsFramework patchesCopyForUndoSupport.CopyFrom(nestedInstanceLinkPatches->get(), patchesCopyForUndoSupport.GetAllocator()); PrefabUndoHelpers::RemoveLink( sourceInstance->GetTemplateId(), targetTemplateId, sourceInstance->GetInstanceAlias(), sourceInstance->GetLinkId(), - patchesCopyForUndoSupport, undoBatch); + AZStd::move(patchesCopyForUndoSupport), undoBatch); } PrefabOperationResult PrefabPublicHandler::SavePrefab(AZ::IO::Path filePath) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h index 5ade666a40..0e15fd9a15 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h @@ -80,7 +80,7 @@ namespace AzToolsFramework */ void CreateLink( const EntityList& topLevelEntities, Instance& sourceInstance, TemplateId targetTemplateId, - UndoSystem::URSequencePoint* undoBatch, AZ::EntityId commonRootEntityId); + UndoSystem::URSequencePoint* undoBatch, AZ::EntityId commonRootEntityId, const bool IsUndoRedoSupportNeeded = true); /** * Removes the link between template of the sourceInstance and the template corresponding to targetTemplateId. diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp index c6a95b01fe..0b447ef786 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp @@ -583,7 +583,7 @@ namespace AzToolsFramework const TemplateId& linkTargetId, const TemplateId& linkSourceId, const InstanceAlias& instanceAlias, - const PrefabDomReference linkPatch, + PrefabDom linkPatch, const LinkId& linkId) { if (linkTargetId == InvalidTemplateId) @@ -667,9 +667,9 @@ namespace AzToolsFramework rapidjson::StringRef(PrefabDomUtils::SourceName), rapidjson::StringRef(sourceTemplate.GetFilePath().c_str()), newLink.GetLinkDom().GetAllocator()); - if (linkPatch && linkPatch->get().IsArray() && !(linkPatch->get().Empty())) + if (linkPatch.IsArray() && !(linkPatch.Empty())) { - m_instanceToTemplatePropagator.AddPatchesToLink(linkPatch.value(), newLink); + m_instanceToTemplatePropagator.AddPatchesToLink(AZStd::move(linkPatch), newLink); } //update the target template dom to have the proper values for the source template dom diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.h index 30d03ba2ef..b5f499296d 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.h @@ -156,7 +156,7 @@ namespace AzToolsFramework const TemplateId& linkTargetId, const TemplateId& linkSourceId, const InstanceAlias& instanceAlias, - const PrefabDomReference linkPatch, + PrefabDom linkPatch, const LinkId& linkId = InvalidLinkId) override; /** diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponentInterface.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponentInterface.h index 6491d67401..347b48a648 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponentInterface.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponentInterface.h @@ -44,7 +44,7 @@ namespace AzToolsFramework //creates a new Link virtual LinkId CreateLink(const TemplateId& linkTargetId, const TemplateId& linkSourceId, - const InstanceAlias& instanceAlias, const PrefabDomReference linkPatch, + const InstanceAlias& instanceAlias, PrefabDom linkPatch, const LinkId& linkId = InvalidLinkId) = 0; virtual void RemoveLink(const LinkId& linkId) = 0; diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.cpp index ad0cdc166a..e82e9da90a 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.cpp @@ -124,7 +124,7 @@ namespace AzToolsFramework const TemplateId& targetId, const TemplateId& sourceId, const InstanceAlias& instanceAlias, - PrefabDomReference linkPatches, + PrefabDom linkPatches, const LinkId linkId) { m_targetId = targetId; @@ -132,10 +132,7 @@ namespace AzToolsFramework m_instanceAlias = instanceAlias; m_linkId = linkId; - if (linkPatches.has_value()) - { - m_linkPatches = AZStd::move(linkPatches->get()); - } + m_linkPatches = AZStd::move(linkPatches); //if linkId is invalid, set as ADD if (m_linkId == InvalidLinkId) @@ -193,7 +190,9 @@ namespace AzToolsFramework void PrefabUndoInstanceLink::AddLink() { - m_linkId = m_prefabSystemComponentInterface->CreateLink(m_targetId, m_sourceId, m_instanceAlias, m_linkPatches, m_linkId); + PrefabDom linkPatchesCopy; + linkPatchesCopy.CopyFrom(m_linkPatches, linkPatchesCopy.GetAllocator()); + m_linkId = m_prefabSystemComponentInterface->CreateLink(m_targetId, m_sourceId, m_instanceAlias, AZStd::move(linkPatchesCopy), m_linkId); } void PrefabUndoInstanceLink::RemoveLink() @@ -228,9 +227,12 @@ namespace AzToolsFramework if (link.has_value()) { - m_linkDomPrevious = AZStd::move(link->get().GetLinkDom()); + m_linkDomPrevious.CopyFrom(link->get().GetLinkDom(), m_linkDomPrevious.GetAllocator()); } + PrefabDomUtils::PrintPrefabDomValue("m_linkDomPrevious is : ", m_linkDomPrevious); + PrefabDomUtils::PrintPrefabDomValue("link->get().GetLinkDom() is : ", link->get().GetLinkDom()); + //get source templateDom TemplateReference sourceTemplate = m_prefabSystemComponentInterface->FindTemplate(link->get().GetSourceTemplateId()); @@ -275,7 +277,7 @@ namespace AzToolsFramework if (patchesIter == m_linkDomNext.MemberEnd()) { m_linkDomNext.AddMember( - rapidjson::GenericStringRef(PrefabDomUtils::PatchesName), patchLinkCopy, m_linkDomNext.GetAllocator()); + rapidjson::GenericStringRef(PrefabDomUtils::PatchesName), AZStd::move(patchLinkCopy), m_linkDomNext.GetAllocator()); } else { @@ -303,9 +305,14 @@ namespace AzToolsFramework return; } + /* PrefabDom moveLink; moveLink.CopyFrom(linkDom, linkDom.GetAllocator()); link->get().GetLinkDom() = AZStd::move(moveLink); + */ + link->get().SetLinkDom(linkDom); + + PrefabDomUtils::PrintPrefabDomValue("dom after updating link is : ", link->get().GetLinkDom()); //propagate the link changes link->get().UpdateTarget(); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.h index 49bae4eda0..33d9e5ad33 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.h @@ -101,7 +101,7 @@ namespace AzToolsFramework const TemplateId& targetId, const TemplateId& sourceId, const InstanceAlias& instanceAlias, - PrefabDomReference linkPatches = PrefabDomReference(), + PrefabDom linkPatches = PrefabDom(), const LinkId linkId = InvalidLinkId); void Undo() override; diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndoHelpers.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndoHelpers.cpp index 24eb71e055..5ba8d3c940 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndoHelpers.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndoHelpers.cpp @@ -34,11 +34,11 @@ namespace AzToolsFramework } LinkId CreateLink( - TemplateId sourceTemplateId, TemplateId targetTemplateId, PrefabDomReference patch, + TemplateId sourceTemplateId, TemplateId targetTemplateId, PrefabDom patch, const InstanceAlias& instanceAlias, UndoSystem::URSequencePoint* undoBatch) { auto linkAddUndo = aznew PrefabUndoInstanceLink("Create Link"); - linkAddUndo->Capture(targetTemplateId, sourceTemplateId, instanceAlias, patch, InvalidLinkId); + linkAddUndo->Capture(targetTemplateId, sourceTemplateId, instanceAlias, AZStd::move(patch), InvalidLinkId); linkAddUndo->SetParent(undoBatch); linkAddUndo->Redo(); @@ -47,10 +47,10 @@ namespace AzToolsFramework void RemoveLink( TemplateId sourceTemplateId, TemplateId targetTemplateId, const InstanceAlias& instanceAlias, LinkId linkId, - PrefabDomReference linkPatches, UndoSystem::URSequencePoint* undoBatch) + PrefabDom linkPatches, UndoSystem::URSequencePoint* undoBatch) { auto linkRemoveUndo = aznew PrefabUndoInstanceLink("Remove Link"); - linkRemoveUndo->Capture(targetTemplateId, sourceTemplateId, instanceAlias, linkPatches, linkId); + linkRemoveUndo->Capture(targetTemplateId, sourceTemplateId, instanceAlias, AZStd::move(linkPatches), linkId); linkRemoveUndo->SetParent(undoBatch); linkRemoveUndo->Redo(); } diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndoHelpers.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndoHelpers.h index 6429df3b04..17cf7f52c1 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndoHelpers.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndoHelpers.h @@ -22,11 +22,11 @@ namespace AzToolsFramework const Instance& instance, AZStd::string_view undoMessage, const PrefabDom& instanceDomBeforeUpdate, UndoSystem::URSequencePoint* undoBatch); LinkId CreateLink( - TemplateId sourceTemplateId, TemplateId targetTemplateId, PrefabDomReference patch, + TemplateId sourceTemplateId, TemplateId targetTemplateId, PrefabDom patch, const InstanceAlias& instanceAlias, UndoSystem::URSequencePoint* undoBatch); void RemoveLink( TemplateId sourceTemplateId, TemplateId targetTemplateId, const InstanceAlias& instanceAlias, LinkId linkId, - PrefabDomReference linkPatches, UndoSystem::URSequencePoint* undoBatch); + PrefabDom linkPatches, UndoSystem::URSequencePoint* undoBatch); } } // namespace Prefab } // namespace AzToolsFramework diff --git a/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabUndoLinkTests.cpp b/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabUndoLinkTests.cpp index 2122b116d8..3d540a7a95 100644 --- a/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabUndoLinkTests.cpp +++ b/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabUndoLinkTests.cpp @@ -120,7 +120,7 @@ namespace UnitTest //create an undo node to apply the patch and prep for undo PrefabUndoInstanceLink undoInstanceLinkNode("Undo Link Patch"); - undoInstanceLinkNode.Capture(rootTemplateId, nestedTemplateId, aliases[0], patch, InvalidLinkId); + undoInstanceLinkNode.Capture(rootTemplateId, nestedTemplateId, aliases[0], AZStd::move(patch), InvalidLinkId); undoInstanceLinkNode.Redo(); m_instanceUpdateExecutorInterface->UpdateTemplateInstancesInQueue(); @@ -196,7 +196,7 @@ namespace UnitTest //create an undo node to apply the patch and prep for undo PrefabUndoInstanceLink undoInstanceLinkNode("Undo Link Patch"); - undoInstanceLinkNode.Capture(rootTemplateId, nestedTemplateId, aliases[0], linkPatch, InvalidLinkId); + undoInstanceLinkNode.Capture(rootTemplateId, nestedTemplateId, aliases[0], AZStd::move(linkPatch), InvalidLinkId); undoInstanceLinkNode.Redo(); m_instanceUpdateExecutorInterface->UpdateTemplateInstancesInQueue(); From 1c04160966bf3d358f07192ec81cf2247d26e1e3 Mon Sep 17 00:00:00 2001 From: srikappa Date: Fri, 7 May 2021 16:23:14 -0700 Subject: [PATCH 02/15] Added a missing header --- .../Prefab/Instance/InstanceToTemplatePropagator.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.h index ca1f3a7c91..9a6aad8ac1 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.h @@ -41,7 +41,7 @@ namespace AzToolsFramework void ApplyPatchesToInstance(const AZ::EntityId& entityId, PrefabDom& patches, const Instance& instanceToAddPatches) override; - void AddPatchesToLink(PrefabDom& patches, Link& link); + void AddPatchesToLink(const PrefabDom& patches, Link& link); private: From 0da6e450a68ad2a20a8962a42ed52a58f34d2885 Mon Sep 17 00:00:00 2001 From: srikappa Date: Mon, 10 May 2021 11:43:32 -0700 Subject: [PATCH 03/15] Improved comments --- .../PrefabEditorEntityOwnershipService.cpp | 10 ------- .../Instance/InstanceToTemplatePropagator.cpp | 13 ++++----- .../AzToolsFramework/Prefab/Link/Link.cpp | 3 --- .../Prefab/PrefabPublicHandler.cpp | 27 +++++++++---------- .../Prefab/PrefabPublicHandler.h | 1 + 5 files changed, 19 insertions(+), 35 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp index 58aa245397..59e50fdf65 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp @@ -281,16 +281,6 @@ namespace AzToolsFramework containerEntity->AddComponent(aznew Prefab::EditorPrefabComponent()); HandleEntitiesAdded({containerEntity}); HandleEntitiesAdded(entities); - - /* - // Update the template of the instance since we modified the entities of the instance by calling HandleEntitiesAdded. - Prefab::PrefabDom serializedInstance; - if (Prefab::PrefabDomUtils::StoreInstanceInPrefabDom(addedInstance, serializedInstance)) - { - m_prefabSystemComponent->UpdatePrefabTemplate(addedInstance.GetTemplateId(), serializedInstance); - } - */ - return addedInstance; } diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.cpp index d298e1e2b2..6f812df89b 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.cpp @@ -176,15 +176,10 @@ namespace AzToolsFramework { PrefabDom& templateDomReference = m_prefabSystemComponentInterface->FindTemplateDom(templateId); - PrefabDomUtils::PrintPrefabDomValue("patch is ", providedPatch); - PrefabDomUtils::PrintPrefabDomValue("template dom before is ", templateDomReference); - //apply patch to template AZ::JsonSerializationResult::ResultCode result = AZ::JsonSerialization::ApplyPatch(templateDomReference, templateDomReference.GetAllocator(), providedPatch, AZ::JsonMergeApproach::JsonPatch); - PrefabDomUtils::PrintPrefabDomValue("template dom after is ", templateDomReference); - //trigger propagation if (result.GetOutcome() == AZ::JsonSerializationResult::Outcomes::Success) { @@ -284,9 +279,11 @@ namespace AzToolsFramework // This logic only covers addition of patches. If patches already exists, the given list of patches must be appended to them. if (!linkPatchesReference.has_value()) { - // If the original allocator the patches were created with gets destroyed, then the patches would become garbage in the - // linkDom. Since we cannot guarantee the lifecycle of the patch allocators, we are doing a copy of the patches here to - // associate them with the linkDom's allocator. This is a limitation with rapidjson. + /* + If the original allocator the patches were created with gets destroyed, then the patches would become garbage in the + linkDom. Since we cannot guarantee the lifecycle of the patch allocators, we are doing a copy of the patches here to + associate them with the linkDom's allocator. This is a limitation with rapidjson. + */ PrefabDom patchesCopy; patchesCopy.CopyFrom(patches, linkDom.GetAllocator()); linkDom.AddMember(rapidjson::StringRef(PrefabDomUtils::PatchesName), patchesCopy, linkDom.GetAllocator()); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Link/Link.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Link/Link.cpp index e421e55a11..e0834ed53b 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Link/Link.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Link/Link.cpp @@ -181,8 +181,6 @@ namespace AzToolsFramework } else { - PrefabDomUtils::PrintPrefabDomValue("Patches are : ", m_linkDom); - PrefabDomUtils::PrintPrefabDomValue("Linked instance dom before is : ", linkedInstanceDom); AZ::JsonSerializationResult::ResultCode applyPatchResult = AZ::JsonSerialization::ApplyPatch( linkedInstanceDom, targetTemplatePrefabDom.GetAllocator(), @@ -195,7 +193,6 @@ namespace AzToolsFramework "Link::UpdateTarget - " "ApplyPatches failed for Prefab DOM from source Template '%u' and target Template '%u'.", m_sourceTemplateId, m_targetTemplateId); - PrefabDomUtils::PrintPrefabDomValue("Linked instance dom after is : ", linkedInstanceDom); return false; } } diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp index 80e1b558e7..9959ae18fa 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp @@ -122,18 +122,14 @@ namespace AzToolsFramework AZ::EntityId containerEntityId = instanceToCreate->get().GetContainerEntityId(); - // Change top level entities to be parented to the container entity - // Mark them as dirty so this change is correctly applied to the template + // Parent the entities to the container entity. Parenting the container entities of the instances passed to createPrefab + // will be done during the creation of links below. for (AZ::Entity* topLevelEntity : entities) { - //m_prefabUndoCache.UpdateCache(topLevelEntity->GetId()); - // undoBatch.MarkEntityDirty(topLevelEntity->GetId()); AZ::TransformBus::Event(topLevelEntity->GetId(), &AZ::TransformBus::Events::SetParent, containerEntityId); - //ToolsApplicationRequests::Bus::Broadcast( - // &ToolsApplicationRequests::Bus::Events::RemoveDirtyEntity, topLevelEntity->GetId()); } - // Update the template of the instance since we modified the entities of the instance by calling HandleEntitiesAdded. + // Update the template of the instance since the entities are modified since the template creation. Prefab::PrefabDom serializedInstance; if (Prefab::PrefabDomUtils::StoreInstanceInPrefabDom(instanceToCreate->get(), serializedInstance)) { @@ -145,27 +141,28 @@ namespace AzToolsFramework EntityOptionalReference nestedInstanceContainerEntity = nestedInstance->GetContainerEntity(); AZ_Assert( nestedInstanceContainerEntity, "Invalid container entity found for the nested instance used in prefab creation."); + + // These link creations shouldn't be undone because that would put the template in a non-usable state if a user + // chooses to instantiate the template after undoing the creation. CreateLink( {&nestedInstanceContainerEntity->get()}, *nestedInstance, instanceToCreate->get().GetTemplateId(), undoBatch.GetUndoBatch(), containerEntityId, false); }); CreateLink( - topLevelEntities, instanceToCreate->get(), commonRootEntityOwningInstance->get().GetTemplateId(), undoBatch.GetUndoBatch(), - commonRootEntityId); + topLevelEntities, instanceToCreate->get(), commonRootEntityOwningInstance->get().GetTemplateId(), + undoBatch.GetUndoBatch(), commonRootEntityId); - // Change top level entities to be parented to the container entity - // Mark them as dirty so this change is correctly applied to the template for (AZ::Entity* topLevelEntity : topLevelEntities) { m_prefabUndoCache.UpdateCache(topLevelEntity->GetId()); - //undoBatch.MarkEntityDirty(topLevelEntity->GetId()); - //AZ::TransformBus::Event(topLevelEntity->GetId(), &AZ::TransformBus::Events::SetParent, containerEntityId); + + // Parenting entities would mark entities as dirty. But we want to unmark the top level entities as dirty because + // if we don't, the template created would be updated and cause issues with undo operation followed by instantiation. ToolsApplicationRequests::Bus::Broadcast( &ToolsApplicationRequests::Bus::Events::RemoveDirtyEntity, topLevelEntity->GetId()); } - // Select Container Entity { auto selectionUndo = aznew SelectionCommand({containerEntityId}, "Select Prefab Container Entity"); @@ -442,6 +439,8 @@ namespace AzToolsFramework PrefabDom patch; m_instanceToTemplateInterface->GeneratePatch(patch, beforeState, afterState); + PrefabDomUtils::PrintPrefabDomValue(entity->GetName(), patch); + if (patch.IsArray() && !patch.Empty() && beforeState.IsObject()) { if (IsInstanceContainerEntity(entityId) && !IsLevelInstanceContainerEntity(entityId)) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h index 0e15fd9a15..771f73a815 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h @@ -77,6 +77,7 @@ namespace AzToolsFramework * \param targetInstance The id of the target template. * \param undoBatch The undo batch to set as parent for this create link action. * \param commonRootEntityId The id of the entity that the source instance should be parented under. + * \param IsUndoRedoSupportNeeded The flag indicating whether the link should be created with undo/redo support or not. */ void CreateLink( const EntityList& topLevelEntities, Instance& sourceInstance, TemplateId targetTemplateId, From 49941036276264c3248b42cf2fdba9fe8469f386 Mon Sep 17 00:00:00 2001 From: srikappa Date: Mon, 10 May 2021 11:58:54 -0700 Subject: [PATCH 04/15] Add const ref function parameters as needed --- .../Prefab/PrefabSystemComponent.cpp | 6 +++--- .../Prefab/PrefabSystemComponent.h | 2 +- .../Prefab/PrefabSystemComponentInterface.h | 6 +++--- .../AzToolsFramework/Prefab/PrefabUndo.cpp | 14 +------------- 4 files changed, 8 insertions(+), 20 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp index 0b447ef786..985d2333bc 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp @@ -583,7 +583,7 @@ namespace AzToolsFramework const TemplateId& linkTargetId, const TemplateId& linkSourceId, const InstanceAlias& instanceAlias, - PrefabDom linkPatch, + const PrefabDomConstReference linkPatches, const LinkId& linkId) { if (linkTargetId == InvalidTemplateId) @@ -667,9 +667,9 @@ namespace AzToolsFramework rapidjson::StringRef(PrefabDomUtils::SourceName), rapidjson::StringRef(sourceTemplate.GetFilePath().c_str()), newLink.GetLinkDom().GetAllocator()); - if (linkPatch.IsArray() && !(linkPatch.Empty())) + if (linkPatches && linkPatches->get().IsArray() && !(linkPatches->get().Empty())) { - m_instanceToTemplatePropagator.AddPatchesToLink(AZStd::move(linkPatch), newLink); + m_instanceToTemplatePropagator.AddPatchesToLink(linkPatches.value(), newLink); } //update the target template dom to have the proper values for the source template dom diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.h index b5f499296d..0a9a450f64 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.h @@ -156,7 +156,7 @@ namespace AzToolsFramework const TemplateId& linkTargetId, const TemplateId& linkSourceId, const InstanceAlias& instanceAlias, - PrefabDom linkPatch, + const PrefabDomConstReference linkPatches, const LinkId& linkId = InvalidLinkId) override; /** diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponentInterface.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponentInterface.h index 347b48a648..f47941254a 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponentInterface.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponentInterface.h @@ -43,9 +43,9 @@ namespace AzToolsFramework PrefabDomValue::MemberIterator& instanceIterator, InstanceOptionalReference instance) = 0; //creates a new Link - virtual LinkId CreateLink(const TemplateId& linkTargetId, const TemplateId& linkSourceId, - const InstanceAlias& instanceAlias, PrefabDom linkPatch, - const LinkId& linkId = InvalidLinkId) = 0; + virtual LinkId CreateLink( + const TemplateId& linkTargetId, const TemplateId& linkSourceId, const InstanceAlias& instanceAlias, + const PrefabDomConstReference linkPatches, const LinkId& linkId = InvalidLinkId) = 0; virtual void RemoveLink(const LinkId& linkId) = 0; diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.cpp index e82e9da90a..aadcdcdea0 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.cpp @@ -190,9 +190,7 @@ namespace AzToolsFramework void PrefabUndoInstanceLink::AddLink() { - PrefabDom linkPatchesCopy; - linkPatchesCopy.CopyFrom(m_linkPatches, linkPatchesCopy.GetAllocator()); - m_linkId = m_prefabSystemComponentInterface->CreateLink(m_targetId, m_sourceId, m_instanceAlias, AZStd::move(linkPatchesCopy), m_linkId); + m_linkId = m_prefabSystemComponentInterface->CreateLink(m_targetId, m_sourceId, m_instanceAlias, m_linkPatches, m_linkId); } void PrefabUndoInstanceLink::RemoveLink() @@ -230,9 +228,6 @@ namespace AzToolsFramework m_linkDomPrevious.CopyFrom(link->get().GetLinkDom(), m_linkDomPrevious.GetAllocator()); } - PrefabDomUtils::PrintPrefabDomValue("m_linkDomPrevious is : ", m_linkDomPrevious); - PrefabDomUtils::PrintPrefabDomValue("link->get().GetLinkDom() is : ", link->get().GetLinkDom()); - //get source templateDom TemplateReference sourceTemplate = m_prefabSystemComponentInterface->FindTemplate(link->get().GetSourceTemplateId()); @@ -305,15 +300,8 @@ namespace AzToolsFramework return; } - /* - PrefabDom moveLink; - moveLink.CopyFrom(linkDom, linkDom.GetAllocator()); - link->get().GetLinkDom() = AZStd::move(moveLink); - */ link->get().SetLinkDom(linkDom); - PrefabDomUtils::PrintPrefabDomValue("dom after updating link is : ", link->get().GetLinkDom()); - //propagate the link changes link->get().UpdateTarget(); m_prefabSystemComponentInterface->PropagateTemplateChanges(link->get().GetTargetTemplateId()); From b57dd02d90139ce33928d2d5cba1d569c8b8e7dd Mon Sep 17 00:00:00 2001 From: srikappa Date: Mon, 10 May 2021 12:28:33 -0700 Subject: [PATCH 05/15] Removed a debug print command --- .../AzToolsFramework/Prefab/PrefabPublicHandler.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp index 9959ae18fa..4d1a657245 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp @@ -297,7 +297,7 @@ namespace AzToolsFramework else { linkId = m_prefabSystemComponentInterface->CreateLink( - targetTemplateId, sourceInstance.GetTemplateId(), sourceInstance.GetInstanceAlias(), AZStd::move(patch), + targetTemplateId, sourceInstance.GetTemplateId(), sourceInstance.GetInstanceAlias(), patch, InvalidLinkId); m_prefabSystemComponentInterface->PropagateTemplateChanges(targetTemplateId); } @@ -439,8 +439,6 @@ namespace AzToolsFramework PrefabDom patch; m_instanceToTemplateInterface->GeneratePatch(patch, beforeState, afterState); - PrefabDomUtils::PrintPrefabDomValue(entity->GetName(), patch); - if (patch.IsArray() && !patch.Empty() && beforeState.IsObject()) { if (IsInstanceContainerEntity(entityId) && !IsLevelInstanceContainerEntity(entityId)) From b11234097d8cc4741ef3e786a6823966633eb8ae Mon Sep 17 00:00:00 2001 From: srikappa Date: Tue, 11 May 2021 11:30:16 -0700 Subject: [PATCH 06/15] Fixed a capitalization issue --- .../Prefab/Instance/InstanceToTemplatePropagator.cpp | 2 +- .../AzToolsFramework/Prefab/PrefabPublicHandler.cpp | 4 ++-- .../AzToolsFramework/Prefab/PrefabPublicHandler.h | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.cpp index 6f812df89b..a21c5301aa 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.cpp @@ -282,7 +282,7 @@ namespace AzToolsFramework /* If the original allocator the patches were created with gets destroyed, then the patches would become garbage in the linkDom. Since we cannot guarantee the lifecycle of the patch allocators, we are doing a copy of the patches here to - associate them with the linkDom's allocator. This is a limitation with rapidjson. + associate them with the linkDom's allocator. */ PrefabDom patchesCopy; patchesCopy.CopyFrom(patches, linkDom.GetAllocator()); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp index fee455b710..02fa2d14fc 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp @@ -295,7 +295,7 @@ namespace AzToolsFramework void PrefabPublicHandler::CreateLink( const EntityList& topLevelEntities, Instance& sourceInstance, TemplateId targetTemplateId, - UndoSystem::URSequencePoint* undoBatch, AZ::EntityId commonRootEntityId, const bool IsUndoRedoSupportNeeded) + UndoSystem::URSequencePoint* undoBatch, AZ::EntityId commonRootEntityId, const bool isUndoRedoSupportNeeded) { AZ::EntityId containerEntityId = sourceInstance.GetContainerEntityId(); AZ::Entity* containerEntity = GetEntityById(containerEntityId); @@ -322,7 +322,7 @@ namespace AzToolsFramework m_instanceToTemplateInterface->AppendEntityAliasToPatchPaths(patch, containerEntityId); LinkId linkId; - if (IsUndoRedoSupportNeeded) + if (isUndoRedoSupportNeeded) { linkId = PrefabUndoHelpers::CreateLink( sourceInstance.GetTemplateId(), targetTemplateId, AZStd::move(patch), sourceInstance.GetInstanceAlias(), undoBatch); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h index 31937cf028..138bc84aa0 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h @@ -77,11 +77,11 @@ namespace AzToolsFramework * \param targetInstance The id of the target template. * \param undoBatch The undo batch to set as parent for this create link action. * \param commonRootEntityId The id of the entity that the source instance should be parented under. - * \param IsUndoRedoSupportNeeded The flag indicating whether the link should be created with undo/redo support or not. + * \param isUndoRedoSupportNeeded The flag indicating whether the link should be created with undo/redo support or not. */ void CreateLink( const EntityList& topLevelEntities, Instance& sourceInstance, TemplateId targetTemplateId, - UndoSystem::URSequencePoint* undoBatch, AZ::EntityId commonRootEntityId, const bool IsUndoRedoSupportNeeded = true); + UndoSystem::URSequencePoint* undoBatch, AZ::EntityId commonRootEntityId, const bool isUndoRedoSupportNeeded = true); /** * Removes the link between template of the sourceInstance and the template corresponding to targetTemplateId. From 4d8ed6c0cb09b2abe43b01eea5003ab989377c6a Mon Sep 17 00:00:00 2001 From: zsolleci Date: Tue, 11 May 2021 14:19:45 -0500 Subject: [PATCH 07/15] T92563569 completed and suite updated --- .../scripting/Node_HappyPath_DuplicateNode.py | 116 ++++++++++++++++++ .../PythonTests/scripting/TestSuite_Active.py | 10 +- 2 files changed, 123 insertions(+), 3 deletions(-) create mode 100644 AutomatedTesting/Gem/PythonTests/scripting/Node_HappyPath_DuplicateNode.py diff --git a/AutomatedTesting/Gem/PythonTests/scripting/Node_HappyPath_DuplicateNode.py b/AutomatedTesting/Gem/PythonTests/scripting/Node_HappyPath_DuplicateNode.py new file mode 100644 index 0000000000..9d8cccd027 --- /dev/null +++ b/AutomatedTesting/Gem/PythonTests/scripting/Node_HappyPath_DuplicateNode.py @@ -0,0 +1,116 @@ +""" +All or portions of this file Copyright (c) Amazon.com, Inc. or its affiliates or +its licensors. + +For complete copyright and license terms please see the LICENSE at the root of this +distribution (the "License"). All use of this software is governed by the License, +or, if provided, by the license below or the license accompanying this file. Do not +remove or modify any license notices. This file is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +""" + + +# fmt: off +class Tests(): + open_sc_window = ("Script Canvas window is opened", "Failed to open Script Canvas window") + node_added = ("Successfully added node to graph", "Failed to add node to graph") + node_duplicated = ("Successfully duplicated node", "Failed to duplicate the node") +# fmt: on + + +def Node_HappyPath_DuplicateNode(): + """ + Summary: + Duplicating node in graph + + Expected Behavior: + Upon selecting a node and pressing Ctrl+D, the node will be duplicated + + Test Steps: + 1) Open Script Canvas window (Tools > Script Canvas) + 2) Open a new graph + 3) Add node to graph + 4) Select node in graph to verify existence + 5) Mock Ctrl+D to duplicate node + 6) Verify the node was duplicated + + Note: + - This test file must be called from the Open 3D Engine Editor command terminal + - Any passed and failed tests are written to the Editor.log file. + Parsing the file or running a log_monitor are required to observe the test results. + + :return: None + """ + from PySide2 import QtWidgets, QtTest + from PySide2.QtCore import Qt + + from editor_python_test_tools.utils import Report + from editor_python_test_tools.utils import TestHelper as helper + import editor_python_test_tools.pyside_utils as pyside_utils + + import azlmbr.legacy.general as general + + WAIT_FRAMES = 200 + + NODE_NAME = "Print" + NODE_CATEGORY = "Debug" + EXPECTED_STRING = f"{NODE_NAME} - {NODE_CATEGORY} (2 Selected)" + + def command_line_input(command_str): + cmd_action = pyside_utils.find_child_by_pattern( + sc_main, {"objectName": "action_ViewCommandLine", "type": QtWidgets.QAction} + ) + cmd_action.trigger() + textbox = sc.findChild(QtWidgets.QLineEdit, "commandText") + QtTest.QTest.keyClicks(textbox, command_str) + QtTest.QTest.keyClick(textbox, Qt.Key_Enter, Qt.NoModifier) + + def grab_title_text(): + scroll_area = node_inspector.findChild(QtWidgets.QScrollArea, "") + QtTest.QTest.keyClick(graph, "a", Qt.ControlModifier, WAIT_FRAMES) + general.idle_wait(1.0) + background = scroll_area.findChild(QtWidgets.QFrame, "Background") + title = background.findChild(QtWidgets.QLabel, "Title") + text = title.findChild(QtWidgets.QLabel, "Title") + print(text.text()) + return text.text() + + # 1) Open Script Canvas window (Tools > Script Canvas) + general.idle_enable(True) + general.open_pane("Script Canvas") + helper.wait_for_condition(lambda: general.is_pane_visible("Script Canvas"), 5.0) + + # # 2) Open a new graph + editor_window = pyside_utils.get_editor_main_window() + sc = editor_window.findChild(QtWidgets.QDockWidget, "Script Canvas") + sc_main = sc.findChild(QtWidgets.QMainWindow) + create_new_graph = pyside_utils.find_child_by_pattern( + sc_main, {"objectName": "action_New_Script", "type": QtWidgets.QAction} + ) + node_inspector = sc.findChild(QtWidgets.QDockWidget, "NodeInspector") + create_new_graph.trigger() + + # 3) Add node + command_line_input("add_node Print") + + # 4) Select node in graph to verify existence + graph_view = sc.findChild(QtWidgets.QFrame, "graphicsViewFrame") + graph = graph_view.findChild(QtWidgets.QWidget, "") + + # 5) Duplicate node + sc_main.activateWindow() + QtTest.QTest.keyClick(graph, "a", Qt.ControlModifier, WAIT_FRAMES) + QtTest.QTest.keyClick(graph, "d", Qt.ControlModifier, WAIT_FRAMES) + + # 6) Verify the node was duplicated + after_dup = grab_title_text() + Report.result(Tests.node_duplicated, after_dup == EXPECTED_STRING) + + +if __name__ == "__main__": + import ImportPathHelper as imports + + imports.init() + from editor_python_test_tools.utils import Report + + Report.start_test(Node_HappyPath_DuplicateNode) diff --git a/AutomatedTesting/Gem/PythonTests/scripting/TestSuite_Active.py b/AutomatedTesting/Gem/PythonTests/scripting/TestSuite_Active.py index dda75f0a0c..2cd41d1980 100755 --- a/AutomatedTesting/Gem/PythonTests/scripting/TestSuite_Active.py +++ b/AutomatedTesting/Gem/PythonTests/scripting/TestSuite_Active.py @@ -84,7 +84,7 @@ class TestAutomation(TestAutomationBase): file_system.delete([os.path.join(workspace.paths.project(), "Levels", level)], True, True) from . import OnEntityActivatedDeactivated_PrintMessage as test_module self._run_test(request, workspace, editor, test_module) - + @pytest.mark.test_case_id("T92562993") def test_NodePalette_ClearSelection(self, request, workspace, editor, launcher_platform, project): from . import NodePalette_ClearSelection as test_module @@ -122,7 +122,7 @@ class TestAutomation(TestAutomationBase): def test_NodeInspector_RenameVariable(self, request, workspace, editor, launcher_platform, project): from . import NodeInspector_RenameVariable as test_module self._run_test(request, workspace, editor, test_module) - + @pytest.mark.test_case_id("T92569137") def test_Debugging_TargetMultipleGraphs(self, request, workspace, editor, launcher_platform, project): from . import Debugging_TargetMultipleGraphs as test_module @@ -195,7 +195,7 @@ class TestAutomation(TestAutomationBase): def test_NodeCategory_ExpandOnClick(self, request, workspace, editor, launcher_platform): from . import NodeCategory_ExpandOnClick as test_module self._run_test(request, workspace, editor, test_module) - + def test_NodePalette_SearchText_Deletion(self, request, workspace, editor, launcher_platform): from . import NodePalette_SearchText_Deletion as test_module self._run_test(request, workspace, editor, test_module) @@ -204,6 +204,10 @@ class TestAutomation(TestAutomationBase): from . import VariableManager_UnpinVariableType_Works as test_module self._run_test(request, workspace, editor, test_module) + def test_Node_HappyPath_DuplicateNode(self, request, workspace, editor, launcher_platform): + from . import Node_HappyPath_DuplicateNode as test_module + self._run_test(request, workspace, editor, test_module) + # NOTE: We had to use hydra_test_utils.py, as TestAutomationBase run_test method # fails because of pyside_utils import @pytest.mark.SUITE_periodic From 21dfcfed74ed657656c2d67ccda70a871b6bfd24 Mon Sep 17 00:00:00 2001 From: mbalfour Date: Tue, 11 May 2021 15:26:07 -0500 Subject: [PATCH 08/15] Fix bug where setting autoLoad to false for one gem will prevent every gem after it from loading as well. --- Code/Framework/AzCore/AzCore/Component/ComponentApplication.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Code/Framework/AzCore/AzCore/Component/ComponentApplication.cpp b/Code/Framework/AzCore/AzCore/Component/ComponentApplication.cpp index daf5b5efd2..b2fe4417d3 100644 --- a/Code/Framework/AzCore/AzCore/Component/ComponentApplication.cpp +++ b/Code/Framework/AzCore/AzCore/Component/ComponentApplication.cpp @@ -1304,7 +1304,7 @@ namespace AZ // Add all auto loadable non-asset gems to the list of gem modules to load if (!moduleLoadData.m_autoLoad) { - break; + continue; } for (AZ::OSString& dynamicLibraryPath : moduleLoadData.m_dynamicLibraryPaths) { From b5e87d3601d6fadf2a7f92531d317463efbb0cec Mon Sep 17 00:00:00 2001 From: mbalfour Date: Tue, 11 May 2021 16:13:04 -0500 Subject: [PATCH 09/15] Change SerializeContextTools into a ToolsApplication so that it can correctly read in slice data: - Uses ToolsApplication instead of ComponentApplication so that built-in Editor components are recognized and read in correctly - Starts up all the DynamicModules immediately so that the System Components are activated, which registers asset handlers and allows asset references to serialize in correctly - Adds a -specialization command-line flag to specify which project specialization to use (editor, game, etc) - Removes the filter to ignore unknown classes since they should all now be "known" - Adds a few gem autoload flags and a null thumbnail service so that Qt and Python systems can be skipped, as they aren't needed for the data conversions and would bring additional overhead and complications --- .../SerializeContextTools/Application.cpp | 43 ++++++++++++++++--- .../Tools/SerializeContextTools/Application.h | 5 ++- .../SerializeContextTools/CMakeLists.txt | 2 + .../Tools/SerializeContextTools/Utilities.cpp | 1 - Code/Tools/SerializeContextTools/main.cpp | 7 +-- .../gem_autoload.serializecontexttools.setreg | 15 +++++++ 6 files changed, 59 insertions(+), 14 deletions(-) create mode 100644 Registry/gem_autoload.serializecontexttools.setreg diff --git a/Code/Tools/SerializeContextTools/Application.cpp b/Code/Tools/SerializeContextTools/Application.cpp index 2ab9847aa6..72e68ebb1e 100644 --- a/Code/Tools/SerializeContextTools/Application.cpp +++ b/Code/Tools/SerializeContextTools/Application.cpp @@ -16,12 +16,23 @@ #include #include +#include + namespace AZ { + // SerializeContextTools is a full ToolsApplication that will load a project's Gem DLLs and initialize the system components. + // This level of initialization is required to get all the serialization contexts and asset handlers registered, so that when + // data transformations take place, none of the data is dropped due to not being recognized. + // However, as a simplification, anything requiring Python or Qt is skipped during initialization: + // - The gem_autoload.serializecontexttools.setreg file disables autoload for QtForPython, EditorPythonBindings, and PythonAssetBuilder + // - The system component initialization below uses ThumbnailerNullComponent so that other components relying on a ThumbnailService + // can still be started up, but the thumbnail service itself won't do anything. The real ThumbnailerComponent uses Qt, which is why + // it isn't used. + namespace SerializeContextTools { Application::Application(int argc, char** argv) - : AZ::ComponentApplication(argc, argv) + : AzToolsFramework::ToolsApplication(&argc, &argv) { AZ::IO::FixedMaxPath projectPath = AZ::Utils::GetProjectPath(); if (projectPath.empty()) @@ -51,14 +62,25 @@ namespace AZ else { AZ::SettingsRegistryInterface::Specializations projectSpecializations{ projectName }; - AZ::IO::PathView configFilenameStem = m_configFilePath.Stem(); - if (AZ::StringFunc::Equal(configFilenameStem.Native(), "Editor")) + + // If a project specialization has been passed in via the command line, use it. + if (m_commandLine.HasSwitch("specialization")) { - projectSpecializations.Append("editor"); + AZStd::string specialization = m_commandLine.GetSwitchValue("specialization", 0); + projectSpecializations.Append(specialization); } - else if (AZ::StringFunc::Equal(configFilenameStem.Native(), "Game")) + // Otherwise, if a config file was passed in, auto-set the specialization based on the config file name. + else { - projectSpecializations.Append(projectName + "_GameLauncher"); + AZ::IO::PathView configFilenameStem = m_configFilePath.Stem(); + if (AZ::StringFunc::Equal(configFilenameStem.Native(), "Editor")) + { + projectSpecializations.Append("editor"); + } + else if (AZ::StringFunc::Equal(configFilenameStem.Native(), "Game")) + { + projectSpecializations.Append(projectName + "_GameLauncher"); + } } // Used the project specializations to merge the build dependencies *.setreg files @@ -78,5 +100,14 @@ namespace AZ AZ::ComponentApplication::SetSettingsRegistrySpecializations(specializations); specializations.Append("serializecontexttools"); } + + AZ::ComponentTypeList Application::GetRequiredSystemComponents() const + { + // Use all of the default system components, but also add in the ThumbnailerNullComponent so that components requiring + // a ThumbnailService can still be started up. + AZ::ComponentTypeList components = AzToolsFramework::ToolsApplication::GetRequiredSystemComponents(); + components.emplace_back(azrtti_typeid()); + return components; + } } // namespace SerializeContextTools } // namespace AZ diff --git a/Code/Tools/SerializeContextTools/Application.h b/Code/Tools/SerializeContextTools/Application.h index 63bc1892ed..b1b818e27d 100644 --- a/Code/Tools/SerializeContextTools/Application.h +++ b/Code/Tools/SerializeContextTools/Application.h @@ -12,7 +12,7 @@ #pragma once -#include +#include #include namespace AZ @@ -20,13 +20,14 @@ namespace AZ namespace SerializeContextTools { class Application final - : public AZ::ComponentApplication + : public AzToolsFramework::ToolsApplication { public: Application(int argc, char** argv); ~Application() override = default; const char* GetConfigFilePath() const; + AZ::ComponentTypeList GetRequiredSystemComponents() const override; protected: void SetSettingsRegistrySpecializations(AZ::SettingsRegistryInterface::Specializations& specializations) override; diff --git a/Code/Tools/SerializeContextTools/CMakeLists.txt b/Code/Tools/SerializeContextTools/CMakeLists.txt index 1a993de53c..e73fd014ab 100644 --- a/Code/Tools/SerializeContextTools/CMakeLists.txt +++ b/Code/Tools/SerializeContextTools/CMakeLists.txt @@ -29,4 +29,6 @@ ly_add_target( BUILD_DEPENDENCIES PRIVATE AZ::AzCore + AZ::AzFramework + AZ::AzToolsFramework ) diff --git a/Code/Tools/SerializeContextTools/Utilities.cpp b/Code/Tools/SerializeContextTools/Utilities.cpp index 127f833f6b..1c50466443 100644 --- a/Code/Tools/SerializeContextTools/Utilities.cpp +++ b/Code/Tools/SerializeContextTools/Utilities.cpp @@ -236,7 +236,6 @@ namespace AZ::SerializeContextTools AZ::IO::MemoryStream stream(data.data(), fileLength); ObjectStream::FilterDescriptor filter; - filter.m_flags = ObjectStream::FILTERFLAG_IGNORE_UNKNOWN_CLASSES; // Never load dependencies. That's another file that would need to be processed // separately from this one. filter.m_assetCB = AZ::Data::AssetFilterNoAssetLoading; diff --git a/Code/Tools/SerializeContextTools/main.cpp b/Code/Tools/SerializeContextTools/main.cpp index ee505c9cc0..4bff597d81 100644 --- a/Code/Tools/SerializeContextTools/main.cpp +++ b/Code/Tools/SerializeContextTools/main.cpp @@ -23,6 +23,7 @@ void PrintHelp() AZ_Printf("Help", "Serialize Context Tool\n"); AZ_Printf("Help", " [-config] *\n"); AZ_Printf("Help", " [opt] -config=: optional path to application's config file. Default is 'config/editor.xml'.\n"); + AZ_Printf("Help", " [opt] -specialization=: optional Registry project specialization, such as 'editor' or 'game'. Default is none. \n"); AZ_Printf("Help", "\n"); AZ_Printf("Help", " 'help': Print this help\n"); AZ_Printf("Help", " example: 'help'\n"); @@ -81,11 +82,7 @@ int main(int argc, char** argv) bool result = false; Application application(argc, argv); AZ::ComponentApplication::StartupParameters startupParameters; - startupParameters.m_loadDynamicModules = false; - application.Create({}, startupParameters); - // Load the DynamicModules after the Application starts to prevent Gem System Components - // from activating - application.LoadDynamicModules(); + application.Start({}, startupParameters); const AZ::CommandLine* commandLine = application.GetAzCommandLine(); if (commandLine->GetNumMiscValues() < 1) diff --git a/Registry/gem_autoload.serializecontexttools.setreg b/Registry/gem_autoload.serializecontexttools.setreg new file mode 100644 index 0000000000..1f4a8931c5 --- /dev/null +++ b/Registry/gem_autoload.serializecontexttools.setreg @@ -0,0 +1,15 @@ +{ + "Amazon": { + "Gems": { + "QtForPython.Editor": { + "AutoLoad": false + }, + "EditorPythonBindings.Editor": { + "AutoLoad": false + }, + "PythonAssetBuilder.Editor": { + "AutoLoad": false + } + } + } +} From ae9b36c135ac17d1e3a22e626084b4a4e359717e Mon Sep 17 00:00:00 2001 From: mbalfour Date: Wed, 12 May 2021 08:56:49 -0500 Subject: [PATCH 10/15] PR feedback - now allows for multiple specializations on the command-line, and changed the switch name to "specializations" to reflect that. --- Code/Tools/SerializeContextTools/Application.cpp | 8 +++++--- Code/Tools/SerializeContextTools/main.cpp | 3 ++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/Code/Tools/SerializeContextTools/Application.cpp b/Code/Tools/SerializeContextTools/Application.cpp index 72e68ebb1e..da0cca5ca6 100644 --- a/Code/Tools/SerializeContextTools/Application.cpp +++ b/Code/Tools/SerializeContextTools/Application.cpp @@ -64,10 +64,12 @@ namespace AZ AZ::SettingsRegistryInterface::Specializations projectSpecializations{ projectName }; // If a project specialization has been passed in via the command line, use it. - if (m_commandLine.HasSwitch("specialization")) + if (size_t specializationCount = m_commandLine.GetNumSwitchValues("specializations"); specializationCount > 0) { - AZStd::string specialization = m_commandLine.GetSwitchValue("specialization", 0); - projectSpecializations.Append(specialization); + for (size_t specializationIndex = 0; specializationIndex < specializationCount; ++specializationIndex) + { + projectSpecializations.Append(m_commandLine.GetSwitchValue("specializations", specializationIndex)); + } } // Otherwise, if a config file was passed in, auto-set the specialization based on the config file name. else diff --git a/Code/Tools/SerializeContextTools/main.cpp b/Code/Tools/SerializeContextTools/main.cpp index 4bff597d81..fb2dc442ed 100644 --- a/Code/Tools/SerializeContextTools/main.cpp +++ b/Code/Tools/SerializeContextTools/main.cpp @@ -23,7 +23,8 @@ void PrintHelp() AZ_Printf("Help", "Serialize Context Tool\n"); AZ_Printf("Help", " [-config] *\n"); AZ_Printf("Help", " [opt] -config=: optional path to application's config file. Default is 'config/editor.xml'.\n"); - AZ_Printf("Help", " [opt] -specialization=: optional Registry project specialization, such as 'editor' or 'game'. Default is none. \n"); + AZ_Printf("Help", " [opt] -specializations=: -separated list of optional Registry project\n"); + AZ_Printf("Help", " specializations, such as 'editor' or 'game' or 'editor;test'. Default is none. \n"); AZ_Printf("Help", "\n"); AZ_Printf("Help", " 'help': Print this help\n"); AZ_Printf("Help", " example: 'help'\n"); From 3383cfc95a33145114893ba6dd98cc9775a90ab5 Mon Sep 17 00:00:00 2001 From: amzn-sj Date: Wed, 12 May 2021 10:20:03 -0700 Subject: [PATCH 11/15] Fix Jenkins failure during engine registration for iOS --- scripts/build/Platform/iOS/build_config.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/build/Platform/iOS/build_config.json b/scripts/build/Platform/iOS/build_config.json index 4566d243ee..bb5f2d2fe6 100644 --- a/scripts/build/Platform/iOS/build_config.json +++ b/scripts/build/Platform/iOS/build_config.json @@ -27,7 +27,7 @@ "PARAMETERS": { "CONFIGURATION": "debug", "OUTPUT_DIRECTORY": "build/ios", - "CMAKE_OPTIONS": "-G Xcode -DCMAKE_TOOLCHAIN_FILE=cmake/Platform/iOS/Toolchain_ios.cmake -DLY_MONOLITHIC_GAME=TRUE -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGNING_REQUIRED=FALSE -DLY_IOS_CODE_SIGNING_IDENTITY=\"\" -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGN_ENTITLEMENTS=\"\" -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGNING_ALLOWED=FALSE -DLY_UNITY_BUILD=TRUE -DO3DE_HOME_PATH=\"!WORKSPACE!/home\" -DO3DE_REGISTER_ENGINE_PATH=\"!WORKSPACE!/o3de\" -DO3DE_REGISTER_THIS_ENGINE=TRUE", + "CMAKE_OPTIONS": "-G Xcode -DCMAKE_TOOLCHAIN_FILE=cmake/Platform/iOS/Toolchain_ios.cmake -DLY_MONOLITHIC_GAME=TRUE -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGNING_REQUIRED=FALSE -DLY_IOS_CODE_SIGNING_IDENTITY=\"\" -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGN_ENTITLEMENTS=\"\" -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGNING_ALLOWED=FALSE -DLY_UNITY_BUILD=TRUE -DO3DE_HOME_PATH=\"${WORKSPACE}/home\" -DO3DE_REGISTER_ENGINE_PATH=\"${WORKSPACE}/o3de\" -DO3DE_REGISTER_THIS_ENGINE=TRUE", "CMAKE_LY_PROJECTS": "AutomatedTesting", "CMAKE_TARGET": "ALL_BUILD", "CMAKE_NATIVE_BUILD_ARGS": "-destination generic/platform=iOS" @@ -44,7 +44,7 @@ "PARAMETERS": { "CONFIGURATION": "profile", "OUTPUT_DIRECTORY": "build/ios", - "CMAKE_OPTIONS": "-G Xcode -DCMAKE_TOOLCHAIN_FILE=cmake/Platform/iOS/Toolchain_ios.cmake -DLY_MONOLITHIC_GAME=TRUE -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGNING_REQUIRED=FALSE -DLY_IOS_CODE_SIGNING_IDENTITY=\"\" -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGN_ENTITLEMENTS=\"\" -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGNING_ALLOWED=FALSE -DLY_UNITY_BUILD=TRUE -DO3DE_HOME_PATH=\"!WORKSPACE!/home\" -DO3DE_REGISTER_ENGINE_PATH=\"!WORKSPACE!/o3de\" -DO3DE_REGISTER_THIS_ENGINE=TRUE", + "CMAKE_OPTIONS": "-G Xcode -DCMAKE_TOOLCHAIN_FILE=cmake/Platform/iOS/Toolchain_ios.cmake -DLY_MONOLITHIC_GAME=TRUE -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGNING_REQUIRED=FALSE -DLY_IOS_CODE_SIGNING_IDENTITY=\"\" -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGN_ENTITLEMENTS=\"\" -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGNING_ALLOWED=FALSE -DLY_UNITY_BUILD=TRUE -DO3DE_HOME_PATH=\"${WORKSPACE}/home\" -DO3DE_REGISTER_ENGINE_PATH=\"${WORKSPACE}/o3de\" -DO3DE_REGISTER_THIS_ENGINE=TRUE", "CMAKE_LY_PROJECTS": "AutomatedTesting", "CMAKE_TARGET": "ALL_BUILD", "CMAKE_NATIVE_BUILD_ARGS": "-destination generic/platform=iOS" @@ -60,7 +60,7 @@ "PARAMETERS": { "CONFIGURATION": "profile", "OUTPUT_DIRECTORY": "build/ios", - "CMAKE_OPTIONS": "-G Xcode -DCMAKE_TOOLCHAIN_FILE=cmake/Platform/iOS/Toolchain_ios.cmake -DLY_MONOLITHIC_GAME=TRUE -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGNING_REQUIRED=FALSE -DLY_IOS_CODE_SIGNING_IDENTITY=\"\" -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGN_ENTITLEMENTS=\"\" -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGNING_ALLOWED=FALSE -DLY_UNITY_BUILD=FALSE -DO3DE_HOME_PATH=\"!WORKSPACE!/home\" -DO3DE_REGISTER_ENGINE_PATH=\"!WORKSPACE!/o3de\" -DO3DE_REGISTER_THIS_ENGINE=TRUE", + "CMAKE_OPTIONS": "-G Xcode -DCMAKE_TOOLCHAIN_FILE=cmake/Platform/iOS/Toolchain_ios.cmake -DLY_MONOLITHIC_GAME=TRUE -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGNING_REQUIRED=FALSE -DLY_IOS_CODE_SIGNING_IDENTITY=\"\" -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGN_ENTITLEMENTS=\"\" -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGNING_ALLOWED=FALSE -DLY_UNITY_BUILD=FALSE -DO3DE_HOME_PATH=\"${WORKSPACE}/home\" -DO3DE_REGISTER_ENGINE_PATH=\"${WORKSPACE}/o3de\" -DO3DE_REGISTER_THIS_ENGINE=TRUE", "CMAKE_LY_PROJECTS": "AutomatedTesting", "CMAKE_TARGET": "ALL_BUILD", "CMAKE_NATIVE_BUILD_ARGS": "-destination generic/platform=iOS" @@ -94,7 +94,7 @@ "PARAMETERS": { "CONFIGURATION": "release", "OUTPUT_DIRECTORY": "build/ios", - "CMAKE_OPTIONS": "-G Xcode -DCMAKE_TOOLCHAIN_FILE=cmake/Platform/iOS/Toolchain_ios.cmake -DLY_MONOLITHIC_GAME=TRUE -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGNING_REQUIRED=FALSE -DLY_IOS_CODE_SIGNING_IDENTITY=\"\" -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGN_ENTITLEMENTS=\"\" -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGNING_ALLOWED=FALSE -DLY_UNITY_BUILD=TRUE -DO3DE_HOME_PATH=\"!WORKSPACE!/home\" -DO3DE_REGISTER_ENGINE_PATH=\"!WORKSPACE!/o3de\" -DO3DE_REGISTER_THIS_ENGINE=TRUE", + "CMAKE_OPTIONS": "-G Xcode -DCMAKE_TOOLCHAIN_FILE=cmake/Platform/iOS/Toolchain_ios.cmake -DLY_MONOLITHIC_GAME=TRUE -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGNING_REQUIRED=FALSE -DLY_IOS_CODE_SIGNING_IDENTITY=\"\" -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGN_ENTITLEMENTS=\"\" -DCMAKE_XCODE_ATTRIBUTE_CODE_SIGNING_ALLOWED=FALSE -DLY_UNITY_BUILD=TRUE -DO3DE_HOME_PATH=\"${WORKSPACE}/home\" -DO3DE_REGISTER_ENGINE_PATH=\"${WORKSPACE}/o3de\" -DO3DE_REGISTER_THIS_ENGINE=TRUE", "CMAKE_LY_PROJECTS": "AutomatedTesting", "CMAKE_TARGET": "ALL_BUILD", "CMAKE_NATIVE_BUILD_ARGS": "-destination generic/platform=iOS" From 3569ac9b873f1a996d306eb06646195574710095 Mon Sep 17 00:00:00 2001 From: Aristo7 <5432499+Aristo7@users.noreply.github.com> Date: Wed, 12 May 2021 13:13:28 -0500 Subject: [PATCH 12/15] Bringing back RHI modules to tools deps --- .../Source/Platform/Linux/additional_linux_tool_deps.cmake | 3 +++ .../Code/Source/Platform/Mac/additional_mac_tool_deps.cmake | 3 +++ .../Source/Platform/Windows/additional_windows_tool_deps.cmake | 3 +++ 3 files changed, 9 insertions(+) diff --git a/Gems/AtomLyIntegration/AtomBridge/Code/Source/Platform/Linux/additional_linux_tool_deps.cmake b/Gems/AtomLyIntegration/AtomBridge/Code/Source/Platform/Linux/additional_linux_tool_deps.cmake index 908417b1f8..c76527afa0 100644 --- a/Gems/AtomLyIntegration/AtomBridge/Code/Source/Platform/Linux/additional_linux_tool_deps.cmake +++ b/Gems/AtomLyIntegration/AtomBridge/Code/Source/Platform/Linux/additional_linux_tool_deps.cmake @@ -10,6 +10,9 @@ # set(LY_RUNTIME_DEPENDENCIES + Gem::Atom_RHI_Vulkan.Private + Gem::Atom_RHI_DX12.Private + Gem::Atom_RHI_Metal.Private Gem::Atom_RHI_Vulkan.Builders Gem::Atom_RHI_DX12.Builders Gem::Atom_RHI_Metal.Builders diff --git a/Gems/AtomLyIntegration/AtomBridge/Code/Source/Platform/Mac/additional_mac_tool_deps.cmake b/Gems/AtomLyIntegration/AtomBridge/Code/Source/Platform/Mac/additional_mac_tool_deps.cmake index 445a416a68..81046d3071 100644 --- a/Gems/AtomLyIntegration/AtomBridge/Code/Source/Platform/Mac/additional_mac_tool_deps.cmake +++ b/Gems/AtomLyIntegration/AtomBridge/Code/Source/Platform/Mac/additional_mac_tool_deps.cmake @@ -10,6 +10,9 @@ # set(LY_RUNTIME_DEPENDENCIES + Gem::Atom_RHI_Metal.Private + Gem::Atom_RHI_Vulkan.Private + Gem::Atom_RHI_DX12.Private Gem::Atom_RHI_Metal.Builders Gem::Atom_RHI_Vulkan.Builders Gem::Atom_RHI_DX12.Builders diff --git a/Gems/AtomLyIntegration/AtomBridge/Code/Source/Platform/Windows/additional_windows_tool_deps.cmake b/Gems/AtomLyIntegration/AtomBridge/Code/Source/Platform/Windows/additional_windows_tool_deps.cmake index 908417b1f8..c76527afa0 100644 --- a/Gems/AtomLyIntegration/AtomBridge/Code/Source/Platform/Windows/additional_windows_tool_deps.cmake +++ b/Gems/AtomLyIntegration/AtomBridge/Code/Source/Platform/Windows/additional_windows_tool_deps.cmake @@ -10,6 +10,9 @@ # set(LY_RUNTIME_DEPENDENCIES + Gem::Atom_RHI_Vulkan.Private + Gem::Atom_RHI_DX12.Private + Gem::Atom_RHI_Metal.Private Gem::Atom_RHI_Vulkan.Builders Gem::Atom_RHI_DX12.Builders Gem::Atom_RHI_Metal.Builders From b5d7ae829a4c44a9d89bb104b55135ffc9059d19 Mon Sep 17 00:00:00 2001 From: zsolleci Date: Wed, 12 May 2021 14:10:54 -0500 Subject: [PATCH 13/15] addressed internal review feedback --- .../scripting/Node_HappyPath_DuplicateNode.py | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/AutomatedTesting/Gem/PythonTests/scripting/Node_HappyPath_DuplicateNode.py b/AutomatedTesting/Gem/PythonTests/scripting/Node_HappyPath_DuplicateNode.py index 9d8cccd027..77d3b2fcde 100644 --- a/AutomatedTesting/Gem/PythonTests/scripting/Node_HappyPath_DuplicateNode.py +++ b/AutomatedTesting/Gem/PythonTests/scripting/Node_HappyPath_DuplicateNode.py @@ -12,9 +12,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # fmt: off class Tests(): - open_sc_window = ("Script Canvas window is opened", "Failed to open Script Canvas window") - node_added = ("Successfully added node to graph", "Failed to add node to graph") - node_duplicated = ("Successfully duplicated node", "Failed to duplicate the node") + node_duplicated = ("Successfully duplicated node", "Failed to duplicate the node") # fmt: on @@ -30,9 +28,8 @@ def Node_HappyPath_DuplicateNode(): 1) Open Script Canvas window (Tools > Script Canvas) 2) Open a new graph 3) Add node to graph - 4) Select node in graph to verify existence - 5) Mock Ctrl+D to duplicate node - 6) Verify the node was duplicated + 4) Duplicate node + 5) Verify the node was duplicated6) Verify the node was duplicated Note: - This test file must be called from the Open 3D Engine Editor command terminal @@ -68,11 +65,9 @@ def Node_HappyPath_DuplicateNode(): def grab_title_text(): scroll_area = node_inspector.findChild(QtWidgets.QScrollArea, "") QtTest.QTest.keyClick(graph, "a", Qt.ControlModifier, WAIT_FRAMES) - general.idle_wait(1.0) background = scroll_area.findChild(QtWidgets.QFrame, "Background") title = background.findChild(QtWidgets.QLabel, "Title") text = title.findChild(QtWidgets.QLabel, "Title") - print(text.text()) return text.text() # 1) Open Script Canvas window (Tools > Script Canvas) @@ -80,29 +75,35 @@ def Node_HappyPath_DuplicateNode(): general.open_pane("Script Canvas") helper.wait_for_condition(lambda: general.is_pane_visible("Script Canvas"), 5.0) - # # 2) Open a new graph + # 2) Open a new graph editor_window = pyside_utils.get_editor_main_window() sc = editor_window.findChild(QtWidgets.QDockWidget, "Script Canvas") sc_main = sc.findChild(QtWidgets.QMainWindow) create_new_graph = pyside_utils.find_child_by_pattern( sc_main, {"objectName": "action_New_Script", "type": QtWidgets.QAction} ) + if sc.findChild(QtWidgets.QDockWidget, "NodeInspector") is None: + action = pyside_utils.find_child_by_pattern(sc, {"text": "Node Inspector", "type": QtWidgets.QAction}) + action.trigger() node_inspector = sc.findChild(QtWidgets.QDockWidget, "NodeInspector") create_new_graph.trigger() # 3) Add node command_line_input("add_node Print") - # 4) Select node in graph to verify existence + # 4) Duplicate node graph_view = sc.findChild(QtWidgets.QFrame, "graphicsViewFrame") graph = graph_view.findChild(QtWidgets.QWidget, "") - - # 5) Duplicate node + # There are currently no utilities available to directly duplicate the node, + # therefore the node is selected using CTRL+A on the graph to select + # it and then CTRL+D to duplicate sc_main.activateWindow() QtTest.QTest.keyClick(graph, "a", Qt.ControlModifier, WAIT_FRAMES) QtTest.QTest.keyClick(graph, "d", Qt.ControlModifier, WAIT_FRAMES) - # 6) Verify the node was duplicated + # 5) Verify the node was duplicated + # As direct interaction with node is not available the text on the label + # inside the Node Inspector is validated showing two nodes exist after_dup = grab_title_text() Report.result(Tests.node_duplicated, after_dup == EXPECTED_STRING) From 98a579abefc0ac64e6ee4ae021c7e8e824dafaa5 Mon Sep 17 00:00:00 2001 From: srikappa Date: Wed, 12 May 2021 12:17:36 -0700 Subject: [PATCH 14/15] Added a comment --- .../AzToolsFramework/Prefab/PrefabPublicHandler.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp index 3214905721..9649818ed9 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp @@ -149,6 +149,7 @@ namespace AzToolsFramework undoBatch.GetUndoBatch(), containerEntityId, false); }); + // Create a link between the templates of the newly created instance and the instance it's being parented under. CreateLink( topLevelEntities, instanceToCreate->get(), commonRootEntityOwningInstance->get().GetTemplateId(), undoBatch.GetUndoBatch(), commonRootEntityId); From 50abafbab1bc0033395654c7491bfd3c2720d441 Mon Sep 17 00:00:00 2001 From: Steve Pham <82231385+spham-amzn@users.noreply.github.com> Date: Wed, 12 May 2021 14:31:39 -0700 Subject: [PATCH 15/15] Add orchestrating script to install ubuntu packages for O3DE on Linux (#721) SPEC-3510: Linux environment setup scripts * Add orchestrating script to install ubuntu packages for O3DE on Linux * Fix issue parsing package content file for build-tools --- .../Platform/Linux/install-ubuntu-awscli.sh | 2 +- .../Linux/install-ubuntu-build-tools.sh | 21 ++++---- .../Platform/Linux/install-ubuntu-git.sh | 8 ++-- .../Platform/Linux/install-ubuntu.sh | 48 +++++++++++++++++++ 4 files changed, 64 insertions(+), 15 deletions(-) create mode 100755 scripts/build/build_node/Platform/Linux/install-ubuntu.sh diff --git a/scripts/build/build_node/Platform/Linux/install-ubuntu-awscli.sh b/scripts/build/build_node/Platform/Linux/install-ubuntu-awscli.sh index 4c2f350859..8649f61fd1 100755 --- a/scripts/build/build_node/Platform/Linux/install-ubuntu-awscli.sh +++ b/scripts/build/build_node/Platform/Linux/install-ubuntu-awscli.sh @@ -38,7 +38,7 @@ then ./aws/install rm -rf ./aws else - AWS_CLI_VERSION=`aws --version | awk '{print $1}' | awk -F/ '{print $2}'` + AWS_CLI_VERSION=$(aws --version | awk '{print $1}' | awk -F/ '{print $2}') echo AWS CLI \(version $AWS_CLI_VERSION\) already installed fi diff --git a/scripts/build/build_node/Platform/Linux/install-ubuntu-build-tools.sh b/scripts/build/build_node/Platform/Linux/install-ubuntu-build-tools.sh index fd7b59592a..6fea3b03fa 100755 --- a/scripts/build/build_node/Platform/Linux/install-ubuntu-build-tools.sh +++ b/scripts/build/build_node/Platform/Linux/install-ubuntu-build-tools.sh @@ -26,7 +26,7 @@ then exit 1 fi -UBUNTU_DISTRO="`lsb_release -c | awk '{print $2}'`" +UBUNTU_DISTRO="$(lsb_release -c | awk '{print $2}')" if [ "$UBUNTU_DISTRO" == "bionic" ] then echo "Setup for Ubuntu 18.04 LTS ($UBUNTU_DISTRO)" @@ -53,7 +53,7 @@ fi # will install it from the bionic distro manually into focal. This is needed since Ubuntu 20.04 supports # python 3.8 out of the box, but we are using 3.7 # -LIBFFI6_COUNT=`apt list --installed 2>/dev/null | grep libffi6 | wc -l` +LIBFFI6_COUNT=$(apt list --installed 2>/dev/null | grep libffi6 | wc -l) if [ "$UBUNTU_DISTRO" == "focal" ] && [ $LIBFFI6_COUNT -eq 0 ] then echo "Installing libffi for Ubuntu 20.04" @@ -90,7 +90,7 @@ fi # Add the kitware repository for cmake if necessary # -KITWARE_REPO_COUNT=`cat /etc/apt/sources.list | grep ^deb | grep https://apt.kitware.com/ubuntu/ | wc -l` +KITWARE_REPO_COUNT=$(cat /etc/apt/sources.list | grep ^deb | grep https://apt.kitware.com/ubuntu/ | wc -l) if [ $KITWARE_REPO_COUNT -eq 0 ] then @@ -121,33 +121,34 @@ PACKAGE_FILE_LIST=package-list.ubuntu-$UBUNTU_DISTRO.txt echo Reading package list $PACKAGE_FILE_LIST # Read each line (strip out comment tags) -for LINE in `cat $PACKAGE_FILE_LIST | sed 's/#.*$//g'` +for PREPROC_LINE in $(cat $PACKAGE_FILE_LIST | sed 's/#.*$//g') do - PACKAGE=`echo $LINE | awk -F / '{print $1}'` + LINE=$(echo $PREPROC_LINE | tr -d '\r\n') + PACKAGE=$(echo $LINE | awk -F / '{$1=$1;print $1}') if [ "$PACKAGE" != "" ] # Skip blank lines then - PACKAGE_VER=`echo $LINE | awk -F / '{print $2}'` + PACKAGE_VER=$(echo $LINE | awk -F / '{$2=$2;print $2}') if [ "$PACKAGE_VER" == "" ] then # Process non-versioned packages - INSTALLED_COUNT=`apt list --installed 2>/dev/null | grep ^$PACKAGE/ | wc -l` + INSTALLED_COUNT=$(apt list --installed 2>/dev/null | grep ^$PACKAGE/ | wc -l) if [ $INSTALLED_COUNT -eq 0 ] then echo Installing $PACKAGE apt-get install $PACKAGE -y else - INSTALLED_VERSION=`apt list --installed 2>/dev/null | grep ^$PACKAGE/ | awk '{print $2}'` + INSTALLED_VERSION=$(apt list --installed 2>/dev/null | grep ^$PACKAGE/ | awk '{print $2}') echo $PACKAGE already installed \(version $INSTALLED_VERSION\) fi else # Process versioned packages - INSTALLED_COUNT=`apt list --installed 2>/dev/null | grep ^$PACKAGE/ | wc -l` + INSTALLED_COUNT=$(apt list --installed 2>/dev/null | grep ^$PACKAGE/ | wc -l) if [ $INSTALLED_COUNT -eq 0 ] then echo Installing $PACKAGE \( $PACKAGE_VER \) apt-get install $PACKAGE=$PACKAGE_VER -y else - INSTALLED_VERSION=`apt list --installed 2>/dev/null | grep ^$PACKAGE/ | awk '{print $2}'` + INSTALLED_VERSION=$(apt list --installed 2>/dev/null | grep ^$PACKAGE/ | awk '{print $2}') if [ "$INSTALLED_VERSION" != "$PACKAGE_VER" ] then echo $PACKAGE already installed but with the wrong version. Purging the package diff --git a/scripts/build/build_node/Platform/Linux/install-ubuntu-git.sh b/scripts/build/build_node/Platform/Linux/install-ubuntu-git.sh index 53554f4175..028b0082d7 100755 --- a/scripts/build/build_node/Platform/Linux/install-ubuntu-git.sh +++ b/scripts/build/build_node/Platform/Linux/install-ubuntu-git.sh @@ -26,7 +26,7 @@ then exit 1 fi -UBUNTU_DISTRO="`lsb_release -c | awk '{print $2}'`" +UBUNTU_DISTRO="$(lsb_release -c | awk '{print $2}')" if [ "$UBUNTU_DISTRO" == "bionic" ] then echo "Setup for Ubuntu 18.04 LTS ($UBUNTU_DISTRO)" @@ -49,14 +49,14 @@ then apt-get update apt-get install git -y else - GIT_VERSION=`git --version | awk '{print $3}'` + GIT_VERSION=$(git --version | awk '{print $3}') echo Git $GIT_VERSION already Installed. Skipping Git installation fi # # Setup Git-LFS if needed # -GIT_LFS_PACKAGE_COUNT=`apt list --installed 2>/dev/null | grep git-lfs/ | wc -l` +GIT_LFS_PACKAGE_COUNT=$(apt list --installed 2>/dev/null | grep git-lfs/ | wc -l) if [ $GIT_LFS_PACKAGE_COUNT -eq 0 ] then echo Setting up Git-LFS @@ -87,7 +87,7 @@ then dpkg -i $GCM_PACKAGE_NAME popd else - GCM_VERSION=`git-credential-manager-core --version` + GCM_VERSION=$(git-credential-manager-core --version) echo Git Credential Manager \(GCM\) version $GCM_VERSION already installed. Skipping GCM installation fi diff --git a/scripts/build/build_node/Platform/Linux/install-ubuntu.sh b/scripts/build/build_node/Platform/Linux/install-ubuntu.sh new file mode 100755 index 0000000000..9cdc94fa9e --- /dev/null +++ b/scripts/build/build_node/Platform/Linux/install-ubuntu.sh @@ -0,0 +1,48 @@ +#!/bin/bash + +# All or portions of this file Copyright (c) Amazon.com, Inc. or its affiliates or +# its licensors. +# +# For complete copyright and license terms please see the LICENSE at the root of this +# distribution (the "License"). All use of this software is governed by the License, +# or, if provided, by the license below or the license accompanying this file. Do not +# remove or modify any license notices. This file is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + +# This script must be run as root +if [[ $EUID -ne 0 ]] +then + echo "This script must be run as root (sudo)" + exit 1 +fi + +echo Installing packages and tools for O3DE development + +# Install awscli +./install-ubuntu-awscli.sh +if [ $? -ne 0 ] +then + echo Error installing AWSCLI + exit 1 +fi + + +# Install git +./install-ubuntu-git.sh +if [ $? -ne 0 ] +then + echo Error installing Git + exit 1 +fi + +# Install the necessary build tools +./install-ubuntu-build-tools.sh +if [ $? -ne 0 ] +then + echo Error installing ubuntu tools + exit 1 +fi + + +echo Packages and tools for O3DE setup complete +exit 0