Setting RigidBody global_transform into the _input function fails.

[Cykyrios]

Godot version:
3.2 custom build, issue bisected to commit e7d8464

OS/device including version:
Linux Mint 20

Issue description:
When setting the global_transform property of a RigidBody, the expected result sometimes happens, sometimes does not, seemingly at random.

Steps to reproduce:
Add a RigidBody to a scene, add a script that sets its global_transform property following an input event, run the game and press the key to confirm whether the transform actually changes.

Minimal reproduction project:
BulletGlobalTransform.zip
This project contains a workaround for the issue. First run the project and press space to reset the falling cube's position. Doing so should fail most of the time.
Open the RigidBody script and uncomment the yield line: now the transform should only be set during a physics update, and pressing space will always work.

Is this issue a regression or expected behavior following PR #40185?

This PR has also completely changed the behavior of my physics-based game (a drone simulator), with the drone getting its thrust more or less halved, even though thrust is calculated during physics frames only.

[TheDuriel]

That seems expected?

You are not meant to be able to directly modify a RigidBody in this way. Never were.

The "correct" way to do this, would be to suspend the physics processing of the body for 3 frames. And set the position on the second frame.

[Cykyrios]

You are not supposed to change a RigidBody's transform every frame (or even every few frames), that I can agree. I'm talking about one-time changes here, a way to teleport the RigidBody whenever it needs to be reset. Here's what the documentation says about this:

If you only need to place a rigid body once, for example to set its initial location, you can use the methods provided by the Spatial node, such as set_global_transform() or look_at(). However, these methods cannot be called every frame or the physics engine will not be able to correctly simulate the body's state.

I'm honestly not convinced a simple reset would require pausing the physics (this could also introduce noticeable jitter in scenarios where you're teleporting a moving RigidBody that needs to keep moving after being teleported).
As for the 3-frame pause, I believe the result would be the same as changing the body's state at the end of a physics frame, except with a small delay. As long as you synchronize the state change with the physics processing, this should not be an issue.

I am perfectly fine with explicitly synchronizing with the physics processing though, as it does make more sense than randomly calling from an input event or a signal.

[AndreaCatania]

@Cybolic can you also open an issue with a sample project that shows that your applied force is halved?

[AndreaCatania]

This issue happens on Godot Physics too, and it was not an issue before because Bullet was 2 Physics Frame behind.

Cause

The cause of this issue is a processing order, and it's a bit complex to explain.

First, remember that the code that updates the PhysicsServer is the following:

PhysicsServer3D::get_singleton()->body_set_state(rid, PhysicsServer3D::BODY_STATE_TRANSFORM, get_global_transform());

Note: it takes the transform returned by Node3D::get_global_transform and set it to PhysicsServer.

When the input is fetched by this line https://github.com/godotengine/godot/blob/master/platform/linuxbsd/os_linuxbsd.cpp#L234 it triggers your _input function that changes only the Node3D transform - so doesn't update the transform on the physics server (because it's a lazy update).

The PhysicsServer is updated only when the first transform flush happens, here: https://github.com/godotengine/godot/blob/master/scene/main/scene_tree.cpp#L412 - triggered by this https://github.com/godotengine/godot/blob/master/main/main.cpp#L2169

Look closely on this portion of the code:

main.cpp::iteration

	for (int iters = 0; iters < advance.physics_steps; ++iters) {
		uint64_t physics_begin = OS::get_singleton()->get_ticks_usec();

		PhysicsServer3D::get_singleton()->sync();
		PhysicsServer3D::get_singleton()->flush_queries();

		PhysicsServer2D::get_singleton()->sync();
		PhysicsServer2D::get_singleton()->flush_queries();

		if (OS::get_singleton()->get_main_loop()->iteration(frame_slice * time_scale)) {
			exit = true;
			break;
		}

Notice that flush_query (in order to update the transform on the rendering engine) overwrites your changes done into the _input function, before it's submitted to the PhysicsServer.

So once the OS::get_singleton()->get_main_loop()->iteration(frame_slice * time_scale) is executed, it will just notify the wrong transform to PhysicsServer and not yours.

To do a recap, the flow is the following:

• Fetch event (triggers _input, and set the Node3D Transform)
• Flush Query (Updates the Node3D Transform)
• Flush Transform (Notify the Node3D Transform to PhysicsServer)

The reason why it happens randomly is because the inputs sometimes is fetched by also _process and _physics_process.

[AndreaCatania]

This can be easily fixed by putting a transform flush at the start of the iteration function, but I have the strong feeling that it's not a good thing to do, so let's summon @reduz .

[Cykyrios]

@Cybolic can you also open an issue with a sample project that shows that your applied force is halved?

The halved force was a gut feeling, it was actually related to issue #40508: I was applying gravity in integrate_forces(), so it was not my force that was too weak, but rather gravity applied twice.

Thanks for the explanation about the flow of operations, I never took a look at this before so that's some interesting info to me.
If the engine-side fix is simple enough and worth it, I'm fine with this, otherwise I think it's sensible enough to expect users to synchronize to the next physics frame, in which case this should probably be documented somewhere.

[TheDuriel]

we're you applying gravity by setting the correctly adjusted linear velocity on the physics state? or we're you not using the provided state. in which case, yeah it wouldnt work.

[Cykyrios]

I'm using custom integration, so I'm applying gravity manually in integrate_forces() when calculating the total acceleration:

var a := forces * state.inverse_mass + state.total_gravity
state.linear_velocity += a * state.step

After a reset input, I would set the transform directly and also set the RigidBody's linear_velocity to zero (which does work), but from what @AndreaCatania said, only the transform part is ignored as it gets overridden. Just to be clear, I was getting twice the gravity because of #40508.