Remove throws from GraphNode which were unnecessarily interrupting execution #4350
Conversation
mvaligursky
requested review from
willeastcott,
GSterbrant,
slimbuck and
yaustar
| this._debugInsertChild(node); | ||
| // #endif | ||
|
|
||
| this._prepareInsertChild(node); |
There was a problem hiding this comment.
This function now does node._parent.removeChild(node); where it didn't before.
Might that change behaviour?
(Same for insertChild()).
There was a problem hiding this comment.
That's what is needed to continue with a valid graph. Better solution than throw.
src/scene/graph-node.js
Outdated
| if (node._parent) | ||
| node._parent.removeChild(node); |
There was a problem hiding this comment.
If the new behavior is that a node is unparented if it already has a parent, perhaps this should be mentioned in the API ref docs?
There was a problem hiding this comment.
But this triggers a console error, so it's not expected / new behaviour. It's just a mitigation for the app to reasonably continue instead of crash / create invalid graph.
There was a problem hiding this comment.
Any reason it can't just be documented behavior and not a mitigation?
There was a problem hiding this comment.
done, I agree it's a more expected behaviour.
Fixes #4228