diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index d7a2228a..5a7b4a4f 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -7115,9 +7115,9 @@ public sealed class GameWindow : IDisposable (lbY - _liveCenterY) * 192f, 0f); - // Per-landblock id namespace: 0x40000000 | (lbX << 16) | (lbY << 8) | local_counter — - // the same 0xNNXXYY## scheme scenery uses (0x80XXYY##). Distinct from scenery - // (0x80000000+) and stabs (ids from LandblockLoader). + // Per-landblock id namespace — see AcDream.Core.World.InteriorEntityIdAllocator + // for the full bit layout + history. Distinct from scenery (0x80000000+) and + // landblock stabs (0xC0000000+, ids from LandblockLoader). // // #119 ROOT-CAUSE FIX (2026-06-11): this used to be // `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 // 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 + - // water barrel". Counter overflow past 0xFF still bleeds into the lbY byte (the - // documented #53 scenery-namespace caveat); the cache's (EntityId, owner-derived - // LandblockHint) tuple key disambiguates that residual case. + // water barrel". + // + // #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 interiorLbY = (landblockId >> 16) & 0xFFu; - uint interiorIdBase = 0x40000000u | (interiorLbX << 16) | (interiorLbY << 8); + uint interiorIdBase = AcDream.Core.World.InteriorEntityIdAllocator.Base(interiorLbX, interiorLbY); uint localCounter = 0; uint firstCellId = (landblockId & 0xFFFF0000u) | 0x0100u; @@ -7345,6 +7353,17 @@ public sealed class GameWindow : IDisposable var worldPos = stab.Frame.Origin + lbOffset; 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 { Id = interiorIdBase + localCounter++, diff --git a/src/AcDream.Core/World/InteriorEntityIdAllocator.cs b/src/AcDream.Core/World/InteriorEntityIdAllocator.cs new file mode 100644 index 00000000..cc4cecff --- /dev/null +++ b/src/AcDream.Core/World/InteriorEntityIdAllocator.cs @@ -0,0 +1,49 @@ +namespace AcDream.Core.World; + +/// +/// Packs a stable, collision-free id base for a landblock's dat-hydrated +/// EnvCell-interior entities. +/// +/// +/// Why this exists (#190, 2026-07-09). The id space is +/// 0x4X XXXXX — the top NIBBLE fixed at 4, distinguishing interior +/// statics from live weenies (ids < 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 _isOutdoorMesh/_isLandblockStab +/// classification) reads only a THRESHOLD or a full-byte PREFIX — so the +/// internal field widths below are free to change without touching any +/// consumer. +/// +/// +/// +/// 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. +/// +/// +public static class InteriorEntityIdAllocator +{ + /// 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. + public const uint MaxCounter = 0xFFFu; + + /// The first id in this landblock's reserved range (counter=0). + /// Add a counter in [0, MaxCounter] to allocate a unique entity id + /// within the landblock. + public static uint Base(uint landblockX, uint landblockY) + => 0x40000000u | ((landblockX & 0xFFu) << 20) | ((landblockY & 0xFFu) << 12); +} diff --git a/tests/AcDream.Core.Tests/World/InteriorEntityIdAllocatorTests.cs b/tests/AcDream.Core.Tests/World/InteriorEntityIdAllocatorTests.cs new file mode 100644 index 00000000..bdbb32cc --- /dev/null +++ b/tests/AcDream.Core.Tests/World/InteriorEntityIdAllocatorTests.cs @@ -0,0 +1,90 @@ +using AcDream.Core.World; +using Xunit; + +namespace AcDream.Core.Tests.World; + +/// +/// #190 (2026-07-09) — the interior entity id space (0x40XXYY##, +/// GameWindow.cs's interiorIdBase + localCounter) 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 _isOutdoorMesh threshold, _isLandblockStab +/// prefix, scenery bit) since none of them decode X/Y back out. +/// +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); + } +}