Godot version:
3.1, 9eb4d4ab2
Issue description:
The function void apply_impulse( Vector3 position, Vector3 impulse ) is not intuitive to use.
What we would expect is :
So let's say we have a rigid body and we want to apply an impulse at an offset of 2 on the Y axis of the rigid body (i.e the impulse point is at (0,2,0) in local coordinates) with a force along the X axis of the rigid body (i.e the force vector is for eg (1,0,0) in local coordinates).
We should be able to do :
apply_impulse( to_global(Vector3(0,2,0)) , to_global(Vector(1,0,0) )
Or equivalently :
apply_impulse( to_global(Vector3(0,2,0)), transform.basis.x )
But this is not correct, instead we currently have to do :
apply_impulse(transform.basis.xform(Vector3(0,2,0)), transform.basis.x)
For e.g in Unity AddForceAtPosition seems much clearer and easier to understand : Force and Position are in global coordinates and that's it.
Same in Unreal Engine, Same in Lumberyard, etc...
This happens because to_global(pos) is equivalent to transform.xform(pos) but apply_impulse() seems to expect transform.basis.xform(pos) instead.
It's because it uses local position but world rotation
@raymoo Indeed, but do you find it intuitive to have a position that is neither a World Position nor a Local Position ? I find " local position but world rotation" un-intuitive when we could just have a world position.
It's intuitive in the case that the impulse comes from an external source. In that case you don't want the rotation to affect the direction of the force
Hum, here is what I understand :
The direction of the force (impulse) is a Vector3 in world space. Here no problem, we are aligned with other engine. So if you don't want the rotation of the rigid body to influence the direction of the force you pass for eg Vector(0,1,0) (and it will be attracted to the sky whatever rotation the rigid body has). On the contrary, if you have a rocket, then you pass to_global(Vector(0,1,0)) (or transform.axis.y) and it will push the rocket forward (the force depends on the rotation of the rigid body)
Now the position where the impulse is applied (position) is actually not a position. And this is where we are not aligned with other engines. In other engines it is just a position in World Space, in Godot this is actually "a world space offset from the rigidbody's origin".
So on a drawing :

For eg in Unreal Engine AddForceAtLocation will take the force direction in World Space (like in Godot) and the impulse position in World Space (in Pink on the drawing). Then AddForceAtLocationLocal will take the force direction in Local Space and the impulse position in Local Space (in Yellow on the drawing), the equivalent in Godot would be #20031.
But for apply_impulse, in Godot we don't take the Pink point, but the Blue one.
Maybe I completely misunderstood, but in case I got this right, then this Blue point does not correspond to the position of anything in any space, but corresponds to the offset between the rigidbody center and the impulse position.
In that case I think we should :
apply_impulse ( Vector3 position, Vector3 impulse ) to apply_impulse ( Vector3 offset, Vector3 impulse ) and change the doc description from : _Both the impulse and the position are in global coordinates, and the position is relative to the object鈥檚 origin._
To maybe something like :
_impulse is a (global coordinate) vector representing the force applied. offset is the (global coordinate) vector between the RigidBody center and the impulse position._
Related: https://github.com/godotengine/godot/issues/16863#issuecomment-416791048
We may have something like apply_impulse_local being local and apply_impulse being global. This is how many methods work, by adding _local. Or, we could add a _global suffix if the local one is mord common.
For apply_impulse_local there wouldn't be any confusion as the offset from the rigidbody's center in Local Coor is the same as the Local Coor position (both are the Yellow point on the drawing). So you could name the function apply_impulse_local(position, impulse) or apply_impulse_local(offset, impulse) both names would be correct.
The problem is for apply_impulse as the offset from the rigidbody's center in Global Coor (Blue point) is not the same as from the Global Coor position (Pink point). That's why here I think we should either rename position to offset or actually use the global position like all other engines.
If we choose to use the global position like the other engines, it would be a good idea to do it at the same time as #16863 as this would also break compat
Why is offset the first argument in apply_impulse and apply_force? Isn't the magnitude (currently 2nd argument) practically always supplied, but offset is only sometimes supplied? Wouldn't it make more sense to make the offset and optional argument where the default is vector.zero?
@Oranjoose https://github.com/godotengine/godot/issues/16863#issuecomment-416788645
Most helpful comment
Related: https://github.com/godotengine/godot/issues/16863#issuecomment-416791048
We may have something like
apply_impulse_localbeing local andapply_impulsebeing global. This is how many methods work, by adding_local. Or, we could add a_globalsuffix if the local one is mord common.