FOREIGN_PLAYER_RANGE is declared twice with different values, and new_in_range can allocate outside it

[LautaroPetaccio]

FOREIGN_PLAYER_RANGE is declared twice, with two different values, and neither one actually bounds the allocator that consumes it.

1. The two declarations disagree

// crates/dcl_component/src/lib.rs:135  (associated const on SceneEntityId)
pub const FOREIGN_PLAYER_RANGE: RangeInclusive<u16> = 6..=405;

// crates/comms/src/global_crdt.rs:46   (module-level const)
const FOREIGN_PLAYER_RANGE: RangeInclusive<u16> = 6..=406;

They differ on whether entity 406 is a valid foreign-player slot. The only consumer is global_crdt.rs:449:

let Some(next_free) = state.context.new_in_range(&FOREIGN_PLAYER_RANGE) else {

The bare identifier resolves to the module-level const, so 6..=406 is the value in force and SceneEntityId::FOREIGN_PLAYER_RANGE (6..=405) is never read anywhere — grep -rn "SceneEntityId::FOREIGN_PLAYER_RANGE" returns nothing. The public, documented-looking constant is dead; the private one is authoritative.

Both lines date to 425ea40e (2023-05-09, "Comms (#18)") and neither has been edited since, so they were born disagreeing rather than drifting apart. I don't know which value is intended — that's the question here.

2. Neither value bounds anything

The more consequential half. CrdtContext::new_in_range (crates/dcl/src/interface/crdt_context.rs:143-164) tests the wrong variable when it wraps:

while next_new != self.last_new {
    if !self.entity_entry(next_new).1 { /* ... */ return Some(new_id); }
    next_new += 1;
    if !range.contains(&self.last_new) {   // <-- last_new, which the loop never advances
        self.last_new = *range.start();
    }
}

The guard should test next_new. As written, next_new increments past *range.end() unchecked, and entity_entry accepts any u16, so the scan keeps walking and returns ids outside the range.

Reproduced by lifting the function verbatim with a stubbed live_entities table:

allocated 402 slots before the range was exhausted   (6..=406 is 401 slots, + 407)
next 12 results once full: 408* None 409* None 410* None 411* None 412* None 413* None
(* = outside FOREIGN_PLAYER_RANGE)

out-of-range ids handed out: 414 .. 613 (count 200)
any >= 512 (collides with scene-allocated entity numbers): true

Two consequences:

• It escapes into scene-owned entity numbers. @dcl/ecs reserves [0, 512) for the renderer and allocates its own scene entities at 512 and above. This walk reaches 613 in the repro and keeps climbing, so a foreign player can be handed an entity number a scene also believes it owns. That is the same collision class as fix(ecs): never generate, recycle or delete a renderer-reserved entity id js-sdk-toolchain#1544, arriving from the opposite direction — and the fix there cannot help, because it refuses ids below RESERVED_STATIC_ENTITIES and these are above it.
• next_new += 1 overflow-panics. It is += 1, not wrapping_add, so once the walk reaches 65535 it panics in any build with overflow checks on:

  iter 120000 highest=60206
thread 'main' panicked at 'attempt to add with overflow'

In release it wraps to 0 instead and scans the named reserved slots (ROOT 0, PLAYER 1, CAMERA 2, WORLD_ORIGIN 5).

Precondition, stated plainly: all 401 slots must be simultaneously live — entity_entry(id).1 — before any of this is reachable. Departed peers free their slots, so this needs 400+ concurrent foreign entities in one context, which is a stress scenario rather than something a normal session hits. The overflow needs ~65k further allocation attempts on top. So: not urgent, but the range currently documents an invariant it does not enforce, and the failure mode when it is crossed is silent cross-wiring rather than a clean None.

Suggested fix

1. Delete one of the two constants — keep SceneEntityId::FOREIGN_PLAYER_RANGE as the single public definition, import it in global_crdt.rs, and decide 405 vs 406 deliberately.
2. In new_in_range, test the variable that moves:

next_new = if next_new >= *range.end() { *range.start() } else { next_new + 1 };

which bounds the scan, removes the overflow, and makes exhaustion return None as intended. Worth a test that fills the range and asserts None rather than an out-of-range id.

Context

Found while tracing an entity-id recycling bug in @dcl/ecs (decentraland/js-sdk-toolchain#1544), where a scene's allocator recycles renderer-owned avatar ids. Checking whether bevy-explorer was exposed to that one surfaced this. Note bevy is the only host that picks a range other than [32, 256), and @dcl/protocol specifies neither the range nor the id packing, so each host hardcodes it independently.