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
25 changes: 2 additions & 23 deletions src/framework/entity.js
Original file line number Diff line number Diff line change
Expand Up @@ -514,34 +514,13 @@ class Entity extends GraphNode {
this.c[name].enabled = false;
}

super.destroy();
kungfooman marked this conversation as resolved.
Show resolved Hide resolved

// Remove all components
for (const name in this.c) {
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();

// remove from entity index
if (this._guid) {
delete this._app._entityIndex[this._guid];
Expand Down
16 changes: 9 additions & 7 deletions src/framework/scene-registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -376,14 +376,16 @@ 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();
}

Debug.assert(
kungfooman marked this conversation as resolved.
Show resolved Hide resolved
Object.keys(app._entityIndex).length === 0,
"Destroyed all entities but these entities survived:",
app._entityIndex
);
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 it from the hierarchy. Then recursively destroy all ancestors.
kungfooman marked this conversation as resolved.
Show resolved Hide resolved
*
* @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 optimisation step, to prevent calling
kungfooman marked this conversation as resolved.
Show resolved Hide resolved
// 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