Fix the crash when switching between sim obj and physix layout in animation editor. (#4185)

* Update the simulated object layout

Signed-off-by: rhhong <rhhong@amazon.com>

* Fixed the bug. Do not store uncessory cached selected model indices which would become stall.

Signed-off-by: rhhong <rhhong@amazon.com>

* update the simulted object layout.

Signed-off-by: rhhong <rhhong@amazon.com>

* Get rid of locally cached selectedIndices variable.

Signed-off-by: rhhong <rhhong@amazon.com>

* Remove debug code

Signed-off-by: rhhong <rhhong@amazon.com>

Gems/EMotionFX/Code/Source/Editor/Plugins/Cloth/ClothJointWidget.cpp

@@ -69,7 +69,7 @@ namespace EMotionFX
 
     void ClothJointWidget::InternalReinit()
     {
-        if (m_selectedModelIndices.size() == 1)
+        if (GetSelectedModelIndices().size() == 1)
         {
             Physics::CharacterColliderNodeConfiguration* nodeConfig = GetNodeConfig();
             if (nodeConfig)
@@ -94,17 +94,17 @@ namespace EMotionFX
 
     void ClothJointWidget::OnAddCollider(const AZ::TypeId& colliderType)
     {
-        ColliderHelpers::AddCollider(m_selectedModelIndices , PhysicsSetup::Cloth, colliderType);
+        ColliderHelpers::AddCollider(GetSelectedModelIndices(), PhysicsSetup::Cloth, colliderType);
     }
 
     void ClothJointWidget::OnCopyCollider(size_t colliderIndex)
     {
-        ColliderHelpers::CopyColliderToClipboard(m_selectedModelIndices.first(), colliderIndex, PhysicsSetup::Cloth);
+        ColliderHelpers::CopyColliderToClipboard(GetSelectedModelIndices().first(), colliderIndex, PhysicsSetup::Cloth);
     }
 
     void ClothJointWidget::OnPasteCollider(size_t colliderIndex, bool replace)
     {
-        ColliderHelpers::PasteColliderFromClipboard(m_selectedModelIndices.first(), colliderIndex, PhysicsSetup::Cloth, replace);
+        ColliderHelpers::PasteColliderFromClipboard(GetSelectedModelIndices().first(), colliderIndex, PhysicsSetup::Cloth, replace);
     }
 
     void ClothJointWidget::OnRemoveCollider(size_t colliderIndex)
@@ -114,7 +114,7 @@ namespace EMotionFX
 
     Physics::CharacterColliderNodeConfiguration* ClothJointWidget::GetNodeConfig() const
     {
-        AZ_Assert(m_selectedModelIndices.size() == 1, "Get Node config function only return the config when it is single seleted");
+        AZ_Assert(GetSelectedModelIndices().size() == 1, "Get Node config function only return the config when it is single seleted");
         Actor* actor = GetActor();
         Node* joint = GetNode();
         if (!actor || !joint)

Gems/EMotionFX/Code/Source/Editor/Plugins/HitDetection/HitDetectionJointWidget.cpp

@@ -65,7 +65,7 @@ namespace EMotionFX
 
     void HitDetectionJointWidget::InternalReinit()
     {
-        if (m_selectedModelIndices.size() == 1)
+        if (GetSelectedModelIndices().size() == 1)
         {
             Physics::CharacterColliderNodeConfiguration* hitDetectionNodeConfig = GetNodeConfig();
             if (hitDetectionNodeConfig)
@@ -90,17 +90,17 @@ namespace EMotionFX
 
     void HitDetectionJointWidget::OnAddCollider(const AZ::TypeId& colliderType)
     {
-        ColliderHelpers::AddCollider(m_selectedModelIndices, PhysicsSetup::HitDetection, colliderType);
+        ColliderHelpers::AddCollider(GetSelectedModelIndices(), PhysicsSetup::HitDetection, colliderType);
     }
 
     void HitDetectionJointWidget::OnCopyCollider(size_t colliderIndex)
     {
-        ColliderHelpers::CopyColliderToClipboard(m_selectedModelIndices.first(), colliderIndex, PhysicsSetup::HitDetection);
+        ColliderHelpers::CopyColliderToClipboard(GetSelectedModelIndices().first(), colliderIndex, PhysicsSetup::HitDetection);
     }
 
     void HitDetectionJointWidget::OnPasteCollider(size_t colliderIndex, bool replace)
     {
-        ColliderHelpers::PasteColliderFromClipboard(m_selectedModelIndices.first(), colliderIndex, PhysicsSetup::HitDetection, replace);
+        ColliderHelpers::PasteColliderFromClipboard(GetSelectedModelIndices().first(), colliderIndex, PhysicsSetup::HitDetection, replace);
     }
 
     void HitDetectionJointWidget::OnRemoveCollider(size_t colliderIndex)
@@ -110,7 +110,7 @@ namespace EMotionFX
 
     Physics::CharacterColliderNodeConfiguration* HitDetectionJointWidget::GetNodeConfig()
     {
-        AZ_Assert(m_selectedModelIndices.size() == 1, "Get Node config function only return the config when it is single seleted");
+        AZ_Assert(GetSelectedModelIndices().size() == 1, "Get Node config function only return the config when it is single seleted");
         Actor* actor = GetActor();
         Node* node = GetNode();
         if (!actor || !node)

Gems/EMotionFX/Code/Source/Editor/Plugins/Ragdoll/RagdollNodeWidget.cpp

@@ -116,7 +116,8 @@ namespace EMotionFX
 
     void RagdollNodeWidget::InternalReinit()
     {
-        if (m_selectedModelIndices.size() == 1)
+        const QModelIndexList& selectedModelIndices = GetSelectedModelIndices();
+        if (selectedModelIndices.size() == 1)
         {
             m_ragdollNodeEditor->ClearInstances(false);
 
@@ -142,7 +143,7 @@ namespace EMotionFX
                     m_collidersWidget->Reset();
                 }
 
-                m_jointLimitWidget->Update(m_selectedModelIndices[0]);
+                m_jointLimitWidget->Update(selectedModelIndices[0]);
                 m_ragdollNodeCard->setExpanded(true);
                 m_ragdollNodeCard->show();
                 m_jointLimitWidget->show();
@@ -169,31 +170,32 @@ namespace EMotionFX
 
     void RagdollNodeWidget::OnAddRemoveRagdollNode()
     {
+        const QModelIndexList& selectedModelIndices = GetSelectedModelIndices();
         if (GetRagdollNodeConfig())
         {
             // The node is present in the ragdoll, remove it.
-            RagdollNodeInspectorPlugin::RemoveFromRagdoll(m_selectedModelIndices);
+            RagdollNodeInspectorPlugin::RemoveFromRagdoll(selectedModelIndices);
         }
         else
         {
             // The node is not part of the ragdoll, add it.
-            RagdollNodeInspectorPlugin::AddToRagdoll(m_selectedModelIndices);
+            RagdollNodeInspectorPlugin::AddToRagdoll(selectedModelIndices);
         }
     }
 
     void RagdollNodeWidget::OnAddCollider(const AZ::TypeId& colliderType)
     {
-        ColliderHelpers::AddCollider(m_selectedModelIndices, PhysicsSetup::Ragdoll, colliderType);
+        ColliderHelpers::AddCollider(GetSelectedModelIndices(), PhysicsSetup::Ragdoll, colliderType);
     }
 
     void RagdollNodeWidget::OnCopyCollider(size_t colliderIndex)
     {
-        ColliderHelpers::CopyColliderToClipboard(m_selectedModelIndices.first(), colliderIndex, PhysicsSetup::Ragdoll);
+        ColliderHelpers::CopyColliderToClipboard(GetSelectedModelIndices().first(), colliderIndex, PhysicsSetup::Ragdoll);
     }
 
     void RagdollNodeWidget::OnPasteCollider(size_t colliderIndex, bool replace)
     {
-        ColliderHelpers::PasteColliderFromClipboard(m_selectedModelIndices.first(), colliderIndex, PhysicsSetup::Ragdoll, replace);
+        ColliderHelpers::PasteColliderFromClipboard(GetSelectedModelIndices().first(), colliderIndex, PhysicsSetup::Ragdoll, replace);
     }
 
     void RagdollNodeWidget::OnRemoveCollider(size_t colliderIndex)

Gems/EMotionFX/Code/Source/Editor/Plugins/SimulatedObject/SimulatedObjectColliderWidget.cpp

@@ -131,7 +131,8 @@ namespace EMotionFX
 
     void SimulatedObjectColliderWidget::InternalReinit()
     {
-        if (m_selectedModelIndices.size() == 1)
+        const QModelIndexList& selectedModelIndices = GetSelectedModelIndices();
+        if (selectedModelIndices.size() == 1)
         {
             Physics::CharacterColliderNodeConfiguration* nodeConfig = GetNodeConfig();
             if (nodeConfig)
@@ -172,12 +173,13 @@ namespace EMotionFX
         }
 
         AZStd::string labelText;
+        const QModelIndexList& selectedModelIndices = GetSelectedModelIndices();
         const AZStd::vector<SimulatedObject*>& simObjs = actor->GetSimulatedObjectSetup()->GetSimulatedObjects();
         for (const SimulatedObject* obj : simObjs)
         {
-            for (int i = 0; i < m_selectedModelIndices.size(); ++i)
+            for (int i = 0; i < selectedModelIndices.size(); ++i)
             {
-                Node* node = m_selectedModelIndices[i].data(SkeletonModel::ROLE_POINTER).value<Node*>();
+                Node* node = selectedModelIndices[i].data(SkeletonModel::ROLE_POINTER).value<Node*>();
                 if (obj->FindSimulatedJointBySkeletonJointIndex(node->GetNodeIndex()))
                 {
                     if (!labelText.empty())
@@ -208,8 +210,9 @@ namespace EMotionFX
             return;
         }
 
+        const QModelIndexList& selectedModelIndices = GetSelectedModelIndices();
         // Only show the notification when it is single selection. 
-        if (m_selectedModelIndices.size() != 1)
+        if (selectedModelIndices.size() != 1)
         {
             return;
         }
@@ -250,17 +253,18 @@ namespace EMotionFX
 
     void SimulatedObjectColliderWidget::OnAddCollider(const AZ::TypeId& colliderType)
     {
-        ColliderHelpers::AddCollider(m_selectedModelIndices, PhysicsSetup::SimulatedObjectCollider, colliderType);
+        ColliderHelpers::AddCollider(GetSelectedModelIndices(), PhysicsSetup::SimulatedObjectCollider, colliderType);
     }
 
     void SimulatedObjectColliderWidget::OnCopyCollider(size_t colliderIndex)
     {
-        ColliderHelpers::CopyColliderToClipboard(m_selectedModelIndices.first(), colliderIndex, PhysicsSetup::SimulatedObjectCollider);
+        ColliderHelpers::CopyColliderToClipboard(GetSelectedModelIndices().first(), colliderIndex, PhysicsSetup::SimulatedObjectCollider);
     }
 
     void SimulatedObjectColliderWidget::OnPasteCollider(size_t colliderIndex, bool replace)
     {
-        ColliderHelpers::PasteColliderFromClipboard(m_selectedModelIndices.first(), colliderIndex, PhysicsSetup::SimulatedObjectCollider, replace);
+        ColliderHelpers::PasteColliderFromClipboard(
+            GetSelectedModelIndices().first(), colliderIndex, PhysicsSetup::SimulatedObjectCollider, replace);
     }
 
     void SimulatedObjectColliderWidget::OnRemoveCollider(size_t colliderIndex)
@@ -270,7 +274,7 @@ namespace EMotionFX
 
     Physics::CharacterColliderNodeConfiguration* SimulatedObjectColliderWidget::GetNodeConfig() const
     {
-        AZ_Assert(m_selectedModelIndices.size() == 1, "Get Node config function only return the config when it is single seleted");
+        AZ_Assert(GetSelectedModelIndices().size() == 1, "Get Node config function only return the config when it is single seleted");
         Actor* actor = GetActor();
         Node* joint = GetNode();
         if (!actor || !joint)

Gems/EMotionFX/Code/Source/Editor/Plugins/SkeletonOutliner/SkeletonOutlinerPlugin.cpp

@@ -211,15 +211,15 @@ namespace EMotionFX
 
     AZ::Outcome<const QModelIndexList&> SkeletonOutlinerPlugin::GetSelectedRowIndices()
     {
-        return AZ::Success(m_selectedRows);
+        return AZ::Success(m_treeView->selectionModel()->selectedRows());
     }
 
     void SkeletonOutlinerPlugin::OnSelectionChanged([[maybe_unused]] const QItemSelection& selected, [[maybe_unused]] const QItemSelection& deselected)
     {
-        m_selectedRows = m_treeView->selectionModel()->selectedRows();
-        if (m_selectedRows.size() == 1)
+        QModelIndexList selectedRows = m_treeView->selectionModel()->selectedRows();
+        if (selectedRows.size() == 1)
         {
-            const QModelIndex& modelIndex = m_selectedRows[0];
+            const QModelIndex& modelIndex = selectedRows[0];
             Node* selectedNode = modelIndex.data(SkeletonModel::ROLE_POINTER).value<Node*>();
             Actor* selectedActor = modelIndex.data(SkeletonModel::ROLE_ACTOR_POINTER).value<Actor*>();
             SkeletonOutlinerNotificationBus::Broadcast(&SkeletonOutlinerNotifications::SingleNodeSelectionChanged, selectedActor, selectedNode);

Gems/EMotionFX/Code/Source/Editor/SkeletonModelJointWidget.cpp

@@ -72,14 +72,7 @@ namespace EMotionFX
 
         setLayout(mainLayout);
 
-        AZ::Outcome<const QModelIndexList&> selectedRowIndicesOutcome;
-        QModelIndexList selectedModelIndices;
-        SkeletonOutlinerRequestBus::BroadcastResult(selectedRowIndicesOutcome, &SkeletonOutlinerRequests::GetSelectedRowIndices);
-        if (selectedRowIndicesOutcome.IsSuccess())
-        {
-            selectedModelIndices = selectedRowIndicesOutcome.GetValue();
-        }
-        Reinit(selectedModelIndices);
+        Reinit();
 
         // Connect to the model.
         SkeletonModel* skeletonModel = nullptr;
@@ -92,9 +85,9 @@ namespace EMotionFX
         }
     }
 
-    void SkeletonModelJointWidget::Reinit(const QModelIndexList& selectedModelIndices)
+    void SkeletonModelJointWidget::Reinit()
     {
-        m_selectedModelIndices = selectedModelIndices;
+        const QModelIndexList& selectedModelIndices = GetSelectedModelIndices();
 
         if (!EMStudio::GetManager()->GetIgnoreVisibility() && !isVisible())
         {
@@ -103,15 +96,15 @@ namespace EMotionFX
 
         if (GetActor())
         {
-            if (!m_selectedModelIndices.isEmpty())
+            if (!selectedModelIndices.isEmpty())
             {
-                if (m_selectedModelIndices.size() == 1)
+                if (selectedModelIndices.size() == 1)
                 {
                     m_jointNameLabel->setText(GetNode()->GetName());
                 }
                 else
                 {
-                    m_jointNameLabel->setText(QString("%1 joints selected").arg(m_selectedModelIndices.size()));
+                    m_jointNameLabel->setText(QString("%1 joints selected").arg(selectedModelIndices.size()));
                 }
 
                 m_noSelectionWidget->hide();
@@ -136,7 +129,7 @@ namespace EMotionFX
     void SkeletonModelJointWidget::showEvent(QShowEvent* event)
     {
         QWidget::showEvent(event);
-        Reinit(m_selectedModelIndices);
+        Reinit();
     }
 
     void SkeletonModelJointWidget::OnSelectionChanged([[maybe_unused]] const QItemSelection& selected, [[maybe_unused]] const QItemSelection& deselected)
@@ -146,36 +139,28 @@ namespace EMotionFX
         if (skeletonModel)
         {
             const QModelIndexList selectedRows = skeletonModel->GetSelectionModel().selectedRows();
-            Reinit(selectedRows);
         }
+        Reinit();
     }
 
     void SkeletonModelJointWidget::OnDataChanged([[maybe_unused]] const QModelIndex& topLeft, [[maybe_unused]] const QModelIndex& bottomRight, [[maybe_unused]] const QVector<int>& roles)
     {
-        Reinit(m_selectedModelIndices);
+        Reinit();
     }
 
     void SkeletonModelJointWidget::OnModelReset()
     {
-        Reinit(QModelIndexList());
+        Reinit();
     }
 
     Actor* SkeletonModelJointWidget::GetActor() const
     {
         Actor* actor = nullptr;
-        if (!m_selectedModelIndices.empty())
+        SkeletonModel* skeletonModel = nullptr;
+        SkeletonOutlinerRequestBus::BroadcastResult(skeletonModel, &SkeletonOutlinerRequests::GetModel);
+        if (skeletonModel)
         {
-            actor = m_selectedModelIndices[0].data(SkeletonModel::ROLE_ACTOR_POINTER).value<Actor*>();
-        }
-        
-        if (!actor)
-        {
-            SkeletonModel* skeletonModel = nullptr;
-            SkeletonOutlinerRequestBus::BroadcastResult(skeletonModel, &SkeletonOutlinerRequests::GetModel);
-            if (skeletonModel)
-            {
-                actor = skeletonModel->GetActor();
-            }
+            actor = skeletonModel->GetActor();
         }
         return actor;
     }
@@ -183,10 +168,24 @@ namespace EMotionFX
     Node* SkeletonModelJointWidget::GetNode() const
     {
         Node* node = nullptr;
-        if (!m_selectedModelIndices.empty())
+        const QModelIndexList& selectedModelIndices = GetSelectedModelIndices();
+        if (!selectedModelIndices.empty())
         {
-            node = m_selectedModelIndices[0].data(SkeletonModel::ROLE_POINTER).value<Node*>();
+            node = selectedModelIndices[0].data(SkeletonModel::ROLE_POINTER).value<Node*>();
         }
         return node;
     }
+
+    QModelIndexList SkeletonModelJointWidget::GetSelectedModelIndices() const
+    {
+        QModelIndexList selectedModelIndices;
+        SkeletonModel* skeletonModel = nullptr;
+        SkeletonOutlinerRequestBus::BroadcastResult(skeletonModel, &SkeletonOutlinerRequests::GetModel);
+        if (skeletonModel)
+        {
+            selectedModelIndices = skeletonModel->GetSelectionModel().selectedRows();
+        }
+
+        return selectedModelIndices;
+    }
 } // namespace EMotionFX

Gems/EMotionFX/Code/Source/Editor/SkeletonModelJointWidget.h

@@ -33,13 +33,14 @@ namespace EMotionFX
 
         virtual void CreateGUI();
 
-        void Reinit(const QModelIndexList& selectedModelIndices);
+        void Reinit();
 
         void showEvent(QShowEvent* event) override;
 
     protected:
         Actor* GetActor() const;
         Node* GetNode() const;
+        QModelIndexList GetSelectedModelIndices() const;
         virtual QWidget* CreateContentWidget(QWidget* parent) = 0;
         virtual QWidget* CreateNoSelectionWidget(QWidget* parent) = 0;
         virtual void InternalReinit() = 0;
@@ -50,7 +51,6 @@ namespace EMotionFX
         void OnModelReset();
 
     protected:
-        QModelIndexList m_selectedModelIndices;
         QLabel*         m_jointNameLabel;
         static int      s_jointLabelSpacing;
         static int      s_jointNameSpacing;