From 52095e3e165ccc0d1cab0d11da74e882efa73919 Mon Sep 17 00:00:00 2001 From: hultonha <82228511+hultonha@users.noreply.github.com> Date: Wed, 15 Sep 2021 11:12:26 +0100 Subject: [PATCH] Ensure undo/redo operation for change entity selection is atomic (#4122) --- .../EditorTransformComponentSelection.cpp | 258 +++++++++--------- ...EditorTransformComponentSelectionTests.cpp | 28 +- 2 files changed, 158 insertions(+), 128 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorTransformComponentSelection.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorTransformComponentSelection.cpp index 5a7b097e29..064fbb9da6 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorTransformComponentSelection.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorTransformComponentSelection.cpp @@ -86,59 +86,60 @@ namespace AzToolsFramework "Sticky select implies a single click will not change selection with an entity already selected"); // strings related to new viewport interaction model (EditorTransformComponentSelection) - static const char* const s_togglePivotTitleRightClick = "Toggle pivot"; - static const char* const s_togglePivotTitleEditMenu = "Toggle Pivot Location"; - static const char* const s_togglePivotDesc = "Toggle pivot location"; - static const char* const s_manipulatorUndoRedoName = "Manipulator Adjustment"; - static const char* const s_lockSelectionTitle = "Lock Selection"; - static const char* const s_lockSelectionDesc = "Lock the selected entities so that they can't be selected in the viewport"; - static const char* const s_hideSelectionTitle = "Hide Selection"; - static const char* const s_hideSelectionDesc = "Hide the selected entities so that they don't appear in the viewport"; - static const char* const s_unlockAllTitle = "Unlock All Entities"; - static const char* const s_unlockAllDesc = "Unlock all entities the level"; - static const char* const s_showAllTitle = "Show All"; - static const char* const s_showAllDesc = "Show all entities so that they appear in the viewport"; - static const char* const s_selectAllTitle = "Select All"; - static const char* const s_selectAllDesc = "Select all entities"; - static const char* const s_invertSelectionTitle = "Invert Selection"; - static const char* const s_invertSelectionDesc = "Invert the current entity selection"; - static const char* const s_duplicateTitle = "Duplicate"; - static const char* const s_duplicateDesc = "Duplicate selected entities"; - static const char* const s_deleteTitle = "Delete"; - static const char* const s_deleteDesc = "Delete selected entities"; - static const char* const s_resetEntityTransformTitle = "Reset Entity Transform"; - static const char* const s_resetEntityTransformDesc = "Reset transform based on manipulator mode"; - static const char* const s_resetManipulatorTitle = "Reset Manipulator"; - static const char* const s_resetManipulatorDesc = "Reset the manipulator to recenter it on the selected entity"; - static const char* const s_resetTransformLocalTitle = "Reset Transform (Local)"; - static const char* const s_resetTransformLocalDesc = "Reset transform to local space"; - static const char* const s_resetTransformWorldTitle = "Reset Transform (World)"; - static const char* const s_resetTransformWorldDesc = "Reset transform to world space"; + static const char* const TogglePivotTitleRightClick = "Toggle pivot"; + static const char* const TogglePivotTitleEditMenu = "Toggle Pivot Location"; + static const char* const TogglePivotDesc = "Toggle pivot location"; + static const char* const ManipulatorUndoRedoName = "Manipulator Adjustment"; + static const char* const LockSelectionTitle = "Lock Selection"; + static const char* const LockSelectionDesc = "Lock the selected entities so that they can't be selected in the viewport"; + static const char* const HideSelectionTitle = "Hide Selection"; + static const char* const HideSelectionDesc = "Hide the selected entities so that they don't appear in the viewport"; + static const char* const UnlockAllTitle = "Unlock All Entities"; + static const char* const UnlockAllDesc = "Unlock all entities the level"; + static const char* const ShowAllTitle = "Show All"; + static const char* const ShowAllDesc = "Show all entities so that they appear in the viewport"; + static const char* const SelectAllTitle = "Select All"; + static const char* const SelectAllDesc = "Select all entities"; + static const char* const InvertSelectionTitle = "Invert Selection"; + static const char* const InvertSelectionDesc = "Invert the current entity selection"; + static const char* const DuplicateTitle = "Duplicate"; + static const char* const DuplicateDesc = "Duplicate selected entities"; + static const char* const DeleteTitle = "Delete"; + static const char* const DeleteDesc = "Delete selected entities"; + static const char* const ResetEntityTransformTitle = "Reset Entity Transform"; + static const char* const ResetEntityTransformDesc = "Reset transform based on manipulator mode"; + static const char* const ResetManipulatorTitle = "Reset Manipulator"; + static const char* const ResetManipulatorDesc = "Reset the manipulator to recenter it on the selected entity"; + static const char* const ResetTransformLocalTitle = "Reset Transform (Local)"; + static const char* const ResetTransformLocalDesc = "Reset transform to local space"; + static const char* const ResetTransformWorldTitle = "Reset Transform (World)"; + static const char* const ResetTransformWorldDesc = "Reset transform to world space"; - static const char* const s_entityBoxSelectUndoRedoDesc = "Box Select Entities"; - static const char* const s_entityDeselectUndoRedoDesc = "Deselect Entity"; - static const char* const s_entitiesDeselectUndoRedoDesc = "Deselect Entities"; - static const char* const s_entitySelectUndoRedoDesc = "Select Entity"; - static const char* const s_dittoManipulatorUndoRedoDesc = "Ditto Manipulator"; - static const char* const s_resetManipulatorTranslationUndoRedoDesc = "Reset Manipulator Translation"; - static const char* const s_resetManipulatorOrientationUndoRedoDesc = "Reset Manipulator Orientation"; - static const char* const s_dittoEntityOrientationIndividualUndoRedoDesc = "Ditto orientation individual"; - static const char* const s_dittoEntityOrientationGroupUndoRedoDesc = "Ditto orientation group"; - static const char* const s_resetTranslationToParentUndoRedoDesc = "Reset translation to parent"; - static const char* const s_resetOrientationToParentUndoRedoDesc = "Reset orientation to parent"; - static const char* const s_dittoTranslationGroupUndoRedoDesc = "Ditto translation group"; - static const char* const s_dittoTranslationIndividualUndoRedoDesc = "Ditto translation individual"; - static const char* const s_dittoScaleIndividualWorldUndoRedoDesc = "Ditto scale individual world"; - static const char* const s_dittoScaleIndividualLocalUndoRedoDesc = "Ditto scale individual local"; - static const char* const s_snapToWorldGridUndoRedoDesc = "Snap to world grid"; - static const char* const s_showAllEntitiesUndoRedoDesc = s_showAllTitle; - static const char* const s_lockSelectionUndoRedoDesc = s_lockSelectionTitle; - static const char* const s_hideSelectionUndoRedoDesc = s_hideSelectionTitle; - static const char* const s_unlockAllUndoRedoDesc = s_unlockAllTitle; - static const char* const s_selectAllEntitiesUndoRedoDesc = s_selectAllTitle; - static const char* const s_invertSelectionUndoRedoDesc = s_invertSelectionTitle; - static const char* const s_duplicateUndoRedoDesc = s_duplicateTitle; - static const char* const s_deleteUndoRedoDesc = s_deleteTitle; + static const char* const EntityBoxSelectUndoRedoDesc = "Box Select Entities"; + static const char* const EntityDeselectUndoRedoDesc = "Deselect Entity"; + static const char* const EntitiesDeselectUndoRedoDesc = "Deselect Entities"; + static const char* const ChangeEntitySelectionUndoRedoDesc = "Change Selected Entity"; + static const char* const EntitySelectUndoRedoDesc = "Select Entity"; + static const char* const DittoManipulatorUndoRedoDesc = "Ditto Manipulator"; + static const char* const ResetManipulatorTranslationUndoRedoDesc = "Reset Manipulator Translation"; + static const char* const ResetManipulatorOrientationUndoRedoDesc = "Reset Manipulator Orientation"; + static const char* const DittoEntityOrientationIndividualUndoRedoDesc = "Ditto orientation individual"; + static const char* const DittoEntityOrientationGroupUndoRedoDesc = "Ditto orientation group"; + static const char* const ResetTranslationToParentUndoRedoDesc = "Reset translation to parent"; + static const char* const ResetOrientationToParentUndoRedoDesc = "Reset orientation to parent"; + static const char* const DittoTranslationGroupUndoRedoDesc = "Ditto translation group"; + static const char* const DittoTranslationIndividualUndoRedoDesc = "Ditto translation individual"; + static const char* const DittoScaleIndividualWorldUndoRedoDesc = "Ditto scale individual world"; + static const char* const DittoScaleIndividualLocalUndoRedoDesc = "Ditto scale individual local"; + static const char* const SnapToWorldGridUndoRedoDesc = "Snap to world grid"; + static const char* const ShowAllEntitiesUndoRedoDesc = ShowAllTitle; + static const char* const LockSelectionUndoRedoDesc = LockSelectionTitle; + static const char* const HideSelectionUndoRedoDesc = HideSelectionTitle; + static const char* const UnlockAllUndoRedoDesc = UnlockAllTitle; + static const char* const SelectAllEntitiesUndoRedoDesc = SelectAllTitle; + static const char* const InvertSelectionUndoRedoDesc = InvertSelectionTitle; + static const char* const DuplicateUndoRedoDesc = DuplicateTitle; + static const char* const DeleteUndoRedoDesc = DeleteTitle; static const char* const TransformModeClusterTranslateTooltip = "Switch to translate mode"; static const char* const TransformModeClusterRotateTooltip = "Switch to rotate mode"; @@ -148,14 +149,14 @@ namespace AzToolsFramework static const char* const SpaceClusterLocalTooltip = "Toggle local space lock"; static const char* const SnappingClusterSnapToWorldTooltip = "Snap selected entities to the world space grid"; - static const AZ::Color s_fadedXAxisColor = AZ::Color(AZ::u8(200), AZ::u8(127), AZ::u8(127), AZ::u8(255)); - static const AZ::Color s_fadedYAxisColor = AZ::Color(AZ::u8(127), AZ::u8(190), AZ::u8(127), AZ::u8(255)); - static const AZ::Color s_fadedZAxisColor = AZ::Color(AZ::u8(120), AZ::u8(120), AZ::u8(180), AZ::u8(255)); + static const AZ::Color FadedXAxisColor = AZ::Color(AZ::u8(200), AZ::u8(127), AZ::u8(127), AZ::u8(255)); + static const AZ::Color FadedYAxisColor = AZ::Color(AZ::u8(127), AZ::u8(190), AZ::u8(127), AZ::u8(255)); + static const AZ::Color FadedZAxisColor = AZ::Color(AZ::u8(120), AZ::u8(120), AZ::u8(180), AZ::u8(255)); - static const AZ::Color s_pickedOrientationColor = AZ::Color(0.0f, 1.0f, 0.0f, 1.0f); - static const AZ::Color s_selectedEntityAabbColor = AZ::Color(0.6f, 0.6f, 0.6f, 0.4f); + static const AZ::Color PickedOrientationColor = AZ::Color(0.0f, 1.0f, 0.0f, 1.0f); + static const AZ::Color SelectedEntityAabbColor = AZ::Color(0.6f, 0.6f, 0.6f, 0.4f); - static const float s_pivotSize = 0.075f; // the size of the pivot (box) to render when selected + static const float PivotSize = 0.075f; // the size of the pivot (box) to render when selected // data passed to manipulators when processing mouse interactions // m_entityIds should be sorted based on the entity hierarchy @@ -1107,7 +1108,7 @@ namespace AzToolsFramework { // begin selection undo/redo command entityBoxSelectData->m_boxSelectSelectionCommand = - AZStd::make_unique(EntityIdList(), s_entityBoxSelectUndoRedoDesc); + AZStd::make_unique(EntityIdList(), EntityBoxSelectUndoRedoDesc); // grab currently selected entities entityBoxSelectData->m_selectedEntityIdsBeforeBoxSelect = m_selectedEntityIds; }); @@ -1131,7 +1132,7 @@ namespace AzToolsFramework if (!entityBoxSelectData->m_potentialDeselectedEntityIds.empty() || !entityBoxSelectData->m_potentialSelectedEntityIds.empty()) { - ScopedUndoBatch undoBatch(s_entityBoxSelectUndoRedoDesc); + ScopedUndoBatch undoBatch(EntityBoxSelectUndoRedoDesc); // restore manipulator overrides when undoing if (m_entityIdManipulators.m_manipulators && m_selectedEntityIds.empty()) @@ -1174,7 +1175,7 @@ namespace AzToolsFramework } debugDisplay.DepthTestOff(); - debugDisplay.SetColor(s_selectedEntityAabbColor); + debugDisplay.SetColor(SelectedEntityAabbColor); for (AZ::EntityId entityId : entityBoxSelectData->m_potentialSelectedEntityIds) { @@ -1222,7 +1223,7 @@ namespace AzToolsFramework { // check here if translation or orientation override are set m_manipulatorMoveCommand = - AZStd::make_unique(CreateManipulatorCommandStateFromSelf(), s_manipulatorUndoRedoName); + AZStd::make_unique(CreateManipulatorCommandStateFromSelf(), ManipulatorUndoRedoName); } } @@ -1696,7 +1697,7 @@ namespace AzToolsFramework if (!UndoRedoOperationInProgress()) { - ScopedUndoBatch undoBatch(s_entitiesDeselectUndoRedoDesc); + ScopedUndoBatch undoBatch(EntitiesDeselectUndoRedoDesc); // restore manipulator overrides when undoing if (m_entityIdManipulators.m_manipulators) @@ -1706,7 +1707,7 @@ namespace AzToolsFramework // select must happen after to ensure in the undo/redo step the selection command // happens before the manipulator command - auto selectionCommand = AZStd::make_unique(EntityIdList(), s_entitiesDeselectUndoRedoDesc); + auto selectionCommand = AZStd::make_unique(EntityIdList(), EntitiesDeselectUndoRedoDesc); selectionCommand->SetParent(undoBatch.GetUndoBatch()); selectionCommand.release(); } @@ -1732,7 +1733,7 @@ namespace AzToolsFramework const auto nextEntityIds = EntityIdVectorFromContainer(m_selectedEntityIds); - ScopedUndoBatch undoBatch(s_entityDeselectUndoRedoDesc); + ScopedUndoBatch undoBatch(EntityDeselectUndoRedoDesc); // store manipulator state when removing last entity from selection if (m_entityIdManipulators.m_manipulators && nextEntityIds.empty()) @@ -1740,7 +1741,7 @@ namespace AzToolsFramework CreateEntityManipulatorDeselectCommand(undoBatch); } - auto selectionCommand = AZStd::make_unique(nextEntityIds, s_entityDeselectUndoRedoDesc); + auto selectionCommand = AZStd::make_unique(nextEntityIds, EntityDeselectUndoRedoDesc); selectionCommand->SetParent(undoBatch.GetUndoBatch()); selectionCommand.release(); @@ -1755,8 +1756,8 @@ namespace AzToolsFramework const auto nextEntityIds = EntityIdVectorFromContainer(m_selectedEntityIds); - ScopedUndoBatch undoBatch(s_entitySelectUndoRedoDesc); - auto selectionCommand = AZStd::make_unique(nextEntityIds, s_entitySelectUndoRedoDesc); + ScopedUndoBatch undoBatch(EntitySelectUndoRedoDesc); + auto selectionCommand = AZStd::make_unique(nextEntityIds, EntitySelectUndoRedoDesc); selectionCommand->SetParent(undoBatch.GetUndoBatch()); selectionCommand.release(); @@ -1772,6 +1773,13 @@ namespace AzToolsFramework void EditorTransformComponentSelection::ChangeSelectedEntity(const AZ::EntityId entityId) { + AZ_Assert( + !UndoRedoOperationInProgress(), + "ChangeSelectedEntity called from undo/redo operation - this is unexpected and not currently supported"); + + // ensure deselect/select is tracked as an atomic undo/redo operation + ScopedUndoBatch undoBatch(ChangeEntitySelectionUndoRedoDesc); + DeselectEntities(); SelectDeselect(entityId); } @@ -1799,7 +1807,7 @@ namespace AzToolsFramework const AZ::Transform& worldFromLocal = m_entityDataCache->GetVisibleEntityTransform(*entityIndex); const AZ::Vector3 boxPosition = worldFromLocal.TransformPoint(CalculateCenterOffset(entityId, m_pivotMode)); const AZ::Vector3 scaledSize = - AZ::Vector3(s_pivotSize) * CalculateScreenToWorldMultiplier(worldFromLocal.GetTranslation(), cameraState); + AZ::Vector3(PivotSize) * CalculateScreenToWorldMultiplier(worldFromLocal.GetTranslation(), cameraState); if (AabbIntersectMouseRay( mouseInteraction.m_mouseInteraction, @@ -2002,10 +2010,10 @@ namespace AzToolsFramework { if (m_entityIdManipulators.m_manipulators) { - ScopedUndoBatch undoBatch(s_dittoManipulatorUndoRedoDesc); + ScopedUndoBatch undoBatch(DittoManipulatorUndoRedoDesc); auto manipulatorCommand = - AZStd::make_unique(CreateManipulatorCommandStateFromSelf(), s_manipulatorUndoRedoName); + AZStd::make_unique(CreateManipulatorCommandStateFromSelf(), ManipulatorUndoRedoName); if (entityId.IsValid()) { @@ -2131,7 +2139,7 @@ namespace AzToolsFramework { AZ_PROFILE_FUNCTION(AzToolsFramework); - ScopedUndoBatch undoBatch(s_lockSelectionUndoRedoDesc); + ScopedUndoBatch undoBatch(LockSelectionUndoRedoDesc); if (m_entityIdManipulators.m_manipulators) { @@ -2151,7 +2159,7 @@ namespace AzToolsFramework // lock selection AddAction( - m_actions, { QKeySequence(Qt::Key_L) }, LockSelection, s_lockSelectionTitle, s_lockSelectionDesc, + m_actions, { QKeySequence(Qt::Key_L) }, LockSelection, LockSelectionTitle, LockSelectionDesc, [lockUnlock]() { lockUnlock(true); @@ -2159,7 +2167,7 @@ namespace AzToolsFramework // unlock selection AddAction( - m_actions, { QKeySequence(Qt::CTRL + Qt::Key_L) }, UnlockSelection, s_lockSelectionTitle, s_lockSelectionDesc, + m_actions, { QKeySequence(Qt::CTRL + Qt::Key_L) }, UnlockSelection, LockSelectionTitle, LockSelectionDesc, [lockUnlock]() { lockUnlock(false); @@ -2169,7 +2177,7 @@ namespace AzToolsFramework { AZ_PROFILE_FUNCTION(AzToolsFramework); - ScopedUndoBatch undoBatch(s_hideSelectionUndoRedoDesc); + ScopedUndoBatch undoBatch(HideSelectionUndoRedoDesc); if (m_entityIdManipulators.m_manipulators) { @@ -2189,7 +2197,7 @@ namespace AzToolsFramework // hide selection AddAction( - m_actions, { QKeySequence(Qt::Key_H) }, HideSelection, s_hideSelectionTitle, s_hideSelectionDesc, + m_actions, { QKeySequence(Qt::Key_H) }, HideSelection, HideSelectionTitle, HideSelectionDesc, [showHide]() { showHide(false); @@ -2197,7 +2205,7 @@ namespace AzToolsFramework // show selection AddAction( - m_actions, { QKeySequence(Qt::CTRL + Qt::Key_H) }, ShowSelection, s_hideSelectionTitle, s_hideSelectionDesc, + m_actions, { QKeySequence(Qt::CTRL + Qt::Key_H) }, ShowSelection, HideSelectionTitle, HideSelectionDesc, [showHide]() { showHide(true); @@ -2205,12 +2213,12 @@ namespace AzToolsFramework // unlock all entities in the level/scene AddAction( - m_actions, { QKeySequence(Qt::CTRL + Qt::SHIFT + Qt::Key_L) }, UnlockAll, s_unlockAllTitle, s_unlockAllDesc, + m_actions, { QKeySequence(Qt::CTRL + Qt::SHIFT + Qt::Key_L) }, UnlockAll, UnlockAllTitle, UnlockAllDesc, []() { AZ_PROFILE_FUNCTION(AzToolsFramework); - ScopedUndoBatch undoBatch(s_unlockAllUndoRedoDesc); + ScopedUndoBatch undoBatch(UnlockAllUndoRedoDesc); EnumerateEditorEntities( [](AZ::EntityId entityId) @@ -2222,12 +2230,12 @@ namespace AzToolsFramework // show all entities in the level/scene AddAction( - m_actions, { QKeySequence(Qt::CTRL + Qt::SHIFT + Qt::Key_H) }, ShowAll, s_showAllTitle, s_showAllDesc, + m_actions, { QKeySequence(Qt::CTRL + Qt::SHIFT + Qt::Key_H) }, ShowAll, ShowAllTitle, ShowAllDesc, []() { AZ_PROFILE_FUNCTION(AzToolsFramework); - ScopedUndoBatch undoBatch(s_showAllEntitiesUndoRedoDesc); + ScopedUndoBatch undoBatch(ShowAllEntitiesUndoRedoDesc); EnumerateEditorEntities( [](AZ::EntityId entityId) @@ -2239,17 +2247,17 @@ namespace AzToolsFramework // select all entities in the level/scene AddAction( - m_actions, { QKeySequence(Qt::CTRL + Qt::Key_A) }, SelectAll, s_selectAllTitle, s_selectAllDesc, + m_actions, { QKeySequence(Qt::CTRL + Qt::Key_A) }, SelectAll, SelectAllTitle, SelectAllDesc, [this]() { AZ_PROFILE_FUNCTION(AzToolsFramework); - ScopedUndoBatch undoBatch(s_selectAllEntitiesUndoRedoDesc); + ScopedUndoBatch undoBatch(SelectAllEntitiesUndoRedoDesc); if (m_entityIdManipulators.m_manipulators) { auto manipulatorCommand = - AZStd::make_unique(CreateManipulatorCommandStateFromSelf(), s_manipulatorUndoRedoName); + AZStd::make_unique(CreateManipulatorCommandStateFromSelf(), ManipulatorUndoRedoName); // note, nothing will change that the manipulatorCommand needs to keep track // for after so no need to call SetManipulatorAfter @@ -2269,7 +2277,7 @@ namespace AzToolsFramework auto nextEntityIds = EntityIdVectorFromContainer(m_selectedEntityIds); - auto selectionCommand = AZStd::make_unique(nextEntityIds, s_selectAllEntitiesUndoRedoDesc); + auto selectionCommand = AZStd::make_unique(nextEntityIds, SelectAllEntitiesUndoRedoDesc); selectionCommand->SetParent(undoBatch.GetUndoBatch()); selectionCommand.release(); @@ -2279,17 +2287,17 @@ namespace AzToolsFramework // invert current selection AddAction( - m_actions, { QKeySequence(Qt::CTRL + Qt::SHIFT + Qt::Key_I) }, InvertSelect, s_invertSelectionTitle, s_invertSelectionDesc, + m_actions, { QKeySequence(Qt::CTRL + Qt::SHIFT + Qt::Key_I) }, InvertSelect, InvertSelectionTitle, InvertSelectionDesc, [this]() { AZ_PROFILE_FUNCTION(AzToolsFramework); - ScopedUndoBatch undoBatch(s_invertSelectionUndoRedoDesc); + ScopedUndoBatch undoBatch(InvertSelectionUndoRedoDesc); if (m_entityIdManipulators.m_manipulators) { auto manipulatorCommand = - AZStd::make_unique(CreateManipulatorCommandStateFromSelf(), s_manipulatorUndoRedoName); + AZStd::make_unique(CreateManipulatorCommandStateFromSelf(), ManipulatorUndoRedoName); // note, nothing will change that the manipulatorCommand needs to keep track // for after so no need to call SetManipulatorAfter @@ -2316,7 +2324,7 @@ namespace AzToolsFramework auto nextEntityIds = EntityIdVectorFromContainer(entityIds); - auto selectionCommand = AZStd::make_unique(nextEntityIds, s_invertSelectionUndoRedoDesc); + auto selectionCommand = AZStd::make_unique(nextEntityIds, InvertSelectionUndoRedoDesc); selectionCommand->SetParent(undoBatch.GetUndoBatch()); selectionCommand.release(); @@ -2326,7 +2334,7 @@ namespace AzToolsFramework // duplicate selection AddAction( - m_actions, { QKeySequence(Qt::CTRL + Qt::Key_D) }, DuplicateSelect, s_duplicateTitle, s_duplicateDesc, + m_actions, { QKeySequence(Qt::CTRL + Qt::Key_D) }, DuplicateSelect, DuplicateTitle, DuplicateDesc, []() { AZ_PROFILE_FUNCTION(AzToolsFramework); @@ -2338,8 +2346,8 @@ namespace AzToolsFramework QApplication::focusWidget()->clearFocus(); } - ScopedUndoBatch undoBatch(s_duplicateUndoRedoDesc); - auto selectionCommand = AZStd::make_unique(EntityIdList(), s_duplicateUndoRedoDesc); + ScopedUndoBatch undoBatch(DuplicateUndoRedoDesc); + auto selectionCommand = AZStd::make_unique(EntityIdList(), DuplicateUndoRedoDesc); selectionCommand->SetParent(undoBatch.GetUndoBatch()); selectionCommand.release(); @@ -2351,12 +2359,12 @@ namespace AzToolsFramework // delete selection AddAction( - m_actions, { QKeySequence(Qt::Key_Delete) }, DeleteSelect, s_deleteTitle, s_deleteDesc, + m_actions, { QKeySequence(Qt::Key_Delete) }, DeleteSelect, DeleteTitle, DeleteDesc, [this]() { AZ_PROFILE_FUNCTION(AzToolsFramework); - ScopedUndoBatch undoBatch(s_deleteUndoRedoDesc); + ScopedUndoBatch undoBatch(DeleteUndoRedoDesc); CreateEntityManipulatorDeselectCommand(undoBatch); @@ -2375,14 +2383,14 @@ namespace AzToolsFramework }); AddAction( - m_actions, { QKeySequence(Qt::Key_P) }, EditPivot, s_togglePivotTitleEditMenu, s_togglePivotDesc, + m_actions, { QKeySequence(Qt::Key_P) }, EditPivot, TogglePivotTitleEditMenu, TogglePivotDesc, [this]() { ToggleCenterPivotSelection(); }); AddAction( - m_actions, { QKeySequence(Qt::Key_R) }, EditReset, s_resetEntityTransformTitle, s_resetEntityTransformDesc, + m_actions, { QKeySequence(Qt::Key_R) }, EditReset, ResetEntityTransformTitle, ResetEntityTransformDesc, [this]() { switch (m_mode) @@ -2400,11 +2408,11 @@ namespace AzToolsFramework }); AddAction( - m_actions, { QKeySequence(Qt::CTRL + Qt::Key_R) }, EditResetManipulator, s_resetManipulatorTitle, s_resetManipulatorDesc, + m_actions, { QKeySequence(Qt::CTRL + Qt::Key_R) }, EditResetManipulator, ResetManipulatorTitle, ResetManipulatorDesc, AZStd::bind(AZStd::mem_fn(&EditorTransformComponentSelection::DelegateClearManipulatorOverride), this)); AddAction( - m_actions, { QKeySequence(Qt::ALT + Qt::Key_R) }, EditResetLocal, s_resetTransformLocalTitle, s_resetTransformLocalDesc, + m_actions, { QKeySequence(Qt::ALT + Qt::Key_R) }, EditResetLocal, ResetTransformLocalTitle, ResetTransformLocalDesc, [this]() { switch (m_mode) @@ -2422,7 +2430,7 @@ namespace AzToolsFramework }); AddAction( - m_actions, { QKeySequence(Qt::SHIFT + Qt::Key_R) }, EditResetWorld, s_resetTransformWorldTitle, s_resetTransformWorldDesc, + m_actions, { QKeySequence(Qt::SHIFT + Qt::Key_R) }, EditResetWorld, ResetTransformWorldTitle, ResetTransformWorldDesc, [this]() { switch (m_mode) @@ -2431,7 +2439,7 @@ namespace AzToolsFramework { // begin an undo batch so operations inside CopyOrientation... and // DelegateClear... are grouped into a single undo/redo - ScopedUndoBatch undoBatch{ s_resetTransformWorldTitle }; + ScopedUndoBatch undoBatch{ ResetTransformWorldTitle }; CopyOrientationToSelectedEntitiesIndividual(AZ::Quaternion::CreateIdentity()); ClearManipulatorOrientationOverride(); } @@ -2685,7 +2693,7 @@ namespace AzToolsFramework const AZStd::array snapAxes = { AZ::Vector3::CreateAxisX(), AZ::Vector3::CreateAxisY(), AZ::Vector3::CreateAxisZ() }; - ScopedUndoBatch undoBatch(s_snapToWorldGridUndoRedoDesc); + ScopedUndoBatch undoBatch(SnapToWorldGridUndoRedoDesc); for (const AZ::EntityId& entityId : m_selectedEntityIds) { ScopedUndoBatch::MarkEntityDirty(entityId); @@ -2870,10 +2878,10 @@ namespace AzToolsFramework if (m_entityIdManipulators.m_manipulators) { - ScopedUndoBatch undoBatch(s_resetManipulatorTranslationUndoRedoDesc); + ScopedUndoBatch undoBatch(ResetManipulatorTranslationUndoRedoDesc); auto manipulatorCommand = - AZStd::make_unique(CreateManipulatorCommandStateFromSelf(), s_manipulatorUndoRedoName); + AZStd::make_unique(CreateManipulatorCommandStateFromSelf(), ManipulatorUndoRedoName); m_pivotOverrideFrame.ResetPickedTranslation(); m_pivotOverrideFrame.m_pickedEntityIdOverride.SetInvalid(); @@ -2896,10 +2904,10 @@ namespace AzToolsFramework if (m_entityIdManipulators.m_manipulators) { - ScopedUndoBatch undoBatch{ s_resetManipulatorOrientationUndoRedoDesc }; + ScopedUndoBatch undoBatch{ ResetManipulatorOrientationUndoRedoDesc }; auto manipulatorCommand = - AZStd::make_unique(CreateManipulatorCommandStateFromSelf(), s_manipulatorUndoRedoName); + AZStd::make_unique(CreateManipulatorCommandStateFromSelf(), ManipulatorUndoRedoName); m_pivotOverrideFrame.ResetPickedOrientation(); m_pivotOverrideFrame.m_pickedEntityIdOverride.SetInvalid(); @@ -2961,13 +2969,13 @@ namespace AzToolsFramework if (m_entityIdManipulators.m_manipulators) { - ScopedUndoBatch undoBatch(s_dittoTranslationGroupUndoRedoDesc); + ScopedUndoBatch undoBatch(DittoTranslationGroupUndoRedoDesc); // store previous translation manipulator position const AZ::Vector3 previousPivotTranslation = m_entityIdManipulators.m_manipulators->GetLocalTransform().GetTranslation(); auto manipulatorCommand = - AZStd::make_unique(CreateManipulatorCommandStateFromSelf(), s_manipulatorUndoRedoName); + AZStd::make_unique(CreateManipulatorCommandStateFromSelf(), ManipulatorUndoRedoName); // refresh the transform pivot override if it's set if (m_pivotOverrideFrame.m_translationOverride) @@ -3017,10 +3025,10 @@ namespace AzToolsFramework if (m_entityIdManipulators.m_manipulators) { - ScopedUndoBatch undoBatch(s_dittoTranslationIndividualUndoRedoDesc); + ScopedUndoBatch undoBatch(DittoTranslationIndividualUndoRedoDesc); auto manipulatorCommand = - AZStd::make_unique(CreateManipulatorCommandStateFromSelf(), s_manipulatorUndoRedoName); + AZStd::make_unique(CreateManipulatorCommandStateFromSelf(), ManipulatorUndoRedoName); // refresh the transform pivot override if it's set if (m_pivotOverrideFrame.m_translationOverride) @@ -3055,7 +3063,7 @@ namespace AzToolsFramework { AZ_PROFILE_FUNCTION(AzToolsFramework); - ScopedUndoBatch undoBatch(s_dittoScaleIndividualWorldUndoRedoDesc); + ScopedUndoBatch undoBatch(DittoScaleIndividualWorldUndoRedoDesc); ManipulatorEntityIds manipulatorEntityIds; BuildSortedEntityIdVectorFromEntityIdMap(m_entityIdManipulators.m_lookups, manipulatorEntityIds.m_entityIds); @@ -3089,7 +3097,7 @@ namespace AzToolsFramework { AZ_PROFILE_FUNCTION(AzToolsFramework); - ScopedUndoBatch undoBatch(s_dittoScaleIndividualLocalUndoRedoDesc); + ScopedUndoBatch undoBatch(DittoScaleIndividualLocalUndoRedoDesc); ManipulatorEntityIds manipulatorEntityIds; BuildSortedEntityIdVectorFromEntityIdMap(m_entityIdManipulators.m_lookups, manipulatorEntityIds.m_entityIds); @@ -3110,10 +3118,10 @@ namespace AzToolsFramework if (m_entityIdManipulators.m_manipulators) { - ScopedUndoBatch undoBatch{ s_dittoEntityOrientationIndividualUndoRedoDesc }; + ScopedUndoBatch undoBatch{ DittoEntityOrientationIndividualUndoRedoDesc }; auto manipulatorCommand = - AZStd::make_unique(CreateManipulatorCommandStateFromSelf(), s_manipulatorUndoRedoName); + AZStd::make_unique(CreateManipulatorCommandStateFromSelf(), ManipulatorUndoRedoName); ManipulatorEntityIds manipulatorEntityIds; BuildSortedEntityIdVectorFromEntityIdMap(m_entityIdManipulators.m_lookups, manipulatorEntityIds.m_entityIds); @@ -3148,10 +3156,10 @@ namespace AzToolsFramework if (m_entityIdManipulators.m_manipulators) { - ScopedUndoBatch undoBatch(s_dittoEntityOrientationGroupUndoRedoDesc); + ScopedUndoBatch undoBatch(DittoEntityOrientationGroupUndoRedoDesc); auto manipulatorCommand = - AZStd::make_unique(CreateManipulatorCommandStateFromSelf(), s_manipulatorUndoRedoName); + AZStd::make_unique(CreateManipulatorCommandStateFromSelf(), ManipulatorUndoRedoName); ManipulatorEntityIds manipulatorEntityIds; BuildSortedEntityIdVectorFromEntityIdMap(m_entityIdManipulators.m_lookups, manipulatorEntityIds.m_entityIds); @@ -3194,7 +3202,7 @@ namespace AzToolsFramework { AZ_PROFILE_FUNCTION(AzToolsFramework); - ScopedUndoBatch undoBatch(s_resetOrientationToParentUndoRedoDesc); + ScopedUndoBatch undoBatch(ResetOrientationToParentUndoRedoDesc); for (const auto& entityIdLookup : m_entityIdManipulators.m_lookups) { ScopedUndoBatch::MarkEntityDirty(entityIdLookup.first); @@ -3215,7 +3223,7 @@ namespace AzToolsFramework if (m_entityIdManipulators.m_manipulators) { - ScopedUndoBatch undoBatch(s_resetTranslationToParentUndoRedoDesc); + ScopedUndoBatch undoBatch(ResetTranslationToParentUndoRedoDesc); ManipulatorEntityIds manipulatorEntityIds; BuildSortedEntityIdVectorFromEntityIdMap(m_entityIdManipulators.m_lookups, manipulatorEntityIds.m_entityIds); @@ -3251,7 +3259,7 @@ namespace AzToolsFramework void EditorTransformComponentSelection::PopulateEditorGlobalContextMenu( QMenu* menu, [[maybe_unused]] const AZ::Vector2& point, [[maybe_unused]] int flags) { - QAction* action = menu->addAction(QObject::tr(s_togglePivotTitleRightClick)); + QAction* action = menu->addAction(QObject::tr(TogglePivotTitleRightClick)); QObject::connect( action, &QAction::triggered, action, [this]() @@ -3329,15 +3337,15 @@ namespace AzToolsFramework : 1.0f; }; - display.SetColor(s_fadedXAxisColor); + display.SetColor(FadedXAxisColor); display.DrawLine( transform.GetTranslation(), transform.GetTranslation() + transform.GetBasisX().GetNormalizedSafe() * axisLength * axisFlip(AZ::Vector3::CreateAxisX())); - display.SetColor(s_fadedYAxisColor); + display.SetColor(FadedYAxisColor); display.DrawLine( transform.GetTranslation(), transform.GetTranslation() + transform.GetBasisY().GetNormalizedSafe() * axisLength * axisFlip(AZ::Vector3::CreateAxisY())); - display.SetColor(s_fadedZAxisColor); + display.SetColor(FadedZAxisColor); display.DrawLine( transform.GetTranslation(), transform.GetTranslation() + transform.GetBasisZ().GetNormalizedSafe() * axisLength * axisFlip(AZ::Vector3::CreateAxisZ())); @@ -3416,11 +3424,11 @@ namespace AzToolsFramework CalculatePivotTranslation(m_pivotOverrideFrame.m_pickedEntityIdOverride, m_pivotMode)); const float scaledSize = - s_pivotSize * CalculateScreenToWorldMultiplier(pickedEntityWorldTransform.GetTranslation(), cameraState); + PivotSize * CalculateScreenToWorldMultiplier(pickedEntityWorldTransform.GetTranslation(), cameraState); debugDisplay.DepthWriteOff(); debugDisplay.DepthTestOff(); - debugDisplay.SetColor(s_pickedOrientationColor); + debugDisplay.SetColor(PickedOrientationColor); debugDisplay.DrawWireSphere(pickedEntityWorldTransform.GetTranslation(), scaledSize); @@ -3462,7 +3470,7 @@ namespace AzToolsFramework const AZ::Vector3 boxPosition = worldFromLocal.TransformPoint(CalculateCenterOffset(entityId, m_pivotMode)); const AZ::Vector3 scaledSize = - AZ::Vector3(s_pivotSize) * CalculateScreenToWorldMultiplier(worldFromLocal.GetTranslation(), cameraState); + AZ::Vector3(PivotSize) * CalculateScreenToWorldMultiplier(worldFromLocal.GetTranslation(), cameraState); const AZ::Color hiddenNormal[] = { AzFramework::ViewportColors::SelectedColor, AzFramework::ViewportColors::HiddenColor }; @@ -3708,7 +3716,7 @@ namespace AzToolsFramework if (m_entityIdManipulators.m_manipulators) { auto manipulatorCommand = - AZStd::make_unique(CreateManipulatorCommandStateFromSelf(), s_manipulatorUndoRedoName); + AZStd::make_unique(CreateManipulatorCommandStateFromSelf(), ManipulatorUndoRedoName); manipulatorCommand->SetManipulatorAfter(EntityManipulatorCommand::State()); diff --git a/Code/Framework/AzToolsFramework/Tests/EditorTransformComponentSelectionTests.cpp b/Code/Framework/AzToolsFramework/Tests/EditorTransformComponentSelectionTests.cpp index 241ee3f576..72cafe5cd5 100644 --- a/Code/Framework/AzToolsFramework/Tests/EditorTransformComponentSelectionTests.cpp +++ b/Code/Framework/AzToolsFramework/Tests/EditorTransformComponentSelectionTests.cpp @@ -866,9 +866,7 @@ namespace UnitTest EXPECT_THAT(selectedEntitiesAfter, UnorderedElementsAre(m_entityId1)); } - TEST_F( - EditorTransformComponentSelectionViewportPickingManipulatorTestFixture, - BoxSelectWithNoInitialSelectionAddsEntitiesToSelection) + TEST_F(EditorTransformComponentSelectionViewportPickingManipulatorTestFixture, BoxSelectWithNoInitialSelectionAddsEntitiesToSelection) { AzToolsFramework::ed_viewportStickySelect = true; @@ -989,6 +987,30 @@ namespace UnitTest EXPECT_TRUE(selectedEntitiesAfter.empty()); } + TEST_F(EditorTransformComponentSelectionViewportPickingManipulatorTestFixture, UnstickyUndoOperationForChangeInSelectionIsAtomic) + { + AzToolsFramework::ed_viewportStickySelect = false; + + PositionEntities(); + PositionCamera(m_cameraState); + + AzToolsFramework::SelectEntity(m_entityId1); + + // calculate the position in screen space of the second entity + const auto entity2ScreenPosition = AzFramework::WorldToScreen(m_entity2WorldTranslation, m_cameraState); + + // single click select entity2 + m_actionDispatcher->CameraState(m_cameraState)->MousePosition(entity2ScreenPosition)->MouseLButtonDown()->MouseLButtonUp(); + + // undo action + AzToolsFramework::ToolsApplicationRequestBus::Broadcast(&AzToolsFramework::ToolsApplicationRequestBus::Events::UndoPressed); + + // entity1 is selected after undo + using ::testing::UnorderedElementsAre; + auto selectedEntitiesAfter = SelectedEntities(); + EXPECT_THAT(selectedEntitiesAfter, UnorderedElementsAre(m_entityId1)); + } + using EditorTransformComponentSelectionManipulatorTestFixture = IndirectCallManipulatorViewportInteractionFixtureMixin;