User correctly called out: 'why workarounds? Nothing wrong with ACE,
our client is wrong.' Three of the four workarounds in B.6/B.7
(arrival margin, re-send-on-arrival, AP flush on arrival) exist
because our client sends 1Hz position heartbeat. Retail sent every
tick. ACE's CreateMoveToChain polls WithinUseRadius every ~0.1 s
using the latest Player.Location — at 1Hz we leave up to 1 s of
stale position data on the server, so ACE rejects re-sent actions
as still-out-of-range.
Fix: bump heartbeat to ~10 Hz when the body is actively moving
(auto-walk OR user pressing W/A/S/D). Idle still 1 Hz.
ACE sees us approach in near-real-time; server-side MoveToChain
converges normally; CreateMoveToChain's own callback fires the
action when in radius — no client-side re-send needed.
This SHOULD make the existing workarounds redundant:
* Arrival margin (0.2m) — can shrink toward 0 since position
drift is bounded by 100ms instead of 1s
* Re-send on arrival — ACE's chain completes on its own
* AP flush on arrival — included in normal heartbeat
Plan to retire them in a follow-up commit once we verify the
heartbeat bump alone is enough.
Margin trim:
Previous: min(0.5, threshold * 0.4) — for 3 m NPC arrived at 2.5 m
New: min(0.2, threshold * 0.2) — for 3 m NPC arrives at 2.8 m
User feedback: 'compared to retail, it fires too close. In retail
it fires from a longer range.' Smaller margin matches that — still
safely inside ACE's strict WithinUseRadius but closer to the boundary.
Tight pickup radii (0.6 m item) now arrive at 0.48 m (was 0.36 m).
Filed issues:
#67 Door Use action doesn't complete after auto-walk arrival.
NPC dialogue fires correctly post-flush-AP+re-send, but
doors still go silent — need to investigate door-specific
state requirements in ACE's Door.ActOnUse or our wire
payload differences.
#66 Rotation: local player flips back after auto-walk arrival
(observed from retail observer); NPCs don't turn to face
the player when used. Both rooted in missing MovementType=8
TurnToObject handling. Supersedes #65 (which was local-only)
with a unified rotation-handling phase scope.
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.
User-observed bug: 'I selected a retail player once, now I cant select
anything else.' Cause: the 5 m fixed pick sphere covered most of the
visible area around an entity, so once the cursor was anywhere near an
NPC or player, every subsequent click resolved to that same NPC/player
instead of the actual cursor target.
0.7 m roughly matches the actual hitbox radius of humanoid bodies and
most pickable items. Clicking on the entity's silhouette still hits;
clicking next to it or through it to a closer target now correctly
picks the closer target.
Existing 9 WorldPicker unit tests all pass — they tested geometric
behaviour at picked test radii, not the literal 5 m constant.
Follow-ups (deferred to a future picker phase):
- Per-itemType radius (tighter for tapers, looser for chests).
- Priority sorting at equal hit-distance (items beat NPCs).
- Ray-vs-actual-mesh test instead of bounding sphere.
Together with B.7's target indicator (corner triangles, c7e5f9f /
4bc95ec) this gives the user both 'I can hit what I'm aiming at'
AND 'I can see what I just hit' — fixes the over-pick at the source
plus surfaces it visually when it does still happen.
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.
Static helper resolving a target indicator / radar blip colour from
ItemType + the raw PublicWeenieDesc._bitfield acdream already parses
onto EntitySpawn. Dispatch order matches retail decomp at 0x004d76f0:
Portal (BF_PORTAL = 0x40000) → cyan
Vendor (BF_VENDOR = 0x200) → green
Creature && !IsPlayer → yellow
Player + IsPK (BF_PLAYER_KILLER = 0x20) → red
Player + IsPKLite (= 0x2000000) → pink
Player (other) → white (Default)
Otherwise (item / object) → light grey
RGBA values are hand-tuned to visually match retail screenshots; the
real RGBAColor_Radar* constants live in retail static data and can be
swapped in later without breaking call sites.
8 unit tests cover the full type/flag matrix (item, NPC, friendly
player, PK, PKLite, vendor, portal-priority-over-flags).
Next: TargetIndicatorPanel (App, ImGui draw) that uses this lookup.
Retail-anchored design for the missing visual feedback on selection:
four corner triangles + radar-blip colour coding around the selected
entity, drawn via ImGui in screen space.
Retail evidence (named decomp):
* VividTargetIndicator::SetSelected at 0x004f5ce0
* gmRadarUI::GetBlipColor at 0x004d76f0 (Portal / Vendor / Creature /
Player / PK / PKLite / Default colours from pwd._bitfield bits +
IsCreature/IsPlayer/IsPK predicates we already parse)
* VividTargetIndicator::CopyImage at 0x004f5dd0 (tints a source
bitmap by RGBA)
MVP scope:
1. RadarBlipColors helper (Core, with unit tests)
2. TargetIndicatorPanel (App, ImGui draw via background draw list)
3. Wire to existing _selectedGuid from B.4b
4. ~200 LOC + tests
Deferred to follow-ups: off-screen edge arrow, DAT-loaded sprite (MVP
draws procedurally), mesh-tint highlight, player-option toggle, server
selection-relay.
Pairs with #59 (WorldPicker over-pick): the indicator makes the
mis-pick visible, so the user can clear + reselect even before the
underlying picker is tightened.
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.
Captured a live ACDREAM_PROBE_AUTOWALK trace double-clicking +Je from
~3.5m. Findings folded into the spec's State at design freeze section:
1. Wire parser is correct (matches ACE MoveToObject.Write +
MoveToParameters.Write byte-for-byte).
2. ACE sends mtRun=0.00. Not a parser bug — that's the wire value.
Retail's apply_run_to_command (0x00527BE0) fell back to the
player's own rate; our Slice 2 needs the same fallback chain.
3. Player position never changed during the entire trace — current
behavior is pure no-op on the inbound MoveToObject (literally
ignored, as our code at OnLiveMotionUpdated:3289 suggests).
4. ACE does NOT broadcast UpdatePosition for the local player during
auto-walk. Definitively kills Option C — nothing to blend with.
Local body must drive itself.
The trace validates the spec's Option A path. Slice 2 implementation
can proceed without further wire-format guessing.
Wires PhysicsDiagnostics.ProbeAutoWalkEnabled into the DebugVM + ImGui
panel checkbox alongside the existing Probe Resolve / Probe Cell /
Probe BSP hits checkboxes. Following the L.2a + L.2d pattern: the
panel toggle takes effect live (no relaunch needed) because the
diagnostic call sites read the static flag every frame.
Lets the next B.6 trace session toggle the probe via panel checkbox
when ACDREAM_DEVTOOLS=1, without an env-var dance.
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.
Grounded the design in named-retail evidence. MovementManager::Perform
Movement at 0x00524440 case 6 (decomp lines 300628-300648) shows the
retail client's local-side dispatcher for inbound MoveToObject:
unpacks the wire, sets motion_interpreter->my_run_rate, calls
CPhysicsObj::MoveToObject on the LOCAL player's physics body. Same
code path retail used for every creature chasing the player.
Conclusion: Option A (run a local driver against the player's body)
is retail-faithful. Option C (server-position-blend) is a non-retail
shortcut and is now eliminated from consideration.
Re-scoped the spec into 4 slices:
1. ACDREAM_PROBE_AUTOWALK diagnostic baseline (~30 LOC)
2. PlayerMovementController.BeginServerAutoWalk + reuse of
RemoteMoveToDriver against the local player's body (~100 LOC)
3. Animation cycle selection during auto-walk (~20 LOC)
4. Local pickup-animation echo (closes#64, ~10 LOC)
Total ~160 LOC, no new files. All existing acdream infrastructure
(RemoteMoveToDriver, ServerControlledLocomotion, MotionState.MoveTo
Path parsing) is reused; the work is wiring it for _playerServerGuid
in addition to remote guids.
Captures the wire-format facts that are already parsed (MotionState.
IsServerControlledMoveTo + MoveToPath fields), the two gating sites
that drop the inbound MoveToObject for the local player today, and a
three-option solution space (run remote-driver locally, visual tween,
server-position-authoritative blend).
Recommendation: Option C first (smallest blast radius, single-commit
hotfix if ACE's UpdatePosition broadcast cadence is adequate); promote
to Option A only if the trace shows server broadcasts are too sparse
to render smoothly.
Explicitly does NOT implement yet. The 'walks then snaps back' visible
symptom is observed but the mechanism isn't characterized in detail —
the spec calls for a diagnostic-trace session first (ACDREAM_PROBE_
AUTOWALK env var, ~30 LOC) to capture exactly what ACE sends during a
failed auto-walk. The trace decides between Option C (sufficient
position broadcasts) and Option A (need to fill in per-tick locally).
#64 (local pickup animation) is flagged as likely-related — same
OnLiveMotionUpdated:3289 self-echo filter drops both. May fix in
the same B.6 work.
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.
M1's demo scenario is mechanically complete:
1. Walk through Holtburg — met via L.2a/d/g
2. Open the inn door — met via B.4b + B.4c
3. Click an NPC — met via B.4b chain + chat handlers
(visually verified 2026-05-14 on Tirenia + Royal Guard)
4. Pick up an item — met via B.5 + 87ba5c9 feedback polish
What's left to formally land: record ≈30s demo video, pin still +
writeup, flip freeze list, point CLAUDE.md "currently working toward"
at M2. Per the milestone-discipline rules, milestone landing is a
user-driven event with an artifact; this commit only updates the
factual demo-target status.
Filed but explicitly deferred (don't block M1 recording): #61 (door
swing cycle-boundary flash), #62 (PARTSDIAG null-guard), #63
(server-initiated MoveToObject auto-walk — candidate Phase B.6),
#64 (local-player pickup animation).
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.
Phase B.5 (ground-item pickup, close-range path) shipped and
visual-verified 2026-05-14 at Holtburg. M1 demo target 4/4 ("pick up
an item") met.
New ship-handoff doc captures the 5-commit history including the
post-visual-test PickupEvent (0xF74A) wire-handler fix that closes
the local-despawn gap.
Roadmap and CLAUDE.md updated to reflect the ship + the new follow-up
issues:
- #63 (MEDIUM) — server-initiated MoveToObject auto-walk not
honored; blocks double-click pickup + out-of-range F. Filed as
candidate Phase B.6. holtburger has the reference implementation.
- #64 (LOW) — local-player pickup animation does not render
(retail observers see it correctly). Likely a self-echo filter
dropping UpdateMotion(Pickup) on the local player.
Carry-overs from B.4c (#61 link-cycle flash, #62 PARTSDIAG null-guard)
unchanged.
ACE sends GameMessagePickupEvent (opcode 0xF74A) instead of
GameMessageDeleteObject (0xF747) for items removed via player pickup
(Player_Tracking.RemoveTrackedObject with fromPickup=true).
Without this handler, BuildPickUp succeeded server-side (item moved
into the player's container, retail observers saw it disappear), but
our local client kept rendering it on the ground because the despawn
message went to the unhandled-opcode bucket.
PickupEvent's wire body adds an objectPositionSequence field on top
of DeleteObject's layout, so the parser is its own type. The
downstream view-removal semantics are identical to DeleteObject, so
the dispatcher routes both opcodes into the same EntityDeleted event
via a small adapter.
The original test only used placement=0, which encodes identically
under WriteInt32 and WriteUInt32. Add a -1 case so a future regression
to the unsigned writer would actually fail the test.
Flagged by Task 1 code review.
Captures post-B.4c state, click-NPC investigation findings (chain
already wired via Tell/CommunicationTransientString/etc; verify
opportunistically during B.5 visual test), and B.5 scope decisions
made in chat before the user requested a session handoff:
- Trigger: F-key (SelectionPickUp action, already bound)
- Target: requires _selectedGuid (no pick-under-cursor fallback)
- Wire opcode 0x0019 (GameAction.PutItemInContainer)
- Payload: itemGuid + containerGuid + placement (12 bytes)
- Container = _playerServerGuid
- Three changes in two existing files (~50 LOC total)
Plus carry-overs from B.4c (#61 cycle-boundary flap, #62 PARTSDIAG
null-guard), the B.4b ID-translation gotcha pattern to watch for,
and the standard ACE session-race tip.
Branch `claude/phase-b5-pickup` (renamed from
`claude/investigate-npc-click`) is the workspace; the fresh session
should start there.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase B.4c shipped end-to-end with 4 implementation commits + 4 docs commits.
M1 demo target 'open the inn door' now has full visual feedback at Holtburg.
Code:
- IsDoorSpawn / IsDoorName helpers + Door spawn-time AnimationSequencer
registration with state-seeded initial cycle (Off/On from spawn PhysicsState
ETHEREAL bit)
- [door-anim] registration diagnostic + [door-cycle] UM dispatch diagnostic
(both gated on ACDREAM_PROBE_BUILDING)
- Stance-value fix: NonCombat is 0x3D not 0x01; cycle key is 0x8000003D
not 0x80000001. Without the fix, HasCycle always returned false and
doors collapsed to entity origin (halfway underground).
- Refactor: shared IsDoorName(string?) predicate eliminates the open-coded
duplicate name check; durable subsystem-named comment.
Closes#58. Files #61 (link->cycle flash, polish) + #62 (PARTSDIAG null-guard,
latent). Final whole-branch code review (Opus) approved.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Opus final review of B.4c flagged that Task 4's handoff doc invented
implementation details that don't exist in the code:
1. IsDoorSpawn claimed to check "spawn.WeenieObj.WeenieType == 8 OR
IsDoorName(spawn.Name)" — the actual code is just IsDoorName(spawn.Name)
delegating to "name == "Door"". No WeenieType lookup exists.
2. A "_doorSequencers" per-door dict was referenced in three places — that
dict doesn't exist. The actual code reuses the existing
_animatedEntities[entity.Id] dict (same one that holds creatures + the
player), with Animation = null! per the existing pattern at line 7885.
3. The UM dispatch path was described as a new B.4c-added branch with
pseudocode — that's wrong. B.4c does NOT add a new dispatch path;
OnLiveMotionUpdated's existing TryGetValue against _animatedEntities
handles doors automatically once Task 1's spawn-time branch registers
them. The only UM-dispatch B.4c contribution is the [door-cycle]
diagnostic line, gated on IsDoorName.
Corrects sections "At world load (spawn time)", "When the door opens",
"Per-frame mesh rebuild", and "Door types covered" to reflect the actual
shipped code. cmd→motion mapping (cmd=0x000C → open, cmd=0x000B → close)
left as-is — it was correct.
No code change. Verified by re-reading GameWindow.cs IsDoorSpawn /
IsDoorName helpers, the Task 1 spawn-time branch body, and the
TickAnimations sequencer dispatch.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
Task-by-task plan with full code in every step, no placeholders.
Task 1: IsDoorSpawn helper + Door registration branch (state-seeded
SetCycle from spawn PhysicsState ETHEREAL bit).
Task 2: [door-cycle] diagnostic in OnLiveMotionUpdated for greppable
verification.
Task 3: Holtburg inn doorway visual test (user-performed).
Task 4: ship handoff + close#58 + roadmap/CLAUDE/memory updates.
Self-review table at bottom maps every spec section to its task(s);
all covered. Companion to spec
docs/superpowers/specs/2026-05-13-phase-b4c-design.md.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase B.4c closes#58 (filed during B.4b ship). When a door's
state flips via ACE Door.ActOnUse, the L.2g chain handles the
SetState collision-bit flip but no UpdateMotion handler ever
animated the door visually. Investigation traced the gap to the
spawn-time registration gate at GameWindow.cs:2692 which requires
a multi-frame idle cycle — doors have no idle.
Design: door-specific spawn-time branch that bypasses the gate,
builds an AnimationSequencer, seeds it with Off (closed) or On
(open) cycle based on spawn PhysicsState. ACE Door.cs:43 sets the
same initial state. ~40 LOC in one file. Reuses the existing
AnimationSequencer + per-frame tick + WB renderer pipeline. No
changes downstream.
Discovered during self-review that the per-frame tick at
GameWindow.cs:7691-7697 unconditionally overwrites ae.Entity.MeshRefs
with sequencer-derived transforms; an empty sequencer would collapse
the door to origin. The state-seeded SetCycle at spawn keeps the
sequencer always producing valid frames. Also documented:
ae.Animation = null is safe because the tick's sequencer branch at
line 7497 never reads it (only the legacy slerp else branch does).
Diagnostic tags renamed from phase-named [B.4c] to durable
[door-anim] / [door-cycle] per Opus reviewer feedback on B.4b.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Final whole-branch code review (Opus) surfaced two Important post-merge
follow-ups + a one-word inaccuracy in the handoff doc:
- #59: tighten WorldPicker per-entity Setup.Radius (M1-deferred; the
ServerGuid==0 invariant is load-bearing and worth documenting before
L.2d's CBuildingObj port lands).
- #60: port retail's obstruction_ethereal downstream path so combat-HUD
contact reporting works for ethereal creatures (M2-combat).
- handoff: corrected "Added a _entitiesByServerGuid reverse-lookup" to
"Used the pre-existing _entitiesByServerGuid" — the dict has existed
since Phase 6.6/6.7; slice 1c used it, didn't add it.
Review verdict: branch ready to merge to main.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
