From a9b33e40554d73cdd487ef217379b3d0edb63925 Mon Sep 17 00:00:00 2001 From: ctan Date: Sat, 12 Sep 2026 11:14:30 +0200 Subject: [PATCH] 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 --- src/pf1e_simulator/combat.py | 60 ++++++++++++++++++++++++++--------- tests/test_class_abilities.py | 52 ++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 15 deletions(-) diff --git a/src/pf1e_simulator/combat.py b/src/pf1e_simulator/combat.py index c9e1d46..93c7180 100644 --- a/src/pf1e_simulator/combat.py +++ b/src/pf1e_simulator/combat.py @@ -4822,7 +4822,9 @@ class CombatEngine: 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 - 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 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): # Arcane pool not tracked yet, allow for now pass - # Cast the spell - self._cast_spell(state, target, spell_name) + # Cast the spell — PF1e (CRB, Magus Spell Combat): "any attack roll + # 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) self._execute_attack( state, target, "full_attack", attack_penalty=_SPELL_COMBAT_ATTACK_PENALTY @@ -5038,6 +5044,7 @@ class CombatEngine: center_pos: tuple[int, int] | None = None, metamagic: tuple[str, ...] = (), cast_defensively: bool = False, + attack_penalty: int = 0, ) -> ActionResult | None: """Cast ``spell_name``. @@ -5050,8 +5057,11 @@ class CombatEngine: THIS cast (only feats the caster knows apply, and the spell then occupies — and must have — a slot of base level + adjustment). ``cast_defensively`` (B24) skips the casting AoO for a concentration - check. Returns an explicit rejection (``no_slot``) when the required - slot is unavailable; None means the cast proceeded through the gates. + check. ``attack_penalty`` folds into this cast's touch attack rolls + (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) if spell is None: @@ -5093,7 +5103,9 @@ class CombatEngine: return None # PF1e (CRB): hostile touch delivery needs a melee touch attack (B14) # 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 self._apply_and_log_cast( caster, target, spell, spell_name, center_pos, @@ -5134,16 +5146,20 @@ class CombatEngine: spell: SpellSpec, *, has_area: bool, + attack_penalty: int = 0, ) -> bool: """Delivery gates: a hostile touch spell must land its melee touch attack (B14) and a hostile ranged touch spell (ray) must land its 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 ( target is not None and spell.range == "touch" 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 if ( @@ -5151,7 +5167,9 @@ class CombatEngine: and spell.requires_ranged_touch and not has_area 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 ( @@ -5161,19 +5179,25 @@ class CombatEngine: ) def _spell_touch_attack_hits( - self, caster: CombatantState, target: CombatantState + self, + caster: CombatantState, + target: CombatantState, + *, + attack_penalty: int = 0, ) -> bool: """Melee touch attack to deliver a touch-range spell (B14). 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 not modeled; natural 20 hits and natural 1 misses per the engine's attack-roll convention. The touch AC reuses the resolve_attack denies-Dex path (flat-footed/running target, invisible attacker). """ 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 = ( self.has_condition_flag(target, "denies_dex_to_ac") or target.running @@ -5192,13 +5216,19 @@ class CombatEngine: return hit def _spell_ranged_touch_attack_hits( - self, caster: CombatantState, target: CombatantState + self, + caster: CombatantState, + target: CombatantState, + *, + attack_penalty: int = 0, ) -> bool: """Ranged touch attack to deliver a ray spell. 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 - 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, Weapon Finesse and other situational bonuses are not modeled; natural 20 hits and natural 1 misses per the engine's attack-roll convention. @@ -5206,7 +5236,7 @@ class CombatEngine: running target, invisible attacker). """ 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 = ( self.has_condition_flag(target, "denies_dex_to_ac") or target.running diff --git a/tests/test_class_abilities.py b/tests/test_class_abilities.py index 36d636d..e3d3ced 100644 --- a/tests/test_class_abilities.py +++ b/tests/test_class_abilities.py @@ -31,6 +31,7 @@ def _combatant( *, resources: dict[str, int] | None = None, spells: list[str] | None = None, + spell_slots: dict[str, int] | None = None, hp_max: int = 20, creature_type: Literal["living", "undead", "construct"] = "living", ) -> Combatant: @@ -59,6 +60,7 @@ def _combatant( attacks=[attack], resources=resources or {}, spells=spells or [], + spell_slots=spell_slots or {}, 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]: """Misty + one ally in 30ft, inspire courage executed on the ally.""" misty = _combatant("misty", resources={"bardic_rounds": 12})