Fixing undo/redo not updating transform pivot point (#4375)

* Fixing undo/redo not updating transform pivot point

Signed-off-by: Mikhail Naumov <mnaumov@amazon.com>

* PR feedback

Signed-off-by: Mikhail Naumov <mnaumov@amazon.com>

* PR feedback

Signed-off-by: Mikhail Naumov <mnaumov@amazon.com>

* fixing non-redo operations to still use batching (fixes some tests)

Signed-off-by: Mikhail Naumov <mnaumov@amazon.com>

Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplateInterface.h

@@ -46,10 +46,11 @@ namespace AzToolsFramework
             //! Updates the template links (updating instances) for the given template and triggers propagation on its instances.
             //! @param providedPatch The patch to apply to the template.
             //! @param templateId The id of the template to update.
+            //! @param immediate An optional flag whether to apply the patch immediately (needed for Undo/Redos) or wait until next system tick.
             //! @param instanceToExclude An optional reference to an instance of the template being updated that should not be refreshes as part of propagation.
             //!     Defaults to nullopt, which means that all instances will be refreshed.
             //! @return True if the template was patched correctly, false if the operation failed.
-            virtual bool PatchTemplate(PrefabDomValue& providedPatch, TemplateId templateId, InstanceOptionalReference instanceToExclude = AZStd::nullopt) = 0;
+            virtual bool PatchTemplate(PrefabDomValue& providedPatch, TemplateId templateId, bool immediate = false, InstanceOptionalReference instanceToExclude = AZStd::nullopt) = 0;
 
             virtual void ApplyPatchesToInstance(const AZ::EntityId& entityId, PrefabDom& patches, const Instance& instanceToAddPatches) = 0;
 

Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.cpp

@@ -156,7 +156,7 @@ namespace AzToolsFramework
             }
         }
 
-        bool InstanceToTemplatePropagator::PatchTemplate(PrefabDomValue& providedPatch, TemplateId templateId, InstanceOptionalReference instanceToExclude)
+        bool InstanceToTemplatePropagator::PatchTemplate(PrefabDomValue& providedPatch, TemplateId templateId, bool immediate, InstanceOptionalReference instanceToExclude)
         {
             PrefabDom& templateDomReference = m_prefabSystemComponentInterface->FindTemplateDom(templateId);
 
@@ -178,7 +178,7 @@ namespace AzToolsFramework
                     (result.GetOutcome() != AZ::JsonSerializationResult::Outcomes::PartialSkip),
                     "Some of the patches were not successfully applied.");
                 m_prefabSystemComponentInterface->SetTemplateDirtyFlag(templateId, true);
-                m_prefabSystemComponentInterface->PropagateTemplateChanges(templateId, instanceToExclude);
+                m_prefabSystemComponentInterface->PropagateTemplateChanges(templateId, immediate, instanceToExclude);
                 return true;
             }
         }

Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.h

@@ -33,7 +33,7 @@ namespace AzToolsFramework
 
             InstanceOptionalReference GetTopMostInstanceInHierarchy(AZ::EntityId entityId) override;
 
-            bool PatchTemplate(PrefabDomValue& providedPatch, TemplateId templateId, InstanceOptionalReference instanceToExclude = AZStd::nullopt) override;
+            bool PatchTemplate(PrefabDomValue& providedPatch, TemplateId templateId, bool immediate = false, InstanceOptionalReference instanceToExclude = AZStd::nullopt) override;
 
             void ApplyPatchesToInstance(const AZ::EntityId& entityId, PrefabDom& patches, const Instance& instanceToAddPatches) override;
 

Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceUpdateExecutor.cpp

@@ -52,7 +52,7 @@ namespace AzToolsFramework
             AZ::Interface<InstanceUpdateExecutorInterface>::Unregister(this);
         }
 
-        void InstanceUpdateExecutor::AddTemplateInstancesToQueue(TemplateId instanceTemplateId, InstanceOptionalReference instanceToExclude)
+        void InstanceUpdateExecutor::AddTemplateInstancesToQueue(TemplateId instanceTemplateId, bool immediate, InstanceOptionalReference instanceToExclude)
         {
             auto findInstancesResult =
                 m_templateInstanceMapperInterface->FindInstancesOwnedByTemplate(instanceTemplateId);
@@ -79,6 +79,11 @@ namespace AzToolsFramework
                     m_instancesUpdateQueue.emplace_back(instance);
                 }
             }
+
+            if (immediate)
+            {
+                UpdateTemplateInstancesInQueue();
+            }
         }
 
         void InstanceUpdateExecutor::RemoveTemplateInstanceFromQueue(const Instance* instance)

Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceUpdateExecutor.h

