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>
12 KiB
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:
// 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:
result > OK_TS && result <= SLID_TS— the transition ended in Collided, Adjusted, or Slid (not OK, not Invalid).collision_info.last_known_contact_plane_valid != 0— there is a remembered floor plane from a prior frame.- 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 sphereglobal_sphere) is still geometrically close to the last-known plane.
When all three pass, retail:
// 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:
- Add
ci.SetContactPlane(ci.LastKnownContactPlane, ci.LastKnownContactPlaneCellId, ci.LastKnownContactPlaneIsWater); - Change the proximity sphere center from
sp.GlobalSphere[0].Origin(line 2843) tosp.GlobalCurrCenter[0].Originto match retail'sglobal_curr_center.
The addition goes at TransitionTypes.cs approximately line 2849 (just before
the oi.State |= ObjectInfoState.Contact at current line 2852), producing:
// 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:
SetContactPlanealso re-latchesLastKnownContactPlane,LastKnownContactPlaneCellId, andLastKnownContactPlaneIsWater(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 usingSetContactPlane.
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.txtlines 309570–309595 (CEnvCell::find_env_collisions)acclient_2013_pseudo_c.txtlines 272547–272700 (CTransition::validate_transition)src/AcDream.Core/Physics/TransitionTypes.cslines 2751–2873 (ValidateTransition)src/AcDream.Core/Physics/TransitionTypes.cslines 1514–1777 (FindEnvCollisions)src/AcDream.Core/Physics/PhysicsEngine.cslines 640–692 (RunTransitionResolve)src/AcDream.Core/Physics/BSPQuery.cslines 1204, 1713 (Mechanism ASetContactPlane)