Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve and harmonize API for forces in RigidDynamicBody2D and RigidDynamicBody3D #3601

Closed
pouleyKetchoupp opened this issue Nov 26, 2021 · 17 comments · Fixed by godotengine/godot#55736
Milestone

Comments

@pouleyKetchoupp
Copy link

pouleyKetchoupp commented Nov 26, 2021

Describe the project you are working on

Godot Engine

Describe the problem or limitation you are having in your project

Following up from godotengine/godot#42850 (comment).

The current problem is that add_force and equivalent functions have a different meaning in 2D and 3D:
In 2D, they add constant forces that keep being applied over time.
In 3D, they add one-shot forces so you need to call them every frame.

On the other hand, apply_impulse and equivalent functions are always applied only once in 2D and 3D.

In the linked PR, the proposed solution is to modify the 3D implementation to do the same thing as in 2D, so forces are always applied over time whereas impulses are one-shot.

This generally makes sense from a physical point of view, and makes some simple cases very easy, like setting a constant thruster force on and off.

On the other hand, it can make it cumbersome in some use cases:
-Applying one-shot forces wouldn't be possible directly anymore, it would require using impulses with a * delta factor which can be difficult to understand for users not familiar with physics systems.
-When multiple forces need to be set on and off, it would require to clear all forces and re-apply the desired ones.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

This proposal aims at finding a compromise for an API that allows these use cases to be doable and easy for beginners.

The general idea is to propose two sets of functions for forces and torques:
-apply_force and variants can be used for one-shot forces, like the equivalent apply_impulse functions
-add_constant_force and variants can be used for forces that keep being applied over time

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Here's the full API for forces, torques and impulses for 2D (the equivalent will be done for 3D):

Functions:

void apply_central_impulse ( Vector2 impulse )
Applies a one-time impulse at the body's center of mass. It's time independent and meant to be used for one-time events.
void apply_impulse ( Vector2 impulse, Vector2 offset )
Applies a one-time impulse at the given local position. It's time independent and meant to be used for one-time events.
void apply_torque_impulse ( float torque )
Applies a one-time torque impulse. It's time independent and meant to be used for one-time events.

void apply_central_force ( Vector2 force )
Adds a one-time force at the body's center of mass. It's time dependent and meant to be applied every frame.
void apply_force ( Vector2 force, Vector2 offset )
Adds a one-time force at the given local position. It's time dependent and meant to be applied every frame.
void apply_torque ( float torque )
Adds a one-time torque. It's time dependent and meant to be applied every frame.

void add_constant_central_force ( Vector2 force )
Adds a constant force at the body's center of mass that keeps being applied over time.
void add_constant_force ( Vector2 force, Vector2 offset )
Adds a constant force at the given local position that keeps being applied over time.
void add_constant_torque ( float torque )
Adds a constant torque that keeps being applied over time.

Properties:

Vector2 constant_force
The body's total applied constant force. Can be used to clear all constant forces with constant_force = Vector2().
(in 2D, this was previously named applied_force but this new name seems less ambiguous given the API)

float constant_torque
The body's total applied constant torque. Can be used to clear all constant torques with constant_torque = 0.0.
(in 2D, this was previously named applied_torque but this new name seems less ambiguous given the API)

If this enhancement will not be used often, can it be worked around with a few lines of script?

The harmonization between 2D and 3D has to take place in the core engine.

Some parts of the API could be worked around by script, but they would involve some knowledge in general physics properties, so a more complete API is needed for extended usability.

Is there a reason why this should be core and not an add-on in the asset library?

This is core because it changes basic functionalities of core nodes.

@aaronfranke
Copy link
Member

aaronfranke commented Nov 26, 2021

void apply_force ( Vector2 offset, Vector2 force )
Adds a one-time force at the given local position. It's time dependent and meant to be applied every frame.

This has been switched in 4.0 for consistency so that the force/impulse value comes first: godotengine/godot#37350

void add_constant_force ( Vector2 offset, Vector2 force )
Adds a constant force at the given local position that keeps being applied over time.

