Disposing of a light's parent leaves the light untouched

[Joakim]

Hi

I tried turning a mesh into the parent of a SpotLight. It worked fine! But when i dispose the parent mesh, the spot light still remains in the scene.

Searching the forums i found this thread: http://www.html5gamedevs.com/topic/11230-how-do-i-attach-a-light-to-a-mesh/

That thread holds this example: http://playground.babylonjs.com/#2AKHZK#2

Try to add the following lines of code to row 69...

lightSphere0.dispose();lightSphere1.dispose();lightSphere2.dispose();

...and you will find that the spheres are removed, but the lights remain.

Is this a bug or a feature? How do I dispose of the lights if I only know the parent mesh?

[Wingnut]

Hi Joakim, welcome to the forum.  I am somewhat sure that this is neither a bug nor a feature.  It is simply the way it works. 

In BabylonJS, parenting MESH simply means that the child inherits the position, rotation, and scale of it's parent.  But there IS a MESH disposal "feature" that allows you to recursively dispose of the MESH children.

mesh.dispose(doNotRecurse)  Check out https://github.com/BabylonJS/Babylon.js/blob/master/src/Mesh/babylon.abstractMesh.js#L861 

If NOT doNotRecurse... then recurse the dispose.

So we are in a DO RECURSE section.  See lines to 872-873, where the child dispose happens.

After you get done looking at that, let's look at light disposal.  https://github.com/BabylonJS/Babylon.js/blob/master/src/Lights/babylon.light.js#L66

In that disposer, no DoNotRecurse is available... but it COULD be added, I think.  But similar to mesh recursively disposing ONLY other mesh, light dispose would only dispose other lights.  Do you have a need for this?  I think it could be arranged... and this is an open source project, so you could add it yourself and do a GitHub PR.

You make a good point, actually.  Mesh sometimes have "personal lights"... lights used to illuminate THAT MESH and nothing else.  So when you dispose the mesh, you want its lights to dispose as well.  It would be a simple addition to mesh.dispose... happening between lines 875 and 876.

var lights = this.getScene().lights.slice(0);for (index = 0; index < lights.length; index++) {    if (lights[index].parent === this) {        lights[index].dispose();    }}

Basically a repeat of the !doNotRecurse MESH child-check, done instead on the scene.lights array.  *shrug*  But it is not quite as simple as I have made it appear.  Not only do we want to dispose of lights that are parented to the primary disposed mesh, but we would ALSO want to dispose lights that are parented to the disposed CHILD meshes.  So, BEFORE line 873 (before we dispose the child mesh), we want to check if that child has any lights parented to it... and dispose them before we dispose the child.  Then still do the child-lights check on the primary disposed mesh... after that... like mentioned above.

Seeing that this iteration must be called twice, instead, maybe build a BABYLON.Tools.DisposeChildLights(mesh) function.  A tool.

An interesting topic, Joakim!  I'm not a very good coder, nor am I a wise person in seeing "the big picture"... but... hmm.  Maybe we should listen for more comments and see what others have to say about it.  I kind-of like the Tools way of doing it.  I think this would be a rather handy tool to have.  Thanks for bringing this issue to "light".    I think it warrants further thought... and let's hear more thoughts from you, too.  Thanks!

[Joakim]

Thank you for the very detailed response!

I see now that even the Mesh.getChildren function does not return lights which have been connected to it.

Your suggested fix of checking for lights which have the current mesh as a parent seems like it would do the trick. But I don't agree with your following explaination. This snippet of code would be run by all child meshes as well, since it is recursive! Right? To me, this seems simpler than adding a function to the Tools.

I do like your solution, but I am not a fan of doing modifications to the framework at this point However, I tried overriding the BABYLON.AbstractMesh.prototype.dispose function and added your code. It works like a charm!

It might be worth having a discussion about this to see if something like this should be added to the framework.