From 95d6fd3b14fac6c875bd364dfa0f516dbf7963f3 Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Sat, 19 Sep 2026 05:32:29 +0200 Subject: [PATCH] F4/F5: shield restore survives inactivity, and CanTakeHit stops mutating F4: a hit that breaks the shield usually also cooks it (TakeHit's own AddHeat), which can knock the item thermally offline in the same tick. The reserve's recharge and its Broken-clearing check lived inside Execute, which Entity.Update only runs while Item.Active is true -- so a shield broken by a hit that also shuts it down could never come back, no matter how long RestoreDuration allows for. That logic now lives in Shield.Update (IAlwaysUpdatedBehavior), which Entity.Update calls every tick unconditionally (Cooldown.cs already relies on the same interface for the same reason). F5: CanTakeHit broke the shield and drained the reserve as a side effect of the query itself, on the premise that every caller queries it exactly once per hit. That premise was false: RaycastAll/SphereCastAll/OverlapSphere hand every caller under Assets/Scripts/Gameplay/Weapons both the shield collider and the hull collider for one shot, so the same shield's CanTakeHit was queried twice per hit. CanTakeHit is now a pure read (Broken + the reserve's own CanSpend); the mutation moved to a new Shield.Break(), idempotent, called by each weapon file exactly where it already decided to route a hit past the shield instead of absorbing it. Pinned in tests/Aetheria.Shared.Tests/ShieldReserveTests.cs: RestorationProceedsWhileTheItemIsInactive (F4), CanTakeHitNeverMutatesRegardlessOfHowManyTimesItIsCalled and BreakIsIdempotentAcrossRepeatedCalls (F5). Updated every existing test that relied on CanTakeHit's old mutating side effect to call Break() explicitly, matching the new caller pattern. Cannot compile-verify Assets/Scripts/Gameplay/Weapons/*.cs here (UnityEngine, Unity batchmode unavailable in this environment) -- reviewed by hand for brace/control-flow correctness; Self or a Unity batchmode pass should confirm. Co-Authored-By: Claude Opus 5 --- .../Scripts/Gameplay/Weapons/ConstantLaser.cs | 13 ++- .../Gameplay/Weapons/GuidedProjectile.cs | 13 ++- Assets/Scripts/Gameplay/Weapons/Laser.cs | 13 ++- Assets/Scripts/Gameplay/Weapons/Lightning.cs | 24 +++- Assets/Scripts/Gameplay/Weapons/Mine.cs | 18 ++- Assets/Scripts/Gameplay/Weapons/Projectile.cs | 14 ++- .../Scripts/ServerShared/Behaviors/Shield.cs | 68 +++++++---- .../ShieldReserveTests.cs | 106 ++++++++++++++++-- 8 files changed, 221 insertions(+), 48 deletions(-) diff --git a/Assets/Scripts/Gameplay/Weapons/ConstantLaser.cs b/Assets/Scripts/Gameplay/Weapons/ConstantLaser.cs index aded241fb..002b4a367 100644 --- a/Assets/Scripts/Gameplay/Weapons/ConstantLaser.cs +++ b/Assets/Scripts/Gameplay/Weapons/ConstantLaser.cs @@ -76,10 +76,19 @@ public class ConstantLaser : MonoBehaviour var shield = hit.collider.GetComponent(); if (shield) { - if (!(shield.Entity.Shield != null && shield.Entity.Shield.Item.Active.Value && shield.Entity.Shield.CanTakeHit(DamageType, Damage))) continue; + var shieldBehavior = shield.Entity.Shield; + var shieldActive = shieldBehavior != null && shieldBehavior.Item.Active.Value; + var shieldAbsorbs = shieldActive && shieldBehavior.CanTakeHit(DamageType, Damage); + // F5 (docs/stats-and-power-cut.md, Soul pass 2026-09-19): CanTakeHit is a pure query now -- read + // once into shieldAbsorbs, and break the shield exactly once, right here, at the point this hit + // is actually decided to route past it (to the hull collider RaycastAll returns further down + // this same loop) instead of being absorbed. The hull branch below re-queries CanTakeHit safely + // -- it is side-effect-free -- and will see Broken already true. + if (shieldActive && !shieldAbsorbs) shieldBehavior.Break(); + if (!shieldAbsorbs) continue; if (shield.Entity != SourceEntity) { - shield.Entity.Shield.TakeHit(DamageType, Damage); + shieldBehavior.TakeHit(DamageType, Damage); shield.ShowHit(hit.point, sqrt(Damage)); LineRenderer.SetPosition(1, hit.point); hitFound = true; diff --git a/Assets/Scripts/Gameplay/Weapons/GuidedProjectile.cs b/Assets/Scripts/Gameplay/Weapons/GuidedProjectile.cs index 16005ca3d..079644a8d 100644 --- a/Assets/Scripts/Gameplay/Weapons/GuidedProjectile.cs +++ b/Assets/Scripts/Gameplay/Weapons/GuidedProjectile.cs @@ -155,10 +155,19 @@ public class GuidedProjectile : MonoBehaviour var shield = hit.collider.GetComponent(); if (shield) { - if (!(shield.Entity.Shield != null && shield.Entity.Shield.Item.Active.Value && shield.Entity.Shield.CanTakeHit(DamageType, Damage))) continue; + var shieldBehavior = shield.Entity.Shield; + var shieldActive = shieldBehavior != null && shieldBehavior.Item.Active.Value; + var shieldAbsorbs = shieldActive && shieldBehavior.CanTakeHit(DamageType, Damage); + // F5 (docs/stats-and-power-cut.md, Soul pass 2026-09-19): CanTakeHit is a pure query now -- + // read once into shieldAbsorbs, and break the shield exactly once, right here, at the point + // this hit is actually decided to route past it (to the hull collider RaycastAll returns + // further down this same loop) instead of being absorbed. The hull branch below re-queries + // CanTakeHit safely -- it is side-effect-free -- and will see Broken already true. + if (shieldActive && !shieldAbsorbs) shieldBehavior.Break(); + if (!shieldAbsorbs) continue; if (shield.Entity != SourceEntity) { - shield.Entity.Shield.TakeHit(DamageType, Damage); + shieldBehavior.TakeHit(DamageType, Damage); shield.ShowHit(hit.point, sqrt(Damage)); } } diff --git a/Assets/Scripts/Gameplay/Weapons/Laser.cs b/Assets/Scripts/Gameplay/Weapons/Laser.cs index a111de91d..0c010c8a2 100644 --- a/Assets/Scripts/Gameplay/Weapons/Laser.cs +++ b/Assets/Scripts/Gameplay/Weapons/Laser.cs @@ -42,10 +42,19 @@ public class Laser : MonoBehaviour var shield = hit.collider.GetComponent(); if (shield) { - if (!(shield.Entity.Shield != null && shield.Entity.Shield.Item.Active.Value && shield.Entity.Shield.CanTakeHit(DamageType, Damage))) continue; + var shieldBehavior = shield.Entity.Shield; + var shieldActive = shieldBehavior != null && shieldBehavior.Item.Active.Value; + var shieldAbsorbs = shieldActive && shieldBehavior.CanTakeHit(DamageType, Damage); + // F5 (docs/stats-and-power-cut.md, Soul pass 2026-09-19): CanTakeHit is a pure query now -- read + // once into shieldAbsorbs, and break the shield exactly once, right here, at the point this hit + // is actually decided to route past it (to the hull collider RaycastAll returns further down + // this same loop) instead of being absorbed. The hull branch below re-queries CanTakeHit safely + // -- it is side-effect-free -- and will see Broken already true. + if (shieldActive && !shieldAbsorbs) shieldBehavior.Break(); + if (!shieldAbsorbs) continue; if (shield.Entity != SourceEntity) { - shield.Entity.Shield.TakeHit(DamageType, Damage); + shieldBehavior.TakeHit(DamageType, Damage); shield.ShowHit(hit.point, sqrt(Damage)); LineRenderer.SetPosition(1, hit.point); hitFound = true; diff --git a/Assets/Scripts/Gameplay/Weapons/Lightning.cs b/Assets/Scripts/Gameplay/Weapons/Lightning.cs index da2a4cb7e..641d44212 100644 --- a/Assets/Scripts/Gameplay/Weapons/Lightning.cs +++ b/Assets/Scripts/Gameplay/Weapons/Lightning.cs @@ -35,14 +35,26 @@ public class Lightning : MonoBehaviour foreach (var hit in hits) { var shield = hit.collider.GetComponent(); - if (shield && (shield.Entity.Shield != null && shield.Entity.Shield.Item.Active.Value && shield.Entity.Shield.CanTakeHit(DamageType, Damage))) + if (shield) { - if (shield.Entity == Source.Entity) continue; - LightningCompute.OnLeaderComplete = () => + var shieldBehavior = shield.Entity.Shield; + var shieldActive = shieldBehavior != null && shieldBehavior.Item.Active.Value; + var shieldAbsorbs = shieldActive && shieldBehavior.CanTakeHit(DamageType, Damage); + // F5 (docs/stats-and-power-cut.md, Soul pass 2026-09-19): CanTakeHit is a pure query now -- read + // once into shieldAbsorbs, and break the shield exactly once, right here, at the point this hit + // is actually decided to route past it (to the hull collider SphereCastAll returns as a + // separate entry further down this same loop) instead of being absorbed. The hull branch below + // re-queries CanTakeHit safely -- it is side-effect-free -- and will see Broken already true. + if (shieldActive && !shieldAbsorbs) shieldBehavior.Break(); + if (shieldAbsorbs) { - shield.Entity.Shield.TakeHit(DamageType, Damage); - shield.ShowHit(hit.point, sqrt(Damage)); - }; + if (shield.Entity == Source.Entity) continue; + LightningCompute.OnLeaderComplete = () => + { + shieldBehavior.TakeHit(DamageType, Damage); + shield.ShowHit(hit.point, sqrt(Damage)); + }; + } } var hull = hit.collider.GetComponent(); if (hull && !(hull.Entity.Shield != null && hull.Entity.Shield.Item.Active.Value && hull.Entity.Shield.CanTakeHit(DamageType, Damage))) diff --git a/Assets/Scripts/Gameplay/Weapons/Mine.cs b/Assets/Scripts/Gameplay/Weapons/Mine.cs index 7c0699d75..3807270d3 100644 --- a/Assets/Scripts/Gameplay/Weapons/Mine.cs +++ b/Assets/Scripts/Gameplay/Weapons/Mine.cs @@ -92,10 +92,22 @@ public class Mine : MonoBehaviour foreach (var collider in Physics.OverlapSphere(position, BlastRange, 1 | (1 << 17))) { var shield = collider.GetComponent(); - if (shield && (shield.Entity.Shield != null && shield.Entity.Shield.Item.Active.Value && shield.Entity.Shield.CanTakeHit(DamageType, Damage))) + if (shield) { - shield.Entity.Shield.TakeHit(DamageType, Damage); - shield.ShowHit(position, sqrt(Damage)); + var shieldBehavior = shield.Entity.Shield; + var shieldActive = shieldBehavior != null && shieldBehavior.Item.Active.Value; + var shieldAbsorbs = shieldActive && shieldBehavior.CanTakeHit(DamageType, Damage); + // F5 (docs/stats-and-power-cut.md, Soul pass 2026-09-19): CanTakeHit is a pure query now -- read + // once into shieldAbsorbs, and break the shield exactly once, right here, at the point this hit + // is actually decided to route past it (to the hull collider OverlapSphere returns as a + // separate entry in this same loop) instead of being absorbed. The hull branch below re-queries + // CanTakeHit safely -- it is side-effect-free -- and will see Broken already true. + if (shieldActive && !shieldAbsorbs) shieldBehavior.Break(); + if (shieldAbsorbs) + { + shieldBehavior.TakeHit(DamageType, Damage); + shield.ShowHit(position, sqrt(Damage)); + } } var hull = collider.GetComponent(); if (hull && !(hull.Entity.Shield != null && hull.Entity.Shield.Item.Active.Value && hull.Entity.Shield.CanTakeHit(DamageType, Damage))) diff --git a/Assets/Scripts/Gameplay/Weapons/Projectile.cs b/Assets/Scripts/Gameplay/Weapons/Projectile.cs index f93e2eb82..8e6972c4d 100644 --- a/Assets/Scripts/Gameplay/Weapons/Projectile.cs +++ b/Assets/Scripts/Gameplay/Weapons/Projectile.cs @@ -61,10 +61,20 @@ public class Projectile : MonoBehaviour var hull = hit.collider.GetComponent(); if (shield) { - if (!(shield.Entity.Shield != null && shield.Entity.Shield.Item.Active.Value && shield.Entity.Shield.CanTakeHit(DamageType, Damage))) continue; + var shieldBehavior = shield.Entity.Shield; + var shieldActive = shieldBehavior != null && shieldBehavior.Item.Active.Value; + var shieldAbsorbs = shieldActive && shieldBehavior.CanTakeHit(DamageType, Damage); + // F5 (docs/stats-and-power-cut.md, Soul pass 2026-09-19): CanTakeHit is a pure query now -- + // read once into shieldAbsorbs, and break the shield exactly once, right here, at the point + // this hit is actually decided to route past it (to the hull collider RaycastAll returns + // as a separate entry further down this same loop) instead of being absorbed. The hull + // branch below re-queries CanTakeHit safely -- it is side-effect-free -- and will see Broken + // already true. + if (shieldActive && !shieldAbsorbs) shieldBehavior.Break(); + if (!shieldAbsorbs) continue; if (shield.Entity != SourceEntity) { - shield.Entity.Shield.TakeHit(DamageType, Damage*DirectHitDamageMultiplier); + shieldBehavior.TakeHit(DamageType, Damage*DirectHitDamageMultiplier); shield.ShowHit(hit.point, sqrt(Damage * DirectHitDamageMultiplier)); } } diff --git a/Assets/Scripts/ServerShared/Behaviors/Shield.cs b/Assets/Scripts/ServerShared/Behaviors/Shield.cs index 0818f0377..1dfe421f4 100644 --- a/Assets/Scripts/ServerShared/Behaviors/Shield.cs +++ b/Assets/Scripts/ServerShared/Behaviors/Shield.cs @@ -41,7 +41,7 @@ public class ShieldData : BehaviorData } } -public class Shield : Behavior, IProgressBehavior, IPowerConsumer +public class Shield : Behavior, IProgressBehavior, IPowerConsumer, IAlwaysUpdatedBehavior { public float Efficiency { get; private set; } public float EnergyUsage { get; private set; } @@ -56,9 +56,11 @@ public class Shield : Behavior, IProgressBehavior, IPowerConsumer // // Operator's second ruling, same date, on the fork the first ruling left open: the breaking hit passes // through in full -- no partial absorption. The reserve does not spend the portion it could have covered; - // TakeHit is never called for this hit at all (CanTakeHit's false branch below is the caller's sole signal - // to route the whole hit to the hull instead -- see every caller under Assets/Scripts/Gameplay/Weapons). - // The break is a distinct event the player feels, not a discount on an overkill hit. + // TakeHit is never called for this hit at all -- CanTakeHit's false return is the caller's signal to route + // the whole hit to the hull instead, and Break() below (F5, Soul pass 2026-09-19) is what the caller invokes + // at that same point to actually break the shield, since CanTakeHit itself is now a pure query (see every + // caller under Assets/Scripts/Gameplay/Weapons). The break is a distinct event the player feels, not a + // discount on an overkill hit. // // A second question the operator asked me to settle and state: what happens to whatever charge the reserve // held at the moment it broke. Chosen: emptied by the break, not left at its pre-hit charge. A reserve that @@ -116,36 +118,62 @@ public class Shield : Behavior, IProgressBehavior, IPowerConsumer Capacity = Evaluate(_data.Capacity); RefillDuration = Evaluate(_data.RefillDuration); RestoreDuration = Evaluate(_data.RestoreDuration); - // This tick's grant, read back via Item.PowerSupply -- Item is non-null here because a null-Item - // instance never reaches PowerBus.Step (see CanTakeHit/TakeHit) and so never accrues charge. - if (Item != null) - { - _reserve.AddCharge(_reserve.RequestedFill(dt) * Item.PowerSupply); - // Restored to full on the restore duration: back up. - if (Broken && _reserve.Charge >= _reserve.Capacity - .01f) Broken = false; - } return true; } + // F4 (docs/stats-and-power-cut.md, Soul pass 2026-09-19): the reserve's own recharge, and clearing Broken + // once it is full, used to live in Execute above -- which Entity.Update only calls while Item.Active is + // true. A hit that breaks the shield usually also cooks it (AddHeat in TakeHit below), which can knock the + // item thermally offline in the same moment it breaks -- Execute then stops running for as long as the item + // stays offline, so the shield could never come back no matter how long RestoreDuration allows for. Restoring + // must not depend on whether the item is currently Active, so it moved to IAlwaysUpdatedBehavior.Update, + // which Entity.Update calls every tick unconditionally (Cooldown.cs already relies on the same interface for + // exactly this reason -- see Entity.cs's own alwaysUpdatedBehavior loop, outside the `if (Active.Value)` + // block Execute lives in). PowerBus (F2) only bills and refreshes Item.PowerSupply while the item is Active, + // so while offline this keeps recharging at the last grant it actually received rather than freezing dead or + // fabricating a fresh one -- the punish window still runs down on its own clock instead of being held + // hostage by an unrelated shutdown. + public void Update(float dt) + { + // Item is non-null-checked the same way Execute's old version was: a null-Item (consumable-hosted) + // instance never reaches PowerBus.Step and so never accrues charge -- see CanTakeHit's own comment. + if (Item == null) return; + RefreshReserve(); + _reserve.AddCharge(_reserve.RequestedFill(dt) * Item.PowerSupply); + // Restored to full on the restore duration: back up. + if (Broken && _reserve.Charge >= _reserve.Capacity - .01f) Broken = false; + } + // Cut 4 (docs/stats-and-power-cut.md, Cut 4): a hit taken, not a chosen activation, drawing from the // shield's own continuous reserve instead of the entity's shared bus capacitors -- closing Cut 3's named, // temporary exception. A consumable-hosted instance (Item == null) has no PowerBus entry, so nothing would // ever fill this reserve; bypass it rather than starving such an instance forever. // - // Operator ruling, 2026-09-19: a broken shield absorbs nothing (Broken short-circuits to false). A hit the - // reserve cannot fully cover breaks the shield as a side effect of this same check -- see the Broken field - // comment above for why that mutation lives here rather than in TakeHit. Every caller queries CanTakeHit - // exactly once per hit and only calls TakeHit when it returns true (Assets/Scripts/Gameplay/Weapons/*), so - // this is not a repeated-query hazard in practice. Draining the reserve on break (rather than leaving its - // pre-hit charge, see the Broken field comment) happens here too, in the same atomic decision. + // F5 (docs/stats-and-power-cut.md, Soul pass 2026-09-19): this used to mutate -- breaking the shield and + // draining the reserve -- as a side effect of the query itself, on the premise that "every caller queries + // CanTakeHit exactly once per hit." That premise was false: RaycastAll returns both the shield collider and + // the hull collider for one shot, and every caller under Assets/Scripts/Gameplay/Weapons queries this twice + // per hit (once deciding whether the shield collider absorbs it, once deciding whether the hull collider is + // exposed). A caller must be able to ask without consequence, so this is now a pure read: Broken and the + // reserve's own CanSpend, nothing more. See Break() below for where the mutation moved. public bool CanTakeHit(DamageType type, float damage) { if (Item == null) return true; if (Broken) return false; - if (_reserve.CanSpend(damage * EnergyUsage)) return true; + return _reserve.CanSpend(damage * EnergyUsage); + } + + // F5: the mutation CanTakeHit used to perform when it returned false, now a caller invokes explicitly at the + // one point damage is actually applied -- specifically, the point a caller decides to route a hit past this + // shield (to the hull) instead of absorbing it, which is also the only point that decision is made at all, + // so this is naturally called at most once per hit despite the double CanTakeHit query above. Idempotent + // (a no-op once Broken is already true) as a defense-in-depth against a caller structure that cannot + // guarantee single-call discipline, not as licence to call it more than once on purpose. + public void Break() + { + if (Item == null || Broken) return; Broken = true; _reserve.AddCharge(-_reserve.Charge); // emptied by the break, not left at its pre-hit charge - return false; } public void TakeHit(DamageType type, float damage) diff --git a/tests/Aetheria.Shared.Tests/ShieldReserveTests.cs b/tests/Aetheria.Shared.Tests/ShieldReserveTests.cs index b05fa55eb..9c3afb11b 100644 --- a/tests/Aetheria.Shared.Tests/ShieldReserveTests.cs +++ b/tests/Aetheria.Shared.Tests/ShieldReserveTests.cs @@ -15,6 +15,12 @@ using Xunit; // broke. This file pins all of that; InputCapacitorTests.cs keeps pinning the surviving Cut 4 rules (a hit // within the reserve may draw a partial amount; TrySpend/AddCharge atomicity) against the same class. Each rule // here has a matching mutation in tests/mutation_tests_shield_reserve.py. +// +// F5 (docs/stats-and-power-cut.md, Soul pass 2026-09-19): CanTakeHit no longer breaks the shield as a side +// effect of the query -- it did, on the false premise that every caller queries it exactly once per hit, but +// RaycastAll hands every real caller both the shield and the hull collider for one shot, so it was queried +// twice. Every test below that means to simulate a breaking hit now calls Shield.Break() explicitly, the same +// way Assets/Scripts/Gameplay/Weapons/* now does at the point it decides to route a hit past the shield. public sealed class ShieldReserveTests : IDisposable { private readonly string _root = Path.Combine(Path.GetTempPath(), "aetheria-shieldreserve-" + Guid.NewGuid().ToString("N")); @@ -113,8 +119,9 @@ public sealed class ShieldReserveTests : IDisposable // Proves the hit actually drew the reserve down (not a free absorb that left Charge at 5): a second hit // that would have fit inside the original 5 (4, with clear margin either side of the exact 2 left, to // stay clear of float rounding at the boundary) no longer fits inside what remains. Per the ruling that - // any refusal is itself a break, this also puts the shield into Broken. + // any refusal is itself a break, Break() (the caller's own next step, F5) puts the shield into Broken. Assert.False(shield.CanTakeHit(DamageType.Kinetic, 4f)); + shield.Break(); Assert.True(shield.Broken); } @@ -128,6 +135,7 @@ public sealed class ShieldReserveTests : IDisposable Assert.False(shield.Broken); Assert.False(shield.CanTakeHit(DamageType.Kinetic, 6f)); // exceeds the 5 held + shield.Break(); Assert.True(shield.Broken); } @@ -145,7 +153,8 @@ public sealed class ShieldReserveTests : IDisposable using var cache = OpenCatalog(); var (ship, shield) = BuildShipWithShield(cache, reactorGeneration: 20); ship.Update(1f); // Charge = 5 - Assert.False(shield.CanTakeHit(DamageType.Kinetic, 6f)); // breaks; TakeHit must not run for this hit + Assert.False(shield.CanTakeHit(DamageType.Kinetic, 6f)); // would break; TakeHit must not run for this hit + shield.Break(); ship.Update(3f); // from zero: 6 of 10 -- not yet full. From a surviving 5 (TakeHit ran, refused, kept it // untouched): 5 + 6 clamped to 10 -- already full. @@ -163,7 +172,8 @@ public sealed class ShieldReserveTests : IDisposable using var cache = OpenCatalog(); var (ship, shield) = BuildShipWithShield(cache, reactorGeneration: 20); ship.Update(1f); // Charge = 5 - Assert.False(shield.CanTakeHit(DamageType.Kinetic, 6f)); // breaks, reserve now 0 + Assert.False(shield.CanTakeHit(DamageType.Kinetic, 6f)); // would break, reserve now emptied by Break() + shield.Break(); Assert.True(shield.Broken); ship.Update(1f); // restoring at 2/s -> 2 of 10, still short of full -- still Broken @@ -185,7 +195,8 @@ public sealed class ShieldReserveTests : IDisposable using var cache = OpenCatalog(); var (ship, shield) = BuildShipWithShield(cache, reactorGeneration: 20); ship.Update(1f); // Charge = 5 - Assert.False(shield.CanTakeHit(DamageType.Kinetic, 6f)); // breaks with 5 charge in play + Assert.False(shield.CanTakeHit(DamageType.Kinetic, 6f)); // would break with 5 charge in play + shield.Break(); ship.Update(3f); // from zero: 6 of 10 -- not yet full. From a surviving 5: would already be full. Assert.True(shield.Broken); @@ -204,7 +215,8 @@ public sealed class ShieldReserveTests : IDisposable using var cache = OpenCatalog(); var (ship, shield) = BuildShipWithShield(cache, reactorGeneration: 20); ship.Update(1f); // Charge = 5 - Assert.False(shield.CanTakeHit(DamageType.Kinetic, 6f)); // breaks, reserve now 0 + Assert.False(shield.CanTakeHit(DamageType.Kinetic, 6f)); // would break, reserve now emptied by Break() + shield.Break(); ship.Update(2f); // RefillDuration's span -- must not be enough to restore at RestoreDuration's rate Assert.True(shield.Broken); @@ -231,14 +243,86 @@ public sealed class ShieldReserveTests : IDisposable var (halfShip, halfShield) = BuildShipWithShield(halfCache, reactorGeneration: 2.5f); // half of the 5/s request halfShip.Update(1f); // granted half -> Charge = 2.5 - // Check the affordable amount FIRST: any refused CanTakeHit call breaks the shield (the ruling this - // file pins throughout), which would zero the reserve and make every later call read false regardless - // of grant. The half-grant reserve affords roughly its half share (2, with margin either side of the - // exact 2.5 to stay clear of float rounding at the boundary); only then do we spend the one refusal - // this test needs on a hit big enough that even the full-grant reserve could not have covered it more - // than four-fold over, to show the two really do differ. + // F5: CanTakeHit is a pure query now, so calling it more than once no longer risks the shield breaking + // itself out from under a later assertion -- these three reads can be in any order. The half-grant + // reserve affords roughly its half share (2, with margin either side of the exact 2.5 to stay clear of + // float rounding at the boundary); the final check is a hit big enough that even the full-grant reserve + // could not have covered it more than four-fold over, to show the two really do differ. Assert.True(halfShield.CanTakeHit(DamageType.Kinetic, 2f)); Assert.True(fullShield.CanTakeHit(DamageType.Kinetic, 4f)); Assert.False(halfShield.CanTakeHit(DamageType.Kinetic, 4f)); } + + // --- F5 (docs/stats-and-power-cut.md, Soul pass 2026-09-19): "make the query a query." Calling CanTakeHit + // --- any number of times, in any mix of true/false answers, must never move the reserve or set Broken -- + // --- exactly what every real caller under Assets/Scripts/Gameplay/Weapons now relies on (RaycastAll hands + // --- it both the shield and the hull collider for one shot, so it is queried twice per hit). --- + [Fact] + public void CanTakeHitNeverMutatesRegardlessOfHowManyTimesItIsCalled() + { + using var cache = OpenCatalog(); + var (ship, shield) = BuildShipWithShield(cache, reactorGeneration: 20); + ship.Update(1f); // Charge = 5 + + Assert.True(shield.CanTakeHit(DamageType.Kinetic, 3f)); + Assert.True(shield.CanTakeHit(DamageType.Kinetic, 3f)); // asked again -- still true, nothing spent + Assert.False(shield.Broken); + + Assert.False(shield.CanTakeHit(DamageType.Kinetic, 6f)); // would refuse -- exceeds the 5 held + Assert.False(shield.CanTakeHit(DamageType.Kinetic, 6f)); // asked again -- still false, still not broken + Assert.False(shield.Broken); + + // The reserve itself never moved either: a hit well within the original 5 still fits. + Assert.True(shield.CanTakeHit(DamageType.Kinetic, 4f)); + } + + // --- The mutation side of the same split: Break() is what a caller now invokes explicitly, and it must be + // --- safe to call more than once for one hit (a caller structure that cannot guarantee single-call + // --- discipline, per the six weapon files' own double-query shape) -- idempotent, not a double-drain or a + // --- crash on an already-empty reserve. --- + [Fact] + public void BreakIsIdempotentAcrossRepeatedCalls() + { + using var cache = OpenCatalog(); + var (ship, shield) = BuildShipWithShield(cache, reactorGeneration: 20); + ship.Update(1f); // Charge = 5 + + shield.Break(); + shield.Break(); // called again for the same hit (e.g. both the shield- and hull-collider branches) + shield.Break(); + Assert.True(shield.Broken); + + // Restores exactly on RestoreDuration from zero, same as a single Break() call -- proving the repeats + // did not re-empty an already-restoring reserve or otherwise disturb the clock. + ship.Update(3f); + Assert.True(shield.Broken); + ship.Update(2f); // total 5s -- exactly RestoreDuration + Assert.False(shield.Broken); + } + + // --- F4 (docs/stats-and-power-cut.md, Soul pass 2026-09-19): "restoration must not depend on the item being + // --- active." A hit that breaks the shield usually also cooks it, which can knock the item Active.Value + // --- false in the same moment (thermal/durability shutdown) -- Entity only runs Execute (and, pre-F4, the + // --- unbreak check that lived inside it) while Active is true, so the shield could never come back no + // --- matter how long it stayed offline. Simulate that here directly (Enabled = false covers every reason + // --- Active can go false; the mechanism doesn't care which) and confirm the reserve keeps restoring and + // --- clears Broken on schedule regardless. --- + [Fact] + public void RestorationProceedsWhileTheItemIsInactive() + { + using var cache = OpenCatalog(); + var (ship, shield) = BuildShipWithShield(cache, reactorGeneration: 20); + ship.Update(1f); // Charge = 5 + Assert.False(shield.CanTakeHit(DamageType.Kinetic, 6f)); // would break + shield.Break(); + Assert.True(shield.Broken); + + shield.Item.Enabled.Value = false; + Assert.False(shield.Item.Active.Value); + + // RestoreDuration (5s) entirely while inactive -- if restoration depended on Active (Execute-gated, the + // pre-F4 shape), none of this would move the reserve at all and Broken would still be true. + ship.Update(5f); + Assert.False(shield.Broken); + } }