Merge commit 'a29623e5f0d5d1bd83bcc7fa043f70e4c84e1123' into daimini/gitflow_211027_o3de

Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com>

Gems/PhysX/Code/Source/EditorShapeColliderComponent.cpp

@@ -348,19 +348,7 @@ namespace PhysX
         LmbrCentral::BoxShapeComponentRequestsBus::EventResult(boxDimensions, GetEntityId(),
             &LmbrCentral::BoxShapeComponentRequests::GetBoxDimensions);
 
-        if (m_shapeType != ShapeType::Box)
+        SetShapeConfig(ShapeType::Box, Physics::BoxShapeConfiguration(boxDimensions));
-        {
-            m_shapeConfigs.clear();
-            m_shapeConfigs.emplace_back(AZStd::make_shared<Physics::BoxShapeConfiguration>(boxDimensions));
-
-            m_shapeType = ShapeType::Box;
-        }
-        else
-        {
-            Physics::BoxShapeConfiguration& configuration =
-                static_cast<Physics::BoxShapeConfiguration&>(*m_shapeConfigs.back());
-            configuration = Physics::BoxShapeConfiguration(boxDimensions);
-        }
 
         m_shapeConfigs.back()->m_scale = scale;
         m_geometryCache.m_boxDimensions = scale * boxDimensions;
@@ -374,19 +362,7 @@ namespace PhysX
         const Physics::CapsuleShapeConfiguration& capsuleShapeConfig =
             Utils::ConvertFromLmbrCentralCapsuleConfig(lmbrCentralCapsuleShapeConfig);
 
-        if (m_shapeType != ShapeType::Capsule)
+        SetShapeConfig(ShapeType::Capsule, capsuleShapeConfig);
-        {
-            m_shapeConfigs.clear();
-            m_shapeConfigs.emplace_back(AZStd::make_shared<Physics::CapsuleShapeConfiguration>(capsuleShapeConfig));
-
-            m_shapeType = ShapeType::Capsule;
-        }
-        else
-        {
-            Physics::CapsuleShapeConfiguration& configuration =
-                static_cast<Physics::CapsuleShapeConfiguration&>(*m_shapeConfigs.back());
-            configuration = capsuleShapeConfig;
-        }
 
         m_shapeConfigs.back()->m_scale = scale;
         const float scalarScale = scale.GetMaxElement();
@@ -400,19 +376,7 @@ namespace PhysX
         LmbrCentral::SphereShapeComponentRequestsBus::EventResult(radius, GetEntityId(),
             &LmbrCentral::SphereShapeComponentRequests::GetRadius);
 
-        if (m_shapeType != ShapeType::Sphere)
+        SetShapeConfig(ShapeType::Sphere, Physics::SphereShapeConfiguration(radius));
-        {
-            m_shapeConfigs.clear();
-            m_shapeConfigs.emplace_back(AZStd::make_shared<Physics::SphereShapeConfiguration>(radius));
-
-            m_shapeType = ShapeType::Sphere;
-        }
-        else
-        {
-            Physics::SphereShapeConfiguration& configuration =
-                static_cast<Physics::SphereShapeConfiguration&>(*m_shapeConfigs.back());
-            configuration = Physics::SphereShapeConfiguration(radius);
-        }
         
         m_shapeConfigs.back()->m_scale = scale;
         m_geometryCache.m_radius = scale.GetMaxElement() * radius;
@@ -455,19 +419,7 @@ namespace PhysX
 
         if (shapeConfig.has_value())
         {
-            if (m_shapeType != ShapeType::Cylinder)
+            SetShapeConfig(ShapeType::Cylinder, shapeConfig.value());
-            {
-                m_shapeConfigs.clear();
-                m_shapeConfigs.push_back(AZStd::make_shared<Physics::CookedMeshShapeConfiguration>(shapeConfig.value()));
-
-                m_shapeType = ShapeType::Cylinder;
-            }
-            else
-            {
-                Physics::CookedMeshShapeConfiguration& configuration =
-                    static_cast<Physics::CookedMeshShapeConfiguration&>(*m_shapeConfigs.back());
-                configuration = Physics::CookedMeshShapeConfiguration(shapeConfig.value());
-            }
 
             CreateStaticEditorCollider();
         }

Gems/PhysX/Code/Source/EditorShapeColliderComponent.h

@@ -8,6 +8,7 @@
 
 #pragma once
 
+#include <AzCore/std/smart_ptr/make_shared.h>
 #include <AzCore/Component/TransformBus.h>
 #include <AzCore/Component/NonUniformScaleBus.h>
 #include <AzFramework/Physics/Shape.h>
@@ -90,12 +91,15 @@ namespace PhysX
         void UpdateBoxConfig(const AZ::Vector3& scale);
         void UpdateCapsuleConfig(const AZ::Vector3& scale);
         void UpdateSphereConfig(const AZ::Vector3& scale);
+        void UpdateCylinderConfig(const AZ::Vector3& scale);
         void UpdatePolygonPrismDecomposition();
         void UpdatePolygonPrismDecomposition(const AZ::PolygonPrismPtr polygonPrismPtr);
 
-        void RefreshUiProperties();
+        // Helper function to set a specific shape configuration
+        template<typename ConfigType>
+        void SetShapeConfig(ShapeType shapeType, const ConfigType& shapeConfig);
 
-        void UpdateCylinderConfig(const AZ::Vector3& scale);
+        void RefreshUiProperties();
 
         AZ::u32 OnSubdivisionCountChange();
         AZ::Crc32 SubdivisionCountVisibility();
@@ -154,4 +158,28 @@ namespace PhysX
         AZ::NonUniformScaleChangedEvent::Handler m_nonUniformScaleChangedHandler; //!< Responds to changes in non-uniform scale.
         AZ::Vector3 m_currentNonUniformScale = AZ::Vector3::CreateOne(); //!< Caches the current non-uniform scale.
     };
+
+    template<typename ConfigType>
+    void EditorShapeColliderComponent::SetShapeConfig(ShapeType shapeType, const ConfigType& shapeConfig)
+    {
+        if (m_shapeType != shapeType)
+        {
+            m_shapeConfigs.clear();
+            m_shapeType = shapeType;
+        }
+
+        if (m_shapeConfigs.empty())
+        {
+            m_shapeConfigs.emplace_back(AZStd::make_shared<ConfigType>(shapeConfig));
+        }
+        else
+        {
+            AZ_Assert(m_shapeConfigs.back()->GetShapeType() == shapeConfig.GetShapeType(),
+                "Expected Physics shape configuration with shape type %d but found one with shape type %d.",
+                static_cast<int>(shapeConfig.GetShapeType()), static_cast<int>(m_shapeConfigs.back()->GetShapeType()));
+            ConfigType& configuration =
+                static_cast<ConfigType&>(*m_shapeConfigs.back());
+            configuration = shapeConfig;
+        }
+    }
 } // namespace PhysX

Gems/PhysX/Code/Tests/ShapeColliderComponentTests.cpp

@@ -320,7 +320,7 @@ namespace PhysXEditorTests
     
     TEST_F(PhysXEditorFixture, EditorShapeColliderComponent_ShapeColliderWithCylinderWithNullHeight_HandledGracefully)
     {
-        ValidateInvalidEditorShapeColliderComponentParams(0.f, 1.f);
+        ValidateInvalidEditorShapeColliderComponentParams(1.f, 0.f);
     }
     
     TEST_F(PhysXEditorFixture, EditorShapeColliderComponent_ShapeColliderWithCylinderWithNullRadiusAndNullHeight_HandledGracefully)
@@ -338,6 +338,44 @@ namespace PhysXEditorTests
         ValidateInvalidEditorShapeColliderComponentParams(0.f, -1.f);
     }
 
+    TEST_F(PhysXEditorFixture, EditorShapeColliderComponent_ShapeColliderWithCylinderSwitchingFromNullHeightToValidHeight_HandledGracefully)
+    {
+        // create an editor entity with a shape collider component and a cylinder shape component
+        EntityPtr editorEntity = CreateInactiveEditorEntity("ShapeColliderComponentEditorEntity");
+        editorEntity->CreateComponent<PhysX::EditorShapeColliderComponent>();
+        editorEntity->CreateComponent(LmbrCentral::EditorCylinderShapeComponentTypeId);
+        editorEntity->Activate();
+
+        const float validRadius = 1.0f;
+        const float nullHeight = 0.0f;
+        const float validHeight = 1.0f;
+
+        LmbrCentral::CylinderShapeComponentRequestsBus::Event(editorEntity->GetId(),
+            &LmbrCentral::CylinderShapeComponentRequests::SetRadius, validRadius);
+
+        {
+            UnitTest::ErrorHandler dimensionWarningHandler("Negative or zero cylinder dimensions are invalid");
+            UnitTest::ErrorHandler colliderWarningHandler("No Collider or Shape information found when creating Rigid body");
+
+            LmbrCentral::CylinderShapeComponentRequestsBus::Event(editorEntity->GetId(),
+                &LmbrCentral::CylinderShapeComponentRequests::SetHeight, nullHeight);
+
+            EXPECT_EQ(dimensionWarningHandler.GetExpectedWarningCount(), 1);
+            EXPECT_EQ(colliderWarningHandler.GetExpectedWarningCount(), 1);
+        }
+
+        {
+            UnitTest::ErrorHandler dimensionWarningHandler("Negative or zero cylinder dimensions are invalid");
+            UnitTest::ErrorHandler colliderWarningHandler("No Collider or Shape information found when creating Rigid body");
+
+            LmbrCentral::CylinderShapeComponentRequestsBus::Event(editorEntity->GetId(),
+                &LmbrCentral::CylinderShapeComponentRequests::SetHeight, validHeight);
+
+            EXPECT_EQ(dimensionWarningHandler.GetExpectedWarningCount(), 0);
+            EXPECT_EQ(colliderWarningHandler.GetExpectedWarningCount(), 0);
+        }
+    }
+
     TEST_F(PhysXEditorFixture, EditorShapeColliderComponent_ShapeColliderWithBoxAndRigidBody_CorrectRuntimeComponents)
     {
         // create an editor entity with a shape collider component and a box shape component