Vector2 constant_force
The body's total applied constant force. Can be used to clear all constant forces with constant_force = Vector2().
(in 2D, this was previously named applied_force but this new name seems less ambiguous given the API)

Why have add_constant_force but no remove_constant_force or clear_constant_force or similar? Why not just have users do constant_force += force?

@pouleyKetchoupp
Copy link
Author

This has been switched in 4.0 for consistency so that the force/impulse value comes first: godotengine/godot#37350

Thanks, I've just edited the proposal to take this change into account.

Why have add_constant_force but no remove_constant_force or clear_constant_force or similar? Why not just have users do constant_force += force?

constant_force += force wouldn't allow applying forces with an offset, and in general it seems clearer for scripting to propose a similar API between one-shot forces and constant forces.

The property is still useful for extra cases, like checking the total applied (including torque created by a force with offset) and clearing forces.

I was thinking about adding clear_constant_force, but it seems a bit redundant since there's already set_constant_force and constant_force = Vector2() from the property.

@mini-glitch
Copy link

I think this is a good compromise. I’m assuming this means the 2D physics engine will need to clear the internal applied_force and applied_torque body properties like 3D physics, since this is necessary for apply_force to be consistent?

@pouleyKetchoupp
Copy link
Author

I’m assuming this means the 2D physics engine will need to clear the internal applied_force and applied_torque body properties like 3D physics, since this is necessary for apply_force to be consistent?

Yes, indeed. 2D and 3D will need to handle both forces that get cleared and constant forces that don't.

@madmiraal
Copy link

I'm not convinced that the arguments:

-Applying one-shot forces wouldn't be possible directly anymore, it would require using impulses with a * delta factor which can be difficult to understand for users not familiar with physics systems.
-When multiple forces need to be set on and off, it would require to clear all forces and re-apply the desired ones.

justify adding a third set of methods:

void add_constant_central_force ( Vector2 force )
void add_constant_force ( Vector2 force, Vector2 offset )
void add_constant_torque ( float torque )

Addressing each argument individually:

Applying one-shot forces wouldn't be possible directly anymore, it would require using impulses with a * delta factor which can be difficult to understand for users not familiar with physics systems.

A one-shot force is an impulse, which can be applied directly using apply_impulse(). I think requiring the use of a * delta time factor (which is standard physics i.e. impulse = force x time) is far less difficult to understand than trying to explain why we have three sets of methods all basically doing the same thing.

When multiple forces need to be set on and off, it would require to clear all forces and re-apply the desired ones.

With godotengine/godot#42850 this would simply require a single line of code: applied_force = Vector3(). The user is already "re-apply the desired ones." Again, I think this is a lot less difficult to understand than trying to explain when to use add_constant_force() and when to use add_force(); especially when we also have apply_impulse().

Note: This is how it works in 2D at the moment! godotengine/godot#42850 is simply making 3D the same as 2D. It's just making them consistent. It's not introducing anything new.

As per the best practices, we need one solution for each problem. We should not have multiple methods doing the same thing.

@mini-glitch
Copy link

With godotengine/godot#42850 this would simply require a single line of code: applied_force = Vector3(). The user is already "re-apply the desired ones." Again, I think this is a lot less difficult to understand than trying to explain when to use add_constant_force() and when to use add_force(); especially when we also have apply_impulse().

Why should I have to manually clear out the force each frame to apply this force? Going back to the thruster example: why is

func _physics_process():
    applied_force = Vector3()
    if forward pressed:
        add_force(-global_transform.basis.z * 10)

better than just

 func _physics_process():
    if forward pressed:
        add_force(-global_transform.basis.z * 10)

The vast majority of the time someone would want to apply a continuous force, it's dependent on an ever changing direction (ship direction, direction from object a to object b, etc). It doesn't make any sense to make the default behavior apply the force in single unchanging direction forever. It's just adding an extra step to the behavior for no reason.

