Add directly constructed Nodes to rootNodes

[ryantrem]

This is in response to this forum post: https://forum.babylonjs.com/t/root-node-not-shown-in-inspector-but-transformnodes-are/50432

When a Node is created directly (not a subclass of Node), it does not get added to the rootNodes of the Scene. However, if you set the parent on that Node and then set it back to null, then it does get added to rootNodes. From what I can tell, this is just wrong behavior and is a bug. My proposed fix is to add an optional parameter to the Node constructor that controls whether it gets added to the rootNodes. This is the same pattern that already existed for the same purpose with TransformNode and it's derived classes (e.g. AbstratMesh). That said, I personally don't love this pattern because as far as I can tell the isPure idea is meant to be an implementation detail to deal with the subclassing, but is publicly exposed (I know there is already a lot of this across the API). An alternative would be to just add this to the Node constructor and not change any of the subclasses:

if (this.constructor === Node) {
    this._addToSceneRootNodes();
}

This would just be saying "if a Node is directly constructed, add it to the scene's rootNodes, otherwise it must be a subclass, and the subclass is responsible for deciding how to add it to the rootNodes." Thoughts @deltakosh or anyone else?

[bjsplat]

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

[bjsplat]

Snapshot stored with reference name:
refs/pull/15089/merge

Test environment:
https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15089/merge/index.html

To test a playground add it to the URL, for example:

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15089/merge/index.html#WGZLGJ#4600

Links to test babylon tools with this snapshot:

https://playground.babylonjs.com/?snapshot=refs/pull/15089/merge
https://sandbox.babylonjs.com/?snapshot=refs/pull/15089/merge
https://gui.babylonjs.com/?snapshot=refs/pull/15089/merge
https://nme.babylonjs.com/?snapshot=refs/pull/15089/merge

To test the snapshot in the playground with a playground ID add it after the snapshot query string:

https://playground.babylonjs.com/?snapshot=refs/pull/15089/merge#BCU1XR#0

[bjsplat]

Visualization tests for WebGPU (Experimental)
Important - these might fail sporadically. This is an optional test.

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15089/merge/testResults/webgpuplaywright/index.html

[bjsplat]

WebGL2 visualization test reporter:

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15089/merge/testResults/webgl2playwright/index.html

[RaananW]

This is in response to this forum post: https://forum.babylonjs.com/t/root-node-not-shown-in-inspector-but-transformnodes-are/50432

When a Node is created directly (not a subclass of Node), it does not get added to the rootNodes of the Scene. However, if you set the parent on that Node and then set it back to null, then it does get added to rootNodes. From what I can tell, this is just wrong behavior and is a bug. My proposed fix is to add an optional parameter to the Node constructor that controls whether it gets added to the rootNodes. This is the same pattern that already existed for the same purpose with TransformNode and it's derived classes (e.g. AbstratMesh). That said, I personally don't love this pattern because as far as I can tell the isPure idea is meant to be an implementation detail to deal with the subclassing, but is publicly exposed (I know there is already a lot of this across the API). An alternative would be to just add this to the Node constructor and not change any of the subclasses:

if (this.constructor === Node) {
    this._addToSceneRootNodes();
}

This would just be saying "if a Node is directly constructed, add it to the scene's rootNodes, otherwise it must be a subclass, and the subclass is responsible for deciding how to add it to the rootNodes." Thoughts @deltakosh or anyone else?

As you say, this exists extensively in the framework. I like this solution more than the this.constructor one, but both do solve the problem, so - IMO - whatever you feel is cleaner.

[deltakosh]

Both work for me.

[ryantrem]

Ok, it sounds like overall their is a slight preference for the isPure approach, so I'll just stick with that.