Transition.FindObjCollisionsInCell discarded a shadow candidate when |currPos - obj.Position| > sphereRadius + obj.Radius + movement.Length() + 2f obj.Position is the part ORIGIN; obj.Radius is the physics-BSP ROOT BOUNDING SPHERE's radius, measured about a centre AP-156 established is frequently metres from that origin (376 of 973 installed physics-BSP parts sit further from their part origin than half their own radius, worst 20.762 m). Geometry deep inside the real bounding sphere was therefore thrown away before BSPQuery ever ran: solid near the origin, permeable in a bounded shell beyond it. For the Neftet rock 0xC8766009 / gfx=0x01004751 the two points are 23.556 m apart, which is #337 — wedged on the plateau, jumps sinking into the mesh, corpses falling through. A live capture recorded 7,225 rejections on that one owner, every single one with wouldAcceptAtCenter=True. Deleted rather than re-centred. Retail has no distance pre-filter, disassembled from the PDB-paired v11.4186 binary (CodeView GUID 9e847e2f-777c-4bd9-886c-22256bb87f32) rather than read from Binary Ninja: CObjCell::find_obj_collisions @0x0052b750 walks shadow_object_list and calls CPhysicsObj::FindObjCollisions (0x0052b78b) UNCONDITIONALLY; its only early-out is insert_type == INITIAL_PLACEMENT_INSERT (0x0052b759). CPhysicsObj::FindObjCollisions @0x0050f050 contains no float compare at all. CPartArray::FindObjCollisions @0x00518180 is a bare do/while over parts, and CPhysicsPart::find_obj_collisions @0x0050d8d0 is two null checks plus a call. Retail's only spatial rejection is the BSP node bounding-sphere test inside the walk — correctly centred, which is exactly what the deleted filter was not. Re-centring it (carry BoundsCenter on ShadowEntry) would have preserved an invention retail does not have, including a +2f slack and a movement.Length() term with no retail counterpart, and left a second reach budget to be tuned forever. Retail's own cross-cell slack constant is F_EPSILON = 0.0002 m, not 2 m. The method's comment claimed the filter was "the analog of the part sorting-sphere early-outs inside retail's CPhysicsObj::FindObjCollisions — response-neutral, pure perf". Both halves were false and cost #333 and #337; it is replaced by the disassembly above. Gate: Issue333BroadphaseReachFilterTests drives the production path end-to-end (ResolveWithTransition -> FindObjCollisionsInCell -> CollisionTraversal) on a DAT-free fixture so it runs everywhere, as a discriminating pair. Sabotage-verified: restore the pre-check and OffCentreBspFloorStopsAFallingMover reaches z=37.800 — exactly the unobstructed fall, blockedAtLeastOnce=False — while CentredBspFloorStopsAFallingMover keeps passing. Without the control a fixture unable to fall would pass the first test for the wrong reason. Issue337's skipped TheBroadphaseAdmitsTheSurfaceTheMoverIsStandingOn asserted the now-deleted predicate and could never have gone green; it is rewritten as installed-DAT evidence pinning BOTH halves of the diagnosis and is no longer skipped. Perf measured, not assumed (Release, synthetic all-BSP cell, per ResolveWithTransition): at 38 candidates — the live maximum — 10.61 us -> 16.68 us (1.57x); at a deliberately unreachable 200, 17.34 -> 39.48 us (2.28x); ~0.16 us per additional candidate tested. Over 19,701 live [reach-q] samples the in-cell count is p50 = 9, p99 = 32, max 38. The ACDREAM_PROBE_REACH rejectedReach column is kept and is now structurally 0, so a post-fix capture stays comparable with the pre-fix one; dropping it would make the two incomparable. AP-158 retired (110 active AP rows). #333 and #337 closed pending the user's live acceptance at Neftet. Solution suite 11,231 passed / 4 skipped / 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Erik
parent
5a1eeace73
commit
ea83b043df
7 changed files with 478 additions and 85 deletions
|
|
@ -0,0 +1,240 @@
|
|||
using System.Collections.Generic;
|
||||
using System.Numerics;
|
||||
using AcDream.Core.Physics;
|
||||
using DatReaderWriter.Enums;
|
||||
using DatReaderWriter.Types;
|
||||
using Xunit;
|
||||
|
||||
namespace AcDream.Core.Tests.Physics;
|
||||
|
||||
/// <summary>
|
||||
/// #333 / #337 regression — the query-site broadphase reach filter.
|
||||
///
|
||||
/// <para>
|
||||
/// <c>Transition.FindObjCollisionsInCell</c> used to discard a shadow candidate
|
||||
/// when <c>|currPos - obj.Position| > sphereRadius + obj.Radius +
|
||||
/// movement.Length() + 2f</c>. <c>obj.Position</c> is the part ORIGIN while
|
||||
/// <c>obj.Radius</c> is the physics-BSP ROOT BOUNDING SPHERE's radius, measured
|
||||
/// about a centre that is frequently metres away from that origin (AP-156:
|
||||
/// 376 of 973 installed physics-BSP parts sit further from their part origin
|
||||
/// than half their own radius, worst 20.762 m). Geometry deep inside the real
|
||||
/// bounding sphere was therefore rejected before <c>BSPQuery</c> ever ran —
|
||||
/// solid near the origin, permeable in a bounded shell beyond it. That is the
|
||||
/// mechanism of #337 (the Neftet plateau: wedged on top, jumps sink in,
|
||||
/// corpses fall through), measured in
|
||||
/// <c>docs/research/2026-08-06-337-neftet-wedge-mechanism.md</c>.
|
||||
/// </para>
|
||||
///
|
||||
/// <para>
|
||||
/// Retail has no such filter. Disassembled from the PDB-paired v11.4186
|
||||
/// binary: <c>CObjCell::find_obj_collisions</c> @0x0052b750 calls
|
||||
/// <c>CPhysicsObj::FindObjCollisions</c> unconditionally,
|
||||
/// <c>CPhysicsObj::FindObjCollisions</c> @0x0050f050 has no float compare at
|
||||
/// all, <c>CPartArray::FindObjCollisions</c> @0x00518180 is a bare loop over
|
||||
/// parts, and <c>CPhysicsPart::find_obj_collisions</c> @0x0050d8d0 is two null
|
||||
/// checks plus <c>CGfxObj::find_obj_collisions</c> @0x00534700.
|
||||
/// </para>
|
||||
///
|
||||
/// <para>
|
||||
/// This fixture is deliberately DAT-free so the gate runs everywhere, and it
|
||||
/// drives the PRODUCTION path end-to-end
|
||||
/// (<c>PhysicsEngine.ResolveWithTransition</c> →
|
||||
/// <c>Transition.FindObjCollisionsInCell</c> → <c>CollisionTraversal</c>)
|
||||
/// rather than re-computing a predicate in the test. Sabotage: restore the
|
||||
/// <c>maxReach</c> pre-check and <see cref="OffCentreBspFloorStopsAFallingMover"/>
|
||||
/// fails while <see cref="CentredBspFloorStopsAFallingMover"/> keeps passing —
|
||||
/// the pair discriminates "the filter is gone" from "the fixture cannot fall".
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public sealed class Issue333BroadphaseReachFilterTests
|
||||
{
|
||||
private const uint LandblockId = 0xA9B60000u;
|
||||
private const uint CellId = LandblockId | 0x0001u;
|
||||
private const uint EntityId = 0x333BEEF1u;
|
||||
private const uint GfxObjId = 0x333BEEF2u;
|
||||
private const ushort FloorPolyId = 1;
|
||||
|
||||
private const float SphereRadius = 0.48f;
|
||||
private const float SphereHeight = 1.835f; // human Setup 0x02000001
|
||||
private const float StepUpHeight = 0.60f;
|
||||
private const float StepDownHeight = 0.04f;
|
||||
|
||||
/// <summary>Part origin. The filter measured to THIS point.</summary>
|
||||
private static readonly Vector3 PartOrigin = new(12f, 12f, 0f);
|
||||
|
||||
/// <summary>
|
||||
/// Root bounding-sphere radius the registry publishes for this object,
|
||||
/// and the radius of the single BSP leaf node.
|
||||
/// </summary>
|
||||
private const float RootSphereRadius = 6f;
|
||||
|
||||
/// <summary>
|
||||
/// #333 REGRESSION. A floor slab whose geometry — and whose BSP root
|
||||
/// bounding sphere — sit 40 m above the part origin must stop a mover
|
||||
/// falling onto it.
|
||||
///
|
||||
/// <para>
|
||||
/// With the deleted filter the mover was 40.48 m from the part origin
|
||||
/// against a budget of 0.48 + 6 + ~0.3 + 2 = 8.78 m, so the candidate was
|
||||
/// discarded and the mover fell straight through. Measured to the root
|
||||
/// sphere's own centre it is 1.48 m against a 6 m radius — inside by
|
||||
/// 4.5 m of margin. Retail's BSP node test uses that centre; the filter
|
||||
/// used the origin.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void OffCentreBspFloorStopsAFallingMover()
|
||||
{
|
||||
var engine = BuildEngineWithFloorSlab(slabLocalZ: 40f);
|
||||
|
||||
// Precondition, so a membership failure cannot masquerade as the
|
||||
// defect this test is about.
|
||||
Assert.NotEmpty(engine.ShadowObjects.GetObjectsInCell(CellId));
|
||||
|
||||
var (finalFeetZ, blocked) = DropOnto(engine, startFeetZ: 41.4f);
|
||||
|
||||
Assert.True(
|
||||
finalFeetZ > 39.5f,
|
||||
$"the mover fell through a floor slab 40 m above its owner's part "
|
||||
+ $"origin: feet reached z={finalFeetZ:F3}, and an unobstructed "
|
||||
+ $"fall would have reached 37.8. blockedAtLeastOnce={blocked}. "
|
||||
+ $"This is #333 — the query-site broadphase discarded the "
|
||||
+ $"candidate before BSPQuery ever ran.");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// CONTROL. The identical slab, this time AT the part origin, so the
|
||||
/// deleted filter admitted it. It blocked before the fix and must still
|
||||
/// block after. Without this row, a fixture that simply cannot fall would
|
||||
/// pass <see cref="OffCentreBspFloorStopsAFallingMover"/> for the wrong
|
||||
/// reason.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void CentredBspFloorStopsAFallingMover()
|
||||
{
|
||||
var engine = BuildEngineWithFloorSlab(slabLocalZ: 0f);
|
||||
|
||||
Assert.NotEmpty(engine.ShadowObjects.GetObjectsInCell(CellId));
|
||||
|
||||
var (finalFeetZ, blocked) = DropOnto(engine, startFeetZ: 1.4f);
|
||||
|
||||
Assert.True(
|
||||
finalFeetZ > -0.5f,
|
||||
$"the control slab at the part origin must still stop the mover; "
|
||||
+ $"feet reached z={finalFeetZ:F3}, blockedAtLeastOnce={blocked}.");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Drop the mover straight down in 0.30 m steps for 12 ticks. Returns the
|
||||
/// final FEET height and whether any tick reported a collision normal.
|
||||
/// </summary>
|
||||
private static (float FinalFeetZ, bool Blocked) DropOnto(
|
||||
PhysicsEngine engine, float startFeetZ)
|
||||
{
|
||||
var pos = new Vector3(PartOrigin.X, PartOrigin.Y, startFeetZ);
|
||||
bool blocked = false;
|
||||
|
||||
for (int tick = 0; tick < 12; tick++)
|
||||
{
|
||||
Vector3 target = pos - new Vector3(0f, 0f, 0.30f);
|
||||
var result = engine.ResolveWithTransition(
|
||||
pos, target, CellId,
|
||||
SphereRadius, SphereHeight,
|
||||
StepUpHeight, StepDownHeight,
|
||||
isOnGround: false,
|
||||
body: null,
|
||||
moverFlags: ObjectInfoState.IsPlayer | ObjectInfoState.EdgeSlide,
|
||||
movingEntityId: 0);
|
||||
|
||||
blocked |= result.CollisionNormalValid;
|
||||
pos = result.Position;
|
||||
}
|
||||
|
||||
return (pos.Z, blocked);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// One static BSP owner at <see cref="PartOrigin"/> carrying a single
|
||||
/// horizontal floor polygon (8 × 8 m, normal +Z) at
|
||||
/// <paramref name="slabLocalZ"/> in the owner's local frame, with the BSP
|
||||
/// root node's bounding sphere centred on the slab — exactly the
|
||||
/// origin-vs-centre offset the DAT authors for large formations.
|
||||
/// </summary>
|
||||
private static PhysicsEngine BuildEngineWithFloorSlab(float slabLocalZ)
|
||||
{
|
||||
var cache = new PhysicsDataCache();
|
||||
var engine = new PhysicsEngine { DataCache = cache };
|
||||
|
||||
// Flat terrain far below: it must never be what stops the mover.
|
||||
var heights = new byte[81];
|
||||
var heightTable = new float[256];
|
||||
for (int i = 0; i < 256; i++) heightTable[i] = -1000f;
|
||||
engine.AddLandblock(
|
||||
landblockId: LandblockId,
|
||||
terrain: new TerrainSurface(heights, heightTable),
|
||||
cells: System.Array.Empty<CellSurface>(),
|
||||
portals: System.Array.Empty<PortalPlane>(),
|
||||
worldOffsetX: 0f,
|
||||
worldOffsetY: 0f);
|
||||
|
||||
var floorVerts = new[]
|
||||
{
|
||||
new Vector3(-4f, -4f, slabLocalZ),
|
||||
new Vector3( 4f, -4f, slabLocalZ),
|
||||
new Vector3( 4f, 4f, slabLocalZ),
|
||||
new Vector3(-4f, 4f, slabLocalZ),
|
||||
};
|
||||
var floorNormal = new Vector3(0f, 0f, 1f);
|
||||
var floorPoly = new ResolvedPolygon
|
||||
{
|
||||
Vertices = floorVerts,
|
||||
Plane = new Plane(floorNormal, -Vector3.Dot(floorNormal, floorVerts[0])),
|
||||
NumPoints = 4,
|
||||
SidesType = CullMode.None,
|
||||
};
|
||||
|
||||
var leaf = new PhysicsBSPNode
|
||||
{
|
||||
Type = BSPNodeType.Leaf,
|
||||
BoundingSphere = new Sphere
|
||||
{
|
||||
Origin = new Vector3(0f, 0f, slabLocalZ),
|
||||
Radius = RootSphereRadius,
|
||||
},
|
||||
};
|
||||
leaf.Polygons.Add(FloorPolyId);
|
||||
|
||||
var physics = new GfxObjPhysics
|
||||
{
|
||||
BSP = new PhysicsBSPTree { Root = leaf },
|
||||
PhysicsPolygons = new Dictionary<ushort, Polygon>(),
|
||||
Vertices = new VertexArray(),
|
||||
Resolved = new Dictionary<ushort, ResolvedPolygon> { [FloorPolyId] = floorPoly },
|
||||
BoundingSphere = new Sphere
|
||||
{
|
||||
Origin = new Vector3(0f, 0f, slabLocalZ),
|
||||
Radius = RootSphereRadius,
|
||||
},
|
||||
};
|
||||
cache.RegisterGfxObjForTest(GfxObjId, physics);
|
||||
|
||||
// Registration publishes the ROOT SPHERE'S RADIUS about the PART
|
||||
// ORIGIN — the exact pairing the deleted filter then mis-measured.
|
||||
engine.ShadowObjects.Register(
|
||||
entityId: EntityId,
|
||||
gfxObjId: GfxObjId,
|
||||
worldPos: PartOrigin,
|
||||
rotation: Quaternion.Identity,
|
||||
radius: RootSphereRadius,
|
||||
worldOffsetX: 0f,
|
||||
worldOffsetY: 0f,
|
||||
landblockId: LandblockId,
|
||||
collisionType: ShadowCollisionType.BSP,
|
||||
cylHeight: 0f,
|
||||
scale: 1.0f,
|
||||
state: 0x1u, // STATIC_PS
|
||||
flags: EntityCollisionFlags.None);
|
||||
|
||||
return engine;
|
||||
}
|
||||
}
|
||||
|
|
@ -583,19 +583,19 @@ public sealed class Issue337NeftetRockGeometryInspectionTests
|
|||
}
|
||||
|
||||
/// <summary>
|
||||
/// #337 REPRODUCER — currently FAILING, hence skipped.
|
||||
/// #337 EVIDENCE — the installed-DAT measurement that condemned the
|
||||
/// query-site broadphase, pinned so the diagnosis stays checkable.
|
||||
///
|
||||
/// <para>
|
||||
/// The per-object broadphase in
|
||||
/// <c>Transition.FindObjCollisionsInCell</c> (TransitionTypes.cs, the
|
||||
/// <c>maxReach</c> test) measures the mover's distance to the shadow
|
||||
/// entry's <c>Position</c> — the part ORIGIN — and compares it against
|
||||
/// <c>obj.Radius</c>, which is the physics-BSP ROOT BOUNDING SPHERE's
|
||||
/// radius. For this rock those two are 23.6 m apart, so a mover standing
|
||||
/// on its plateau is inside the bounding sphere by ~20 m of margin and
|
||||
/// still fails the test. It is the same defect AP-156 fixed in the flood
|
||||
/// The deleted per-object filter in
|
||||
/// <c>Transition.FindObjCollisionsInCell</c> measured the mover's distance
|
||||
/// to the shadow entry's <c>Position</c> — the part ORIGIN — and compared
|
||||
/// it against <c>obj.Radius</c>, which is the physics-BSP ROOT BOUNDING
|
||||
/// SPHERE's radius. For this rock those two points are 23.6 m apart, so a
|
||||
/// mover standing on its plateau is inside the bounding sphere by ~20 m of
|
||||
/// margin and still failed the test. Same defect AP-156 fixed in the flood
|
||||
/// and #334 fixed in the registration extent walk, left in place at the
|
||||
/// query site (filed as #333).
|
||||
/// query site — filed as #333.
|
||||
/// </para>
|
||||
///
|
||||
/// <para>
|
||||
|
|
@ -606,14 +606,24 @@ public sealed class Issue337NeftetRockGeometryInspectionTests
|
|||
/// verified instruction-by-instruction against the PDB-paired
|
||||
/// v11.4186 binary. Neither contains a compare or any float math. The
|
||||
/// only spatial rejection retail performs is the BSP node bounding-sphere
|
||||
/// test inside the walk, which is correctly centred.
|
||||
/// test inside the walk, which is correctly centred. #333 therefore
|
||||
/// deleted the filter outright rather than re-centring it.
|
||||
/// </para>
|
||||
///
|
||||
/// Un-skip this as the acceptance gate for the fix.
|
||||
/// <para>
|
||||
/// This test asserts the DATA, not the production predicate — the
|
||||
/// production gate is
|
||||
/// <c>Issue333BroadphaseReachFilterTests.OffCentreBspFloorStopsAFallingMover</c>,
|
||||
/// which drives <c>ResolveWithTransition</c> end-to-end. Both halves must
|
||||
/// hold for the diagnosis to be the one recorded: the origin-measured
|
||||
/// distance OUTSIDE the old budget, the centre-measured distance
|
||||
/// comfortably INSIDE the same radius. If a future DAT or transform change
|
||||
/// makes either false, the recorded mechanism no longer describes this
|
||||
/// object and the research note needs revisiting.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
[Fact(Skip = "#337: fails until the query-site broadphase measures to the "
|
||||
+ "BSP bounding-sphere centre (or is removed, as retail has none).")]
|
||||
public void TheBroadphaseAdmitsTheSurfaceTheMoverIsStandingOn()
|
||||
[Fact]
|
||||
public void TheOldBroadphaseMeasuredToTheOriginAndSoRejectedGeometryItStoodOn()
|
||||
{
|
||||
string? datDir = ConformanceDats.ResolveDatDir();
|
||||
if (datDir is null) return;
|
||||
|
|
@ -628,19 +638,27 @@ public sealed class Issue337NeftetRockGeometryInspectionTests
|
|||
const float SphereRadius = 0.48f;
|
||||
const float Movement = 0.308f; // the live step length
|
||||
|
||||
float ownerRadius = rock.Bsp.Nodes[rock.Bsp.RootIndex].BoundingSphere.Radius;
|
||||
var root = rock.Bsp.Nodes[rock.Bsp.RootIndex].BoundingSphere;
|
||||
float ownerRadius = root.Radius;
|
||||
|
||||
// Verbatim from the production predicate.
|
||||
// Verbatim from the deleted predicate.
|
||||
float distToOrigin = (currPos - rock.Position).Length();
|
||||
float maxReach = SphereRadius + ownerRadius + Movement + 2f;
|
||||
|
||||
Assert.True(
|
||||
distToOrigin <= maxReach,
|
||||
$"broadphase rejected a candidate the mover is standing on: "
|
||||
+ $"distToOrigin={distToOrigin:F3} > maxReach={maxReach:F3}. "
|
||||
+ $"Measured to the BSP bounding-sphere CENTRE it is "
|
||||
+ $"{(currPos - (rock.Position + Vector3.Transform(rock.Bsp.Nodes[rock.Bsp.RootIndex].BoundingSphere.Origin, rock.Rotation))).Length():F3} m, "
|
||||
+ $"comfortably inside the same radius.");
|
||||
distToOrigin > maxReach,
|
||||
$"the old filter is supposed to have REJECTED this candidate: "
|
||||
+ $"distToOrigin={distToOrigin:F3} vs maxReach={maxReach:F3}.");
|
||||
|
||||
// What retail's BSP node test measures instead.
|
||||
Vector3 centre = rock.Position
|
||||
+ Vector3.Transform(root.Origin, rock.Rotation);
|
||||
float distToCentre = (currPos - centre).Length();
|
||||
|
||||
Assert.True(
|
||||
distToCentre <= ownerRadius,
|
||||
$"the mover must be inside the root bounding SPHERE it was standing "
|
||||
+ $"on: distToCentre={distToCentre:F3} > radius={ownerRadius:F3}.");
|
||||
}
|
||||
|
||||
private static Placed ResolveOwner(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue