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

[3.x] Fix RigidDynamicBody gaining momentum with bounce #76216

Merged
merged 1 commit into from
Apr 25, 2023

Conversation

belzecue
Copy link

This PR implements the physics material bounce fix done for Godot 4 in #55313.

As explained in that PR: The bounce calculation now uses the previous frame's velocity, so it's consistent with the actual motion of the bodies involved and not the yet-to-be-applied forces. The bounce velocity is no longer higher than the falling velocity at the moment of impact, which was adding more and more energy over time.

This greatly reduces rigidbody settlement jitter caused by the body incorrectly gaining too much momentum upon each collision.

Comparison videos for 3.5.2-stable and 3.5.2-stable+fix: before fix; after fix. Same project used for both with no code or settings changed.

pouleyKetchoup flagged this as a potentially compat-breaking change:

This change can be considered compatibility breaking, because it changes the effect of bounce and might require users to modify their physics material or damping settings to keep the same behavior in some cases.

@belzecue belzecue requested a review from a team as a code owner April 18, 2023 17:58
@rburing rburing changed the title Fix RigidDynamicBody gaining momentum with bounce [3.x] Fix RigidDynamicBody gaining momentum with bounce Apr 18, 2023
Copy link
Member

@rburing rburing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say this is worth doing for 3.6, so I propose to merge without cherry-picking to 3.5 because it changes behavior.

The change in behavior (to fix a bug) can be mentioned in the 3.6 release notes.

@belzecue
Copy link
Author

belzecue commented Apr 18, 2023

Note that the Godot 4 PR removes two defines, which I have also removed for this 3.x PR:

servers/physics_2d/body_pair_2d_sw.cpp
#define POSITION_CORRECTION

servers/physics/body_pair_sw.cpp
#define RELAXATION_TIMESTEPS 3

I have checked and these statements are still redundant in the 3.x branch, so the cleanup is similarly valid.

The Godot 4 PR also removed this comment line in Godot 3.x equivalent file: servers/physics/body_pair_sw.cpp

//normal impule
c.bounce = c.bounce * dv.dot(c.normal);

Given that the comment line appears to apply to the following unremoved line, I left the comment in.

@belzecue
Copy link
Author

belzecue commented Apr 18, 2023

There's another consideration here.

When using default 3.5.2-stable physics settings, physics jitter still occurs frequently for 2D rigidbody piles, both with and without physics materials with fixed bounce. I've done a lot of testing with my test setup as demonstrated in the linked videos for this PR.

I recall seeing somewhere in my travels investigating this bounce issue, in the Godot 4 code, where pouleyKetchoup bumped up the defaults for certain physics settings. That change might also be appropriate to backport to 3.x. The settings I'm using in my test bed give very good results compared to the 3.5.2-stable defaults.

So I'm going to go look for that Godot 4 PR where I think I saw the bump in defaults and see if that's something suitable for 3.x.

FYI, here's the code I'm using in a global autoload to bump up certain physics defaults at startup for my test project:

https://gist.github.com/belzecue/8dfc97f476dec300571d6f5b61ce61c4

UPDATE: This was the Godot 4 PR I was thinking of: #55602 The changes to do with defaults are more minor than I thought, so nothing much to cherrypick there in regard to defaults. The overall PR is pretty impactful and complex in other ways, and probably worth a look to see if it too can be backported to 3.x in some form.

@akien-mga akien-mga added this to the 3.6 milestone Apr 18, 2023
@akien-mga akien-mga merged commit a2534ce into godotengine:3.x Apr 25, 2023
@akien-mga
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

3 participants