Commit graph

111 commits

Author SHA1 Message Date
Erik
6636e50c2a feat(D.5.3a): selected-object meter — Health bar + name on the action bar
Port of gmToolbarUI::HandleSelectionChanged (acclient_2013_pseudo_c.txt:198635).
When the player selects a world object the action bar's bottom strip shows the
object name + (for player/pet/attackable targets) a live Health meter; deselect
clears it. Mana (#140) + stack slider deferred.

- SelectedObjectController (new): clear-then-populate on selection change; sets
  name (UiText child, VitalsController pattern), overlay state (ObjectSelected /
  StackedItemSelected via UiDatElement.ActiveState), shows the health meter and
  sends QueryHealth for health targets. Subscribes via a delegate seam (no
  GameWindow coupling).
- GameWindow: _selectedGuid field -> SelectedGuid property + SelectionChanged
  event (fires on actual change only); 3 write sites converted, reads untouched.
  All selection-write paths (LMB pick, Tab/Q, despawn-clear via Tick()) run on
  the render thread, so the event-driven UI mutation is single-threaded.
- WorldSession.SendQueryHealth (0x01BF) — wraps SocialActions.BuildQueryHealth.
- DatWidgetFactory.BuildMeter: handle the single-image toolbar meter shape
  (back-track on the element's own DirectState, fill on one Type-3 child). The
  sprites go in the TILE slot (DrawMode=Normal tiles to full bar geometry per
  UIElement_Meter::DrawChildren) — a left-cap assignment would gap/clamp a
  sub-140px sprite. Vitals 3-slice path unchanged.
- ToolbarController.HiddenIds: A1 (health) now owned by SelectedObjectController;
  A2 (mana) + A4 (stack) stay hidden (deferred) so their dat back-tracks don't
  render as stray empty bars.

Adversarial Opus review found + fixed: the mana-meter orphan (A2 left unhidden)
and the meter tile-vs-cap render bug (C1). Divergence rows AP-46 (health gate
approximation: IsLiveCreatureTarget vs IsPlayer||pet||attackable) + AP-47
(meter shown on select vs on UpdateHealth reply). Spec §5 corrected.

Build + full test suite green (2,684 passed / 4 skipped). Health meter render
fidelity (full-width fill + fraction mapping) pending the user's visual gate.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-18 22:47:24 +02:00
Erik
cbbfe4cd49 feat(D.5.4): PlayerDescription = membership manifest; drop WeenieClassId=ContainerType misuse
The old seeding block set WeenieClassId = inv.ContainerType (a 0/1/2
container-kind discriminator, not a weenie class id) and used MoveItem
for the equipped block. Replace both loops with RecordMembership calls:
inventory guids get a bare stub (WeenieClassId stays 0); equipped guids
get the equip slot set directly. Weenie data arrives via CreateObject /
ObjectTableWiring, not PlayerDescription.

New test PlayerDescription_SeedsMembership_NotWeenieClassIdMisuse proves:
(a) inv guid is registered, (b) WeenieClassId==0 not ContainerType, and
(c) equipped guid CurrentlyEquippedLocation is set to MeleeWeapon.
No existing tests pinned the old behavior; all 15 GameEventWiringTests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-18 16:38:54 +02:00
Erik
82f5968316 feat(D.5.4): ObjectTableWiring (CreateObject=upsert, Delete=evict, 0x02CE) off GameWindow
CreateObject ingestion moves to Core.Net; GameWindow drops the EnrichItem call +
inline 0x02CE handler. Fixes the Coldeve blank-icon root cause: items with no PD
stub are now created, not dropped.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-18 16:34:56 +02:00
Erik
b00a373c5a feat(D.5.4): forward full item field set through WorldSession.EntitySpawn
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-18 15:53:07 +02:00
Erik
91970c4fe9 feat(D.5.4): capture full item field set in CreateObject parser
WeenieClassId + Value/StackSize/MaxStackSize/Burden/capacities/Container/Wielder/
ValidLocations/CurrentWieldedLocation/Priority/Structure/Workmanship. Nullable =
flag absent (don't clobber on merge). Cursor walk unchanged; +cursor-integrity test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-18 15:50:27 +02:00
Erik
b506f53633 refactor(D.5.4): rename ItemRepository->ClientObjectTable, ItemInstance->ClientObject
Broaden naming to the data side of every server object (retail weenie_object_table
shape). Pure rename; no behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-18 15:36:16 +02:00
Erik
e7b6e83cf8 feat(D.5.2): thread UiEffects through EntitySpawn + route 0x02CE PublicUpdatePropertyInt
- EntitySpawn record: add UiEffects = 0 field (after IconUnderlayId)
- EntitySpawn construction site: thread parsed.Value.UiEffects as the new tail arg
- WorldSession: declare ObjectIntPropertyUpdate payload record + ObjectIntPropertyUpdated event (after StateUpdated)
- Message loop: add else-if branch for PublicUpdatePropertyInt.Opcode (0x02CE), parses + fires ObjectIntPropertyUpdated

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-17 18:33:18 +02:00
Erik
242bc9286d feat(D.5.2): PublicUpdatePropertyInt (0x02CE) parser
New standalone parser for the server's live PropertyInt update targeting
a VISIBLE object (carries guid). Wire layout: u32 opcode + u8 sequence +
u32 guid + u32 property + i32 value (17 bytes total).

The sequence byte is parsed-past but not honored (latest-wins; DR-4).
The companion PrivateUpdatePropertyInt (0x02CD) targets the player's own
object (no guid) and is not parsed here.

Three tests: uiEffectsUpdate (round-trip guid/prop/value), wrongOpcode
(returns null), truncated (returns null on 16-byte input).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-17 18:29:23 +02:00
Erik
8df0b64676 feat(D.5.2): capture UiEffects from CreateObject weenie header
Previously, weenieFlags bit 0x80 (UiEffects) was read + discarded with
`pos += 4`. Now it is captured into `uiEffects` and surfaced as
`Parsed.UiEffects` — the sole wire path for the effect bitfield since
PropertyInt.UiEffects (18) has no [AssessmentProperty] and never appears
in appraise responses.

Test builder gains `uint uiEffects = 0` param; write line updated to use
it. Three new parse tests: UiEffects_Captured, UiEffectsThenIconOverlay
(cursor-arithmetic regression), and NoUiEffectsBit_LeavesUiEffectsZero.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-17 18:28:31 +02:00
Erik
8a42066192 feat(D.5.1): parse item IconOverlay/IconUnderlay from CreateObject -> faithful icon overlay layer
The CreateObject optional-tail walker previously stopped at UseRadius (~20 fields
before IconOverlay). This left ItemInstance.IconOverlayId/IconUnderlayId always 0,
so IconComposer's underlay/overlay layers were never drawn on toolbar icons.

Exact field order verified against ACE WorldObject_Networking.cs:87-219 (the
serializer is the authority; acdream connects to a local ACE server):
  UseRadius → TargetType(u32) → UiEffects(u32) → CombatUse(sbyte) →
  Structure(u16) → MaxStructure(u16) → StackSize(u16) → MaxStackSize(u16) →
  Container(u32) → Wielder(u32) → ValidLocations(u32) →
  CurrentlyWieldedLocation(u32) → Priority(u32) → RadarBlipColor(u8) →
  RadarBehavior(u8) → PScript(u16) → Workmanship(f32) → Burden(u16) →
  Spell(u16) → HouseOwner(u32) → HouseRestrictions(variable RestrictionDB) →
  HookItemTypes(u32) → Monarch(u32) → HookType(u16) →
  IconOverlay(PackedDwordKnownType) ← CAPTURE →
  IconUnderlay from weenieFlags2 bit 0x01 ← CAPTURE

RestrictionDB handled correctly: Version(u32) + OpenStatus(u32) + MonarchId(u32)
+ count(u16) + numBuckets(u16) + count×8 bytes entries. Length-aware skip, not a
fixed constant.

weenieFlags2 is now CAPTURED (not skipped) when IncludesSecondHeader
(objDescFlags bit 0x04000000) is set, so the IconUnderlay bit can be tested.

The entire extended walk is inside try/catch: truncated packets degrade to
IconOverlayId=0 / IconUnderlayId=0 (no overlay drawn), never corrupting.

Threading: CreateObject.Parsed → WorldSession.EntitySpawn → GameWindow
OnLiveEntitySpawned → Items.EnrichItem — both ids thread through all three
seams. EnrichItem extended with optional iconOverlayId + iconUnderlayId params
(defaulted 0, backward-compatible).

No change to IconComposer or ToolbarController (they already consume the ids).

Tests: 4 new CreateObject tests (IconOverlay only, overlay+underlay, no-overlay
regression, intermediate-fields cursor arithmetic). Full suite: 0 failures,
2636 passed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-17 14:34:47 +02:00
Erik
6c485c2f06 feat(D.5.1): persist PlayerDescription shortcuts (were parsed then discarded)
Add optional `onShortcuts` callback to `GameEventWiring.WireAll`; invoke
it with `parsed.Shortcuts` after the inventory/equipped loops in the
PlayerDescription handler.  `GameWindow` holds the list in a new
`Shortcuts` property (initialized to empty) so the toolbar (D.5.1 Task 5)
can read hotbar slots without keeping a parser reference.  Existing callers
compile unchanged — the parameter defaults to null.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-16 22:01:40 +02:00
Erik
5382d0a9d2 feat(D.5.1): thread CreateObject IconId into ItemRepository via spawn event
Added uint IconId = 0 (defaulted, last positional param) to the EntitySpawn
record so existing call sites outside WorldSession compile unchanged. The
WorldSession invoke now passes parsed.Value.IconId as the final arg.
OnLiveEntitySpawned calls Items.EnrichItem unconditionally — it's a no-op
for non-item spawns (players/NPCs/furniture aren't in the repo), so the call
is safe for every incoming CreateObject.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-16 21:54:48 +02:00
Erik
da171cb4e3 feat(D.5.1): capture IconId in CreateObject.Parsed (was discarded at cs:516)
ReadPackedDwordOfKnownType at the old line 516 was throwing the icon dat
id away. Declare iconId before the try-block, assign it there, and pass
IconId: iconId in the Parsed initializer so downstream UI (action bar /
equipment panels) can read the 0x06xxxxxx dat id without a separate lookup.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-16 21:44:01 +02:00
Erik
3be700020b fix(physics): close #77 — auto-walk honors ACE CanCharge bit; zero velocity in turn-in-place
Two related close-range bugs reported in #77 share a root in
PlayerMovementController.DriveServerAutoWalk + BeginServerAutoWalk:

1. **Walk-vs-run misclassification.** BeginServerAutoWalk decided
   `_autoWalkInitiallyRunning = (initialDist - distanceToObject) >= 1.0f`,
   forcing run at any chase past ~1.6 m. ACE's wire-level walk-vs-run
   answer is the MovementParameters CanCharge bit (0x10), which
   Creature.SetWalkRunThreshold sets when server-side player→target
   distance >= WalkRunThreshold/2 (= 7.5 m default). Retail's
   MovementParameters::get_command (decomp 0x0052aa00) gates the run
   path on CanCharge first; the inner walk_run_threshold check
   practically always walks given ACE's 15 m default. The hardcoded
   1.0 m threshold pushed run into the 3-5 m walk-range the user
   reported should walk.

2. **Velocity leak in turn-in-place phase.** When the auto-walked body
   crossed the destination, desiredYaw flipped ~180°, walkAligned
   dropped to false, and the `if (!moveForward) return true;` branch
   returned without zeroing body velocity. The body kept the prior
   frame's running velocity (RunAnimSpeed × runRate ≈ 11 m/s) and
   slid 4-5 m past the target before the turn-around rotation
   completed — the "runs and slides away, runs back, picks up"
   symptom in #77 bug B.

Changes:

- `CreateObject.ServerMotionState.CanCharge`: new bool prop reading
  bit 0x10 of MoveToParameters. Cross-ref ACE
  MovementParams.CanCharge = 0x10.
- `PlayerMovementController.BeginServerAutoWalk`: replaces the unused
  `walkRunThreshold` parameter with `bool canCharge`; sets
  `_autoWalkInitiallyRunning = canCharge`.
- `PlayerMovementController.DriveServerAutoWalk` turn-in-place branch:
  calls `_motion.DoMotion(Ready, 1.0)` and zeros body horizontal
  velocity (preserving Z for gravity). No-op for case (a) initial-turn
  with stationary body; fixes (b) overshoot recovery and (c) settling
  cases.
- `GameWindow.OnLiveMotionUpdated`: passes
  `update.MotionState.CanCharge` through; [autowalk-begin] trace
  shows `canCharge=` instead of `walkRunThresh=`.
- `GameWindow.InstallSpeculativeTurnToTarget`: predicts ACE's
  CanCharge from local distance using ACE's exact 7.5 m rule, so the
  speculative install agrees with the wire-triggered overwrite that
  arrives moments later.

Visual-verified at Holtburg 2026-05-18: walk-range NPC click walks +
fires Use, walk-range F-key pickup walks + no overshoot, far-range
(8-10 m) pickup still runs. Test baseline unchanged (8 Core pre-existing
failures, 0 net-new failures across Core/Net/UI/App suites).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-18 09:33:33 +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
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
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
f7636a9e78 fix(B.5): handle PickupEvent 0xF74A so picked-up items despawn locally
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.
2026-05-14 16:13:16 +02:00
Erik
e8a20f26c7 feat(B.5): InteractRequests.BuildPickUp — PutItemInContainer 0x0019
TDD: failing test first (CS0117 on BuildPickUp + PutItemInContainerOpcode),
then implementation. Wire layout matches ACE GameActionPutItemInContainer:
0xF7B1 envelope + seq + 0x0019 opcode + itemGuid + containerGuid + placement
(24 bytes). For F-key ground-pickup, caller passes player's server guid as
containerGuid; Task 2 (GameWindow wiring) will handle that dispatch.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 15:01:24 +02:00
Erik
536a608093 feat(phys L.2g slice 1): WorldSession dispatches SetState (0xF74B) + hex probe
Three changes folded into one commit:

1. New public StateUpdated event on WorldSession + dispatcher branch
   for op == SetState.Opcode. Mirrors the VectorUpdated /
   MotionUpdated event pattern. GameWindow will subscribe in the next
   commit and feed the parsed (guid, newState) pair to
   ShadowObjectRegistry.UpdatePhysicsState.

2. One-shot probe-gated hex-dump (ACDREAM_PROBE_BUILDING) emits the
   first inbound SetState message's body bytes. Originally planned
   as a separate slice 1.5 confidence-check on holtburger's claimed
   12-byte payload vs ACE's GameMessageSetState.cs. Folded into the
   dispatcher to avoid re-touching the same branch. The new
   _setStateHexDumped guard keeps the log clean — auto-close every
   30s would otherwise produce noise.

3. Doc-comment polish on SetState.cs requested by Task 1's code
   review: remove false uncertainty about ACE's sequence-field width
   (ACE's UShortSequence.CurrentBytes provably writes 2 bytes via
   BitConverter), and align the 'total body size' phrasing with
   VectorUpdate.cs's convention. Folded here to avoid churning the
   file twice this slice.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-12 22:28:04 +02:00
Erik
2459f287e4 feat(phys L.2g slice 1): inbound SetState (0xF74B) parser
DTO + TryParse for the GameMessageSetState wire message. The server
broadcasts this when an already-spawned entity's PhysicsState changes
post-CreateObject — chiefly when a door's Ethereal bit toggles on Use.

Wire format per holtburger SetStateData (validated against retail-format
servers): u32 opcode + u32 guid + u32 state + u16 instanceSequence + u16
stateSequence = 16 bytes total. Mirrors the existing VectorUpdate.cs
template.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-12 22:15:31 +02:00
Erik
078919cc18 feat(net): #13 register PD trailer inventory+equipped in ItemRepository
After PlayerDescription is dispatched, the Inventory and Equipped lists
produced by the parser are now fed into ItemRepository via AddOrUpdate +
MoveItem so inventory/paperdoll panels see items after login.

Acceptance test PlayerDescription_RegistersInventoryEntries_InItemRepository
confirms ItemCount goes 0→2 for a synthetic PD with two inventory entries.
282 Net.Tests pass.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-10 09:43:46 +02:00
Erik
91693ea44c feat(net): #13 heuristic inventory locator after gameplay_options blob
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-10 09:37:46 +02:00
Erik
d9a5e40203 feat(net): #13 strict inventory+equipped reader (no GAMEPLAY_OPTIONS)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-10 08:49:10 +02:00
Erik
98eebef740 feat(net): #13 read options2 gated on CHARACTER_OPTIONS2 flag
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-10 08:46:32 +02:00
Erik
b17dc3b152 feat(net): #13 read optional spellbook_filters u32 2026-05-10 08:44:05 +02:00
Erik
75e8e260f2 feat(net): #13 read desired_comps list in PD trailer
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-10 08:39:31 +02:00
Erik
8cbb991d95 feat(net): #13 read hotbar spells (SPELL_LISTS8 + legacy path)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-10 08:35:03 +02:00
Erik
f7a5eea8e8 feat(net): #13 read shortcuts list (SHORTCUT bit) in PD trailer
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-10 08:28:25 +02:00
Erik
9a0dfe03da refactor(net): #13 Parsed.TrailerTruncated + diag logging
Code-quality review followup on Task 2 (becbde6) — addresses I1 (the
forward-looking concern that Tasks 3-9's inner-catch will leave partial
lists visible to callers with no signal) and M1 (silent inner catch).

Changes:
  - Parsed gains a trailing `bool TrailerTruncated` field. Both
    construction sites pass `false` by default; the trailer try/catch
    flips a local `trailerTruncated` to `true` on FormatException and
    feeds it into the final return.
  - Inner catch logs `pos`/`payload.Length`/exception message under
    ACDREAM_DUMP_VITALS=1, mirroring the outer catch's diagnostic
    pattern.
  - Task 2 test strengthened to assert defaults on Options2 /
    SpellbookFilters / HotbarSpells / DesiredComps / GameplayOptions /
    Equipped + TrailerTruncated=false (M2 followup — gives Tasks 3-9
    a regression guard if they consume into the wrong field).
  - New test `TryParse_TrailerAbsent_LessThan8BytesAfterEnchantments_*`
    documents the contract that <8 bytes after enchantments means the
    trailer is absent (not truncated): TrailerTruncated stays false,
    upstream attribute data survives.
  - Plan updated in lockstep so Tasks 3-11 implementers see the
    `trailerTruncated` local and the new return-arg position.

271/271 AcDream.Core.Net.Tests pass.
2026-05-10 08:26:08 +02:00
Erik
becbde60a4 feat(net): #13 read OptionFlags + Options1 after enchantments
First step of the PD trailer walk. Wraps trailer reads in their own
try/catch so a malformed trailer does not null out the upstream
attribute/skill/spell/enchantment data we already extracted.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-10 08:18:38 +02:00
Erik
65870349a8 refactor(net): #13 rename Shortcut → ShortcutEntry, expand doc citations
Code review nit-fix on top of d3b58c9 — addresses two issues from the
quality review of Task 1:

  I1 (Important): the record struct `Shortcut` was a homograph with the
  flag member `CharacterOptionDataFlag.Shortcut`. Both names live inside
  `PlayerDescriptionParser`'s scope. Rename to `ShortcutEntry` aligns
  with `InventoryEntry`/`EquippedEntry` and removes the trap before
  Task 3's walker references both names in the same method body.

  M2 (Minor): `EquippedEntry` had no holtburger source citation; added
  one referencing events.rs:180-190. Also expanded `InventoryEntry`'s
  comment with the strict reader's validation reference.

Plan doc updated in lockstep so Task 3+ implementers see the new name.
8/8 PlayerDescriptionParser tests still pass.
2026-05-10 08:16:01 +02:00
Erik
d3b58c97e0 feat(net): #13 scaffold trailer fields on PlayerDescriptionParser.Parsed
No behavior change yet — adds CharacterOptionDataFlag, Shortcut/Inventory/
EquippedEntry records, and extends Parsed with trailer fields filled with
empty defaults. Sets up the per-section TDD walk in subsequent commits.
2026-05-10 08:06:33 +02:00
Erik
e471527924 feat(net): wire 0xF625 ObjDescEvent for live appearance updates
Retail-driven players observed from acdream rendered with stale
appearance — wrong skin/hair palettes, missing clothing — because
ACE's mid-session appearance broadcasts (equip/unequip/tailoring/
recipe/option-toggle) ride opcode 0xF625 ObjDescEvent and acdream
silently dropped them. Initial CreateObject carries the appearance
at spawn time, but every later equip change only updates via 0xF625
(per Skunkwors protocol docs in ACE/.../GameMessageObjDescEvent.cs).
Retail handles via SmartBox::HandleObjDescEvent (named-retail 0x453340).

Why: the retail observer sees the *server-relayed* view of remotes,
not retail's local build, so dropping ObjDescEvent freezes appearance
at the partial state in the first CreateObject.

How:
- Extract CreateObject's ModelData parsing into reusable
  CreateObject.ReadModelData(span, ref pos) returning
  (BasePaletteId, SubPalettes, TextureChanges, AnimPartChanges).
- Add ObjDescEvent.cs (parser for 0xF625):
  body = u32 opcode | u32 guid | ModelData | u32 instanceSeq | u32 visualDescSeq.
- WorldSession.AppearanceUpdated event + dispatcher branch.
- GameWindow.OnLiveAppearanceUpdated splices new ModelData onto the
  cached spawn and replays via OnLiveEntitySpawned. The dedup at the
  start of OnLiveEntitySpawnedLocked tears down the old GPU/animated/
  collision state cleanly before rebuild.
- _lastSpawnByGuid cache populated at spawn-end and tracked through
  UpdatePosition so re-applies use current position (no pop-back to
  login spot on equip toggle).
- ACDREAM_DUMP_APPEARANCE=1 env var prints structured SP/TC/APC
  decode for every 0xF625 — replaces the earlier raw-hex preview.
- ACDREAM_DUMP_CLOTHING extended with setup.Parts.Count, flatten.Count,
  and per-part triangle counts for offline polygon-budget audit.

Tests: 4 new ObjDescEvent tests (round-trip + parser drift guard);
269 net tests green. User-verified live: skin/hair colors match
retail's character data; equip/unequip no longer pops position.

Note: a separate "puffy arms / bulky body" geometry issue remains
where base body parts visibly overlap clothing meshes — different
root cause, tracked separately.
2026-05-06 10:46:14 +02:00
Erik
5d717312cc feat(net): plumb IsGrounded through EntityPositionUpdate (L.3.2 Task 2)
PositionFlags.IsGrounded (0x04) was already parsed by UpdatePosition
but not exposed through the Parsed record or EntityPositionUpdate.
Adds the bool field to both records so OnLivePositionUpdated can
consume it for retail-faithful MoveOrTeleport routing
(acclient @ 0x00516330: has_contact=false → no-op during airborne arc).

Consumed in subsequent task (L.3.1+L.3.2 Task 3).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-03 10:15:02 +02:00
Erik
851e88364d feat(net): L.3b — capture per-object friction + elasticity from CreateObject
Companion to L.3a (a1c27b3) which ported the velocity-reflection bounce.
Previously the CreateObject parser did `pos += 4` for both Friction and
Elasticity floats — silently dropping the wire data so every entity got
the PhysicsBody constructor default (0.05 elasticity, 0.5 friction).

Server-set bouncier surfaces or stickier objects therefore felt
identical to inert walls on collision. Inelastic projectiles via
PhysicsState bit 0x20000 (already plumbed in Commit A) had no per-
object elasticity to override.

Now the parser captures the floats, surfaces them on Parsed +
EntitySpawn, leaving the values at default (null) when their
PhysicsDescriptionFlag bits aren't set. Subscribers (e.g., the
remote-entity dead-reckoning path, future spell-projectile rendering)
can apply them when they wire elasticity to PhysicsBody.Elasticity.

The local player's PhysicsBody is constructed at controller init,
not from a CreateObject — so this commit alone produces no
user-visible local-player change. Effect lands when remote/projectile
physics consume EntitySpawn.Elasticity.

Files:
- CreateObject.cs:284-294: declare friction + elasticity accumulators.
- CreateObject.cs:467-487: parse floats instead of skipping.
- CreateObject.cs:543-555: propagate to Parsed via both return paths.
- WorldSession.cs:67-71: extend EntitySpawn record.
- WorldSession.cs:665-668: pipe through to subscribers.

Tests: 1491 still pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-30 09:43:27 +02:00
Erik
ffefc6977f feat(physics): live-entity collision plumbing (Commit A)
Plumbing-only foundation for the upcoming live-entity (NPC / monster
/ player) collision port. No behavior change — the new fields default
to zero/None so the 5 existing static-entity Register call sites in
GameWindow.cs are untouched.

Wire layer:
- CreateObject parser now surfaces PhysicsState (acclient.h:2815 —
  ETHEREAL_PS=0x4, IGNORE_COLLISIONS_PS=0x10, HAS_PHYSICS_BSP_PS=0x10000,
  ...) which the parser previously dropped at line ~337 with a bare
  `pos += 4`.
- CreateObject parser now surfaces ObjectDescriptionFlags (the retail
  PWD._bitfield trailer per acclient.h:6431-6463), where
  acclient_2013_pseudo_c.txt:406898-406918 ACCWeenieObject::IsPK /
  IsPKLite / IsImpenetrable read bits 5 / 25 / 21 directly. Previously
  read-and-discarded.
- WorldSession.EntitySpawn carries both new fields through to subscribers.

Physics layer:
- New `EntityCollisionFlags` enum (IsPlayer / IsCreature / IsPK /
  IsPKLite / IsImpenetrable) + `FromPwdBitfield` helper. Bit
  positions verified against retail's SetPlayerKillerStatus (
  acclient_2013_pseudo_c.txt:441868-441890) which maps
  PKStatusEnum→bitfield exactly: PK=0x4→bit5, PKLite=0x40→bit25,
  Free=0x20→bit21.
- `ShadowEntry` extended with `State` (raw PhysicsState bits) +
  `Flags` (decoded EntityCollisionFlags). Backward-compatible — all
  five existing landblock-entity Register call sites omit them.
- `ShadowObjectRegistry.UpdatePosition(entityId, pos, rot, ...)` —
  fast-path for the 5–10 Hz UpdatePosition (0xF748) stream the server
  emits per visible entity. Reuses the entry's existing shape +
  state + flags. Mirrors retail's CPhysicsObj::SetPosition
  (acclient_2013_pseudo_c.txt:284276) which keeps the same shape and
  re-registers cell membership.
- `ObjectInfoState` adds `IsPK = 0x800` and `IsPKLite = 0x1000`
  matching retail's OBJECTINFO::state bits (acclient.h:6190-6194).
  Used by Commit C's PvP exemption gate.

Tests:
- `EntityCollisionFlagsTests` — 7 tests covering empty / each bit
  alone / PK+player combo / unrelated-bit ignore.
- `ShadowObjectRegistryTests` — 5 new tests: UpdatePosition moves
  entry to new cell, preserves State/Flags, unregistered no-op,
  Register stores State/Flags, defaults are zero/None.
- `CreateObjectTests` — 3 new tests verifying PhysicsState + PWD
  bitfield (with PK / PKLite bit cases) parse and surface.

1454 → 1454 + 15 = covered by suite. dotnet build + dotnet test
green.

Foundation for Commit B (live-entity registration) and Commit C
(PvP exemption block in FindObjCollisions).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-29 13:12:56 +02:00
Erik
186a584404 feat(anim): Phase L.1c port MoveTo path data + per-tick steer
Root-causing the user-reported "monsters disappearing some time +
laggy/jittery locomotion" via systematic-debugging Phase 1: our
UpdateMotion parser kept only speed/runRate/flags from a movementType
6/7 packet and discarded Origin (destination), targetGuid, and the
distance/walkRunThreshold/desiredHeading half of MovementParameters.
The integrator consequently held Body.Velocity at zero during MoveTo
("incomplete state" stabilizer 882a07c), so the body froze with legs
animating until UpdatePosition snap-teleported it — sometimes outside
the visible window (disappearing) — and constant-velocity drift along
the old heading between snaps produced jitter on every UP correction.

The 882a07c stabilizer was deliberately conservative because the state
WAS incomplete. Completing the data plumbing makes its restriction
moot: with the full MoveTo payload captured, the body solver has every
field retail's MoveToManager::HandleMoveToPosition (0x00529d80) reads.

Why: server re-emits MoveTo packets ~1 Hz with refreshed Origin while
chasing — verified in the live log (guid 0x800003B5 seq 0x01FE→0x0204
all show different cell/xyz floats). Those are heading updates we'd
been throwing away. With the full payload retained, the per-tick driver
steers body orientation toward Origin (±20° snap tolerance, π/2 rad/s
turn rate above tolerance) and lets apply_current_movement fill in
Velocity from the existing RunForward cycle — no new motion path,
just the right heading.

Scope is the minimum viable subset: target re-tracking, sticky/StickTo,
fail-distance progress detector, and sphere-cylinder distance are
server-side concerns we don't need (server's emit cadence handles all
of them). MoveToObject_Internal target-guid resolution is also skipped
— Origin is refreshed each packet, so the effective target tracks the
real entity even without a guid lookup.

