Trace showed local arrival landing at 3.025 m from a target with
objDist=3.00. The previous arrival used ArrivalEpsilon=0.05 to
EXPAND the threshold (dist <= 3.05), so the body stopped right at
the boundary. ACE's server-side WithinUseRadius is strict
(dist <= radius), so 3.025 > 3.00 — ACE rejected the re-sent
action and looped back to MoveToObject. User had to manually
re-press R because auto-arrival kept landing just-outside-range.
Fix: walk INSIDE ACE's radius by a safety margin (0.3–0.5 m, capped
at 40 % of threshold so tight pickup radii like 0.6 m stay
reachable).
arrivalThreshold = wire's distanceToObject or minDistance
safetyMargin = min(0.5, arrivalThreshold * 0.4)
effectiveArrival = max(arrivalThreshold - safetyMargin, 0.1)
Examples:
objDist=3.00 (NPC) → walk to ≤2.50 m (ACE happy at 3.0)
objDist=2.00 (door) → walk to ≤1.50 m
objDist=0.60 (item) → walk to ≤0.36 m
objDist=0.50 (small) → walk to ≤0.30 m
Flee case (moveTowards=false) keeps its original predicate with
+ArrivalEpsilon — that's the boundary check semantics for fleeing
to a min-distance, not a max-distance use radius.
Previous re-send-on-arrival didn't actually unstick the action. Trace
showed ACE replying to the re-sent Use with another MoveToObject —
i.e. ACE's Player.Location was still the pre-walk position, so the
'I'm in range' fast-path didn't fire.
Cause: packet ordering. OnAutoWalkArrivedReSendAction was firing the
re-send immediately (sub-frame), BEFORE the next per-frame
AutonomousPosition heartbeat. ACE processed the Use against stale
location data.
Fix: SendAutonomousPositionNow() — an out-of-frame AutonomousPosition
build using the controller's current authoritative position +
rotation + cell. Called from OnAutoWalkArrivedReSendAction BEFORE the
re-send. ACE now processes 'I'm here at (target_pos)' then 'Use'
in order; CreateMoveToChain's WithinUseRadius shortcut
(Player_Move.cs:66) fires immediately and the action completes.
[autowalk-flush-ap] trace line under ACDREAM_PROBE_AUTOWALK so the
sequence is visible end-to-end:
autowalk-end → autowalk-flush-ap → autowalk-arrived-resend → autowalk-out
User report: 'It still however just approach it and does not use it.'
Root cause: local auto-walk arrives at the target visually, but ACE's
server-side MoveToChain may have timed out before our position was
recognised as in-range (we don't echo authoritative position back to
ACE during the walk yet). The action never fires.
Fix (re-send on arrival):
* PlayerMovementController.AutoWalkArrived event fires once when
EndServerAutoWalk(reason='arrived') is called.
* GameWindow tracks _pendingPostArrivalAction = (guid, isPickup)
on each SendUse / SendPickUp.
* OnAutoWalkArrivedReSendAction (subscribed at EnterPlayerModeNow)
re-sends the action with isRetryAfterArrival=true. The retry
flag prevents the re-sent action from itself setting a new
pending action — breaks any potential re-fire loop.
* The re-sent action is close-range from the local body's
perspective, so ACE's CreateMoveToChain hits the WithinUseRadius
shortcut (Player_Move.cs:66) and completes immediately —
dialogue opens, item picks up.
User report: 'items dropped on the ground now have a smaller triangle
box, perhaps too small. Also now other stuff like signs also have a
very small triangle box, should not have it should scale to the size
of the object.'
Fix (scale-aware indicator height):
* TargetIndicatorPanel.TargetInfo now carries entity Scale.
* EntityHeightFor multiplies the per-type base by Scale so an
upscaled NPC / sign / lifestone gets a proportionally larger box.
* Per-type table refined:
Creature : 1.8 m * scale
Door/Lifestone/Portal : 2.4 m * scale
Small carry items (weapon/armor/clothing/jewelry/food/money/
misc/missile-weapon/container/gem/spellcomp/writable/key/
caster — most pickup-able): 0.8 m * scale (up from 0.5 m)
Everything else (signs / scenery interactables / untyped):
1.5 m * scale (up from 0.5 m default)
Deferred to follow-up: exact mesh-AABB-derived box (need to read
each entity's actual rendered bounds at registration time).
Three user-reported fixes:
1. (B.6) Run-vs-walk decision lifted out of the per-frame overlay
into BeginServerAutoWalk. Once set at auto-walk start, the
character runs (or walks) the full way to the target instead of
transitioning. Matches user-observed retail behaviour:
'if its far away it should run all the way to the object and
then stop'.
_autoWalkWalkRunThreshold → _autoWalkInitiallyRunning (bool,
sampled once from initial distance vs the wire's WalkRunThreshold).
2. (B.7) TargetIndicatorPanel now picks EntityHeight per-type:
Creature (NPC/player) → 1.8 m
Door / Lifestone / Portal (tall structures) → 2.4 m
Default (small ground item) → 0.5 m
Items now get a small box hugging the silhouette instead of a
humanoid-tall rectangle floating around them.
3. (Interact) R-key (UseCurrentSelection) now dispatches by target
type:
Item (no Creature flag, no BF_DOOR|LIFESTONE|PORTAL|CORPSE)
→ SendPickUp (PutItemInContainer 0x0019)
Everything else → SendUse (0x0036)
Single hotkey to interact with whatever's selected.
Deferred (separate phase): turn-to-face on close-range use. ACE
server-side does Rotate(target) before the close-range pickup
callback (Player_Move.cs:71), but our local body doesn't echo
the turn yet — needs a synthesized client-side rotation or
MovementType=8 TurnToObject handling. Filing as follow-up.
User reported intermittent selection — 'sometimes can be selected,
sometimes not'. Cause: WorldEntity.Position is at FEET level (Z=ground
for standing humanoids), so a 0.7m sphere centred there only covered
the lower legs. Clicks on chest (Z≈1.2m) or head (Z≈1.7m) missed
because the closest-approach distance from the cursor ray to the
feet-centered sphere exceeded the radius.
Fix:
- Sphere centre now defaults to position.Z + 0.9 m (humanoid
mid-body). New optional verticalOffsetForGuid callback overrides
per entity.
- Default radius bumped 0.7 → 1.0 m to match the new sphere
placement (1.0 m at 0.9 m height covers a 1.8 m humanoid from
shin to top-of-head).
GameWindow.PickAndStoreSelection wires the callback:
- Creatures (ItemType.Creature flag): vz = 0.9 m (humanoid centre)
- Large flat objects (BF_DOOR | BF_LIFESTONE | BF_PORTAL |
BF_CORPSE): vz = 1.0 m + radius 2.0 m (mid-door/lifestone)
- Everything else (ground items): vz = 0.2 m (just above feet)
Existing 9 WorldPicker tests still pass — their head-on ray geometry
doesn't depend on the vertical offset.
Visual test surfaced three follow-ups:
1. Square box, not 1:2 rectangle.
WidthHeightRatio: 0.5 → 1.0. Retail's Vivid Target Indicator draws
a square; the earlier humanoid-aspect ratio looked wrong for
non-humanoids and didn't match retail screenshots.
2. Large flat objects (doors / lifestones / portals / corpses)
weren't selectable with the new tight 0.7 m pick sphere.
WorldPicker.Pick now takes an optional radiusForGuid callback so
the host can per-entity decide a larger radius. GameWindow's pick
site supplies a lambda that bumps to 2.0 m for any entity with
BF_DOOR (0x1000), BF_LIFESTONE (0x4000), BF_PORTAL (0x40000), or
BF_CORPSE (0x2000) set in ObjectDescriptionFlags. Default stays
at 0.7 m for humanoids and items.
3. New [B.7] pick-info diagnostic on each successful pick:
[B.7] pick-info guid=0x... itemType=0x... pwd=0x... color=(r,g,b)
Lets us verify e.g. whether a 'green NPC' really is server-side
flagged as Vendor (BF_VENDOR=0x200, retail-defined green) vs a
bug in our colour lookup. The pwd bit table is acclient.h:6431-
6463 — same flags retail's gmRadarUI::GetBlipColor branches on.
Note: textured retail-sprite corner triangles remain a B.7 follow-up
deferred per the spec. MVP uses procedural fills.
Visual test surfaced two B.7 MVP issues:
1. Box anchored at abdomen + fixed 48px size meant the rectangle
shrank visually as the camera approached the entity (entity got
bigger on screen, box stayed 48px → triangles ended up inside
the silhouette).
2. Origin was a single point (entity position + 0.9m WorldVerticalOffset)
so the box wasn't centred on the visible body.
Fix: project both feet (WorldPosition) and head (WorldPosition.Z +
EntityHeight=1.8m) to screen space. Apparent pixel height between the
two = box height; halve it for width (WidthHeightRatio=0.5 ≈
humanoid). Box centred at midpoint of projected feet+head.
- Closer entity → bigger projected height → bigger box. Distance
scaling is automatic from the perspective projection.
- Farther entity → smaller projected height → MinScreenHeight=16px
floor prevents the box collapsing to a point.
- Box is screen-axis-aligned (always rectangular on screen) but
sized + positioned by the entity's actual world-space silhouette.
Properties exposed (TriangleSize, EntityHeight, WidthHeightRatio,
MinScreenHeight) so the panel can be tuned per-instance if a future
caller wants short-item boxes (drop EntityHeight to ~0.3m for tapers,
keep WidthHeightRatio at 1.0 for a square box).
Stuck-on-+Je issue (clicking other things still returns +Je) is
Issue #59 — picker over-pick — and unaffected by this commit.
Per the B.7 design spec, wires a Vivid-Target-Indicator-style overlay
into GameWindow's ImGui pass:
TargetIndicatorPanel (src/AcDream.App/UI/TargetIndicatorPanel.cs)
- Three delegates injected from GameWindow:
selectedGuidProvider -> _selectedGuid
entityResolver -> (worldPos, itemType, pwdBits) from
_entitiesByServerGuid + _liveEntityInfoByGuid
+ _lastSpawnByGuid
cameraProvider -> (view, projection, viewport) from
_cameraController.Active + _window.Size
- Per-frame Render():
* Bail on null selection / despawned entity / zero viewport.
* Project entity world position (+0.9m mid-body offset) to NDC.
* Bail off-screen (no edge arrow in MVP).
* Convert to viewport pixel coords, draw 4 right-angle triangles
at corners of a 48px square around the projected center.
* Colour from RadarBlipColors.For(itemType, pwdBits).
GameWindow wiring:
- Construct _targetIndicator right after _panelHost during ImGui init.
- Call _targetIndicator?.Render() between _panelHost.RenderAll and
_imguiBootstrap.Render — draws to the ImGui background list so
docked panels can occlude the indicator if they overlap.
Build green. Core.Tests went 1046 -> 1054 (+8 RadarBlipColors tests
from the prior commit). Baseline failures unchanged at 8.
Visual verification next: launch, click an NPC → yellow corners; click
an item -> white corners; deselect -> corners disappear.
User-observed behaviour: 'When at a distance X it should start running
towards the double clicked target and then stop close to it. When at a
shorter distance it should walk to it.' That's retail's MoveToManager
behaviour driven by the wire's WalkRunThreshold field, which Slice 2
ignored (it always synthesised Run=true regardless of distance).
ACE's defaults (MoveToParameters.SetDefaults): WalkRunThreshold=15.0 m
for Use/PickUp paths — so close-range auto-walks are walks, not runs.
ACE's combat-charge override: 1.0 m — chase runs until the last metre.
Both retail and ACE compute Run vs Walk per-frame from remaining
distance vs threshold.
Wire WalkRunThreshold:
- Already parsed into CreateObject.MoveToPathData.WalkRunThreshold.
- Now plumbed through to PlayerMovementController.BeginServerAutoWalk
as a new parameter, stored in _autoWalkWalkRunThreshold.
- ApplyAutoWalkOverlay sets Run = (dist > _autoWalkWalkRunThreshold)
per frame. The synthesised input flips Run as the body approaches.
The motion-interpreter pipeline downstream picks RunForward vs
WalkForward from input.Run, so the animation cycle naturally switches
as the body crosses into the walk band. Run rate falls back to the
local PlayerWeenie.InqRunRate as before (ACE sends mtRun=0.00 for
Use/PickUp, so we never read mtRun; this is unchanged from Slice 2).
[autowalk-begin] diagnostic now includes walkRunThresh={x:F2} so the
threshold is visible alongside dest/minDist/objDist in the trace.
Prior trace (launch-slice2.log) showed ACE follows every mt=0x06
MoveToObject immediately with an mt=0x00 InterpretedMotionState
(cmd=0x0007 RunForward, fwdSpd=2.86) — the locomotion echo for the
same auto-walk, NOT a cancel. My wiring was treating the second
packet as 'server intent changed' and calling EndServerAutoWalk,
which killed the auto-walk on frame 1. Result: [autowalk-begin]
immediately followed by [autowalk-end reason=motion-non-moveto] and
zero visible motion.
Remove the over-eager cancel. The two natural cancel paths remain:
arrival detection inside ApplyAutoWalkOverlay, and user-input
cancellation (any movement key). A fresh MoveToObject re-targets via
BeginServerAutoWalk overwrite, which is the correct sticky-targeting
behavior.
Retail-faithful per MovementManager::PerformMovement (0x00524440 case 6,
decomp 300628-300648): when ACE broadcasts MoveToObject for the local
player, the local client runs its OWN auto-walk on its OWN body —
heading correction toward the target, run-forward velocity, arrival
detected via the wire's min_distance / distance_to_object predicates.
Implementation:
PlayerMovementController:
+ IsServerAutoWalking property (read-only)
+ BeginServerAutoWalk(destWorld, minDist, objDist, moveTowards)
+ EndServerAutoWalk(reason) // idempotent, logs to [autowalk-end]
// when ACDREAM_PROBE_AUTOWALK is on
+ ApplyAutoWalkOverlay(dt, input) — called at the top of Update.
- User movement key (Forward/Backward/Strafe/Turn) cancels.
- Arrival predicate matches RemoteMoveToDriver / retail.
- Heading steered toward destination at ±20° snap-on-aligned
tolerance / π/2 rad/s rotation rate (same constants the
remote-creature path uses).
- Synthesizes input as Forward+Run; the rest of Update's
MotionInterpreter + body-velocity pipeline runs unchanged.
GameWindow.OnLiveMotionUpdated (local-player branch):
+ when update.MotionState.IsServerControlledMoveTo and MoveToPath
is populated: translate origin to world via RemoteMoveToDriver
.OriginToWorld, call _playerController.BeginServerAutoWalk.
+ when a non-MoveTo motion arrives and auto-walk is active:
EndServerAutoWalk(reason="motion-non-moveto").
+ [autowalk-begin] trace line under ACDREAM_PROBE_AUTOWALK.
The mtRun=0 case from the spec trace is handled implicitly: this
slice doesn't read MoveToRunRate at all — it relies on the existing
input.Run path which uses the player's local InqRunRate (env-var
defaulted to 200). Future slice can layer in mtRun!=0 honor if needed.
Slices 3 (animation cycle source while auto-walking) and 4 (local
pickup animation echo for #64) deferred to follow-up commits.
Per the B.6 design spec (now retail-grounded on Option A), slice 1 is
pure-additive logging so the next session has a clean trace of what
ACE actually sends to the local player during a server-initiated
auto-walk.
New PhysicsDiagnostics.ProbeAutoWalkEnabled static flag, env-var-
initialized from ACDREAM_PROBE_AUTOWALK=1. Probe sites:
[autowalk-out] on SendUse + SendPickUp — the packets that trigger
ACE's CreateMoveToChain when the target is out of WithinUseRadius.
[autowalk-mt] on OnLiveMotionUpdated for _playerServerGuid only —
captures MovementType + MoveToPath origin/min-dist/obj-dist +
moveTowards + speed/runRate. Lets us see exactly the wire data
retail's PerformMovement case 6 (0x00524440) was acting on.
[autowalk-up] on OnLivePositionUpdated for _playerServerGuid only —
cadence + payload of ACE's position broadcasts during auto-walk.
No behavior change. All flags off by default; opt in with the env var
during a focused reproduction. Designed to be mirrored into DebugVM
checkbox state later (parallel to ProbeResolve / ProbeCell / ProbeBuilding)
but not wired yet — env-var-only for the first trace session.
B.4c sets Animation = null! for sequencer-driven door entities (sites
at line 2867 + 7892), but the declared type is non-nullable. Today
doors never enter _remoteDeadReckon (ACE doesn't send UpdatePosition
for them), so the PARTSDIAG block's unguarded read is unreachable —
but the moment something flips that, ACDREAM_REMOTE_VEL_DIAG=1 would
NRE the tick.
Local-var + is-not-null check keeps the guard scoped to this block;
the legacy slerp branch downstream still treats ae.Animation as
non-null per its declared type, so the flow analysis doesn't propagate
nullable warnings to unrelated sites.
The previous "Can't pick that up" toast wasn't retail behavior either —
retail silently dropped the malformed pickup with no client-side
feedback. Drop the toast, keep the guard. The user learns by trying
again (which IS the retail UX). Filter still prevents the malformed
packet from reaching ACE, so the WeenieError 0x0029 + NPC-emote chain
that originally surfaced this bug stays suppressed.
Visual test surfaced a UX bug: when the user single-clicked an NPC last
before pressing F, _selectedGuid carried over to SendPickUp and the
client sent PutItemInContainer(itemGuid = NPC's serverGuid, ...). ACE
responded with WeenieError 0x0029 (Stuck — "You cannot pick that up!")
AND triggered the NPC's emote chain, producing the confusing
"NPCs talk to me when I press F" symptom.
F is only for ground items. Use (double-click) is the right action for
NPCs and is unaffected. Guard SendPickUp with the existing
IsLiveCreatureTarget predicate and show a toast instead. Same defensive
pattern as the 'Not in world' / 'Nothing selected' guards already
present in SendUse / SendPickUp / OnInputAction.
After B.5 shipped, the actual pickup was invisible feedback-wise: the
item left the ground, ACE despawned it via PickupEvent (0xF74A), and
the ItemRepository got updated — but the player had no visual
acknowledgement that anything happened. The M1 demo's "pick up an
item" target visually felt like the item just vanished into the void.
Add a new EntityPickedUp event to WorldSession that fires from the
PickupEvent (0xF74A) dispatch branch BEFORE EntityDeleted, so the
subscriber can still read the entity's display name from
_entitiesByServerGuid before the despawn handler clears it.
GameWindow subscribes during the live-session wiring block and emits
a retail-style system chat line plus a debug toast on every successful
pickup, mirroring retail behavior (retail synthesized this line
client-side; ACE doesn't echo it).
Closes the M1 demo "pick up" target's visible-payoff gap.
Visual test revealed doors rendered halfway in the ground because the
spawn-time SetCycle seed never fired:
- Spec specified NonCombat stance = 0x01, but ACE's MotionStance.NonCombat
is 0x3D (61). The cycle key is `0x80000000 | stance`, so the correct
initial style is 0x8000003D, not 0x80000001.
- HasCycle(0x80000001, ...) always returned false -> SetCycle was skipped
-> sequencer left with no current motion -> Advance(dt) returned empty
frames -> per-frame MeshRefs rebuild at line 7691 set every part to
(origin, identity) -> door parts collapsed to the entity origin (which
sits at the door's pivot, halfway underground for inn doors).
Fix:
1. Rename inline `NonCombatStance` -> `NonCombatStyle` and use the correct
0x8000003D value.
2. Defensively prefer spawn.MotionState?.Stance when present (the wire
may carry an explicit non-NonCombat stance for unusual doors), falling
back to NonCombat. Mirrors OnLiveMotionUpdated's existing pattern at
line 3148: `uint fullStyle = stance != 0 ? (0x80000000u | (uint)stance) : ae.Sequencer.CurrentStyle`.
3. Extend [door-anim] registered diagnostic to include initialStyle so
future visual tests can verify the stance value at a glance.
Verified by reading the prior visual test's log: ACE broadcasts UMs
with stance=0x003D and the runtime sequencer keyed cycles by
style=0x8000003D. Same value now used at spawn.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code-quality review of the [door-cycle] diagnostic flagged three items:
- Important: open-coded doorInfo.Name == "Door" duplicated IsDoorSpawn's
predicate. Introduces IsDoorName(string?) as the shared core both
IsDoorSpawn and the diagnostic call.
- Minor: the diagnostic's comment said "Phase B.4c" which rots after
archival; rewrite to use the durable [door-cycle] grep target instead.
- Minor: the diagnostic re-read update.MotionState.Stance / ForwardCommand
instead of the stance/command locals every other diagnostic in the
method uses. Switched to the locals for pattern consistency.
No behavior change. Build green; tests 1046/8 baseline unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Logs one line per UpdateMotion arriving for an entity named "Door"
when ACDREAM_PROBE_BUILDING=1. Greppable trail for the B.4c visual
test: confirms the dispatcher hit the sequencer for door open / close.
Durable subsystem-named tag per the Opus reviewer's B.4b feedback
([B.4c] would rot after phase archival; [door-cycle] survives).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds IsDoorSpawn helper and a sibling branch to the live-spawn
handler's animation registration gate. Detects entities where
spawn.Name == "Door" and registers them in _animatedEntities with an
AnimationSequencer seeded from the spawn PhysicsState's ETHEREAL bit
(Off cycle if closed, On if already open).
Mirrors ACE Door.cs:43 (CurrentMotionState = motionClosed) so the
sequencer always has frames for the per-frame tick to advance from
the first render. Without the seed, Advance(dt) returns no frames and
the MeshRefs rebuild at line 7691 collapses the door to origin.
No changes to OnLiveMotionUpdated, AnimationSequencer, EntitySpawnAdapter,
or the per-frame tick. The tick's sequencer branch at line 7497 reads
ae.Sequencer.Advance(dt) and never touches ae.Animation in this path
(only the legacy slerp else branch at line 7644+ does).
[door-anim] registered diagnostic gated on ACDREAM_PROBE_BUILDING.
One spec deviation: Animation = null! (null-forgiving) instead of
Animation = null — AnimatedEntity.Animation is a required non-nullable
field; null! is the same pattern used at line 7857 for sequencer-driven
AnimatedEntity registrations in the same file.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
B.4b visual test revealed the L.2g pipeline was a phantom:
- Server SetState arrives with parsed.Guid = ServerGuid (0x7A9B4015)
- ShadowObjectRegistry keys by local entity.Id (0x000F4245)
- UpdatePhysicsState(0x7A9B4015, ...) misses the lookup -> no-op
- Cached state stays 0x00010008 forever
- CollisionExemption.ShouldSkip sees the unchanged state
- Door keeps blocking the player
Translate in OnLiveStateUpdated by looking up the WorldEntity via
_entitiesByServerGuid and using entity.Id as the registry key.
Also extends the [setstate] diagnostic to include entityId=0x... so
the next visual-test grep can confirm the translation lands.
This was the actual blocker the user reported as "I cant go through
it" -- ACE was flipping ETHEREAL, our pipeline acknowledged it in the
diagnostic, but the cached state for the resolver-side check never
moved. Both L.2g slice 1's unit tests and slice 1b's collision
exemption widening were correct in isolation; the integration between
them was broken by the ID-space mismatch.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
GameWindow.OnInputAction had an early-return gate dropping every
non-Press activation. With the new InputDispatcher firing
SelectDblLeft as ActivationType.DoubleClick, the case in the switch
was unreachable -- visual test confirmed [input] SelectDblLeft
DoubleClick fired but [B.4b] pick never followed.
Fix: also let DoubleClick through the gate. The existing case label
matches on action (not activation), so SelectDblLeft fires
PickAndStoreSelection(useImmediately: true) as designed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes#57. Adds three OnInputAction switch cases (SelectLeft,
SelectDblLeft, UseSelected) and three private helpers
(PickAndStoreSelection, UseCurrentSelection, SendUse). Single-click
selects but does not Use; double-click selects + Uses; R hotkey
sends Use on the existing _selectedGuid. ImGui mouse-capture
filtering already happens in InputDispatcher — no new guard needed.
Diagnostic lines emitted for log grep:
[B.4b] pick guid=0x{guid:X8} name={label}
[B.4b] use guid=0x{guid:X8} seq={seq}
Also adds a one-line doc comment on _selectedGuid clarifying its
dual-purpose role (combat Q-cycle + interaction click), per the Task 3
review.
Build green; tests 1046/1054 (8 pre-existing-baseline fails
unchanged). Switch-case behavior verified at runtime via the Holtburg
inn doorway visual test (per spec §Testing → Runtime verification).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Retail's selection model is a single "current target" used by combat,
interaction, NPC dialog, and HUD alike - not two parallel selections.
Renames the existing combat-only field on GameWindow so the upcoming
B.4b click handler and the existing Q-cycle SelectClosestCombatTarget
share the same selection state.
Mechanical rename, no behavior change. Build + tests green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two changes folded into one commit:
1. GameWindow subscribes to WorldSession.StateUpdated and routes the
parsed (guid, newState) pair into
ShadowObjectRegistry.UpdatePhysicsState. End-to-end wiring complete:
server SetState (0xF74B) -> WorldSession dispatcher -> StateUpdated
event -> GameWindow handler -> registry mutation -> next resolver
tick sees the new ETHEREAL bit and CollisionExemption short-circuits
the door cylinder. After this commit the M1 'open the inn door'
scenario is unblocked at the code-path level; visual verification
follows in Task 7 (user-driven).
The handler also emits a [setstate] diagnostic line when
ACDREAM_PROBE_BUILDING is enabled, giving a greppable trail when
the visual test runs.
2. Slice 0.5 freebie folded in: the [entity-source] probe lines now
include state=0x... flags=... so ETHEREAL flips are greppable
end-to-end from spawn through state change. Resolves the 'slice
1.6' suggestion from the L.2d ship handoff
(docs/research/2026-05-13-l2d-slice1-shipped-handoff.md).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds ACDREAM_PROBE_BUILDING — a read-only per-shadow-entry probe that
captures full BSP collision evidence whenever TransitionTypes.FindObjCollisions
attributes a hit (via the existing L.2a slice 3 chain). One multi-line
[resolve-bldg] entry per attributed hit: partIdx, hasPhys, bspR vs
vAabbR, world-space entOrigin_lb, and the actual hit polygon's vertices
in both object-local and world space.
Paired with a one-time [entity-source] line at every ShadowObjects.Register
call site in GameWindow so entityId from a probe line is greppable to its
WorldEntity source within a single log file.
Plumbing: BSPQuery writes the resolved hit polygon to a new
PhysicsDiagnostics.LastBspHitPoly side-channel at the 5 SetCollisionNormal
sites in Paths 5/6 + CollideWithPt. TransitionTypes clears that field
before each shadow-entry dispatch and reads it back at the L.2a slice 3
attribution site to emit the probe line.
Spec component 4 originally described an out ResolvedPolygon? parameter
on BSPQuery.FindCollisions; the static side-channel achieves the same
observable behavior without plumbing through BSPQuery's recursive private
methods. Deviation noted in PhysicsDiagnostics.LastBspHitPoly's XML doc.
Reframes the plan-of-record's L.2d sub-direction paragraph: the 2026-05-12
handoff proposed porting CBuildingObj + per-cell walkability, but ACE
BuildingObj.cs:39-52 + named-retail acclient_2013_pseudo_c.txt:701260
show find_building_collisions is one BSP test on Parts[0]. Per-cell
walkability belongs to L.2e, not L.2d. L.2d slice 1 is the diagnostic;
slice 2 is the actual fix scoped from slice 1's evidence (one of three
hypotheses: wrong BSP loaded / over-registered parts / BSPQuery flaw).
Tests: 2 synthetic unit tests in PhysicsDiagnosticsTests.cs pin the
static API contract that the BSPQuery → side-channel → TransitionTypes
emission chain depends on. The multi-line line format itself is verified
by acceptance criterion 2 (live Holtburg-doorway capture) — covering it
here would require a heavy PhysicsEngine + Transition fixture for a
diagnostic-only emission.
Verified: dotnet build green; the 2 new tests pass; the 8 pre-existing
test failures listed in the L.2a handoff (MotionInterpreter GetMaxSpeed_*,
PositionManager.ComputeOffset_BothActive_Combined,
PlayerMovementController.Update_ForwardInput_*, Dispatcher.W_held_*,
BSPStepUpTests.{D4,C3}) remain failing — none introduced by this slice.
Spec: docs/superpowers/specs/2026-05-13-l2d-cbuildingobj-collision-design.md
Conformance anchors:
- acclient_2013_pseudo_c.txt:701260 (CBuildingObj::find_building_collisions)
- acclient_2013_pseudo_c.txt:323725 (BSPTREE::find_collisions)
- ACE references/ACE/Source/ACE.Server/Physics/Common/BuildingObj.cs:39-52
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New static `AcDream.Core.Physics.PhysicsDiagnostics` holds two
runtime-toggleable flags initialized from env vars:
- ACDREAM_PROBE_RESOLVE=1 — emit one [resolve] line per
PhysicsEngine.ResolveWithTransition call: input/target/output
position+cell, ok-vs-partial, grounded-in, contact-plane status,
wall normal if hit, walkable-polygon valid, moving entity id.
- ACDREAM_PROBE_CELL=1 — emit one [cell-transit] line per
PlayerMovementController.CellId change: old → new cell, current
world position, reason tag (resolver / teleport).
Both also exposed as runtime-toggleable checkboxes in the DebugPanel
"Diagnostics" section. Unlike the existing four Dump-* checkboxes
(which only mirror sticky-at-startup env vars), the two new ones
forward directly to PhysicsDiagnostics — toggling on/off takes
effect on the next physics resolve, no relaunch.
Why now: L.2's plan-of-record (docs/plans/2026-04-29-movement-collision-
conformance.md) explicitly says "Land L.2a diagnostics first. Do not
make another physics change blind." This slice closes the most-load-
bearing gap in L.2a — a general-purpose probe on the resolver outcome
and a cell-transit log — so that later L.2b/c/d/e physics changes can
be evidence-driven instead of guessed. Foundation for the indoor /
dungeon walking trajectory (G.3 unblock).
Pure additive: when both flags are off (default), the probes collapse
to a single static-bool read per resolve, zero log cost. PlayerMovement
Controller's two CellId-mutation sites are now routed through a
private UpdateCellId(reason) helper for diag chokepoint.
Build green, 1032/1040 unit tests pass. The 8 failing tests are
pre-existing on the branch base (verified by stash-and-rerun);
none touch resolver or cell-transit code; all fail identically with
this slice stashed. Investigation deferred to a follow-up.
Refs: docs/plans/2026-04-29-movement-collision-conformance.md (L.2a
shipped-slice note added in same commit).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four new foreach blocks in GpuWorldState wire EntityScriptActivator
into the dat-hydration spawn/despawn paths:
- AddLandblock: fires OnCreate for each entity with ServerGuid==0
(live entities filtered out — they got OnCreate at AppendLiveEntity
and would double-fire on pending-bucket merges).
- AddEntitiesToExistingLandblock: fires OnCreate for each entity in
the promoted batch (all dat-hydrated by construction).
- RemoveLandblock: fires OnRemove(entity.Id) for each ServerGuid==0
entity before the loaded record is dropped.
- RemoveEntitiesFromLandblock: fires OnRemove for the demote-tier
entities about to be cleared (Near→Far demotion).
5 new integration tests cover the four fire-sites + the no-double-fire
invariant on pending-bucket merges. Pattern matches existing
GpuWorldStateTests (stub LandBlock heightmap + WorldEntity factory).
Closes#56 end-to-end. Slice A (per-part transforms in Tasks 1-3) +
Slice B (dat-hydrated entity DefaultScript firing, this task) both
ready for visual verification at Holtburg portal + Inn fireplace +
cottage chimney + spell cast.
Note: 8 pre-existing failures in Physics/Input/MotionInterpreter test
families are unrelated to this work (verified by re-running with this
task's changes stashed).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolver returns ScriptActivationInfo(ScriptId, PartTransforms) — one
dat lookup per spawn yields both pieces of info. The C.1.5a ServerGuid==0
guard is relaxed: activator now keys by ServerGuid when nonzero, else
entity.Id, so dat-hydrated entities (EnvCell statics, exterior stabs)
flow through the same code path as server-spawned ones. PartTransforms
pushed into ParticleHookSink before scheduling Play, closing the
activator side of #56.
GameWindow resolver lambda upgraded: now constructs ScriptActivationInfo
from setup.DefaultScript.DataId + SetupPartTransforms.Compute(setup),
swallowing dat-lookup throws the same way C.1.5a did.
Tests: 4 existing tests updated for new ScriptActivationInfo signature;
3 new tests cover entity.Id keying for dat-hydrated entities, end-to-end
part-transform pipeline (resolver → sink → particle world position), and
OnRemove with an arbitrary caller-picked key. 77 Vfx+Meshing+Activator
tests green.
GpuWorldState fire-site wiring (Task 4) lands next.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Visual verification at the Holtburg Town network portal revealed the swirl
was oriented along world axes (NS) instead of the portal's actual facing
(EW), and partially buried in the ground because the hook's local-frame
Offset.Origin was being applied in world axes too.
Root cause: EntityScriptActivator.OnCreate fired _scriptRunner.Play but
never called _particleSink.SetEntityRotation. When the runner's
CreateParticleHook fires, the sink reads per-entity rotation from
_rotationByEntity (defaults to Quaternion.Identity for unknown entities)
and uses it to transform the hook's Offset.Origin from entity-local to
world space. Without the seed call, the rotation lookup falls through to
Identity and the offset goes off along world XYZ.
Fix is a single SetEntityRotation call before the Play call. Added a 4th
unit test that constructs an entity with a 90 deg yaw and asserts the spawned
particle's world position reflects the rotated offset.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code-review follow-up to 65d833d:
ResolveDefaultScript was closing over its own var capturedDatsForActivator
= _dats, but the sibling SequencerFactory in the same block already
declared var capturedDats = _dats. The two locals pointed at the same
reference and served the same purpose; the alias added no value and
muddied the closure pattern.
Reuse capturedDats. No behavior change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires the activator into the production lifecycle:
- Construct alongside _wbEntitySpawnAdapter using _scriptRunner +
_particleSink (both built earlier in OnLoad).
- Production resolver lambda hits _dats.Get<Setup>(...) wrapped in
try/catch returning 0 on miss/throw — matches ParticleRenderer's
defensive read pattern.
- Pass into GpuWorldState's new optional ctor parameter.
Closes the wiring half of C.1.5a. Visual verification at the Holtburg
Town network portal is the acceptance gate.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
GpuWorldState grows a fourth optional ctor parameter for the activator,
paralleling how EntitySpawnAdapter is plumbed. AppendLiveEntity calls
OnCreate after the existing _wbEntitySpawnAdapter?.OnCreate;
RemoveEntityByServerGuid calls OnRemove after the existing OnRemove.
Symmetric, same order, null-safe.
GameWindow still passes the old 3-arg ctor — activator construction +
wire-through lands in the next commit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code-review follow-up to 003c502:
1. Test 3 (OnRemove_StopsScriptsAndEmitters) now wires the runner into
the real ParticleHookSink instead of a RecordingSink, registers a
persistent EmitterDesc, lets the CreateParticleHook actually spawn an
emitter, then asserts the sink killed it after OnRemove. Previously
the test only verified runner-side state — sink.StopAllForEntity was
never observably exercised, so a regression dropping that call would
have passed silently.
2. Removed unused `using System.Numerics` from EntityScriptActivator.cs.
No production code changes. Tests 1 and 2 unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New ~50-line orchestrator that fires Setup.DefaultScript through the
already-shipped PhysicsScriptRunner on entity spawn and stops scripts +
live emitters on despawn. Resolver delegate avoids DatCollection coupling
so the class is fully unit-testable with stubs.
Three xUnit tests cover the three branches: fire-with-script,
no-op-without-script, stop-on-remove. No wiring into the live spawn path
yet -- that lands in the next commit.
Spec: docs/superpowers/specs/2026-05-12-phase-c1.5a-portals-design.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Capture authoritative CPU+GPU dispatch numbers at Holtburg with the
gpu_us diagnostic now working (commit 25cb147). Three radii (4/8/12)
x two motion modes (standstill/walking) + a surface-format histogram
from ACDREAM_DUMP_SURFACES=1.
Adds env-gated one-shot dump path (TextureCache.TickSurfaceHistogramDumpIfEnabled,
called from GameWindow.OnRender) that fires once after both (a) frame
600 of the session AND (b) the upload-metadata dict reaches 100 entries
-- the cache-size gate prevents the dump from firing during pre-world
GUI ticks where OnRender spins at high rates but no scenery has streamed.
Output writes to %LOCALAPPDATA%\acdream\n6-surfaces.txt with a try/catch
around the I/O so disk-full / permission errors don't crash mid-measurement.
Baseline document at docs/plans/2026-05-11-phase-n6-perf-baseline.md
documents:
- CPU dominates GPU by 30-50x at every radius (strongly CPU-bound)
- GPU wildly under-utilized (max gpu_us p95 ~600us vs 16,600us frame budget)
- CPU scales superlinearly with N1 (Tier 1 cache wins on inner loop but
not outer LB walk)
- Surface atlas opportunity high (59% of textures in top-3 triples) but
win is memory-only since GPU isn't bottlenecked
Recommendation: C.1.5 (PES emitter wiring) next, then a reduced-scope
N.6 slice 2 (drop atlas + persistent-mapped buffers -- not justified by
the GPU under-utilization observed).
Roadmap entry amended to split N.6 into slice 1 (shipped) and slice 2
(planned, reduced scope, deferred until after C.1.5).
Spec: docs/superpowers/specs/2026-05-11-phase-n6-slice1-design.md.
Plan: docs/superpowers/plans/2026-05-11-phase-n6-slice1.md (Task 4).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code-quality review on Task 1 (commit a7c9800) flagged an asymmetric
diag gate: the read-before-overwrite block at the top of the dispatcher
was not gated on diag, but the frame-counter increment and BeginQuery
calls were. If a maintainer toggled ACDREAM_WB_DIAG from "1" to "" mid-
session, _gpuQueryFrameIndex would freeze (gated inside if(diag)) while
the read kept firing every frame at the same slot — producing duplicate
stale samples.
Add diag to the read block's outer condition so the read/issue/increment
trio is symmetric. One-line change; behavior under the normal usage
pattern (env var set at launch, never toggled) is unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The dispatcher's GPU TimeElapsed queries were polled in the same frame
as the indirect draw, so glGetQueryObject(ResultAvailable) always
returned 0 and gpu_us in [WB-DIAG] was stuck at 0m/0p95.
Replace the 2 single-handle queries with ring-of-3 arrays and move the
result read to BEFORE issuing the next frame's queries into the same
slot — at frame N we read slot N%3 which holds frame N-3's queries
(oldest in the ring, ~50ms old at 60fps and definitely done across all
desktop GL drivers). Vendor-neutral: AMD/NVIDIA/Intel desktop GL all
work without driver-specific code.
The gpuQuerySlot variable is hoisted to function scope (just before
Phase 7 opaque pass) so both the opaque and transparent passes
reference the same slot — the plan placed it inside the opaque-pass
if-block, which would have been out of scope for the transparent
BeginQuery; corrected in the implementation.
No new tests — the change is purely a diagnostic readout fix, no
observable behavior in the rendering path. Build green; tests at
baseline (1711 passing, 8 pre-existing physics/MotionInterpreter
failures unchanged). Manual gpu_us verification still pending in-world.
Spec: docs/superpowers/specs/2026-05-11-phase-n6-slice1-design.md (§4).
Plan: docs/superpowers/plans/2026-05-11-phase-n6-slice1.md (Task 1).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
User reported (cache enabled, post-c55acdc): drudge statue renders fully
but many trees are missing branches. Cache-disabled A/B run rendered trees
correctly. So the bug is in the cache wiring.
Root cause: c55acdc's `currentEntityIncomplete = false;` reset fired
UNCONDITIONALLY at the top of every iteration. For a tree with MeshRefs
[trunk valid, branches null, leaves valid], the tuple sequence is:
- tuple 0 (trunk): no flag set
- tuple 1 (branches): TryGetRenderData null → set flag, continue
- tuple 2 (leaves): unconditional reset → flag = false (WRONG)
- end-of-entity: flag is false, scratch has trunk+leaves batches but NOT
branches → MaybeFlushOnEntityChange populates a PARTIAL cache entry
- cache hits forever serve trunk+leaves with no branches
Drudge happened to render correctly because its missing MeshRef was at the
END of its MeshRefs list — no later tuple reset the flag.
Adds a per-tuple `prevTupleEntityId` tracker for entity-change detection,
updated UNCONDITIONALLY at end of each tuple (including tuples that skip
via null renderData). The flag-reset block now fires ONLY on actual entity
change. Within the same entity, the flag accumulates across tuples.
Also includes ACDREAM_DISABLE_TIER1_CACHE=1 diagnostic env-var added
inline (was stashed previously) for future A/B testing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
User reported: the drudge statue on top of the Foundry (a multi-part
live-spawned entity with AnimPartChange + texChanges) renders only
PARTIALLY — some parts visible, some missing.
Root cause: the dispatcher's slow path skips a MeshRef when
_meshAdapter.TryGetRenderData returns null (mesh still async-decoding
via ObjectMeshManager.PrepareMeshDataAsync). The classified-batches
collector accumulates only the MeshRefs that DID resolve. At entity
boundary, the cache populates with the PARTIAL set. Frame-2 cache hits
serve that partial entry forever — even after the missing mesh loads,
the cache continues to skip those parts because classification never
reruns for cached entities.
Fix: track currentEntityIncomplete during the foreach. Set it true on
any null renderData. At entity boundary (and at end-of-loop), if the
flag is set, DROP the accumulated populate scratch instead of writing
it to the cache. The slow path retries on the next frame; once all
meshes have loaded, the populate fires correctly with the complete
classification.
Adds a regression test pinning the contract — incomplete entities
produce zero cache entries.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
Code review of f16604b flagged that DebugCrossCheck's XML doc claimed
"called once per static-entity cache hit per frame" — overstated. The
method is currently exercised by unit tests only; the dispatcher's
cache-hit branch fires a simpler predicate assert (!isAnimated) at
production hit time, not the full live-state cross-check. Wiring the
full cross-check is the spec section 6.5 stretch goal, kept open as a
follow-up.
Doc-only change. No behavior change. 1708 / 8 baseline preserved.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
