`apply_impulse` uses outdated `mass` value

[Haydoggo]

Godot version

v4.1.rc.custom_build [16dd4e572]

System information

Godot v4.1.rc (16dd4e572) - Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce GTX 1080 (NVIDIA; 31.0.15.1694) - Intel(R) Core(TM) i5-10600KF CPU @ 4.10GHz (12 Threads)

Issue description

calling apply_impulse or apply_central_impulse shortly after changing the mass of a RigidBody3D or RigidBody2D results in an impulse that does not respect the new mass.

Steps to reproduce

Change the mass of a rigidbody and apply an impulse before 2 physics frames have passed.

body.mass = 0.5
body.apply_central_impulse(Vector3.FORWARD)

A workaround is to force update the mass of the body as a side effect of calling:

PhysicsServer3D.body_set_mode(body.get_rid(), PhysicsServer3D.BODY_MODE_RIGID)

or to wait 2 physics frames with

for i in 2:
  await get_tree().physics_frame

(waiting one frame with this method doesn't seem to work, nor does a deferred call to apply_impulse)

The MRP has buttons for applying impulses and changing mass.

Minimal reproduction project

MRP.zip

[quinnyo]

The behaviour is as you describe it, but you've got an error/typo in your example/MRP -- note the '.bind' in body.apply_central_impulse.bind(Vector3.FORWARD), in your comment and project.

I think it's expected that there will be some delay when setting the physics properties via the node's properties. Especially so when changing the properties is done in gui/idle process. But the current behaviour feels like calling the method is working (albeit with a stale value) and setting the property is not, which is unhelpful and surprising.

[Haydoggo]

you've got an error/typo in your example/MRP

Oops, looks like the was leftover from some last minute testing with deferred calls, updated the MRP now!

[DashiellRaccoon]

I discovered that RigidBody3D.ApplyImpulse will always use the wrong mass when called in the _Ready function. I think this is caused by the same issue. In my case the mass was defined in the scene, not changed in the code. The workaround is to skip a physics frame before applying impulses.

[Qoltaic]

The same behavior occurs when instantiating a rigidbody3d scene with a preset mass value. If apply_impulse or apply_central_impulse are used directly it seems to use a default of 1Kg regardless of the set value, however, waiting a bit resolves it.

[knil79]

I can confirm that for i in 2: await get_tree().physics_frame Is a working workaround. Mass is used properly by then.

I was starting to go crazy trying to figure out what was wrong before I found this bug!

[bbbscarter]

Just to add - this affects inertia as well; if you call apply_impulse just after an object has been created, no rotational velocity is applied, since inertia is 0.

Digging around, the problem appears to be that apply_impulse uses _inv_mass and _inv_inertia directly; but _inv_mass and inv_inertia are initialised in the physics tick, rather than when the object is constructed. As such any calls to apply_impulse and related methods call before the first physics tick are working with bad data.

Detail:

• _inv_mass and inv_inertia are set up in update_mass_properties.

• Calls to update_mass_properties are all deferred - they are put into mass_properties_update_list, which is polled at the beginning of the physics server update (via GodotSpace3D::Setup()). See https://github.com/godotengine/godot/blob/7529c0bec597d70bc61975a82063bb5112ac8879/servers/physics_3d/godot_body_3d.cpp#L37-L41

• This problem doesn’t affect apply_force because here the forces are accumulated, and only combined with mass/inertia inside integrate_forces, which is inside the physics server step, and guaranteed to be called after update_mass_properties.

Workaround without modifying Godot:

• Call apply_impulse in _process_physics. This doesn’t work because it’s called before the physics step.
• As mentioned above - use PhysicsServer3D.body_set_mode(body.get_rid(), PhysicsServer3D.BODY_MODE_RIGID). This works for mass, but doesn’t update inertia.
• As mentioned above - wait for a couple of physics ticks before calling apply_impulse. This works, but is not at all obvious; and also results in one full physics update before the impulse is applied.
• Use Jolt. This seems a bit extreme!

Options

I've been looking around at how to fix most easily within Godot, and there seem to be a few options:

1. When calling GodotPhysicsServer3D::body_apply_impulse(), insert a call to update_mass_properties. e.g. here: https://github.com/godotengine/godot/blob/7529c0bec597d70bc61975a82063bb5112ac8879/servers/physics_3d/godot_physics_server_3d.cpp#L704-L712. This would be the smallest change. Potential downsides:

• From the code it's not obvious why calls to update_mass_properties are deferred in the first place - e.g. I'm assuming it's for multithreaded physics. If so, calling it out of order will be problematic.

2. Treat apply_impulse like apply_force - accumulate the impulses, and defer multiplying them with mass/inertia to integrate_forces. This doesn't change the existing order of mass calculations, so should be safer - but, potential downsides:

• Internal parts of the physics code rely on apply_impulse being applied immediately - e.g. godot_body_pair, joints, etc, so this would need to only apply to server API calls - e.g GodotPhysicsServer3D::body_apply_impulse.
• Currently linear_velocity and angular_velocity are immediately updated when body_apply_impulse() is called - deferring it means they won’t be updated until the next physics tick. This might not matter, if it only affects external calls to body_apply_impulse.

Proposal

Given the multi-threading concern with option 1, I'm tempted to give option 2 a go, unless someone tells me it's a terrible idea?

Thanks!

Simon