Cross-references:
- docs/research/named-retail/acclient_2013_pseudo_c.txt — MoveToManager
  + MovementParameters::UnPackNet (0x0052ac50) + apply_run_to_command
  (0x00527be0). 18,366 named PDB symbols make this the primary oracle.
- references/ACE/Source/ACE.Server/Physics/Animation/MoveToManager.cs
  — port aid; flagged divergences (WalkRunThreshold default, set_heading
  snap, inRange one-shot) called out in the new pseudocode doc.
- docs/research/2026-04-28-remote-moveto-pseudocode.md — pseudocode +
  ACE divergence flags + out-of-scope list per CLAUDE.md mandatory
  workflow (decompile → cross-reference → pseudocode → port).

Tests: 1404 → 1412 (parser type-7 path retention + type-6 target guid
retention; driver arrival, in-tolerance snap, beyond-tolerance step,
behind-target shortest-path turn, arrival preserves orientation,
Origin→world landblock-grid arithmetic).

Pending visual sign-off — handoff stabilizer 882a07c was the last
commit the user tested.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-28 21:49:22 +02:00
Erik
9812965183 fix(anim): Phase L.1c match MoveTo run speed
Retail MovementManager::PerformMovement (0x00524440) reads MoveTo speed and runRate from the packet, MovementParameters::UnPackNet (0x0052AC50) defines the layout, and CMotionInterp::apply_run_to_command (0x00527BE0) multiplies RunForward by runRate. Parse those fields for UpdateMotion/CreateObject, seed server-controlled MoveTo locomotion with the retail speed multiplier, and avoid overriding active monster MoveTo with sparse UpdatePosition-derived velocity.
2026-04-28 20:58:22 +02:00
Erik
7656fe0970 fix(anim): Phase L.1c animate server-controlled chase 2026-04-28 19:38:52 +02:00
Erik
b96b680a20 fix(anim): Phase L.1c route creature actions and despawns
Handle retail ObjectDelete (0xF747) using CM_Physics::DispatchSB_DeleteObject 0x006AC6A0 / SmartBox::HandleDeleteObject 0x00451EA0 and ACE GameMessageDeleteObject so dead creatures are removed when corpses spawn.

