From 7e1be3def05a3a1270b018562fc5578638e97663 Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 30 Jul 2026 12:36:05 +0200 Subject: [PATCH] fix(physics): TransitionalInsert returns the real exhausted-retry state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per the TS-4/#116 oracle plan (docs/research/2026-07-30-ts4-116-oracle-plan.md §1.4, §4 item 1): TransitionalInsert's retry loop hardcoded `return TransitionState.Slid;` when the attempt budget exhausted, despite the comment's own claim of returning "whatever the last iteration said." ACE's equivalent (Transition.cs:933, `return transitState;`) and retail's (pc:273363, 0x0050b949, `return edi;`) both reuse one state variable across the composite per-attempt call and return whatever it holds. acdream's per-phase dispatch (env/building/object/other-cells/neg-poly/ step-down) is split across several locals instead of ACE's single composite call, so `transitState` is now re-synced from whichever phase-local variable most recently caused a retry `continue`, and the final return uses that real value instead of the hardcoded constant. Blast radius is zero: ValidateTransition's "not OK" branch treats Collided/Adjusted/Slid identically, and every caller of TransitionalInsert either feeds the result straight into ValidateTransition/ ValidatePlacementTransition (both `== OK` vs. not) or checks `== OK` directly. Full AcDream.Core.Tests suite: 4059 passed / 2 skipped, no change in pass count. Co-Authored-By: Claude Fable 5 --- src/AcDream.Core/Physics/TransitionTypes.cs | 45 +++++++++++++++++++-- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/src/AcDream.Core/Physics/TransitionTypes.cs b/src/AcDream.Core/Physics/TransitionTypes.cs index 343a52e6..d565bcec 100644 --- a/src/AcDream.Core/Physics/TransitionTypes.cs +++ b/src/AcDream.Core/Physics/TransitionTypes.cs @@ -1700,7 +1700,12 @@ public sealed class Transition var ci = CollisionInfo; var oi = ObjectInfo; - TransitionState transitState; + // Initializer is unreachable in practice (numAttempts > 0 is + // guaranteed by the guard above, so the loop below always runs at + // least once and assigns transitState before any read) — required + // only to satisfy C#'s definite-assignment analysis, which can't + // connect the early-return guard to the loop bound. + TransitionState transitState = TransitionState.OK; for (int attempt = 0; attempt < numAttempts; attempt++) { @@ -1742,6 +1747,7 @@ public sealed class Transition if (bldgState == TransitionState.Slid) { + transitState = bldgState; ci.ContactPlaneValid = false; ci.ContactPlaneIsWater = false; sp.NegPolyHit = false; @@ -1750,6 +1756,7 @@ public sealed class Transition if (bldgState == TransitionState.Adjusted) { + transitState = bldgState; sp.NegPolyHit = false; continue; } @@ -1771,6 +1778,7 @@ public sealed class Transition // Object collision applied a push-out and set sliding normal. // Retry at the new CheckPos — we may have slid into another // object, or need to re-verify env at the new position. + transitState = objState; ci.ContactPlaneValid = false; ci.ContactPlaneIsWater = false; sp.NegPolyHit = false; @@ -1781,6 +1789,7 @@ public sealed class Transition { // Object modified CheckPos (e.g. PerfectClip adjust_to_plane). // Retry at the new position. + transitState = objState; sp.NegPolyHit = false; continue; } @@ -1805,7 +1814,10 @@ public sealed class Transition return TransitionState.Collided; if (otherState != TransitionState.OK) + { + transitState = otherState; continue; // ADJUSTED / SLID → retry the attempt + } // ── Phase 3: both env and objects returned OK ────────────── // Handle Collide flag (BSP path 6 set it on a non-contact hit). @@ -1943,6 +1955,7 @@ public sealed class Transition var stepUpSlideRes = sp.StepUpSlide(this); if (stepUpSlideRes == TransitionState.Slid) { + transitState = stepUpSlideRes; ci.ContactPlaneValid = false; ci.ContactPlaneIsWater = false; continue; @@ -1979,6 +1992,7 @@ public sealed class Transition return TransitionState.Collided; // degenerate slide → hard stop // Slid / Adjusted / OK → re-test at the (slid) CheckPos, mirroring // retail's insert-loop continuation after slide_sphere. + transitState = slideRes; continue; } } @@ -2070,6 +2084,7 @@ public sealed class Transition var edgeState = EdgeSlideAfterStepDownFailed(engine, stepDownHeight, zVal); if (edgeState == TransitionState.Slid) { + transitState = edgeState; ci.ContactPlaneValid = false; ci.ContactPlaneIsWater = false; sp.NegPolyHit = false; @@ -2078,6 +2093,7 @@ public sealed class Transition if (edgeState == TransitionState.Adjusted) { + transitState = edgeState; sp.NegPolyHit = false; continue; } @@ -2088,9 +2104,30 @@ public sealed class Transition return TransitionState.OK; } - // Exhausted retry attempts — return whatever the last iteration said. - // (Defaults to Slid in practice since that's the only case that retries.) - return TransitionState.Slid; + // Exhausted retry attempts — return the real last transition state. + // + // TS-4/#116 oracle pass (Campaign P final physics slice, + // docs/research/2026-07-30-ts4-116-oracle-plan.md §1.4, §4 item 1): + // this was hardcoded to TransitionState.Slid despite the comment's + // own claim of returning "whatever the last iteration said" — a + // real, citable port-accuracy divergence from ACE's + // `return transitState;` (Transition.cs:933) and retail's + // `return edi;` (acclient_2013_pseudo_c.txt:273363, 0x0050b949), + // both of which reuse ONE state variable across the composite + // per-attempt call and return whatever it holds when the retry + // budget is exhausted. acdream's per-phase dispatch (env/building/ + // object/other-cells/neg-poly/step-down) is split across several + // locals instead of ACE's single composite call, so `transitState` + // is explicitly re-synced from whichever phase-local variable most + // recently caused a retry `continue` (see the `transitState = ...` + // assignments immediately above each `continue` in this loop). + // Blast radius: zero. ValidateTransition's "not OK" branch + // (`transitionState != TransitionState.Invalid`) treats + // Collided/Adjusted/Slid identically — every caller of + // TransitionalInsert either feeds its result straight into + // ValidateTransition/ValidatePlacementTransition (both of which + // only branch on `== OK` vs. not) or only checks `== OK` directly. + return transitState; } private TransitionState EdgeSlideAfterStepDownFailed(