docs: #317 velocity-chain audit — verdict NO RETAIL BASIS (report-only)

Report-only per CLAUDE.md's investigation rule; no fix applied and none
approved. The call at LiveEntityNetworkUpdateController.cs:2459 is
untouched.

Verdict: velocity is a SEPARATE WIRE CHANNEL in retail, and acdream's
accepted-Position path crosses it.

SmartBox::DoVectorUpdate 0x004521C0 is retail's sole velocity installer
for a remote (set_velocity 0x0045221E + set_omega 0x0045222C), gated on
update_times[3] = VECTOR_TS — not Position's update_times[0]. An
exhaustive grep of its call sites returns exactly two, and neither is the
Position path: SmartBox::HandleVectorUpdate 0x00453480 (call 0x004534E6)
and SmartBox::HandleCreateObject 0x00454C80 (call 0x00454EE9).
HandleReceivedPosition's only set_velocity is 0x004541B4, which ZEROES the
local player on the teleport arm.

Retail is not merely silent here, it is deliberate: PositionPack::UnPack
0x00516740 does decode a velocity off the Position wire (field written
0x005167E9) — retail receives the value and drops it on this path.

acdream instead commits acceptedSpawn.Physics?.Velocity on every accepted
Position, and the retail-correct mechanism ALREADY EXISTS one method away
(TryCommitAuthoritativeVector, whose doc comment describes DoVectorUpdate's
exact paired shape). The Position-path call is therefore both non-retail
and redundant with a correct sibling. Sharpening the divergence: the call
passes `?? Vector3.Zero`, so a Position without HasVelocity actively zeroes
the body — something retail never does on this path.

Recommended (NOT approved): either remove the call, or keep it and file a
register row as a deliberate adaptation in AP-135's class. Three unresolved
inputs decide which, listed in the report's section 5 — chiefly what
consumes body.Velocity for a remote (AP-80's velocity-derived animation
cycle is the specific unknown), and whether ACE sets HasVelocity at all. The
retail half of the audit is settled; those three are cheap follow-ups that
do not need the binary again.

Successor note: this function family carries Binary Ninja's dropped-flag
artifact (`-((eax_4 - eax_4))` at 0x004521F5 and 0x00452186), the same one
the C5b review hit in Gate A. Do not read a comparison here from pseudo-C.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-06 06:29:13 +02:00
parent 429775d4c4
commit 1d2d4bb8bd

View file

