diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 749895e4..e9e732b4 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -280,7 +280,26 @@ rests TANGENT to the slope. Two independent decompilers now agree. Consequences, under the bare trigger, a planted start lifts once — which of the two the harness settle leaves is plausibly order-dependent test state. -**DECIDED 2026-08-07 by the user: "port the retail pair."** Tangent resting +**RE-DECIDED 2026-08-07 night (S4b's D0 STOP fired and REFUTED the tangent- +placement premise):** retail's `OBJECTINFO::validate_walkable` @0x0050d010 is +PLANTED (vertical foot point) for every normal mover — Ghidra + BN + ACE all +agree, and acdream's ValidateWalkable is ALREADY a byte-faithful port; only +the IsViewer/camera branch is tangent. The coherent retail mechanism is +therefore PLANT-THEN-LIFT: validate_walkable plants, adjust_offset's +bare-radius push fires once per settle and raises the body to tangent +equilibrium (dist=r), where BOTH checks go quiet — the slope float comes +from the PUSH, not the placement, exactly as the original AD-66 code +comment's "retail itself has the spurious lift" argued. The 84% live fire +rate measured the trigger against our push-disabled planted steady state; +post-fix, bodies settle tangent on first contact and the trigger goes +silent. The user's "port the retail pair" therefore maps to RELANDING +AD-66's bare radius ALONE (validate_walkable and adjust_sphere_to_plane +need nothing), and the #341 flip now has a mechanical story: whether the +harness's settle had already performed the one-time lift is order-dependent +state, which is what the assert-shape correlation was reflecting. +Byte-pin doc: `docs/research/2026-08-07-s4b-validate-walkable-bytepin.md`. + +**Original decision record:** **DECIDED 2026-08-07 by the user: "port the retail pair."** Tangent resting placement + bare-radius trigger land TOGETHER as one slice (S4b), gated by the user's eyes on a slope. The slice must first establish WHERE our planted rest comes from (the live capture's r*N.z rest was measured on outdoor diff --git a/docs/research/2026-08-07-s4b-validate-walkable-bytepin.md b/docs/research/2026-08-07-s4b-validate-walkable-bytepin.md new file mode 100644 index 00000000..013d7017 --- /dev/null +++ b/docs/research/2026-08-07-s4b-validate-walkable-bytepin.md @@ -0,0 +1,262 @@ +# S4b D0 — byte-pin of `OBJECTINFO::validate_walkable`'s distance basis + +**Date:** 2026-08-07. **Author:** S4b implementer (single Sonnet agent, no +subagents). **Status: PREMISE REFUTED — SLICE STOPPED AT D0.** No code was +changed. D1/D2/D3/D4 were not performed. Nothing is committed. + +## The question D0 was scoped to answer + +Per `docs/research/2026-08-07-s4b-tangent-rest-contract.md`: does retail's +`OBJECTINFO::validate_walkable` measure the **perpendicular sphere clearance** +(`dot(center,N)+D − radius`, tangent semantics — the same shape as +`CPolygon::adjust_sphere_to_plane`), or a **vertical foot point** +(`lowPoint = center − (0,0,radius)`, planted semantics — the shape acdream's +current `Transition.ValidateWalkable` already implements)? + +The contract's explicit, pre-authorized stop condition: *"If retail's +validate_walkable turns out to ALSO use a vertical foot point, STOP — the +whole slice premise changes and the session lead re-decides."* + +**Answer: vertical foot point (planted), for the branch that governs every +normal mover (players, creatures — anything not the camera/viewer).** The +stop condition is triggered by direct byte evidence from two independent +decompilers plus ACE's independent C# port. All three agree with each other +and with acdream's current code. + +## Byte evidence + +### 1. Ghidra decompile (primary; PDB-paired, `patchmem.gpr`, port 8081) + +`OBJECTINFO::validate_walkable @ 0x0050d010`: + +```c +TransitionState __thiscall +OBJECTINFO::validate_walkable(OBJECTINFO *this, CSphere *param_1, Plane *param_2, + int param_3, float param_4, SPHEREPATH *param_5, + COLLISIONINFO *param_6, ulong param_7) +{ + ... + if ((this->state & 4) != 0) { // IsViewer branch + fVar2 = (N.x*center.x + N.y*center.y + N.z*center.z + d) - radius; // BARE radius — tangent + ... + return OK_TS / ADJUSTED_TS; + } + // else branch — every normal mover (player, creature, missile, etc.) + fVar2 = center.x*N.x + (center.z - radius)*N.z + center.y*N.y + d + param_4; + // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + // = dot(center, N) + d - radius*N.z + waterDepth -- PLANTED, not tangent + if (-EPSILON <= fVar2) { ... return OK_TS; } + else { + if (param_5->check_walkable != 0) return COLLIDED_TS; + fVar2 = fVar2 / N.z; // zDist + ... push sphere up by -fVar2 along Z, step-down interp, etc. + return ADJUSTED_TS; + } +} +``` + +Full raw decompile captured via `GET /decompile_function?address=0x0050d010` +against the currently-open `patchmem.gpr` project (Sept 2013 EoR build, same +source as `docs/research/named-retail/`). + +**`this->state & 4` is `ObjectInfoState.IsViewer` (`= 0x4`)** — confirmed by +`references/ACE/Source/ACE.Server/Physics/ObjectInfo.cs:13` and by +`LandCell.cs:56`'s `!objInfo.State.HasFlag(ObjectInfoState.IsViewer) && +!objInfo.Object.State.HasFlag(PhysicsState.Missile)` guard on env-collision +processing. IsViewer marks the camera/eye collision object, not the player's +own movement sphere. Every walking mover (player body, creature, NPC) takes +the **else** branch — the planted one. + +**`param_4` is waterDepth** — confirmed by the caller (below): it is used +exactly once, added directly after the `d` term: +`... + param_2->d + param_4`. It is NOT folded into the `lowPoint` subtraction +— it modifies the threshold, not the sphere's virtual bottom. This is +identical in shape and placement to ACE's +`... contactPlane.D + waterDepth` and to acdream's current +`contactPlane.D + waterDepth` (`TransitionTypes.cs:3673`). AP-10's sink-in +byte-pin: **`dist = dot(center,N) + D − radius·N.z + waterDepth`**, +unchanged from acdream's current formula. + +### 2. Binary Ninja pseudo-C (secondary; corroborates, was initially ambiguous) + +`acclient_2013_pseudo_c.txt:274479-274617`. The `state & 4` branch (lines +274487-274488) computes `((N.x*cx + N.y*cy + cz*N.z) + d) - radius` — bare +radius, matching Ghidra's IsViewer arm exactly. + +The `else` branch (lines 274531-274532) computes +`N.y*cy + (cz - radius)*N.z + N.x*cx + d` — matching Ghidra's planted arm +exactly, **modulo one artifact**: BN reuses the SSA name `x87_r7_16` for both +the bare `radius` value (line 274531) and the freshly-computed dot-product +expression one line later (274532), because both values transiently occupy +the same logical x87 stack slot. This is the "flag-idiom artifact class" the +project CLAUDE.md warns about — a naming collision, not an algebraic one; the +raw expression at 274532 is unambiguous regardless of what BN chose to call +its result. **`arg5` (waterDepth) never appears by name inside BN's rendering +of this function** — another BN-only artifact (the float stack parameter gets +folded directly into the compound expression without a preserved `argN` +token). Ghidra's `param_4` resolves this cleanly. This is exactly the +"disassemble if BN is ambiguous" case D0 anticipated, and Ghidra is the +byte-pin of record here. + +### 3. ACE's independently-authored C# port (tertiary cross-check) + +`references/ACE/Source/ACE.Server/Physics/ObjectInfo.cs:101-172`, +`ValidateWalkable`: + +```csharp +if (State.HasFlag(ObjectInfoState.IsViewer)) +{ + var dist = Vector3.Dot(checkPos.Center, contactPlane.Normal) + contactPlane.D - checkPos.Radius; + ... // bare radius — tangent +} +else +{ + var dist = Vector3.Dot(checkPos.Center - new Vector3(0, 0, checkPos.Radius), contactPlane.Normal) + + contactPlane.D + waterDepth; + ... // vertical foot point — planted, waterDepth added after +} +``` + +Independently produced (ACE was reverse-engineered years before this +project's PDB-named decomp existed), and it agrees with Ghidra byte-for-byte +on both branches, including waterDepth's placement. Three independent +sources, zero disagreement. + +### 4. Caller context — confirms AD-69 is a separate, correctly-scoped finding + +`CLandCell::find_env_collisions @ 0x00532f20` (pseudo-C +lines 317070-317113, Ghidra-cross-readable) is the sole caller of +`validate_walkable` for outdoor terrain. It calls +`LandDefs::get_block_offset` **twice**: once to convert +`global_low_point` into the terrain-poly lookup's block frame (for +`find_terrain_poly`), and again (0x00533007) to subtract the block offset +from `global_sphere->center` **before** the sphere passed into +`validate_walkable` is built. This is the cell-relative correction AD-69 +already names, scoped to `CTransition::adjust_offset @ 0x0050a370` +(a **different** function, `pc:272271-272393` per the register row) — not to +`validate_walkable`. AD-69 is unaffected by this finding; it was already +correctly scoped before this slice started. + +### 5. `CPolygon::adjust_sphere_to_plane @ 0x00538210` — re-confirmed independently, unaffected + +Ghidra decompile (`GET /decompile_function?address=0x00538210`): + +```c +fVar3 = dot(N, center) + d; +fVar4 = dot(N, movement_dir); +if (fVar4 <= EPS) { if (-EPS <= fVar4) return 0; fVar3 -= radius; } // bare radius +else { fVar3 = -radius - fVar3; } // bare radius +fVar3 /= fVar4; +... +``` + +Bare radius, both branches — matches AD-66's byte evidence exactly and +matches acdream's existing `BSPQuery.cs:364 AdjustSphereToPlane` (already a +faithful port, confirmed by direct read: `dist = dpPos - validPos.Radius` / +`dist = -validPos.Radius - dpPos`, bare `Radius`, no `N.z` factor). This +function's tangent semantics are correct and were never in question. + +## What this means for the contract's premise + +The contract's framing was: *"acdream today mixes tangent (BSP walk solve) +with planted (ValidateWalkable) and the planted one wins the resting height +on terrain [instead of retail's own consistent tangent pairing]."* That +framing assumed retail pairs tangent-with-tangent (`adjust_sphere_to_plane` ++ `validate_walkable` both bare-radius) and that acdream's planted +`ValidateWalkable` is therefore a deviation to fix. + +**The byte evidence shows retail itself pairs tangent-with-planted**, not +tangent-with-tangent: + +| Function | Retail semantics | acdream today | +|---|---|---| +| `CPolygon::adjust_sphere_to_plane` @0x00538210 (BSP walk / push-out during collision resolution) | tangent, bare radius | tangent, bare radius (`BSPQuery.cs:364`) — matches | +| `OBJECTINFO::validate_walkable` @0x0050d010, mover branch (per-tick terrain rest/step validation) | **planted**, radius·N.z, waterDepth added after | **planted**, radius·N.z, waterDepth added after (`TransitionTypes.cs:3652-3730`) — matches | +| `CTransition::adjust_offset` @0x0050a370 safety push-out (AD-66) | tangent, bare radius (byte-confirmed, withheld pending #341) | planted substitution (radius·N.z), the AD-66 deviation | ← genuinely still a deviation, unaffected by this finding | + +acdream's `ValidateWalkable` is **already a byte-faithful port** of retail's +mover-path formula — same low-point construction, same epsilon bracket +(`dist >= -EPSILON` / `dist <= EPSILON`), same `zDist = dist/N.z` push-up, +same step-down interpolation guard, same `OnWalkable`/`Contact` flag-setting +conditions (state bits 1/2 map onto `oi.Contact`/`oi.OnWalkable` exactly). +There is no `ValidateWalkable` distance-basis change to port — porting one +would make acdream **diverge from** retail, not converge to it. + +**D1 item 1 (ValidateWalkable's distance basis → tangent) is refuted and must +not be implemented.** Because the contract frames the pair as inseparable +("The pair ports together or not at all" / "STOP — the whole slice premise +changes"), **the whole D1/D2/D3 program as specified is out**, not just item +1 in isolation. Items 2 (AD-66 bare-radius landing in `AdjustOffset`) and 3 +(AD-69's `get_block_offset` correction) remain independently well-founded by +their own byte evidence and their own register rows — but landing them was +explicitly conditioned by this contract on the ValidateWalkable pairing +argument that just failed, and the #341 anomaly this slice was named for +(the flipping absorbed-tick behavior on AD-66 alone) has **not been +explained** by anything found here. Landing AD-66/AD-69 now, under a +different justification than the one this contract authorized, is a +decision for the session lead, not a call for the implementer to make +unilaterally under a refuted contract. + +## What was NOT done (by design, per the contract's own stop clause) + +- D1 (port the pair) — not implemented. No changes to `TransitionTypes.cs` + `ValidateWalkable` or `AdjustOffset`, no changes to `BSPQuery.cs` / + `FlatBspQuery.cs`. +- D2 (ten-run `RuntimeRemoteUphillProgressTests` retest) — not run under + this slice's changes, because there are no changes to test. (The existing + `S4AdjustOffsetConformanceTests.Uphill_NoContactFlapAcrossTicks` and the + two `[Skip]`-ed AD-66 tests were read but left untouched and un-unskipped.) +- D3 (conformance tests + sabotage) — not written. +- D4 (register/ISSUES bookkeeping) — not performed; AD-66 and AD-69 remain + exactly as they are (AD-66 ACTIVE/WITHHELD, AD-69 FILED, #341 open). +- No files under `src/` were edited. No commits were made. + +## Open question for the session lead + +The #341 measurement anomaly (AD-66's bare radius alone producing +contradictory absorbed-tick outcomes that flip with test-assert shape) is +still unexplained. This slice's finding rules out "ValidateWalkable should +also be tangent" as the fix, since that would itself be a retail deviation. +Candidate directions the session lead may want to weigh (not investigated +here — out of this slice's scope once D0 refuted the premise): + +- The anomaly may be a test-harness artifact (the tests' own docstring + already flags "flipping with nothing but the shape of the test's post-tick + asserts" as suspicious) rather than a real engine behavior difference. +- AD-69's `get_block_offset` gap could be entangled with AD-66's in a way + that only shows up when both change together — but that is a testable + hypothesis, not yet tested. +- The #331 absorb-tick semantics (explicitly OUT of scope for both this + contract and S4b) may be the actual source of the flip, independent of + AD-66/AD-69 entirely. + +## Citations + +- `OBJECTINFO::validate_walkable` @ 0x0050d010 (Ghidra `patchmem.gpr` + decompile; BN pseudo-C `acclient_2013_pseudo_c.txt:274479-274617`) +- `ObjectInfoState.IsViewer = 0x4` + (`references/ACE/Source/ACE.Server/Physics/ObjectInfo.cs:13`) +- `references/ACE/Source/ACE.Server/Physics/ObjectInfo.cs:101-172` + (`ValidateWalkable`, independent port) +- `references/ACE/Source/ACE.Server/Physics/Common/LandCell.cs:56` + (IsViewer/Missile env-collision guard) +- `CLandCell::find_env_collisions` @ 0x00532f20 + (`acclient_2013_pseudo_c.txt:317070-317113`; the `get_block_offset` calls + and `validate_walkable` call site) +- `CPolygon::adjust_sphere_to_plane` @ 0x00538210 (Ghidra decompile; AD-66's + cited address, re-confirmed) +- `src/AcDream.Core/Physics/TransitionTypes.cs:3652-3730` (acdream's current + `ValidateWalkable` — read directly, confirmed byte-faithful to the mover + branch above) +- `src/AcDream.Core/Physics/TransitionTypes.cs:5501-5655` (acdream's current + `AdjustOffset`, including the AD-66 WITHHELD comment block and AD-69's + target `dist` computation) +- `src/AcDream.Core/Physics/BSPQuery.cs:364-428` (acdream's current + `AdjustSphereToPlane` — read directly, confirmed byte-faithful bare-radius) +- `docs/architecture/retail-divergence-register.md` rows AD-66 (line 65 + header context), AD-69 (line 166) +- `tests/AcDream.Core.Tests/Physics/S4AdjustOffsetConformanceTests.cs` + (the two `[Skip]`-ed AD-66 tests, read but left untouched) +- `docs/research/2026-08-07-s4b-tangent-rest-contract.md` (this slice's + pinned contract; the stop clause invoked above is quoted from its D0 + section verbatim)