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.
This commit is contained in:
Keith Solomon
2026-07-05 20:33:03 -05:00
parent 12f5c0bd3a
commit 39b15c35da
3 changed files with 3 additions and 8 deletions
-4
View File
@@ -89,10 +89,6 @@ final class CombatEngine
$definition = $definitions[$abilityId]; $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)) { if (!in_array($abilityId, $caster->allowedAbilities(), true)) {
throw new CombatException('Unit archetype forbids that ability.'); throw new CombatException('Unit archetype forbids that ability.');
} }
+2 -3
View File
@@ -72,12 +72,11 @@ final readonly class Scenario
throw new InvalidArgumentException("Unit {$unit->id} is outside the battlefield."); 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."); 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) { if ($unit->maxHealth < $template->minHealth || $unit->maxHealth > $template->maxHealth) {
throw new InvalidArgumentException("Unit {$unit->id} health is outside archetype bounds."); throw new InvalidArgumentException("Unit {$unit->id} health is outside archetype bounds.");
} }
+1 -1
View File
@@ -971,7 +971,7 @@ final class CombatEngineTest extends TestCase
); );
$this->expectException(CombatException::class); $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)); (new CombatEngine())->useAbility($match, 'scout-1', 'heal', new Position(0, 1));
} }