GameCult/Aetheria
Watch
9
0
Fork
You've already forked Aetheria
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Cut 12.3 fix batch: unify Lane and Lanes on one membership query

Browse source 
Self's review found that commit 4e6495dd's fix for F1 replaced the per-lane
forward-arithmetic defect with a second, still-independent membership
computation: Lane's own half-open admission (Lo <= s < Hi, from a cell's own
Extent) versus Lanes' ceil((cell.Lo - centreLateral) / laneSpacing) lane
index. The two agree only when a cell's own (Hi - Lo) equals the externally
supplied laneSpacing exactly in float -- the identical two-derivations trap
Cut 12.2's own fix batch already found and removed from Lane itself,
reintroduced one layer up. Confirmed reachable: on commit 4e6495dd, solid5
b=(0,1) s=0.49999997 n=1 leaves the ceil-based centre lane EMPTY while Lane(s)
itself finds all 5 cells of the row -- breaking R3 ("the centre lane always
meets metal", Commit's own guard) and the 12.2 placement contract.

FireControl.Lanes is now built entirely out of FireControl.Lane: it calls
Lane(hull, b, centreLateral + k * laneSpacing, ...) once per lane index, so
Lane(s) is literally this function's n = 0 case rather than a parallel
re-implementation of its rule. Commit's placement and Apply's damage split
now read one function. The narrower question left -- can two individually
correct Lane() calls, at two different lines, still walk the same cell? --
is resolved by ownership (the lane closest to the centre keeps a contested
cell; the centre lane, k = 0, never loses one), not by a competing formula,
so the centre lane's final cell set is always exactly Lane(centreLateral)'s
own output.

Lane() itself is unchanged (still called directly by Commit and by
FireControlCut12Tests' edge-seed and interval-edge tests, all still green:
HitsLandOnTheFacingEdge, LaneSeamIsHalfOpen, LaneNeverEmptiesAtAnIntervalEdge,
Cut12-edge-68442709, Cut12-edge-64078070).

Adds LanesQueryIsDisjointMatchesLaneAtCentreAndNeverEmptyOnHull: now that
Lanes is a public, pure geometric query, the boundary sweep is a behavioural
test of its own contract, no reflection -- over every cell's own shadow edge
and float neighbours, at exact-axis, 45-degree, |bx| > |by| bearings and
Soul's own two reproduction cases, spread 1-2: (a) no cell is walked by two
lanes, (b) the centre lane equals a plain Lane(s) call, (c) the centre lane
is never empty when the centre lateral is inside the hull's own shadow. Hand-
confirmed to fail on 4e6495dd's ceil-based Lanes (property (b), the same
input above) and to pass on this commit (fc123fix2-mut3.log).

Adds CommittedCellIsTheFirstCellTheCentreLaneDamages: an end-to-end check
through Fire -> Commit -> Apply (damageSpread 0, a single lane) that the
committed outcome.Cell is the first cell Apply damages.

Full suite: 286 tests, identical across 3 runs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Meta Vens 2026-09-25 14:24:55 +02:00
commit cb71a3cfad
2 changed files with 190 additions and 49 deletions
Download patch file Download diff file Expand all files Collapse all files

93
Assets/Scripts/ServerShared/FireControl.cs
View file