Route action-class ForwardCommand values through AnimationCommandRouter/PlayAction instead of SetCycle so creature attack commands 0x51/0x52/0x53 survive the immediate Ready echo, matching CMotionTable::GetObjectSequence 0x00522860 / ACE MotionTable.GetObjectSequence.

Use server-authoritative UpdatePosition velocity, or observed server position delta for non-player entities when HasVelocity is absent, to reduce monster/NPC chase lag without applying player RUM prediction to server-controlled creatures.
2026-04-28 19:21:02 +02:00
Erik
4874d8595a feat(combat): Phase L.1c wire live attack input 2026-04-28 11:58:57 +02:00
Erik
25b9616703 feat(combat): Phase L.1c add outbound combat actions 2026-04-28 10:57:12 +02:00
Erik
29afc94b94 fix(net): Phase L.1c conform combat wire events 2026-04-28 10:54:50 +02:00
Erik
0ebf0cad09 fix(net): VectorUpdate parser was reading guid from opcode bytes — remote jumps invisible
User report: "in ACdream client, when other client is jumping,
nothing happens at all".

Diagnostic [VU.recv] revealed the parser was reading
guid = 0x0000F74E (= the opcode itself) and velocity values in
the billions:

  [VU.recv] guid=0x0000F74E vel=(8589944832.00,0.00,0.00)
            isLocal=False hasRemote=False