While you can achieve this behavior using apply_impulse instead of add_force, that's not what apply_impulse is for. It's meant as a one-shot time-independent force. It's used outside _physics_process() for single big impulses, not continuous force applied over time like add_force.

The other problem with requiring a user to manually clear applied_force is when a single body is being acted upon by other nodes. How do we ensure that the node's script that clears applied_force is always run consistently before or after every other script that would act on it? This is what the physics engine should be doing by clearing applied_force at the end of each physics step.

@aaronfranke
Copy link
Member

aaronfranke commented Nov 27, 2021

@atinymelon I apologize if I'm missing something, but what exactly is the problem with using apply_impulse and multiplying the impulse value by delta time? Doesn't that accomplish the goal of a one-time application of a continuous force? Still, I guess if that case is very common it makes sense to have a method for it.

@mini-glitch
Copy link

@aaronfranke It's mostly a naming issue. Calling apply_impulse each frame feels wrong because it's creating a continuous force over time, not a single big impulse of force.

@madmiraal
Copy link

Why should I have to manually clear out the force each frame to apply this force? Going back to the thruster example: why is

func _physics_process():
   applied_force = Vector3()
   if forward pressed:
       add_force(-global_transform.basis.z * 10)

better than just

func _physics_process():
   if forward pressed:
       add_force(-global_transform.basis.z * 10)

With godotengine/godot#42850 you can also do:

 func _physics_process(_delta):
    if forward pressed:
        applied_force = -global_transform.basis.z * 10

or

 func _physics_process(_delta):
    if forward pressed:
        set_applied_force(-global_transform.basis.z * 10)

or

 func _physics_process(delta):
    if forward pressed:
        apply_impulse(-global_transform.basis.z * 10 * delta)

They're equivalent and the choice is up to the user and depends on the circumstances.

@mini-glitch
Copy link

With godotengine/godot#42850 you can also do:

code

or

code

or

code

They're equivalent and the choice is up to the user and depends on the circumstances.

None of these really work for a situation where other nodes are applying forces to a body, because the affected body needs to ensure its applied_force and applied_torque are cleared before or after every other node that might effect it has run that frame. This creates extra work to ensure these nodes have correct process_priority relative to one another, when the physics engine should be handling this automatically.

For reference, the reason many physics engines accumulate forces into a temp field like applied_forces that gets cleared each step is because it allows you to have a more accurate and more stable simulation. This is done via Semi-implicit Euler integration, which better accounts for acceleration changing in the time between physics steps. You can read explanations on the benefits of this method here, here (page 7), and here (page 31).

This is one major reason why using apply_impulse each step with dt isn't sufficient. apply_impulse modifies the velocity directly, rather than the force being integrated separately. Applying continuous forces this way is innacurate, because it's being integrated via Explicit Euler integration rather than Semi-implicit Euler integration

@madmiraal
Copy link

Whether or not using apply_impulse() uses explicit or semi-implict integration (and one could argue that it's changing the velocity first so it's using semi-implicit integration) I think what's important is whether or not this will cause a problem for users that needs to be solved.

As per the best practices, there needs to be a problem before providing a solution. Do you have a practical example that demonstrates the problem with using apply_impulse() vs the current add_force()? I've previously provided demo projects showing that they're practically the same.

@mini-glitch
Copy link

Well the actual problem we're trying to solve is that 2D and 3D physics are inconsistent in their behavior. I'm arguing that clearing the 2D forces as in godotengine/godot#38648 is the better option than not clearing the 3D forces as in godotengine/godot#42850, because clearing the forces results in a simpler and more intuitive API. What practical benefit is there to not clearing the forces and requiring the user to manually clear it themselves?

@madmiraal
Copy link

What practical benefit is there to not clearing the forces

As stated previously: This is the expected behaviour when using standard event driven programming. For example, enabling a thruster when a key is pressed and disabling it when it is released.

and requiring the user to manually clear it themselves?

This is my previous comment: If the user uses the apply_impulse() methods, or sets the applied_force property, then they don't have to clear it themselves.

