fix(ecs): never generate, recycle or delete a renderer-reserved entity id

[LautaroPetaccio]

createEntityContainer can hand a scene an entity id inside the renderer-reserved range, where it collides exactly with a live remote player's avatar entity. Engine.removeEntity then purges that player's components from the scene's local ECS.

Mechanism

Two defects combine.

The free list accepts reserved entity numbers. systems/crdt/index.ts calls entityContainer.updateRemovedEntity for every inbound DELETE_ENTITY, including the renderer's tombstones for departed peers. That records a reserved entity number in removedEntities. The DELETE_ENTITY branch never consults entity state, so EntityState.Reserved cannot skip it.

The recycling loop doesn't filter them. generateEntity scans removedEntities with only a version < MAX_U16 guard and returns toEntityId(number, version + 1).

The result is not an overlapping range but the same id, because the renderer computes the same recurrence from the same stored version:

Renderer	reissues a vacated slot as	toEntityId(number, storedVersion + 1)
This loop	recycles it as	toEntityId(number, storedVersion + 1)

Reproduced against the default container — no injected container, nothing custom:

scene creates an entity, a peer disconnects (inbound DELETE_ENTITY for #40 v0),
scene releases its own entity:

engine.addEntity() -> 65576 (#40 v1)
  entity number 40 is RESERVED (renderer-owned)
  in the avatar range [32,256)? true

removeEntity compounds it by comparing the packed id against reservedStaticEntities. The version occupies the high 16 bits, so that only catches version 0 — number 32 at version 1 packs to 65568 and passes:

removeEntity(196640 (n=32, v=3)) -> true
   guard is `packed < 512`; packed = 196640 -> guard PASSES
>>> generateEntity() -> 262176 (n=32, v=4) | RESERVED? true

So a scene could delete a live remote player's entity, and each such removal re-armed the slot at version+1, making every poisoned slot a renewable source. Over 5000 randomized ops, 7 seeded reserved numbers produced 244 reserved-range allocations, versions climbing to 24–45. getEntityState decomposed correctly all along; these now agree.

Why the recycling branch is reachable

generateEntity only scans the free list when usedEntities.size + reservedStaticEntities < entityCounter. A composite build puts scenes there permanently: DCL_MAX_COMPOSITE_ENTITY starts entityCounter past the composite's max entity while main.crdt marks fewer entities used. For one production scene that is a standing deficit of 4 with zero scene-side removals, plus one slot per removal after that:

phase 1 (no scene removals): 4 reserved of 20 allocations
phase 2 (10 removals, 30 allocations): 7 reserved of 30

What it breaks

Engine.removeEntity purged components off any entity, including renderer-owned ones. The renderer drops the scene's outgoing deletes for the avatar range, so it keeps the entity alive and never re-sends:

scene calls engine.removeEntity( 65571 (#35 v1) ) on a LIVE player
   outbound: type=2 entity=65571 component=1, type=2 entity=65571 component=1089, type=3 entity=65571
   players visible to getEntitiesWith(PlayerIdentityData, Transform): 0
   after 8 more renderer Transform packets:
     Transform          -> back (y=82)
     PlayerIdentityData -> STILL GONE

Transform returns because it is restreamed; PlayerIdentityData is sent once per peer, so it does not. The entity is left as a moving Transform with no identity, invisible to every getEntitiesWith(PlayerIdentityData, …) query. Recovery needs the peer reconnecting or the scene's subscription being recreated — neither happens on its own mid-session.

Which component is lost depends on the interleaving. entityDeleted clears data and lastSentData but never timestamps, so if the scene held the id first, the renderer's Transform PUTs lose until its own timestamp passes the retained one — roughly one packet per scene write:

scene allocated 655395 (#35 v10)   <- 7 writes, retained timestamp = 7
  renderer Transform ts=1 -> ABSENT
  renderer Transform ts=9 -> 59        <- recovers only here

Either ordering surfaces as a player with no resolvable position.

Field evidence

From a 3 h authoritative-server log where all proximity decisions route through one getEntitiesWith(PlayerIdentityData, Transform) lookup:

• Direction of causation. The scene wrote a Transform to #32 v21 5.7 s before the renderer minted that id (Reused entity 32 version 20 → blocked scene op on 1376288 → Reused entity 32 version 21 (id: 1376288)). The scene's allocator invented it; the renderer collided with it afterwards.
• Silent player removal. Of 223 player-left events, 16 had no peer disconnect within 3 s. 15 of those 16 share a millisecond with a blocked scene CRDT op; none occur without one.
• Permanence. 13 identifiable players were removed from the authoritative scene's world model while still connected and playing. None recovered, over windows up to 24 minutes. Of the 7 who kept playing, 7/7 were broken for the rest of the session.

Host scope

Worth stating precisely, because the four hosts differ and the fix has to hold for all of them:

Host	Avatar entity range	Sends DELETE_ENTITY for avatars	Bumps version on reuse	Exact collision
headless runtime	[32, 256)	every departure	yes	yes, always
godot-explorer	[32, 256)	only for peers live at the scene's tick 0	yes	yes, in that window
bevy-explorer	6..=405	yes, for every census.died	yes	yes
unity-explorer	[32, 256)	never — no entity-level delete in its outgoing API	no, always v0	no

unity emits five per-component DELETE_COMPONENTs on disconnect instead, and since updateRemovedEntity is reachable only from DELETE_ENTITY, it never poisons the free list. It also has no reserved-range guard on scene-authored ops, so a scene write to an avatar id is applied there rather than dropped — and deletedEntities is keyed by entity number while entitiesMap is keyed by packed id, so a scene-authored DELETE_ENTITY on an avatar number would permanently tombstone that number for the scene's lifetime.

bevy is worth calling out because it picks an entirely different range — foreign players start at 6, and it reserves WORLD_ORIGIN = 5 on top of root/player/camera. Its kill bumps the generation and forwards a DeleteEntity to the scene for every death, so the collision arithmetic lands identically: a peer leaving slot 6 makes the renderer reissue new(6, 1) = 65542, which is exactly what the scene's recycling loop produces from toEntityId(6, 0 + 1).

None of this is specified. @dcl/protocol pins neither the avatar range nor the id packing, and no CRDT wire-protocol document ships with it — yet five codebases hardcode these constants independently (@dcl/ecs, the headless runtime, and the three explorers), they disagree on the range, on whether reuse bumps the version, and on whether reserved writes are rejected. bevy even disagrees with itself: dcl_component declares 6..=405 while comms declares 6..=406, and the allocator uses the latter.

The container is therefore the only place this invariant can hold regardless of which host is attached.

The fix

entity.ts — one helper, isReservedEntity(entity, reservedStaticEntities), masking the low 16 bits so it holds at every version. Used to refuse reserved numbers in removeEntity, updateRemovedEntity and updateUsedEntity, to skip them in generateEntity's recycling loop, and to short-circuit getEntityState — so the engine's classification and the container's release decision are literally the same expression.

index.ts — Engine.removeEntity classifies before acting and skips the component purge only for entities the renderer streams:

const [entityNumber] = EntityUtils.fromEntityId(entity)
const isAvatarEntity =
  entityNumber >= NAMED_STATIC_ENTITIES && entityContainer.getEntityState(entity) === EntityState.Reserved

const released = entityContainer.removeEntity(entity)
if (isAvatarEntity) return released

Deliberately not the whole reserved range. The renderer denies scene component ops only on the avatar range; ops on RootEntity/PlayerEntity/CameraEntity reach it and are applied — that is how InputModifier.deleteFrom(engine.PlayerEntity) clears an input lock, and its wire frame is byte-identical to the one a removal emits. Treating those three like avatars would turn a working removal into a silent no-op.

Asking the container rather than a module constant means a custom container injected through IEngineOptions.entityContainer is handled correctly too, on a member every implementation already provides.

getEntityState on the engine is delegated rather than aliased: a detached entityContainer.getEntityState reference binds this to the engine, so a container that uses this would report a state contradicting the engine's own behaviour.

types.ts — IEngine.removeEntity returns boolean: whether the id was released for reuse. Public API surface changes by exactly one line.

-    removeEntity(entity: Entity): void;
+    removeEntity(entity: Entity): boolean;

Non-breaking — widening a return type doesn't affect callers that ignore it, nothing implements IEngine, and the only Pick<IEngine, …'removeEntity'> consumers are removeEntityWithChildren / removeNetworkEntityChildrens, both fed the internal function.

Tests

test/ecs/reserved-entity-range.spec.ts, 29 tests. Full suite: 160 suites, 1231 tests.

Mutation-checked — every mutation of the changed logic is killed:

Mutation	Result
isReservedEntity < → <=	killed (6 tests)
NAMED_STATIC_ENTITIES 3 → 0	killed (2)
NAMED_STATIC_ENTITIES 3 → 4	killed (1)
invert the purge guard	killed (9)
drop the named-static lower bound	killed (2)
bound from the module default, not the container	killed (3)
removeEntity always returns true	killed (2)
drop the mask (entity < bound)	killed (5)
getEntityState aliased instead of delegated	killed (1)
generateEntity reserved-skip removed	survives — unreachable by construction

That last one survives because every writer into removedEntities refuses reserved numbers, so the loop can never see one; the guard is kept as a second enforcement point and the comment says so.

The CRDT output is byte-identical to main for non-reserved entities across both the renderer and network transports — outgoing bytes, onProcessEntityComponentChange sequence, and onChange callbacks. The only diff in that scenario is that main emitted DELETE_ENTITY to the renderer for a live remote player and this branch does not.

Known limitations

• removeEntityWithChildren can complete partially and does not report it. A reserved node anywhere in a Transform tree survives while its descendants are removed, leaving its Transform.parent pointing at a removed entity. It returns void, so a caller cannot detect this. Reachable only if a scene parents a reserved entity under a scene entity; documented on the interface rather than changed, since fixing it means either skipping such subtrees or altering the public signature.
• A non-integer reservedStaticEntities makes generateEntity's >= and isReservedEntity's < non-complementary, so a generated entity can be classified Reserved. Nothing passes one.

Not in this change

systems/crdt/index.ts purges every component for any inbound DELETE_ENTITY with no reserved-range guard. That is reachable from the network transport, which lets a peer erase a live remote player's identity and transform from a scene — which the scene then forwards onward:

after inbound DELETE_ENTITY on the NETWORK transport:
  identity=null  transform=null  container state=Reserved
  forwarded to renderer: DELETE_ENTITY e=0x00010020,
    DELETE_COMPONENT e=0x00010020 c=1, DELETE_COMPONENT e=0x00010020 c=1089

An unconditional guard there would be wrong — the renderer's tombstone for a departed player must purge — so the fix is transport-gating, a change to CRDT receive semantics that deserves its own review. Filed separately.

🤖 Generated with Claude Code

[cloudflare-workers-and-pages]

Deploying js-sdk-toolchain with    Cloudflare Pages

Latest commit:	9b06596
Status:	✅  Deploy successful!
Preview URL:	https://3b9d6502.js-sdk-toolchain.pages.dev
Branch Preview URL:	https://fix-ecs-reserved-entity-rang.js-sdk-toolchain.pages.dev

View logs

[github-actions]

Test this pull request

• The @dcl/sdk package can be tested in scenes by running

  npm install "https://sdk-team-cdn.decentraland.org/@dcl/js-sdk-toolchain/branch/fix/ecs-reserved-entity-range/dcl-sdk-7.26.1-32144224411.commit-890c64a.tgz"

• The @dcl/js-runtime package can be tested in scenes by running

  npm install "https://sdk-team-cdn.decentraland.org/@dcl/js-sdk-toolchain/branch/fix/ecs-reserved-entity-range/@dcl/js-runtime/dcl-js-runtime-7.26.1-32144224411.commit-890c64a.tgz"

• To test with npx init

  export SDK_COMMANDS="https://sdk-team-cdn.decentraland.org/@dcl/js-sdk-toolchain/branch/fix/ecs-reserved-entity-range/dcl-sdk-commands-7.26.1-32144224411.commit-890c64a.tgz"
  npx $SDK_COMMANDS init

• The /changerealm command to test test in-world

  /changerealm https://sdk-team-cdn.decentraland.org/ipfs/fix/ecs-reserved-entity-range-e2e
  

• You can preview this build entering:
  https://playground.decentraland.org/?sdk-branch=fix/ecs-reserved-entity-range

[decentraland-bot]

Review complete — approved.

I did not find any P0/P1 issues. The changes consistently enforce the renderer-owned entity-number boundary in the allocator, entity removal, inbound tombstone handling, and component purge ordering. The regression coverage exercises the collision path, reserved removal refusal, and ordinary scene-owned removal behavior.

Security review: No security issues found.

Public API / consumer impact: This is a corrective behavior change for invalid scene attempts to remove renderer-owned entities. Existing scene-owned entity removal remains covered and unchanged.

Non-blocking follow-up: Engine.removeEntity() now has tests that assert its boolean return, but the public IEngine.removeEntity type is still void. If callers are expected to observe false for refused reserved-entity removals, consider updating that type and release notes in a follow-up.

CI status: all reported checks are passing.

---

Reviewed by Jarvis 🤖 · Requested by Lautaro Petaccio (<@U025WCHLMN3>) via Slack

[decentraland-bot]

Deep re-review complete — approved.

I re-read the current head (ee5c3c9) and focused on the allocator invariant, inbound CRDT tombstone paths, component purge ordering, public API impact, tests, and security. I did not find any P0/P1 issues.

Findings:

• P2: left one non-blocking inline comment about Engine.removeEntity using the default renderer-streamed bound even when an injected/custom entity container owns a different reserved range. This is unlikely to affect the default production path, but it is worth tightening because IEngineOptions.entityContainer is an extension seam.

Security review: No security issues found.

Public API / consumer impact: IEngine.removeEntity is now typed as returning boolean, matching runtime behavior and the API report. Callers that ignore the return remain compatible; custom implementations/mocks may need to return a boolean. I did not find a blocking downstream usage issue.

CI status: all reported checks are passing.

---

Reviewed by Jarvis 🤖 · Requested by Lautaro Petaccio (<@U025WCHLMN3>) via Slack

[decentraland-bot]

[P2] Non-blocking: this purge decision uses isRendererStreamedNumber(entity) with the default RESERVED_STATIC_ENTITIES bound, while Engine can be constructed with an injected entityContainer whose reserved/owned range differs. In that case the container may return released === true for an entity number that the default helper still classifies as renderer-streamed, so components would be left attached to an entity the container already released. Consider tying the purge decision to the container result as well, e.g. purge when released || !isRendererStreamedNumber(entity), and add a small regression test with Engine({ entityContainer: createEntityContainer({ reservedStaticEntities: 32 }) }).

[decentraland-bot]

Deep re-review complete — approved.

I reviewed the current head (d8e093e) across allocator behavior, packed entity-number/version handling, CRDT tombstone paths, Engine.removeEntity purge ordering, custom entityContainer bounds, public API impact, snapshots, and the new regression coverage.

Findings:

• No P0/P1 issues found.
• No security issues found.
• The previous custom-container bound concern is covered by the latest change and regression tests.
• Remaining P2/non-blocking consideration: IEngine.removeEntity is now typed as returning boolean. That matches the implementation behavior and the new tests, and callers that ignore the result remain compatible; custom mocks/implementations may need to return a boolean after upgrading.

Consumer impact: this is a corrective behavior change for renderer-reserved entity ownership. I did not find blocking downstream usage in org search for custom entityContainer injection or removeEntity(entity implementers.

CI status: all reported checks are passing.

---

Reviewed by Jarvis 🤖 · Requested by Lautaro Petaccio (<@U025WCHLMN3>) via Slack

[decentraland-bot]

Deep re-review complete — approved.

I reviewed the current head (3d4c50d) with a fresh pass over the allocator invariant, packed entity-number/version handling, inbound CRDT tombstones, Engine.removeEntity purge ordering, named-static vs avatar-range behavior, custom entityContainer bounds, public API impact, snapshots, and the expanded regression tests. I also ran parallel TypeScript/security/architecture/pattern/simplicity review agents and deduplicated their findings.

Findings:

• No P0/P1 issues found.
• No security issues found.
• The latest change removes the optional public IEntityContainer.reservedStaticEntities surface and keeps the purge decision aligned with the container by classifying through getEntityState before removal.
• Remaining P2/non-blocking consideration: IEngine.removeEntity is now typed as returning boolean. That matches the implementation and tests, and callers that ignore the result remain compatible; custom mocks/implementations typed as IEngine may need to return a boolean after upgrading.

Consumer impact: I re-checked org usage for IEngine / removeEntity. I found call sites that ignore the return, but no external implements IEngine implementation that would block this change. The API report is updated.

CI status: all reported checks are passing (lint, docs, test, build, CLI E2E, dependency check, Cloudflare Pages).

---

Reviewed by Jarvis 🤖 · Requested by Lautaro Petaccio (<@U025WCHLMN3>) via Slack

[pravusjif]

LGTM