Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Entity#destroy #5414

Merged
merged 9 commits into from
Jun 22, 2023
2 changes: 1 addition & 1 deletion src/framework/components/model/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ class ModelComponent extends Component {
this._model._immutable = false;

this.removeModelFromLayers();
this.entity.removeChild(this._model.getGraph());
this._model.getGraph().destroy();
delete this._model._entity;

if (this._clonedModel) {
Expand Down
23 changes: 1 addition & 22 deletions src/framework/entity.js
Original file line number Diff line number Diff line change
Expand Up @@ -519,28 +519,7 @@ class Entity extends GraphNode {
this.c[name].system.removeComponent(this);
}

// Detach from parent
if (this._parent)
this._parent.removeChild(this);

// recursively destroy all children
const children = this._children;
while (children.length) {

// remove a child from the array and disconnect it from the parent
const child = children.pop();
child._parent = null;

if (child instanceof Entity) {
child.destroy();
}
}

// fire destroy event
this.fire('destroy', this);

// clear all events
this.off();
super.destroy();

// remove from entity index
if (this._guid) {
Expand Down
11 changes: 4 additions & 7 deletions src/framework/scene-registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -376,14 +376,11 @@ class SceneRegistry {
const app = this._app;

const onBeforeAddHierarchy = (sceneItem) => {
// Destroy/Remove all nodes on the app.root
const rootChildren = app.root.children;
while (rootChildren.length > 0) {
const child = rootChildren[0];
child.reparent(null);
child.destroy?.();
// Destroy all nodes on the app.root
const { children } = app.root;
while (children.length) {
children[0].destroy();
}

app.applySceneSettings(sceneItem.data.settings);
};

Expand Down
31 changes: 31 additions & 0 deletions src/scene/graph-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,37 @@ class GraphNode extends EventHandler {
return this;
}


/**
* Detach a GraphNode from the hierarchy and recursively destroy all children.
*
* @example
* const firstChild = this.entity.children[0];
* firstChild.destroy(); // delete child, all components and remove from hierarchy
*/
destroy() {
// Detach from parent
this._parent?.removeChild(this);

// Recursively destroy all children
const children = this._children;
while (children.length) {
// Remove last child from the array
const child = children.pop();
// Disconnect it from the parent: this is only an optimization step, to prevent calling
// GraphNode#removeChild which would try to refind it via this._children.indexOf (which
// will fail, because we just removed it).
child._parent = null;
child.destroy();
}

// fire destroy event
this.fire('destroy', this);

// clear all events
this.off();
}

/**
* Search the graph node and all of its descendants for the nodes that satisfy some search
* criteria.
Expand Down