From 39b15c35da82968c2028d7a0c46a735f7e78ed30 Mon Sep 17 00:00:00 2001
From: Keith Solomon <keith@vincentdesign.ca>
Date: Sun, 5 Jul 2026 20:33:03 -0500
Subject: [PATCH] refactor: tighten useAbility and Scenario archetype lookup

Final-review-driven changes (Tasks 6/M1, I1 from the ledger):

- Drop the redundant abilities-list check in CombatEngine::useAbility.
  By construction (Scenario::__construct enforces the allowlist), any
  ability the unit 'knows' is also archetype-allowed; the second check
  alone is sufficient and removes a dead code path.
- Update testUseAbilityRejectsAnAbilityTheArchetypeForbids to assert the
  'Unit archetype forbids that ability.' message it actually exercises.
- Compute $template once in Scenario::__construct's per-unit loop and
  drop the redundant catalog lookup on the next line.
---
 src/Domain/CombatEngine.php            | 4 ----
 src/Domain/Scenario.php                | 5 ++---
 tests/Unit/Domain/CombatEngineTest.php | 2 +-
 3 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/src/Domain/CombatEngine.php b/src/Domain/CombatEngine.php
index 2880a8c..c96ca19 100644
--- a/src/Domain/CombatEngine.php
+++ b/src/Domain/CombatEngine.php
@@ -89,10 +89,6 @@ final class CombatEngine
 
         $definition = $definitions[$abilityId];
 
-        if (!in_array($abilityId, $caster->abilities, true)) {
-            throw new CombatException('Unit does not know that ability.');
-        }
-
         if (!in_array($abilityId, $caster->allowedAbilities(), true)) {
             throw new CombatException('Unit archetype forbids that ability.');
         }
diff --git a/src/Domain/Scenario.php b/src/Domain/Scenario.php
index 4a3c4ac..9d9efb6 100644
--- a/src/Domain/Scenario.php
+++ b/src/Domain/Scenario.php
@@ -72,12 +72,11 @@ final readonly class Scenario
                 throw new InvalidArgumentException("Unit {$unit->id} is outside the battlefield.");
             }
 
-            if (!ArchetypeCatalog::templates()[$unit->archetype->value] ?? false) { // @phpstan-ignore nullCoalesce.expr, if.alwaysFalse, booleanNot.alwaysFalse (Runtime guard: defense in depth against an unknown Archetype enum value.)
+            $template = ArchetypeCatalog::templates()[$unit->archetype->value] ?? null;
+            if ($template === null) {
                 throw new InvalidArgumentException("Unit {$unit->id} uses an unknown archetype.");
             }
 
-            $template = ArchetypeCatalog::templates()[$unit->archetype->value];
-
             if ($unit->maxHealth < $template->minHealth || $unit->maxHealth > $template->maxHealth) {
                 throw new InvalidArgumentException("Unit {$unit->id} health is outside archetype bounds.");
             }
diff --git a/tests/Unit/Domain/CombatEngineTest.php b/tests/Unit/Domain/CombatEngineTest.php
index 59f0026..57b33bd 100644
--- a/tests/Unit/Domain/CombatEngineTest.php
+++ b/tests/Unit/Domain/CombatEngineTest.php
@@ -971,7 +971,7 @@ final class CombatEngineTest extends TestCase
         );
 
         $this->expectException(CombatException::class);
-        $this->expectExceptionMessage('Unit does not know that ability.');
+        $this->expectExceptionMessage('Unit archetype forbids that ability.');
 
         (new CombatEngine())->useAbility($match, 'scout-1', 'heal', new Position(0, 1));
     }