fix(physics): P5 commit 3 - retire TS-35, close #167 (ConstraintManager leash)

PhysicsBody.IsFullyConstrained now reflects real ConstraintManager state
(pushed every tick by the same per-tick pumps commit 2 wired), so
jump_is_allowed's already-ported gate (WeenieError 0x47) actually fires
while an object is rubber-banding hard against a server position
correction, closing the last piece of #167.

Housekeeping:
- Delete register row TS-35 (retired: the write side is no longer stubbed).
- Rewrite the stale doc comments on PhysicsBody.IsFullyConstrained,
  ConstraintManager (class + IsFullyConstrained), PositionManager.ConstrainTo,
  EntityPhysicsHost.PositionManager, and PlayerMovementController.PositionManager
  that described the leash as permanently unarmed/stubbed.
- Close #167 in ISSUES.md citing the research doc and commits e0629145 /
  7719d25b.
- Add an "as-ported" addendum to
  docs/research/2026-07-30-constraint-leash-constants.md naming the actual
  current seam owners (the doc's own open question flagged this as
  implementer-verify-required post-J-slices).
- Update docs/plans/2026-07-29-physics-parity-campaign.md's P5 status and
  CLAUDE.md's Campaign P summary to reflect #167's closure (items #153/#72
  remain open in P5).

Verification: complete solution suite green - 9,978 tests, 5 skips, 0
failures across all 9 test projects (Core.Tests, Runtime.Tests, App.Tests,
Headless.Tests, Core.Net.Tests, Content.Tests, UI.Abstractions.Tests,
Bake.Tests, Cli.Tests).
This commit is contained in:
Erik 2026-07-30 12:12:29 +02:00
parent 7719d25bc5
commit 9e17554ed4
10 changed files with 159 additions and 48 deletions

View file

@ -4217,7 +4217,20 @@ Test: `GpuWorldStateTests.RelocateEntity_StrandedInPending_MovesToLoadedTarget`
## #167 — ConstraintManager leash unported (arming + two unknown x87 constants)
**Status:** OPEN (deferred, filed 2026-07-03 during R5-V1)
**Status:** DONE — 2026-07-30 (Campaign P Slice P5). Both blockers were
research-solved without a cdb session: the two x87-elided constants were
recovered by disassembling the matching retail binary's raw machine code
(`docs/research/2026-07-30-constraint-leash-constants.md`), and the arming
site is now every current acdream inbound-position acceptance seam. Commit
`e0629145` (constants + `ConstraintDistance`), commit `7719d25b` (arming at
`LiveEntityNetworkUpdateController` for remotes and
`PlayerMovementController.SetPosition`/`BlipPosition` for the local player,
plus the per-tick `PhysicsBody.IsFullyConstrained` push), and this commit
(TS-35 retirement + stale-comment cleanup). Register row **TS-35** is
deleted in the same session. Full Core/Runtime/App suites pass with no
regressions; new conformance tests cover leash-armed jump refusal,
teleport-vs-blip anchor/teardown behavior, taper reduction over ticks, and
the remote-tick `IsFullyConstrained` push.
**Severity:** LOW (server-position rubber-band + jump-during-rubber-band gate)
**Component:** physics, constraint
@ -4233,25 +4246,31 @@ not `SmartBox`, so nothing calls `PositionManager.ConstrainTo`, and
`IsFullyConstrained` stays false (= register **TS-35**'s current stub
behavior — jump never blocked by the leash).
**Blockers:** (1) the two distance constants are **x87 float returns BN
elided** — `GetStart/MaxConstraintDistance` decompile to a bare
**Blockers (RESOLVED):** (1) the two distance constants were **x87 float
returns BN elided** — `GetStart/MaxConstraintDistance` decompile to a bare
`this->m_position;` expression with the actual returned value lost to the
FPU-return-elision artifact. Recovering them needs a live cdb read of `st0`
after the call (retail debugger toolchain) or a Ghidra re-decompile with a
corrected float-return signature. (2) The arming site (`SmartBox`'s inbound
position-reconciliation branches A/B/C) has no acdream equivalent yet — wiring
it means teaching acdream's position path to re-anchor the leash on every
server position update, then feeding the `adjust_offset` taper into the body
integration (same chokepoint as the sticky wiring, R5-V3).
FPU-return-elision artifact. Recovered by disassembling the matching binary's
raw machine code directly (no cdb needed): outdoor start 10 / indoor 5,
outdoor max 50 / indoor 20 — ACE's start mapping is INVERTED (outdoor 5 /
indoor 10); the binary wins. (2) The arming site (`SmartBox`'s inbound
position-reconciliation branches A/B/C) had no acdream equivalent — wired at
`LiveEntityNetworkUpdateController` (remotes, anchored to the object's own
position) and `PlayerMovementController.SetPosition`/`BlipPosition` (local
player, anchored to the received position), feeding the `AdjustOffset` taper
into the body integration at the same per-tick chokepoint as the sticky
wiring (R5-V3).
**Where:** `src/AcDream.Core/Physics/Motion/ConstraintManager.cs` (class,
unarmed); the read gate is `PhysicsBody.IsFullyConstrained` (TS-35) via
`jump_is_allowed`. Decomp: `docs/research/2026-07-03-r5-managers/`.
**Where:** `src/AcDream.Core/Physics/Motion/ConstraintManager.cs` (armed),
`src/AcDream.Core/Physics/Motion/ConstraintDistance.cs` (constants); the read
gate is `PhysicsBody.IsFullyConstrained` (former TS-35) via
`jump_is_allowed`. Decomp: `docs/research/2026-07-03-r5-managers/`,
`docs/research/2026-07-30-constraint-leash-constants.md`.
**Acceptance:** the two constants are recovered (cdb/Ghidra), acdream arms the
leash on inbound server positions, `IsFullyConstrained` fires while
rubber-banding, and a jump attempt inside the tight leash is blocked
(0x47) matching retail; TS-35 + this issue retire together.
**Acceptance:** the two constants are recovered (byte-decoded from the
binary), acdream arms the leash on inbound server positions,
`IsFullyConstrained` fires while rubber-banding, and a jump attempt inside
the tight leash is blocked (0x47) matching retail; TS-35 + this issue retired
together.
## #160 — Remote moveto: run animation pace vs actual movement speed mismatch

