From b461e2ee92ca203061d85ea0a862a06bd132fb69 Mon Sep 17 00:00:00 2001 From: Erik Onarheim Date: Mon, 1 Jun 2026 21:09:56 -0500 Subject: [PATCH] fix: possible mem leak on query entity removal (#3768) --- .../entity-component-system/query-manager.ts | 13 ++++--- src/spec/vitest/query-manager-spec.ts | 37 +++++++++++++++++++ 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/src/engine/entity-component-system/query-manager.ts b/src/engine/entity-component-system/query-manager.ts index 53bd4b8c..51e049df 100644 --- a/src/engine/entity-component-system/query-manager.ts +++ b/src/engine/entity-component-system/query-manager.ts @@ -58,7 +58,7 @@ export class QueryManager { } for (const entity of this._world.entities) { - this.addEntity(entity); + query.checkAndModify(entity); } return query; @@ -85,6 +85,7 @@ export class QueryManager { * @param entity */ addEntity(entity: Entity) { + const alreadyTracked = this._addComponentHandlers.has(entity); const maybeAddComponent = this._addComponentHandlers.get(entity); const maybeRemoveComponent = this._removeComponentHandlers.get(entity); const addComponent = maybeAddComponent ?? this._createAddComponentHandler(entity); @@ -103,10 +104,12 @@ export class QueryManager { query.checkAndModify(entity); } - entity.componentAdded$.subscribe(addComponent); - entity.componentRemoved$.subscribe(removeComponent); - entity.tagAdded$.subscribe(addTag); - entity.tagRemoved$.subscribe(removeTag); + if (!alreadyTracked) { + entity.componentAdded$.subscribe(addComponent); + entity.componentRemoved$.subscribe(removeComponent); + entity.tagAdded$.subscribe(addTag); + entity.tagRemoved$.subscribe(removeTag); + } } /** diff --git a/src/spec/vitest/query-manager-spec.ts b/src/spec/vitest/query-manager-spec.ts index 80f272cd..4dd4eb8b 100644 --- a/src/spec/vitest/query-manager-spec.ts +++ b/src/spec/vitest/query-manager-spec.ts @@ -257,6 +257,43 @@ describe('A QueryManager', () => { expect(queryAB.getEntities()).toEqual([]); }); + it('does not duplicate entity subscriptions when creating queries after entities exist', () => { + const world = new ex.World(null); + const entity = new ex.Entity(); + entity.addComponent(new FakeComponentA()); + entity.addTag('A'); + + world.add(entity); + + expect(entity.componentAdded$.subscriptions.length).toBe(1); + expect(entity.componentRemoved$.subscriptions.length).toBe(1); + expect(entity.tagAdded$.subscriptions.length).toBe(1); + expect(entity.tagRemoved$.subscriptions.length).toBe(1); + + world.query([FakeComponentA]); + world.query([FakeComponentB]); + world.queryTags(['A']); + world.queryTags(['B']); + + expect(entity.componentAdded$.subscriptions.length).toBe(1); + expect(entity.componentRemoved$.subscriptions.length).toBe(1); + expect(entity.tagAdded$.subscriptions.length).toBe(1); + expect(entity.tagRemoved$.subscriptions.length).toBe(1); + }); + + it('does not duplicate entity subscriptions when addEntity is called more than once', () => { + const world = new ex.World(null); + const entity = new ex.Entity(); + + world.queryManager.addEntity(entity); + world.queryManager.addEntity(entity); + + expect(entity.componentAdded$.subscriptions.length).toBe(1); + expect(entity.componentRemoved$.subscriptions.length).toBe(1); + expect(entity.tagAdded$.subscriptions.length).toBe(1); + expect(entity.tagRemoved$.subscriptions.length).toBe(1); + }); + it('can update queries when a component is removed', () => { const world = new ex.World(null); const entity1 = new ex.Entity(); -- 2.51.2