@ -613,8 +613,15 @@ public static class FireControl
// offset by repeated addition can shift a side lane by an ulp or two, and a shifted s(li) can land inside a
// NEIGHBOURING cell's half-open interval that its own, unshifted s(li) was never meant to reach -- the same
// "two derivations of one quantity" defect Cut 12.2's own fix batch found and removed from Lane itself
// (S1 above), now recurring one layer up. See `Lanes` below, which decides each cell's lane membership with
// one computation instead of one comparison per (cell, lane) pair.
// (S1 above), now recurring one layer up.
// Self's review (2026-09-25): the first fix for this replaced the per-lane Lane() loop with a ceil-based
// index formula that computed lane membership from a cell's own Extent directly -- but that formula and
// Lane's own half-open test (Lo <= s < Hi) were still two separate computations, agreeing only when a
// cell's own (Hi - Lo) equalled laneSpacing exactly in float. That is the identical trap one level removed,
// not removed. `Lanes` below is the actual single owner: it calls Lane() itself once per lane index, so
// Commit's placement (Lane, the n = 0 case) and Apply's damage split (Lanes, every case) read one function,
// and resolves the resulting rare cross-lane collision by ownership rather than by a competing formula --
// see `Lanes`.
private static void Apply(PendingShot shot)
{
if (!shot.Outcome.Hit) return;
@ -676,49 +683,67 @@ public static class FireControl
target.DamageHull(hullDamage);
}
// F1 fix batch (Soul's second pass, 2026-09-25): lane membership decided ONCE per cell, replacing the old
// loop that called Lane() once per lane index and let each call test its own forward-accumulated lateral
// against a cell's interval independently of every other lane. Here every candidate cell computes its own
// lane index directly from its own (unmodified) Extent interval: k = ceil((Lo - centreLateral) /
// laneSpacing) is the unique integer for which centreLateral + k * laneSpacing falls in [Lo, Hi) -- Lane's
// own half-open admission -- derived by one division and one Ceiling call from values neither multiplied
// nor summed by any other lane's arithmetic. A single Math.Ceiling call returns exactly one integer, so a
// cell can be assigned to at most one lane by construction, not by float luck: disjointness no longer
// depends on laneSpacing surviving repeated addition without drifting off a neighbour's boundary.
// When k == 0 this is the same real-valued condition Lane() tests for centreLateral itself
// (Lo <= centreLateral < Hi, i.e. Lo - centreLateral in (-laneSpacing, 0]), so the centre lane's cell set
// is unchanged from calling Lane(hull, bearing, centreLateral, ...) directly -- the 12.2 placement contract
// (HitsLandOnTheFacingEdge) still holds. laneSpacing is still used, unchanged, to place the admitted lane's
// own line for AlongBearing's slab solve -- geometry, not membership -- so a lane's entry/exit values and
// its walked-contiguity break are exactly as before.
private static int[] Lanes(HullData hull, float2 b, float centreLateral, int n, float laneSpacing, LaneCell[][] buffers)
// Self's fix (2026-09-25): the ONE public membership query behind both Commit's placement and Apply's
// damage split. Lane(s) is exactly this function's n = 0 case -- Lanes calls Lane(hull, b, centreLateral +
// k * laneSpacing, ...) once per lane index k, so a lane's own cell set can never disagree with what
// Lane(s) alone returns for that s: there is no second formula for "does this cell belong to this lane",
// only Lane's own half-open Extent test, called at 2n+1 different lines. This is what removes the trap a
// ceil-based index formula reintroduced one level up from Cut 12.2's own crash (see Apply's comment above):
// that formula and Lane's own test were still two computations, agreeing only when a cell's own (Hi - Lo)
// equalled the externally supplied laneSpacing exactly in float.
// What is left, once every lane's own result is individually exact, is a DIFFERENT and much narrower
// question: can two separate calls to Lane(), at two different (and individually correct) lines, still walk
// the SAME cell? Yes, rarely -- laneSpacing (F2's shadow width, one value shared by every lane) can differ
// by a few ulps from one specific cell's own computed (Hi - Lo), the same float drift Soul's second pass
// found. Lanes resolves that by ownership, not by a competing membership rule: a cell walked by more than
// one lane keeps the lane closest to the centre (ties are impossible -- equal |k| means the same lane), and
// the centre lane (k = 0) never loses a cell to a side lane. Its own final cell set is therefore always
// exactly Lane(centreLateral)'s own output, unmodified -- R3 (the centre lane always meets metal, Commit's
// own guard) and the 12.2 placement contract (HitsLandOnTheFacingEdge) hold structurally, not by chance. A
// side lane that cedes a cell simply omits it from its own walk; the walk's existing contiguity rule (a
// cell more than 1e-4 past the previous SURVIVING cell's exit ends the lane) then decides on its own whether
// the lane continues past the gap that leaves -- no new rule, the same one `Lane` already applies at a
// genuine gap. laneSpacing itself is untouched: it is still the one F2 value that places every lane's own
// query line.
public static int[] Lanes(HullData hull, float2 b, float centreLateral, int n, float laneSpacing, LaneCell[][] buffers)
{
var ell = Lateral(b);
var coords = hull.Shape.Coordinates;
var laneCount = 2 * n + 1;
var counts = new int[laneCount];
for (var i = 0; i < coords.Length; i++)
var raw = new LaneCell[laneCount][];
var rawCount = new int[laneCount];
for (var li = 0; li < laneCount; li++)
{
var c = (float2) coords[i];
var lateral = Extent(c, ell);
var k = (int) Math.Ceiling((lateral.Lo - centreLateral) / laneSpacing);
if (k < -n || k > n) continue;
var li = k + n;
var k = li - n;
var s = centreLateral + k * laneSpacing;
AlongBearing(c, b, ell, s, out var entry, out var exit);
buffers[li][counts[li]++] = new LaneCell { Cell = coords[i], Entry = entry, Exit = exit, Projection = dot(c, b) };
raw[li] = ArrayPool<LaneCell>.Shared.Rent(hull.Shape.Coordinates.Length);
rawCount[li] = Lane(hull, b, s, raw[li]);
}
// Ownership: whichever lane is closest to the centre keeps a cell two lanes both walked; the centre
// lane (k = 0, |k| = 0) can never be outranked.
var ownerLane = new Dictionary<int2, int>();
for (var li = 0; li < laneCount; li++)
{
var k = Math.Abs(li - n);
for (var i = 0; i < rawCount[li]; i++)
{
var cell = raw[li][i].Cell;
if (!ownerLane.TryGetValue(cell, out var curLi) || Math.Abs(curLi - n) > k)
ownerLane[cell] = li;
}
}
var walked = new int[laneCount];
for (var li = 0; li < laneCount; li++)
{
Array.Sort(buffers[li], 0, counts[li]);
walked[li] = counts[li] > 0 ? 1 : 0;
for (var i = 1; i < counts[li]; i++)
var w = 0;
for (var i = 0; i < rawCount[li]; i++)
{
if (buffers[li][i].Entry > buffers[li][i - 1].Exit + 1e-4f) break;
walked[li]++;
if (ownerLane[raw[li][i].Cell] != li) continue; // ceded to a closer lane -- this lane's own gap
if (w > 0 && raw[li][i].Entry > buffers[li][w - 1].Exit + 1e-4f) break;
buffers[li][w++] = raw[li][i];
}
walked[li] = w;
ArrayPool<LaneCell>.Shared.Return(raw[li]);
}
return walked;
}

146
tests/Aetheria.Shared.Tests/FireControlCut123Tests.cs
View file

@ -953,21 +953,137 @@ public sealed class FireControlCut123Tests : IDisposable
// Lateral's draw visits many different points along the hull's shadow), and ample penetration. Heavy
// uniform armour so every reached cell fires exactly one ArmorDamage event, and a cell reached by two lanes
// in the SAME shot fires twice.
// F1's own structural fix (`FireControl.Lanes`) assigns each cell to exactly one lane index by a single
// computation, so after that fix a wrong `laneSpacing` value can misassign a cell to the wrong lane or drop
// it, but can no longer make two lanes claim the SAME cell -- double-admission is not a reachable outcome
// of a bad spacing constant any more, only of a defect in the single-computation assignment itself. Hand
// verification (fc123fix2-mut.log) confirms this split: reverting the spacing to one cell (R0), or to a
// wrong formula (R1c/R1e/R1h, all `dotnet test`), is caught by the pre-existing damage-shape tests
// (`DirectHitAtAnAngledBearingMatchesAnIndependentModel`, `FootprintWidensAtAnAngledBearingComparedToAxisAligned`,
// and others) rather than by this test -- this test never failed for any of those four reverts, because the
// hull it drove genuinely was never struck twice under any of them. A 1% narrowing (R1b) survives the
// whole 97-test suite, including this one; recorded as an unresolved survivor rather than claimed dead (an
// honest gap, not a claimed kill), left for Soul's own mutation pass. This test's own job is narrower than
// that: it is a standing regression guard on the disjointness invariant itself, through the real
// Fire/Commit/Apply pipeline, so a future change that reintroduces per-lane admission testing (the shape of
// the original F1 defect) has a chance of being caught here even though it can no longer be manufactured by
// hand through a spacing-constant mutation.
// F1's own structural fix (`FireControl.Lanes`, now built on top of `FireControl.Lane` itself -- see that
// function's own comment) makes double-admission unreachable from a bad `laneSpacing` value: a wrong
// spacing can misassign a cell to the wrong lane or drop it, but can no longer make two lanes claim the
// SAME cell. Hand verification (fc123fix2-mut.log) confirms this split: reverting the spacing to one cell
// (R0), or to a wrong formula (R1c/R1e/R1h, all `dotnet test`), is caught by the pre-existing damage-shape
// tests (`DirectHitAtAnAngledBearingMatchesAnIndependentModel`,
// `FootprintWidensAtAnAngledBearingComparedToAxisAligned`, and others) rather than by this test -- this test
// never failed for any of those four reverts, because the hull it drove genuinely was never struck twice
// under any of them. A 1% narrowing (R1b) survives the whole 97-test suite, including this one; recorded as
// an unresolved survivor rather than claimed dead (an honest gap, not a claimed kill), left for Soul's own
// mutation pass. This test's own job is narrower than that: it is a standing regression guard on the
// disjointness invariant itself, through the real Fire/Commit/Apply pipeline, so a future change that
// reintroduces per-lane admission testing (the shape of the original F1 defect) has a chance of being
// caught here even though it can no longer be manufactured by hand through a spacing-constant mutation.
// ===================== F1 (Self's review): Lane and Lanes are one membership query =====================
// Self's finding, 2026-09-25: the first F1 fix (commit 4e6495dd) removed the per-lane forward-arithmetic
// defect but replaced it with a SECOND membership formula -- lane index k = ceil((cell.Lo - centreLateral)
// / laneSpacing), agreeing with FireControl.Lane's own half-open admission (Lo <= s < Hi) only when a
// cell's own (Hi - Lo) equals laneSpacing exactly in float. `FireControl.Lanes` is now built entirely out
// of `FireControl.Lane` -- it calls Lane() once per lane index, so Lane(s) IS the n = 0 case, not a second
// implementation of it, and any remaining cross-lane collision (still possible, since laneSpacing is one
// shared value and one specific cell's own Hi - Lo can still drift a few ulps from it) is resolved by
// ownership (closest to the centre wins; the centre lane never loses), not by a competing formula. This
// test pins that contract directly through the PUBLIC FireControl.Lanes and FireControl.Lane, no reflection
// and no production Lane/Apply used to build an expected value -- the assertions are properties of the
// functions' own output: (a) no cell is ever walked by two lanes, (b) the centre lane's cells are always
// exactly what a plain Lane(s) call returns, (c) the centre lane is never empty when the centre lateral is
// inside the hull's own shadow (Extent/Lateral's documented formula -- ell = (-by, bx), h = (|ellx|+|elly|)
// / 2, interval = [dot(c,ell)-h, dot(c,ell)+h) -- is independently recomputed here purely to build candidate
// laterals). Hand-confirmed to fail on commit 4e6495dd's ceil-based Lanes (fc123fix2-mut3.log): property (b)
// fails partway through the deterministic sweep below (solid5, b=(0,1), s=0.49999997, n=1 -- the ceil
// formula leaves the centre lane empty while Lane(s) itself finds all 5 cells of the row, the exact "centre
// lane always meets metal" break the ceil design could cause). Soul's own two reproduction cases below
// happen to still agree on that specific ceil build; the sweep is what catches it.
[Fact]
public void LanesQueryIsDisjointMatchesLaneAtCentreAndNeverEmptyOnHull()
{
static float2 Ell(float2 b) => float2(-b.y, b.x);
static (float Lo, float Hi) CellExtent(int2 c, float2 ell)
{
var h = (Math.Abs(ell.x) + Math.Abs(ell.y)) / 2f;
var centre = dot((float2) c, ell);
return (centre - h, centre + h);
}
static float Spacing(float2 b) { var ell = Ell(b); return Math.Abs(ell.x) + Math.Abs(ell.y); }
void Check(string name, Shape shape, float2 b, float s, int n)
{
var hull = new HullData { Name = name, HullType = HullType.Ship, Shape = shape };
var spacing = Spacing(b);
var laneCount = 2 * n + 1;
var buffers = new LaneCell[laneCount][];
for (var li = 0; li < laneCount; li++) buffers[li] = new LaneCell[shape.Coordinates.Length];
var walked = FireControl.Lanes(hull, b, s, n, spacing, buffers);
// (a) no cell appears in two lanes.
var seen = new HashSet<int2>();
for (var li = 0; li < laneCount; li++)
for (var i = 0; i < walked[li]; i++)
Assert.True(seen.Add(buffers[li][i].Cell),
$"{name} b=({b.x:R},{b.y:R}) s={s:R} n={n}: cell {buffers[li][i].Cell} walked by two lanes (lane {li - n})");
// (b) the centre lane equals a plain Lane(s) call.
var solo = new LaneCell[shape.Coordinates.Length];
var soloWalked = FireControl.Lane(hull, b, s, solo);
Assert.True(soloWalked == walked[n], $"{name} b=({b.x:R},{b.y:R}) s={s:R} n={n}: centre lane walked {walked[n]}, Lane(s) walked {soloWalked}");
for (var i = 0; i < soloWalked; i++)
Assert.Equal(solo[i].Cell, buffers[n][i].Cell);
// (c) the centre lane is never empty when s is inside the hull's own shadow.
var ell = Ell(b);
var insideShadow = shape.Coordinates.Any(c => { var (lo, hi) = CellExtent(c, ell); return s >= lo && s < hi; });
if (insideShadow)
Assert.True(walked[n] > 0, $"{name} b=({b.x:R},{b.y:R}) s={s:R} n={n}: centre lane empty although s is inside the hull's own shadow");
}
// Soul's own concrete reproductions (SoulApply123b.Disjoint, 2026-09-25).
Check("solid5", SolidShape(5, 5), float2(0, 1), -0.50000006f, 1);
Check("solid9", SolidShape(9, 9), normalize(float2(-0.69478226f, 0.7192201f)), -5.619354f, 1);
// A deterministic sweep at every cell's own shadow edge (Lo and Hi) and its immediate float neighbours,
// across several hulls, bearings (exact axis, 45 degrees, |bx| > |by|, and Soul's own angled example),
// and spreads 1-2.
foreach (var (name, shape) in new (string, Shape)[] { ("solid5", SolidShape(5, 5)), ("solid9", SolidShape(9, 9)), ("solid3x11", SolidShape(3, 11)) })
{
var coords = shape.Coordinates;
foreach (var b in new[] { float2(0, 1), float2(1, 0), normalize(float2(1, 1)), normalize(float2(2, 1)), normalize(float2(-0.69478226f, 0.7192201f)) })
{
var ell = Ell(b);
foreach (var c in coords)
{
var (lo, hi) = CellExtent(c, ell);
foreach (var edge in new[] { lo, hi, MathF.BitIncrement(lo), MathF.BitDecrement(lo), MathF.BitIncrement(hi), MathF.BitDecrement(hi) })
for (var n = 1; n <= 2; n++)
Check(name, shape, b, edge, n);
}
}
}
}
// F1 (Self's review): the committed Cell (Commit's own placement) is the first cell Apply damages in the
// centre lane -- end to end through Fire -> Commit -> Apply, no reflection. DamageSpread 0 (a single lane,
// the centre lane) so the first ArmorDamage event of the whole shot is unambiguously the centre lane's own
// first cell -- Apply's damage loop processes lanes index 0 upward, so with a spread this check would be
// reading a SIDE lane's own first event, not the centre lane's. Heavy uniform armour and ample penetration
// so that first event is well defined.
[Fact]
public void CommittedCellIsTheFirstCellTheCentreLaneDamages()
{
var bearings = new[]
{
float2(0, 1), normalize(float2(1, 1)), normalize(float2(-1, 1)),
normalize(float2(2, 1)), normalize(float2(1, 2))
};
foreach (var travelDirection in bearings)
{
var shape = SolidShape(9, 9);
var e = Build(TestSettings(), shape, precision: 1f, penetration: 100f, damageSpread: 0f);
e.Shooter.Position = e.Target.Position - float3(travelDirection.x, 0, travelDirection.y) * 100f;
foreach (var c in shape.Coordinates) { e.Target.Armor[c.x, c.y] = 1000f; e.Target.MaxArmor[c.x, c.y] = 1000f; }
var firstHit = new int2?[1];
using var s = e.Target.ArmorDamage.Subscribe(x => { if (firstHit[0] == null) firstHit[0] = x.pos; });
var outcome = FireUntilHit(e, damageOverride: 50f);
Assert.NotNull(outcome);
Assert.True(outcome.Hit);
Assert.NotNull(firstHit[0]);
Assert.Equal(outcome.Cell, firstHit[0].Value);
}
}
[Fact]
public void NoCellIsStruckByTwoLanesInOneShotAtAnyBearing()
{