View file

@ -262,7 +262,6 @@ AP-94..AP-112 for the confirmed retail-UI completion gaps.
| TS-32 | `ClientObjectTable` has no pre-queue for a child `CreateObject` that arrives before its parent (out-of-order PARENTED create); such objects are ingested as root objects and their `ContainerId` links a not-yet-known container. Retail's `null_object_table` + `null_weenie_object_table` hold unresolvable objects until the parent arrives | `src/AcDream.Core/Items/ClientObjectTable.cs` (`Ingest`) | PD↔`CreateObject` ordering is handled (upsert semantics); out-of-order PARENTED creates are observed only at high packet loss or in vendor/corpse multi-object bursts on non-loopback links; deferred to D.5.5+ | A container's child object arriving before the container is ingested as a root item — it won't appear in `GetContents` until the next `RecordMembership` or a move event corrects the parent link | `CObjectMaint::null_object_table` / `null_weenie_object_table` (acclient.h / named-retail pc) |
| TS-33 | **NARROWED 2026-07-15** — full AP tracker semantics are ported: MTS stamps time only; AP stamps complete cell-local Position + contact plane + time; `ShouldSendPositionEvent` compares cell/contact inside the interval and the complete Frame including orientation afterward. Residual: acdream's single update path snapshots the AP predicate, emits a same-update MTS first when input changed, then AP. Retail proves `UseTime` performs Should→AP, but MTS originates in separate input callbacks; their relative same-tick callback/wire order is not yet traced | `src/AcDream.Runtime/Gameplay/LocalPlayerOutboundController.cs` (pre/post network slots); `src/AcDream.Runtime/Gameplay/PlayerMovementController.cs` (ported tracker) | Preserve the pre-existing acdream wire order until a focused retail packet/breakpoint trace establishes input callback versus `UseTime`; do not infer it from `UseTime` alone | In the rare update where both packets are due, ACE may observe their position timestamps/action sequences in the opposite order from retail, shifting only that correction tick; stationary target-facing is live-gated because full-frame orientation now publishes | `CommandInterpreter::UseTime` 0x006B3BF0; `SendMovementEvent` 0x006B4680; `SendPositionEvent` 0x006B4770; `ShouldSendPositionEvent` 0x006B45E0; `Frame::is_equal` 0x00424C30 |
| TS-40 | Retail's `physics_obj->cell` ("placed in the world") is proxied by the explicit `PhysicsBody.InWorld` flag — set by `SnapToCell` (local player placement) and `RemoteMotion` construction (remotes exist only for world entities); consumed by `CMotionInterp`'s detached-object link-strip guards (`if (cell == 0) RemoveLinkAnimations`, raw @305627). Replaces the UNREGISTERED `CellPosition.ObjCellId == 0` proxy, which only the local player ever seeded (#145 `SnapToCell`), so every REMOTE body read "detached" and every dispatched transition link (door swings, remote walk↔run links) was stripped the same tick it was appended — the 2026-07-03 door-snap bug | `src/AcDream.Core/Physics/PhysicsBody.cs` (`InWorld`); `src/AcDream.Core/Physics/MotionInterpreter.cs` (3 guard sites) | acdream has no per-body CObjCell pointer; a boolean placement flag carries exactly the guard's retail meaning until cell-pointer plumbing exists | A body used without either placement path (a future entity class constructing bodies directly) reads detached and loses transition links until its creation site sets the flag | `CMotionInterp::DoInterpretedMotion` 0x00528360 tail @305627; `CPhysicsObj::RemoveLinkAnimations` |
| TS-35 | `PhysicsBody.IsFullyConstrained` is a stub property (default `false`, never set by any physics code), read by `jump_is_allowed`'s verbatim `IsFullyConstrained` gate (raw 305524-305525) | `src/AcDream.Core/Physics/PhysicsBody.cs` (`IsFullyConstrained`) | R3-W3 needed the read site to port `jump_is_allowed`'s full chain. **R5-V1 CORRECTED the mechanism** (the earlier "per-cell contact-plane / doorway-jamming" guess was WRONG): the write side is the **ConstraintManager server-position rubber-band leash** — armed by `SmartBox::HandleReceivedPosition` on every inbound server position, `IsFullyConstrained` = `max*0.9 < offset`. R5-V1 ported `ConstraintManager` (`src/AcDream.Core/Physics/Motion/ConstraintManager.cs`) but does NOT arm it (no acdream `SmartBox` + two x87 distance constants BN elided) — so this read stays false. Arming = issue #167 | A body retail would consider fully constrained (still rubber-banding toward a server position inside the tight leash) never refuses the jump (0x47) — a jump succeeds mid-rubber-band where retail blocks it. Low practical risk (the leash band is tight + short-lived) | `CPhysicsObj::IsFullyConstrained` 0x0050ec60 → `ConstraintManager::IsFullyConstrained` 0x005560d0; `jump_is_allowed` 0x005282b0; arming `SmartBox::HandleReceivedPosition` 0x00453fd0 (issue #167) |
| TS-37 | RETIRED misattribution note (not a live divergence — kept here as the historical record R3-W3 closes): the S2a port had `contact_allows_move` (0x00528240) arm `StandingLongJump` as a side effect, explicitly flagged "PRE-EXISTING acdream side effect (not part of 0x00528240)". R3-W3 deletes that side effect; `ChargeJump` (0x005281c0) is now the ONLY arming site, matching retail exactly. No further action — recorded per the register's retire-in-same-commit rule | `src/AcDream.Core/Physics/MotionInterpreter.cs` (`contact_allows_move`, `ChargeJump`) | N/A — retired | N/A — retired | `CMotionInterp::charge_jump` 0x005281c0 @305448 |
| TS-38 | `MotionInterpreter.Initted` defaults to `true` in both constructors, not retail's `false` — retail's `CMotionInterp` is never observed pre-`enter_default_state` (every real construction path calls it before exposing the interpreter); acdream's constructors are used directly by ~40 pre-existing tests and both App call sites as complete, immediately-usable objects with no separate "enter default state" step | `src/AcDream.Core/Physics/MotionInterpreter.cs` (`Initted` property + both constructors) | Defaulting `true` is the C# equivalent of "the constructor already did what `enter_default_state` would have done to this flag" — `EnterDefaultState()` remains available, verbatim, for the REST of retail's reset semantics (state defaults, sentinel enqueue, `LeaveGround` tail) when a caller wants them | None observed: no code path needs `apply_current_movement`/`ReportExhaustion` to no-op before an explicit `EnterDefaultState()` call, since nothing constructs a `MotionInterpreter` and defers initialization today. If a future caller DOES need staged construction (build now, `EnterDefaultState()` later), it must explicitly set `Initted = false` first | `CMotionInterp::enter_default_state` 0x00528c80 @306124 sets `initted = 1`; retire if/when construction is staged through `EnterDefaultState()` uniformly |
| ~~TS-41~~ | **RETIRED 2026-07-07 (remote-creature de-overlap #184)** — the SERVERVEL synth-velocity body-drive (`Body.Velocity = ServerVelocity` / `get_state_velocity()` leg) is DELETED. Grounded NPC remotes now translate by the retail interp CATCH-UP (`RemoteMotionCombiner.ComputeOffset``InterpolationManager::adjust_offset` toward the MoveOrTeleport-queued server waypoint) and `MovementManager::UseTime` (`TickRemoteMoveTo`) runs UNCONDITIONALLY per tick — the retail `UpdateObjectInternal` shape (no wire-velocity leg-driver). The de-overlap sweep resolves the catch-up movement; the resolved position is written back into the SHADOW (AP-86) so it persists. Residual: the non-retail anim-cycle stale-stop heuristic (`ApplyServerControlledVelocityCycle(Zero)` on a >0.6 s velocity-staleness timer) is kept as ANIM-only and stays covered by **AP-80**; it no longer drives the body. | `src/AcDream.App/Physics/RemotePhysicsUpdater.cs` (grounded NPC branch) | — | — | `CPhysicsObj::UpdateObjectInternal` 0x005156b0 (`MovementManager::UseTime` @0x00515998, unconditional); `MoveOrTeleport` 0x00516330; `InterpolationManager::adjust_offset` 0x00555d30 |

View file

@ -220,6 +220,19 @@ RestrictionTables residuals. Suite 9,956/0/5.
### P5 — Deferred fidelity — closes #167, #153, #72
**Status (2026-07-30): item 1 (#167) COMPLETE.** Both blockers resolved
without Ghidra/cdb — the two x87-elided constants were byte-decoded
straight from the matching binary's raw machine code
(`docs/research/2026-07-30-constraint-leash-constants.md`). The leash is
now armed at every current acdream inbound-position acceptance seam
(`ConstraintDistance`, `LiveEntityNetworkUpdateController`,
`PlayerMovementController.SetPosition`/`BlipPosition`), the per-tick
`PhysicsBody.IsFullyConstrained` push replaces the always-false stub, and
register row TS-35 is deleted. Full Core/Runtime/App suites pass with new
conformance tests (leash-armed jump refusal, teleport-vs-blip
anchor/teardown, taper reduction, remote-tick push). Items 2 (#153) and 3
(#72) remain open.
1. **#167:** decode the two unknown x87 ConstraintManager constants
(Ghidra) and port leash arming.
2. **#153:** the far-teleport arrival onto an unstreamed landblock near

View file

@ -94,3 +94,52 @@ None for the constants/flow. Remaining implementation risk is only
where acdream's position-acceptance seams sit today (J6.3 moved
teleport correlation into Runtime — the implementer must find the
current owner rather than trusting older file cites).
## As-ported (Campaign P Slice P5, 2026-07-30)
The implementation risk flagged above resolved to these CURRENT seam owners
(post-J-slices) — recorded here so the next reader doesn't have to re-derive
them:
- **Constants**`src/AcDream.Core/Physics/Motion/ConstraintDistance.cs`.
Keyed purely on the object's own full cell id's low 16 bits (`>= 0x0100` =
indoor); the vestigial player/remote branch from §1 is deliberately not
represented as an API parameter.
- **Remote arm**`src/AcDream.App/Physics/LiveEntityNetworkUpdateController.cs`,
the inbound `UpdatePosition` handler's remote branch (`update.Guid !=
_playerServerGuid`), immediately after the `remotePlacementRequired`
hard-teleport block returns (that block already covers retail's
`MoveOrTeleport` Branch A / hard-place case; everything reached past it is
"did not hard-place"). One call site covers BOTH player-remote and NPC
remotes — retail's `SmartBox::HandleReceivedPosition` doesn't distinguish
them either, only `GetStart/MaxConstraintDistance`'s now-omitted vestigial
branch did. Anchored to the live `IPhysicsObjHost.Position` (which reads
`RemoteMotion.Body.Position` + the tracked cell id), matching retail's
"anchored to the object's own current position" — since the anchor and
`_host.Position` read are the same value at call time,
`ConstraintManager.ConstrainTo`'s initial offset is always 0 regardless of
whether the routing above just far-snapped or left a near-correction
queued.
- **Remote per-tick taper + `IsFullyConstrained` push** — already wired
pre-P5 for the taper (`RuntimeRemotePhysicsUpdater.Tick`/`TickHidden` call
`PositionManager.AdjustOffset` every tick via the pre-existing R5-V3
sticky/constraint chain); P5 added the `PhysicsBody.IsFullyConstrained =
host.PositionManager.IsFullyConstrained()` push at the same two call
sites, since `MotionInterpreter` only holds a `PhysicsBody` (no host
reference) and needs a live value to read.
- **Local player arm**`src/AcDream.Runtime/Gameplay/PlayerMovementController.cs`:
`SetPositionCore` (teleport: `UnConstrain` then re-`ConstrainTo` after the
existing `StopCompletelyAtPhysicsObjectBoundary` velocity zero — composed,
not duplicated) and `CommitPreparedPosition` (mirrors the same pair for
the deferred player-mode-entry commit path); `BlipPosition` (ForcePosition:
`ConstrainTo` only, no teardown — matches `SmartBox::BlipPlayer` surviving
motion/velocity/stick). Anchored to `_body.CellPosition` (the just-applied
received position).
- **Local player per-tick taper + push**`PlayerMovementController.Update`
already called `PositionManager.AdjustOffset` every physics tick pre-P5;
P5 added the `_body.IsFullyConstrained = PositionManager?.IsFullyConstrained()
?? false` push immediately after, at the same chokepoint.
- **TS-35 retirement**`PhysicsBody.IsFullyConstrained` stayed a plain
settable bool (not a computed property) so the ~40 pre-existing direct-set
unit tests keep working; the per-tick pumps above are its single writers
now, matching the project's per-entity single-owner-write pattern.