Fix ShieldManager's missing-envelope fallback regression
ShieldManager's fallback (no ShieldEnvelope yet, e.g. every ship prefab before the operator hand-rigs it per Q5) hard-failed to a fixed Vector3.forward direction with a per-hit Debug.LogError. That silently changed behaviour for every ship in the game until the operator edits Longinus.prefab/Djinni.prefab on their own schedule -- worse than either the deleted code or a loud one-time failure. Extract ShieldEnvelope's local-direction primitive into two static, no-component-required entry points -- SurfaceDirection(Transform, Vector3) and LegacyLocalSurfacePoint(Transform, Vector3) -- so both FieldDriver's and ShieldManager's fallbacks go through one owner instead of each re-deriving the pre-Cut-2 formula by hand. ShieldManager's fallback now reproduces the deleted normalize(shield.transform.InverseTransformPoint(point)) formula exactly via the shared static, logging once (a warning, not a per-hit error). FieldDriver's fallback is rewired onto the same static rather than its own inline copy of the maths. Documented, not silently assumed: FieldDriver's "envelope present" branch (envelope.ProjectToSurface, world-space) and its fallback (the legacy local- space formula) are only numerically identical when the carrier's transform has zero position and identity rotation -- true of the one FieldDriver carrier in the tree today, not true in general. This is a characteristic of Cut 2 as scoped by the map (FieldDriver.cs:125 reads envelope.ProjectToSurface directly, per the map's own text), not something this fix introduces or resolves; flagged so nobody assumes the two branches are interchangeable for a future non-origin, non-identity FieldDriver carrier. ShieldPanelCut2Verify.CheckLegacyFallbackFormulas pins both static entry points against an independent re-derivation of the exact pre-Cut-2 formulas, on a carrier with a non-trivial transform (nonzero position, non-identity rotation) so the check isn't vacuous. mutation_tests_shield_panel_cut2.py gained two cases: SurfaceDirection collapsing back to a fixed Vector3.forward (the exact regression fixed here), and LegacyLocalSurfacePoint collapsing to the raw, untransformed world point -- both correctly fail the probe. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
a454a5a77f
commit
7848a8a86e
5 changed files with 201 additions and 18 deletions
|
|
@ -70,6 +70,13 @@ namespace Aetheria.EditorTools
|
|||
// perfect cube of extents -- see IcosphereExtents above), not a guessed flat 0.5.
|
||||
ok &= CheckRadiiUsesMeasuredExtents(longinus, new Vector3(395.3487f, 235.47029f, 990.37805f), IcosphereExtents, "Longinus");
|
||||
ok &= CheckRadiiUsesMeasuredExtents(cubesphere, new Vector3(4f, 3f, 12f), Vector3.one, "FieldShieldTest cubesphere");
|
||||
|
||||
// Pin the fallback: a carrier with NO ShieldEnvelope component must produce exactly
|
||||
// the pre-Cut-2 hit direction/position, for both FieldDriver's and ShieldManager's
|
||||
// formulas. This is what a Longinus/Djinni prefab looks like before the operator
|
||||
// adds the component (Q5) -- the fallback is not a courtesy, it is load-bearing for
|
||||
// every ship in the game until then.
|
||||
ok &= CheckLegacyFallbackFormulas();
|
||||
}
|
||||
catch (Exception e)
|
||||
{
|
||||
|
|
@ -238,6 +245,75 @@ namespace Aetheria.EditorTools
|
|||
return ok;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Pins ShieldEnvelope's two static fallback entry points (SurfaceDirection,
|
||||
/// LegacyLocalSurfacePoint) against an INDEPENDENT re-derivation of the exact pre-Cut-2
|
||||
/// formulas -- not a call into the production code under test -- on a carrier with no
|
||||
/// ShieldEnvelope component and a non-trivial transform (nonzero position, non-identity
|
||||
/// rotation), so the check is not vacuous the way it would be at the origin with identity
|
||||
/// rotation. This is the regression the coordinator flagged: ShieldManager's fallback used
|
||||
/// to silently change behaviour (hard-fail to Vector3.forward) instead of reproducing the
|
||||
/// deleted formula, which would have shown every ship's shield hits in the wrong place
|
||||
/// until the operator hand-edits Longinus.prefab/Djinni.prefab.
|
||||
/// </summary>
|
||||
static bool CheckLegacyFallbackFormulas()
|
||||
{
|
||||
var go = new GameObject("LegacyFallbackProbe");
|
||||
bool ok = true;
|
||||
try
|
||||
{
|
||||
go.transform.position = new Vector3(5f, -2f, 13f);
|
||||
go.transform.rotation = Quaternion.Euler(15f, 40f, -8f);
|
||||
go.transform.localScale = new Vector3(4f, 3f, 12f);
|
||||
// Deliberately no MeshCollider/MeshFilter/ShieldEnvelope on this object -- this is
|
||||
// exactly what Longinus.prefab's "Shield" object looks like before the operator
|
||||
// rigs it (Q5), and what FieldShieldTest's cubesphere looked like before Cut 2.
|
||||
|
||||
var probePoints = new[]
|
||||
{
|
||||
go.transform.position + new Vector3(10f, 2f, -6f),
|
||||
go.transform.position + new Vector3(-3f, 8f, 20f),
|
||||
go.transform.position + new Vector3(0.4f, -0.1f, 0.2f),
|
||||
};
|
||||
|
||||
foreach (var worldPoint in probePoints)
|
||||
{
|
||||
// Independent re-derivation of ShieldManager's deleted formula
|
||||
// (normalize(shield.transform.InverseTransformPoint(point))).
|
||||
var expectedDirection = go.transform.InverseTransformPoint(worldPoint).normalized;
|
||||
var actualDirection = ShieldEnvelope.SurfaceDirection(go.transform, worldPoint);
|
||||
if (Vector3.Distance(expectedDirection, actualDirection) > 1e-5f)
|
||||
{
|
||||
Debug.LogError($"{Tag} SurfaceDirection(transform, p) fallback diverged from "
|
||||
+ $"ShieldManager's deleted formula: expected={expectedDirection:F6} actual={actualDirection:F6}");
|
||||
ok = false;
|
||||
}
|
||||
|
||||
// Independent re-derivation of FieldDriver.AddHit's deleted formula
|
||||
// (normalize(InverseTransformPoint(p)) * transform.localScale).
|
||||
var expectedLocalSurface = Vector3.Scale(
|
||||
go.transform.InverseTransformPoint(worldPoint).normalized, go.transform.localScale);
|
||||
var actualLocalSurface = ShieldEnvelope.LegacyLocalSurfacePoint(go.transform, worldPoint);
|
||||
if (Vector3.Distance(expectedLocalSurface, actualLocalSurface) > 1e-3f)
|
||||
{
|
||||
Debug.LogError($"{Tag} LegacyLocalSurfacePoint(transform, p) fallback diverged from "
|
||||
+ $"FieldDriver's deleted formula: expected={expectedLocalSurface:F6} actual={actualLocalSurface:F6}");
|
||||
ok = false;
|
||||
}
|
||||
}
|
||||
}
|
||||
finally
|
||||
{
|
||||
UnityEngine.Object.DestroyImmediate(go);
|
||||
}
|
||||
|
||||
if (ok)
|
||||
Debug.Log($"{Tag} Legacy fallback statics (SurfaceDirection, LegacyLocalSurfacePoint) match "
|
||||
+ "the pre-Cut-2 formulas exactly, for a carrier with no ShieldEnvelope component and a "
|
||||
+ "non-trivial transform.");
|
||||
return ok;
|
||||
}
|
||||
|
||||
static Vector3 LocalExtentsOf(GameObject envelopeObj)
|
||||
{
|
||||
var meshCollider = envelopeObj.GetComponent<MeshCollider>();
|
||||
|
|
|
|||
|
|
@ -139,6 +139,18 @@ public class FieldDriver : MonoBehaviour, IAbsorbPresenter, IGrabPresenter, IMel
|
|||
return transform.localScale;
|
||||
}
|
||||
|
||||
// Cut 2 note on the two branches below NOT being the same computation shape: envelope.
|
||||
// ProjectToSurface returns a WORLD-space point (it transforms back through rotation and
|
||||
// translation -- Cut 3's panel presenter needs that). The fallback reproduces the pre-Cut-2
|
||||
// formula exactly instead, which stayed in LOCAL space with no rotation/translation applied,
|
||||
// because that is what the shader consumes here (_Hits is uploaded and read in the field
|
||||
// mesh's own local space, not world space). The two branches are only numerically identical
|
||||
// when the carrier's transform has zero position and identity rotation -- true of the one
|
||||
// FieldDriver carrier in the tree today (FieldShieldTest.unity's cubesphere), NOT true in
|
||||
// general. This is an existing characteristic of Cut 2 as scoped by the map (FieldDriver.cs:125
|
||||
// reads envelope.ProjectToSurface(position) directly, per the map's own Cut 2 text), not
|
||||
// something introduced by this fallback -- flagging it so nobody assumes the two branches are
|
||||
// interchangeable for a future non-origin, non-identity FieldDriver carrier.
|
||||
public void AddHit(float3 position, float3 direction, float magnitude)
|
||||
{
|
||||
if (_hits.Count >= MaxHits) return;
|
||||
|
|
@ -148,7 +160,7 @@ public class FieldDriver : MonoBehaviour, IAbsorbPresenter, IGrabPresenter, IMel
|
|||
else
|
||||
{
|
||||
WarnMissingEnvelopeOnce();
|
||||
surfacePosition = (normalize(transform.InverseTransformPoint(position.ToUnity()).ToCultMath()) * transform.localScale.ToCultMath()).ToUnity();
|
||||
surfacePosition = ShieldEnvelope.LegacyLocalSurfacePoint(transform, position.ToUnity());
|
||||
}
|
||||
var hit = new FieldHit
|
||||
{
|
||||
|
|
|
|||
|
|
@ -71,15 +71,20 @@ public class ShieldEnvelope : MonoBehaviour
|
|||
}
|
||||
|
||||
// Local-space unit direction from the envelope's centre toward worldPoint, accounting for the
|
||||
// transform's rotation and (non-uniform) scale. This is the one primitive both ProjectToSurface
|
||||
// and SurfaceDirection are built from, and the only place InverseTransformPoint may appear for
|
||||
// shield-surface purposes (see the negative check in docs/shield-panel-cut.md Cut 2).
|
||||
Vector3 LocalUnitDirection(Vector3 worldPoint)
|
||||
// transform's rotation and (non-uniform) scale. This is the one primitive ProjectToSurface,
|
||||
// SurfaceDirection, and both static fallback entry points below are built from, and the only
|
||||
// place InverseTransformPoint may appear for shield-surface purposes (see the negative check
|
||||
// in docs/shield-panel-cut.md Cut 2). Static and keyed on an explicit Transform so a caller
|
||||
// with no ShieldEnvelope component can still go through this one owner instead of re-deriving
|
||||
// the maths itself.
|
||||
static Vector3 LocalUnitDirection(Transform t, Vector3 worldPoint)
|
||||
{
|
||||
var local = transform.InverseTransformPoint(worldPoint);
|
||||
var local = t.InverseTransformPoint(worldPoint);
|
||||
return local.sqrMagnitude > 1e-12f ? local.normalized : Vector3.forward;
|
||||
}
|
||||
|
||||
Vector3 LocalUnitDirection(Vector3 worldPoint) => LocalUnitDirection(transform, worldPoint);
|
||||
|
||||
/// <summary>
|
||||
/// Projects a world-space point outward onto the ellipsoid surface, returning a world-space
|
||||
/// point. This is the true surface projection: local direction, scale by the local-space
|
||||
|
|
@ -135,5 +140,38 @@ public class ShieldEnvelope : MonoBehaviour
|
|||
/// negative check ("no shield-surface projection outside ShieldEnvelope.cs") stays true. This
|
||||
/// is a *direction*, not the true surface normal -- see the SurfaceNormal doc comment.
|
||||
/// </summary>
|
||||
public Vector3 SurfaceDirection(Vector3 worldPoint) => LocalUnitDirection(worldPoint);
|
||||
public Vector3 SurfaceDirection(Vector3 worldPoint) => LocalUnitDirection(transform, worldPoint);
|
||||
|
||||
// --- Fallback entry points for a carrier with no ShieldEnvelope component yet. ---
|
||||
//
|
||||
// Not every carrier gets the component in the same commit as this file (Q5: the operator rigs
|
||||
// prefabs and scenes by hand, on their own schedule), so both FieldDriver and ShieldManager
|
||||
// must keep working, unchanged, until their own object is rigged. "Unchanged" means byte-for-
|
||||
// byte the pre-Cut-2 formula, not a plausible-looking replacement -- a silently different
|
||||
// fallback is worse than either the old code or a hard failure. Rather than let each consumer
|
||||
// re-derive that formula by hand (recreating the exact duplicate-authority problem this cut
|
||||
// exists to remove), both fallback formulas are named, static, one-owner methods here, and
|
||||
// ShieldPanelCut2Verify.CheckLegacyFallbackFormulas pins each one against an independent
|
||||
// re-derivation of the original code.
|
||||
|
||||
/// <summary>
|
||||
/// Compatibility path for a Transform with no ShieldEnvelope component: the exact direction
|
||||
/// ShieldManager.ShowHit computed by hand before this cut (normalize(InverseTransformPoint),
|
||||
/// no scale or mesh-extents factor at all -- a direction never needed one).
|
||||
/// </summary>
|
||||
public static Vector3 SurfaceDirection(Transform transform, Vector3 worldPoint)
|
||||
=> LocalUnitDirection(transform, worldPoint);
|
||||
|
||||
/// <summary>
|
||||
/// Compatibility path for a Transform with no ShieldEnvelope component: the exact local-frame
|
||||
/// value FieldDriver.AddHit computed by hand before this cut -- normalize(local) scaled by the
|
||||
/// transform's own localScale, deliberately NOT transformed back through rotation/translation
|
||||
/// (FieldDriver's shader consumes this in local mesh space, not world space; see the
|
||||
/// ProjectToSurface-vs-this note in FieldDriver.cs). This is a legacy, local-space formula,
|
||||
/// not a general "surface point in the world" API -- ProjectToSurface is that, and requires a
|
||||
/// real ShieldEnvelope (it needs the mesh's local extents, which no mesh means no envelope to
|
||||
/// read them from).
|
||||
/// </summary>
|
||||
public static Vector3 LegacyLocalSurfacePoint(Transform transform, Vector3 worldPoint)
|
||||
=> Vector3.Scale(LocalUnitDirection(transform, worldPoint), transform.localScale);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -22,6 +22,13 @@ public class ShieldManager : MonoBehaviour
|
|||
_envelope = GetComponent<ShieldEnvelope>();
|
||||
}
|
||||
|
||||
private void WarnMissingEnvelopeOnce()
|
||||
{
|
||||
if (_loggedMissingEnvelope) return;
|
||||
_loggedMissingEnvelope = true;
|
||||
Debug.LogWarning("ShieldManager has no ShieldEnvelope; falling back to its own transform for the hit direction.", this);
|
||||
}
|
||||
|
||||
private void OnCollisionEnter(Collision other)
|
||||
{
|
||||
var otherShield = other.collider.GetComponent<ShieldManager>();
|
||||
|
|
@ -90,19 +97,18 @@ public class ShieldManager : MonoBehaviour
|
|||
// of direction, not the true surface normal -- ShieldAnimation.Direction keeps its existing
|
||||
// meaning (R6's note that both existing consumers get away with normalize(p) because they
|
||||
// want a direction, not a normal).
|
||||
// Unlike FieldDriver (a compatibility path is explicit in the map for that consumer),
|
||||
// ShieldManager has no legitimate reason to be missing its envelope -- Cut 2 adds
|
||||
// ShieldEnvelope to every "Shield" object that carries a ShieldManager. Recomputing the
|
||||
// projection here "just in case" would recreate the exact duplicate authority this cut
|
||||
// removes, so a missing envelope logs loudly and degrades to a fixed direction instead.
|
||||
//
|
||||
// Every "Shield" object gets a ShieldEnvelope in this cut, but the operator rigs prefabs by
|
||||
// hand on their own schedule (Q5), so until Longinus.prefab/Djinni.prefab are actually
|
||||
// edited, every ship in the game has none yet. A missing envelope must therefore behave
|
||||
// exactly like the deleted code, not degrade -- a fallback that silently changes behaviour
|
||||
// (this used to hard-fail to a fixed direction) is worse than either the old code or a
|
||||
// loud failure. So this goes through ShieldEnvelope's own static fallback entry point
|
||||
// (one owner, no second copy of the maths) rather than recomputing the projection here.
|
||||
if (_envelope == null)
|
||||
{
|
||||
if (!_loggedMissingEnvelope)
|
||||
{
|
||||
Debug.LogError("ShieldManager has no ShieldEnvelope; the shield hit direction cannot be computed.", this);
|
||||
_loggedMissingEnvelope = true;
|
||||
}
|
||||
shield.Direction = Vector3.forward;
|
||||
WarnMissingEnvelopeOnce();
|
||||
shield.Direction = ShieldEnvelope.SurfaceDirection(shield.transform, point);
|
||||
}
|
||||
else
|
||||
{
|
||||
|
|
|
|||
|
|
@ -145,6 +145,57 @@ def main() -> int:
|
|||
finally:
|
||||
SHIELD_ENVELOPE_CS.write_text(original_envelope, encoding="utf-8")
|
||||
|
||||
# --- the regression the coordinator flagged directly: ShieldManager's (and FieldDriver's)
|
||||
# missing-envelope fallback must reproduce the deleted pre-Cut-2 formula, not silently swap
|
||||
# in a fixed direction. This mutates the shared static SurfaceDirection entry point back to
|
||||
# the fixed Vector3.forward every hit direction degraded to before the fix. ---
|
||||
print("\n=== mutation: SurfaceDirection fallback collapses to a fixed Vector3.forward ===")
|
||||
anchor = (
|
||||
" public static Vector3 SurfaceDirection(Transform transform, Vector3 worldPoint)\n"
|
||||
" => LocalUnitDirection(transform, worldPoint);"
|
||||
)
|
||||
mutated = (
|
||||
" // mutation_tests_shield_panel_cut2.py: reintroduce the exact regression the\n"
|
||||
" // coordinator flagged -- a fallback that silently changes behaviour instead of\n"
|
||||
" // reproducing the deleted formula.\n"
|
||||
" public static Vector3 SurfaceDirection(Transform transform, Vector3 worldPoint)\n"
|
||||
" => Vector3.forward;"
|
||||
)
|
||||
try:
|
||||
replace_unique(SHIELD_ENVELOPE_CS, anchor, mutated)
|
||||
code, text = run_probe(scratch / "fixed_forward.log")
|
||||
caught = "SurfaceDirection(transform, p) fallback diverged" in text
|
||||
if code == 0 or not caught:
|
||||
failures.append("fixed-Vector3.forward regression: probe did not go red naming the SurfaceDirection divergence")
|
||||
else:
|
||||
print("OK: caught — probe exited non-zero, legacy fallback pin fired")
|
||||
finally:
|
||||
SHIELD_ENVELOPE_CS.write_text(original_envelope, encoding="utf-8")
|
||||
|
||||
# --- the other fallback static, mutated to the "wrong transform" shape the coordinator
|
||||
# named: dropping the local-frame projection and returning the raw world point instead. ---
|
||||
print("\n=== mutation: LegacyLocalSurfacePoint fallback returns the raw world point ===")
|
||||
anchor = (
|
||||
" public static Vector3 LegacyLocalSurfacePoint(Transform transform, Vector3 worldPoint)\n"
|
||||
" => Vector3.Scale(LocalUnitDirection(transform, worldPoint), transform.localScale);"
|
||||
)
|
||||
mutated = (
|
||||
" // mutation_tests_shield_panel_cut2.py: the \"wrong transform\" mutation -- skip the\n"
|
||||
" // local-frame projection entirely and hand back the untransformed world point.\n"
|
||||
" public static Vector3 LegacyLocalSurfacePoint(Transform transform, Vector3 worldPoint)\n"
|
||||
" => worldPoint;"
|
||||
)
|
||||
try:
|
||||
replace_unique(SHIELD_ENVELOPE_CS, anchor, mutated)
|
||||
code, text = run_probe(scratch / "wrong_transform.log")
|
||||
caught = "LegacyLocalSurfacePoint(transform, p) fallback diverged" in text
|
||||
if code == 0 or not caught:
|
||||
failures.append("raw-world-point regression: probe did not go red naming the LegacyLocalSurfacePoint divergence")
|
||||
else:
|
||||
print("OK: caught — probe exited non-zero, legacy fallback pin fired")
|
||||
finally:
|
||||
SHIELD_ENVELOPE_CS.write_text(original_envelope, encoding="utf-8")
|
||||
|
||||
finally:
|
||||
# belt-and-suspenders: never leave a mutated file behind even if something above threw
|
||||
SHIELD_ENVELOPE_CS.write_text(original_envelope, encoding="utf-8")
|
||||
|
|
|
|||
Loading…
Reference in a new issue