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

Fix Joint2D/Joint3D node path reset on scene switch #46578

Merged

Conversation

pouleyKetchoupp
Copy link
Contributor

When one of the bodies exited the tree, the corresponding node path was reset instead of just resetting the joint from the physics server. That was causing the node path to be reset on scene switch when one of the bodies is under the joint in the scene tree.

Regression from #44703
Fixes #46571
Tested with case from #44510, still fixed.

@akien-mga
Copy link
Member

akien-mga commented Mar 2, 2021

The change seems to invalidate most of the rest of what was added in #44703 (connecting signals, then adding code in several places to disconnect the signals). If the proposed change is sufficient, then the previously added code could likely be removed too and replaced by connecting the body's tree_exited signal directly to update_joints(true), no?

When one of the bodies exited the tree, the corresponding node path was
reset instead of just resetting the joint from the physics server. That
was causing the node path to be reset on scene switch when one of the
bodies is under the joint in the scene tree.
@pouleyKetchoupp pouleyKetchoupp force-pushed the fix-joint-remove-body-regression branch from 09911a5 to 2dc5ff0 Compare March 2, 2021 15:25
@pouleyKetchoupp
Copy link
Contributor Author

@akien-mga Let me know if I'm missing something, but it seems still safer to disconnect signals from both bodies when one of them exits the tree to avoid any side effect.

So I would rather keep _body_exit_tree which disconnects signals and disables the joint, but I've just updated the PR to remove the body id that was passed as an argument since it's not used anymore.

@akien-mga akien-mga merged commit d0e6251 into godotengine:master Mar 3, 2021
@akien-mga
Copy link
Member

Thanks!

@pouleyKetchoupp pouleyKetchoupp deleted the fix-joint-remove-body-regression branch March 3, 2021 16:00
@madmiraal madmiraal mentioned this pull request Mar 6, 2021
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.

Joint2D bug
2 participants