Skip to content

Commit 7217666

Browse files
committed
fix(hooks): trigger on_remove hooks when deleting entities
Previously, deleting an entity via world.delete() did not trigger on_remove callbacks for multi-component hooks. This fix ensures lifecycle hooks are properly fired during entity destruction by collecting removed components and calling triggerLifecycleHooks after the entity is removed from its archetype.
1 parent 48eef2d commit 7217666

2 files changed

Lines changed: 78 additions & 8 deletions

File tree

src/__tests__/world-multi-component-hooks.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,69 @@ import { component, relation, type EntityId } from "../core/entity";
33
import { World } from "../core/world";
44

55
describe("World - Multi-Component Hooks", () => {
6+
it("should trigger init, set, remove events correctly when using array syntax with single element", () => {
7+
const world = new World();
8+
const A = component<number>();
9+
10+
const initCalls: { entityId: EntityId; value: number }[] = [];
11+
const setCalls: { entityId: EntityId; value: number }[] = [];
12+
const removeCalls: { entityId: EntityId; value: number }[] = [];
13+
14+
// First create an entity before registering the hook (for on_init test)
15+
const existingEntity = world.spawn().with(A, 100).build();
16+
world.sync();
17+
18+
// Register hook using array syntax with single element
19+
world.hook([A], {
20+
on_init: (entityId, value) => {
21+
initCalls.push({ entityId, value });
22+
},
23+
on_set: (entityId, value) => {
24+
setCalls.push({ entityId, value });
25+
},
26+
on_remove: (entityId, value) => {
27+
removeCalls.push({ entityId, value });
28+
},
29+
});
30+
31+
// on_init should be triggered for existing entity
32+
expect(initCalls.length).toBe(1);
33+
expect(initCalls[0]!.entityId).toBe(existingEntity);
34+
expect(initCalls[0]!.value).toBe(100);
35+
36+
// Create a new entity - should trigger on_set
37+
const newEntity = world.spawn().with(A, 42).build();
38+
world.sync();
39+
40+
expect(setCalls.length).toBe(1);
41+
expect(setCalls[0]!.entityId).toBe(newEntity);
42+
expect(setCalls[0]!.value).toBe(42);
43+
44+
// Update the component - should trigger on_set again
45+
world.set(newEntity, A, 99);
46+
world.sync();
47+
48+
expect(setCalls.length).toBe(2);
49+
expect(setCalls[1]!.entityId).toBe(newEntity);
50+
expect(setCalls[1]!.value).toBe(99);
51+
52+
// Remove the component - should trigger on_remove
53+
world.remove(newEntity, A);
54+
world.sync();
55+
56+
expect(removeCalls.length).toBe(1);
57+
expect(removeCalls[0]!.entityId).toBe(newEntity);
58+
expect(removeCalls[0]!.value).toBe(99);
59+
60+
// Delete the existing entity - should trigger on_remove
61+
world.delete(existingEntity);
62+
world.sync();
63+
64+
expect(removeCalls.length).toBe(2);
65+
expect(removeCalls[1]!.entityId).toBe(existingEntity);
66+
expect(removeCalls[1]!.value).toBe(100);
67+
});
68+
669
it("should throw error when hook has no required components (only optional)", () => {
770
const world = new World();
871
const A = component<number>();

src/core/world.ts

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -135,24 +135,31 @@ export class World {
135135
const archetype = this.entityToArchetype.get(cur);
136136
if (!archetype) continue;
137137

138-
const componentReferences = Array.from(getEntityReferences(this.entityReferences, cur));
139-
for (const [sourceEntityId, componentType] of componentReferences) {
140-
const sourceArchetype = this.entityToArchetype.get(sourceEntityId);
141-
if (!sourceArchetype) continue;
138+
// Process entity references before removal
139+
for (const [sourceEntityId, componentType] of getEntityReferences(this.entityReferences, cur)) {
140+
if (!this.entityToArchetype.has(sourceEntityId)) continue;
142141

143142
if (isCascadeDeleteRelation(componentType)) {
144143
if (!visited.has(sourceEntityId)) {
145144
queue.push(sourceEntityId);
146145
}
147-
continue;
146+
} else {
147+
this.removeComponentImmediate(sourceEntityId, componentType, cur);
148148
}
149-
150-
this.removeComponentImmediate(sourceEntityId, componentType, cur);
151149
}
152150

151+
// Remove entity from archetype - this also cleans up dontFragment relations
152+
// and returns all removed component data
153153
this.entityReferences.delete(cur);
154-
archetype.removeEntity(cur);
154+
const removedComponents = archetype.removeEntity(cur)!;
155155
this.entityToArchetype.delete(cur);
156+
157+
// Trigger lifecycle hooks for removed components
158+
if (removedComponents.size > 0) {
159+
const emptyArchetype = this.ensureArchetype([]);
160+
triggerLifecycleHooks(this.createHooksContext(), cur, new Map(), removedComponents, archetype, emptyArchetype);
161+
}
162+
156163
this.cleanupArchetypesReferencingEntity(cur);
157164
this.entityIdManager.deallocate(cur);
158165
}

0 commit comments

Comments
 (0)