node.getDescendants(), Find by name Inefficiency

[Srisub]

I am busy porting over an app from THREE.js over to Babylon.js, but am finding some surprising inefficiency and holes, that can easily be addressed i believe..

1. In THREE.js, i was able to simple say: item.findNodeByName( "Foo" ) - and it would do the right thing whether it was a scene or a node. I saw the latest code for getDescendants(). Thanks for adding the directChildrenOnly flag and the predicate, which i used in this case. I wrote a wrapper to mimic THREE, by doing scene.findMeshByName or node.getDescendants(false, predicate-check-for-name)

2. Secondly the code for getMeshByName, as well as getDescendants does a full-scan on ALL objects in the scene. Is that efficient. Wouldnt a recursive traversal of a children field in a node be necessary for efficiency?

 

[GameMonetize]

Hello!

1. We support scene.getNodeByName. Did you see it?

2. The full scan is required because a node does NOT have a list of children. We could think about adding it but then we need to handle .parent with a property in order to maintain the children list. I'm not opposed to it (just need to handle correctly dispose on children). I'll give a try to improve this

[GameMonetize]

Done: https://github.com/BabylonJS/Babylon.js/commit/a42d7f62df8f0b5b0bd24779e0e1be419c30b7ae

 

Thank you for the idea

[Srisub]

You rock, Deltakosh! I just switched to Babylon from THREE just a few days ago, and am already a fan. Hope to contribute soon..

[Kesshi]

6 hours ago, Deltakosh said:

Done: https://github.com/BabylonJS/Babylon.js/commit/a42d7f62df8f0b5b0bd24779e0e1be419c30b7ae

 

Thank you for the idea

Thank you, this is exactly what i need. I can remove my own children caching from my code now.
Actually i was thinking to do a PR already for this but you are faster :-D

[Kesshi]

@Deltakosh  Maybe you could use the new descandents handling to speed up AbstractMesh.dispose() because it also scans the full scene for child meshes.

here: https://github.com/BabylonJS/Babylon.js/blob/master/src/Mesh/babylon.abstractMesh.ts#L1115
and here:  https://github.com/BabylonJS/Babylon.js/blob/master/src/Mesh/babylon.abstractMesh.ts#L1122
 

[GameMonetize]

Correct! I'll fix it tonight

[GameMonetize]

Done

[mwpowellhtx]

So... until this change goes into production, I need to maintain instances of my "children"? It seems that JavaScript is garbage collecting them otherwise, but I could be wrong.

Do you set m.parent = s, where m is my mesh, and s is my scene? If so, that doesn't work, or I don't have the most up to date package.

[GameMonetize]

There is no need to attach a mesh to a scene. It is already done automatically.

you can use Babylonjs 2.4 right now to get the update