fix(rules): apply the spell combat penalty to the spell attack

PF1e (CRB Magus Spell Combat): "any attack roll made as part of this
spell also takes this penalty". _execute_spell_combat now threads the -2
through _cast_spell into the melee/ranged touch attack helpers, so e.g.
Shocking Grasp delivered via Spell Combat attacks at -2 too. No global
state is touched: the penalty is a keyword threaded per cast.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
2026-09-12 11:14:30 +02:00
parent 068b49794e
commit a9b33e4055
2 changed files with 97 additions and 15 deletions
+45 -15
View File
@@ -4822,7 +4822,9 @@ class CombatEngine:
PF1e (CRB, Magus Spell Combat): as a full-round action the magus casts PF1e (CRB, Magus Spell Combat): as a full-round action the magus casts
a standard-action spell and makes all of his melee attacks at -2. The a standard-action spell and makes all of his melee attacks at -2. The
penalty rides the full-attack chain to every swing and crit confirm. penalty rides the full-attack chain to every swing and crit confirm,
and — "any attack roll made as part of this spell also takes this
penalty" — the spell's own melee/ranged touch attack too.
Simplification: the spell may target any resolved enemy (the RAW Simplification: the spell may target any resolved enemy (the RAW
spellcasting-as-move restriction and arcane pool cost are not modeled). spellcasting-as-move restriction and arcane pool cost are not modeled).
""" """
@@ -4831,8 +4833,12 @@ class CombatEngine:
if not state.spend_resource("arcane_pool", 1): if not state.spend_resource("arcane_pool", 1):
# Arcane pool not tracked yet, allow for now # Arcane pool not tracked yet, allow for now
pass pass
# Cast the spell # Cast the spell — PF1e (CRB, Magus Spell Combat): "any attack roll
self._cast_spell(state, target, spell_name) # made as part of this spell also takes this penalty", so the -2 rides
# the spell's touch attack too.
self._cast_spell(
state, target, spell_name, attack_penalty=_SPELL_COMBAT_ATTACK_PENALTY
)
# Then make a full attack at -2 (Spell Combat penalty) # Then make a full attack at -2 (Spell Combat penalty)
self._execute_attack( self._execute_attack(
state, target, "full_attack", attack_penalty=_SPELL_COMBAT_ATTACK_PENALTY state, target, "full_attack", attack_penalty=_SPELL_COMBAT_ATTACK_PENALTY
@@ -5038,6 +5044,7 @@ class CombatEngine:
center_pos: tuple[int, int] | None = None, center_pos: tuple[int, int] | None = None,
metamagic: tuple[str, ...] = (), metamagic: tuple[str, ...] = (),
cast_defensively: bool = False, cast_defensively: bool = False,
attack_penalty: int = 0,
) -> ActionResult | None: ) -> ActionResult | None:
"""Cast ``spell_name``. """Cast ``spell_name``.
@@ -5050,8 +5057,11 @@ class CombatEngine:
THIS cast (only feats the caster knows apply, and the spell then THIS cast (only feats the caster knows apply, and the spell then
occupies — and must have — a slot of base level + adjustment). occupies — and must have — a slot of base level + adjustment).
``cast_defensively`` (B24) skips the casting AoO for a concentration ``cast_defensively`` (B24) skips the casting AoO for a concentration
check. Returns an explicit rejection (``no_slot``) when the required check. ``attack_penalty`` folds into this cast's touch attack rolls
slot is unavailable; None means the cast proceeded through the gates. (PF1e CRB Magus Spell Combat: "any attack roll made as part of this
spell also takes this penalty"). Returns an explicit rejection
(``no_slot``) when the required slot is unavailable; None means the
cast proceeded through the gates.
""" """
spell = self._spell_registry.get(spell_name) spell = self._spell_registry.get(spell_name)
if spell is None: if spell is None:
@@ -5093,7 +5103,9 @@ class CombatEngine:
return None return None
# PF1e (CRB): hostile touch delivery needs a melee touch attack (B14) # PF1e (CRB): hostile touch delivery needs a melee touch attack (B14)
# and spell resistance can still turn the spell away. # and spell resistance can still turn the spell away.
if self._spell_delivery_blocked(caster, target, spell, has_area=has_area): if self._spell_delivery_blocked(
caster, target, spell, has_area=has_area, attack_penalty=attack_penalty
):
return None return None
self._apply_and_log_cast( self._apply_and_log_cast(
caster, target, spell, spell_name, center_pos, caster, target, spell, spell_name, center_pos,
@@ -5134,16 +5146,20 @@ class CombatEngine:
spell: SpellSpec, spell: SpellSpec,
*, *,
has_area: bool, has_area: bool,
attack_penalty: int = 0,
) -> bool: ) -> bool:
"""Delivery gates: a hostile touch spell must land its melee touch """Delivery gates: a hostile touch spell must land its melee touch
attack (B14) and a hostile ranged touch spell (ray) must land its attack (B14) and a hostile ranged touch spell (ray) must land its
ranged touch attack, then spell resistance must be overcome. True = ranged touch attack, then spell resistance must be overcome. True =
no effect (the slot is already spent).""" no effect (the slot is already spent). ``attack_penalty`` applies to
both touch rolls (Spell Combat)."""
if ( if (
target is not None target is not None
and spell.range == "touch" and spell.range == "touch"
and target.side != caster.side and target.side != caster.side
and not self._spell_touch_attack_hits(caster, target) and not self._spell_touch_attack_hits(
caster, target, attack_penalty=attack_penalty
)
): ):
return True return True
if ( if (
@@ -5151,7 +5167,9 @@ class CombatEngine:
and spell.requires_ranged_touch and spell.requires_ranged_touch
and not has_area and not has_area
and target.side != caster.side and target.side != caster.side
and not self._spell_ranged_touch_attack_hits(caster, target) and not self._spell_ranged_touch_attack_hits(
caster, target, attack_penalty=attack_penalty
)
): ):
return True return True
return ( return (
@@ -5161,19 +5179,25 @@ class CombatEngine:
) )
def _spell_touch_attack_hits( def _spell_touch_attack_hits(
self, caster: CombatantState, target: CombatantState self,
caster: CombatantState,
target: CombatantState,
*,
attack_penalty: int = 0,
) -> bool: ) -> bool:
"""Melee touch attack to deliver a touch-range spell (B14). """Melee touch attack to deliver a touch-range spell (B14).
PF1e (CRB): d20 + caster's melee attack bonus vs the target's touch PF1e (CRB): d20 + caster's melee attack bonus vs the target's touch
AC. Engine simplification (documented): the bonus is BAB + Strength AC. ``attack_penalty`` folds into the bonus (Spell Combat: "any
attack roll made as part of this spell also takes this penalty").
Engine simplification (documented): the bonus is BAB + Strength
modifier — Weapon Finesse and other situational touch bonuses are modifier — Weapon Finesse and other situational touch bonuses are
not modeled; natural 20 hits and natural 1 misses per the engine's not modeled; natural 20 hits and natural 1 misses per the engine's
attack-roll convention. The touch AC reuses the resolve_attack attack-roll convention. The touch AC reuses the resolve_attack
denies-Dex path (flat-footed/running target, invisible attacker). denies-Dex path (flat-footed/running target, invisible attacker).
""" """
roll = self._rng.d20() roll = self._rng.d20()
bonus = caster.combatant.bab + caster.combatant.abilities.str_mod bonus = caster.combatant.bab + caster.combatant.abilities.str_mod + attack_penalty
denies_dex = ( denies_dex = (
self.has_condition_flag(target, "denies_dex_to_ac") self.has_condition_flag(target, "denies_dex_to_ac")
or target.running or target.running
@@ -5192,13 +5216,19 @@ class CombatEngine:
return hit return hit
def _spell_ranged_touch_attack_hits( def _spell_ranged_touch_attack_hits(
self, caster: CombatantState, target: CombatantState self,
caster: CombatantState,
target: CombatantState,
*,
attack_penalty: int = 0,
) -> bool: ) -> bool:
"""Ranged touch attack to deliver a ray spell. """Ranged touch attack to deliver a ray spell.
PF1e (CRB, Magic — Ranged Touch Attacks): d20 + caster's ranged attack PF1e (CRB, Magic — Ranged Touch Attacks): d20 + caster's ranged attack
bonus vs the target's touch AC; the spell's range is a single absolute bonus vs the target's touch AC; the spell's range is a single absolute
value so there is no range-increment penalty. Engine simplification value so there is no range-increment penalty. ``attack_penalty`` folds
into the bonus (Spell Combat: "any attack roll made as part of this
spell also takes this penalty"). Engine simplification
(documented): the bonus is BAB + Dexterity modifier — size modifier, (documented): the bonus is BAB + Dexterity modifier — size modifier,
Weapon Finesse and other situational bonuses are not modeled; natural Weapon Finesse and other situational bonuses are not modeled; natural
20 hits and natural 1 misses per the engine's attack-roll convention. 20 hits and natural 1 misses per the engine's attack-roll convention.
@@ -5206,7 +5236,7 @@ class CombatEngine:
running target, invisible attacker). running target, invisible attacker).
""" """
roll = self._rng.d20() roll = self._rng.d20()
bonus = caster.combatant.bab + caster.combatant.abilities.dex_mod bonus = caster.combatant.bab + caster.combatant.abilities.dex_mod + attack_penalty
denies_dex = ( denies_dex = (
self.has_condition_flag(target, "denies_dex_to_ac") self.has_condition_flag(target, "denies_dex_to_ac")
or target.running or target.running
+52
View File
@@ -31,6 +31,7 @@ def _combatant(
*, *,
resources: dict[str, int] | None = None, resources: dict[str, int] | None = None,
spells: list[str] | None = None, spells: list[str] | None = None,
spell_slots: dict[str, int] | None = None,
hp_max: int = 20, hp_max: int = 20,
creature_type: Literal["living", "undead", "construct"] = "living", creature_type: Literal["living", "undead", "construct"] = "living",
) -> Combatant: ) -> Combatant:
@@ -59,6 +60,7 @@ def _combatant(
attacks=[attack], attacks=[attack],
resources=resources or {}, resources=resources or {},
spells=spells or [], spells=spells or [],
spell_slots=spell_slots or {},
creature_type=creature_type, creature_type=creature_type,
) )
@@ -378,6 +380,56 @@ def test_normal_full_attack_has_no_spell_combat_penalty() -> None:
) )
def test_spell_combat_penalty_applies_to_spell_touch_attack() -> None:
"""CRB Magus Spell Combat: "any attack roll made as part of this spell
also takes this penalty" — Shocking Grasp's melee touch attack is at -2
(bab 3 + str 2 - 2 = +3): d20 8 -> 11 misses touch AC 12, so the spell
is wasted. Without the penalty the same roll (8+5=13) would have hit."""
misty = _combatant(
"misty", resources={"bardic_rounds": 12}, spells=["Shocking Grasp"],
spell_slots={1: 1},
)
goblin = _combatant("goblin", hp_max=6)
reg = load_spell_registry(SPELLS_DIR)
# Queue: goblin AoO on the cast (nat 1 miss), touch d20=8, swing d20=10.
engine, me, enemy = _engine(
misty, [goblin], spell_registry=reg, rng_queue=[1, 8, 10]
)
engine._execute(
me,
Action(
kind="spell_combat", target_id=enemy[0].combatant.id,
spell_name="Shocking Grasp",
),
)
assert any(
"touch attack vs goblin d20=8+3=11 AC 12 -> MISS" in line
for line in engine._transcript
)
assert enemy[0].hp == 6 # the missed touch delivers no electricity
def test_plain_cast_touch_attack_has_no_spell_combat_penalty() -> None:
"""Control: the same Shocking Grasp cast through cast_spell keeps +5
(bab 3 + str 2): d20 8 -> 13 hits touch AC 12 and deals damage."""
misty = _combatant(
"misty", spells=["Shocking Grasp"], spell_slots={1: 1}
)
goblin = _combatant("goblin", hp_max=6)
reg = load_spell_registry(SPELLS_DIR)
# Queue: goblin AoO (nat 1 miss), touch d20=8, damage 1d6=4.
engine, me, enemy = _engine(misty, [goblin], spell_registry=reg, rng_queue=[1, 8, 4])
engine._execute(
me, Action(kind="cast_spell", target_id=enemy[0].combatant.id,
spell_name="Shocking Grasp")
)
assert any(
"touch attack vs goblin d20=8+5=13 AC 12 -> HIT" in line
for line in engine._transcript
)
assert enemy[0].hp == 2
def _inspired_ally() -> tuple[CombatEngine, CombatantState, CombatantState]: def _inspired_ally() -> tuple[CombatEngine, CombatantState, CombatantState]:
"""Misty + one ally in 30ft, inspire courage executed on the ally.""" """Misty + one ally in 30ft, inspire courage executed on the ally."""
misty = _combatant("misty", resources={"bardic_rounds": 12}) misty = _combatant("misty", resources={"bardic_rounds": 12})