diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/ToolsComponents/TransformComponent.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/ToolsComponents/TransformComponent.cpp index 7ab7dfdfe4..1f056729d7 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/ToolsComponents/TransformComponent.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/ToolsComponents/TransformComponent.cpp @@ -860,18 +860,20 @@ namespace AzToolsFramework return; } - bool isDuringUndoRedo = false; - EBUS_EVENT_RESULT(isDuringUndoRedo, AzToolsFramework::ToolsApplicationRequests::Bus, IsDuringUndoRedo); - if (!isDuringUndoRedo) + bool suppressTransformChangedEvent = m_suppressTransformChangedEvent; + // temporarily disable calling OnTransformChanged, because CheckApplyCachedWorldTransform is not guaranteed + // to call it when m_cachedWorldTransform is identity. We send it manually later. + m_suppressTransformChangedEvent = false; + // When parent comes online, compute local TM from world TM. + CheckApplyCachedWorldTransform(parentTransform->GetWorldTM()); + if (!m_initialized) { - // When parent comes online, compute local TM from world TM. - CheckApplyCachedWorldTransform(parentTransform->GetWorldTM()); - } - else - { - // During undo operations, just apply our local TM. + m_initialized = true; + // If this is the first time this entity is being activated, manually compute OnTransformChanged + // this can occur when either the entity first created or undo/redo command is performed OnTransformChanged(AZ::Transform::Identity(), parentTransform->GetWorldTM()); } + m_suppressTransformChangedEvent = suppressTransformChangedEvent; auto& parentChildIds = GetParentTransformComponent()->m_childrenEntityIds; if (parentChildIds.end() == AZStd::find(parentChildIds.begin(), parentChildIds.end(), GetEntityId())) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/ToolsComponents/TransformComponent.h b/Code/Framework/AzToolsFramework/AzToolsFramework/ToolsComponents/TransformComponent.h index 931d3b48d6..08d1392d74 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/ToolsComponents/TransformComponent.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/ToolsComponents/TransformComponent.h @@ -242,6 +242,9 @@ namespace AzToolsFramework // element is used rather than a data element. bool m_addNonUniformScaleButton = false; + // Used to check whether entity was just created vs manually reactivated. Set true after OnEntityActivated is called the first time. + bool m_initialized = false; + // Deprecated AZ::InterpolationMode m_interpolatePosition; AZ::InterpolationMode m_interpolateRotation; diff --git a/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabTestFixture.cpp b/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabTestFixture.cpp index 494f635a9f..3067f66aa0 100644 --- a/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabTestFixture.cpp +++ b/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabTestFixture.cpp @@ -11,6 +11,8 @@ #include #include #include +#include +#include #include #include @@ -50,6 +52,15 @@ namespace UnitTest GetApplication()->RegisterComponentDescriptor(PrefabTestComponent::CreateDescriptor()); GetApplication()->RegisterComponentDescriptor(PrefabTestComponentWithUnReflectedTypeMember::CreateDescriptor()); + + AzToolsFramework::ToolsApplicationRequestBus::BroadcastResult( + m_undoStack, &AzToolsFramework::ToolsApplicationRequestBus::Events::GetUndoStack); + AZ_Assert(m_undoStack, "Failed to look up undo stack from tools application"); + } + + void PrefabTestFixture::TearDownEditorFixtureImpl() + { + m_undoStack = nullptr; } AZStd::unique_ptr PrefabTestFixture::CreateTestApplication() @@ -57,12 +68,25 @@ namespace UnitTest return AZStd::make_unique("PrefabTestApplication"); } + void PrefabTestFixture::CreateRootPrefab() + { + auto entityOwnershipService = AZ::Interface::Get(); + ASSERT_TRUE(entityOwnershipService != nullptr); + entityOwnershipService->CreateNewLevelPrefab("UnitTestRoot.prefab", ""); + auto rootEntityReference = entityOwnershipService->GetRootPrefabInstance()->get().GetContainerEntity(); + ASSERT_TRUE(rootEntityReference.has_value()); + auto& rootEntity = rootEntityReference->get(); + rootEntity.Deactivate(); + rootEntity.CreateComponent(); + rootEntity.Activate(); + } + void PrefabTestFixture::PropagateAllTemplateChanges() { m_prefabSystemComponent->OnSystemTick(); } - AZ::Entity* PrefabTestFixture::CreateEntity(const char* entityName, const bool shouldActivate) + AZ::Entity* PrefabTestFixture::CreateEntity(AZStd::string entityName, const bool shouldActivate) { // Circumvent the EntityContext system and generate a new entity with a transformcomponent AZ::Entity* newEntity = aznew AZ::Entity(entityName); @@ -76,8 +100,43 @@ namespace UnitTest return newEntity; } + AZ::EntityId PrefabTestFixture::CreateEntityUnderRootPrefab(AZStd::string name, AZ::EntityId parentId) + { + auto createResult = m_prefabPublicInterface->CreateEntity(parentId, AZ::Vector3()); + AZ_Assert(createResult.IsSuccess(), "Failed to create entity: %s", createResult.GetError().c_str()); + AZ::EntityId entityId = createResult.GetValue(); + + AZ::Entity* entity = nullptr; + AZ::ComponentApplicationBus::BroadcastResult(entity, &AZ::ComponentApplicationRequests::FindEntity, entityId); + + entity->Deactivate(); + + entity->SetName(name); + + // Normally, in invalid parent ID should automatically parent us to the root prefab, but currently in the unit test + // environment entities aren't created with a default transform component, so CreateEntity won't correctly parent. + // We get the actual target parent ID here, then create our missing transform component. + if (!parentId.IsValid()) + { + auto prefabEditorEntityOwnershipInterface = AZ::Interface::Get(); + parentId = prefabEditorEntityOwnershipInterface->GetRootPrefabInstance()->get().GetContainerEntityId(); + } + + auto transform = aznew AzToolsFramework::Components::TransformComponent; + transform->SetParent(parentId); + entity->AddComponent(transform); + + entity->Activate(); + + // Update our undo cache entry to include the rename / reparent as one atomic operation. + m_prefabPublicInterface->GenerateUndoNodesForEntityChangeAndUpdateCache(entityId, m_undoStack->GetTop()); + m_prefabSystemComponent->OnSystemTick(); + + return entityId; + } + void PrefabTestFixture::CompareInstances(const AzToolsFramework::Prefab::Instance& instanceA, - const AzToolsFramework::Prefab::Instance& instanceB, bool shouldCompareLinkIds, bool shouldCompareContainerEntities) + const AzToolsFramework::Prefab::Instance& instanceB, bool shouldCompareLinkIds, bool shouldCompareContainerEntities) { AzToolsFramework::Prefab::TemplateId templateAId = instanceA.GetTemplateId(); AzToolsFramework::Prefab::TemplateId templateBId = instanceB.GetTemplateId(); @@ -131,6 +190,24 @@ namespace UnitTest } } + void PrefabTestFixture::ProcessDeferredUpdates() + { + // Force a prefab propagation for updates that are deferred to the next tick. + m_prefabSystemComponent->OnSystemTick(); + } + + void PrefabTestFixture::Undo() + { + m_undoStack->Undo(); + ProcessDeferredUpdates(); + } + + void PrefabTestFixture::Redo() + { + m_undoStack->Redo(); + ProcessDeferredUpdates(); + } + void PrefabTestFixture::AddRequiredEditorComponents(AZ::Entity* entity) { ASSERT_TRUE(entity != nullptr); diff --git a/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabTestFixture.h b/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabTestFixture.h index e7fe771610..a4cc1c8a7a 100644 --- a/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabTestFixture.h +++ b/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabTestFixture.h @@ -49,13 +49,15 @@ namespace UnitTest inline static const char* CarPrefabMockFilePath = "SomePathToCar"; void SetUpEditorFixtureImpl() override; + void TearDownEditorFixtureImpl() override; AZStd::unique_ptr CreateTestApplication() override; + void CreateRootPrefab(); + AZ::Entity* CreateEntity(AZStd::string entityName, const bool shouldActivate = true); + AZ::EntityId CreateEntityUnderRootPrefab(AZStd::string name, AZ::EntityId parentId = AZ::EntityId()); void PropagateAllTemplateChanges(); - AZ::Entity* CreateEntity(const char* entityName, const bool shouldActivate = true); - void CompareInstances(const Instance& instanceA, const Instance& instanceB, bool shouldCompareLinkIds = true, bool shouldCompareContainerEntities = true); @@ -64,6 +66,15 @@ namespace UnitTest //! Validates that all entities within a prefab instance are in 'Active' state. void ValidateInstanceEntitiesActive(Instance& instance); + // Kicks off any updates scheduled for the next tick + virtual void ProcessDeferredUpdates(); + + // Performs an undo operation and ensures the tick-scheduled updates happen + void Undo(); + + // Performs a redo operation and ensures the tick-scheduled updates happen + void Redo(); + void AddRequiredEditorComponents(AZ::Entity* entity); PrefabSystemComponent* m_prefabSystemComponent = nullptr; @@ -71,5 +82,6 @@ namespace UnitTest PrefabPublicInterface* m_prefabPublicInterface = nullptr; InstanceUpdateExecutorInterface* m_instanceUpdateExecutorInterface = nullptr; InstanceToTemplateInterface* m_instanceToTemplateInterface = nullptr; + AzToolsFramework::UndoSystem::UndoStack* m_undoStack = nullptr; }; } diff --git a/Code/Framework/AzToolsFramework/Tests/TransformComponent.cpp b/Code/Framework/AzToolsFramework/Tests/TransformComponent.cpp index 4fbebc8526..3849ed23c9 100644 --- a/Code/Framework/AzToolsFramework/Tests/TransformComponent.cpp +++ b/Code/Framework/AzToolsFramework/Tests/TransformComponent.cpp @@ -18,7 +18,10 @@ #include #include +#include #include +#include +#include #include @@ -1063,4 +1066,67 @@ R"DELIMITER( } } } + + // Fixture provides a root prefab with Transform component and listens for TransformNotificationBus. + class TransformComponentActivationTest + : public PrefabTestFixture + , public TransformNotificationBus::Handler + { + protected: + void SetUpEditorFixtureImpl() override + { + PrefabTestFixture::SetUpEditorFixtureImpl(); + + CreateRootPrefab(); + } + + void TearDownEditorFixtureImpl() override + { + BusDisconnect(); + + PrefabTestFixture::TearDownEditorFixtureImpl(); + } + + void OnTransformChanged(const Transform& /*local*/, const Transform& /*world*/) override + { + m_transformUpdated = true; + } + + void MoveEntity(AZ::EntityId entityId) + { + AzToolsFramework::ScopedUndoBatch undoBatch("Move Entity"); + TransformBus::Event(entityId, &TransformInterface::SetWorldTranslation, Vector3(1.f, 0.f, 0.f)); + } + + bool m_transformUpdated = false; + }; + + TEST_F(TransformComponentActivationTest, TransformChangedEventIsSentWhenEntityIsActivatedViaUndoRedo) + { + AZ::EntityId entityId = CreateEntityUnderRootPrefab("Entity"); + MoveEntity(entityId); + BusConnect(entityId); + + // verify that undoing/redoing move operations fires TransformChanged event + Undo(); + EXPECT_TRUE(m_transformUpdated); + m_transformUpdated = false; + + Redo(); + EXPECT_TRUE(m_transformUpdated); + m_transformUpdated = false; + } + + TEST_F(TransformComponentActivationTest, TransformChangedEventIsNotSentWhenEntityIsDeactivatedAndActivated) + { + AZ::EntityId entityId = CreateEntityUnderRootPrefab("Entity"); + BusConnect(entityId); + + // verify that simply activating/deactivating an entity does not fire TransformChanged event + Entity* entity = nullptr; + ComponentApplicationBus::BroadcastResult(entity, &AZ::ComponentApplicationRequests::FindEntity, entityId); + entity->Deactivate(); + entity->Activate(); + EXPECT_FALSE(m_transformUpdated); + } } // namespace UnitTest diff --git a/Code/Framework/AzToolsFramework/Tests/UI/EntityOutlinerTests.cpp b/Code/Framework/AzToolsFramework/Tests/UI/EntityOutlinerTests.cpp index eb7d87fb78..36e2398162 100644 --- a/Code/Framework/AzToolsFramework/Tests/UI/EntityOutlinerTests.cpp +++ b/Code/Framework/AzToolsFramework/Tests/UI/EntityOutlinerTests.cpp @@ -37,16 +37,11 @@ namespace UnitTest m_model->Initialize(); m_modelTester = AZStd::make_unique(m_model.get(), QAbstractItemModelTester::FailureReportingMode::Fatal); - - AzToolsFramework::ToolsApplicationRequestBus::BroadcastResult( - m_undoStack, &AzToolsFramework::ToolsApplicationRequestBus::Events::GetUndoStack); - AZ_Assert(m_undoStack, "Failed to look up undo stack from tools application"); - + // Create a new root prefab - the synthetic "NewLevel.prefab" that comes in by default isn't suitable for outliner tests // because it's created before the EditorEntityModel that our EntityOutlinerListModel subscribes to, and we want to // recreate it as part of the fixture regardless. - auto entityOwnershipService = AZ::Interface::Get(); - entityOwnershipService->CreateNewLevelPrefab("UnitTestRoot.prefab", ""); + CreateRootPrefab(); } void TearDownEditorFixtureImpl() override @@ -125,7 +120,7 @@ namespace UnitTest } // Kicks off any updates scheduled for the next tick - void ProcessDeferredUpdates() + void ProcessDeferredUpdates() override { // Force a prefab propagation for updates that are deferred to the next tick. PropagateAllTemplateChanges(); @@ -133,24 +128,9 @@ namespace UnitTest // Ensure the model process its entity update queue m_model->ProcessEntityUpdates(); } - - // Performs an undo operation and ensures the tick-scheduled updates happen - void Undo() - { - m_undoStack->Undo(); - ProcessDeferredUpdates(); - } - - // Performs a redo operation and ensures the tick-scheduled updates happen - void Redo() - { - m_undoStack->Redo(); - ProcessDeferredUpdates(); - } - + AZStd::unique_ptr m_model; AZStd::unique_ptr m_modelTester; - AzToolsFramework::UndoSystem::UndoStack* m_undoStack = nullptr; }; TEST_F(EntityOutlinerTest, TestCreateFlatHierarchyUndoAndRedoWorks)