Well the actual problem we're trying to solve is that 2D and 3D physics are inconsistent in their behavior.

godotengine/godot#42850 is the better option, because, as stated in the OP:

Currently, in 3D, the only difference between add_central_force(), add_force(), add_torque(), and apply_central_impulse(), apply_impulse() and apply_torque_impulse() respectively is the unit used; the factor of delta:

    add_central_force(force, offset) = apply_central_impulse(force * delta, offset)
    add_force(force) = add_impulse(force_vector * delta)
    add_torque(force) = apply_torque_impulse(force * delta)

godotengine/godot#42850 makes 3D physics consistent with 2D physics and the documentation. It ensures that add_central_force(), add_force(), add_torque() correctly add a constant force and/or torque to the RigidDynamicBody3D. It also adds the missing properties applied_force and applied_torque to RigidDynamicBody3D, which are available in RigidDynamicBody2D.

@pouleyKetchoupp
Copy link
Author

Just to reiterate on the two problems we're trying to solve here:

  1. Harmonize the API between 2D and 3D

  2. Make the API easy to use, especially for users not familiar with physics

The second problem is about usability and goes a little bit beyond mathematical or physical justifications.

Applying an impulse and applying a force is not equivalent, and the API should reflect that to be as clear as possible.

The proposed solution here is to use 3 sets of functions to make it perfectly clear:
-Apply impulse for one-shot immediate change of velocity
-Apply force for one frame application of a continuous force
This one is important because it makes it clear what it's used for in the documentation, and the engine takes care of applying the correct delta at the correct moment in the physics simulation.
-Add constant force for ease-of-use application of continuous forces

I'm interested in having a discussion about alternative solutions to this second problem, but it doesn't make a lot of sense to try and force the discussion into going back to the state of a PR that had a majority of votes against it.

@madmiraal
Copy link

The way I see it, there are four options:

Description Pros Cons
1 Make 2D the same as 3D
godotengine/godot#38648
  • add_force() methods work like a physics engine (cleared after each iteration, the same as apply_impulse() methods)
  • 2D loses event driven functionality
  • 2D loses applied_force property
2 Make 3D the same as 2D
godotengine/godot#42850
  • 3D gets event driven functionality
  • 3D gets applied_force property
  • Algorithmic differences between current 3D add_force() methods and apply_impulse(delta) methods
  • Some users don't expect add_force() methods to be accumulative across iterations
3 Make 2D the same as 3D and add 3 add_constant_force() methods
This proposal (#3601)
  • 3D gets event driven functionality
  • add_force() methods work like a physics engine (cleared after each iteration, the same as apply_impulse() methods)
  • 2D loses applied_force property
  • Confusion with difference between add_constant_force() and add_force() methods
  • Confusion with similarity between add_force() and apply_impulse(delta) methods
4 Make 3D the same as 2D and add 3 add_one_shot_force() methods
Variant of this proposal
  • 3D gets event driven functionality
  • add_one_shot_force() methods work like a physics engine (cleared after each iteration, the same as apply_impulse() methods)
  • 2D loses applied_force property
  • Confusion with similarity between add_one_shot_force() and apply_impulse(delta) methods

The option selected should be the best option for Godot users. In my opinion this is option 2: Make 3D the same as 2D. It provides the simplest API with all the functionality.

@pouleyKetchoupp
Copy link
Author

I've opened godotengine/godot#55736 to implement the solution from this proposal that appears to gather the most consensus. This PR also improves the documentation to alleviate any possible confusion with the different methods.

@madmiraal
Copy link

I'm not convinced there is a clear understanding of what is meant by a consensus.

con·sen·sus (kən-sĕn′səs) n.

  1. An opinion or position reached by a group as a whole: "Among political women ... there is a clear consensus about the problems women candidates have traditionally faced" (Wendy Kaminer). See Usage Note at redundancy.
  2. General agreement or accord: government by consensus.

It is definitely not about finding the most popular option.

My understanding is that it's about debating the pros and cons until there is no more sustained opposition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants