Improved RigidDynamicBody linear/angular damping override

[pouleyKetchoupp]

Changes discussed in #37316 (comment)

RigidBody3D and PhysicalBone3D damping is now non-negative.
Area3D can override RigidBody3D damping values, not the other way around.

It's already how it works in Bullet, so the way damping is implemented had to be changed only in Godot Physics 3D.

CC @AndreaCatania

Edit:
Updated to use a new properties linear_damp_mode and angular_damp_mode to set the way RigidDynamicBody and PhysicalBone (2D & 3D) use damping values.
It can now be Combine (default) to add to the default/areas, or Replace to override the value completely (current behavior).

Simple test project for 2D and 3D:
damping-override.zip

[AndreaCatania]

Looks perfect!

[madmiraal]

From comment in #37316:

if we decide areas can override body damping values and not the other way around.

Has this decision been made? Because this is a compact breaking decision; especially considering this change would also need to be made in 2D to make them consistent. The reasoning seems to be based on the way Bullet was implemented; so one could just as easily deem it a bug in the Bullet implementation. Personally, I think it's a bug in the way Bullet was implemented, and we should be fixing Bullet not changing Godot physics.

There is also an issue with the way this change will work when there are no areas, or, more specifically, only the default world area. The RigidBody's dampings are appended to the default world dampings. One is not overriding the other. So, if either damping is large, the damping will be large.

Slightly off topic, but testing this also highlighted another issue with the current Bullet implementation: The RigidBody damping appears to be overriding the default world damping, which is expected, but not when the damping is set to -1. When the RigidBody damping is -1, the damping appears to be clamped to 0 i.e. no damping, regardless of the default world damping setting.

[pouleyKetchoupp]

The reasoning behind this change is based on @AndreaCatania's comment in #37316 (comment):

For example:
I set body damp to 0.5 and I want that the area overrides it with 1 when the body is inside it.

With the above code this would not be possible because the area override is overridden by the body (since 0.5 so greater than 0).

This is a use case that seems pretty useful (custom damping on specific bodies & override in some areas) and it is not possible in the current implementation.

On the other hand, setting things the other way around (forcing a specific custom damping on the body whatever the area it's in) can be easily achieved by setting the right collision mask on the area itself.

Because of these scenarios, I think it's a good idea to make this change for 4.0. As you already mentioned, since Bullet and Godot Physics don't work the same way right now, any change to make things consistent will break compatibility in some way.

There is also an issue with the way this change will work when there are no areas, or, more specifically, only the default world area. The RigidBody's dampings are appended to the default world dampings. One is not overriding the other. So, if either damping is large, the damping will be large.

This is a good point. In order to handle this case, what about making it so the rigid body's damping overrides the default world damping rather than adding to it? And then we can apply the current process to combine the damping with other areas.

Also, I agree the same logic should be applied to 2D physics once we reach a consensus on what to do for 3D.

[madmiraal]

Please excuse me for paraphrasing to make a point:

custom damping on specific bodies & override in some areas ... can be easily achieved by setting the right collision mask on the area itself.

I suppose the real question is: why would a user use the RigidBody's damping settings and Areas? From a user's point of view, with the current (documented) approach, the RigidBdy's settings override not just the default world settings, but also the Areas' settings. If this is not wanted, then a user knows not to use the RigidBody's settings.

Drilling down even further: why does the RigidBody even have damping settings? Damping is an environment parameter not a body parameter. I assume it was to provide an easy way to specify custom damping on specific bodies without using areas and collision layers/masks, but it does raise these questions when more complex scenarios are required.

To answer the question:

what about making it so the rigid body's damping overrides the default world damping rather than adding to it?

Yes, if set, the body's setting should at least override the default world setting, otherwise it would never be used.

Finally, just to respond to:

any change to make things consistent will break compatibility in some way.

I suppose any bug fix could be classified as breaking compatibility. The question is, how long does a bug have to exist before fixing it counts as breaking compatibility? :-)

[pouleyKetchoupp]

About setting damping on rigid bodies:
As a generalist game engine, I think it makes sense to make a decision that allows a wider range of use cases for users and that should be the core of our discussion.

In practice, damping is not used just as an environment parameter. It's used a lot to simulate specific bodies' movements and tweak the lag when movements stop. That's why it's important to keep damping settings in the body itself, and allow different combinations to handle multiple use cases around that.

As for an example scenario where you need body & area settings:
Playable character with high damping to make controls responsive
NPCs with different damping for smoother start/stop animations (or whatever other purpose)
Icy area that lowers damping for both the player and NPCs

Specific damping settings can also help configuring the physics for pushable objects, or rotations using joints, so there can be many other examples. Because of that, unless there's a specific reason not to support it, I would go for a solution that doesn't limit users.

About breaking compatibility:
I don't think any bug fix should be classified as breaking compatibility. But in this case, the default physics engine has behaved in a certain way for a while. Users don't know what the intention is, and imo there's no obvious expected behavior. So yes the documentation says one thing, but most users have just set up their projects according to the current behavior. Given this situation, whatever we're going to change is going to break compatibility for some users (which is fine if we do it for 4.0).

[akien-mga]

So do we have a consensus here? From what I read @AndreaCatania seems in favor of the change, but I'm not sure @madmiraal is convinced yet.

Maybe @reduz can chime in too?

[madmiraal]

Although I understand the arguments for changing the behaviour i.e. allow Areas' damping settings to override the RigidBody settings, @akien-mga is right, I'm not convinced this is preferred to the current, documented behaviour, where RigidBody damping settings override Areas' settings, for the reasons discussed above.

More importantly, even if it is decided that Area damping settings should override RigidBody settings, this PR needs to be updated to allow RigidBody settings to at least override (instead of combine with) the default world Area settings.

Furthermore, if it is decided that Area damping settings should override RigidBody settings, this change should be applied to 2D too to make things consistent, which this PR currently doesn't include.

Personally, I still think it is Bullet physics that should be fixed to ensure RigidBody damping settings override Area and world settings as is the documented behaviour. It makes it easy for a user to set a specific RigidBody's damping, while Areas can be used to achieve any complex combination required.

[pouleyKetchoupp]

Once a decision is made, documentation can be changed (it's already part of this PR), this PR can be updated to make rigid body settings override the default world settings, and 2D Physics can be modified to keep things consistent.

The question we need to answer first is: do we want to change the design to allow users to set specific damping values on different rigid bodies, and override these values in some areas?

I'm advocating for making the change in design, because it would allow users to achieve more possible scenarios very easily, and afaik wouldn't prevent existing scenarios to be achieved just as easily.

[akien-mga]

@godotengine/physics Did we reach any consensus here in the end?

[pouleyKetchoupp]

@akien-mga We did, I just haven't found the time (yet) to make the changes we agreed on.

Here are the details for info.

• Bodies will still have priority over Area, but with 2 modes (Combine/Replace) and only positive values.

It will default to Combine with value 0, which is the same as the current default -1 value (it uses world/area values).

For non-default values:
Replace can be used to apply the previous behavior (area damping values are ignored).
Combine can be used to allow some new use cases (area and body values are added together).

• Compatibility will be kept by converting the old value into the two new properties (Combine with 0 if it's < 0, Replace otherwise).

• The same logic will be applied to 2D physics to keep things consistent.

• Documentation needs to be updated.

[madmiraal]

Bodies will still have priority over Area, but with 2 modes (Combine/Replace) and only positive values.
It will default to Combine with value 0, which is the same as the current default -1 value (it uses world/area values).

Bodies should have three Space Override modes. As with Area the default should be Disabled, which will have the same effect as the current default of -1.

For non-default values:
Replace can be used to apply the previous behavior (area damping values are ignored).
Combine can be used to allow some new use cases (area and body values are added together).

Replace will override everything including the project default values, i.e. the current effect when the values are not -1.
Combine will allow further processing through Areas and the project defaults depending on the Areas' settings.

[pouleyKetchoupp]

@madmiraal As we already discussed, adding an extra option for Disabled doesn't make sense. Combine with 0 does the exact same thing and it's less confusing for users. I want to move forward as planned.

[AndreaCatania]

👍