fix(#190): interior entity id counter overflowed past its 8-bit budget, aliasing into the next landblock

Found while investigating #189 (missing fountain/candle particles):
reverting the A7.L1 light-carrier hydration fix (9ebb2060) made the
Town Network fountain's water-spray particle work again, which didn't
fit the earlier dat-truth finding that the fountain's own entity was
never touched by that fix. Traced with ACDREAM_DUMP_ENTITY: the
fountain's hydrated entity.Id shifted between reverted (0x400007F8)
and fixed (0x40000815) builds — a 29-id delta matching the extra
mesh-less light carriers the A7.L1 fix now keeps alive earlier in the
same landblock's hydration pass.

Root cause: GameWindow's interior-entity id scheme
(interiorIdBase + localCounter, "0x40XXYY##") reserves only 8 bits
(256 values) for a landblock's ENTIRE interior static population — a
residual explicitly flagged in the #119 fix's own comment ("counter
overflow past 0xFF still bleeds into the lbY byte"). The Town Network
hub (205 cells, one landblock) already sat at 248 before A7.L1; the
light fix pushed it to 277, past the boundary. 0x40000815 decodes as
landblock Y=0x08 — NOT this dungeon's true Y=0x07 — the exact #119
cross-landblock aliasing bug, reincarnated by entity count instead of
a computation bug. EntityScriptActivator keys particle-script
instances by entity.Id directly (no landblock-hint disambiguation
unlike the #119 batch cache), so the aliased id silently broke the
fountain's script tracking.

Fix: AcDream.Core.World.InteriorEntityIdAllocator widens the counter
8->12 bits (256->4096) by shrinking the fixed class prefix from a
full byte (0x40) to its top nibble (0x4_) — verified safe against
every entity.Id classification check in GameWindow (none decode X/Y
back out, they only check thresholds/prefixes). Added a loud
one-time [id-overflow] log if a landblock ever exceeds the new
budget, so this class of bug can never hide silently again.

Core 2675+2skip / App 741+2skip / UI 425 / Net 385 green.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-07-09 12:21:57 +02:00
parent f6b054b7d7
commit e651cb6dd1
3 changed files with 165 additions and 7 deletions

View file

@ -7115,9 +7115,9 @@ public sealed class GameWindow : IDisposable
(lbY - _liveCenterY) * 192f, (lbY - _liveCenterY) * 192f,
0f); 0f);
// Per-landblock id namespace: 0x40000000 | (lbX << 16) | (lbY << 8) | local_counter — // Per-landblock id namespace — see AcDream.Core.World.InteriorEntityIdAllocator
// the same 0xNNXXYY## scheme scenery uses (0x80XXYY##). Distinct from scenery // for the full bit layout + history. Distinct from scenery (0x80000000+) and
// (0x80000000+) and stabs (ids from LandblockLoader). // landblock stabs (0xC0000000+, ids from LandblockLoader).
// //
// #119 ROOT-CAUSE FIX (2026-06-11): this used to be // #119 ROOT-CAUSE FIX (2026-06-11): this used to be
// `0x40000000 | (landblockId & 0x00FFFF00)`, which for landblock keys 0xXXYYFFFF // `0x40000000 | (landblockId & 0x00FFFF00)`, which for landblock keys 0xXXYYFFFF
@ -7127,12 +7127,20 @@ public sealed class GameWindow : IDisposable
// staircase, both 0x40B3FF09). The Tier-1 classification cache then served one // staircase, both 0x40B3FF09). The Tier-1 classification cache then served one
// entity's batches to the other (the cache hint at bucket-draw time was the // entity's batches to the other (the cache hint at bucket-draw time was the
// player's landblock, identical for both) — the session-sticky "broken stairs + // player's landblock, identical for both) — the session-sticky "broken stairs +
// water barrel". Counter overflow past 0xFF still bleeds into the lbY byte (the // water barrel".
// documented #53 scenery-namespace caveat); the cache's (EntityId, owner-derived //
// LandblockHint) tuple key disambiguates that residual case. // #190 (2026-07-09): the fix above LEFT a documented residual — "counter overflow
// past 0xFF still bleeds into the lbY byte." That residual manifested for real:
// the Town Network hub (205 cells, one landblock) reached 277 interior entities
// after the #79/#93 A7.L1 light-carrier hydration fix, aliasing into the NEXT
// landblock's Y-byte (entity script/particle tracking is keyed on entity.Id
// directly — EntityScriptActivator — with no landblock-hint disambiguation, so
// the fountain's water-spray script silently stopped firing). Widened the
// counter budget 8→12 bits (256→4096); see InteriorEntityIdAllocator's doc for
// why this is safe (nothing decodes X/Y back out of an entity id).
uint interiorLbX = (landblockId >> 24) & 0xFFu; uint interiorLbX = (landblockId >> 24) & 0xFFu;
uint interiorLbY = (landblockId >> 16) & 0xFFu; uint interiorLbY = (landblockId >> 16) & 0xFFu;
uint interiorIdBase = 0x40000000u | (interiorLbX << 16) | (interiorLbY << 8); uint interiorIdBase = AcDream.Core.World.InteriorEntityIdAllocator.Base(interiorLbX, interiorLbY);
uint localCounter = 0; uint localCounter = 0;
uint firstCellId = (landblockId & 0xFFFF0000u) | 0x0100u; uint firstCellId = (landblockId & 0xFFFF0000u) | 0x0100u;
@ -7345,6 +7353,17 @@ public sealed class GameWindow : IDisposable
var worldPos = stab.Frame.Origin + lbOffset; var worldPos = stab.Frame.Origin + lbOffset;
var worldRot = stab.Frame.Orientation; var worldRot = stab.Frame.Orientation;
// #190: never silently wrap past the counter budget — that's exactly how
// this bug hid the first time (see the InteriorEntityIdAllocator doc
// comment). Log once per landblock the moment it happens; the id below
// still aliases into the next Y-slot when this fires (4096 is generous
// but not infinite), but at least it's visible in launch.log instead of
// silently breaking entity.Id-keyed systems (scripts, particles, shadows).
if (localCounter == AcDream.Core.World.InteriorEntityIdAllocator.MaxCounter + 1)
Console.WriteLine(
$"[id-overflow] landblock 0x{landblockId:X8} exceeded {AcDream.Core.World.InteriorEntityIdAllocator.MaxCounter + 1} " +
"interior entities — ids are now aliasing into the next landblock's reserved range (#190 class)");
var hydrated = new AcDream.Core.World.WorldEntity var hydrated = new AcDream.Core.World.WorldEntity
{ {
Id = interiorIdBase + localCounter++, Id = interiorIdBase + localCounter++,

View file

@ -0,0 +1,49 @@
namespace AcDream.Core.World;
/// <summary>
/// Packs a stable, collision-free id base for a landblock's dat-hydrated
/// EnvCell-interior entities.
///
/// <para>
/// <b>Why this exists (#190, 2026-07-09).</b> The id space is
/// <c>0x4X XXXXX</c> — the top NIBBLE fixed at 4, distinguishing interior
/// statics from live weenies (ids &lt; 0x40000000), procedural scenery
/// (0x80000000+, bit 31 set), and landblock stabs (0xC0000000+). Nothing
/// decodes a landblock's X/Y back out of an entity id — every consumer
/// (GameWindow.cs's <c>_isOutdoorMesh</c>/<c>_isLandblockStab</c>
/// classification) reads only a THRESHOLD or a full-byte PREFIX — so the
/// internal field widths below are free to change without touching any
/// consumer.
/// </para>
///
/// <para>
/// 28 remaining bits split X(8) / Y(8) / counter(12). X and Y stay at a
/// full byte each to match AC's 0-255 landblock grid — shrinking either
/// reintroduces #119's cross-landblock aliasing (two DIFFERENT landblocks
/// sharing one id space). The prior split (X8/Y8/counter8, "0x40XXYY##")
/// fixed #119 but left only 256 ids for ONE landblock's entire interior
/// static population. The Town Network hub (205 cells, one landblock)
/// already reached 248 before the #79/#93 A7.L1 light-carrier hydration
/// fix, which pushed it to 277 — PAST the 8-bit boundary, silently
/// aliasing into the NEXT landblock's reserved Y-byte (id 0x40000815 read
/// back as landblock Y=0x08, not the true Y=0x07): the exact #119 bug,
/// reincarnated by entity COUNT instead of a computation bug. Shrinking
/// the fixed prefix from a full byte (0x40) to its top nibble (0x4_) frees
/// 4 bits for the counter (8→12 bits, 256→4096 capacity) — ~15x headroom
/// over the observed count.
/// </para>
/// </summary>
public static class InteriorEntityIdAllocator
{
/// <summary>Per-landblock counter budget (12 bits). A landblock hydrating
/// more interior entities than this would alias into the next Y-slot —
/// exactly the #190 bug, just at a higher threshold. Callers should log
/// loudly (never silently wrap) if this is ever exceeded.</summary>
public const uint MaxCounter = 0xFFFu;
/// <summary>The first id in this landblock's reserved range (counter=0).
/// Add a counter in <c>[0, MaxCounter]</c> to allocate a unique entity id
/// within the landblock.</summary>
public static uint Base(uint landblockX, uint landblockY)
=> 0x40000000u | ((landblockX & 0xFFu) << 20) | ((landblockY & 0xFFu) << 12);
}

View file

@ -0,0 +1,90 @@
using AcDream.Core.World;
using Xunit;
namespace AcDream.Core.Tests.World;
/// <summary>
/// #190 (2026-07-09) — the interior entity id space (<c>0x40XXYY##</c>,
/// GameWindow.cs's <c>interiorIdBase + localCounter</c>) reserved only 8 bits
/// (256 values) for a landblock's WHOLE interior static population. The Town
/// Network hub (205 cells) reached 248 before the #79/#93 A7.L1 light fix,
/// then 277 after it — PAST the 8-bit boundary, silently aliasing into the
/// next landblock's reserved Y-byte (0x40000815 decoded as landblock Y=0x08,
/// not the true Y=0x07). Same collision CLASS as #119 (two landblocks
/// sharing one id space), triggered by entity COUNT instead of a computation
/// bug. Widened the counter to 12 bits (4095) by shrinking the fixed class
/// prefix from a full byte (0x40) to its top nibble (0x4_) — verified safe
/// against every classification check that reads entity.Id
/// (GameWindow.cs's <c>_isOutdoorMesh</c> threshold, <c>_isLandblockStab</c>
/// prefix, scenery bit) since none of them decode X/Y back out.
/// </summary>
public class InteriorEntityIdAllocatorTests
{
[Fact]
public void Base_AlwaysAtOrAboveStabThreshold()
{
// GameWindow.cs: `entity.Id < 0x40000000u` classifies "stab" —
// every interior-static base must sit AT or ABOVE this floor for
// every landblock coordinate, including the corners.
for (uint y = 0; y <= 255; y += 51)
for (uint x = 0; x <= 255; x += 51)
Assert.True(InteriorEntityIdAllocator.Base(x, y) >= 0x40000000u);
}
[Fact]
public void Base_NeverMatchesLandblockStabPrefix()
{
// GameWindow.cs: `(entity.Id & 0xFF000000u) == 0xC0000000u` identifies
// LandBlockInfo stabs — an interior-static base must never collide.
for (uint y = 0; y <= 255; y += 51)
for (uint x = 0; x <= 255; x += 51)
Assert.NotEqual(0xC0000000u, InteriorEntityIdAllocator.Base(x, y) & 0xFF000000u);
}
[Fact]
public void Base_NeverSetsSceneryBit()
{
// GameWindow.cs: `(entity.Id & 0x80000000u) != 0` identifies procedural
// scenery — an interior-static base must never set bit 31.
for (uint y = 0; y <= 255; y += 51)
for (uint x = 0; x <= 255; x += 51)
Assert.Equal(0u, InteriorEntityIdAllocator.Base(x, y) & 0x80000000u);
}
[Fact]
public void MaxCounter_StaysWithinTheSameLandblocksReservedRange()
{
// The literal #190 repro, generalized: adding the full counter budget
// to a landblock's base must never spill into a DIFFERENT landblock's
// base. Checked against every adjacent-Y and adjacent-X neighbor.
for (uint y = 0; y < 255; y++)
{
uint thisBase = InteriorEntityIdAllocator.Base(0, y);
uint nextYBase = InteriorEntityIdAllocator.Base(0, y + 1);
Assert.True(thisBase + InteriorEntityIdAllocator.MaxCounter < nextYBase,
$"y={y}: counter budget overflows into landblock y={y + 1}'s id range");
}
}
[Fact]
public void TownNetworkRepro_277EntitiesStaysInTheSameLandblock()
{
// The exact #190 repro values: landblock 0x0007xxxx (X=0, Y=7), 277
// hydrated entities (the count AFTER the #79/#93 A7.L1 light fix).
// Under the OLD 8-bit-counter scheme this aliased into Y=8's range
// (0x40000815 read back as Y=0x08). Under the new scheme it must not.
uint id = InteriorEntityIdAllocator.Base(0, 7) + 277u;
uint decodedYByte = (id & 0xFF000000u); // old scheme's Y read (byte-aligned)
Assert.NotEqual(0xC0000000u, decodedYByte); // sanity: still not a landblock-stab
Assert.True(id < InteriorEntityIdAllocator.Base(0, 8),
"277 entities must stay inside landblock Y=7's reserved id range");
}
[Fact]
public void MaxCounter_Is4095()
{
// Locks the documented capacity (12-bit counter) so a future edit
// that accidentally narrows it again gets caught here first.
Assert.Equal(0xFFFu, InteriorEntityIdAllocator.MaxCounter);
}
}