Fix BoneAttachment3D signal connection #81695
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previous attempt PR (#75907) was never merged as the issue became a bit harder to repro, but it can still happen in normal editor use, including in repro project #69614 (comment) by switching tabs and reloading the project. The code snippet in #81552 can repro this 100% of the time.
The problem is that
bound
variable which is used to track if the BoneAttachment3D is attached to a Skeleton3D and connected to itsbone_pose_changed
signal is set immediately, but the connection is made using a deferred call. This leaves one frame of time where the state can get messed up in various ways.Looks like the deferred connection trick is a pretty rarely used pattern in Godot code base, after a quick search I could only find 4 similar uses, all in editor specific UI code. It was added in #51368 and replaced a more direct call
bind_child_node_to_bone()
. There might be a real reason why it creates the connection using a deferred call, but currently it's wrong because if BoneAttachment3D or any of its parent node is reparented or moved in a tree during the same frame the connection tracking (bound
variable) will end out of sync with the real connection state.I tested this with some models+MRP and couldn't notice any regressions, but skeleton related interactions can be pretty complex and subtle. Testing this more for example by importing and re-importing complex models which create BoneAttachment3Ds during import is recommended.