@ -0,0 +1,186 @@
# #317 — accepted-Position velocity chain: retail audit (2026-08-06)
Report-only per `CLAUDE.md`'s investigation rule. **No code changed, no fix
applied, no fix approved.** Written at HEAD `3aab05b0` (branch
`claude/acdream-physics-divergence-5aa784`).
---
## Verdict: **NO RETAIL BASIS.** Velocity is a separate wire channel in retail, and acdream's Position path crosses it.
Retail installs a remote body's velocity in exactly one function, gated on a
timestamp channel that an accepted Position never advances. acdream commits the
Position packet's velocity on *every* accepted Position. The correct retail
mechanism **already exists in acdream** on the VectorUpdate path, so the
Position-path call is both non-retail and redundant with a correct sibling.
Severity stays **LOW** — no observable defect is known, and §5 lists what I
could *not* establish. This is not a "rip it out tonight" finding.
---
## 1. Retail's velocity chain — verified by symbol
### 1.1 The sole installer
`SmartBox::DoVectorUpdate` @0x004521C0 (pseudo-C 91208). Body:
```
004521db wrap-safe compare of incoming stamp (edi) vs arg2->update_times[3]
004521e5 if (eax_4 > 0x7fff) c = edi < esi ; wrapped
004521e7 else c = esi < edi ; unwrapped
004521f5 if (<stamp is newer>)
004521f7 arg2->update_times[3] = edi ; stamp VECTOR_TS
00452204 if (arg2 != this->player):
0045221e CPhysicsObj::set_velocity(arg2, arg3, 1)
0045222c CPhysicsObj::set_omega(arg2, arg4, 1)
00452204 else if (cmdinterp->UsePositionFromServer() != 0):
0045221e CPhysicsObj::set_velocity(arg2, arg3, 1)
0045222c CPhysicsObj::set_omega(arg2, arg4, 1)
```
`update_times[3]` is **VECTOR_TS** — a different channel from Position's
`update_times[0]` (POSITION_TS, stamped at @0x00454079/@0x00454084 in
`HandleReceivedPosition`).
> **Binary Ninja artifact, noted for successors.** The gate renders as
> `if (-((eax_4 - eax_4)) != 0)` @0x004521F5 — always false — the same dropped
> flag-test artifact the C5b review hit in `HandleReceivedPosition`'s Gate A.
> The real predicate is the `c` computed at @0x004521E5@0x004521E7. The
> neighbouring `SmartBox::HandlePlayerTeleport` @0x00452150 carries the
> identical artifact @0x00452186. **Do not read a comparison in this function
> family from the pseudo-C**; disassemble the PDB-paired binary.
### 1.2 Its only two callers — neither is the Position path
Exhaustive grep of `DoVectorUpdate(` across `acclient_2013_pseudo_c.txt`:
| call site | enclosing function |
|---|---|
| @0x004534E6 (92225) | `SmartBox::HandleVectorUpdate` @0x00453480 — the dedicated VectorUpdate wire handler |
| @0x00454EE9 (93916) | `SmartBox::HandleCreateObject` @0x00454C80 — initial PhysicsDesc |
**`SmartBox::HandleReceivedPosition` @0x00453FD0 is not among them.**
### 1.3 The Position path's only `set_velocity` zeroes the LOCAL player
@0x004541B4 (93029), inside `HandleReceivedPosition`'s teleport arm —
`set_velocity(player_2, &var_54, 1)` with a zero vector, after `TeleportPlayer`.
It is a local-player teleport reset, not a remote velocity install. This
confirms C4 route 5's finding rather than inheriting it.
### 1.4 Retail *receives* the wire velocity and discards it on this path
`PositionPack::UnPack` @0x00516740 (284585) parses origin, the flag-gated
quaternion components (bits 8 / 0x10 / 0x20), a **velocity** (written
@0x005167E9), and a `placement_id` (@0x00516828/@0x00516830).
`SmartBox::UnpackPositionEvent` @0x004542C0 unpacks into a stack `PositionPack`
(`var_68` @0x004542E2), resolves the object, and proceeds to the
`update_times[8]` compare. The parsed velocity is never routed to
`DoVectorUpdate` or `set_velocity`.
**So retail's behaviour is deliberate, not accidental**: the byte is on the
wire, retail decodes it, and the Position path does not install it. Only a
VectorUpdate (or a CreateObject's PhysicsDesc) moves a remote's velocity.
---
## 2. acdream's side
**Call site:** `src/AcDream.App/Physics/LiveEntityNetworkUpdateController.cs:2459`
— on every accepted Position that passes the position/velocity authority
checks, commits `acceptedSpawn.Physics?.Velocity ?? Vector3.Zero` through
`_liveEntities.TryCommitAuthoritativeVelocity(...)`.
**Method:** `LiveEntityRuntime.TryCommitAuthoritativeVelocity`
(`src/AcDream.App/World/LiveEntityRuntime.cs:2824`) → `TryCommitAuthoritativeVectorCore`
with `angularVelocity: null``_physics.TryCommitAuthoritativeVector(...)`.
**The sibling that IS retail-correct:** `TryCommitAuthoritativeVector`
(`:2843`), whose own doc comment reads *"Commits the paired retail VectorUpdate
writes (`set_velocity`, then `set_omega`) to one canonical body."* That is
`DoVectorUpdate`'s exact shape, on the correct channel.
**The wire does carry it.** `UpdatePosition` parses
`PositionFlags.HasVelocity = 0x01` → 3×f32
(`src/AcDream.Core.Net/Messages/UpdatePosition.cs:47,132-136`), mirroring
retail's `PositionPack`. So acdream is not inventing data — it is installing
data retail decodes and drops.
**The divergence, stated exactly:** acdream lets the POSITION channel write
`body.Velocity`; retail lets only the VECTOR channel (and CreateObject) do so.
A stale or unrelated velocity riding a Position packet is therefore installed
by acdream and ignored by retail. Note also that acdream's call passes
`?? Vector3.Zero` — a Position packet *without* `HasVelocity` actively zeroes
the body's velocity, which retail never does on this path.
---
## 3. Issue text: accurate at HEAD
`docs/ISSUES.md:340` is correct in every checked particular — the route-5
disassembly finding, the corrected in-place comment (still present and accurate
at `LiveEntityNetworkUpdateController.cs:2435-2451`), and the "call left
unaudited" status. The `~line 2420` / `~line 2444` citations have drifted to
`:2435` and `:2459`. Nothing stale in substance.
---
## 4. Recommended action — **NOT APPROVED**
Two defensible outcomes; §5 decides which.
**(a) Remove the call.** It is a channel crossing retail does not perform, and
the retail-correct mechanism already exists on the VectorUpdate path. This is
the honest fix *if* §5 shows nothing depends on it.
**(b) Keep it and file a register row** as a deliberate acdream adaptation —
the same class as **AP-135**, which files acdream-only per-packet bookkeeping
writes on the airborne no-op branch. If ACE's Position cadence is the only
practical velocity source for remotes in our stack, a row is the honest
outcome and removal would be the regression.
**Do not fold either into another slice.** This call sits on the hottest inbound
path and its removal is a behaviour change; it deserves its own gate.
---
## 5. What I could NOT establish (flagged, not guessed)
1. **What consumes `body.Velocity` for a remote, and whether removal regresses
anything.** This is the decisive input and it is unmeasured. Specifically
unresolved: whether **AP-80**'s velocity-derived animation cycle reads this
field for remotes. The call site's own comment says the Position-*delta*
velocity computed further down is "animation diagnostics, never substituted
into physics" — which implies this call is the one that does write physics,
but I did not trace the readers.
2. **Whether ACE actually sets `HasVelocity` on remote Position updates**, and
how often. If it never does, acdream's `?? Vector3.Zero` is the live
behaviour and the analysis above changes character — it becomes "acdream
zeroes remote velocity on every Position," which is a different and possibly
more consequential divergence than "acdream installs a stale one." A
WireMCP loopback capture against the local ACE answers this cheaply.
3. **Whether acdream's VectorUpdate path is wired and reached in production**
at all. If it is dormant, removing the Position-path call would leave remote
velocity with no writer, and (b) becomes correct by default.
Each is a bounded follow-up. None requires the retail binary again — the retail
half of this audit is settled.
---
## 6. Retail anchors cited
| symbol | address | pseudo-C |
|---|---|---|
| `SmartBox::DoVectorUpdate` | 0x004521C0 | 91208 |
| — its `set_velocity` / `set_omega` | 0x0045221E / 0x0045222C | 91233 / 91234 |
| — its VECTOR_TS gate + stamp | 0x004521E5 / 0x004521F7 | — |
| `SmartBox::HandleVectorUpdate` | 0x00453480 (call @0x004534E6) | 92225 |
| `SmartBox::HandleCreateObject` | 0x00454C80 (call @0x00454EE9) | 93916 |
| `SmartBox::HandleReceivedPosition` | 0x00453FD0 | 92896 |
| — its only `set_velocity` (zeroes LOCAL player) | 0x004541B4 | 93029 |
| `SmartBox::UnpackPositionEvent` | 0x004542C0 | 93055 |
| `PositionPack::UnPack` | 0x00516740 | 284585 |
| — its velocity write | 0x005167E9 | — |
| BN dropped-flag artifacts | 0x004521F5, 0x00452186 | — |