Cleared forces at end of body_2D integrate_forces

[Duroxxigar]

Cleared forces at end of body_2D integrate_forces for consistency with 3D version. Could not find where to fix in bullet implementation.

Fixes #38646

[madmiraal]

This is not a good idea. As stated in my comment in #38646, there is no difference between the 2D and 3D behaviour and the current behaviour is by design.

[Duroxxigar]

@madmiraal That's fine. That topic is still getting debated. This PR is merely here for when the verdict is decided. And the poster in that showed the lines of code in the files that he was referring too.

[madmiraal]

I have been corrected. There is a difference between the 2D and 3D behaviour. However, I still think it's a bad idea, because, as discussed in my following comment, if we clear applied_force (and applied_torque) at the end of integrate_forces() it would make add_force() and apply_impulse() (as well as add_central_force() and apply_central_impulse(), and add_torque() and apply_torque_impulse()) the same (except for the units used).

My inclination is to suggest changing the 3D version to not clear applied_force and applied_torque. However, either way there would be a significant change in the way either 2D or 3D RigidBody physics currently works, and these differences have been there since 0b806ee.

[mini-glitch]

Clearing applied_force would not make add_force() and apply_impulse() the same. apply_impulse() is an immediate change to the velocity, unaffected by the physics step delta, making it dependent on the physics framerate. Conversely add_force() adds to applied_force which is affected by the physics step delta, and is therefore independent of the physics framerate. This difference also highlights the intended usage of each function: apply_impulse() is for an immediate change in velocity such as an explosion or a character dashing, while add_force() is a force applied over time, such as a spaceship thruster or a giant fan pushing an object.

This change is correct. applied_force and applied_torque are just accumulating the total force applied to the rigidbody since the last physics step so it can all be applied at once, and they need to be cleared after each integration or they will wrongly accumulate forever.

[madmiraal]

@sakar If the forces are cleared at the end of every step, add_force() and apply_impulse() will be the same except for the units used. As you point out, the difference would only be the physics step delta: add_force(force_vector) = add_impulse(force_vector * delta).

Your example of a spaceship thruster is exactly why forces should not be cleared at the end of each step. The spaceship thruster is a continuous force that should only be changed when the player sends an input signal. Your code should be event driven. It's the physics engine, not your code, that should apply the force at every step. Gravity is the same. It's set once and the physics engine applies it at every step.

[mini-glitch]

@madmiraal The spaceship thruster demonstrates exactly why the current implementation is wrong. A thruster applies a continual force while it's on, applying some amount of force over time. The over time part is why you should be calling add_force() each _physics_process() while the thruster is on, because the force is affected by delta. This is exactly the same as having to call move_and_slide() each _physics_process(). The entire point of _physics_process() is for this per-step movement-over-time logic.

Clearing the applied forces each step is exactly how a number of other physics engines work, including Godot's own 3D physics implementation, Box2D, Bullet, PhysX, and Chipmunk. Changing the behavior of Godot's 3D physics to match the 2D physics would make it inconsistent with Bullet, creating confusion between the APIs. Also, changing 3D to match 2D would require additional methods to set and get the applied_force and applied_torque, while making 2D match 3D only requires this 2 line fix. It's much easier to just make the 2D physics implementation consistent with other physics engines.

[madmiraal]

First, "everyone else does it this way" is not a good reason. Many bad ideas perpetuate because of this reasoning.

Second, Godot is a game engine that uses physics not a physics engine. Therefore, the interface should make sense for making games not physics; regardless of whether or not it makes sense in a physics engine.

Third, move_and_slide() is another good example of a function that should not need to be called by the user every physics step. Not only are most of the parameters set once and never changed, most of the character controllers people create don't work, because of this interface. It would be a lot simpler to set the parameters once and make changes through event driven programming, while letting the engine do the heavy lifting.

Finally, applying the changes to 3D would not be that much harder, except they would need to be made to both Godot and Bullet. An easier solution is not necessarily a better solution.

The point is, whichever way we go, it's a breaking change; so now is the time to have the discussion to agree the best way forward.

[mini-glitch]

