Three attempts at guarding the indoor transition (cellId mask,
Z threshold, terrainZ comparison) all failed because CellSurface
floor polygons are too aggressive — building footprints, roofs, and
upper floors all have PhysicsPolygons that cover wide XY areas at
various Z levels, and ANY outdoor position near a building matches
a cell floor. The proper solution is portal-based transition
(CellPortal boundary crossing), not floor-polygon containment —
but that's Phase E scope.
For the B.2 MVP: outdoor players NEVER transition to indoor cells.
The else-if branch is compiled out with `if (false)`. Indoor→outdoor
transition (walking OUT of a building) is also effectively disabled
since you can't get indoors in the first place. Walking on outdoor
terrain works correctly; walking into buildings will be blocked by
the terrain heightmap (you walk on the roof-level terrain, not
through the building).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Previous cellId-mask fix was necessary but insufficient: the engine
correctly identified the player as outdoor, but then immediately
transitioned to an indoor cell because a CellSurface floor polygon
covered the player's XY at a Z within stepUpHeight. The floor
polygon was a roof or upper floor of a nearby building that happens
to sit at terrain level — not a walkable indoor floor the player
should snap to.
Fix: outdoor→indoor transition now requires bestCellZ < terrainZ - 1.
A genuine indoor transition is into a cell whose floor is BELOW the
terrain surface (basement, ground floor of elevated building). Cells
at or above terrain Z are roofs/upper floors viewed from outside and
must not capture the player.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ROOT CAUSE of the fall-through-ground bug. The currentlyIndoor check
compared the FULL 32-bit cell ID (0xA9B40001, includes landblock
prefix) against 0x0100. Every outdoor cell ID with a landblock
prefix is >= 0x0100, so the engine ALWAYS took the "stay indoors"
path, snapping the player to the nearest EnvCell floor at Z=66
instead of the outdoor terrain at Z=94.
ACE confirmed the bug: "AddWorldObjectInternal: couldn't spawn
+Acdream at 0xA9B40121 [84.2 37.7 66.0]" — we were sending the
server an indoor cell ID with a below-terrain Z position.
Fix: mask cellId with 0xFFFF before the indoor check (outdoor
cells are 0x0001–0x0040; indoor are 0x0100+; the high 16 bits
are the landblock prefix and must be stripped). Also mask the
returned targetCellId from CellSurface (which carries the full
EnvCell dat id) to just the cell index.
265 tests still green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Three more fixes from the diagnostic dump:
1. Initial position: PhysicsEngine.Resolve was mapping the player
into an indoor EnvCell (foundry at Z=66) when they're standing
on outdoor terrain at Z=93+. The cell-containment check was too
aggressive for initial placement. Now uses the server-sent
position directly — the server already gave us a valid position.
2. Yaw unbounded: mouse delta accumulated without wrapping, growing
to 24+ radians. Now wraps to [-PI, PI] after every turn.
3. Turn command spam: MouseDeltaX > 0.5 threshold was too low for
raw pixel deltas. Any mouse jitter triggered turnCmd flips every
frame → stateChanged=True → MoveToState flood to the server.
Mouse turning now only affects yaw directly; turn COMMANDS only
come from A/D keyboard (matching retail client behavior where
mouse-look doesn't generate a TurnRight/TurnLeft command).
265 tests still green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. Underground on Tab: SetPosition used a hardcoded cell ID (0x0001)
and the entity's raw world position without physics resolution.
Now runs PhysicsEngine.Resolve with zero delta to snap the
initial position to the correct terrain Z before the first frame.
2. No animation: UpdatePlayerAnimation required the player entity
to already be in _animatedEntities, but post-spawn UpdateMotion
could have removed it (Phase 6.8 pattern). Now re-registers the
entity with a fresh Setup + PartTemplate if it's missing from
the animated set, so walk/run/turn cycles always resolve.
3. Side view (no turning): OnCameraModeChanged only set CursorMode.Raw
for isFlyMode=true, so entering chase mode (isFlyMode=false) put
the cursor in Normal mode and mouse deltas weren't captured. Now
sets Raw cursor for both fly AND player mode.
Also derives the initial yaw from the player entity's server-sent
rotation quaternion instead of hardcoding PI/2, so the camera starts
facing the direction the character was facing when Tab was pressed.
265 tests still green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tab toggles between fly mode and player-controlled ground movement
when a live session is in-world. WASD walks/runs, A/D + mouse X
turns the character, Z/X strafes, Shift runs. PhysicsEngine resolves
positions against terrain each frame.
MoveToState sent on motion state changes (start/stop walking,
direction change, speed change). AutonomousPosition heartbeat sent
every ~200ms while moving. Walk/run/turn/idle animations resolved
locally via MotionResolver and played through the existing
TickAnimations path. ChaseCamera follows in third-person, mouse Y
adjusts camera pitch.
Tab on Escape also exits player mode gracefully. Mouse delta is
accumulated in the mouse-move callback and consumed+reset each
OnUpdate frame (no accumulation drift).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Outbound GameAction message builders for player movement:
- MoveToState (0xF61C): sent on motion state changes (start/stop
walking, turn, speed change). Carries RawMotionState (flag-driven
variable fields) + WorldPosition + sequence numbers.
- AutonomousPosition (0xF753): periodic position heartbeat sent
every ~200ms while moving. No RawMotionState — just WorldPosition
+ sequences + contact byte.
Both follow the GameAction envelope pattern (0xF7B1 + sequence +
action type) established by GameActionLoginComplete. Wire format
ported from references/holtburger movement protocol — field order
and alignment match exactly (contact byte + pad_to_4).
Also:
- Adds WriteFloat to PacketWriter (needed by both builders)
- Adds SendGameAction + NextGameActionSequence to WorldSession
(public wrappers for PlayerMovementController in Task 2)
11 new tests, 265 total, all green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Per-frame controller that reads MovementInput (WASD/ZX/Shift/mouse),
drives PhysicsEngine.Resolve for collision, and tracks motion state
changes for outbound server messages + animation switching. Walk
(~4 u/s) and run (~7 u/s) speeds match AC retail. Heartbeat timer
triggers AutonomousPosition every ~200ms while moving.
5 new tests covering idle, forward, run, turn, and state-change
detection.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tab-toggled player mode with WASD ground walking, A/D + mouse
turning, Z/X strafing, Shift for run, third-person chase camera,
local walk/run/turn/idle animations, and outbound MoveToState +
AutonomousPosition server messages. Uses PhysicsEngine from B.3
for collision-resolved positions.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Populates the collision engine with TerrainSurface + CellSurface
entries when landblocks stream in, removes them when they stream
out. CellSurface vertices are transformed from cell-local to world
space using EnvCell.Position orientation + origin.
Phase B.2 (player movement mode) will call PhysicsEngine.Resolve()
to get collision-validated positions before sending them to the
server.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Combines TerrainSurface + CellSurface into a single Resolve() API
that handles outdoor terrain walking, indoor floor walking,
outdoor<->indoor cell transitions, step-height enforcement, and
ground detection.
Step-height blocks upward Z deltas exceeding the limit (walls,
cliffs); downhill movement is always accepted. Indoor transitions
pick the cell whose floor Z is closest to the entity's current Z
(handles multi-story buildings). Reports IsOnGround=false when
no landblock or surface covers the entity's position (gravity
applied by the caller).
One API mismatch fixed vs plan: plan encoded the upper 16 landblock
bits into the returned cell ID, but the tests assert the raw cell ID
(0x0100, <0x0100) — so Resolve returns targetCellId directly.
6 new tests covering flat terrain, slopes, step-height rejection,
indoor entry/exit, and void detection. 243 total, all green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extracts the bilinear heightmap interpolation from GameWindow's
inlined SampleTerrainZ into a reusable class. Also adds outdoor
cell ID computation (8×8 grid of 24-unit cells, 0x0001..0x0040).
First component of the physics collision engine.
6 new tests, all green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Projects an XY point onto a cell's floor polygons via brute-force
triangle iteration + barycentric Z interpolation. Fan-triangulates
quads and larger polygons. Returns null when outside all floor
surfaces. Accepts pre-transformed world-space vertex positions so
the caller handles EnvCell coordinate transforms.
Second component of the physics collision engine.
4 new tests, all green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
5-task TDD plan: TerrainSurface (outdoor heightmap Z + cell ID),
CellSurface (indoor floor polygon projection via barycentric interp),
PhysicsEngine (top-level resolver with step-height + cell transitions),
GameWindow integration (populate from streaming), and roadmap update.
~16 new unit tests with fake data. No dat files or rendering needed.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pure-computation collision engine for ground-based entity movement.
Three components: TerrainSurface (outdoor heightmap Z interpolation),
CellSurface (indoor EnvCell floor polygon projection), PhysicsEngine
(top-level resolver with step-height enforcement, outdoor/indoor
cell transitions via CellPortals, and gravity reporting).
Uses PhysicsPolygons from CellStruct for walkable surfaces with
brute-force polygon iteration (< 20 polys per cell). BSP tree
acceleration deferred — same collision fidelity, simpler code.
Standalone module with no rendering or networking dependencies.
~15-20 unit tests with fake data covering flat terrain, slopes,
stairs, wall rejection, and cell transitions. Integration with
the streaming system via ApplyLoadedTerrainLocked. Consumed by
Phase B.2 (player movement mode, separate spec).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
All Foundation sub-pieces now shipped:
A.1 ✓ Streaming landblock loader
A.2 ✓ Frustum culling (~160fps)
A.3 ✓ Background net receive thread
A.4 Folded into A.1 (deferred — DatCollection not thread-safe)
Phase A (Foundation) is complete. Next: Phase B (Gameplay).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
VSync was locking the framerate to 32fps (half of 60Hz) because
frames consistently took slightly over the 16.67ms budget. With
VSync off the perf overlay shows the true framerate so we can
measure the actual impact of frustum culling and future optimizations.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Moves the UDP receive onto a dedicated daemon thread that
continuously pulls raw datagrams from the kernel buffer and posts
them to a Channel<byte[]>. Tick() on the render thread drains the
channel instead of calling _net.TryReceive() directly. All decode,
fragment assembly, ISAAC crypto, event dispatch, and ack-sending
remain on the render thread — this is the minimal change that
prevents packet drops during render-thread stalls without the
complexity of moving decode/dispatch off-thread.
The net thread starts at the end of EnterWorld() after the handshake
is complete — during Connect() and EnterWorld(), PumpOnce() still
reads directly from the socket (the net thread isn't running yet).
Dispose() cancels the thread via CancellationToken, joins with a
2-second timeout, then disposes the socket.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
~160fps uncapped at 5×5 radius with per-landblock AABB culling.
Perf overlay in window title shows visible/total landblock ratio
so the culling impact is visible at a glance.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Updates the window title every ~0.5s with:
FPS | frame time (ms) | visible/total landblocks | entity count | animated count
Example: "acdream | 60 fps | 16.7 ms | lb 12/25 visible | ent 847 | anim 19"
Zero rendering cost — uses Silk.NET's window title setter, no font
renderer or GPU overlay needed. The visible/total landblock ratio
makes Phase A.2 frustum culling's impact visible at a glance: turning
the camera should show the visible count drop as landblocks behind
the camera get culled.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Per-landblock AABB culling against the view frustum. Each loaded
landblock has a 192×192 XY footprint + a Z range derived from the
terrain vertex min/max (padded +50 above / -10 below for entities
on top and basements). One AABB test per landblock per frame;
landblocks fully outside the frustum skip ALL their terrain draws
and entity draws (both opaque and translucent passes).
GpuWorldState gains SetLandblockAabb + LandblockEntries (per-landblock
iteration with AABB data). TerrainRenderer.Draw and
StaticMeshRenderer.Draw both accept an optional FrustumPlanes and
skip culled landblocks. GameWindow.OnRender extracts FrustumPlanes
from camera.View * camera.Projection and passes to both.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Per-landblock frustum culling for the streaming renderer. Extracts
6 normalized view-frustum planes from a View×Projection matrix using
the standard Gribb-Hartmann method. IsAabbVisible tests the AABB's
most-positive vertex against each plane — conservative (no false
negatives) and zero-allocation.
Key implementation note: System.Numerics.Matrix4x4 uses ROW-VECTOR
convention (clip = worldPos * VP), so Gribb-Hartmann must operate on
the COLUMNS of the matrix (not rows). The spec's row-based pseudocode
assumed column-major (OpenGL) convention; the fix is col4 ± col{1..3}.
7 new tests covering ortho, perspective (front/behind/left/far/
near-straddling), and acdream's actual Z-up camera convention.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When closing the acdream window, WorldSession.Dispose now sends
CharacterLogOff (game message 0xF653, no payload) followed by a bare
DISCONNECT control packet (header flag 0x8000) before closing the
UDP socket. This tells ACE to release the character lock immediately
instead of waiting for the ~60s network timeout — which was blocking
rapid iteration during testing since acdream now does a proper login
(Phase 4.8-4.10) and ACE holds the character in-world.
Pattern from references/holtburger/.../client/commands.rs lines
879-892 (Quit handler). Best-effort: if the socket is already dead,
the exception is eaten and Dispose finishes cleanup normally.
220 tests still green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
User reported that some NPCs (Pathwarden, Town Crier) didn't breathe.
Diagnostic logging revealed:
1. Both NPCs are correctly registered as animated at CreateObject
time with the standard 30fps human breath cycle (anim=0x03000001).
2. Immediately after spawn the server sends an UpdateMotion with
stance=0x0003 cmd=0x0000 for these NPCs.
3. MotionResolver.GetIdleCycle returned NULL for that combination,
because StyleDefaults didn't have an entry for stance=3.
4. OnLiveMotionUpdated treated the NULL as "switch to a static pose"
and removed the entity from _animatedEntities.
Two fixes:
A. MotionResolver.ResolveIdleCycleInternal — when stance is set but
StyleDefaults has no entry for it, fall back to the table's
DefaultStyle/DefaultSubstate instead of returning null. The
server-supplied stance was just an unmappable override; the table
default is the correct "I have no better information" answer.
Pulled the table-default lookup into a small TryGetTableDefault
helper so both fallback paths use the same code.
B. OnLiveMotionUpdated — never REMOVE an animated entity. If the
re-resolved cycle is bad (null, framerate=0, or single-frame),
leave the existing cycle running so the entity continues to
breathe with whatever it already had. The defensive "remove on
re-resolve failure" was the bug — it silently un-registered NPCs
the moment the server sent any partial motion update.
Together these mean: any NPC that successfully registers as animated
at spawn stays animated, even if the server's subsequent motion
updates are incomplete or use stance values our resolver doesn't
have a mapping for.
Strips the [BREATHE] and [MOTION] diagnostic spew added during the
investigation now that the cause is identified.
220 tests still green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
User reported that even with the Phase 4.9 ack pump, acdream's
character still rendered to other clients as the purple loading haze.
Spent another round in holtburger's references and found two more
gaps in the post-EnterWorld handshake:
1. Server sends DddInterrogation (game opcode 0xF7E5) and waits for
the client to acknowledge dat-list versions. We never replied.
Build the canonical empty response (12 bytes: opcode + language=1
+ count=0 lists) and ship it as soon as DddInterrogation arrives.
2. LoginComplete was being sent immediately after CharacterEnterWorld
in Phase 4.8, which is too early — the server hasn't finished
creating the player object yet so it ignores LoginComplete and
the player stays in transition state. The correct trigger is the
server's PlayerCreate (0xF746) game message for our character;
that's when holtburger fires send_login_complete (see references/
holtburger/.../client/messages.rs::PlayerCreate handler).
Wired both into ProcessDatagram. Removed the unconditional
LoginComplete from the EnterWorld flow. Added a _loginCompleteSent
latch so re-PlayerCreate (e.g., across portal teleports) doesn't
re-fire LoginComplete during the same session.
Reference repo cited per the new CLAUDE.md guidance — holtburger is
the authoritative client-behavior reference. Should have looked there
sooner; this would have saved the Phase 4.8 false fix.
220 tests still green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Root cause of the still-purple-haze symptom AND the ACE-side
"Network Timeout" drop after ~60s. acdream was never sending
acknowledgement packets back to the server, so the server's
reliability layer saw a one-way stream and eventually dropped the
session. During the 60s window the player rendered to other clients
as the stationary purple loading haze (AC's "this client is in
portal-space transition" indicator).
Pattern ported from
references/holtburger/crates/holtburger-session/src/session/
{send.rs::send_ack, receive.rs::finalize_ordered_server_packet}.
The proper holtburger pattern is per-packet acks, NOT a periodic
heartbeat: every received server packet with sequence > 0 and no
ACK_SEQUENCE flag of its own gets a bare control packet sent back
with:
PacketHeader {
Flags = ACK_SEQUENCE (0x4000),
Sequence = current_client_sequence (= last issued, no increment),
Id = session client id,
}
Body = u32 little-endian server sequence being acked
Acks are cleartext control packets (no EncryptedChecksum) and
re-use the most recently issued client sequence rather than
consuming a new one — they aren't part of the reliable stream the
server tracks for retransmits.
Wired into ProcessDatagram so both Tick (post-InWorld) and PumpOnce
(during Connect/EnterWorld) trigger acks on every received non-ack
server packet.
Also (per user request) upgrades the CLAUDE.md description of the
holtburger reference repo from "Rust AC client crate" to "almost-
complete Rust TUI AC client — the most authoritative reference for
client-side behavior in the project, look here FIRST for anything
WorldSession or message-builder related." This was the third time
in two days I would have saved hours by checking holtburger first
instead of guessing at the protocol from ACE alone.
220 tests green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
User reported that when they observed acdream's character through a
second AC client running on a different account, the character
rendered as a stationary purple haze (AC's "loading screen / portal
space" indicator) instead of a normal avatar. The character was
"in-world enough" to receive the CreateObject stream but never
"in-world enough" for the server to flip its first-enter-world flag,
push initial property updates / equipment overrides, or show the
character to other clients in the area.
Root cause: WorldSession.EnterWorld stopped after sending
CharacterEnterWorld (0xF657). The handshake is supposed to continue
with one more message — a GameAction(LoginComplete) — that ACE's
GameActionLoginComplete handler interprets as "client has exited
portal space, mark FirstEnterWorldDone, push property updates,
make the character visible to others."
Wire layout (confirmed via
references/ACE/Source/ACE.Server/Network/GameAction/GameActionPacket.cs
and .../Actions/GameActionLoginComplete.cs):
u32 game-message opcode = 0xF7B1 (GameAction)
u32 sequence = 0 (ACE ignores; TODO comment in source)
u32 GameActionType opc = 0x000000A1 (LoginComplete)
Send happens immediately after CharacterEnterWorld and just before
flipping the WorldSession state to InWorld. acdream has no portal-
space transition animation, so we can claim "loading complete" the
moment we've sent the EnterWorld message — the dat-side world is
already loaded by then.
1 new test (97 Core.Net total). 220 tests green overall.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move A.1 from "ahead" to "shipped" per the roadmap discipline rule.
The shipped row notes that the loader currently runs synchronously
(the original async-worker design hit DatCollection's lack of thread
safety) and that the Channel-based outbox API is preserved so async
loading can return cleanly when Phase A.3 lands a thread-safe dat
wrapper. Pending-spawn list in GpuWorldState handles live spawn /
streaming races without dropping data.
Quick-lookup table updated:
- "Can't walk past the loaded 3×3 window" → A.1 FIXED ✓
- "Frame hitch crossing landblock boundary" → Phase A.3
(synchronous loader for now; async returns when DatCollection is
thread-safe)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The previous fix in f792931 set MaxCompletionsPerFrame to int.MaxValue
on the theory that synchronous loading made the cap pointless. That
ignored the GPU upload cost: applying 25 landblocks in one Tick
allocates ~25 terrain VBOs + hundreds of entity GfxObj sub-mesh VBOs
+ all unique texture uploads in a single frame, which observably
crashes with OutOfMemoryException on the first frame after login.
The pending-spawn list (also added in f792931) is what actually
fixes the spawn-drop bug — it makes the cap safe by parking
late-arriving spawns until their landblock loads. With both fixes:
- Cap=4 spreads the 25-landblock first-frame load over ~7 frames
(~116ms at 60fps, below human perception)
- Spawns for the 21 not-yet-loaded landblocks land in pending and
back-fill as each one arrives over the next 6 frames
- No data lost, no OOM
219 tests still green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fifth and final Phase A.1 hotfix. Replaces the previous "drop on
miss" semantics in GpuWorldState.AppendLiveEntity with a per-landblock
pending bucket that survives the race where a CreateObject arrives
before its landblock has been streamed in.
Root cause:
The post-login spawn flood (40+ NPCs/items) drains in a single
WorldSession.Tick() call. The synchronous streamer enqueues all 25
visible-window landblocks in one shot but StreamingController.Tick
was capped at MaxCompletionsPerFrame=4, so only 4 landblocks landed
in GpuWorldState on the first frame. The center landblock 0xA9B4FFFF
may or may not have been in those first 4 (HashSet iteration order
is undefined). Spawns whose target landblock wasn't yet loaded were
silently dropped by AppendLiveEntity. Re-ordering the OnUpdate
(streaming first, live second) didn't fix it because the cap still
limited to 4 per frame; spawns for landblocks #5+ kept dropping
until the queue drained, by which point the spawn flood was over.
The reordering was correct but insufficient. The cap was a relic of
the original async streamer design (limit GPU upload spikes per
frame). With the synchronous streamer there's no backlog to spread,
so the cap was pure latency for no benefit. Setting it to int.MaxValue
restores "drain everything you just enqueued" semantics.
The pending-spawn list is the *correct* architecture fix that makes
the system robust against any future ordering bug, not just the cap:
- AppendLiveEntity for an unloaded landblock parks the entity in a
per-landblock pending bucket instead of dropping it.
- AddLandblock drains pending entries for its landblock and merges
them into the loaded record before storing.
- RemoveLandblock drops pending entries for the same landblock —
if the player moved away, the spawns are no longer relevant; the
server resends them via CreateObject when the player returns.
Diagnostic counter PendingLiveEntityCount exposes the bucket size
so future regressions are visible without spelunking.
7 new GpuWorldStateTests pin the contract:
- AppendLiveEntity_LandblockAlreadyLoaded_AppendsImmediately
- AppendLiveEntity_LandblockNotLoaded_ParksInPending
- AddLandblock_DrainsPendingEntriesForThatLandblock
- AddLandblock_DoesNotDrainPendingForADifferentLandblock
- RemoveLandblock_DropsPendingForThatLandblock
- RemoveLandblock_LoadedThenRemoved_DropsItsEntities
- IsLoaded_ReturnsTrueForLoaded_FalseForPendingOnly
Also removes the diagnostic Console.WriteLine I added in the previous
debugging round and the old LiveAppendsResolved/Dropped counters that
were never read by anyone.
219 tests green (212 + 7 new).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fourth Phase A.1 hotfix. Terrain rendered correctly after the 0xFFFF
fix and the canonicalize fix, but live NPCs and weenies remained
invisible. Root cause: in OnUpdate the live session was draining its
incoming-message queue BEFORE the streaming controller had a chance
to load the initial 5×5 window. The first frame's order was:
1. _liveSession.Tick() — drains the post-login CreateObject flood
(40+ NPCs + items at the player's spawn landblock)
2. _streamingController.Tick() — first call, loads the 5×5 window
AppendLiveEntity is a no-op when its target landblock isn't loaded
yet. So all 40+ spawns landed in step 1 before any landblock existed
in GpuWorldState, were silently dropped, and never came back even
after the landblocks finished loading in step 2.
Fix: swap the order. Streaming runs first so the initial window
exists in GpuWorldState before any CreateObject events drain. This is
correct because the streaming Tick is now synchronous (per the
531c9f9 hotfix) — by the time it returns, all landblocks in the
window are loaded and ready for AppendLiveEntity to find them.
A more robust solution would be a pending-spawn list inside
GpuWorldState that backfills entities when their landblock loads
later. That stays as a future improvement; the simple reorder is
correct for the dominant case (login → flood of spawns into the
already-known initial landblock).
212 tests green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
After the 0xFFFF terminator fix in f83a8c1, terrain renders correctly
but live NPCs and weenies disappeared. Root cause: the server's
ServerPosition.LandblockId is in cell-resolved form (0xAAAA00CC where
the low 16 bits are the cell index within the landblock), but the
streaming system stores landblocks in GpuWorldState keyed by their
canonical 0xAAAAFFFF form. AppendLiveEntity was passing the raw
server id straight into the dict TryGetValue, missing every time, and
silently dropping the spawn.
Fix: canonicalize at the GpuWorldState boundary by masking with
(0xFFFF0000u | 0xFFFFu). The XML doc on the method explains the two
forms so future callers don't have to guess. Calling code stays
unchanged.
212 tests green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ROOT CAUSE of the "giant ball with spikes" terrain corruption that
the previous two hotfix attempts (lock + synchronous loading) failed
to address. Threading was a red herring all along.
AC dat conventions:
0xAAAA0xFFFF — LandBlock dat (terrain heightmap)
0xAAAA0xFFFE — LandBlockInfo dat (static-object metadata)
WorldView.NeighborLandblockIds correctly uses 0xFFFF. My
StreamingRegion.EncodeLandblockId from Phase A.1 Task 1 used 0xFFFE
by mistake. Every streaming load was therefore calling
LandblockLoader.Load with the LandBlockInfo id, which makes
DatCollection ask DatBinReader to read a LandBlock from the
LandBlockInfo file. The reader's internal buffer position lands in
the middle of the wrong file's bytes, ReadBytesInternal asks for an
out-of-range slice, throws ArgumentOutOfRangeException, and the
landblocks that DON'T throw return half-populated LandBlock objects
whose Height[] arrays contain garbage. Garbage Z values render as
the spike pattern.
The kicker: my Task-1 review fix added a test
(Constructor_SmallRadius_IDsMatchEncodingRule) that asserted
Assert.Contains(0x1234FFFEu, region.Visible). The test was passing
because it pinned the wrong value. I literally codified the bug.
Fix: change EncodeLandblockId's terminator from 0xFFFEu to 0xFFFFu
and update the test to assert 0x1234FFFFu. The XML doc on Visible
now explicitly explains the 0xFFFF/0xFFFE distinction so this can't
recur.
The previous two hotfixes (_datLock in c991fb2, synchronous streamer
in 531c9f9) stay in place — _datLock is defensive belt-and-suspenders
that documents which entry points read dats, and synchronous loading
is correct-by-default until we decide whether to reintroduce
background loading (Phase A.3 may make it unnecessary anyway).
212 tests green. With this fix the streaming should actually work.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Second hotfix attempt for the "ball of spikes" terrain corruption.
The previous _datLock fix was insufficient because dat reads happen
from many render-thread code paths I didn't enumerate (animation
tick, OnLiveMotionUpdated, OnLivePositionUpdated, the live spawn
hydration, ApplyLoadedTerrain) and locking each is invasive and
fragile.
DatReaderWriter's DatCollection is fundamentally not thread-safe:
DatBinReader's internal buffer position is shared per-database, so
two concurrent .Get<T> calls corrupt each other's read state. The
ArgumentOutOfRangeException at DatBinReader.ReadBytesInternal in
the failure log is the smoking gun — one read started reading a
LandBlock, another moved the reader's position, the first one
asked for the wrong number of bytes.
Until Phase A.3 introduces a thread-safe dat wrapper (or until we
preload all dats into pure in-memory dictionaries), the streamer
runs synchronously: EnqueueLoad invokes the load delegate inline
on the calling thread and writes the result to the outbox in a
single call. The render-thread DrainCompletions loop picks it up
on the same frame.
API surface unchanged — Channel-based outbox, EnqueueLoad/Unload,
DrainCompletions, Start (now no-op), Dispose all preserved. Move
back to async loading is a single-class change once dat thread
safety lands.
Cost: visible frame hitch when crossing landblock boundaries
(rendering the new landblock is now on the render thread). For
default 5×5 the hitch is one landblock per cardinal step, ~50ms
worst case. Acceptable for the MVP — correctness over hitches.
Updated the off-thread test to assert the new synchronous contract
(loader runs on the calling thread). The other 4 tests still pass
unchanged because their spin-drain pattern works with synchronous
delivery too.
The previous _datLock from commit c991fb2 stays in place as
defensive belt-and-suspenders — it's free in synchronous mode and
keeps the contract documented at every dat-reading entry point.
212 tests green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Critical hotfix for the first live run of streaming. User reported
terrain rendered as "a giant ball with spikes with textures on" and
the log showed concurrent-read corruption:
streaming: load failed for 0xA8B3FFFE: ArgumentOutOfRangeException
at AcDream.Core.World.LandblockLoader.Load ... line 18
Line 18 is `dats.Get<LandBlock>(landblockId)`. Root cause: my spec
claimed DatCollection is thread-safe for reads. It isn't. DatCollection
delegates to per-dat DatDatabase instances holding file handles + cache
dictionaries + buffer readers, none of which have locks. The streaming
worker's BuildLandblockForStreaming was reading dats concurrently with
the render thread's ApplyLoadedTerrain and OnLiveEntitySpawned, which
corrupted the internal caches and returned half-populated LandBlock
objects whose Height[] array contained garbage values. Garbage Z
coordinates in the terrain mesh produced the "ball with spikes"
distortion.
Fix: add a single lock object `_datLock` on GameWindow and wrap the
three entry points that read dats on competing threads:
- BuildLandblockForStreaming (worker thread)
- ApplyLoadedTerrain (render thread via StreamingController.Tick)
- OnLiveEntitySpawned (render thread via WorldSession events)
Each is split into a public wrapper that takes the lock and a private
...Locked helper with the original body, so the locking surface is
minimal and easy to audit. The lock is re-entrant per C# semantics,
so nested helper calls within BuildLandblockForStreamingLocked don't
double-acquire.
Contention is acceptable for the MVP: worker loads hold the lock for
tens of milliseconds at most (a single landblock's CPU build), and
the render thread's dat reads are typically <1μs cache hits. A future
pass can reduce contention by pre-building the render-thread work on
the worker and passing it through the completion outbox.
Also updates the spec's erroneous thread-safety claim note in a
follow-up commit once visual verification confirms the fix works.
212 tests still green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Task 7 shipped the streaming MVP with stabs only; this commit ports
the pre-streaming scenery generator and EnvCell interior walker
into BuildLandblockForStreaming so every streamed landblock now
carries the full entity set the old one-shot preload produced.
Scenery (trees/rocks/bushes) from SceneryGenerator.Generate runs
per-landblock on the worker thread with a landblock-derived id
namespace (0x80000000 | (lbId & 0x00FFFF00) | local_index) so
scenery ids don't collide across landblocks.
Interior (EnvCell walls/floors/ceilings via Phase 7.1 CellMesh
plus static interior objects) runs on the worker thread with a
0x40000000-based id namespace. Cell sub-meshes are pre-built on
the worker and handed to the render thread via a
ConcurrentDictionary<uint, IReadOnlyList<GfxObjSubMesh>> which
ApplyLoadedTerrain drains before its per-MeshRef upload loop.
The per-MeshRef upload loop in ApplyLoadedTerrain now skips
non-0x01xxxxxx ids (EnvCell synthetic ids, Setup ids) so it no
longer attempts GfxObj.Get on ids that aren't GfxObj dat records.
The cross-thread cell-mesh dictionary is the only shared mutable
state between the worker and render threads — everything else
flows through the Channel<LandblockStreamResult> outbox.
212 tests green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replaces the one-shot 3×3 preload in OnLoad with
StreamingController + LandblockStreamer. Runtime-configurable
window radius via ACDREAM_STREAM_RADIUS (default 2 → 5×5). OnUpdate
drives StreamingController.Tick once per frame with the current
observer landblock coordinates (camera-offset in offline, player
last-known in live).
_entities flat list replaced by GpuWorldState.Entities. Live
CreateObject handler uses GpuWorldState.AppendLiveEntity instead of
the old list-rebuild-and-replace pattern. Streamer is disposed in
OnClosing before GL teardown so the worker thread is joined before
we release resources.
Terrain build dependencies (heightTable, blendCtx, surfaceCache) are
stored as fields so ApplyLoadedTerrain can call LandblockMesh.Build
on the render thread without re-deriving them per landblock.
ICamera.Position fix: offline observer coordinate uses
_cameraController.Fly.Position (FlyCamera exposes Position; ICamera
does not), which is always up-to-date regardless of active camera mode.
MVP scope: stabs only. Scenery (trees/rocks/bushes) and interior
(EnvCell walls/floors + static objects) will land in Task 8 and
are currently DROPPED from streamed landblocks. The offline view
will show terrain + stabs but no vegetation and no building
interiors until Task 8 lands. Live mode is unaffected since
CreateObject spawns come through a different path.
212 tests green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Called once per frame from OnUpdate. Owns a StreamingRegion and uses
delegates into LandblockStreamer + a terrain-apply callback so unit
tests can inject fakes. Handles first-tick bootstrap (whole window
loads), boundary recenter (diff against previous center), and
drain completions (up to N per frame to cap GPU upload spikes).
4 new tests, all green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replaces GameWindow._entities flat list with a per-landblock dict
keyed by landblock id. The flat entity view is rebuilt on add/remove
so the renderer keeps its simple "iterate Entities" loop. Also
provides AppendLiveEntity for the CreateObject path that spawns
entities into an already-loaded landblock after hydration.
Not thread-safe — all mutation is render-thread only.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
TerrainRenderer's internal landblock collection is now a Dictionary
keyed by landblock id so the streaming system can release GPU
resources per-landblock as the visible window moves. AddLandblock
takes the id as its first parameter; if the same id is added twice,
the old buffers are freed before the new ones land (defensive but
cheap). RemoveLandblock is a no-op for unknown ids and deletes
VBO/EBO/VAO for known ones.
Single existing caller in GameWindow.cs updated to pass the id.
Build green. No unit tests — direct-to-GL methods need a live context.
Tasks 5-7 will validate end-to-end.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Code review follow-up to commit 0904372. Five Important fixes plus
three Minor polish items found by the reviewer before StreamingController
depends on this class under churn.
I1: Dispose is now thread-safe via Interlocked.Exchange on an int
guard. Two concurrent Dispose calls no longer double-dispose the
CancellationTokenSource.
I2: EnqueueLoad/EnqueueUnload now throw ObjectDisposedException when
called after Dispose instead of silently dropping the job. Jobs
vanishing into a completed channel was a debugging hazard.
I3: Start throws ObjectDisposedException when called after Dispose
instead of silently doing nothing (the old guard only checked
whether the thread was non-null, not whether the streamer was
still usable).
I4: New test Load_ExecutesLoaderOnBackgroundThread captures the
loader delegate's ManagedThreadId and asserts it differs from
the test thread's id, proving the whole reason this class
exists (off-thread execution) is actually happening.
I5: New LandblockStreamResult.WorkerCrashed record type for the
outer catch in WorkerLoop. Previously the crash path wrote
Failed(0, ex.ToString()) which collided with landblock (0, 0)
in the north ocean, making "worker crashed" indistinguishable
from "landblock 0 failed to load".
Minor polish:
- M1: Test spin constants (SpinTimeoutMs, SpinStepMs,
SpinMaxIterations) extracted so the 200 x 10ms pattern has one
source of truth.
- M2: DefaultDrainBatchSize public const on LandblockStreamer so
the batch cap has a name and a comment explaining why 4.
- M3: Safety-argument comment on the sync-over-async
WaitToReadAsync call explaining why it cannot deadlock (dedicated
thread, no SyncContext).
- M6: XML remarks on the class and on DrainCompletions documenting
threading contract (Enqueue = any thread, Drain = single consumer
thread).
112 Core + 96 Core.Net tests green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Background thread pulls load/unload jobs from an inbox channel, invokes
a caller-supplied Func<uint, LoadedLandblock?> (production wraps
LandblockLoader.Load, tests inject a fake), and posts results to an
outbox channel the render thread drains. Graceful shutdown via
CancellationToken; failed loads reported rather than retried.
4 new tests, all green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
LandblockStreamJob (Load/Unload) and LandblockStreamResult
(Loaded/Failed/Unloaded) are the channel payload types the next
task's LandblockStreamer will use. Separate file because they're
shared between the worker thread and the render thread and deserve
a focused home.
Folds in two carryover nits from the Task 1 fix review:
- Stale "radius + 1" comments in StreamingRegionTests updated to
match the real radius+2 threshold (no numeric-assertion changes).
- Single-step recenter test now asserts Visible.Count == 25 and
Resident.Count == 30, locking in the Visible/Resident semantic
split behaviorally.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review follow-up from commit 11df793. Three fixes:
1. Visible semantics: StreamingRegion.Visible now strictly describes the
current (2r+1)×(2r+1) window, not window + hysteresis retainees.
Added a parallel Resident property exposing the actual loaded set
(window + hysteresis buffer). This matters because StreamingController
(next task) reads these to decide what to render vs what to unload;
conflating them in one set would have forced awkward post-processing
downstream.
2. Doc/code disagreement: updated the RecenterTo and RegionDiff doc
comments from "radius + 1" to "radius + 2" to match the actual
implementation (which is what the tests require). Also updated the
plan doc so future readers don't hit the same contradiction.
3. Edge-clamping test coverage: added a single-axis edge test
(cx=0, cy=50 → 15 entries) and an ID-encoding test (radius=0 at
0x12,0x34 → 0x1234FFFE) so a swapped-shift bug in EncodeLandblockId
or an asymmetric off-by-one would fail a test instead of passing
silently.
9 tests green, full suite regressions-free.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pure data type describing the set of landblocks inside the current
streaming window, with a diff-style Recenter that returns (toLoad,
toUnload) pairs the LandblockStreamer consumes as jobs. Hysteresis
of radius+2 prevents load/unload churn at boundary crossings (spec
says radius+1 but tests confirm radius+2 is the correct buffer size).
First piece of Phase A.1 per docs/superpowers/plans/2026-04-11-foundation-a1-streaming.md.
7 new tests, all green. Total suite: 105/105.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
9-task TDD plan for the first chunk of Phase A (Foundation): the
streaming landblock loader. Covers StreamingRegion (pure data +
hysteresis diff), LandblockStreamer (background worker + channels),
GpuWorldState (render-thread entity registry), StreamingController
(glue), TerrainRenderer.RemoveLandblock, GameWindow wiring (env var,
camera/player center switch, Dispose), scenery + interior integration,
and the roadmap-shipped-table update.
Each task is a full TDD cycle: failing test, run, minimal impl, run,
commit. ~16 new unit tests land alongside the implementation.
Phase A.2 (frustum culling) and A.3 (net I/O thread) get their own
plans once A.1 ships — they're independent subsystems per the
brainstorming skill's decomposition guidance.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