@@ -31,7 +31,7 @@ namespace AzToolsFramework
 
             explicit InstanceUpdateExecutor(int instanceCountToUpdateInBatch = 0);
 
-            void AddTemplateInstancesToQueue(TemplateId instanceTemplateId, InstanceOptionalReference instanceToExclude = AZStd::nullopt) override;
+            void AddTemplateInstancesToQueue(TemplateId instanceTemplateId, bool immediate = false, InstanceOptionalReference instanceToExclude = AZStd::nullopt) override;
             bool UpdateTemplateInstancesInQueue() override;
             virtual void RemoveTemplateInstanceFromQueue(const Instance* instance) override;
 

Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceUpdateExecutorInterface.h

@@ -23,7 +23,7 @@ namespace AzToolsFramework
             virtual ~InstanceUpdateExecutorInterface() = default;
 
             // Add all Instances of Template with given Id into a queue for updating them later.
-            virtual void AddTemplateInstancesToQueue(TemplateId instanceTemplateId, InstanceOptionalReference instanceToExclude = AZStd::nullopt) = 0;
+            virtual void AddTemplateInstancesToQueue(TemplateId instanceTemplateId, bool immediate = false, InstanceOptionalReference instanceToExclude = AZStd::nullopt) = 0;
 
             // Update Instances in the waiting queue.
             virtual bool UpdateTemplateInstancesInQueue() = 0;

Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp

@@ -1041,7 +1041,7 @@ namespace AzToolsFramework
                 PrefabUndoInstance* command = aznew PrefabUndoInstance("Entity/Instance duplication");
                 command->SetParent(undoBatch.GetUndoBatch());
                 command->Capture(instanceDomBefore, instanceDomAfter, commonOwningInstance->get().GetTemplateId());
-                command->Redo();
+                command->RedoBatched();
 
                 DuplicateNestedInstancesInInstance(commonOwningInstance->get(),
                     instances, instanceDomAfter, duplicatedEntityAndInstanceIds, newInstanceAliasToOldInstanceMap);

Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp

@@ -146,9 +146,9 @@ namespace AzToolsFramework
             }
         }
 
-        void PrefabSystemComponent::PropagateTemplateChanges(TemplateId templateId, InstanceOptionalReference instanceToExclude)
+        void PrefabSystemComponent::PropagateTemplateChanges(TemplateId templateId, bool immediate, InstanceOptionalReference instanceToExclude)
         {
-            UpdatePrefabInstances(templateId, instanceToExclude);
+            UpdatePrefabInstances(templateId, immediate, instanceToExclude);
 
             auto templateIdToLinkIdsIterator = m_templateToLinkIdsMap.find(templateId);
             if (templateIdToLinkIdsIterator != m_templateToLinkIdsMap.end())
@@ -177,9 +177,9 @@ namespace AzToolsFramework
             }
         }
 
-        void PrefabSystemComponent::UpdatePrefabInstances(TemplateId templateId, InstanceOptionalReference instanceToExclude)
+        void PrefabSystemComponent::UpdatePrefabInstances(TemplateId templateId, bool immediate, InstanceOptionalReference instanceToExclude)
         {
-            m_instanceUpdateExecutor.AddTemplateInstancesToQueue(templateId, instanceToExclude);
+            m_instanceUpdateExecutor.AddTemplateInstancesToQueue(templateId, immediate, instanceToExclude);
         }
 
         void PrefabSystemComponent::UpdateLinkedInstances(AZStd::queue<LinkIds>& linkIdsQueue)

Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.h

@@ -230,14 +230,17 @@ namespace AzToolsFramework
              */
             void UpdatePrefabTemplate(TemplateId templateId, const PrefabDom& updatedDom) override;
 
-            void PropagateTemplateChanges(TemplateId templateId, InstanceOptionalReference instanceToExclude = AZStd::nullopt) override;
+            void PropagateTemplateChanges(TemplateId templateId, bool immediate = false, InstanceOptionalReference instanceToExclude = AZStd::nullopt) override;
 
             /**
              * Updates all Instances owned by a Template.
              *
              * @param templateId The id of the Template owning Instances to update.
+             * @param immediate An optional flag whether to apply the patch immediately (needed for Undo/Redos) or wait until next system tick.
+             * @param instanceToExclude An optional reference to an instance of the template being updated that should not be refreshes as part of propagation.
+             *     Defaults to nullopt, which means that all instances will be refreshed.
              */
