Skip to content

Trigger on your own countered spell (Multani's Presence) - #4903

Merged
matthewevans merged 1 commit into
phase-rs:mainfrom
e11734937-beep:fix-own-spell-countered-trigger
Jul 2, 2026
Merged

Trigger on your own countered spell (Multani's Presence)#4903
matthewevans merged 1 commit into
phase-rs:mainfrom
e11734937-beep:fix-own-spell-countered-trigger

Conversation

@e11734937-beep

Copy link
Copy Markdown
Contributor

Summary

Multani's Presence"Whenever a spell you've cast is countered, draw a card." — parsed its trigger to Unknown("Whenever a spell you've cast is countered") and silently dropped it. The engine already models this via TriggerMode::Countered, but only the active side ("a spell or ability you control counters a spell") had a recognizer.

Change

Add parse_own_spell_countered_line to try_parse_player_trigger (crates/engine/src/parser/oracle_trigger.rs) — the passive dual of the existing countering-side arm. It recognizes "whenever|when a spell you've cast|you control is countered" (whole-clause all_consuming, nom combinators only) and emits TriggerMode::Countered with valid_card = Typed(controller = You).

At runtime, match_countered (game/trigger_matchers.rs) evaluates valid_card against the SpellCountered event's object_id (the countered spell), so the You controller filter restricts the trigger to your own countered spell — the passive dual of the active arm, which gates the countering source via valid_source. The Draw child was already supported.

Semantics: a spell's controller is its caster (CR 108.4), so "you've cast" ≡ "you control" for a spell; both route to ControllerRef::You. The trigger fires only for your own countered spell, never an opponent's.

CR annotations grep-verified: CR 701.6a (countering), CR 603.2 (trigger conditions), CR 108.4 (a spell's controller).

Tests

  • trigger_a_spell_youve_cast_is_countered / trigger_a_spell_you_control_is_countered — both forms parse to Countered with valid_card = Controller(You), valid_source = None.
  • countered_trigger_valid_card_gates_own_spellruntime: your countered spell fires the trigger; an opponent's countered spell does NOT.

Fails-before (trigger → Unknown) / passes-after confirmed. Green: rustfmt, parser-combinator (Rule Zero) gate, clippy -D warnings, full engine suite (14546 passed / 0 failed), cargo coverage clean. The new arm is a whole-clause all_consuming match on a clause that previously parsed to Unknown, so no other card's parse is affected.

"Whenever a spell you've cast is countered, draw a card." parsed to an
Unknown trigger and was silently dropped, so Multani's Presence never
fired. This is the passive dual of the already-supported active-side
"a spell or ability you control counters a spell" trigger.

Add a recognizer arm in `try_parse_player_trigger` that maps
"whenever/when a spell you've cast|you control is countered" to the
existing `TriggerMode::Countered`, gating the *countered* spell via
`valid_card = Controller(You)`. At runtime `match_countered` evaluates
`valid_card` against the `SpellCountered` event's `object_id` (the
countered spell) through `target_filter_matches_object`, so the trigger
fires only when the countered spell's controller is you — exactly the
card's semantics. This differs from the active-side arm, which gates the
countering source via `valid_source`. A spell's controller is its caster
(CR 108.4), so "you've cast" and "you control" both route to the single
`ControllerRef::You` filter.

No new Effect/trigger mode or resolver machinery: reuses the existing
`Countered` mode, its `match_countered` matcher, `valid_card` filtering,
and the supported `Draw` child effect.

Unlocks: Multani's Presence.

Tests:
- parser::oracle_trigger::tests::trigger_a_spell_youve_cast_is_countered
- parser::oracle_trigger::tests::trigger_a_spell_you_control_is_countered
- game::trigger_matchers::tests::countered_trigger_valid_card_gates_own_spell
  (runtime: own countered spell fires; opponent's countered spell does not)

CR 701.6a (counter), CR 603.2 (trigger conditions), CR 108.4 (a spell's
controller). Verified against docs/MagicCompRules.txt.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements parser support and matching logic for triggers that fire when a spell you control or have cast is countered (such as Multani's Presence), along with corresponding unit tests. The review feedback correctly identifies that the comments in all three modified files cite CR 701.6a (token creation) instead of CR 701.5a (countering), violating the repository's rule on verified CR annotations.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

}
}

// CR 701.6a + CR 603.2 + CR 108.4: "Whenever a spell you've cast is

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

[HIGH] Incorrect CR citation for countering. Evidence: crates/engine/src/parser/oracle_trigger.rs:11712. Why it matters: The comment cites CR 701.6a (which defines token creation) instead of CR 701.5a (which defines countering). Suggested fix: Change CR 701.6a to CR 701.5a.

Suggested change
// CR 701.6a + CR 603.2 + CR 108.4: "Whenever a spell you've cast is
// CR 701.5a + CR 603.2 + CR 108.4: "Whenever a spell you've cast is
References
  1. Every rules-touching line of engine code must carry a comment of the form CR : where the cited rule's body describes what the code is doing. (link)


#[test]
fn countered_trigger_valid_card_gates_own_spell() {
// CR 701.6a + CR 108.4: Multani's Presence -- "Whenever a spell you've

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

[HIGH] Incorrect CR citation for countering. Evidence: crates/engine/src/game/trigger_matchers.rs:4682. Why it matters: The comment cites CR 701.6a (which defines token creation) instead of CR 701.5a (which defines countering). Suggested fix: Change CR 701.6a to CR 701.5a.

Suggested change
// CR 701.6a + CR 108.4: Multani's Presence -- "Whenever a spell you've
// CR 701.5a + CR 108.4: Multani's Presence -- "Whenever a spell you've
References
  1. Every rules-touching line of engine code must carry a comment of the form CR : where the cited rule's body describes what the code is doing. (link)


#[test]
fn trigger_a_spell_youve_cast_is_countered() {
// CR 701.6a + CR 108.4: Multani's Presence -- the passive dual of the

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

[HIGH] Incorrect CR citation for countering. Evidence: crates/engine/src/parser/oracle_trigger_tests.rs:8207. Why it matters: The comment cites CR 701.6a (which defines token creation) instead of CR 701.5a (which defines countering). Suggested fix: Change CR 701.6a to CR 701.5a.

Suggested change
// CR 701.6a + CR 108.4: Multani's Presence -- the passive dual of the
// CR 701.5a + CR 108.4: Multani's Presence -- the passive dual of the
References
  1. Every rules-touching line of engine code must carry a comment of the form CR : where the cited rule's body describes what the code is doing. (link)

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

Parse changes introduced by this PR · 1 card(s), 2 signature(s) (baseline: main 6cefafb21d54)

1 card(s) · trigger/Countered · added: Countered (active in=battlefield, watches=you)

Examples: Multani's Presence

1 card(s) · trigger/Whenever a spell you've cast is countered · removed: Whenever a spell you've cast is countered (active in=battlefield)

Examples: Multani's Presence

1 card(s) had Oracle-text changes (errata/reprint) — excluded as non-parser.

@matthewevans matthewevans self-assigned this Jul 2, 2026

@matthewevans matthewevans left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved current head 58bd95b2abad18558b08505b586a8d921a688336.

Evidence checked: parse-diff is scoped to Multani's Presence (one card / two signatures), the parser-combinator gate passes locally, CI is green, and the implementation is at the existing TriggerMode::Countered seam. Hand trace: the parser maps "spell you've cast / spell controlled by you is countered" to valid_card = controller(You), and match_countered applies valid_card to the countered stack object while preserving the separate valid_source check for the countering spell/ability. The added matcher tests cover own-spell vs opponent-spell behavior and the parser test would fail if the trigger remained unimplemented. CR references used here were verified against the local rules text.

@matthewevans matthewevans added the bug Bug fix label Jul 2, 2026
@matthewevans
matthewevans added this pull request to the merge queue Jul 2, 2026
@matthewevans matthewevans removed their assignment Jul 2, 2026
Merged via the queue into phase-rs:main with commit e9cb200 Jul 2, 2026
11 checks passed
@e11734937-beep

Copy link
Copy Markdown
Contributor Author

Hi @matthewevans — quick labeling question. The card implementations I've been landing (Regenerate, Role Reversal, Coastal Wizard, this countered-spell trigger, etc.) are coming through as bug. Since these add new card support rather than fix regressions, would feature — or quality for the larger ones — be the more appropriate label, or is bug intentional for coverage-completion work? Happy to follow whatever convention you prefer; just want to tag them right going forward. Thanks for the fast merges!

@matthewevans

Copy link
Copy Markdown
Member

Hi @matthewevans — quick labeling question. The card implementations I've been landing (Regenerate, Role Reversal, Coastal Wizard, this countered-spell trigger, etc.) are coming through as bug. Since these add new card support rather than fix regressions, would feature — or quality for the larger ones — be the more appropriate label, or is bug intentional for coverage-completion work? Happy to follow whatever convention you prefer; just want to tag them right going forward. Thanks for the fast merges!

Heya! If your PRs are being misclassified please let me know. In the case of adding new card support (wiring it up) that would be enhancement and is actually worth more than the feature label.

quality label is at my discretion and is for PRs that are submitted that effectively check all the boxes. Right architectural seam, elegant code, discriminating tests, little to no review churn. :)

Hope this helps!

@e11734937-beep

Copy link
Copy Markdown
Contributor Author

That's super helpful, thank you! Yes — these are all new-card wiring (Regenerate, Role Reversal, Coastal Wizard, the skip-step form, this countered-spell trigger, Valakut Fireboar), so enhancement fits. Would you be open to re-tagging the already-merged ones from bugenhancement when you get a moment? And I'll aim for the quality bar (right seam, discriminating tests, minimal churn) on the ones that warrant it. Really appreciate the guidance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Bug fix enhancement New feature or request quality For high-quality minimal to no-churn PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants