Fix body leaving area gravity influence

[Occalepsus]

This pull request fixes and closes issue Area3D gravity problem in Godot 4 #77682

We solve this issue with @Wiltof by adding a check in the destructor of GodotAreaPair3D ; if the area has a gravity override then we remove the area from the body so it is not influenced by it anymore.

This solves the cases where we have a Rigidbody3D inside an Area3D with a gravity override mode, when the RigidBody3D is teleported outside the Area using its Tranform, or when the Area3D is disabled or deleted. The RigidBody3D is no longer subject to the Area3D gravity.

• Bugsquad edit, fixes: Area3D gravity problem in Godot 4 #77682
• Bugsquad edit, fixes: Area2D instancing causes unexpected effects on RigidBody2D #81550
• fixes: Area2D gravity affects bodies which are no longer in the area #77449

[Occalepsus]

I made modifications in the code to solve all space override cases. To do that, I created a new field in GodotAreaPairXD (with XD is 2D and 3D, i've modified both at the same time) which is a bool called body_sub, that corresponds to the state where the area is in GodotBodyXD::area. body_sub is changed each time the area is added or removed in the methods of GodotAreaPairXD.
At last, in GodotAreaPairXD::~GodotAreaPairXD(), if the area is in GodotBodyXD::area, that we know thanks to body_sub, then we remove it.

I've seen that this PR might be in conflict with #81434 and #82470, but this PR covers more case and is more efficient since the field has_space_override is not computed each frame, but only when needed.

[FeralBytes]

This is definitely needed I hope it gets merged soon, because the current behavior in Godot 4 is incorrect and even unpredictable at best.

[radishes]

Thank you for this fix. Would love to see this approved to go into production as soon as possible 🙏

[NolanDC]

Even though it's less efficient, it may actually actually makes sense to calculate has_space_override every frame so that that anyone working in this class can assume that all the variables are in their correct state after setup is called.

Also, totally a nitpick, but I wonder if the variable name could be a bit more descriptive. Maybe something along the lines of body_has_attached_area? I don't think I'd quite know what body_sub was referring to unless I dug into the code.

[Occalepsus]

Also, totally a nitpick, but I wonder if the variable name could be a bit more descriptive. Maybe something along the lines of body_has_attached_area? I don't think I'd quite know what body_sub was referring to unless I dug into the code.

Good point! Indeed I put that name to test something at first, and then totally forgot it. If you think it is really needed I can update the PR to refactor this variable name.

[NolanDC]

Sure! I think anything that improves clarity is always worth doing, especially if it's easy to change.

[radishes]

Looks like the requested updates are in. Are any other changes needed for the review?

[rthugh02]

Hoping to see this pushed soon, I believe the project I'm working on is affected by this issue. My setup:

• On mouse click, an Area2D and a RigidBody2D are spawned at mouse position.
• Area2D editor values are gravity_space_override = SPACE_OVERRIDE_REPLACE, gravity_point = true, gravity_point_center = Vector2(), gravity = 1370
• While mouse click remains held, Area2D.position is set to mouse position.
• On mouse click release, Area2D.queue_free() is called

My intention was for the RigidBody2D to no longer be affected by the gravity of the Area2D, but this is not the case. It appears that the last known gravity_point_center remains "in tact" so to speak. Video of this behavior:

gravity.still.active.mp4

[YuriSizov]

Looks sensible, good catch! Should this also be done to GodotAreaSoftBodyPair3D?

Also, could you amend the commit message so it doesn't include the issue reference? You can add it to the body, just not the title.

[rburing]

Thanks for this PR, I think it has the right spirit but not exactly the right solution. I think the role of the newly added variable body_has_attached_area should already be played by the existing has_space_override. The bug is that it gets set to false too often in GodotAreaPair2D::setup, GodotAreaPair3D::setup, and GodotAreaSoftBodyPair3D::setup.

Indeed, in the current master branch has_space_override can only get set to true on the tick where colliding changes from false to true or vice versa, and on the next tick (assuming colliding doesn't change again) it will get reset to false for no reason, which will eventually cause the bug of the destructor not doing the necessary cleanup.

I think the fix should be just this (in each of the three setup functions mentioned above):

-       has_space_override = false;
        if (result != colliding) {
+               has_space_override = false;

That is, it should just be the initial value in the branch where we're checking if the area overrides anything.

This also fixes the three linked issues in my testing.

[NolanDC]

@rburing Does that still leave some possibility that a programmatic change of the Area2D's override mode could result in the connection not being removed?

I might be missing something, but I think in theory:

• mode is set to COMBINE
• objects collide, area2 connection is added
• mode is programatically set to DISABLED
• objects stop colliding, but has_space_override is now false, so the body is not removed

This is why I closed my original solution here: https://github.com/godotengine/godot/pull/81434/files, since it would have a similar issue.

[rburing]

@NolanDC Good point, this PR covers that case while my suggested alternative doesn't.

So as #82961 (review) suggests the commit message should be updated to remove the issue number from the first line and the same approach should be applied to GodotAreaSoftBodyPair3D, and then this is good to go.

[Occalepsus]

Oh thanks for all of your messages, I'll look to make the changes in GodotAreaSoftBodyPair3D and GodotAreaSoftBodyPair2D (if that exists).

@rburing right now I don't remember why, but I think there was something that made me create this new field, i'll see.
I'll make these changes and also change the commit name then.

[Occalepsus]

@rburing i just tried your fix and I agree with NolanNC, the fix does not cover the case when the body is in the area while it gets deleted or disabled. I still think both has_space_override and body_has_attached_area are two separated variables with their own use.
In my commit amend i've changed the commit message and added the fix in GodotAreaSoftBodyPair3D.
Don't hesitate to tell me if there is anything!

[rburing]

Thanks! LGTM.

[YuriSizov]

Thanks! And congrats on your first merged Godot contribution 🎉

[radishes]

Thanks everyone, this is an important fix for me and many others.

[graehu]

Hi, I've been writing a quick fix for something similar and just want to confirm if this will also fix the issue. I don't teleport my object, but I do set it to be kinematic / frozen and move it. Then when I unfreeze it in a new location, it's still got areas associated with it.

My fix was to write a script function that clears areas, and do that when I set the body kinematic.

https://github.com/graehu/godot/tree/body_areas_fix

If this fixes that I can remove my branch. 🙂

Edit: Tested myself, this fixes the issue! thanks!

[YuriSizov]

Cherry-picked for 4.2.2.

[YuriSizov]

Cherry-picked for 4.1.4.

[ericvennemeyer]

Hi everyone, I'm a fairly new Godot user and just upgraded to 4.2.1. I am currently experiencing this exact problem and it's game breaking for me. It looks like this fix may not have made it into an official release yet. If that's the case, how do I go about testing it out for myself the way others on this thread seem to have?

Thanks for helping out a noob :)

[FeralBytes]

@ericvennemeyer the fix is not in 4.2.1. It should be in 4.2.2, 4.1.4, and 4.3, when they are released. You should try one of the latest development releases to get the fix.

[ericvennemeyer]

@FeralBytes will do. Thanks for the quick response!

Edit: 4.1.4 dev release fixed it for me. What a relief! Thanks again