From 7d4f8e42812c352445a9ba971347fe8c620ce61b Mon Sep 17 00:00:00 2001 From: pereslav Date: Tue, 24 Aug 2021 13:39:54 +0100 Subject: [PATCH 1/2] Added a helper function for constructing CollisionGroup from CollisionGroups::Id. Removed remaining touch bending related code Signed-off-by: pereslav --- .../Physics/Collision/CollisionGroups.cpp | 7 +++ .../Physics/Collision/CollisionGroups.h | 6 ++- .../Physics/Collision/CollisionLayers.h | 1 - Gems/PhysX/Code/Include/PhysX/UserDataTypes.h | 53 ++++++------------- .../Code/Include/PhysX/UserDataTypes.inl | 27 +++------- 5 files changed, 34 insertions(+), 60 deletions(-) diff --git a/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionGroups.cpp b/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionGroups.cpp index 84e0184462..a0fcd2e206 100644 --- a/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionGroups.cpp +++ b/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionGroups.cpp @@ -309,5 +309,12 @@ namespace AzPhysics group.SetLayer(layer, true); return group; } + + CollisionGroup MakeCollisionGroup(const CollisionGroups::Id& id) + { + CollisionGroup group; + Physics::CollisionRequestBus::BroadcastResult(group, &Physics::CollisionRequests::GetCollisionGroupById, id); + return group; + } } diff --git a/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionGroups.h b/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionGroups.h index 5340a1a344..59f1cfb885 100644 --- a/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionGroups.h +++ b/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionGroups.h @@ -34,7 +34,6 @@ namespace AzPhysics static const CollisionGroup None; //!< Collide with nothing static const CollisionGroup All; //!< Collide with everything - static const CollisionGroup All_NoTouchBend; //!< Collide with everything, except Touch Bendable Vegetation. //! Construct a Group with the given bitmask. //! The each bit in the bitmask corresponds to a CollisionLayer. @@ -174,4 +173,9 @@ namespace AzPhysics private: AZStd::vector m_groups; }; + + //! Construct a Group with the given Id of a collision group. + //! This will lookup the group Id to retrieve the group mask. If not found, CollisionGroup::All is returned. + //! @param id The Id of the group to look up the group mask. + CollisionGroup MakeCollisionGroup(const CollisionGroups::Id& id); } diff --git a/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionLayers.h b/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionLayers.h index 6b5a4cd2a9..01141c611c 100644 --- a/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionLayers.h +++ b/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionLayers.h @@ -31,7 +31,6 @@ namespace AzPhysics static void Reflect(AZ::ReflectContext* context); static const CollisionLayer Default; //!< Default collision layer, 0. - static const CollisionLayer TouchBend; //!< Touch Bendable Vegetation collision layer. //! Construct a layer with the given index. //! @param index The index of the layer. Must be between 0 - CollisionLayers::MaxCollisionLayers. Default CollisionLayer::Default. diff --git a/Gems/PhysX/Code/Include/PhysX/UserDataTypes.h b/Gems/PhysX/Code/Include/PhysX/UserDataTypes.h index 03add0a7ec..f41526fa08 100644 --- a/Gems/PhysX/Code/Include/PhysX/UserDataTypes.h +++ b/Gems/PhysX/Code/Include/PhysX/UserDataTypes.h @@ -22,49 +22,15 @@ namespace AzPhysics namespace PhysX { - enum class BaseActorType : AZ::u32 - { - PHYSX_DEFAULT = 0, - TOUCHBENDING_TRIGGER, - }; - ///PxActor.userData is the custom data pointer that NVIDIA PhysX provides for applications to attach - ///private data. The PhysX Gem requires that this userData points to objects that subclass BaseActorData. - ///For Example: - ///The TouchBending Gem defines "struct TouchBendingInstanceHandle : public PhysX::BaseActorData", - ///While regular PhysX Gem Components use "class ActorData : public BaseActorData". - class BaseActorData - { - protected: - using PxActorUniquePtr = AZStd::unique_ptr >; - - ///This is an arbitary value used to verify the cast from void* userdata pointer on a pxActor to BaseActorData - ///is safe. If m_sanity does not have this value, then it is not safe to use the casted pointer. - ///Helps to debug if someone is setting userData pointer to something other than this class during development - static const AZ::u32 s_sanityValue = 0xba5eba11; - - AZ::u32 m_sanity = s_sanityValue; - BaseActorType m_actorType = BaseActorType::PHYSX_DEFAULT; - PxActorUniquePtr m_actor; - - BaseActorData() = default; - BaseActorData(BaseActorType type, physx::PxActor* actor); - BaseActorData(BaseActorData&& other); - BaseActorData& operator=(BaseActorData&& other); - - public: - bool IsValid() const; - BaseActorType GetType() const; - }; - - - class ActorData : public BaseActorData + ///private data. The PhysX Gem requires that this userData points to ActorData objects. + class ActorData { public: ActorData() = default; ActorData(physx::PxActor* actor); - ActorData(ActorData&& actorData) = default; - ActorData& operator=(ActorData&& actorData) = default; + ActorData(ActorData&& actorData); + ActorData& operator=(ActorData&& actorData); void Invalidate(); AZ::EntityId GetEntityId() const; @@ -86,7 +52,18 @@ namespace PhysX AzPhysics::SimulatedBody* GetSimulatedBody() const; + bool IsValid() const; + private: + using PxActorUniquePtr = AZStd::unique_ptr >; + + ///This is an arbitary value used to verify the cast from void* userdata pointer on a pxActor to ActorData + ///is safe. If m_sanity does not have this value, then it is not safe to use the casted pointer. + ///Helps to debug if someone is setting userData pointer to something other than this class during development + static const AZ::u32 s_sanityValue = 0xba5eba11; + + AZ::u32 m_sanity = s_sanityValue; + PxActorUniquePtr m_actor; struct Payload { diff --git a/Gems/PhysX/Code/Include/PhysX/UserDataTypes.inl b/Gems/PhysX/Code/Include/PhysX/UserDataTypes.inl index d0fa850dbf..97d0350a90 100644 --- a/Gems/PhysX/Code/Include/PhysX/UserDataTypes.inl +++ b/Gems/PhysX/Code/Include/PhysX/UserDataTypes.inl @@ -9,9 +9,8 @@ namespace PhysX { - // BaseActorData START **************************************************** - inline BaseActorData::BaseActorData(BaseActorType type, physx::PxActor* actor) : - m_sanity(s_sanityValue), m_actorType(type) + inline ActorData::ActorData(physx::PxActor* actor) + : m_sanity(s_sanityValue) { auto nullUserData = [](physx::PxActor* actorToSet) { @@ -23,38 +22,26 @@ namespace PhysX actor->userData = this; } - inline BaseActorData::BaseActorData(BaseActorData&& other) : - m_sanity(s_sanityValue), m_actorType(other.m_actorType), m_actor(AZStd::move(other.m_actor)) + inline ActorData::ActorData(ActorData&& other) + : m_sanity(s_sanityValue) + , m_actor(AZStd::move(other.m_actor)) { m_actor->userData = this; } - inline BaseActorData& BaseActorData::operator=(BaseActorData&& other) + inline ActorData& ActorData::operator=(ActorData&& other) { m_sanity = s_sanityValue; - m_actorType = other.m_actorType; m_actor = AZStd::move(other.m_actor); m_actor->userData = this; return *this; } - inline bool BaseActorData::IsValid() const + inline bool ActorData::IsValid() const { return m_sanity == s_sanityValue; } - inline BaseActorType BaseActorData::GetType() const - { - return m_actorType; - } - // BaseActorData END ****************************************************** - - - // ActorData START ******************************************************** - inline ActorData::ActorData(physx::PxActor* actor) : BaseActorData(BaseActorType::PHYSX_DEFAULT, actor) - { - } - inline void ActorData::Invalidate() { m_actor = nullptr; From 21605f15ccc934133022d0ed68037425fd1c1a15 Mon Sep 17 00:00:00 2001 From: pereslav Date: Tue, 24 Aug 2021 16:00:00 +0100 Subject: [PATCH 2/2] CR Feedback addressing Signed-off-by: pereslav --- .../AzFramework/Physics/Collision/CollisionGroups.cpp | 2 +- .../AzFramework/Physics/Collision/CollisionGroups.h | 4 ++-- Gems/PhysX/Code/Include/PhysX/UserDataTypes.h | 4 ++-- Gems/PhysX/Code/Include/PhysX/UserDataTypes.inl | 10 +++++----- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionGroups.cpp b/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionGroups.cpp index a0fcd2e206..2100b0c394 100644 --- a/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionGroups.cpp +++ b/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionGroups.cpp @@ -310,7 +310,7 @@ namespace AzPhysics return group; } - CollisionGroup MakeCollisionGroup(const CollisionGroups::Id& id) + CollisionGroup GetCollisionGroupById(const CollisionGroups::Id& id) { CollisionGroup group; Physics::CollisionRequestBus::BroadcastResult(group, &Physics::CollisionRequests::GetCollisionGroupById, id); diff --git a/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionGroups.h b/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionGroups.h index 59f1cfb885..8f75d2560d 100644 --- a/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionGroups.h +++ b/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionGroups.h @@ -174,8 +174,8 @@ namespace AzPhysics AZStd::vector m_groups; }; - //! Construct a Group with the given Id of a collision group. + //! Retrieves a Group with the given Id of a collision group. //! This will lookup the group Id to retrieve the group mask. If not found, CollisionGroup::All is returned. //! @param id The Id of the group to look up the group mask. - CollisionGroup MakeCollisionGroup(const CollisionGroups::Id& id); + CollisionGroup GetCollisionGroupById(const CollisionGroups::Id& id); } diff --git a/Gems/PhysX/Code/Include/PhysX/UserDataTypes.h b/Gems/PhysX/Code/Include/PhysX/UserDataTypes.h index f41526fa08..aad2a716ec 100644 --- a/Gems/PhysX/Code/Include/PhysX/UserDataTypes.h +++ b/Gems/PhysX/Code/Include/PhysX/UserDataTypes.h @@ -60,9 +60,9 @@ namespace PhysX ///This is an arbitary value used to verify the cast from void* userdata pointer on a pxActor to ActorData ///is safe. If m_sanity does not have this value, then it is not safe to use the casted pointer. ///Helps to debug if someone is setting userData pointer to something other than this class during development - static const AZ::u32 s_sanityValue = 0xba5eba11; + static constexpr AZ::u32 SanityValue = 0xba5eba11; - AZ::u32 m_sanity = s_sanityValue; + AZ::u32 m_sanity = SanityValue; PxActorUniquePtr m_actor; struct Payload diff --git a/Gems/PhysX/Code/Include/PhysX/UserDataTypes.inl b/Gems/PhysX/Code/Include/PhysX/UserDataTypes.inl index 97d0350a90..e01163d7ee 100644 --- a/Gems/PhysX/Code/Include/PhysX/UserDataTypes.inl +++ b/Gems/PhysX/Code/Include/PhysX/UserDataTypes.inl @@ -10,7 +10,6 @@ namespace PhysX { inline ActorData::ActorData(physx::PxActor* actor) - : m_sanity(s_sanityValue) { auto nullUserData = [](physx::PxActor* actorToSet) { @@ -23,23 +22,25 @@ namespace PhysX } inline ActorData::ActorData(ActorData&& other) - : m_sanity(s_sanityValue) + : m_sanity(other.m_sanity) , m_actor(AZStd::move(other.m_actor)) + , m_payload(AZStd::move(other.m_payload)) { m_actor->userData = this; } inline ActorData& ActorData::operator=(ActorData&& other) { - m_sanity = s_sanityValue; + m_sanity = other.m_sanity; m_actor = AZStd::move(other.m_actor); m_actor->userData = this; + m_payload = AZStd::move(other.m_payload); return *this; } inline bool ActorData::IsValid() const { - return m_sanity == s_sanityValue; + return m_sanity == SanityValue; } inline void ActorData::Invalidate() @@ -136,5 +137,4 @@ namespace PhysX return nullptr; } } - // ActorData END ******************************************************** } //namespace PhysX