New in 5.6.0:
Uncaught (in promise) TypeError: r.metadata is null
… and the rest is my code. But it should not matter, right?
@Josip_Almasi Do you have a repro in a playground? I think I know what the problem is, but it would be good to verify against a repro.
I will also note that I have repro’d the same with 5.6.0, and it appears to happen when I dispose of the root mesh (meshes) of a gtlf scene. Other meshes can be disposed of just fine (meshes in my case).
I’ll see if I can get a playground set up for it momentarily.
here is a repro playground: https://playground.babylonjs.com/#1Y0CSS#1
Notes what is a problem, and also noted that some glbs do not have this issue on dispose of root mesh! Very interesting indeed.
Ahh, this is different than what I thought it was.
Will have a fix shortly.
yep, I figured that would be what could fix it. Good old null checks
Still odd that the line just above there doesn’t error out on line 807, yet by the time the mesh is disposed that metadata is missing. Not sure how that happens, but I’m not very familiar with Babylon code myself.
Before I found this thread, I was debugging this same issue. For reference, here is a minimal playground repro with a mesh that exhibits the problem.
I recognize that a fix is already out for review, but here’s a layman’s analysis from someone who is reading the code for the first time: In the repro, the mesh called spring has a child BouncerArmature, which has 3 children, two of which are called Bouncer. The first of these is
bTNFS) and the second is
bTN) as mentioned on this line. After that line, both nodes share the same
bTNFS is being disposed first, which clears its
metadata property. When
bTN is later disposed, the callback on line 808 triggers, which tries to delete
bTNFS.metadata.gltf.skinnedMesh and hits the
null pointer. Skipping this delete means that
bTN.metadata will also be set to
null. Then, if there happen to be no more references to
gltf, everything will be GC’d, and the delete was moot in the first place. If that’s the case, I’d consider removing this extra callback completely.
Assuming that the delete is important to preserve, then other possible fixes are to either:
onDisposeObservable callback on
bTNFS instead of
bTN, so that this delete statement executes before metadata is nulled out.
Change the callback to
delete bTN.metadata.gltf.skinnedMesh. That would be functionally the same, because of the copy of the
metadata link earlier.
gltf object in a closure-captured local and delete it directly inside onDisposeObservable, ignoring the node objects and metadata fields altogether.
Of these, I think 1 or 3 are the better choices. 1 because
bTNFS is the original “owner” of
metadata, or 3 because it eliminates the ownership concern completely.
Apologies if I’m way off base here. Just trying to help!
PR: More fix for glTF loader skinning code by bghgary · Pull Request #12538 · BabylonJS/Babylon.js (github.com)
Turns out that copy of metadata is not right to begin with. The
gltf.pointer metadata will also be lost. I changed it to remove the copy. We now have a link to the other mesh, so other metadata from loader extensions can be retrieved from there.