c6bc2b9980
Code-review feedback on commit 6b4be7f:
- Section 1: strip stale [309NNN] inline annotations (off by 2-8
lines from actual file content; the 0052c1xx address comments
are the reliable anchor); address comments already present in the
decomp output are now used as inline anchors instead
- Section 2: validate_transition function header is at file line
272547 (was: 272538, inside the preceding check_collisions
function). Address 0050aa70 + LKCP-block range 272565-272583
were already correct. References section updated to match.
- Section 5: add note that SetContactPlane re-latches LKCP fields
(no-op when LKCP is the source, but non-obvious side effect)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
264 lines
12 KiB
Markdown
264 lines
12 KiB
Markdown
# A6.P3 Slice 1 — Retail Mechanism B Oracle for Indoor CP Retention
|
||
|
||
**Date:** 2026-05-21
|
||
**Author:** Claude (research agent)
|
||
**Task:** Pre-fix research grounding the indoor ContactPlane-retention refactor in
|
||
retail's exact LKCP-restore pattern before the synthesis path is removed.
|
||
|
||
---
|
||
|
||
## 1. `CEnvCell::find_env_collisions` Shape
|
||
|
||
Retail decomp at `acclient_2013_pseudo_c.txt` lines 309573–309593 (address
|
||
`0052c130`). The complete function is 10 functional lines:
|
||
|
||
```c
|
||
// 0052c130 enum TransitionState __thiscall CEnvCell::find_env_collisions(
|
||
// class CEnvCell const* this, class CTransition* arg2)
|
||
{
|
||
// Check entry restrictions (object ethereal? door closed? etc.)
|
||
enum TransitionState result = CObjCell::check_entry_restrictions(this, arg2);
|
||
|
||
if (result == OK_TS) {
|
||
// 0052c144 Clear obstruction-ethereal so BSP collision is live.
|
||
arg2->sphere_path.obstruction_ethereal = 0;
|
||
|
||
if (this->structure->physics_bsp != 0) {
|
||
// 0052c169 Project sphere into cell-local space.
|
||
SPHEREPATH::cache_localspace_sphere(&arg2->sphere_path, &this->pos, 1f);
|
||
|
||
// 0052c175 Run BSP: INITIAL_PLACEMENT → placement_insert path;
|
||
// all other insert_types → find_collisions path.
|
||
if (arg2->sphere_path.insert_type != INITIAL_PLACEMENT_INSERT)
|
||
result = BSPTREE::find_collisions(this->structure->physics_bsp, arg2, 1f);
|
||
else
|
||
result = BSPTREE::placement_insert(this->structure->physics_bsp, arg2);
|
||
|
||
// 0052c1a5 On collision with environment (non-Contact objects only).
|
||
if (result != OK_TS && (arg2->object_info.state & 1) == 0)
|
||
arg2->collision_info.collided_with_environment = 1;
|
||
}
|
||
}
|
||
return result;
|
||
}
|
||
```
|
||
|
||
**Key observation:** `find_env_collisions` itself does **not** write
|
||
`contact_plane`. It either returns OK (BSP path OK or no BSP) or returns a
|
||
collision state. ContactPlane is written ONLY inside `BSPTREE::find_collisions`
|
||
via Path 6 (land/step-down) — that is Mechanism A. There is no per-frame
|
||
synthesis path anywhere in this function.
|
||
|
||
---
|
||
|
||
## 2. Retail Mechanism B Location
|
||
|
||
**Function:** `CTransition::validate_transition`
|
||
**Retail address:** `0050aa70`
|
||
**Decomp line range:** `acclient_2013_pseudo_c.txt` lines 272547–272700
|
||
**Identified via:** The `validate_transition` function header appears at line
|
||
272547 (`0050aa70`). Line 272538 is inside the preceding
|
||
`CTransition::check_collisions` function.
|
||
|
||
The LKCP-restore block runs at lines 272565–272582 (addresses `0050aaed`–`0050ab4c`).
|
||
|
||
---
|
||
|
||
## 3. Retail Mechanism B Trigger Condition
|
||
|
||
Mechanism B fires when ALL of the following are true:
|
||
|
||
1. **`result > OK_TS && result <= SLID_TS`** — the transition ended in Collided,
|
||
Adjusted, or Slid (not OK, not Invalid).
|
||
2. **`collision_info.last_known_contact_plane_valid != 0`** — there is a
|
||
remembered floor plane from a prior frame.
|
||
3. **Proximity guard:** `|dot(global_curr_center, LKCP.N) + LKCP.d| <= radius + 0.000199f`
|
||
— the sphere's **current position center** (`global_curr_center`, NOT the
|
||
check-position sphere `global_sphere`) is still geometrically close to the
|
||
last-known plane.
|
||
|
||
When all three pass, retail:
|
||
|
||
```c
|
||
// 0050ab37
|
||
COLLISIONINFO::set_contact_plane(
|
||
&this->collision_info,
|
||
&this->collision_info.last_known_contact_plane,
|
||
this->collision_info.last_known_contact_plane_is_water);
|
||
|
||
// 0050ab42
|
||
this->collision_info.contact_plane_cell_id =
|
||
this->collision_info.last_known_contact_plane_cell_id;
|
||
```
|
||
|
||
Then `result = OK_TS` at `0050ab9f` — the collision is resolved by restoring the
|
||
floor and treating the transition as successful.
|
||
|
||
**After that block**, at `0050acff`–`0050ad7d`, retail sets
|
||
`last_known_contact_plane_valid = contact_plane_valid` (unconditional overwrite,
|
||
NOT "only when valid") and then sets `Contact` + `OnWalkable` flags based on
|
||
whether `contact_plane_valid` is non-zero. The LKCP update strategy is
|
||
**unconditional** in retail (even if current CP is invalid, LKCP gets cleared).
|
||
|
||
**The epsilon constant:** `0.000199999995f` — effectively `2e-4`. This is a
|
||
tight epsilon for floating-point error in the dot product; the sphere radius
|
||
already provides the geometric margin.
|
||
|
||
---
|
||
|
||
## 4. Our Equivalent Function
|
||
|
||
From `grep -rn "ValidateTransition" src/AcDream.Core/Physics/`:
|
||
|
||
```
|
||
TransitionTypes.cs:2751 private TransitionState ValidateTransition(TransitionState transitionState)
|
||
TransitionTypes.cs:670 transitionState = ValidateTransition(result);
|
||
```
|
||
|
||
Our C# `ValidateTransition` (TransitionTypes.cs lines 2751–2873) is the
|
||
correct equivalent. The call at line 670 is inside `FindTransitionalPosition`'s
|
||
step loop: each call to `TransitionalInsert` is immediately followed by
|
||
`ValidateTransition(result)`.
|
||
|
||
---
|
||
|
||
## 5. Decision — Where to Add Mechanism B in Our Code
|
||
|
||
### Gap analysis
|
||
|
||
Our `ValidateTransition` has TWO divergences from retail's Mechanism B:
|
||
|
||
**Gap 1: Missing `SetContactPlane` write in the Collided/Slid/Adjusted branch.**
|
||
|
||
Retail's `validate_transition` (lines 272565–272582) calls
|
||
`COLLISIONINFO::set_contact_plane(LKCP, LKCP_is_water)` and sets
|
||
`contact_plane_cell_id = LKCP_cell_id` before returning `OK_TS`.
|
||
|
||
Our `ValidateTransition` at TransitionTypes.cs:2821–2866 (the
|
||
`else if (ci.LastKnownContactPlaneValid)` block) only reads `LastKnownContactPlane`
|
||
to update `oi.State` flags (`Contact`, `OnWalkable`) — it does **not** call
|
||
`ci.SetContactPlane(...)`. This means `ContactPlane` stays invalid even when
|
||
we know the LKCP is close, while `ci.LastKnownContactPlane` holds the value.
|
||
The PhysicsEngine fallback at PhysicsEngine.cs:668–674 partially compensates
|
||
(it reads LKCP to populate `body.ContactPlane` cross-frame), but it only does
|
||
so after `FindTransitionalPosition` returns — not per-step inside the loop.
|
||
|
||
**Gap 2: Wrong sphere used for proximity dot product.**
|
||
|
||
Retail uses `global_curr_center` (pointer to the sphere center at the *current*
|
||
frame-start position) for the dot product. Our code at TransitionTypes.cs:2843
|
||
uses `sp.GlobalSphere[0].Origin` (the *check* position — where we want to move
|
||
to). For the proximity check against a retained floor plane, the correct center
|
||
is `sp.GlobalCurrCenter[0].Origin`, matching retail's `global_curr_center`.
|
||
|
||
This distinction matters when the player is near a cell/floor boundary: if the
|
||
check position has stepped slightly off the floor but the current position is
|
||
still on it, retail correctly restores the CP; our code might fail the proximity
|
||
guard spuriously.
|
||
|
||
### Insertion point (exact)
|
||
|
||
**File:** `src/AcDream.Core/Physics/TransitionTypes.cs`
|
||
**Method:** `ValidateTransition` (line 2751)
|
||
**Target block:** The `else if (ci.LastKnownContactPlaneValid)` block at lines
|
||
2821–2866 (the LKCP proximity-guard branch).
|
||
|
||
**Change required:** Within the `if (radius + PhysicsGlobals.EPSILON > MathF.Abs(angle))` branch (currently at line 2848), BEFORE setting `oi.State` flags:
|
||
|
||
1. Add `ci.SetContactPlane(ci.LastKnownContactPlane, ci.LastKnownContactPlaneCellId, ci.LastKnownContactPlaneIsWater);`
|
||
2. Change the proximity sphere center from `sp.GlobalSphere[0].Origin` (line 2843)
|
||
to `sp.GlobalCurrCenter[0].Origin` to match retail's `global_curr_center`.
|
||
|
||
The addition goes at TransitionTypes.cs approximately **line 2849** (just before
|
||
the `oi.State |= ObjectInfoState.Contact` at current line 2852), producing:
|
||
|
||
```csharp
|
||
// Retail Mechanism B (validate_transition:0050ab37): restore CP from LKCP
|
||
// when sphere is still near the plane. This writes ContactPlane valid so
|
||
// the end-of-function LastKnown-update block (below) re-latches it,
|
||
// and ObjectInfoState.Contact is set from contact_plane_valid.
|
||
ci.SetContactPlane(ci.LastKnownContactPlane,
|
||
ci.LastKnownContactPlaneCellId,
|
||
ci.LastKnownContactPlaneIsWater);
|
||
// Then set Contact + OnWalkable (same logic as retail's 0050ad6a block):
|
||
oi.State |= ObjectInfoState.Contact;
|
||
if (ci.LastKnownContactPlane.Normal.Z >= PhysicsGlobals.FloorZ)
|
||
oi.State |= ObjectInfoState.OnWalkable;
|
||
else
|
||
oi.State &= ~ObjectInfoState.OnWalkable;
|
||
```
|
||
|
||
> **Note:** `SetContactPlane` also re-latches `LastKnownContactPlane`, `LastKnownContactPlaneCellId`, and `LastKnownContactPlaneIsWater` (TransitionTypes.cs:258-261). Passing LKCP as the source means the re-latch is a no-op on those fields — functionally safe, but worth knowing if you later decide to inline the writes instead of using `SetContactPlane`.
|
||
|
||
**Note on the LKCP-update strategy divergence (Gap 3):** Retail's `validate_transition`
|
||
at `0050acff` does `last_known_contact_plane_valid = contact_plane_valid`
|
||
unconditionally — this means when contact is invalid and stays invalid, LKCP is
|
||
cleared. Our code at TransitionTypes.cs:2801 only updates LKCP when current CP
|
||
is valid (L.2.3c deliberate divergence from 2026-04-29 to prevent animation
|
||
flicker on failed step-ups). **Do not change this in slice 1** — the Mechanism B
|
||
`SetContactPlane` call above feeds into the standard contact-valid branch (lines
|
||
2801–2819), which then re-latches LKCP normally. The net effect is equivalent
|
||
to retail's unconditional overwrite in the success case, without the flicker
|
||
regression of clearing LKCP on transient failures.
|
||
|
||
---
|
||
|
||
## 6. Risk — First-Frame Fall-Through
|
||
|
||
**Scenario:** Player teleports into a new indoor cell (or crosses a cell
|
||
boundary). On frame 0 in the new cell: LKCP is invalid (no prior frame data),
|
||
BSP returns OK (no wall collision, player is standing on a floor poly). With
|
||
the synthesis path stripped (Task 5) and Mechanism B requiring a valid LKCP,
|
||
this frame will have `ContactPlane` invalid for the indoor case.
|
||
|
||
**Consequence:** Frame 0 post-cell-cross → `ContactPlane` invalid → outdoor
|
||
terrain fallback fires → ValidateWalkable evaluates outdoor terrain Z → outdoor
|
||
Z is below indoor floor (due to +0.02f Z-bump) → player appears 0.02+ m above
|
||
the outdoor plane → ValidateWalkable decides they're airborne → `OnWalkable=false`
|
||
→ falling animation for one frame. Retail avoids this via Mechanism A: when BSP
|
||
Path 6 (step-down/land) fires on the first indoor frame, it writes CP directly
|
||
from the floor polygon.
|
||
|
||
**Assessment for slice 1:** Mechanism A is already wired in `BSPQuery.FindCollisions`
|
||
(calls `SetContactPlane` at BSPQuery.cs lines 1204 + 1713 for Path 6). If the
|
||
player's foot sphere is close enough to a floor polygon on the first frame
|
||
(within `step_sphere_down`'s probe distance), Path 6 will write CP and LKCP
|
||
will be primed via the `ci.ContactPlaneValid` branch (TransitionTypes.cs:2801).
|
||
Frame 1 will have LKCP valid and Mechanism B can take over.
|
||
|
||
**Risk is LOW for normal walking** (player stays near the floor, Path 6 fires
|
||
on the first frame in any cell). Risk is HIGHER for teleport-into-air edge
|
||
cases where the player spawns slightly above the floor and the step-down probe
|
||
misses. Accept for slice 1; slice 2 (Mechanism C) adds a direct floor-plane
|
||
probe from the new cell's geometry on first entry, closing the gap completely.
|
||
|
||
**Mitigation hedge:** When stripping `TryFindIndoorWalkablePlane` in Task 5,
|
||
do NOT strip the `ValidateWalkable` call — keep it guarded by `walkableHit`
|
||
being true. The fall-through to outdoor terrain remains as a last-resort
|
||
backstop for the single-frame miss (wrong Z, one frame of falling animation,
|
||
then Mechanism A re-grounds on the next frame). This is one visible frame of
|
||
glitch vs the current 86,748 CP writes per walk sequence. Acceptable for
|
||
slice 1.
|
||
|
||
---
|
||
|
||
## Summary Table
|
||
|
||
| Item | Retail | Our Code (pre-fix) |
|
||
|---|---|---|
|
||
| `find_env_collisions` writes CP? | No — only via BSP Path 6 (Mechanism A) | Yes — synthesis path writes CP every frame indoors |
|
||
| Mechanism B location | `CTransition::validate_transition`, Collided/Slid/Adjusted branch | Present but INCOMPLETE — sets flags only, no `SetContactPlane` call |
|
||
| Mechanism B proximity sphere | `global_curr_center` (frame-start center) | `GlobalSphere[0].Origin` (check position — wrong) |
|
||
| LKCP update strategy | Unconditional overwrite | Only on valid CP (L.2.3c deliberate fix) |
|
||
| First-frame risk | Mechanism C closes; Mechanism A covers normal cases | Same risk; accept for slice 1 |
|
||
|
||
---
|
||
|
||
## References
|
||
|
||
- `acclient_2013_pseudo_c.txt` lines 309570–309595 (`CEnvCell::find_env_collisions`)
|
||
- `acclient_2013_pseudo_c.txt` lines 272547–272700 (`CTransition::validate_transition`)
|
||
- `src/AcDream.Core/Physics/TransitionTypes.cs` lines 2751–2873 (`ValidateTransition`)
|
||
- `src/AcDream.Core/Physics/TransitionTypes.cs` lines 1514–1777 (`FindEnvCollisions`)
|
||
- `src/AcDream.Core/Physics/PhysicsEngine.cs` lines 640–692 (`RunTransitionResolve`)
|
||
- `src/AcDream.Core/Physics/BSPQuery.cs` lines 1204, 1713 (Mechanism A `SetContactPlane`)
|