Move ISSUE #52 from Active to Recently closed with full root-cause writeup
referencing commit `e40159f`. Strip lifestone reference from CLAUDE.md
"Currently in flight"; remaining post-A.5 polish scope is #53 (Tier 1
retry) + #54 (JobKind plumbing).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three root causes regressed the Holtburg lifestone since the WB rendering
migration (Phase N.5 retirement amendment, commit dcae2b6, 2026-05-08).
All confirmed via temporary [LIFESTONE-DIAG] instrumentation and visually
verified by the user through the +Acdream test character.
1. **Alpha-test discard** in mesh_modern.frag transparent pass killed
high-α pixels of dat-flagged transparent surfaces. Native AC
transparent surfaces routinely include effectively-opaque pixels —
e.g. the lifestone crystal core (surface 0x080011DE) — that compose
correctly under (SrcAlpha, 1-SrcAlpha) blending. The original N.5
§2 rationale ("high-α belongs in opaque pass") doesn't hold for
surfaces flagged transparent at the dat level: those pixels can't
reach the opaque pass at all. Fix: remove `α >= 0.95 discard` from
the transparent pass, keep `α < 0.05 discard` as a fragment-cost
optimization (skip totally-empty pixels).
2. **Cull state** for the transparent pass was unset by
WbDrawDispatcher after the N.5 retirement amendment deleted
StaticMeshRenderer.cs (which had the Phase 9.2 setup at commit
6f1971a, 2026-04-11). Closed-shell translucents — lifestone crystal,
glow gems — need GL_CULL_FACE + GL_BACK + GL_CCW in the transparent
pass; otherwise back faces composite over front faces in iteration
order under DepthMask(false). Fix: re-establish Phase 9.2's exact
GL state setup at the top of Phase 8.
3. **uDrawIDOffset uniform** was missing from mesh_modern.vert.
gl_DrawIDARB resets to 0 at the start of each
glMultiDrawElementsIndirect call, so the transparent pass — which
begins later in the indirect buffer — was fetching
Batches[0..transparentCount) instead of its actual section at
Batches[opaqueCount..end). The lifestone crystal ended up reading
the FIRST OPAQUE batch's TextureHandle every frame; as the camera
moved and the front-to-back opaque sort reordered which group
landed at BatchData[0], the crystal's apparent texture flickered to
whatever sat first — typically the player character's body parts.
Fix: add `uniform int uDrawIDOffset` to the vertex shader, change
Batches[gl_DrawIDARB] → Batches[uDrawIDOffset + gl_DrawIDARB], and
set the uniform per-pass in WbDrawDispatcher (0 for opaque,
_opaqueDrawCount for transparent). Mirrors WorldBuilder's
BaseObjectRenderManager.cs line 845.
Tests: 1688/1696 passing (8 pre-existing physics/input failures
unchanged). N.5b conformance sentinel 94/94 clean.
Visual: Holtburg lifestone now renders with the spinning blue crystal
correctly composed over the pedestal. Other transparent content (glass,
particle effects, NPC clothing) is unaffected — the same uniform fix
applies globally and is correct for all transparent draws.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Captures the three deferred items from A.5 ship:
- ISSUE #52: lifestone visual missing (1-3 hours, fast win)
- ISSUE #54: JobKind plumbing through BuildLandblockForStreaming
(~30 min - 1 hour, worker-thread efficiency cleanup)
- ISSUE #53: Tier 1 entity-classification cache retry (~5-7 days,
biggest perf win remaining; needs animation-mutation audit before
designing to avoid the freeze-pose bug from the first attempt)
Doc covers: A.5 final state + 3 high-value gotchas, files to read,
per-priority detail with effort estimates and acceptance criteria,
what NOT to do, the first-30-minute workflow, and the full A.5
commit chain for reference.
Phase is sized ~1 week if all three priorities land. The audit
step on Tier 1 is the highest-leverage investment.
Tier 2 + Tier 3 (static/dynamic split + GPU compute culling) are
explicitly out-of-scope for this phase — separate multi-week phases
per docs/plans/2026-05-10-perf-tiers-2-3-roadmap.md.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes ISSUES.md #13. PlayerDescriptionParser now walks the full
trailer (Options1 / Shortcuts / HotbarSpells / DesiredComps /
SpellbookFilters / Options2 / GameplayOptions blob / Inventory /
Equipped) ported from holtburger events.rs:503-625 +
shortcuts.rs:13-34. The trickiest piece — gameplay_options — uses a
4-byte-aligned forward heuristic (TryHeuristicInventoryStart)
mirroring holtburger's find_inventory_start_after_gameplay_options.
Trailer walk wrapped in its own inner try/catch so a malformed
trailer cannot destroy upstream attribute/skill/spell/enchantment
data; new Parsed.TrailerTruncated flag distinguishes clean parse
from graceful-degradation parse, with diagnostic log under
ACDREAM_DUMP_VITALS=1.
GameEventWiring registers parsed Inventory + Equipped into
ItemRepository at login (acceptance criterion: ItemRepository.Count
> 0 after login, exercised by GameEventWiringTests). 20 PD parser
tests + 1 wiring acceptance test; 282/282 AcDream.Core.Net.Tests
pass.
Plan: docs/superpowers/plans/2026-05-10-issue-13-pd-trailer.md.
Roadmap update: F.5a (visible-at-login dev panels) added as the
first deliverable that actually consumes the new trailer data —
ImGui dev panels under ACDREAM_DEVTOOLS=1 binding to
AcDream.UI.Abstractions ViewModels.
13 task commits + 1 review-followup + 1 nit-fix + 1 roadmap = 16
commits on the branch.
Sub-phase under existing F.5 (Core panels) capturing the immediate
follow-up to ISSUES.md #13: now that PlayerDescriptionParser surfaces
the full trailer (Inventory / Equipped / Shortcuts / HotbarSpells /
DesiredComps / Options1+2 / SpellbookFilters) and GameEventWiring
populates ItemRepository at login, F.5a wires that data into minimal
ImGui dev panels under ACDREAM_DEVTOOLS=1 so it's observable in-game.
Establishes the binding pattern (AcDream.UI.Abstractions ViewModels →
ImGui renderer) that the eventual D.2b retail-skinned F.5 panels
reuse. Spec to brainstorm before code.
Per docs/plans/2026-05-10-perf-tiers-2-3-roadmap.md Tier 1: cache the
per-(entity, meshRef, batch) classification (TextureCache lookup,
GroupKey hash, _groups dict insert) so the per-frame Draw inner loop
becomes "look up cache → walk assignments → append matrix to group's
Matrices list."
For static entities (~95% of world: trees, rocks, buildings, scenery),
the answer never changes between frames. Cache once at first visit;
reuse permanently. Per-frame work for static drops from 4 expensive
operations per (meshRef, batch) to 1 list-append.
Estimated entity dispatcher: 3.5ms → ~1-1.5ms median at radius=12.
Should land inside the 2.0ms spec budget.
Implementation:
- New EntityClassificationCache class (per-meshRef list of cached
(group ref, baked-PartTransform) tuples) keyed by entity.Id.
- ClassifyEntity does the one-time work; result populates _groups and
the cache.
- Draw inner loop: cache lookup → for each assignment, model =
PartTransform × entityWorld; group.Matrices.Add(model).
- Cache miss when ClassifyEntity finds NO mesh loaded yet (Vao == 0)
→ don't store; retry next frame. Avoids cache thrash during the
streaming-in window.
- Public InvalidateEntity(uint id) + ClearEntityCache() for explicit
invalidation hooks. Wiring (palette swap on ObjDescEvent, MeshRefs
hot-swap) is post-A.5 follow-up — for now, cache-stale entities
show their pre-swap appearance until next respawn.
Tier 2 (static/dynamic split with persistent groups) and Tier 3 (GPU
compute culling) tracked in the roadmap doc.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After PlayerDescription is dispatched, the Inventory and Equipped lists
produced by the parser are now fed into ItemRepository via AddOrUpdate +
MoveItem so inventory/paperdoll panels see items after login.
Acceptance test PlayerDescription_RegistersInventoryEntries_InItemRepository
confirms ItemCount goes 0→2 for a synthetic PD with two inventory entries.
282 Net.Tests pass.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Captured 2026-05-10 during Phase A.5 polish discussion. User asked why
the 9070 XT @ 1440p doesn't hit Unreal-level FPS for an old game like
AC. Answer: architectural — we rebuild the entire draw plan from
scratch every frame instead of caching pre-baked static-world data.
Tier 1 (entity-classification cache) lands as A.5 polish (separate
commit). Tiers 2 + 3 documented here for future scheduling:
- Tier 2 — Static/dynamic split with persistent groups
~2-week phase. Static entities (~95% of world) get permanent GPU-
resident matrix slots, populated at spawn, dirty-tracked for delta
upload. Per-frame CPU cost for static = LB-cull + dirty-flag check
only. Estimated entity dispatcher: 3.5ms → 0.5-1ms median.
400-600 FPS at standstill, radius=12.
- Tier 3 — GPU-side culling (compute pre-pass)
~1-month phase. Per-instance frustum cull moves to GPU compute
shader. Compute writes draw-indirect buffer; rasterizer reads it.
Estimated CPU dispatcher: ~0.05ms (essentially free).
600-1000+ FPS at standstill, radius=12.
Doc captures effort estimates, sub-decisions, risks, mitigations, and
scheduling triggers for each tier. Also notes the architectural
ceiling (~800-1500 FPS for a C# + GL client; reaching native engine
performance requires becoming a different engine).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
T17's WalkEntities helper allocated a fresh List<(WorldEntity, int)>
per frame to hold the (entity, meshRefIndex) pairs that pass visibility
filters. At ~10K entities × ~3 mesh refs = ~30K tuples × 16 bytes =
~480 KB / frame of GC pressure on the render thread. The implementer's
self-review flagged this as a future N.6 optimization; the post-T26
diagnostic showed it materially contributing to the perf regression
(though Bug A — far-tier entity load — was the dominant factor).
Refactor: split WalkEntities into two overloads.
- WalkEntities(...) — test-friendly, allocates a fresh ToDraw list per
call. Tests keep using this signature unchanged.
- WalkEntitiesInto(..., scratch, ref result) — no-alloc, clears + populates
a caller-provided scratch list. Draw uses this with a per-dispatcher
_walkScratch field reused across frames.
Test count unchanged (40 streaming + 8 bucketing tests still pass).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase A.5's two-tier streaming spec promised that far-tier landblocks
ship terrain ONLY — no entities, no scenery, no interior cells. T13/T16
wired the controller side (RecenterTo emits ToLoadFar/ToLoadNear/ToPromote;
controller passes JobKind to the worker), but the worker's HandleJob
never branched on Kind: every load called BuildLandblockForStreaming
which runs the full hydration + scenery generation + interior cell path.
Result: at default radii (N₁=4 / N₂=12), 540 far-tier LBs each loaded
their full entity layer (~132 entities/LB → ~71K entities total) into
GpuWorldState. The dispatcher then walked all ~54K entities per frame
(post-frustum-cull), driving the entity dispatcher cpu_us from ~3.6ms
median (T24 baseline) to ~18-21ms (post-T22.5 horizon-test). User-
observed: 40 FPS / 25ms frame time at horizon-safe settings; system
crash at full High preset.
Minimum-diff fix: in LandblockStreamer.HandleJob, after
_loadLandblock returns, strip Entities to empty for LoadFar before
posting Loaded. Worker still does wasted hydration CPU (off the render
thread, harmless). Render-side dispatcher walk drops from ~54K to ~10K
entities/frame.
Math: post-fix entity dispatcher should drop to ~3-4ms median at N₁=4 /
N₂=12 (matches T24's 3.6ms at radius=5 single-tier, since N₁=4 has 33%
fewer near entities than N₁=5).
Future optimization (N.6 / A.6): plumb JobKind through
BuildLandblockForStreaming so the worker also skips the wasted CPU.
Out of A.5 scope.
Bug B (T17 WalkEntities allocation) is a smaller perf hit — defer if
post-Bug-A FPS is acceptable.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
GameWindow.OnLoad resolves QualitySettings.From(_persistedDisplay.Quality)
+ WithEnvOverrides() immediately after LoadAndApplyPersistedSettings, stores
result in _resolvedQuality field. All six quality dimensions applied:
- NearRadius / FarRadius: replace old T16 env-var-only block; preset drives
the radii, legacy ACDREAM_STREAM_RADIUS override still honoured.
- MsaaSamples: WindowOptions.Samples reads from startup quality resolution
in Run() (pre-window-create read from SettingsStore). MSAA cannot change
at runtime; ReapplyQualityPreset logs a restart-required warning if the
new preset would change it.
- AnisotropicLevel: TerrainAtlas.SetAnisotropic() called after Build() and
again in ReapplyQualityPreset. Temporarily removes bindless residency
before the GL TexParameter call, re-makes resident after.
- AlphaToCoverage: WbDrawDispatcher.AlphaToCoverage property gates the
glEnable/glDisable(SampleAlphaToCoverage) pair around the opaque pass.
- MaxCompletionsPerFrame: set on StreamingController after construction
and after each mid-session restart.
ReapplyQualityPreset(QualityPreset) method handles mid-session changes
(Settings panel Quality dropdown Save): rebuilds streamer + controller for
radius changes, toggles A2C and aniso immediately, logs MSAA restart caveat.
onSaveDisplay callback updated to call ReapplyQualityPreset when Quality
field changes.
TerrainModernRenderer.Atlas property added to expose the atlas for
mid-session aniso updates.
991 tests passing, 8 pre-existing failures unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add QualityPreset enum + QualitySettings readonly record struct with
From(preset) table and WithEnvOverrides() env-var override layer.
Four presets (Low/Medium/High/Ultra) drive NearRadius, FarRadius,
MsaaSamples, AnisotropicLevel, AlphaToCoverage, MaxCompletionsPerFrame.
Env vars (ACDREAM_NEAR_RADIUS, ACDREAM_FAR_RADIUS, ACDREAM_MSAA_SAMPLES,
ACDREAM_ANISOTROPIC, ACDREAM_A2C, ACDREAM_MAX_COMPLETIONS_PER_FRAME)
override individual preset fields for dev spot-testing.
DisplaySettings gains a Quality: QualityPreset field (default High);
SettingsStore persists/loads it under display."quality" as an enum
name string with Enum.TryParse fallback. 12 new QualityPresetTests
cover the preset table (radii, msaa, aniso, a2c, completions) and all
six env-var override paths. 415 UI.Abstractions tests passing.
Wiring into GameWindow / WbDrawDispatcher / TerrainAtlas follows in
commit 2 of this task.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per Phase A.5 spec §2 acceptance criterion 6: entity dispatcher median
≤ 2.0ms; terrain dispatcher median ≤ 1.0ms at standstill. When the
median exceeds the budget, prefix the DIAG line with " BUDGET_OVER" so
the regression is grep-friendly during perf testing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per Phase A.5 spec §4.8: fog ramp is tuned to mask the N₁ scenery
boundary. FogStart = N₁ × 192m × 0.7 ≈ 538m at default radii (4/12).
FogEnd = N₂ × 192m × 0.95 ≈ 2188m. Multipliers exposed as env vars for
fast iteration during visual gate.
Override is injected into the UBO after SceneLightingUbo.Build() so fog
color, lightning flash and mode still come from the sky keyframe. Adds
ParseEnvFloat helper (InvariantCulture) for float env-var parsing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code-quality review followup on Task 2 (becbde6) — addresses I1 (the
forward-looking concern that Tasks 3-9's inner-catch will leave partial
lists visible to callers with no signal) and M1 (silent inner catch).
Changes:
- Parsed gains a trailing `bool TrailerTruncated` field. Both
construction sites pass `false` by default; the trailer try/catch
flips a local `trailerTruncated` to `true` on FormatException and
feeds it into the final return.
- Inner catch logs `pos`/`payload.Length`/exception message under
ACDREAM_DUMP_VITALS=1, mirroring the outer catch's diagnostic
pattern.
- Task 2 test strengthened to assert defaults on Options2 /
SpellbookFilters / HotbarSpells / DesiredComps / GameplayOptions /
Equipped + TrailerTruncated=false (M2 followup — gives Tasks 3-9
a regression guard if they consume into the wrong field).
- New test `TryParse_TrailerAbsent_LessThan8BytesAfterEnchantments_*`
documents the contract that <8 bytes after enchantments means the
trailer is absent (not truncated): TrailerTruncated stays false,
upstream attribute data survives.
- Plan updated in lockstep so Tasks 3-11 implementers see the
`trailerTruncated` local and the new return-arg position.
271/271 AcDream.Core.Net.Tests pass.
Per Phase A.5 spec §4.9.2: ClipMap foliage uses binary alpha-cutoff.
At N₂=12 horizon distance the pixel-stepped silhouettes are visible.
A2C with MSAA 4x produces smooth retail-faithful tree edges.
GL context now requests Samples=4. WbDrawDispatcher's opaque pass
toggles GL_SAMPLE_ALPHA_TO_COVERAGE on/off around the multi-draw
indirect call. mesh_modern.frag's opaque pass now discards only
truly-empty (α<0.05) so the GPU derives sample mask from coverage;
transparent pass boundary logic is unchanged.
MSAA audit: no custom FBOs found — all rendering uses default
framebuffer. Sky/particles/ImGui are all MSAA-compatible.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per Phase A.5 spec §4.9.1: at N₂=12 distant terrain LBs occupy a few
pixels on screen and shimmer (texel-swap aliasing) without mipmaps.
Generate mips after atlas upload; sampler trilinear + 16x anisotropic.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per Phase A.5 spec §4.6 Change #2: WalkEntities's per-entity AABB
frustum cull was recomputing Position±5 per frame per entity. With
~10.7K entities (N1=4) at 240 FPS that is ~2.5M wasted Vector3
ops/sec.
Read the AABB from the WorldEntity cache (T8 schema) instead.
RefreshAabb runs lazily on AabbDirty=true. Populate at register time:
- LandblockLoader.BuildEntitiesFromInfo: RefreshAabb after each new
WorldEntity construction (stabs + buildings). Refactored from
inline object-initializer to named variable to enable the call.
- EntitySpawnAdapter.OnCreate: RefreshAabb after entity state init
(position/rotation already set via the WorldEntity passed in).
Dynamic entities (NPCs, players) move every frame via direct
Position writes in GameWindow.cs. Migrated all three per-frame
write sites to SetPosition() (T8 mutator) so AabbDirty propagates:
- line 5942: player entity render position update
- line 6951: remote animated entity interpolated path
- line 7279: remote animated entity landing/movement path
The lazy RefreshAabb in WalkEntities catches up on the next frame
after any SetPosition call — render thread only, no races.
Build green, 986 passed / 8 pre-existing failures unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
First step of the PD trailer walk. Wraps trailer reads in their own
try/catch so a malformed trailer does not null out the upstream
attribute/skill/spell/enchantment data we already extracted.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Per Phase A.5 spec §4.6 Change #1: when an LB is invisible AND
animatedEntityIds is non-empty, the inner loop walked every entity
in the LB just to find the few animated ones. At ~10.7K entities
(N1=4) that is wasted iteration cost per frame.
Extracted a pure-CPU internal static WalkEntities helper. When LB
is invisible: iterate animatedEntityIds directly and look each up
in a per-LB AnimatedById dictionary (typically <50 animated vs
~10K total). When LB is visible: walk all entities as before.
GpuWorldState.LandblockEntries now yields an AnimatedById map as a
5th tuple field alongside the AABB tuple. Dictionary is built on
each yield (cheap — ~132 entities/LB max). A caching layer is out
of A.5 scope.
WbDrawDispatcher.Draw signature updated to consume the 5-tuple.
GameWindow.cs call site passes _worldState.LandblockEntries which
now yields the 5-tuple — no change needed there.
8 new tests in WbDrawDispatcherBucketingTests cover T17 Change #1
(invisible LB / animated set / neverCull / null frustum) and
T18 Change #2 guard tests (cached AABB / dirty flag / animated bypass).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code review nit-fix on top of d3b58c9 — addresses two issues from the
quality review of Task 1:
I1 (Important): the record struct `Shortcut` was a homograph with the
flag member `CharacterOptionDataFlag.Shortcut`. Both names live inside
`PlayerDescriptionParser`'s scope. Rename to `ShortcutEntry` aligns
with `InventoryEntry`/`EquippedEntry` and removes the trap before
Task 3's walker references both names in the same method body.
M2 (Minor): `EquippedEntry` had no holtburger source citation; added
one referencing events.rs:180-190. Also expanded `InventoryEntry`'s
comment with the strict reader's validation reference.
Plan doc updated in lockstep so Task 3+ implementers see the new name.
8/8 PlayerDescriptionParser tests still pass.
Commit 19b4465's new ToPromote test pre-loaded an LB with a non-
canonical id (low 16 bits 0x0FFF instead of 0xFFFF). The new
canonicalization in AddEntitiesToExistingLandblock then key-missed and
parked the entity in the pending bucket instead of merging — assertion
failed.
Use canonical id 0x3232FFFFu directly. The test now exercises the
intended hot-path (merge into existing LB), not the cold pending-bucket
fallback (which is exercised by GpuWorldStateTwoTierTests).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Commit 19b4465 broke build by omitting required-member init for
SourceGfxObjOrSetupId/Position/Rotation in the new ToDemote/ToPromote
tests. WorldEntity has [required] on those fields (CS9035). The lone
test run that reported 38 passing used pre-existing binaries built
before this break.
Added all three required initializers (zero / Identity defaults — these
test the routing path; entity content doesn't matter).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code review on T13-T16 bundle (commits fb10c3f/aff35d2/b8d80fe/c4fd373/31d312a)
flagged 3 Important + 2 test-coverage gaps. Apply all 5:
Important #1: GpuWorldState.AddEntitiesToExistingLandblock didn't
canonicalize landblockId. Streaming callers always pass canonical
0xAAAA0xFFFF ids, but the public API silently key-missed for callers
that mirror AppendLiveEntity's cell-resolved-id pattern. Both new
methods now canonicalize the id on entry.
Important #2: RemoveEntitiesFromLandblock asymmetry with RemoveLandblock
re: persistent-entity rescue. Documented as intentional — demote-tier
entities are atlas-tier only (procedural scenery, dat-static stabs/
buildings; never ServerGuid != 0); the local player and live server
spawns live in their LB via RelocateEntity per frame and aren't
affected by atlas-layer demote.
Important #3: StreamingController.NearRadius / FarRadius were { get; set; }
but mutating them after the first Tick is a no-op (StreamingRegion
snapshots the values). Switched to { get; } only with XML doc warning.
Test gap #1: ToDemote routing through Tick — added test that walks
the player past hysteresis and asserts entities drop while terrain
stays.
Test gap #2: Promoted result routing through Tick — added test that
enqueues a Promoted and asserts AddEntitiesToExistingLandblock fires.
Deferred Minor: dead _streamingRadius write + style consistency on
fully-qualified IReadOnlyList — non-load-bearing, can roll into a later
cleanup.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
No behavior change yet — adds CharacterOptionDataFlag, Shortcut/Inventory/
EquippedEntry records, and extends Parsed with trailer fields filled with
empty defaults. Sets up the per-section TDD walk in subsequent commits.
Cosmetic follow-up flagged by spec compliance review on T13-T16 bundle
(commits fb10c3f / aff35d2 / b8d80fe / c4fd373). The debug overlay's
getStreamingRadius callback was reading _streamingRadius — the legacy
single-tier field that's only updated by ACDREAM_STREAM_RADIUS. Operators
using the new ACDREAM_NEAR_RADIUS / ACDREAM_FAR_RADIUS env vars would
see the overlay frozen at the default 2.
Switch to _nearRadius. The overlay still shows a single number (matching
its label "Streaming radius"); operators who want both tier numbers can
read the launch log.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
GameWindow now constructs StreamingController with nearRadius / farRadius
defaults of 4 / 12 (per spec acceptance criterion). Env vars:
- ACDREAM_NEAR_RADIUS (default 4)
- ACDREAM_FAR_RADIUS (default 12)
- ACDREAM_STREAM_RADIUS (legacy; if set, treats as nearRadius and
bumps farRadius to max(stream, default))
Fields _nearRadius / _farRadius added alongside legacy _streamingRadius
(kept so the debug overlay's getStreamingRadius callback stays valid).
ApplyLoadedTerrainLocked routes to TerrainModernRenderer.AddLandblockWithMesh
(T15) instead of AddLandblock directly, making the two-tier entry point
the canonical call path.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the single-radius Tick with a two-tier model that consumes
StreamingRegion's TwoTierDiff (5-list) and routes to the appropriate
JobKind:
- ToLoadFar -> _enqueueLoad(id, LoadFar)
- ToLoadNear -> _enqueueLoad(id, LoadNear)
- ToPromote -> _enqueueLoad(id, PromoteToNear)
- ToDemote -> _state.RemoveEntitiesFromLandblock(id) on render thread
- ToUnload -> _enqueueUnload(id)
Drain switch handles Loaded (terrain + entity layer), Promoted (entity
layer only -- terrain already loaded), Unloaded, Failed, WorkerCrashed.
Constructor signature: nearRadius/farRadius separate ints. Old single-
radius ctor removed; existing single-radius tests updated to pass
nearRadius=farRadius for backward-compat coverage.
GameWindow's enqueueLoad lambda updated from (id =>...) to (id, kind) =>
to match new Action<uint, LandblockStreamJobKind> signature; radius: arg
renamed to nearRadius:/farRadius: (both set to _streamingRadius until T16
wires the full two-tier env-var parsing).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
T13 routes worker-built meshes from LandblockStreamResult.Loaded.MeshData
into the renderer. AddLandblockWithMesh accepts a prebuilt mesh + origin
and delegates to the existing AddLandblock(uint, LandblockMeshData, Vector3)
so both paths share one upload path (Approach B -- AddLandblock already
takes a prebuilt mesh; no inline build to extract).
GameWindow's T16 lambda captures liveCenterX/Y and passes the derived
origin; the renderer stays origin-agnostic.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two new methods on GpuWorldState, used by two-tier streaming (T13):
- RemoveEntitiesFromLandblock(id): drop all entities from an LB while
keeping the terrain. Used for Near->Far demote (player walks past the
inner ring; LB stays loaded but entities leave).
- AddEntitiesToExistingLandblock(id, entities): merge new entities into
an already-loaded LB record. Used for Far->Near promote (terrain is
already on the GPU; just streaming the entity layer in).
Falls back to the pending bucket if the LB hasn't loaded yet.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code review on T10-T12 bundle (commits 0cf86bb/00bb030/0405947 + audit
fix 76e1a64) found 3 Important issues:
1. LandblockStreamer.Start() had an idempotency race — the XML doc
claimed thread-safety but the implementation checked _worker != null
before assigning, allowing two callers to both pass the check and
spawn duplicate worker threads. Fixed via Interlocked.CompareExchange.
2. No test verified the worker emits Failed when buildMeshOrNull returns
null. Added Load_WhenBuildMeshReturnsNull_ReportsFailed.
3. StreamingControllerTests.cs:81 used MeshData: default! when
constructing a Loaded result. If a future test flows MeshData
through the apply callback, the null reference would NRE rather
than producing a meaningful assertion failure. Replaced with a real
empty LandblockMeshData instance.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Spec compliance review on T10-T12 bundle (commits 0cf86bb/00bb030/0405947)
caught 2 unprotected dat reads that the original T10 audit missed:
- GameWindow.UpdatePlayerAnimation (line ~7546): reads Setup when the
player entity is missing from _animatedEntities (post-respawn pattern).
- GameWindow.EnterPlayerModeNow (line ~8567): reads Setup when entering
player mode to derive StepUpHeight / StepDownHeight from the dat.
Both run on the render thread post-_streamer.Start(), so they can race
with the worker thread's BuildLandblockForStreamingLocked. DatBinReader's
shared buffer position would corrupt — same class of "ball with spikes"
bug the original Phase A.1 hotfix addressed.
Wrap both reads in lock (_datLock).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the T7-temporary default! MeshData placeholder. Streamer
now takes Func<uint, LoadedLandblock?, LandblockMeshData?> at
construction; the worker calls it after _loadLandblock succeeds and
passes the pre-built mesh into LandblockStreamResult.Loaded.
GameWindow's buildMeshOrNull factory takes the already-loaded
LoadedLandblock (lb.Heightmap is the LandBlock dat object), so no
additional dat read is needed — _heightTable and _blendCtx are
read-only after init, _surfaceCache is ConcurrentDictionary (T9).
Zero dat lock needed inside the mesh-build closure.
StreamingController._applyTerrain delegate signature widened to
Action<LoadedLandblock, LandblockMeshData> so the pre-built mesh
flows render-thread-side via the Loaded result. ApplyLoadedTerrainLocked
now accepts meshData and calls _terrain.AddLandblock directly, skipping
the per-frame LandblockMesh.Build that previously ran on the render
thread (~5ms per LB at radius=12 first traversal).
StreamingControllerTests updated: all four applyTerrain lambdas
adapted to the two-arg Action signature.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase A.1 reverted to synchronous mode due to DatCollection thread-
safety; T10 documented the lock that makes concurrent reads safe. T11
activates the dedicated worker thread and switches enqueue methods
to non-blocking Channel.Writer.TryWrite.
EnqueueLoad now takes LandblockStreamJobKind (default: LoadNear from
all callers, matching previous full-load semantics). T13/T16 will
route by kind per TwoTierDiff.
Constructor gains optional buildMeshOrNull param (defaults to null-
returning stub); T12 wires the real LandblockMesh.Build factory.
GameWindow construction site updated: Action<uint> enqueueLoad
delegate now wraps a lambda (method group won't bind to Action<uint>
when the method has an optional second param).
LandblockStreamerTests updated: the synchronous-thread-pinning test
replaced by Load_ExecutesLoaderOnWorkerThread which asserts the
loader runs on a different thread; Load_FollowedByDrain now supplies
a stubMesh so the worker can produce Loaded (not Failed) results.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase A.5 T11 activates the LandblockStreamer worker thread, making
concurrent dat reads possible. DatReaderWriter's DatBinReader uses a
shared buffer position internally — concurrent _dats.Get<T> calls from
worker + render thread corrupt that state and produce half-populated
LandBlock.Height[] arrays (renders as wildly distorted terrain).
The _datLock field already existed from the Phase A.1 hotfix, and the
high-traffic worker-facing paths (BuildLandblockForStreaming,
ApplyLoadedTerrain, OnLiveEntitySpawned) already hold it. This commit
updates the field comment to precisely document the T10 contract:
all worker-thread dat reads enter via factory closures that acquire
_datLock; render-thread paths are already covered by their outer
lock wrappers.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
