Browse Source

Bugfix: When there were no active bodies, the step listeners weren't called. (#1557)

This meant they couldn't wake up bodies. The step listeners are now only skipped if the physics system is updated with zero delta time.

Fixes #1545
Jorrit Rouwe 4 tháng trước cách đây
mục cha
commit
3f62f22248

+ 1 - 0
Docs/ReleaseNotes.md

@@ -53,6 +53,7 @@ For breaking API changes see [this document](https://github.com/jrouwe/JoltPhysi
 * When calling `PhysicsSystem::Update` with a delta time of 0, contact remove callbacks were triggered by accident for all existing contacts.
 * Fixed 'HingeConstraint' not having limits if `LimitsMin` was set to `-JPH_PI` or `LimitsMax` was set to `JPH_PI`. It should only be turned off if both are.
 * Fixed 'CylinderShape::GetSupportingFace' returning the wrong face. When the height of a cylinder was small compared to its radius, it would sink more into the ground than needed during simulation.
+* When there were no active bodies, the step listeners weren't called. This meant they couldn't wake up bodies. The step listeners are now only skipped if the physics system is updated with zero delta time.
 
 ## v5.2.0
 

+ 2 - 2
Jolt/Physics/PhysicsSystem.cpp

@@ -141,10 +141,10 @@ EPhysicsUpdateError PhysicsSystem::Update(float inDeltaTime, int inCollisionStep
 	// Sync point for the broadphase. This will allow it to do clean up operations without having any mutexes locked yet.
 	mBroadPhase->FrameSync();
 
-	// If there are no active bodies or there's no time delta
+	// If there are no active bodies (and no step listener to wake them up) or there's no time delta
 	uint32 num_active_rigid_bodies = mBodyManager.GetNumActiveBodies(EBodyType::RigidBody);
 	uint32 num_active_soft_bodies = mBodyManager.GetNumActiveBodies(EBodyType::SoftBody);
-	if ((num_active_rigid_bodies == 0 && num_active_soft_bodies == 0) || inDeltaTime <= 0.0f)
+	if ((num_active_rigid_bodies == 0 && num_active_soft_bodies == 0 && mStepListeners.empty()) || inDeltaTime <= 0.0f)
 	{
 		mBodyManager.LockAllBodies();
 

+ 56 - 10
UnitTests/Physics/PhysicsStepListenerTests.cpp

@@ -41,25 +41,21 @@ TEST_SUITE("StepListenerTest")
 		for (TestStepListener &l : listeners)
 			c.GetSystem()->AddStepListener(&l);
 
-		// Step the simulation
-		c.SimulateSingleStep();
-
-		// There aren't any active bodies so no listeners should have been called
+		// Stepping without delta time should not trigger step listeners
+		c.SimulateNoDeltaTime();
 		for (TestStepListener &l : listeners)
 			CHECK(l.mCount == 0);
 
-		// Now add an active body
-		c.CreateBox(RVec3::sZero(), Quat::sIdentity(), EMotionType::Dynamic, EMotionQuality::Discrete, Layers::MOVING, Vec3::sOne());
-
-		// Step the simulation
+		// Stepping with delta time should call the step listeners as they can activate bodies
 		c.SimulateSingleStep();
-
 		for (TestStepListener &l : listeners)
 			CHECK(l.mCount == inCollisionSteps);
 
+		// Adding an active body should have no effect, step listeners should still be called
+		c.CreateBox(RVec3::sZero(), Quat::sIdentity(), EMotionType::Dynamic, EMotionQuality::Discrete, Layers::MOVING, Vec3::sOne());
+
 		// Step the simulation
 		c.SimulateSingleStep();
-
 		for (TestStepListener &l : listeners)
 			CHECK(l.mCount == 2 * inCollisionSteps);
 
@@ -92,4 +88,54 @@ TEST_SUITE("StepListenerTest")
 	{
 		DoTest(4);
 	}
+
+	// Activate a body in a step listener
+	TEST_CASE("TestActivateInStepListener")
+	{
+		PhysicsTestContext c(1.0f / 60.0f, 2);
+		c.ZeroGravity();
+
+		// Create a box
+		Body &body = c.CreateBox(RVec3::sZero(), Quat::sIdentity(), EMotionType::Dynamic, EMotionQuality::Discrete, Layers::MOVING, Vec3::sOne(), EActivation::DontActivate);
+		body.GetMotionProperties()->SetLinearDamping(0.0f);
+		BodyID body_id = body.GetID();
+
+		static const Vec3 cVelocity(10.0f, 0, 0);
+
+		class MyStepListener : public PhysicsStepListener
+		{
+		public:
+								MyStepListener(const BodyID &inBodyID, BodyInterface &inBodyInterface) : mBodyInterface(inBodyInterface), mBodyID(inBodyID) { }
+
+			virtual void		OnStep(const PhysicsStepListenerContext &inContext) override
+			{
+				if (inContext.mIsFirstStep)
+				{
+					// We activate the body and set a velocity in the first step
+					CHECK(!mBodyInterface.IsActive(mBodyID));
+					mBodyInterface.SetLinearVelocity(mBodyID, cVelocity);
+					CHECK(mBodyInterface.IsActive(mBodyID));
+				}
+				else
+				{
+					// In the second step, the body should already have been activated
+					CHECK(mBodyInterface.IsActive(mBodyID));
+				}
+			}
+
+		private:
+			BodyInterface &		mBodyInterface;
+			BodyID				mBodyID;
+		};
+
+		MyStepListener listener(body_id, c.GetSystem()->GetBodyInterfaceNoLock());
+		c.GetSystem()->AddStepListener(&listener);
+
+		c.SimulateSingleStep();
+
+		BodyInterface &bi = c.GetBodyInterface();
+		CHECK(bi.IsActive(body_id));
+		CHECK(bi.GetLinearVelocity(body_id) == cVelocity);
+		CHECK(bi.GetPosition(body_id) == RVec3(c.GetDeltaTime() * cVelocity));
+	}
 }

+ 9 - 0
UnitTests/PhysicsTestContext.cpp

@@ -86,6 +86,15 @@ Body &PhysicsTestContext::CreateSphere(RVec3Arg inPosition, float inRadius, EMot
 	return CreateBody(new SphereShapeSettings(inRadius), inPosition, Quat::sIdentity(), inMotionType, inMotionQuality, inLayer, inActivation);
 }
 
+EPhysicsUpdateError PhysicsTestContext::SimulateNoDeltaTime()
+{
+	EPhysicsUpdateError errors = mSystem->Update(0.0f, mCollisionSteps, mTempAllocator, mJobSystem);
+#ifndef JPH_DISABLE_TEMP_ALLOCATOR
+	JPH_ASSERT(static_cast<TempAllocatorImpl *>(mTempAllocator)->IsEmpty());
+#endif // JPH_DISABLE_TEMP_ALLOCATOR
+	return errors;
+}
+
 EPhysicsUpdateError PhysicsTestContext::SimulateSingleStep()
 {
 	EPhysicsUpdateError errors = mSystem->Update(mDeltaTime, mCollisionSteps, mTempAllocator, mJobSystem);

+ 3 - 0
UnitTests/PhysicsTestContext.h

@@ -52,6 +52,9 @@ public:
 		return *constraint;
 	}
 
+	// Call the update with zero delta time
+	EPhysicsUpdateError SimulateNoDeltaTime();
+
 	// Simulate only for one delta time step
 	EPhysicsUpdateError	SimulateSingleStep();