Skip to content

Commit f3593b0

Browse files
fix(ecs): type IEngine.removeEntity as boolean, matching what it returns
Review follow-up. The tests assert that removeEntity returns false for a refused reserved-entity removal, but IEngine declared it `void`, so no caller could act on that without a cast — `if (!engine.removeEntity(e))` does not compile against a void return. The mismatch predates this branch: main already ended removeEntity with `return entityContainer.removeEntity(entity)`, and IEntityContainer has always typed that as boolean. The API report shows both side by side, one `void` and one `boolean`. What changed here is that the value became meaningful — false now specifically means "refused, components untouched" — so this is the right place to stop the type lying about it. Non-breaking: widening a return type does not affect callers that ignore it, and nothing implements IEngine. The only `Pick<IEngine, ... 'removeEntity'>` consumers are removeEntityWithChildren / removeNetworkEntityChildrens in tree.ts, both fed the internal removeEntity, which already returns boolean. removeEntityWithChildren is deliberately left `void`: it delegates to removeNetworkEntityChildrens, which is genuinely void, and iterates a tree where a per-entity result has no single meaning. Public API surface changes by exactly one line, regenerated via `make build`: - removeEntity(entity: Entity): void; + removeEntity(entity: Entity): boolean; Bundle snapshots are unchanged, as expected for a types-only edit. Full suite green: 160 suites, 1216 tests.
1 parent 5c171f3 commit f3593b0

2 files changed

Lines changed: 12 additions & 2 deletions

File tree

packages/@dcl/ecs/src/engine/types.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,18 @@ export interface IEngine {
9292
* @public
9393
* Remove all components of an entity
9494
* @param entity - entity
95+
* @returns true if the entity was removed; false if the removal was refused, leaving the
96+
* entity's components untouched. A removal is refused when the entity's NUMBER falls in
97+
* the renderer-reserved range (`RESERVED_STATIC_ENTITIES`): the root, player and camera
98+
* entities, plus the range the avatar system allocates remote players from. Those belong
99+
* to the renderer rather than the scene, and purging them locally desynchronizes the
100+
* scene with no way back — the outgoing deletes are dropped by the renderer's scene
101+
* write guard, so it keeps the entity alive and never re-sends the components.
102+
*
103+
* The check is on the entity NUMBER, so it holds at every version: `#32 v1` packs to
104+
* `65568` and is refused just as `#32 v0` is.
95105
*/
96-
removeEntity(entity: Entity): void
106+
removeEntity(entity: Entity): boolean
97107

98108
/**
99109
* Remove all components of each entity in the tree made with Transform parenting

packages/@dcl/playground-assets/etc/playground-assets.api.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1461,7 +1461,7 @@ export interface IEngine {
14611461
registerComponentDefinition<T>(componentName: string, componentDefinition: ComponentDefinition<T>): ComponentDefinition<T>;
14621462
// (undocumented)
14631463
removeComponentDefinition(componentId: number | string): void;
1464-
removeEntity(entity: Entity): void;
1464+
removeEntity(entity: Entity): boolean;
14651465
removeEntityWithChildren(entity: Entity): void;
14661466
removeSystem(selector: string | SystemFn): boolean;
14671467
readonly RootEntity: Entity;

0 commit comments

Comments
 (0)