Commit graph

8 commits

Author SHA1 Message Date
Erik
2163308032 #119 ROOT CAUSE: interior-id X-byte collision + player-landblock cache hints = cross-entity batch serving
The decisive probe (3cf6bcc) caught it live in ONE session: a 43-part
staircase entity (src=0x020003F2, healthy MeshRefs tZ=[0.35..15.15])
drew with cache=hit:3 restZero=3 - THREE batches belonging to a 1-part
entity - then under a different hint the correct hit:119. Two
compounding bugs:

1. interiorIdBase = 0x40000000 | (landblockId & 0x00FFFF00) resolved to
   0x40YYFF00 for landblock keys 0xXXYYFFFF - the landblock X byte
   DISCARDED. Every landblock in a map Y-row shared one id space:
   Holtburg town A9B3's 9th interior stab == the AAB3 tower's spiral
   staircase, both 0x40B3FF09. Fixed to 0x40000000|(lbX<<16)|(lbY<<8)
   (the scenery 0x80XXYY## scheme).

2. The Tier-1 classification cache's #53 tuple key (EntityId,
   LandblockHint) was fed the PLAYER's landblock at bucket-draw time
   (RetailPViewRenderer.DrawEntityBucket fabricates its tuple with
   ctx.PlayerLandblockId), so colliding ids from different landblocks
   shared a key: whichever entity classified first under a hint won,
   and the loser wore its batches all session (static fast path never
   re-classifies). Also: bucket-hinted entries were never swept by
   InvalidateLandblock(owner) - stale entries survived owner unload.
   Fixed: ResolveCacheLandblockHint derives the hint from the entity's
   owning cell (ParentCellId landblock, canonical 0xXXYYFFFF), falling
   back to the tuple id for ownerless paths (outdoor stabs/scenery,
   where the tuple IS the owner).

Explains the session-shaped repro exactly: town-login + run to the
tower hydrates/classifies town interiors first -> the tower staircase
cache-hits the town twin's batches (stairs missing/partial + a wrong
object near the floor - the "water barrel"); login-inside classifies
the tower first -> usually clean. meshMissing=0 / entSeen==entDrawn
both ways (everything draws, wrong batches). Likely also feeds #113's
distance-dependent phantom staircase (the town twin wearing the
tower's staircase batches).

3 new cache tests pin the collision contract + hint derivation.
Suites: App green / Core 1430+2skip / UI 420 / Net 294.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-06-11 21:43:45 +02:00
Erik
95ebbf3004 fix(render #53): key cache by (entityId, landblockHint) to defeat ID collision
User confirmed via A/B test (ACDREAM_DISABLE_TIER1_CACHE=1) that the
visual bug — buildings rendering up in the air outside Holtburg — is in
the cache wiring, not elsewhere. The matrix math (restPose * entityWorld
== model) was provably correct, so the bug had to be cache key collision.

Stabs were namespaced in commit 71d0edc, but scenery (0x80LLBB00 +
localIndex) and interior (0x40LLBB00 + localCounter) still have the
same 256-overflow risk. Dense LBs outside Holtburg (forest, urban) push
localIndex past 255, wrapping into the lbY byte and creating cross-LB
collisions.

Fix: change the cache key from uint entityId to (uint, uint) tuple of
(EntityId, LandblockHint). The cache is now correct-by-construction
regardless of any hydration path's Id-generation strategy. Defensive
against future regressions in any ID namespace.

InvalidateEntity becomes a sweep (was O(1)), but it's called rarely
(only on live-entity despawn). InvalidateLandblock was already a sweep.

Updated 14 existing cache tests + 1 dispatcher integration test to thread
landblockHint through TryGet / DebugCrossCheck calls.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-10 23:02:14 +02:00
Erik
f16604b60b feat(render #53): DEBUG cross-check guards against the prior Tier 1 bug class
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>
2026-05-10 19:43:24 +02:00
Erik
1d1afcd562 feat(render #53): wire EntityClassificationCache.InvalidateEntity at despawn
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>
2026-05-10 19:22:50 +02:00
Erik
a171e7007b feat(render #53): EntityClassificationCache.InvalidateLandblock + tests
Sweep-by-landblock removal for the streaming demote/unload path. Tests
#6, #7, #8 from spec section 7.1 lock in: (a) all matching entries removed,
(b) non-matching entries preserved, (c) idempotent on missing LB.

Phase 1 (cache foundation) complete. 11 cache tests passing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-10 17:47:57 +02:00
Erik
aea4460eae feat(render #53): EntityClassificationCache.InvalidateEntity + tests
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>
2026-05-10 17:42:09 +02:00
Erik
694815c499 feat(render #53): EntityClassificationCache.Populate + roundtrip tests
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>
2026-05-10 17:34:48 +02:00
Erik
773e9703da feat(render #53): EntityClassificationCache skeleton + first test
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>
2026-05-10 17:23:37 +02:00