Commit graph

974 commits

Author SHA1 Message Date
Erik
4f3b8a6824 docs(issues): file #76 — Step 2 LiveSessionController attempt regressed
A first attempt at extracting LiveSessionController out of GameWindow.cs
(Step 2 of docs/architecture/code-structure.md §4) was implemented and
reverted in the same session after visual verification at Holtburg
exposed three regressions: chat input field accepted text but nothing
was sent; door/NPC double-click fired the Use packet outbound but had
no visible client-side effect; R + click-target auto-walk no longer
fired the deferred Use on arrival (re-opens issue #63 / #75 territory).

Filing as a tracked open issue so the next refactor attempt has a
clear root-cause-investigation starting point with four hypotheses to
test. Step 1 (eda936d) and the Rule 5 follow-up (32423c2) remain
clean and verified.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-17 10:37:34 +02:00
Erik
32423c2ba2 refactor(physics): promote ACDREAM_DUMP_STEEP_ROOF into PhysicsDiagnostics
First application of CLAUDE.md's new Code Structure Rules §5
("Runtime probes belong in diagnostic owner classes"). Migrates the
four ACDREAM_DUMP_STEEP_ROOF call-site env reads into a single
PhysicsDiagnostics.DumpSteepRoofEnabled property initialized from the
env var at type init, with a runtime setter that follows the existing
ProbeResolveEnabled / ProbeCellEnabled / ProbeBuildingEnabled pattern.

Sites migrated:
  - AcDream.Core/Physics/PhysicsEngine.cs:637 (KILL-VELOCITY-APPLIED log)
  - AcDream.Core/Physics/TransitionTypes.cs:718 (PHASE3-RESET log)
  - AcDream.App/Input/PlayerMovementController.cs:1117 (FRAME log)
  - AcDream.App/Input/PlayerMovementController.cs:1199 (per-frame bounce log)

Behavior-preservation only. ACDREAM_DUMP_STEEP_ROOF=1 still produces
identical [steep-roof] log output. The class-comment in
PhysicsDiagnostics already anticipated this migration
("Future slices may fold the older ACDREAM_DUMP_* env vars into this
class for unified runtime toggling").

Not yet wired to a DebugVM checkbox — runtime toggling is available
via the property setter for future debugging sessions, but exposing
it on the panel is a 30-second future cut, not in scope here.

Build: green.
Tests: same pass/fail profile as before this commit (8 pre-existing
Core failures unrelated to physics-diagnostics; App.Tests / Core.Net.Tests
/ UI.Abstractions.Tests all green).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-17 09:20:00 +02:00
Erik
eda936dc4d refactor(app): extract typed RuntimeOptions for startup env vars (Step 1)
Lifts 13 startup-time environment variables out of GameWindow.cs into a
single typed AcDream.App.RuntimeOptions record read once in Program.cs.
Behavior-preservation only — no live behavior change, no visual change.
Verified end-to-end against ACE on 127.0.0.1:9000: full M1 demo loop
(walk Holtburg, click door, click NPC, portal entry) plus DEVTOOLS
ImGui panels load cleanly.

Why: GameWindow.cs is 10,304 LOC and scattered Environment.GetEnvironmentVariable
calls were one of the structural smells called out in the new
"Code Structure Rules" doc. Typed options is the safest cut to make
first because the substitution is mechanical and parsing semantics
get pinned by unit tests.

What lands:
  - CLAUDE.md: removed stale R1→R8 execution-phases line, replaced with
    pointers to the milestones doc + strategic roadmap (the actual
    source of truth). Tightened the "check ALL FOUR references"
    section to describe WB as the production rendering base, not
    just a reference. New "Code Structure Rules" section (6 rules)
    captures the discipline we're committing to.
  - docs/architecture/acdream-architecture.md: removed dangling link
    to the deleted memory/project_ui_architecture.md.
  - docs/architecture/code-structure.md (NEW, 376 LOC): rationale for
    the 6 rules + 6-step extraction sequence
    (RuntimeOptions → LiveSessionController → LiveEntityRuntime →
    SelectionInteractionController → RenderFrameOrchestrator →
    GameEntity aggregation). This PR is Step 1.
  - src/AcDream.App/RuntimeOptions.cs (NEW, 100 LOC): typed record
    with FromEnvironment(string) factory and Parse(datDir, env)
    overload for testability. Covers ACDREAM_LIVE, _TEST_HOST/PORT/
    USER/PASS, _DEVTOOLS, _DUMP_MOVE_TRUTH, _NO_AUDIO,
    _ENABLE_SKY_PES, _HIDE_PART, _RETAIL_CLOSE_DEGRADES,
    _DUMP_SCENERY_Z, _STREAM_RADIUS.
  - src/AcDream.App/Program.cs: builds RuntimeOptions once, passes
    to GameWindow.
  - src/AcDream.App/Rendering/GameWindow.cs: ctor takes RuntimeOptions;
    7 startup-cached env-var fields become expression-bodied
    properties or direct _options.X reads; TryStartLiveSession,
    audio init, legacy stream-radius branch all route through
    _options.
  - tests/AcDream.App.Tests/ (NEW project, 10 unit tests + csproj):
    pins parser semantics — default-off bools, the literal "0"
    gate for RETAIL_CLOSE_DEGRADES, the >=0 guard for
    STREAM_RADIUS, null-vs-empty for user/pass, exact-"1" check
    for diagnostic flags. Registered in AcDream.slnx.

Out of scope (per code-structure.md §4):
  - Per-call-site ACDREAM_DUMP_* / _REMOTE_VEL_DIAG diagnostic reads
    sprinkled through GameWindow (~40 sites). Rule 5 in CLAUDE.md
    commits us to migrating these opportunistically as larger
    extractions land, not in a bulk pass.
  - AcDream.Core's project-reference to Chorizite.OpenGLSDLBackend.
    Only the stateless .Lib namespace is used; tightening the project
    reference is documented as future work in code-structure.md §2.

Build: green.
Tests: AcDream.App.Tests 10/10 ✓, Core.Net.Tests 294/294 ✓,
       UI.Abstractions.Tests 419/419 ✓,
       AcDream.Core.Tests 1073/1081 (8 pre-existing failures verified
       against pre-refactor baseline by stash-and-rerun).

Visual verification: full M1 demo loop against ACE +Acdream login
including DEVTOOLS panel host load.

Next: Step 2 — extract LiveSessionController per code-structure.md §4.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-17 09:16:55 +02:00
Erik
2950cd5740 docs: rewrite handoff — M2 redirected from "kill a drudge" to indoor walkability
User chose fundamentals-first after M1 landed: indoor walking is
untested (M1 only verified outdoor + doorway), indoor lighting is
broken, camera clips into walls indoors. Better to fix indoor
foundations before stacking combat on top.

Three sub-phases proposed for the new M2:
  1. Camera correctness (~1 day)
  2. Indoor collision audit (~1 week)
  3. Indoor lighting basics (~1-2 weeks)

Combat ("Kill a drudge") slides to M3.

Next session opens with superpowers:brainstorming to scope the
demo scenario + agree on sub-phase boundaries + update the
milestones doc + CLAUDE.md to formalize the reorder.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-16 21:01:30 +02:00
Erik
5d79dd3b88 docs: session handoff 2026-05-16
Captures M1 landing, Phase B.6 architectural details, new rules
(no-workarounds, no-demo-videos, graceful-shutdown), M2 next steps,
and test baselines for fresh-session pickup.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-16 17:53:11 +02:00
Erik
fb92122731 milestone: M1 landed; flip "currently working toward" to M2
M1 "Walkable + clickable world" landed 2026-05-16 with Phase B.6
(d640ed7). All four demo targets work end-to-end retail-faithfully:
walk Holtburg, open inn door, click NPC, pick up item.

Freeze list applied: L.2 (collision), B.4 (interaction outbound),
B.5 (pickup) — these phases are off-limits until M7 polish unless
something is actively broken.

CLAUDE.md "currently working toward" advanced to M2 — "Kill a drudge."
Phases to ship: F.2 (Inventory panel), F.3 (Combat math + damage),
F.5a (dev panels Attributes/Skills/Equipped/Inventory), L.1c
(combat animation wiring), L.1b (command router prereq).

Also removes the "record a demo video" requirement from milestone
discipline (CLAUDE.md rule #3 + milestones doc operating rule #3) —
user finds video recording pointless. Milestones are now textual
events: writeup + freeze flip + CLAUDE.md "currently working toward"
update. Saved as feedback memory.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-16 17:29:21 +02:00
Erik
d640ed74e1 feat(retail): Phase B.6 — server-driven auto-walk done right
Closes #63, #69, #74, #75. Replaces the chain of Commit-B workarounds
that compensated for ACE's MoveToChain getting cancelled by a leaked
user-MoveToState packet during inbound auto-walk. The fix is
architectural — auto-walk drives the body directly from the
server-supplied path data, no player-input synthesis, no spurious
wire-packet transitions, no grace-period band-aid.

Architectural change (closes #75):
  PlayerMovementController.ApplyAutoWalkOverlay → DriveServerAutoWalk.
  - Steps Yaw toward target at retail-faithful turn rates.
  - Computes desired forward velocity from path runRate.
  - Calls _motion.DoMotion(WalkForward, speed) directly for the
    motion-interpreter state (drives animation cycle).
  - Sets _body.set_local_velocity directly when grounded.
  - Returns true to gate the user-input motion + velocity section
    in Update so user-input flow doesn't overwrite auto-walk
    velocity or motion state.
  Mirrors retail's MovementManager::PerformMovement case 6 (decomp
  0x00524440) which never touches the user-input pipeline during
  server-controlled auto-walk.

Wire-layer guard at GameWindow.cs:6419 retained as a SEMANTIC
statement (`if (result.MotionStateChanged && !IsServerAutoWalking)`):
user-MoveToState packets are for user-driven motion intent. During
server-controlled auto-walk, the motion-state transitions caused by
the animation override (RunForward / WalkForward / TurnLeft /
TurnRight cycles) must not leak as user-cancellation packets. This
is NOT the deleted 500ms grace-period band-aid; it's the wire-layer
expressing the user-vs-server motion split.

Animation plumbed for auto-walk phases (closes #69):
  - Moving forward → WalkForward (speed=1.0) / RunForward (speed=runRate)
  - Turn-first phase → TurnLeft / TurnRight (sign of yawStep)
  - Aligned-but-pre-step / arrival → no override (idle)
Driven via _autoWalkMovingForwardThisFrame + _autoWalkTurnDirectionThisFrame
fields set in DriveServerAutoWalk and read in the MovementResult
construction at the bottom of Update. UpdatePlayerAnimation picks up
the localAnimCmd as the highest-priority animation source.

Walk/run threshold = 1.0m, retail-observed. ACE's wire-default of
15.0f is too generous; ACE's own physics layer uses 1.0f at
MovementParameters.cs:50 (with the 15.0f line commented out) and
Creature.cs:312 notes "default 15 distance seems too far". The
formula matches retail's MovementParameters::get_command at decomp
0x0052aa00: running = (initialDist - distance_to_object) >=
threshold, evaluated ONCE at chain start and held for the rest of
the auto-walk (matches retail "runs all the way / walks all the way"
behaviour). Wire-supplied threshold is ignored.

Pickup gate (IsPickupableTarget) now uses BF_STUCK
(acclient.h:6435, bit 0x4) to discriminate immovable scenery from
real pickup items that share a Misc ItemType. Sign (pwd=0x14 with
BF_STUCK) → blocked; spell component (pwd=0x10, no BF_STUCK) →
allowed. ACE's PutItemInContainer (Player_Inventory.cs:831-836)
responds with WeenieError.Stuck (0x29) on stuck items so the gate
prevents wasted wire packets + a UX dead-end.

R-key dispatch by target type. UseCurrentSelection's top-level
IsUseableTarget gate was wrong (blocked USEABLE_NO=1 items that
ARE pickupable). Reordered:
  1. Creature → SendUse
  2. Pickupable → SendPickUp
  3. Useable → SendUse
  4. Otherwise → "cannot be used" toast
Each handler keeps its own gate. Matches retail's per-action
server-side validation.

AP cadence revert (closes #74). With the MoveToChain race fixed,
the per-frame "send while moving" cadence is no longer load-bearing.
Reverted to retail's two-branch ShouldSendPositionEvent gate
(acclient_2013_pseudo_c.txt:700233-700285):
  Interval NOT elapsed (< 1 sec): send if cell or contact-plane changed.
  Interval elapsed (>= 1 sec):    send if cell or position frame changed.
Adds _lastSentContactPlane field + ApproxPlaneEqual helper +
PlayerMovementController.ContactPlane public accessor. Extended
NotePositionSent(Vector3, uint, Plane, float) — both outbound sites
(MoveToState + AP) pass _playerController.ContactPlane.
Effective rates: 0 Hz idle, ~1 Hz smooth motion, per-event on
cell/plane changes, 0 Hz airborne.

CLAUDE.md updated with no-workarounds rule (commit `da126f9` on
the worktree branch). Saved as feedback memory at
memory/feedback_no_workarounds.md.

Tests: build green; Core.Net 294/294; Core 1073/1081 (baseline,
8 pre-existing Physics failures unchanged). Visual-verified
end-to-end on 2026-05-16 for far/near Use + PickUp on NPCs,
doors, items, spell components, signs (correctly blocked), corpses,
turn-first animation, run/walk thresholds, idle quiet, smooth-
motion 1Hz.

Spec: docs/superpowers/specs/2026-05-16-phase-b6-suppress-movetostate-during-inbound-autowalk-design.md
Plan: docs/superpowers/plans/2026-05-16-phase-b6-suppress-movetostate-during-inbound-autowalk.md

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-16 16:14:44 +02:00
Erik
b5da17db76 feat(retail): Commit B — retail-faithful AP cadence + screen-rect picker
Retires divergences flagged in the 2026-05-16 faithfulness audit:

1. AP cadence. Replaces the 1 Hz idle / 10 Hz active flat heartbeat
   with a diff-driven model gated on `Contact && OnWalkable`
   (acclient_2013_pseudo_c.txt:700327 SendPositionEvent). Sends on
   position or cell change while grounded on walkable, plus a 1 sec
   heartbeat; suppressed entirely airborne. PlayerMovementController
   exposes `NotePositionSent(pos, cellId, now)` which GameWindow stamps
   after each AutonomousPosition / MoveToState send — mirrors retail's
   shared `last_sent_position_time` between SendPositionEvent
   (0x006b4770) and SendMovementEvent (0x006b4680). Known divergence
   from retail: ours is per-frame-while-moving, retail's effective rate
   is ~1 Hz during smooth motion (cell/plane checks). Filed as #74,
   blocked by #63 — when #63 lands we revert to retail's narrower gate.

2. Workaround retirement. Removes TinyMargin (0.05 m inside arrival)
   and the AP-flush before re-send (`SendAutonomousPositionNow`). The
   diff-driven cadence makes both obsolete. Close-range turn-first
   deferred Use is kept (it IS retail — ACE Player_Move.cs:66-87
   mirrors retail's CreateMoveToChain pre-callback rotation), renamed
   `OnAutoWalkArrivedSendDeferredAction` to clarify it's a FIRST send.
   `isRetryAfterArrival` parameter dropped.

3. Far-range Use/PickUp retry. Restored — was load-bearing, not the
   "redundant cleanup" the Group 2 audit thought. Issue #63 means ACE
   drops the first Use as too-far without re-polling on subsequent APs;
   the arrival re-send is what makes far-range Use complete. Logs
   include `(queued for arrival re-send pending #63)` to make this
   explicit. Removes when #63 closes.

4. Screen-rect picker. New `AcDream.Core.Selection.ScreenProjection`
   helper shared by `WorldPicker` and `TargetIndicatorPanel`. The
   `Setup.SelectionSphere` projects to a screen-space square (retail
   anchor `SmartBox::GetObjectBoundingBox` 0x00452e20); picker
   hit-tests the mouse pixel against the same rect the indicator draws,
   inflated by 8 px (`TriangleSize`). Guarantees what-you-see is
   what-you-click — including rect corners that were dead zones under
   the old ray-sphere picker. Per-type radius (1.0/1.6/2.0 m) and
   vertical-offset (0.2/0.9/1.0/1.5 m) heuristic lambdas retired;
   `IsTallSceneryGuid` deleted; `EntityHeightFor` trimmed to 1.5 m × scale
   defensive default. No defensive sphere synth — entities without a
   baked `SelectionSphere` are skipped, matching retail's
   `GfxObjUnderSelectionRay` (0x0054c740).

5. Rotation rate run multiplier (Commit A precursor). `TurnRateFor(running)`
   helper applies retail's `run_turn_factor = 1.5f` (PDB-named
   0x007c8914) under HoldKey.Run, matching `apply_run_to_command` at
   0x00527be0 (line 305098). Effective: walking ≈ 90°/s, running ≈ 135°/s.
   Keyboard A/D + ApplyAutoWalkOverlay both use it.

6. Useability gate (Commit A precursor). `IsUseableTarget` corrected to
   `useability != 0` per `ItemUses::IsUseable` at 256455 — ANY non-zero
   passes (USEABLE_NO=1, USEABLE_CONTAINED=8, etc.), not just the
   USEABLE_REMOTE bit. Cross-checked against 4 call sites in retail
   (ItemHolder::UseObject 0x00588a80, DetermineUseResult 0x402697,
   UsingItem 0x367638, disable-button-state 0x198826). Added
   `ProbeUseabilityFallbackEnabled` diagnostic
   (`ACDREAM_PROBE_USEABILITY_FALLBACK=1`) to measure how often the
   creature/BF_DOOR fallback fires for ACE-seed-DB entities with
   null useability.

CLAUDE.md updated with the graceful-shutdown rule for relaunch:
Stop-Process bypasses the logout packet, leaving ACE's session marked
logged-in for ~3+ min. CloseMainWindow() sends WM_CLOSE so the
shutdown hook runs and the logout packet reaches ACE.

Tests: +3 ScreenProjectionTests + 6 WorldPickerRectOverloadTests = +9.
Core.Net 294/294 pass; Core 1073/1081 (8 pre-existing Physics failures
unchanged). Visual-verified 2026-05-16: rotation rate, useability,
screen-rect click area, double-click + R-key + F-key Use/PickUp at
short and long range — dialogue/door/pickup fire on arrival.

Filed follow-ups #70 (triangle apex/size DAT sprite), #71 (picker
Stage B polygon refine), #72 (cdb omega.z probe), #73 (retail-message
sweep pattern), #74 (per-frame AP chattier than retail — blocked by
#63). Old ray-sphere `WorldPicker.Pick(origin, direction, ...)`
overload kept for back-compat; no callers in acdream proper.

Plan: docs/superpowers/plans/2026-05-16-retail-faithfulness-fixes.md

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-16 13:56:08 +02:00
Erik
e2bc3a9e99 docs(CLAUDE.md): document Ghidra MCP + WireMCP availability
Adds a 'MCP servers (live tooling)' section after the cdb retail debugger. Ghidra MCP (LaurieWired v1.4 HTTP) on :8081 serving patchmem.gpr provides live decomp lookups by address/name/xref without dumping acclient_2013_pseudo_c.txt into context. WireMCP (stdio, Node, tshark wrapper) enables loopback capture against 127.0.0.1:9000 for ACE wire-protocol cross-checks (0xF61C, 0xF74A, 0xF7DE parsing). Both extend the static-decomp + cdb workflow with live introspection.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-16 12:47:12 +02:00
Erik
e0d5d271f3 fix(retail): rotation rate, useability gate, retail toast strings
Two retail divergences fixed from the 2026-05-16 faithfulness audit
(Commit A of the plan at docs/superpowers/plans/2026-05-16-retail-faithfulness-fixes.md).

1. Rotation rate ignored HoldKey.Run. Retail's CMotionInterp::
   apply_run_to_command (decomp 0x00527be0 line 305098) multiplies
   turn_speed by run_turn_factor (1.5, PDB-named symbol at 0x007c8914)
   when input is TurnRight/TurnLeft under HoldKey.Run. Effective
   running rotation is 50% faster (~135°/s vs walking ~90°/s).
   Our keyboard A/D and ApplyAutoWalkOverlay used a fixed walking
   rate.

   New: RemoteMoveToDriver.TurnRateFor(running) helper. Keyboard
   path passes input.Run; auto-walk overlay passes
   _autoWalkInitiallyRunning. The walking-rate base
   (BaseTurnRateRadPerSec = π/2) is unchanged; TurnRateRadPerSec
   constant is preserved as the walking-rate alias for callers
   that don't have run/walk state (NPC remotes).

2. IsUseableTarget gated on `useability & USEABLE_REMOTE (0x20)`,
   which was stricter than retail. Per ItemUses::IsUseable
   (acclient_2013_pseudo_c.txt:256455) cross-referenced with 4
   call sites, retail's IsUseable() semantic is `_useability != 0`.
   But visually retail's USEABLE_NO (1) entities don't approach
   either, because ACE never broadcasts MovementType=6 for them.
   Our client installs a speculative auto-walk BEFORE the server
   responds, so we'd visibly approach + face signs before the
   wire packet was rejected.

   Pragmatic fix: block USEABLE_UNDEF (0) AND USEABLE_NO (1) in
   IsUseableTarget — slightly stricter than retail's
   IsUseable but matches retail's user-visible behaviour
   ("R on sign does nothing"). Documented in the doc-comment so
   a future implementer knows the gap.

3. New IsPickupableTarget gate for F-key path — requires
   USEABLE_REMOTE (0x20) bit. Null-useability fallback for
   BF_CORPSE + small-item ItemTypes (preserves M1 ground-item
   pickup flow when ACE seed DB doesn't publish useability).

4. R-key (UseCurrentSelection) upfront gate now ALWAYS uses
   IsUseableTarget. R is conceptually "use" with smart-routing
   to pickup as a downstream optimization. F-key (SendPickUp)
   uses IsPickupableTarget directly.

5. Retail toast strings on block, centralised in new
   src/AcDream.Core/Ui/RetailMessages.cs:
   - "The X cannot be used" (data 0x007e2a70, sprintf 0x00588ea4)
     fires on UseCurrentSelection / SendUse gate block.
   - "The X can't be picked up!" (sprintf 0x00587353) fires on
     SendPickUp non-pickupable block.
   - "You cannot pick up creatures!" (data 0x007e22b4) fires on
     SendPickUp creature block (was previously silent).
   - Plus 4 inactive retail strings ready for future call sites:
     CannotBeUsedWith (two-target Use), CannotBePickedUp (formal
     pickup variant), CannotBeUsedWhileOnHook_HooksOff +
     CannotBeUsedWhileOnHook_NotOwner (housing). All cite their
     retail data addresses + runtime sprintf addresses.

6. ProbeUseabilityFallbackEnabled diagnostic (env var
   ACDREAM_PROBE_USEABILITY_FALLBACK=1) logs every time the
   null-useability fallback fires. Settles whether the
   fallback for creature + BF_DOOR/LIFESTONE/PORTAL/CORPSE
   entries in ACE's seed DB without useability is hot code
   or theoretical defense.

Test coverage:
- +3 RemoteMoveToDriverTests cover TurnRateFor walking/running/back-compat.
- +7 RetailMessagesTests cover each retail string with retail anchor.
- +1 CreateObjectTests TryParse_WeenieFlagsUsable_ReadsUseableNoValue
  pins parser correctness for USEABLE_NO=1.
- 294/294 Core.Net pass; 24/24 new+touched Core tests pass.
- Pre-existing baseline of 8 Physics test failures unchanged
  (BSPStepUp + MotionInterpreter regression noise from prior
  sessions; out of scope here).

Deferred to a separate session per user direction:
- Click area = indicator-rect retail fidelity. Retail's picker
  uses per-part CGfxObj.drawing_sphere + polygon refine
  (0x0054c740); ours uses single Setup.SelectionSphere ray-
  intersect. The rect corners are dead zones today. Three fix
  options analyzed: screen-space rectangle hit-test, sqrt(2)
  sphere inflation, polygon refine Stage B.

Plan: docs/superpowers/plans/2026-05-16-retail-faithfulness-fixes.md

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-16 12:17:54 +02:00
Erik
f4f4143ac0 feat(B.7): retail-faithful target indicator via Setup.SelectionSphere
Replaces the mesh-AABB approximation with retail's actual selection
mechanism. The user observed the indicator was too small and didn't
scale with the object the way retail does — root cause was using the
wrong source data.

Retail trace (decomp anchors in named-retail/acclient_2013_pseudo_c.txt):
- VividTargetIndicator::Draw at 0x004f6c30 is registered as the
  SmartBox targetting callback (0x004f6df6).
- SmartBox::DoTargettingChecks at 0x00453bb4 calls
  SmartBox::GetObjectBoundingBox (0x00452e20) to compute the rect.
- GetObjectBoundingBox uses CPhysicsObj::GetSelectionSphere
  (0x0050ea40) → CPartArray::GetSelectionSphere (0x00518b80) which
  reads setup->selection_sphere from the DAT, applies part-array
  scale (component-wise on center, Z-scale on radius), then calls
  Render::GetViewerBBox (0x0054b400) to project the sphere as a
  screen-space camera-aligned BBox.
- VividTargetIndicator::OnDraw at 0x004f62b0 inflates that rect by
  one triangle width/height on every side before drawing (eax_21 /
  eax_23 in 0x004f6a0b–0x004f6a99), so the corner triangles sit
  outside the projected sphere with a small gap.

Implementation:
- GameWindow.TryGetEntitySelectionSphere reads setup.SelectionSphere
  from the DAT (Setup type already exposes Origin + Radius),
  applies entity scale, rotates center via entity orientation, and
  produces a world-space sphere.
- TargetIndicatorPanel.TryComputeScreenRectFromSphere projects the
  sphere center via the view-projection matrix and computes
  screenRadius = worldRadius * projection.M22 * viewport.Y /
  (2 * clip.W). M22 = cot(fovY/2) for a standard right-handed
  perspective. Mathematically equivalent to retail's
  Render::GetViewerBBox followed by 2-corner xformPointInternal,
  faster (no double projection).
- TargetInfo carries WorldSphereCenter + WorldSphereRadius (replaces
  the previous WorldAabbMin/Max). Fallback to per-type height
  heuristic still in place if Setup has no baked selection_sphere
  (rare; Radius <= 1e-4f short-circuits).
- Inflate by TriangleSize on every side matches retail's eax_21 +
  eax_23 offsets exactly.
- Triangle right-angle apex flipped to point INWARD toward the
  target (per user feedback) — apex at corner + (±t, ±t),
  hypotenuse along the outer diagonal of the corner.
- TriangleSize 10 → 14 → 8 (retail sprite is small).

Also fixes a parser bug in CreateObject.cs introduced in 58e1556:
BF_INCLUDES_SECOND_HEADER is 0x04000000 per acclient.h:6458 (ACE
ObjectDescriptionFlag.IncludesSecondHeader matches), NOT 0x80000000.
The wrong bit meant the weenieFlags2 4-byte skip never fired for
entities that had the bit set, potentially shifting Useability /
UseRadius reads by 4 bytes. Now correct.

Visual verification (2026-05-16):
- Holtburg town sign — indicator traces the visible sign + pole at
  the right size (matches retail screenshot proportions).
- Sign R-key still silent no-op (B.8 useability gate intact).
- NPCs / doors / items still get correctly-sized indicators.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-15 21:25:00 +02:00
Erik
58e155615d feat(B.8): retail useability gate + tall-scenery indicator scaling
Two retail divergences fixed end-to-end:

1. R-key Use on non-useable entities (signs, banners, decorative
   scenery) was silently sending Use/PickUp to ACE, triggering
   auto-walk + NPC-style chat fallback. Retail's client checks
   ITEM_USEABLE (acclient.h:6478) and silently ignores Use when
   the USEABLE_REMOTE (0x20) bit isn't set. Now ports that gate.

2. Holtburg town sign indicator + click sphere only covered the
   base of the pole because the "everything else" default in
   EntityHeightFor was 1.5 m and the picker's vertical offset
   for default class was 0.2 m. A 3 m sign on a pole was almost
   entirely outside both shapes.

Wire change:
- CreateObject parser now walks the WeenieHeader optional tail
  (per ACE WorldObject_Networking.cs:87-114) up through Useability
  + UseRadius. Captures weenieFlags upfront, then conditionally
  skips PluralName, ItemCapacity, ContainerCapacity, AmmoType,
  Value before reading Useability (u32) and UseRadius (f32).
- CreateObject.Parsed + WorldSession.EntitySpawn record append two
  new optional fields (Useability uint?, UseRadius float?), both
  defaulting to null. Existing call sites unchanged.
- 3 new tests cover: no weenieFlags → null, weenieFlags=0x10 alone
  → useability read, weenieFlags=0x8|0x10|0x20 → walker skips Value
  then reads Useability + UseRadius in correct order.

Behaviour change:
- GameWindow.IsUseableTarget(guid) — authoritative path uses spawn
  .Useability when present (REMOTE bit gate); fallback when null
  permits Use on creatures + BF_DOOR/LIFESTONE/PORTAL/CORPSE for
  M1 flow continuity.
- UseCurrentSelection (R-key dispatcher) and SendUse + SendPickUp
  (double-click + F-key direct paths) gate on IsUseableTarget,
  silent early-return matching retail. isRetryAfterArrival skips
  the gate (re-fires only previously-gated actions).
- TargetIndicatorPanel.EntityHeightFor default branch 1.5 m → 3 m
  for non-creature non-flat non-small-item entities (sign-class).
  Scale > 1 still grows proportionally.
- WorldPicker callbacks: new IsTallSceneryGuid branch lifts sphere
  centre to 1.5 m with 1.6 m radius for sign-class entities,
  mirroring the indicator's 3 m default so click sphere matches
  the visible box.

Tests: 293/293 pass in AcDream.Core.Net.Tests (+3 new walker
tests). dotnet build clean.

Retail anchors:
- acclient.h:6478 — ITEM_USEABLE enum (USEABLE_REMOTE = 0x20)
- acclient.h:6431-6463 — PWD bitfield (BF_DOOR etc.)
- ACE WorldObject_Networking.cs:87-114 — wire field order
- ACE WeenieHeaderFlag — Usable = 0x10, UseRadius = 0x20

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-15 20:07:32 +02:00
Erik
520badd566 docs(B.6+B.7): ship handoff — 36 commits, faithfulness audit, workaround retirement plan
Covers the 2026-05-15 session in full: B.6 local-player auto-walk on
inbound MoveToObject (issue #63 working), B.7 Vivid Target Indicator
MVP, WorldPicker tightening (#59 closed).

Includes:
- 36-commit table cf22f9c..e49c704
- Wire-format facts (MovementType 6/7/8, WalkRunThreshold, heartbeat cadence)
- Auto-walk state machine current shape + GameWindow wiring
- Picker + target-indicator current shapes
- Honest faithfulness audit (user-requested) — workarounds are our bugs not ACE's
- Closed issues (#59, #62, #67) + open follow-ups (#65, #66, #68, #69)
- Reproducibility commands + diagnostic env vars
- Files touched
- Workaround retirement plan (single fix retires four workarounds: per-tick outbound)
- Next-session entry points (sign indicator size, #66 MovementType=8, etc.)

Single biggest lesson surfaced in session: when a workaround starts feeling
load-bearing, find the heartbeat-cadence root cause first. The 10 Hz
AutonomousPosition bump (301281d) closed #67 and tightened firing distance
for every other interaction in one commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-15 18:29:53 +02:00
Erik
e49c704b39 fix(B.6): speculative auto-walk uses WalkRunThreshold=15 to match ACE
User report: 'No we only walk, not running from the correct threshold.
regression?'

Cause: InstallSpeculativeTurnToTarget passed walkRunThreshold=9999,
which made BeginServerAutoWalk evaluate the initial-distance run/walk
decision as walk-mode (no distance > 9999). ACE's MovementType=6
arrives ~100 ms later with the real wire threshold (15.0) and
overwrites, but the body had already started walking by then; for
far targets near the 15 m boundary, the speculative walking shortened
the distance enough that ACE's overwrite re-evaluated to walk also.

Fix: pass 15.0 in the speculative install — matches ACE's default
MoveToParameters.SetDefaults() for non-combat Use/PickUp.

Effect: a >15 m target now correctly enters run-mode at the
speculative install, and the ACE overwrite preserves that decision.
The body runs all the way, stopping at the target as before.
2026-05-15 15:23:30 +02:00
Erik
7158c46d46 fix(B.6): smooth local rotation — remove 20° snap-on-approach (not retail)
User report: 'quick snap at like 30 degrees to the last position. Not
a smooth turn. Did you verify with retail?'

Verified against retail decomp at MoveToManager::HandleTurnToHeading
(0x0052a0c0). Retail's pattern:

  - Body rotates continuously via a TurnLeft/TurnRight motion cycle.
  - The ONLY snap is set_heading(target, 1) after heading_greater()
    detects we've passed the target (overshoot protection).
  - No 'snap when close to target' tolerance band — that's purely
    a sparse-update fudge in RemoteMoveToDriver (the remote-creature
    path with ~1Hz UpdatePosition broadcasts).

I'd copied the snap-on-approach tolerance from RemoteMoveToDriver to
ApplyAutoWalkOverlay. Wrong: local player rotates at per-tick
resolution, no sparse-update problem to compensate for. Removed.

The MathF.Min(|delta|, maxStep) clamp naturally lands the body on
the target heading without overshoot in the final partial tick, so
no separate snap-on-overshoot branch is needed for our integrator
either.

Visible effect: 1.8m humanoid rotating ~180° at π/2 rad/s takes ~2 s
of smooth turn now, instead of ~1.3 s of turn + instant 20° snap at
the end.
2026-05-15 15:19:29 +02:00
Erik
cffb10ff18 fix(B.6): tighter 5° alignment + defer Use until rotation completes; file #69 turn anim
User report: 'You should be face to face with the NPC before sending
use. So first is rotation, when you are facing, then using.' and
'it does not face it completely.'

Two changes:

1. Split alignment thresholds in ApplyAutoWalkOverlay:
     walkAligned  (30°) — gate for synthesised Forward+Run motion
                          during far-range approach; body walks
                          while finishing residual turn within 30°.
     aligned      (5°)  — gate for arrival-fire. Final facing
                          before the auto-walk ends and the action
                          re-sends. Matches retail's tight pre-Use
                          rotation tolerance.
   Within-arrival check still requires alignment; without alignment
   the body holds in turn-only mode regardless of distance.

2. Defer wire Use/PickUp packet for CLOSE-range targets. SendUse
   and SendPickUp now check IsCloseRangeTarget(guid): if the player
   is already within the target's use-radius, we install the
   speculative overlay, set _pendingPostArrivalAction, and RETURN
   without sending the wire packet. AutoWalkArrived fires after the
   local rotation completes (alignment within 5°); the existing
   re-send handler then fires SendUse with isRetryAfterArrival=true,
   sending the wire packet at that moment. Effect: rotate first,
   THEN Use — the NPC/door/item only sees the action after the
   character has turned to face it.

   Far-range path unchanged: send immediately, ACE auto-walks,
   arrival re-sends.

Filed #69: turn animation (leg/arm cycle while pivoting). The body
now rotates but doesn't play the TurnLeft/TurnRight cycle the user
wants to see. Separate scope — needs motion-interpreter integration.
2026-05-15 15:15:30 +02:00
Erik
5b908bcca2 fix(B.6): close-range turn-to-face — install overlay on Use/PickUp send
User report: 'It should always face the NPC. When I'm close I'm not
facing. But now it turns if I'm far away.'

Cause: ACE skips MoveToChain when the player is already within
WithinUseRadius (Player_Move.cs:66 shortcut) — it rotates the body
server-side via Rotate(target) but doesn't broadcast a MovementType=6
to us, so our auto-walk overlay never installs. The local body never
turns; the player remains facing wherever the camera/mouse last left
them.

Fix has two pieces:

1. PlayerMovementController.ApplyAutoWalkOverlay: arrival is now
   gated on BOTH within-radius AND aligned. Previously a body that
   started already in-range ended the overlay before turning. Now
   it turns in place, then ends once facing.

   Also: forward motion stays suppressed while withinArrival (we
   just need to finish the turn, no point stepping forward into a
   target we're already touching).

2. GameWindow.SendUse / SendPickUp: install a speculative auto-walk
   overlay at send time via new InstallSpeculativeTurnToTarget
   helper. For far targets ACE's MovementType=6 arrives moments
   later and overwrites with its wire-supplied use-radius. For
   close targets our overlay is the only thing that runs — body
   turns, then ends.

The per-type use-radius mirrors the picker's heuristic (3 m
creature / 2 m large flat / 0.6 m item).
2026-05-15 12:05:37 +02:00
Erik
32352af583 fix(B.6): turn-first auto-walk + tiny margin; close #67 doors; file #68 remote arrival
10 Hz heartbeat (301281d) made ACE see us in-radius before its
MoveToChain timeout; user confirmed doors work now. Closing #67 — root
cause was 1 Hz position outbound on our side, not anything door-
specific. Same fix unblocked door + NPC.

Two visible refinements:

1. Turn-first gate. User report: 'when I use from far range, I should
   face that object and then start moving. Now it starts running
   before facing is complete.'
   ApplyAutoWalkOverlay now suppresses Forward motion when the
   heading delta to the target is > 30°. Body turns IN PLACE first,
   then walks forward once roughly aligned. Within the 30° band the
   body walks while finishing the residual turn. Matches the user-
   observed retail rhythm.

2. Arrival margin shrunk 0.2 m → 0.05 m. User report: 'NPC dialogue
   fires, but still a bit too close. In retail it fires from a longer
   range.' With the 10 Hz heartbeat the server-side Player.Location
   tracks us within ~100 ms, so the bigger safety margin is no longer
   needed — only a tiny epsilon to absorb the sub-tick race between
   local arrival fire and the next outbound packet.

Filed #68: remote players' running animation doesn't transition to
Ready on auto-walk arrival when observed from acdream. Separate
visual bug — server-side action completes correctly; just the cycle
on the dead-reckoned remote body doesn't flip back to idle.
2026-05-15 11:49:55 +02:00
Erik
301281d8d0 fix(B.6+B.7): bump AutonomousPosition heartbeat 1Hz -> 10Hz while moving
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.
2026-05-15 11:39:14 +02:00
Erik
64c9793248 fix(B.6+B.7): shrink arrival safety margin; file #66 rotation, #67 door
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.
2026-05-15 11:28:06 +02:00
Erik
39ff3a5505 fix(B.6+B.7): arrival predicate uses safety margin INSIDE ACE's WithinUseRadius
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.
2026-05-15 11:19:04 +02:00
Erik
a0fa3d68a7 fix(B.6+B.7): flush AutonomousPosition on arrival before re-sending action
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
2026-05-15 07:56:02 +02:00
Erik
2dc28bb61f fix(B.6+B.7): re-send action on local arrival; scale indicator box by entity Scale
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).
2026-05-15 07:45:27 +02:00
Erik
5f83766de5 docs: file #65 — local player doesn't turn to face on close-range Use 2026-05-15 07:36:14 +02:00
Erik
211fe240b8 fix(B.6+B.7): run-all-the-way auto-walk, per-type indicator height, R = smart interact
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.
2026-05-15 07:35:38 +02:00
Erik
1a0656a3ce fix(picker): lift sphere centre to mid-body so chest/head clicks hit
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.
2026-05-15 07:23:41 +02:00
Erik
23cb1e9636 fix(B.7): square indicator box + bigger pick sphere for doors/lifestones/portals + diag
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.
2026-05-15 07:13:23 +02:00
Erik
631571a6ef docs: close #59 — picker radius tightened in 5e29773 2026-05-15 07:05:04 +02:00
Erik
5e29773e92 fix #59: tighten WorldPicker radius from 5 m to 0.7 m
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.
2026-05-15 07:04:34 +02:00
Erik
4bc95eca01 fix(B.7): scale indicator box from projected entity height, not fixed pixels
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.
2026-05-15 07:02:35 +02:00
Erik
c7e5f9f00f feat(B.7): TargetIndicatorPanel — corner triangles around selected entity
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.
2026-05-15 06:54:24 +02:00
Erik
8544a785d7 feat(B.7): RadarBlipColors — port of gmRadarUI::GetBlipColor
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.
2026-05-15 06:49:46 +02:00
Erik
37177a418e docs(B.7): design spec for Vivid Target Indicator (selection feedback)
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.
2026-05-15 06:46:55 +02:00
Erik
5612ce718a feat(B.6): honor wire WalkRunThreshold — walk vs run per retail semantics
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.
2026-05-15 06:22:07 +02:00
Erik
f18de7ccde fix(B.6 slice 2): don't cancel autowalk on the companion InterpretedMotionState
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.
2026-05-14 20:20:21 +02:00
Erik
b936ef8b0b feat(B.6 slice 2): local-player auto-walk on inbound MoveToObject
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.
2026-05-14 18:50:59 +02:00
Erik
d82b0648b5 docs(B.6): record Slice 1 trace findings — ACE sends mtRun=0.00, no UP echo
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.
2026-05-14 18:45:17 +02:00
Erik
1b4f3bac6b feat(B.6 slice 1): DebugPanel mirror for ProbeAutoWalk checkbox
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.
2026-05-14 18:03:05 +02:00
Erik
eda8278a64 feat(B.6 slice 1): ACDREAM_PROBE_AUTOWALK diagnostic baseline
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.
2026-05-14 17:59:57 +02:00
Erik
9e1d33a5f7 docs(B.6): retail decomp settles Option A; revise spec with 4-slice plan
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.
2026-05-14 17:56:35 +02:00
Erik
281d125e9b docs(B.6): design spec for local-player MoveToObject auto-walk (issue #63)
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.
2026-05-14 17:42:16 +02:00
Erik
5053e40e6f docs: close #62 — PARTSDIAG null-guard landed in ec9fd52 2026-05-14 17:13:11 +02:00
Erik
ec9fd52cb2 fix #62: null-guard the PARTSDIAG read of ae.Animation
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.
2026-05-14 17:12:48 +02:00
Erik
e55ad48ade fix(B.5): make creature-pickup guard silent (retail-faithful)
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.
2026-05-14 17:09:45 +02:00
Erik
ab7c04fb4f docs(M1): reflect chat/toast revert + the actual B.5 polish (creature pickup guard) 2026-05-14 17:04:44 +02:00
Erik
a01ebd5e08 fix(B.5): block pickup of creatures client-side; show 'Can't pick that up' toast
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.
2026-05-14 17:04:20 +02:00
Erik
20ecb23396 Revert "feat(B.5): pickup feedback chat line + toast ("You pick up the X.")"
This reverts commit 87ba5c9a98.
2026-05-14 17:02:29 +02:00
Erik
7be13938bc docs(M1): record all 4 demo targets met, list deferred polish
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).
2026-05-14 16:55:21 +02:00
Erik
87ba5c9a98 feat(B.5): pickup feedback chat line + toast ("You pick up the X.")
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.
2026-05-14 16:54:17 +02:00
Erik
cf22f9c031 Merge branch 'claude/phase-b5-pickup' — Phase B.5 pickup 2026-05-14 16:23:34 +02:00