-            void UpdatePrefabInstances(TemplateId templateId, InstanceOptionalReference instanceToExclude = AZStd::nullopt);
+            void UpdatePrefabInstances(TemplateId templateId, bool immediate = false, InstanceOptionalReference instanceToExclude = AZStd::nullopt);
 
         private:
             AZ_DISABLE_COPY_MOVE(PrefabSystemComponent);

Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponentInterface.h

@@ -67,7 +67,7 @@ namespace AzToolsFramework
 
             virtual PrefabDom& FindTemplateDom(TemplateId templateId) = 0;
             virtual void UpdatePrefabTemplate(TemplateId templateId, const PrefabDom& updatedDom) = 0;
-            virtual void PropagateTemplateChanges(TemplateId templateId, InstanceOptionalReference instanceToExclude = AZStd::nullopt) = 0;
+            virtual void PropagateTemplateChanges(TemplateId templateId, bool immediate = false, InstanceOptionalReference instanceToExclude = AZStd::nullopt) = 0;
 
             virtual AZStd::unique_ptr<Instance> InstantiatePrefab(
                 AZ::IO::PathView filePath, InstanceOptionalReference parent = AZStd::nullopt) = 0;

Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.cpp

@@ -43,10 +43,15 @@ namespace AzToolsFramework
 
         void PrefabUndoInstance::Undo()
         {
-            m_instanceToTemplateInterface->PatchTemplate(m_undoPatch, m_templateId);
+            m_instanceToTemplateInterface->PatchTemplate(m_undoPatch, m_templateId, true);
         }
 
         void PrefabUndoInstance::Redo()
+        {
+            m_instanceToTemplateInterface->PatchTemplate(m_redoPatch, m_templateId, true);
+        }
+
+        void PrefabUndoInstance::RedoBatched()
         {
             m_instanceToTemplateInterface->PatchTemplate(m_redoPatch, m_templateId);
         }
@@ -91,7 +96,7 @@ namespace AzToolsFramework
         void PrefabUndoEntityUpdate::Undo()
         {
             [[maybe_unused]] bool isPatchApplicationSuccessful =
-                m_instanceToTemplateInterface->PatchTemplate(m_undoPatch, m_templateId);
+                m_instanceToTemplateInterface->PatchTemplate(m_undoPatch, m_templateId, true);
 
             AZ_Error(
                 "Prefab", isPatchApplicationSuccessful,
@@ -102,7 +107,7 @@ namespace AzToolsFramework
         void PrefabUndoEntityUpdate::Redo()
         {
             [[maybe_unused]] bool isPatchApplicationSuccessful =
-                m_instanceToTemplateInterface->PatchTemplate(m_redoPatch, m_templateId);
+                m_instanceToTemplateInterface->PatchTemplate(m_redoPatch, m_templateId, true);
 
             AZ_Error(
                 "Prefab", isPatchApplicationSuccessful,
@@ -113,7 +118,7 @@ namespace AzToolsFramework
         void PrefabUndoEntityUpdate::Redo(InstanceOptionalReference instanceToExclude)
         {
             [[maybe_unused]] bool isPatchApplicationSuccessful =
-                m_instanceToTemplateInterface->PatchTemplate(m_redoPatch, m_templateId, instanceToExclude);
+                m_instanceToTemplateInterface->PatchTemplate(m_redoPatch, m_templateId, false, instanceToExclude);
 
             AZ_Error(
                 "Prefab", isPatchApplicationSuccessful,
@@ -329,7 +334,7 @@ namespace AzToolsFramework
 
             //propagate the link changes
             link->get().UpdateTarget();
-            m_prefabSystemComponentInterface->PropagateTemplateChanges(link->get().GetTargetTemplateId(), instanceToExclude);
+            m_prefabSystemComponentInterface->PropagateTemplateChanges(link->get().GetTargetTemplateId(), false, instanceToExclude);
 
             //mark as dirty
             m_prefabSystemComponentInterface->SetTemplateDirtyFlag(link->get().GetTargetTemplateId(), true);

Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.h

@@ -53,6 +53,7 @@ namespace AzToolsFramework
 
             void Undo() override;
             void Redo() override;
+            void RedoBatched();
         };
 
         //! handles entity updates, such as when the values on an entity change

Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndoHelpers.cpp

@@ -26,7 +26,7 @@ namespace AzToolsFramework
                 PrefabUndoInstance* state = aznew Prefab::PrefabUndoInstance(undoMessage);
                 state->Capture(instanceDomBeforeUpdate, instanceDomAfterUpdate, instance.GetTemplateId());
                 state->SetParent(undoBatch);
-                state->Redo();
+                state->RedoBatched();
             }
 
             LinkId CreateLink(