I wanted to bring up the spaceship thruster again because it demonstrates what I'm talking about regarding usage of the physics api. Let's say you have a little spaceship that continuously rotates at a constant rate. The player's only control is the spacebar, which when held causes the shapeship to accelerate in the direction it's facing. You might have a script like this:

extends RigidBody2D

export var thrust_force: float = 10.0
export var rotate_speed: float = 3.0

func _physics_process(delta: float) -> void:
    angular_velocity = rotate_speed
    
    if Input.is_action_pressed("thruster"):
        var direction = Vector2(cos(rotation), sin(rotation))
        add_central_force(direction * thrust_force)

Nice and simple. If applied_force is cleared after each physics step, this behaves correctly i.e. as a constant acceleration in the direction the ship is facing.

Now with the current version where applied_force aren't cleared, to accomplish the same thing you'd have to do something like this:

extends RigidBody2D

export var thrust_force: float = 500.0
export var rotate_speed: float = 3.0

var last_force: Vector2 = Vector2.ZERO

func _physics_process(delta: float) -> void:
    angular_velocity = rotate_speed
    
    if Input.is_action_pressed("thruster"):
        add_central_force(-last_force)
        last_force = Vector2(cos(rotation), sin(rotation)) * thrust_force
        add_central_force(last_force)
        
    if Input.is_action_just_released("thruster"):
        add_central_force(-last_force)
        last_force = Vector2.ZERO

Even if you handled the inputs in an event driven way thru _input() the nature of the ship's behavior requires that these changes to forces happen every physics step. This isn't a one off scenario either; the vast majority of physics related behavior I've done in games requires doing something every or nearly every physics step. This script would be even more complex if I want multiple thrusters that apply force at certain positions of the ship, because then I'd also have to manage the applied_torque as well. All this behavior ends up vastly overcomplicated because the current 2D physics are subtly but crucially wrong, and clearing applied_force and applied_torque fixes it.

[madmiraal]

@atinymelon If you want a one-off central force, you should use apply_central_impulse(impulse), where impulse = force * delta. If you want to apply or stop a constant force, you should set or clear the applied_force property.

[mini-glitch]

Having to modify applied_force directly would become a nightmare the moment you want to have multiple forces applied to the same object. I can't clear applied_force because other things may or may not be affecting it, so I have to subtract the force from the last physics step and apply a new one this physics step. But what do I do when I'm applying a force at a position? Calling add_force() will also add torque, but I don't know what that torque is and am thus unable to subtract the torque when I want to stop it. And again, clearing applied_force or applied_torque is not feasible because many things can be acting on it at once, so to undo a force or torque I need to keep track of it so I can subtract it later.

A simple example is the same spaceship, but instead of a single thruster applying force to the center, it has two thrusters on its left and right ends. The controls are such that activating one thruster turns the ship, and activating both moves it forward. But implementing this behavior is a massive pain currently because the thrusters don't just apply force, they also apply some unknown amount of torque.

[madmiraal]

Those that want to clear the forces at each iteration can do so by doing what this PR is suggesting i.e. by adding, to the beginning of their _physics_process() :

applied_force = Vector2();
applied_torque = 0.0f;

There is no need to force this on every user.

[mini-glitch]

Yes, this works for individual objects, but if I want all or most of my rigidbodies to have this behavior, it doesn't quite work.

Either:

1. Every single rigidbody needs this script just to clear the forces
   or
2. I have some manager that keeps track of all them and clears the forces on them

The problem with 1 is that now I have to have more scripts running than I should, which can impact performance. Additionally, what happens if one of these rigidbodies has its script run AFTER a script that pushes it? Suddenly that force gets cleared before it has a chance to be integrated in the physics engine, so things don't work. We don't have precise control over when scripts run, so this can and will happen.

In theory 2 may fix the script execution order problem if I can consistently have it run either before or after everything else, but it's also ridiculous that I'd have to do this.

There is no need to force this on every user.

This goes both ways. Aligning 3D physics with 2D like in #42850 would be forcing a change that introduces more burden for no real gain. It creates more work to produce the same behavior, and any theoretical benefit of a more 'event-based' approach is ultimately lost because most desired physics behavior has to be calculated each step anyways. Clearing 2D forces as this PR does is a change in behavior, yes, but it means these physics behaviors are much simpler because they require less boilerplate for the same thing.