detachFromBone breaks parent-child relationships

repro: https://playground.babylonjs.com/#11BH6Z#671

I’m surprised at not seeing any report on this. Not really sure if its wai or my understanding is borked. The typical use case is for equipping a character with multiple meshes (weapons, items etc) and allow the character to switch between different objects in hand on the fly. sphere is attached to a particular bone, animation runs, sphere is then detached from bone but should still remain parented to dude. For item switch, its a sequence of multiple attaches/detaches.

Adding @Evgeni_Popov

Dude has no child named “sphere1”, so let sphere2 = dude.getChildMeshes(true).find(child => child.name === "sphere1") returns undefined and sphere2.attachToBone(skeleton.bones[34], dude) fails.

If you do sphere.attachToBone(skeleton.bones[34], dude) instead if works.

This is entirely my fault, it was late and I didn’t sanitise/explain the PG properly.
Repro again: https://playground.babylonjs.com/#11BH6Z#673

The problem here is that detachFromBone should not cause my sphere to lose its parent, ie, dude. console will throw undefined for the 2nd call. wai ?

When you attach a mesh to a bone, the parent of this mesh is updated so that it is now the bone you attach it to. When you detach the mesh from the bone, its parent is reset to null. We could probably save the current parent when you call attachToBone and set it back when you call detachFromBone (with a flag to stay backward compatible) but I’m not sure it’s a general enough use case and you can simply set the parent to the node you want after you have called detachFromBone. What do you think @Deltakosh?

Yes, I can do that. But I think this is worthy of discussion as I have no idea what the general use case is as it is. If parenting has no impact on attachToBone, I’d expect it not to have an impact either when detachFromBone runs. User multi triggering an animation (eg:item switch) means additional calls to finding and reparenting meshes, a minor hassle for a single mesh. But a nightmare when multiple agents are animating. Adding an option to detachFromBone to prevent parenting resets is another alternative.

I love the alternative option :slight_smile:

Here’s the PR:

2 Likes

I found this but I guess I was late :frowning: Repro: https://playground.babylonjs.com/#NRNBMM#169
detachFromBone used to null a parent in a way outside of its intended func.

That won’t change with the PR, we don’t check you did call attachToBone before calling detachFromBone, you are responsible for the proper order of operations (it’s a bug on the user side to call detachFromBone without having called attachToBone first).

1 Like

Tested, fix works and marking as solved. Thanks ! :slight_smile:

2 Likes