WorldSession.ProcessDatagram passes the FULL reassembled body
including the 4-byte opcode at offset 0 — every other parser
in src/AcDream.Core.Net/Messages/ verifies the opcode word
before reading payload (UpdateMotion.TryParse:77,
UpdatePosition.TryParse, etc.). VectorUpdate.TryParse skipped
that step and read every field shifted four bytes early,
making the guid the opcode bytes and the velocities random
floats from later in the buffer. With guid=0xF74E never
matching any tracked entity, OnLiveVectorUpdated returned
early and remote jumps rendered nothing.

Fix: read + verify opcode at offset 0 in TryParse, then read
guid at offset 4, velocity at 8/12/16, omega at 20/24/28,
sequences at 32/34. Body length now 4 (opcode) + 32 (payload).

Tests stay 1222 green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-26 21:51:36 +02:00
Erik
a060f4fc98 fix(player): apply AttributeFormula to wire-derived Run/Jump skill — root cause of short jumps
Found the underlying cause of the user's persistent
"jumps don't reach retail height" complaint. The wire's SkillEntry
`init` field is ONLY the InitLevel (training/specialized
chargen bonus, per ACE GameEventPlayerDescription.cs:317
"init_level, for training/specialized bonus from character
creation"). It does NOT include the AttributeFormula
contribution.

ACE's CreatureSkill.Current is computed as:
  AttributeFormula(skill, attrs) + InitLevel + Ranks
  + augs + multipliers - vitae

Pre-fix13 we used `init + ranks` only — dropping the
AttributeFormula term, which is the DOMINANT component for
movement skills (50-100 points typical). For our character
that meant Jump skill 208 instead of the actual ~280-310,
giving a 3.11 m peak instead of the retail ~4 m peak. Hence
"feels like the upward acceleration is too slow and we don't
reach the same height".

Fix:
- GameWindow caches portal.dat's SkillTable (0x0E000004u) at
  WireAll time. Each entry has a SkillFormula with attr1/
  attr2/multipliers/divisor/additive constants
  (formula:  bonus = (attr1*M1 + attr2*M2)/Div + Additive).
- GameEventWiring.WireAll gains a
  `resolveSkillFormulaBonus(skillId, attrCurrents)` callback.
  GameWindow plugs in a resolver that looks up
  SkillTable.Skills[skillId].Formula, applies the formula
  using the player's current attribute values from PD.
- The PD handler builds attrId→current map (ranks+start) from
  the parsed attributes before iterating skills, then passes
  it to the resolver for Run (24) and Jump (22).
- Total skill = formulaBonus + InitLevel + Ranks. Matches ACE
  Current minus augs/multipliers/vitae (close enough — those
  add maybe ±10 % at most).

ACDREAM_DUMP_VITALS=1 logs add a per-skill line:
  "vitals: PD-skill id=22 init=N ranks=N formulaBonus=N total=N"
so live testing can confirm the formula is applied.

Tests stay 1222 green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-26 20:56:38 +02:00
Erik
b609b5ea6e feat(net): handle 0xF74E VectorUpdate so remote players' jumps render
Remote-player jumps were silently dropped — we never parsed the
VectorUpdate broadcast that carries the jump launch velocity, so
the remote body's Z velocity stayed at 0 and the jump animation
showed without any vertical motion.

ACE Player.cs:954 enqueues GameMessageVectorUpdate (opcode 0xF74E)
on every jump in addition to the bracketing UpdateMotion. Wire
layout (GameMessageVectorUpdate.cs):

  u32 opcode (= 0xF74E)
  u32 objectGuid
  3xf32 velocity (world-space, post-rotation)
  3xf32 omega
  u16 instanceSequence
  u16 vectorSequence

This commit:

1. Adds VectorUpdate.TryParse + VectorUpdated session event.
2. WorldSession.ProcessDatagram dispatches 0xF74E.
3. GameWindow subscribes via OnLiveVectorUpdated:
   - Sets remote PhysicsBody.Velocity from the wire vector.
   - When velocity.Z > 0.5 m/s, marks the remote as Airborne,
     clears Contact + OnWalkable bits, and enables the Gravity
     state flag — so calc_acceleration returns (0, 0, -9.8) and
     UpdatePhysicsInternal produces a parabolic arc.
4. The per-tick remote update (TickAnimations remote-physics
   block) now SKIPS the "force OnWalkable + apply_current_movement"
   step when Airborne. Otherwise that path stomps the +Z velocity
   each frame — same shape as the bug the local jump hit before
   K-fix7.
5. ResolveWithTransition for remotes now passes
   isOnGround: !rm.Airborne. Mirrors K-fix7's local-player gate —
   airborne resolves must NOT pre-seed the ContactPlane,
   otherwise AdjustOffset's snap-to-plane branch zeroes the
   upward offset.
6. UpdatePosition handler clears the airborne flag and restores
   ground-contact bits, so the server's authoritative re-grounding
   ends the arc cleanly at the new ground location.

ACDREAM_DUMP_MOTION=1 logs each VectorUpdate as
"VU guid=0x... vel=(...) airborne=...".

Tests stay 1222 green. Live verification pending — watch a remote
character jump.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-26 17:38:32 +02:00
Erik
1fce21034a feat(player): use server-authoritative Run + Jump skill values from PlayerDescription
Before this commit, PlayerWeenie used hardcoded ACDREAM_RUN_SKILL
(default 200) and ACDREAM_JUMP_SKILL (default 300) regardless of
the actual character skill. PlayerDescription's skill table HAS
been parsed since Phase H, but the values weren't plumbed into
PlayerMovementController, so a high-Jump character still got the
3-4m default arc instead of their real 5m+ arc, and a low-Jump
character got too much.

GameEventWiring.WireAll gains an optional `onSkillsUpdated`
callback. The PlayerDescription handler scans the parsed skill
table for SkillId 24 (Run) and SkillId 22 (Jump) — ACE Skill enum
ordinals from references/ACE/.../Enum/Skill.cs:11-37 — and fires
the callback with `init + ranks` for each (the holtburger-named
"init" field is the attribute-derived initial component, ranks
is XP-bought additions; closest sane approximation of ACE's
CreatureSkill.Current short of porting Aug + Multiplier + Vitae
chains).

GameWindow stores the most recent values in _lastSeenRunSkill /
_lastSeenJumpSkill and pushes them into the controller at two
points:
  * Immediately if _playerController already exists (PD arriving
    mid-session, e.g. after a relog).
  * Inside EnterPlayerModeNow when constructing a fresh
    controller (the auto-entry path: PD always arrives at login
    before auto-entry fires, so this is the normal path).

Both sites also log "applied server skills run=X jump=Y" so live
testing can confirm the right values reached the formula.

Console output (ACDREAM_DUMP_VITALS=1) gains a "vitals: PD-skills
run=X jump=Y" line on every PlayerDescription with skill data.

Tests stay 1222 green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-26 17:22:59 +02:00
Erik
3f7821c18d fix(chat): BuildTell wire field order + retail-style FormatEntry + suppress duplicate Channel echo
Three follow-up fixes from the 2026-04-25 live verify session.

1. CRITICAL: BuildTell wire field order. Our outbound layout was
   [target_name, message] but ACE's GameActionTell.Handle reads
   [message, target_name] (verified against
   references/ACE/.../GameActionTell.cs:17-18 verbatim). Result: every
   /tell since Phase I.3 has been failing with WeenieError 0x052B
   (CharacterNotAvailable) because ACE was looking up the message
   text as the recipient name. Swapped the field order in
   ChatRequests.BuildTell so message is written first; updated the
   pinned BuildTell test to expect the corrected layout. The
   WorldSessionChatTests round-trip continues to pass since SendTell
   delegates to BuildTell.

2. Retail-style FormatEntry. The user asked for the canonical retail
   strings:
     /say (own):       You say, "text"
     /say (incoming):  Name says, "text"
     /tell (own echo): You tell Caith, "text"
     /tell (incoming): Caith tells you, "text"
     channel:          [Trade] +Acdream says, "text"
     /shout (own):     You shout, "text"
     /shout (incoming):Name shouts, "text"

   Discriminators: SenderGuid == 0 distinguishes our own outbound
   echoes (set by OnSelfSent) from real incoming whispers (carry the
   sender's player guid). Sender == "" or "You" distinguishes our own
   /say echoes (OnLocalSpeech substitutes "You" when the wire sender
   is empty per holtburger client/messages.rs:476-487).

   ChatEntry gains a new ChannelName slot so Channel-kind entries
   render with the friendly room name ("Trade") instead of "ch 3".
   Falls back to "ch {ChannelId}" when ChannelName isn't populated
   (legacy ChatChannel inbound or older callers).

3. Suppress optimistic Channel echo. The user saw duplicates per
   /trade /lfg in the live trace:
     [ch 0] Trade: hello                     <-- our optimistic
     [ch 3] +Acdream: [Trade] hello          <-- ACE's TurbineChat broadcast
   ACE's TurbineChatHandler at Network/Handlers/TurbineChatHandler.cs
   broadcasts EventSendToRoom to ALL recipients in the room including
   the sender, so the canonical echo always arrives via 0xF7DE. Drop
   the optimistic OnSelfSent for Turbine kinds in GameWindow's
   SendChatCmd handler; trust the server. Legacy ChatChannel paths
   (Fellowship / Allegiance / Patron / Monarch / Vassals / CoVassals)
   keep the optimistic echo because the legacy 0x0147 broadcast may
   not always come back to the sender.

   Inbound TurbineChat also stops embedding "[Trade] " into the
   message text — passes the friendly name out-of-band via the new
   channelName parameter on ChatLog.OnChannelBroadcast.

11 tests updated for the new format strings (8 in ChatVMTests, 1 in
ChatVMCombatTests, 1 BuildTell, plus the format additions cover
incoming/outgoing variants per kind). Solution total: 1007 green
(243 + 114 + 650), 0 warnings.

Tells should now actually deliver. Channel echoes show as
[Trade] +Acdream says, "hello" without the duplicate.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 20:49:02 +02:00