Adds EntityClassificationCache.DebugCrossCheck(entityId, liveBatches) that
asserts cached state matches a live re-classification. Wires a simpler
predicate assert into WbDrawDispatcher's cache-hit branch (asserts
isAnimated == false on cache hit). Tests #13a and #13b cover the
batch-count mismatch and clean-match cases via a custom TraceListener
that captures Debug.Assert calls.
Zero cost in Release. In DEBUG, the assert fires immediately if a future
regression mutates static-entity state outside the audit's known write
sites — the same failure mode that bit the prior Tier 1 attempt.
Phase 4 complete. Cache + invalidation + safety net all in place.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
GpuWorldState.RemoveEntitiesFromLandblock now invokes an optional
Action<uint> callback before zeroing the entity list. GameWindow wires
this to EntityClassificationCache.InvalidateLandblock so cache entries
get swept on LB demote (Near to Far) and unload. Per spec section 5.3 W3b.
The callback receives the canonicalized landblock id (low 16 bits forced
to 0xFFFF), matching the LandblockHint stored at Populate time. Trace:
GpuWorldState._loaded keys are canonical (set by AppendLiveEntity),
LandblockEntries yields kvp.Key as LandblockId, WalkEntitiesInto
propagates entry.LandblockId into _walkScratch, the dispatcher's
populateLandblockId reads that tuple and stores it as LandblockHint.
Phase 3 (invalidation hooks) complete. The cache now stays correct across
all spec-identified mutation events: despawn, ObjDescEvent (despawn+
respawn), LB demote, LB unload.
Two integration tests added:
- RemoveEntitiesFromLandblock_FiresUnloadCallbackWithCanonicalId asserts
the callback fires once with the canonical id even when called with a
cell-resolved input (low 16 bits non-FFFF).
- RemoveEntitiesFromLandblock_NotLoaded_DoesNotFireCallback asserts the
early-return path doesn't fire the callback for unknown landblocks.
Tests: 1706 passed / 8 failed (baseline). Sentinel: 110/110.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
GameWindow.RemoveLiveEntityByServerGuid now invalidates the entity's
cache entry next to the existing _animatedEntities.Remove(). Fires for
DeleteObject (0xF747) and the dedup leg of ObjDescEvent (0xF625).
Adds test #15 (despawn-respawn under reused id repopulates fresh) per
spec section 7.5 — pins the audit's ObjDescEvent-as-despawn-respawn contract.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Task 10 (commit 0cbef3c) called ApplyCacheHit inside the per-(entity, partIdx)
foreach loop, but cachedEntry.Batches is flat across all MeshRefs of the
entity. For a 3-MeshRef static building on frame 2: 3 tuples times 6 cached
batches per call = 18 instances drawn instead of 6. Severe Z-fighting and
3x perf hit on every multi-part static entity (buildings, statues, multi-
MeshRef NPCs).
This is the symmetric mirror of the Task 9 bug fixed at 00fa8ae. Both
spec section 5.2 and the plan describe the foreach as per-entity, but
_walkScratch has been per-tuple since Task 6. The implementation
faithfully ported the buggy spec.
Fix: track lastHitEntityId; the cache-hit fast path fires only on the
first tuple of each entity, and subsequent tuples skip the iteration
body via continue. Adds a regression test pinning the per-entity
amplification invariant.
Caught by code review (subagent-driven-development) before Phase 3
dispatched. The bug was invisible in the no-multi-frame-test 1702/8
baseline; would have manifested as visible Z-fighting on every multi-
part building on second-and-subsequent frames once Task 13 perf gate
captured live runs.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
WbDrawDispatcher.Draw now branches on cache hit before running classification:
on hit, walks the cached flat batch list and appends RestPose times entityWorld
to the matching groups; on miss, runs today's classification and populates
the cache (Task 9). Animated entities skip the cache entirely.
Adds dispatcher integration tests #11 (static entity populates + reuses)
and #12 (animated bypasses) per spec test plan section 7.2, plus the
multi-MeshRef regression test that would have caught the bug fixed in
commit 00fa8ae (cache populate must flush at entity boundary, not per-tuple).
Phase 2 (dispatcher integration) complete. End-to-end caching now live.
Invalidation hooks (Phase 3) ensure correctness across despawns + LB demotes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Task 9 (commit 2f489a8) called _cache.Populate inside the per-tuple
foreach loop, but _walkScratch contains one tuple per (entity, MeshRefIndex)
and the cache is keyed by entity.Id. For multi-MeshRef entities (multi-part
Setup buildings, statues, multi-MeshRef NPCs), each iteration's Populate
OVERWROTE the previous one — only the last MeshRef's batches survived.
The bug was invisible at commit time because Task 10 had not landed
(cache populates but isn't read). It would have manifested the moment
Task 10 wired the cache-hit fast path: every multi-part static building
in Holtburg would render as N stacked copies of its last part.
Fix: restructure the per-entity loop with a flush-on-entity-change pattern.
Track the previous entity's Id; when the iteration moves to a different
entity, flush the previous entity's accumulated _populateScratch via one
Populate call. After the loop, flush the final entity. _populateScratch
is now cleared at flush time, not per-iteration.
Caught by code review (subagent-driven-development) before Task 10 dispatched.
Verified: 1699/8 baseline preserved, sentinel 105/105 unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Restructures Draw's per-entity loop: animated entities still skip the
cache entirely, but static entities now collect their classification into
_populateScratch and call cache.Populate at the end of the iteration.
Cache fast-path (skip slow classification on cache hit) lands in Task 10.
This intermediate state is verifiable: behavior unchanged, but the cache
is being populated as entities render. Diagnostic-friendly split.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ClassifyBatches now accepts a restPose parameter (the model-matrix
component without entityWorld baked in) and an optional collector. When
collector is non-null, each classified batch is appended as a CachedBatch
record. Defaults preserve today's behavior. Used in Task 9 to populate
the cache on a static-entity miss.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the cache as a constructor parameter on WbDrawDispatcher and a
private field on GameWindow. The cache is passed through but not yet
consumed by Draw — that wires up in Task 9 (cache miss / populate) and
Task 10 (cache hit / fast path).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extends the walk scratch tuple from (entity, meshRefIndex) to
(entity, meshRefIndex, landblockId). The dispatcher's per-entity loop now
has the landblock id available for EntityClassificationCache.Populate's
landblockHint argument (consumed in Task 9). No behavior change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Idempotent removal of a cached entry by entity id. Tests #4 and #5 from
spec section 7.1 lock in the contract.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements Populate (insert-or-overwrite) and adds 5 tests covering the
populate->TryGet round-trip including the Setup pre-flatten shape. Per
spec test plan section 7.1 tests #2, #3, #9, #10, #14.
Tests use xUnit Assert.* (not FluentAssertions) to match the Task 2
implementer's choice and the existing 149 sibling assertions in the Wb
test directory.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds CachedBatch, EntityCacheEntry, and EntityClassificationCache with
just TryGet (returns false on empty). The skeleton compiles and the first
test (TryGet_EmptyCache_ReturnsFalse) passes. Subsequent tasks add
Populate, InvalidateEntity, InvalidateLandblock, and the dispatcher
integration. Per spec design Section 6.1.
Note: CachedBatch / EntityCacheEntry / EntityClassificationCache are
internal (not public as the plan snippet showed). Their members
transitively reference the internal GroupKey type, so promoting them to
public produces CS0051 inconsistent-accessibility errors. The cache is
dispatcher-internal coordination state anyway, and the AcDream.App
csproj already exposes internals to AcDream.Core.Tests via
InternalsVisibleTo, so the test sees everything it needs.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mechanical refactor: GroupKey was a private nested record struct on
WbDrawDispatcher. The upcoming EntityClassificationCache (ISSUE #53) needs
to store GroupKey inside CachedBatch records, so it must be visible to
both the dispatcher and the cache. Promoting to internal at file scope is
the smallest change that achieves this.
No behavior change. 1688 tests pass; 8 pre-existing failures unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bug A's fix (commit `9217fd9`) patched at the worker output by stripping
entities from far-tier `LoadedLandblock`s after the full `LoadNear` path
ran. The worker still wasted CPU on `LandBlockInfo` reads + entity
hydration + `SceneryGenerator` math + interior-cell walks for ~544
far-tier LBs at radius=12, just to throw the work away.
This commit plumbs `LandblockStreamJobKind` through to the factory so the
worker can branch at the source:
- `LandblockStreamer.cs`: replace the `Func<uint, LoadedLandblock?>`
factory with `Func<uint, LandblockStreamJobKind, LoadedLandblock?>` as
the primary ctor signature. Add a back-compat overload that wraps the
old single-arg signature (`(id, _) => loadLandblock(id)`) so existing
test code keeps compiling without modification — the 5 ctor sites in
`LandblockStreamerTests.cs` now resolve to the overload. `HandleJob`
passes `load.Kind` to the factory; the post-load entity-strip is
retained as a `Debug.Assert` + Release safety net.
- `GameWindow.cs`: `BuildLandblockForStreaming(uint, JobKind)` branches
on `kind == LoadFar` at the top — reads only the `LandBlock` heightmap
dat and returns a `LoadedLandblock` with `Array.Empty<WorldEntity>()`.
Skips `LandblockLoader.Load` (which reads `LandBlockInfo`),
`BuildSceneryEntitiesForStreaming`, and `BuildInteriorEntitiesForStreaming`
entirely. Near-tier path is unchanged. Both call sites updated to pass
the kind through the lambda: `(id, kind) => BuildLandblockForStreaming(id, kind)`.
Tests: 1688/1696 (8 pre-existing physics/input failures unchanged).
Streaming-targeted filter (30 tests covering LandblockStreamer +
StreamingController + StreamingRegion) all green via the back-compat
overload — no test code needed updating.
Per-LB worker cost on far-tier: was ~tens of ms (full hydration,
including LandBlockInfo + scenery generation + interior cells); now a
single `LandBlock` dat read (~sub-ms).
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>
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>
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.
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>
