From c3f75d872d4d6633cecb34971de72b18c7c5f89d Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Mon, 10 Aug 2026 18:16:01 -0500 Subject: [PATCH 1/5] feat(engine): observe Saga chapter-ability lifecycle so Narci can drain Narci, Fable Singer's third ability ("whenever the final chapter ability of a Saga you control resolves, each opponent loses X life and you gain X life, where X is that Saga's mana value") had no primitive behind it: the engine could not observe another permanent's chapter ability at all. Build the class, not the card. Three printed cards reference a chapter ability (Narci, Tom Bombadil, Historian's Boon) and they vary over two independent axes, so TriggerMode::SagaChapterAbility is parameterized on both rather than split into sibling variants (mirroring the existing Planeswalked { role }): * SagaChapterScope -- any chapter ability vs the final one (CR 714.4). * AbilityLifecyclePoint -- the ability triggering (CR 603.2) vs finishing resolution (CR 608.2). These are genuinely different events: a chapter ability that triggers may still be countered or fail its intervening-if and never resolve. GameEvent::SagaChapterAbilityResolved is new because nothing on the bus reported that fact. StackResolved is pushed on the fizzle and failed-intervening-if paths too, and carries the stack entry id rather than the Saga, so it cannot distinguish an ability that resolved from one that merely left the stack. The new event is published from the single resolve_top exit reached only after a triggered ability genuinely resolved. Chapter identity is captured BEFORE the ability executes, keyed off the exact trigger occurrence that fired. It cannot be recovered afterward: CR 704.5s sacrifices a Saga the moment its final chapter ability leaves the stack, and a chapter ability may exile its own Saga (Fable of the Mirror-Breaker III). Deriving the chapter from the lore count instead would be wrong under Read Ahead and under multi-counter additions, which cross several thresholds at once. "That Saga's mana value" binds to ObjectScope::EventSource, not to Target like the neighbouring "that creature's" arm -- a meta-trigger announces no targets, so the Saga carried by the event is the only referent that exists. It resolves through the existing live-then-LKI mana-value path, which is what makes X correct after the Saga is already gone. final_chapter_number now delegates to a new saga_chapter_numbers and is scoped to lore counters: per CR 714.2a only lore-counter triggers are chapter abilities, so a Saga carrying a thresholded trigger on some other counter type no longer inflates its final chapter. Co-Authored-By: Claude Opus 5 --- client/src/adapter/types.ts | 4 + client/src/animation/eventNormalizer.ts | 3 + .../src/ai_support/shortcut_efficacy.rs | 14 +- crates/engine/src/analysis/ability_graph.rs | 3 + crates/engine/src/game/game_object.rs | 68 +++-- crates/engine/src/game/log.rs | 17 ++ crates/engine/src/game/public_state.rs | 3 + crates/engine/src/game/stack.rs | 58 ++++ crates/engine/src/game/targeting.rs | 5 + crates/engine/src/game/trigger_index.rs | 9 + crates/engine/src/game/trigger_matchers.rs | 124 ++++++++- .../engine/src/parser/oracle_nom/quantity.rs | 9 + crates/engine/src/parser/oracle_trigger.rs | 200 +++++++++++++- crates/engine/src/types/events.rs | 23 ++ crates/engine/src/types/triggers.rs | 41 +++ crates/engine/tests/integration/main.rs | 1 + .../narci_fable_singer_final_chapter_drain.rs | 254 ++++++++++++++++++ 17 files changed, 814 insertions(+), 22 deletions(-) create mode 100644 crates/engine/tests/integration/narci_fable_singer_final_chapter_drain.rs diff --git a/client/src/adapter/types.ts b/client/src/adapter/types.ts index 597ad599fa..327366ad5f 100644 --- a/client/src/adapter/types.ts +++ b/client/src/adapter/types.ts @@ -2460,6 +2460,10 @@ export type GameEvent = | { type: "LandPlayed"; data: { object_id: ObjectId; player_id: PlayerId; from_zone: Zone } } | { type: "StackPushed"; data: { object_id: ObjectId } } | { type: "StackResolved"; data: { object_id: ObjectId } } + // CR 714.2a: a Saga's chapter ability finished resolving. Bookkeeping the + // engine publishes for meta-triggers (Narci, Fable Singer); non-visual, since + // the chapter ability's own effects already animate. + | { type: "SagaChapterAbilityResolved"; data: { saga_id: ObjectId; controller: PlayerId; chapter: number; final_chapter: number } } | { type: "Discarded"; data: { player_id: PlayerId; object_id: ObjectId } } | { type: "EnduringStoryGained"; data: { player_id: PlayerId } } | { type: "DamageCleared"; data: { object_id: ObjectId } } diff --git a/client/src/animation/eventNormalizer.ts b/client/src/animation/eventNormalizer.ts index 28c2213126..0c16281076 100644 --- a/client/src/animation/eventNormalizer.ts +++ b/client/src/animation/eventNormalizer.ts @@ -30,6 +30,9 @@ const NON_VISUAL_EVENTS = new Set([ "PermanentUntapped", "StackPushed", "StackResolved", + // CR 714.2a: same class as StackResolved — the chapter ability's own effects + // are what the player sees; this event exists for meta-triggers. + "SagaChapterAbilityResolved", "ReplacementApplied", "Regenerated", "AttackersDeclared", diff --git a/crates/engine/src/ai_support/shortcut_efficacy.rs b/crates/engine/src/ai_support/shortcut_efficacy.rs index ad2fde44da..3be28ab584 100644 --- a/crates/engine/src/ai_support/shortcut_efficacy.rs +++ b/crates/engine/src/ai_support/shortcut_efficacy.rs @@ -3371,25 +3371,29 @@ mod tests { let body = body.split_once("\n}").expect("…and terminated").0; // The names this enumeration cannot construct, PINNED so the gap cannot - // grow silently. Three carry payloads and have no bare-name spelling for + // grow silently. Four carry payloads and have no bare-name spelling for // `FromStr`; the other three are PRE-EXISTING gaps in // `types::triggers`'s own decoder — they are declared on the enum but have // no `FromStr` arm, so `from_str` degrades them to `Unknown`. That gap is // not this module's to fix (and `types/triggers.rs` is outside this - // change), but it IS this row's to disclose: none of the six is in the + // change), but it IS this row's to disclose: none of the seven is in the // relieved list above, so the reverse containment below covers 165 of the - // enum's 171 variants and this constant names the remaining six. + // enum's 172 variants and this constant names the remaining seven. // Sorted, and compared as a SET: this row's subject is which variants are // undecodable, not where they sit in the declaration. Pinning declaration // order would red this `ai_support` row on a no-op reordering of // `TriggerMode` — a failure that says nothing about either module. - const UNCONSTRUCTIBLE: [&str; 6] = [ + const UNCONSTRUCTIBLE: [&str; 7] = [ "Copied", // no `FromStr` arm "Explored", // no `FromStr` arm "HauntedCreatureDies", // no `FromStr` arm "KeywordAbilityActivated", // payload "Planeswalked", // payload - "Unknown", // payload + // Payload, and deliberately without a `FromStr` arm: Forge has no + // Saga-chapter meta-trigger type, so there is no Forge string to + // decode from. Inventing one would fabricate a mapping. + "SagaChapterAbility", // payload + "Unknown", // payload ]; let mut undecodable = Vec::new(); diff --git a/crates/engine/src/analysis/ability_graph.rs b/crates/engine/src/analysis/ability_graph.rs index 453a780239..aeb877b43d 100644 --- a/crates/engine/src/analysis/ability_graph.rs +++ b/crates/engine/src/analysis/ability_graph.rs @@ -1185,6 +1185,9 @@ fn trigger_axis(trig: &TriggerDefinition) -> Option { | TriggerMode::RoomEntered | TriggerMode::PlanarDice | TriggerMode::Planeswalked { .. } + // CR 714.2a: a Saga-chapter meta-trigger consumes another permanent's + // chapter-ability lifecycle; no modeled producer axis. + | TriggerMode::SagaChapterAbility { .. } | TriggerMode::ChaosEnsues | TriggerMode::RolledDie | TriggerMode::RolledDieOnce diff --git a/crates/engine/src/game/game_object.rs b/crates/engine/src/game/game_object.rs index d95807ec68..92298f8b02 100644 --- a/crates/engine/src/game/game_object.rs +++ b/crates/engine/src/game/game_object.rs @@ -2911,24 +2911,62 @@ impl GameObject { self.owner == player && self.zone == Zone::Graveyard && self.is_represented_by_a_card() } + /// CR 714.2a: Every chapter number this Saga's chapter abilities are keyed + /// to. A chapter ability is not a distinct AST concept — it is a + /// lore-counter threshold trigger on the Saga itself (see + /// `parser::oracle_saga`), so the chapter numbers are exactly those + /// thresholds. Empty for a non-Saga. + /// + /// Lore-scoped per CR 714.2a: a Saga may also carry a thresholded trigger on + /// some OTHER counter type, and that trigger is not a chapter ability. + /// + /// Structural scan of the Saga's own triggers — intrinsic to the card, not + /// subject to functioning gates. `iter_all` is pub(crate). + pub fn saga_chapter_numbers(&self) -> impl Iterator + '_ { + self.card_types + .subtypes + .iter() + .any(|subtype| subtype == "Saga") + .then(|| self.trigger_definitions.iter_all()) + .into_iter() + .flatten() + .filter_map(|entry| entry.definition.counter_filter.as_ref()) + .filter(|filter| filter.counter_type == CounterType::Lore) + .filter_map(|filter| filter.threshold) + } + /// CR 714.1: Returns the final chapter number for a Saga, or None if not a Saga. - /// Derived at runtime from the maximum threshold in the trigger definitions' counter filters. pub fn final_chapter_number(&self) -> Option { - if !self.card_types.subtypes.iter().any(|s| s == "Saga") { - return None; - } - // Structural scan of this Saga's own triggers — intrinsic to the - // card, not subject to functioning gates. `iter_all` is pub(crate). - self.trigger_definitions + self.saga_chapter_numbers().max() + } + + /// CR 714.2a + CR 714.4: Identify one of this Saga's own chapter abilities + /// by the exact trigger occurrence that produced it, returning + /// `(chapter_number, final_chapter_number)`. + /// + /// A chapter ability is not a distinct AST concept — it is a lore-counter + /// threshold trigger on the Saga itself (see `parser::oracle_saga`). This is + /// the single authority that maps a fired trigger occurrence back to its + /// printed chapter number, so callers never have to re-derive it from the + /// lore count (wrong under Read Ahead and multi-counter additions, which + /// cross several thresholds at once) or from the `"Chapter {n}"` description + /// string. + /// + /// Returns `None` for a non-Saga, or for an occurrence that is not one of + /// this permanent's lore-threshold triggers. + pub fn saga_chapter_for_occurrence( + &self, + occurrence: &TriggerDefinitionOccurrenceRef, + ) -> Option<(u32, u32)> { + let final_chapter = self.final_chapter_number()?; + let chapter = self + .trigger_definitions .iter_all() - .filter_map(|entry| { - entry - .definition - .counter_filter - .as_ref() - .and_then(|f| f.threshold) - }) - .max() + .find(|entry| &entry.occurrence == occurrence) + .and_then(|entry| entry.definition.counter_filter.as_ref()) + .filter(|filter| filter.counter_type == CounterType::Lore) + .and_then(|filter| filter.threshold)?; + Some((chapter, final_chapter)) } /// CR 702.51a: Whether this object can be tapped for convoke mana. diff --git a/crates/engine/src/game/log.rs b/crates/engine/src/game/log.rs index e64c94a9c0..f5a8c0bafd 100644 --- a/crates/engine/src/game/log.rs +++ b/crates/engine/src/game/log.rs @@ -152,6 +152,10 @@ fn importance(event: &GameEvent) -> LogImportance { | GameEvent::BecomesPlotted { .. } | GameEvent::StackPushed { .. } | GameEvent::StackResolved { .. } + // CR 714.2a: bookkeeping the engine publishes so meta-triggers can + // observe a chapter ability finishing; the chapter's own effects carry + // the player-visible signal. + | GameEvent::SagaChapterAbilityResolved { .. } | GameEvent::DamageCleared { .. } | GameEvent::ResolutionHalted { .. } | GameEvent::DamagePrevented { .. } @@ -296,6 +300,9 @@ fn tone(event: &GameEvent) -> LogTone { | GameEvent::LandPlayed { .. } | GameEvent::StackPushed { .. } | GameEvent::StackResolved { .. } + // CR 714.2a: neither good nor bad news on its own — the drain or token + // the observing trigger produces is what carries tone. + | GameEvent::SagaChapterAbilityResolved { .. } | GameEvent::Discarded { .. } | GameEvent::Cycled { .. } | GameEvent::DamageCleared { .. } @@ -416,6 +423,10 @@ fn should_exclude_event(event: &GameEvent, state: &GameState) -> bool { // StackPushed/StackResolved are low-signal bookkeeping — // the meaningful info is in SpellCast/AbilityActivated and EffectResolved GameEvent::StackPushed { .. } | GameEvent::StackResolved { .. } => true, + // CR 714.2a: the chapter-resolution notification exists so meta-triggers + // can observe it; the player already saw the chapter ability itself + // resolve. Same low-signal bookkeeping class as StackResolved. + GameEvent::SagaChapterAbilityResolved { .. } => true, _ => false, } } @@ -491,6 +502,8 @@ fn categorize(event: &GameEvent) -> LogCategory { | GameEvent::KeywordAbilityActivated { .. } | GameEvent::StackPushed { .. } | GameEvent::StackResolved { .. } + // CR 714.2a: a chapter ability finishing resolution is a stack event. + | GameEvent::SagaChapterAbilityResolved { .. } | GameEvent::SpellCountered { .. } => LogCategory::Stack, GameEvent::AttackersDeclared { .. } @@ -797,6 +810,10 @@ fn format_segments(event: &GameEvent, state: &GameState) -> Vec { vec![card_seg(state, *object_id), text(" resolves")] } + // CR 714.2a: filtered out by `is_low_signal` above — the chapter + // ability's own resolution line already told the player what happened. + GameEvent::SagaChapterAbilityResolved { .. } => vec![], + GameEvent::SpellCountered { object_id, countered_by, diff --git a/crates/engine/src/game/public_state.rs b/crates/engine/src/game/public_state.rs index 0ad3146231..0c04acd605 100644 --- a/crates/engine/src/game/public_state.rs +++ b/crates/engine/src/game/public_state.rs @@ -415,6 +415,9 @@ pub fn mark_public_state_from_events(state: &mut GameState, events: &[GameEvent] | GameEvent::LandPlayed { .. } | GameEvent::StackPushed { .. } | GameEvent::StackResolved { .. } + // CR 714.2a: a notification consumed by triggers only; the chapter + // ability's own effects dirty whatever display state they touched. + | GameEvent::SagaChapterAbilityResolved { .. } | GameEvent::GameOver { .. } // CR 732.2: a halted-resolution notification dirties no display state. | GameEvent::ResolutionHalted { .. } diff --git a/crates/engine/src/game/stack.rs b/crates/engine/src/game/stack.rs index 8ce1bd9436..f6576b7386 100644 --- a/crates/engine/src/game/stack.rs +++ b/crates/engine/src/game/stack.rs @@ -990,6 +990,44 @@ pub(crate) fn bind_resolution_scope( true } +/// CR 714.2a + CR 714.4: The Saga-chapter identity of a stack entry that is +/// about to resolve, or `None` if the entry is not a Saga chapter ability. +/// +/// Captured BEFORE the ability executes: a chapter ability may move its own +/// Saga off the battlefield as its effect (Fable of the Mirror-Breaker III +/// exiles and returns it transformed; CR 714.4's sacrifice follows the final +/// chapter), and neither the chapter number nor the final chapter number can be +/// re-derived once the permanent is gone. +struct ResolvingSagaChapter { + saga_id: ObjectId, + controller: PlayerId, + chapter: u32, + final_chapter: u32, +} + +/// CR 714.2a: Classify an about-to-resolve stack entry as a Saga chapter +/// ability, via the exact trigger occurrence that fired rather than any +/// re-derivation from the Saga's current lore count. +fn resolving_saga_chapter(state: &GameState, entry: &StackEntry) -> Option { + let StackEntryKind::TriggeredAbility { + source_id, ability, .. + } = &entry.kind + else { + return None; + }; + let occurrence = &ability.trigger_definition_ref.as_ref()?.occurrence; + let (chapter, final_chapter) = state + .objects + .get(source_id)? + .saga_chapter_for_occurrence(occurrence)?; + Some(ResolvingSagaChapter { + saga_id: *source_id, + controller: entry.controller, + chapter, + final_chapter, + }) +} + /// CR 608.2: Resolve the top object on the stack. pub fn resolve_top(state: &mut GameState, events: &mut Vec) { // CR 603.3c + CR 603.3d: The top of the stack may be a trigger entry that @@ -1089,6 +1127,12 @@ pub fn resolve_top(state: &mut GameState, events: &mut Vec) { return; } + // CR 714.2a: Snapshot the Saga-chapter identity while the Saga is still + // reachable — the chapter ability's own effect may remove it. Only the + // success path below publishes it; a fizzle (CR 608.2b) or a failed + // intervening-if (CR 603.4) leaves the stack without resolving. + let saga_chapter = resolving_saga_chapter(state, &entry); + // Extract the resolved ability from the stack entry. `KeywordAction` is // handled by the early return above and never reaches this match. let (mut ability, is_spell, casting_variant, actual_mana_spent) = match &entry.kind { @@ -2494,6 +2538,20 @@ pub fn resolve_top(state: &mut GameState, events: &mut Vec) { events.push(GameEvent::StackResolved { object_id: entry.id, }); + // CR 714.2a + CR 608.2: This is the only exit from `resolve_top` on which a + // triggered ability actually RESOLVED — the fizzle, no-legal-target and + // failed-intervening-if paths returned earlier, each pushing their own + // `StackResolved`. Publishing the chapter-resolution event only here is what + // keeps "whenever the final chapter ability of a Saga you control resolves" + // (Narci, Fable Singer) from firing on a chapter ability that never did. + if let Some(chapter) = saga_chapter { + events.push(GameEvent::SagaChapterAbilityResolved { + saga_id: chapter.saga_id, + controller: chapter.controller, + chapter: chapter.chapter, + final_chapter: chapter.final_chapter, + }); + } // The popped object remains the resolving carrier through every typed // resolution frame, including a direct optional-choice frame. In particular, // a self-moving trigger needs that carrier to establish its CR 400.7j diff --git a/crates/engine/src/game/targeting.rs b/crates/engine/src/game/targeting.rs index 6db958ee01..776ad25e00 100644 --- a/crates/engine/src/game/targeting.rs +++ b/crates/engine/src/game/targeting.rs @@ -1565,6 +1565,11 @@ pub(crate) fn extract_source_from_event( // trigger fires from; `source_id` is the permanent tapped for mana. GameEvent::TappedForMana { source_id, .. } => Some(*source_id), GameEvent::CounterAdded { object_id, .. } => Some(*object_id), + // CR 608.2k + CR 714.2a: "that Saga" in "each opponent loses X life … + // where X is that Saga's mana value" (Narci, Fable Singer) is an + // untargeted back-reference to the object the trigger condition named — + // the Saga whose chapter ability resolved. + GameEvent::SagaChapterAbilityResolved { saga_id, .. } => Some(*saga_id), GameEvent::Evolved { object_id } => Some(*object_id), GameEvent::CounterRemoved { object_id, .. } => Some(*object_id), GameEvent::TokenCreated { object_id, .. } => Some(*object_id), diff --git a/crates/engine/src/game/trigger_index.rs b/crates/engine/src/game/trigger_index.rs index df47f99fb0..61205e92e9 100644 --- a/crates/engine/src/game/trigger_index.rs +++ b/crates/engine/src/game/trigger_index.rs @@ -238,6 +238,12 @@ pub(crate) fn keys_from_trigger_def(def: &TriggerDefinition) -> (Keys, bool) { | TriggerMode::CounterAddedOnce | TriggerMode::CounterAddedAll | TriggerMode::CounterTypeAddedAll => push(TriggerEventKey::CounterAdded), + // CR 714.2a + CR 714.4: a Saga-chapter meta-trigger's match shape is + // dynamic — "the final chapter" is derived from the OBSERVED Saga's own + // trigger set, not from anything statically on this trigger. Route to + // `unclassified` (the documented safety net for dynamic shapes); the + // three printed cards in the class make the consult cost irrelevant. + TriggerMode::SagaChapterAbility { .. } => return (keys, true), // CR 107.14: "Whenever you get one or more {E}" — energy uses the // player-counter event key, not the object-counter key. TriggerMode::CounterPlayerAddedAll => push(TriggerEventKey::PlayerCounterChanged), @@ -608,6 +614,9 @@ pub(crate) fn keys_from_event(event: &GameEvent, state: &GameState) -> Keys { GameEvent::DamagePrevented { .. } => push(TriggerEventKey::DamagePrevented), GameEvent::SpellCountered { .. } => {} GameEvent::CounterAdded { .. } => push(TriggerEventKey::CounterAdded), + // CR 714.2a: consumed only by `SagaChapterAbility { lifecycle: Resolved }` + // triggers, which live in the `unclassified` bucket. No key of its own. + GameEvent::SagaChapterAbilityResolved { .. } => {} GameEvent::Evolved { .. } => {} GameEvent::ObjectIntensified { .. } => {} GameEvent::CounterRemoved { .. } => push(TriggerEventKey::CounterRemoved), diff --git a/crates/engine/src/game/trigger_matchers.rs b/crates/engine/src/game/trigger_matchers.rs index ed2dc524f8..2775243bdd 100644 --- a/crates/engine/src/game/trigger_matchers.rs +++ b/crates/engine/src/game/trigger_matchers.rs @@ -10,7 +10,9 @@ use crate::types::events::{GameEvent, PlayerActionKind}; use crate::types::game_state::{GameState, TriggerSourceContext}; use crate::types::identifiers::ObjectId; use crate::types::player::PlayerId; -use crate::types::triggers::{PlaneswalkRole, TriggerMode}; +use crate::types::triggers::{ + AbilityLifecyclePoint, PlaneswalkRole, SagaChapterScope, TriggerMode, +}; use crate::types::zones::Zone; use super::triggers::TriggerMatcher; @@ -126,6 +128,10 @@ pub fn trigger_matcher(mode: TriggerMode) -> Option { // matcher that reads the `PlaneswalkRole` off the trigger's mode — `From` // and `To` bind the source to that endpoint, `Any` is source-independent. TriggerMode::Planeswalked { .. } => match_planeswalked, + // CR 714.2a + CR 714.4: "whenever the final chapter ability of a Saga + // you control triggers/resolves" — one matcher reads both axes off the + // mode. + TriggerMode::SagaChapterAbility { .. } => match_saga_chapter_ability, // CR 904.9 / CR 701.32b: "When you set this scheme in motion" fires for // the scheme set in motion. TriggerMode::SetInMotion => match_set_in_motion, @@ -403,6 +409,20 @@ pub fn build_trigger_registry() -> HashMap { ] { r.insert(TriggerMode::Planeswalked { role }, match_planeswalked); } + // CR 714.2a + CR 714.4: one matcher for every (chapter, lifecycle) pair; it + // reads both axes off the trigger's mode. Each pair is a distinct registry + // key (both participate in `TriggerMode`'s Hash/Eq). + for chapter in [SagaChapterScope::Any, SagaChapterScope::Final] { + for lifecycle in [ + AbilityLifecyclePoint::Triggered, + AbilityLifecyclePoint::Resolved, + ] { + r.insert( + TriggerMode::SagaChapterAbility { chapter, lifecycle }, + match_saga_chapter_ability, + ); + } + } // CR 904.9 / CR 701.32b / CR 701.33b: Archenemy scheme triggers r.insert(TriggerMode::SetInMotion, match_set_in_motion); r.insert(TriggerMode::Abandoned, match_abandoned); @@ -1064,6 +1084,9 @@ fn count_matching_trigger_event_subjects( | GameEvent::StartingPlayerContest { .. } | GameEvent::Foretold { .. } | GameEvent::BecameForetold { .. } + // CR 714.2a: names a Saga, but chapter-ability meta-triggers are never + // batched ("one or more" has no reading over chapter resolutions). + | GameEvent::SagaChapterAbilityResolved { .. } | GameEvent::HiddenSearchViewed { .. } => 0, } } @@ -2205,6 +2228,105 @@ pub(super) fn match_counter_added( } } +/// CR 714.2a + CR 714.4: "Whenever [the final] chapter ability of a Saga you +/// control triggers/resolves" (Historian's Boon, Narci, Fable Singer, Tom +/// Bombadil). +/// +/// The observed Saga is constrained by the trigger's ordinary `valid_card` +/// filter ("a Saga you control"), matched with last-known information: a Saga +/// that reached its final chapter is sacrificed by CR 704.5s the moment its +/// chapter ability leaves the stack, and a chapter ability may remove the Saga +/// itself (Fable of the Mirror-Breaker III), so the permanent frequently no +/// longer exists when this trigger is collected. +/// +/// The two lifecycle points read different events because they ARE different +/// events (CR 603.2 vs CR 608.2): +/// +/// * `Triggered` — chapter abilities have no event of their own; they *are* the +/// Saga's lore-counter threshold triggers, so the trigger event is the same +/// `CounterAdded { Lore }` that `match_counter_added` consumes. +/// * `Resolved` — `SagaChapterAbilityResolved`, published by `stack.rs` only on +/// the path where a triggered ability genuinely finished resolving. +pub(super) fn match_saga_chapter_ability( + event: &GameEvent, + trigger: &TriggerDefinition, + source_context: &TriggerSourceContext, + state: &GameState, +) -> bool { + // The registry only routes `SagaChapterAbility` triggers here, but read the + // axes off the mode rather than assuming them. + let TriggerMode::SagaChapterAbility { chapter, lifecycle } = &trigger.mode else { + return false; + }; + + match (lifecycle, event) { + ( + AbilityLifecyclePoint::Resolved, + GameEvent::SagaChapterAbilityResolved { + saga_id, + chapter: resolved_chapter, + final_chapter, + .. + }, + ) => { + if !valid_card_matches_with_lki(trigger, state, *saga_id, source_context) { + return false; + } + match chapter { + SagaChapterScope::Any => true, + SagaChapterScope::Final => resolved_chapter == final_chapter, + } + } + ( + AbilityLifecyclePoint::Triggered, + // CR 714.2a: the chapter ability's own trigger event. `actor` (who + // placed the counter) is irrelevant — CR 714.3c's turn-based action + // and any effect that adds lore both make chapter abilities trigger. + GameEvent::CounterAdded { + object_id, + counter_type, + count, + .. + }, + ) => { + if *counter_type != crate::types::counter::CounterType::Lore { + return false; + } + if !valid_card_matches_with_lki(trigger, state, *object_id, source_context) { + return false; + } + // CR 714.2a: a chapter ability triggers when the lore count crosses + // its chapter number — `previous < n <= current`. The same crossing + // arithmetic `match_counter_added` performs for the Saga's own + // chapter triggers, evaluated here against the observed Saga. + let Some(saga) = state.objects.get(object_id) else { + return false; + }; + let current = saga + .counters + .get(&crate::types::counter::CounterType::Lore) + .copied() + .unwrap_or(0); + let previous = current.saturating_sub(*count); + let crosses = |n: u32| previous < n && n <= current; + match chapter { + // CR 714.2c: a single addition can cross several chapter + // thresholds at once, triggering that many chapter abilities. + // This event-driven matcher fires once for such a batch. No + // printed card observes `Any` today (all three cards in the + // class say "the final chapter ability"), so the parser never + // produces this combination against a real card; the arm exists + // so the axis is total rather than silently absent. + SagaChapterScope::Any => saga.saga_chapter_numbers().any(crosses), + // A lore counter added to a Saga already past its final chapter + // (proliferate before CR 704.5s sacrifices it) crosses nothing. + SagaChapterScope::Final => saga.final_chapter_number().is_some_and(crosses), + } + } + _ => false, + } +} + pub(super) fn match_evolved( event: &GameEvent, trigger: &TriggerDefinition, diff --git a/crates/engine/src/parser/oracle_nom/quantity.rs b/crates/engine/src/parser/oracle_nom/quantity.rs index 9a5a432c9e..9fb7f54618 100644 --- a/crates/engine/src/parser/oracle_nom/quantity.rs +++ b/crates/engine/src/parser/oracle_nom/quantity.rs @@ -4700,6 +4700,15 @@ fn parse_object_possessive_scope(input: &str) -> OracleResult<'_, ObjectScope> { value(ObjectScope::Target, tag("target creature's")), value(ObjectScope::Target, tag("target permanent's")), value(ObjectScope::EventSource, tag("that spell's")), + // CR 608.2k + CR 714.2a: "that Saga's mana value" (Narci, Fable Singer). + // Same shape as the "that spell's" arm above and bound the same way: an + // untargeted back-reference to the object the TRIGGER CONDITION named, + // not a threaded target. The "that 's" arms below bind to + // `Target` because their referent is a target this ability announced; + // a Saga-chapter meta-trigger announces none, so `EventSource` — the + // Saga carried by `GameEvent::SagaChapterAbilityResolved` — is the only + // referent that exists. + value(ObjectScope::EventSource, tag("that saga's")), // CR 202.3 + CR 608.2c: "that card's" — the type-qualified anaphor // for the exile-until hit ("that nonland card's mana value", Lady Loki). // The type qualifier is REQUIRED, not optional: a bare "that card's" is diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 8e57131a91..14127c9031 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -64,7 +64,9 @@ use crate::types::events::{ClashResult, PlayerActionKind}; use crate::types::keywords::{Keyword, KeywordKind}; use crate::types::mana::{ManaColor, ManaType}; use crate::types::phase::Phase; -use crate::types::triggers::{AttackTargetFilter, PlaneswalkRole, TriggerMode}; +use crate::types::triggers::{ + AbilityLifecyclePoint, AttackTargetFilter, PlaneswalkRole, SagaChapterScope, TriggerMode, +}; use crate::types::zones::Zone; use std::str::FromStr; @@ -9223,6 +9225,15 @@ pub(crate) fn parse_trigger_condition( return result; } + // CR 714.2a: "whenever the final chapter ability of a Saga you control + // resolves". Dispatched before subject decomposition — the grammatical + // subject of this clause is an ABILITY, not a permanent, so the generic + // subject/event-verb path would mis-bind "chapter ability" as the object + // filter. + if let Some(result) = try_parse_saga_chapter_ability_trigger(&lower) { + return result; + } + // --- Phase triggers: "At the beginning of..." --- if let Some(result) = try_parse_phase_trigger(&lower) { return result; @@ -17706,6 +17717,67 @@ fn try_parse_discard_trigger( /// "Another" self-exclusion (e.g., Mazirek's "another permanent") is carried by /// `FilterProp::Another` from `parse_trigger_subject`; the runtime matcher enforces /// it via `FilterProp::Another` → `object_id != source.id` in `filter.rs`. +/// CR 714.2a + CR 714.4: "Whenever [the final] chapter ability of \ +/// triggers/resolves" — Historian's Boon, Narci Fable Singer, Tom Bombadil. +/// +/// The class varies over two independent axes, so each is one `alt()` composed +/// in sequence rather than an enumeration of phrasings: WHICH chapter ability is +/// observed (`the final chapter ability` / `a chapter ability`) and WHICH point +/// of its lifecycle fires the trigger (`triggers` / `resolves`). The observed +/// Saga flows through the shared `parse_trigger_subject` building block, so +/// "a Saga you control", a bare "a Saga", and an opponent-scoped subject all +/// work without any Saga-specific subject grammar here. +fn try_parse_saga_chapter_ability_trigger(lower: &str) -> Option<(TriggerMode, TriggerDefinition)> { + let (event, ()) = alt(( + value((), tag::<_, _, OracleError<'_>>("whenever ")), + value((), tag("when ")), + )) + .parse(lower) + .ok()?; + + // "the final chapter ability of " / "a chapter ability of ". The article is + // optional because it is not part of the chapter-scope distinction. + let (subject_text, (_, chapter, _)) = ( + opt(alt(( + tag::<_, _, OracleError<'_>>("the "), + tag("a "), + tag("an "), + ))), + alt(( + value(SagaChapterScope::Final, tag("final chapter ability")), + value(SagaChapterScope::Any, tag("chapter ability")), + )), + tag(" of "), + ) + .parse(event) + .ok()?; + + let (filter, remainder) = parse_trigger_subject(subject_text, &mut ParseContext::default()); + + let (remainder, lifecycle) = alt(( + value( + AbilityLifecyclePoint::Triggered, + tag::<_, _, OracleError<'_>>("triggers"), + ), + value(AbilityLifecyclePoint::Resolved, tag("resolves")), + )) + .parse(remainder.trim()) + .ok()?; + // Anything left over is grammar this combinator did not model; fail closed + // rather than silently accepting a clause whose remainder carries meaning. + if !remainder.trim().is_empty() { + return None; + } + + let mode = TriggerMode::SagaChapterAbility { chapter, lifecycle }; + let mut def = make_base(); + def.mode = mode.clone(); + // CR 714.1: the observed Saga is an ordinary trigger subject — the runtime + // matcher applies this filter to the Saga carried by the event. + def.valid_card = Some(filter); + Some((mode, def)) +} + fn try_parse_sacrifice_trigger( lower: &str, make_base: &dyn Fn() -> TriggerDefinition, @@ -18754,3 +18826,129 @@ mod cost_x_totality_guard_tests { ); } } + +/// CR 714.2a + CR 714.4: the Saga-chapter meta-trigger building block — +/// "whenever [the final] chapter ability of \ triggers/resolves". +/// +/// These exercise the two axes the combinator composes (chapter scope × +/// lifecycle point) and the shared subject grammar, not one card's printed line. +#[cfg(test)] +mod saga_chapter_ability_trigger_tests { + use super::parse_trigger_line; + use crate::parser::oracle_nom::quantity::parse_quantity_ref; + use crate::types::ability::{ + ControllerRef, ObjectScope, QuantityRef, TargetFilter, TypeFilter, + }; + use crate::types::triggers::{AbilityLifecyclePoint, SagaChapterScope, TriggerMode}; + + fn saga_filter(filter: Option<&TargetFilter>) -> (Vec, Option) { + let Some(TargetFilter::Typed(typed)) = filter else { + panic!("expected a typed Saga subject filter, got {filter:?}"); + }; + (typed.type_filters.clone(), typed.controller.clone()) + } + + /// Narci, Fable Singer / Tom Bombadil — the resolution half of the class. + #[test] + fn final_chapter_resolves() { + let def = parse_trigger_line( + "Whenever the final chapter ability of a Saga you control resolves, draw a card.", + "Test", + ); + assert_eq!( + def.mode, + TriggerMode::SagaChapterAbility { + chapter: SagaChapterScope::Final, + lifecycle: AbilityLifecyclePoint::Resolved, + } + ); + assert_eq!( + saga_filter(def.valid_card.as_ref()), + ( + vec![TypeFilter::Subtype("Saga".to_string())], + Some(ControllerRef::You) + ) + ); + } + + /// Historian's Boon — the same clause on the other end of the lifecycle + /// axis. Only the verb differs, so only the lifecycle point may differ. + #[test] + fn final_chapter_triggers() { + let def = parse_trigger_line( + "Whenever the final chapter ability of a Saga you control triggers, draw a card.", + "Test", + ); + assert_eq!( + def.mode, + TriggerMode::SagaChapterAbility { + chapter: SagaChapterScope::Final, + lifecycle: AbilityLifecyclePoint::Triggered, + } + ); + assert_eq!( + saga_filter(def.valid_card.as_ref()), + ( + vec![TypeFilter::Subtype("Saga".to_string())], + Some(ControllerRef::You) + ) + ); + } + + /// The chapter axis is a real parameter, not a hardcoded "final": an + /// unqualified chapter ability parses rather than falling out of the + /// grammar. + #[test] + fn any_chapter_resolves() { + let def = parse_trigger_line( + "Whenever a chapter ability of a Saga you control resolves, draw a card.", + "Test", + ); + assert_eq!( + def.mode, + TriggerMode::SagaChapterAbility { + chapter: SagaChapterScope::Any, + lifecycle: AbilityLifecyclePoint::Resolved, + } + ); + } + + /// The observed Saga flows through the shared subject grammar, so an + /// unscoped subject keeps the subtype and drops only the controller + /// constraint. + #[test] + fn unscoped_saga_subject() { + let def = parse_trigger_line( + "Whenever the final chapter ability of a Saga resolves, draw a card.", + "Test", + ); + assert_eq!( + def.mode, + TriggerMode::SagaChapterAbility { + chapter: SagaChapterScope::Final, + lifecycle: AbilityLifecyclePoint::Resolved, + } + ); + assert_eq!( + saga_filter(def.valid_card.as_ref()), + (vec![TypeFilter::Subtype("Saga".to_string())], None) + ); + } + + /// CR 608.2k + CR 202.3: "that Saga's mana value" is an untargeted + /// back-reference to the object the trigger condition named, so it binds to + /// the event source — NOT to the target slot like the "that creature's" + /// sibling, whose referent is a target the ability announced. A Saga-chapter + /// meta-trigger announces none. + #[test] + fn that_sagas_mana_value_binds_the_event_source() { + let (rest, qty) = parse_quantity_ref("that saga's mana value").unwrap(); + assert_eq!(rest, ""); + assert_eq!( + qty, + QuantityRef::ObjectManaValue { + scope: ObjectScope::EventSource + } + ); + } +} diff --git a/crates/engine/src/types/events.rs b/crates/engine/src/types/events.rs index 89af542186..275e2762c4 100644 --- a/crates/engine/src/types/events.rs +++ b/crates/engine/src/types/events.rs @@ -1053,6 +1053,29 @@ pub enum GameEvent { #[serde(default)] actor: PlayerId, }, + /// CR 714.2a + CR 608.2: A Saga's chapter ability finished resolving. + /// + /// A chapter ability is not a distinct AST concept — it is the Saga's own + /// lore-counter threshold trigger (CR 714.2a). This event is the resolution + /// half of that lifecycle, which nothing else on the bus reports: + /// `StackResolved` is emitted for fizzles and failed intervening-ifs too, + /// and carries the stack entry id rather than the Saga. + /// + /// `chapter` and `final_chapter` are captured BEFORE the chapter ability + /// executes, because a chapter ability may remove its own Saga from the + /// battlefield as its effect (CR 714.4 / Fable of the Mirror-Breaker III), + /// after which neither number could be re-derived. + SagaChapterAbilityResolved { + /// The Saga permanent whose chapter ability resolved. + saga_id: ObjectId, + /// CR 109.5: controller of the resolved chapter ability. + controller: PlayerId, + /// CR 714.2a: the chapter number (lore threshold) that resolved. + chapter: u32, + /// CR 714.4: the Saga's greatest chapter number. `chapter == + /// final_chapter` is what makes this the *final* chapter ability. + final_chapter: u32, + }, /// Digital-only Alchemy (no CR entry): a card's intensity increased by /// `amount`. Emitted per affected card so consumers (triggers that watch for /// intensifying, frontend animation) can see exactly which cards changed. diff --git a/crates/engine/src/types/triggers.rs b/crates/engine/src/types/triggers.rs index 0e3d443a40..a39d671de0 100644 --- a/crates/engine/src/types/triggers.rs +++ b/crates/engine/src/types/triggers.rs @@ -218,6 +218,32 @@ pub enum PlaneswalkRole { Any, } +/// CR 714.2a + CR 714.4: which chapter ability of a Saga a meta-trigger listens +/// for. A Saga's chapter abilities are its lore-counter threshold triggers +/// (CR 714.2a); the *final* chapter ability is the one whose chapter number is +/// the greatest among them (CR 714.4 — reaching it is what makes the Saga's +/// controller sacrifice it). +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +pub enum SagaChapterScope { + /// Any chapter ability of the Saga. + Any, + /// CR 714.4: only the final (highest-numbered) chapter ability. + Final, +} + +/// CR 603.2 + CR 608.2: the point in another ability's lifecycle that a +/// meta-trigger observes. The two points are distinct events with distinct +/// timing consequences: an ability that triggers may still be countered +/// (CR 701.5) or have its intervening-if fail (CR 603.4) and therefore never +/// resolve. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +pub enum AbilityLifecyclePoint { + /// CR 603.2: the observed ability's trigger condition was met. + Triggered, + /// CR 608.2: the observed ability finished resolving. + Resolved, +} + /// All trigger modes from Forge's TriggerType enum (CR 603). /// /// Triggered abilities have a trigger condition and an effect, written as @@ -598,6 +624,21 @@ pub enum TriggerMode { /// `game::haunt::match_haunted_creature_dies`. HauntedCreatureDies, + /// CR 714.2a + CR 714.4: a meta-trigger on another permanent's Saga chapter + /// abilities — "whenever the final chapter ability of a Saga you control + /// resolves" (Narci, Fable Singer; Tom Bombadil) / "… triggers" + /// (Historian's Boon). Parameterized on the two independent axes the class + /// varies over: WHICH chapter ability is observed (`chapter`) and WHICH + /// point of that ability's lifecycle fires this trigger (`lifecycle`). + /// + /// The Saga itself is constrained by the trigger's ordinary `valid_card` + /// filter ("a Saga you control"), so no Saga-specific filter axis is needed + /// here. + SagaChapterAbility { + chapter: SagaChapterScope, + lifecycle: AbilityLifecyclePoint, + }, + /// Fallback for unrecognized trigger mode strings. Unknown(String), } diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 7fe6c1febd..5362e4900d 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -817,6 +817,7 @@ mod mutable_pupa_perpetual_keyword_mirror; mod mycoloth_upkeep_trigger; mod myrkul_crew_phase1_incarnation; mod mystic_forge_regression; +mod narci_fable_singer_final_chapter_drain; mod narset_jeskai_waymaster_draw_spells_cast; mod natural_balance; mod necrodominance_pay_any_life_draw; diff --git a/crates/engine/tests/integration/narci_fable_singer_final_chapter_drain.rs b/crates/engine/tests/integration/narci_fable_singer_final_chapter_drain.rs new file mode 100644 index 0000000000..a45312f5a7 --- /dev/null +++ b/crates/engine/tests/integration/narci_fable_singer_final_chapter_drain.rs @@ -0,0 +1,254 @@ +//! CR 714.2a + CR 714.4 + CR 608.2k — Narci, Fable Singer's final-chapter +//! drain, driven end-to-end through the production Saga pipeline. +//! +//! Oracle: `Whenever the final chapter ability of a Saga you control resolves, +//! each opponent loses X life and you gain X life, where X is that Saga's mana +//! value.` +//! +//! What makes this worth a runtime test rather than a parser assertion: the +//! chapter number and the Saga's mana value are read at two different moments +//! and the Saga does not survive between them. CR 704.5s sacrifices a Saga as +//! soon as its final chapter ability leaves the stack, so by the time Narci's +//! own trigger resolves, "that Saga" is a last-known-information reference to a +//! permanent that no longer exists. A test that only checked the AST would pass +//! while X resolved to 0. + +use engine::game::scenario::{GameRunner, GameScenario, P0, P1}; +use engine::game::triggers::drain_order_triggers_with_identity; +use engine::types::actions::GameAction; +use engine::types::counter::CounterType; +use engine::types::game_state::WaitingFor; +use engine::types::identifiers::ObjectId; +use engine::types::mana::ManaCost; +use engine::types::phase::Phase; +use engine::types::zones::Zone; + +const NARCI_ORACLE: &str = "Lifelink\n\ +Whenever you sacrifice an enchantment, draw a card.\n\ +Whenever the final chapter ability of a Saga you control resolves, each opponent loses X life and you gain X life, where X is that Saga's mana value."; + +/// A two-chapter Saga whose chapters are inert with respect to life totals, so +/// the only life change in the test is Narci's drain. +const SAGA_ORACLE: &str = "I — Create a 1/1 white Soldier creature token.\n\ +II — Create a 1/1 white Soldier creature token."; + +fn lore_count(runner: &GameRunner, saga_id: ObjectId) -> u32 { + runner + .state() + .objects + .get(&saga_id) + .and_then(|obj| obj.counters.get(&CounterType::Lore).copied()) + .unwrap_or(0) +} + +/// Park the game at the end of P0's turn so the next `advance_to_phase` walks +/// through P1's turn and back into a fresh P0 precombat main — the CR 714.3c +/// turn-based action that adds the Saga's next lore counter. +fn park_for_next_p0_precombat_main(runner: &mut GameRunner) { + let state = runner.state_mut(); + state.turn_number = 1; + state.active_player = P0; + state.phase = Phase::End; + state.priority_player = P0; + state.waiting_for = WaitingFor::Priority { player: P0 }; +} + +/// Resolve everything currently on the stack, answering trigger-order prompts. +fn drain_stack(runner: &mut GameRunner) { + for _ in 0..64 { + if matches!(runner.state().waiting_for, WaitingFor::OrderTriggers { .. }) { + drain_order_triggers_with_identity(runner.state_mut()); + continue; + } + if runner.state().stack.is_empty() { + break; + } + if matches!(runner.state().waiting_for, WaitingFor::Priority { .. }) { + let _ = runner.act(GameAction::PassPriority); + let _ = runner.act(GameAction::PassPriority); + } else { + break; + } + } +} + +#[test] +fn narci_drains_for_the_sagas_mana_value_when_its_final_chapter_resolves() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + // Narci's sacrifice-an-enchantment trigger fires when CR 704.5s sacrifices + // the Saga; give P0 something to draw so that draw cannot end the game. + scenario.with_library_top(P0, &["Forest", "Forest", "Forest", "Forest"]); + // Both players must survive their draw steps while the Saga walks to its + // final chapter — an empty-library draw would end the game (CR 104.3c) + // before the chapter ability ever resolves. + scenario.with_library_top(P1, &["Forest", "Forest", "Forest", "Forest"]); + + scenario + .add_creature(P0, "Narci, Fable Singer", 3, 3) + .as_legendary() + .from_oracle_text(NARCI_ORACLE); + + // {3} — mana value 3, the amount the drain must move. + let saga_id = scenario + .add_creature(P0, "Test Saga", 0, 0) + .as_enchantment() + .with_subtypes(vec!["Saga"]) + .with_mana_cost(ManaCost::generic(3)) + .from_oracle_text(SAGA_ORACLE) + .id(); + // CR 714.3a: scenario seeding bypasses the ETB pipeline, so stand in for the + // lore counter the Saga would have entered with. The next precombat main's + // turn-based action (CR 714.3c) then takes it to its FINAL chapter. + scenario.with_counter(saga_id, CounterType::Lore, 1); + + let mut runner = scenario.build(); + + let p0_life_before = runner.state().players[P0.0 as usize].life; + let p1_life_before = runner.state().players[P1.0 as usize].life; + + // CR 714.3c: the next precombat main adds the second (final) lore counter. + park_for_next_p0_precombat_main(&mut runner); + runner.advance_to_phase(Phase::PreCombatMain); + runner.pass_both_players(); + runner.advance_to_phase(Phase::PreCombatMain); + + assert_eq!( + lore_count(&runner, saga_id), + 2, + "CR 714.3c must add the Saga's final lore counter" + ); + + drain_stack(&mut runner); + + // CR 704.5s: the Saga is sacrificed once its final chapter ability has left + // the stack, so Narci's drain resolved against last-known information. + assert_ne!( + runner + .state() + .objects + .get(&saga_id) + .map(|saga| saga.zone) + .unwrap_or(Zone::Graveyard), + Zone::Battlefield, + "CR 704.5s must sacrifice the Saga after its final chapter resolves" + ); + + assert_eq!( + ( + runner.state().players[P0.0 as usize].life - p0_life_before, + runner.state().players[P1.0 as usize].life - p1_life_before, + ), + (3, -3), + "the final chapter ability resolving must drain each opponent for the Saga's mana value (3) and gain that much" + ); +} + +/// CR 714.4: a NON-final chapter ability resolving must not fire the trigger. +/// Without the chapter/final-chapter comparison this test drains on chapter I +/// as well, which would double Narci's output on every multi-chapter Saga. +#[test] +fn narci_does_not_drain_on_a_nonfinal_chapter() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.with_library_top(P0, &["Forest", "Forest", "Forest", "Forest"]); + // Both players must survive their draw steps while the Saga walks to its + // final chapter — an empty-library draw would end the game (CR 104.3c) + // before the chapter ability ever resolves. + scenario.with_library_top(P1, &["Forest", "Forest", "Forest", "Forest"]); + + scenario + .add_creature(P0, "Narci, Fable Singer", 3, 3) + .as_legendary() + .from_oracle_text(NARCI_ORACLE); + + // Three chapters: the counter added below reaches chapter II, not the final + // chapter III. + let saga_id = scenario + .add_creature(P0, "Long Test Saga", 0, 0) + .as_enchantment() + .with_subtypes(vec!["Saga"]) + .with_mana_cost(ManaCost::generic(3)) + .from_oracle_text( + "I — Create a 1/1 white Soldier creature token.\n\ +II — Create a 1/1 white Soldier creature token.\n\ +III — Create a 1/1 white Soldier creature token.", + ) + .id(); + // CR 714.3a stand-in, as above: the advance below reaches chapter II. + scenario.with_counter(saga_id, CounterType::Lore, 1); + + let mut runner = scenario.build(); + let p0_life_before = runner.state().players[P0.0 as usize].life; + let p1_life_before = runner.state().players[P1.0 as usize].life; + + park_for_next_p0_precombat_main(&mut runner); + runner.advance_to_phase(Phase::PreCombatMain); + runner.pass_both_players(); + runner.advance_to_phase(Phase::PreCombatMain); + + assert_eq!(lore_count(&runner, saga_id), 2); + drain_stack(&mut runner); + + // CR 704.5s: chapter II left the stack and the Saga survived, so a chapter + // ability really did resolve here — the life assertion below is about the + // final-chapter comparison, not about nothing having happened. + assert_eq!( + runner.state().objects[&saga_id].zone, + Zone::Battlefield, + "a three-chapter Saga is not sacrificed after chapter II" + ); + assert_eq!( + ( + runner.state().players[P0.0 as usize].life, + runner.state().players[P1.0 as usize].life, + ), + (p0_life_before, p1_life_before), + "chapter II of a three-chapter Saga is not the final chapter ability" + ); +} + +/// CR 603.2: the other end of the lifecycle axis — Historian's Boon observes the +/// final chapter ability *triggering*, which is the Saga's own lore-counter +/// threshold crossing, not a resolution. Narci's clause is identical except for +/// that verb, so the two share one trigger mode and one matcher; this pins the +/// half Narci does not exercise. +#[test] +fn a_final_chapter_triggers_observer_fires_on_the_lore_crossing() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.with_library_top(P0, &["Forest", "Forest", "Forest", "Forest"]); + scenario.with_library_top(P1, &["Forest", "Forest", "Forest", "Forest"]); + + scenario + .add_creature(P0, "Chapter Watcher", 2, 2) + .from_oracle_text( + "Whenever the final chapter ability of a Saga you control triggers, each opponent loses 1 life.", + ); + + let saga_id = scenario + .add_creature(P0, "Test Saga", 0, 0) + .as_enchantment() + .with_subtypes(vec!["Saga"]) + .with_mana_cost(ManaCost::generic(3)) + .from_oracle_text(SAGA_ORACLE) + .id(); + scenario.with_counter(saga_id, CounterType::Lore, 1); + + let mut runner = scenario.build(); + let p1_life_before = runner.state().players[P1.0 as usize].life; + + park_for_next_p0_precombat_main(&mut runner); + runner.advance_to_phase(Phase::PreCombatMain); + runner.pass_both_players(); + runner.advance_to_phase(Phase::PreCombatMain); + + assert_eq!(lore_count(&runner, saga_id), 2); + drain_stack(&mut runner); + + assert_eq!( + runner.state().players[P1.0 as usize].life - p1_life_before, + -1, + "the lore crossing onto the final chapter must fire a `triggers` observer exactly once" + ); +} From 70b74a061fd4a1eb943ac6ecfde5e59470cd9df4 Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Mon, 10 Aug 2026 20:08:43 -0500 Subject: [PATCH 2/5] fix(engine): verify the Saga CR citations and drop the unproven chapter scope Addresses review on #7224. Both blockers were real. Verified every citation against docs/MagicCompRules.txt, which had never been fetched in this worktree -- the original annotations were written from memory, which is exactly what CLAUDE.md's verification rule exists to prevent. The rules text says: 714.2 a chapter symbol is a keyword ability representing a triggered ability referred to as a chapter ability 714.2a defines ONLY the Roman numeral 714.2b "{rN}--[Effect]" means "When one or more lore counters are put onto this Saga, if the number of lore counters on it was less than N and became at least N, [effect]" 714.2d final chapter NUMBER is the greatest among a Saga's chapter abilities 714.2e final chapter ABILITY is the one carrying that number 714.4 the sacrifice state-based action So 714.2a was wrong everywhere it was used for the threshold-trigger claim, and 714.4 was wrong everywhere it was used to define the final chapter ability. Each citation is now the rule that actually states the claim. The same audit covers four PRE-EXISTING 714.2a citations on the shared counter-filter path (CounterTriggerFilter, match_counter_added and its CR 310.12b mirror), which make the identical error about the identical rule and sit directly on the code this change reads; leaving them would have re-earned the same review note. Removed SagaChapterScope. Its Any variant was speculative -- no printed card uses an unqualified "a chapter ability" -- and, worse, unmodelable here: CR 714.2b makes each chapter symbol its own triggered ability, so one lore-counter addition crossing several chapter numbers triggers that many chapter abilities. An observer of all of them owes one firing per crossed ability, which an event-keyed matcher reading a single CounterAdded cannot express. The original commit documented that shortfall in a comment instead of refusing it, which advertised a rules-incorrect branch through a public enum and parser grammar. The mode is now TriggerMode::FinalSagaChapterAbility { lifecycle }, the parser requires "final", and a new test pins the refusal: an unqualified clause stays Unknown (honestly coverage-red) rather than minting a trigger that under-fires. Restoring the scope needs an occurrence-level chapter event first, not a wider enum -- recorded on the variant. Also fail closed on an incarnation mismatch when classifying a resolving chapter ability (CR 400.7). source_id is storage identity: a Saga that left and re-entered can occupy the same id as a new object whose chapter numbers are not the ones this ability triggered from, and publishing those would misclassify a non-final chapter as final or drain for the wrong mana value. Co-Authored-By: Claude Opus 5 --- client/src/adapter/types.ts | 2 +- client/src/animation/eventNormalizer.ts | 2 +- .../src/ai_support/shortcut_efficacy.rs | 14 +-- crates/engine/src/analysis/ability_graph.rs | 4 +- crates/engine/src/game/game_object.rs | 37 +++--- crates/engine/src/game/log.rs | 10 +- crates/engine/src/game/public_state.rs | 2 +- crates/engine/src/game/stack.rs | 32 ++++-- crates/engine/src/game/targeting.rs | 2 +- crates/engine/src/game/trigger_index.rs | 18 +-- crates/engine/src/game/trigger_matchers.rs | 105 ++++++++---------- .../engine/src/parser/oracle_nom/quantity.rs | 2 +- crates/engine/src/parser/oracle_trigger.rs | 89 ++++++++------- crates/engine/src/types/ability.rs | 4 +- crates/engine/src/types/events.rs | 25 +++-- crates/engine/src/types/triggers.rs | 38 +++---- .../narci_fable_singer_final_chapter_drain.rs | 4 +- 17 files changed, 194 insertions(+), 196 deletions(-) diff --git a/client/src/adapter/types.ts b/client/src/adapter/types.ts index 327366ad5f..05ee1431aa 100644 --- a/client/src/adapter/types.ts +++ b/client/src/adapter/types.ts @@ -2460,7 +2460,7 @@ export type GameEvent = | { type: "LandPlayed"; data: { object_id: ObjectId; player_id: PlayerId; from_zone: Zone } } | { type: "StackPushed"; data: { object_id: ObjectId } } | { type: "StackResolved"; data: { object_id: ObjectId } } - // CR 714.2a: a Saga's chapter ability finished resolving. Bookkeeping the + // CR 714.2: a Saga's chapter ability finished resolving. Bookkeeping the // engine publishes for meta-triggers (Narci, Fable Singer); non-visual, since // the chapter ability's own effects already animate. | { type: "SagaChapterAbilityResolved"; data: { saga_id: ObjectId; controller: PlayerId; chapter: number; final_chapter: number } } diff --git a/client/src/animation/eventNormalizer.ts b/client/src/animation/eventNormalizer.ts index 0c16281076..ae7965e3c6 100644 --- a/client/src/animation/eventNormalizer.ts +++ b/client/src/animation/eventNormalizer.ts @@ -30,7 +30,7 @@ const NON_VISUAL_EVENTS = new Set([ "PermanentUntapped", "StackPushed", "StackResolved", - // CR 714.2a: same class as StackResolved — the chapter ability's own effects + // CR 714.2: same class as StackResolved — the chapter ability's own effects // are what the player sees; this event exists for meta-triggers. "SagaChapterAbilityResolved", "ReplacementApplied", diff --git a/crates/engine/src/ai_support/shortcut_efficacy.rs b/crates/engine/src/ai_support/shortcut_efficacy.rs index 3be28ab584..73031c4249 100644 --- a/crates/engine/src/ai_support/shortcut_efficacy.rs +++ b/crates/engine/src/ai_support/shortcut_efficacy.rs @@ -3384,16 +3384,16 @@ mod tests { // order would red this `ai_support` row on a no-op reordering of // `TriggerMode` — a failure that says nothing about either module. const UNCONSTRUCTIBLE: [&str; 7] = [ - "Copied", // no `FromStr` arm - "Explored", // no `FromStr` arm + "Copied", // no `FromStr` arm + "Explored", // no `FromStr` arm + // Payload, and deliberately without a `FromStr` arm: Forge has no + // final-chapter meta-trigger type, so there is no Forge string to + // decode from. Inventing one would fabricate a mapping. + "FinalSagaChapterAbility", // payload "HauntedCreatureDies", // no `FromStr` arm "KeywordAbilityActivated", // payload "Planeswalked", // payload - // Payload, and deliberately without a `FromStr` arm: Forge has no - // Saga-chapter meta-trigger type, so there is no Forge string to - // decode from. Inventing one would fabricate a mapping. - "SagaChapterAbility", // payload - "Unknown", // payload + "Unknown", // payload ]; let mut undecodable = Vec::new(); diff --git a/crates/engine/src/analysis/ability_graph.rs b/crates/engine/src/analysis/ability_graph.rs index aeb877b43d..c553402e97 100644 --- a/crates/engine/src/analysis/ability_graph.rs +++ b/crates/engine/src/analysis/ability_graph.rs @@ -1185,9 +1185,9 @@ fn trigger_axis(trig: &TriggerDefinition) -> Option { | TriggerMode::RoomEntered | TriggerMode::PlanarDice | TriggerMode::Planeswalked { .. } - // CR 714.2a: a Saga-chapter meta-trigger consumes another permanent's + // CR 714.2e: a final-chapter meta-trigger consumes another permanent's // chapter-ability lifecycle; no modeled producer axis. - | TriggerMode::SagaChapterAbility { .. } + | TriggerMode::FinalSagaChapterAbility { .. } | TriggerMode::ChaosEnsues | TriggerMode::RolledDie | TriggerMode::RolledDieOnce diff --git a/crates/engine/src/game/game_object.rs b/crates/engine/src/game/game_object.rs index 92298f8b02..4acf0e4c3d 100644 --- a/crates/engine/src/game/game_object.rs +++ b/crates/engine/src/game/game_object.rs @@ -2911,13 +2911,15 @@ impl GameObject { self.owner == player && self.zone == Zone::Graveyard && self.is_represented_by_a_card() } - /// CR 714.2a: Every chapter number this Saga's chapter abilities are keyed - /// to. A chapter ability is not a distinct AST concept — it is a - /// lore-counter threshold trigger on the Saga itself (see - /// `parser::oracle_saga`), so the chapter numbers are exactly those - /// thresholds. Empty for a non-Saga. + /// CR 714.2 + CR 714.2b: Every chapter number this Saga's chapter abilities + /// are keyed to. A chapter ability is not a distinct AST concept — CR 714.2b + /// defines a chapter symbol as "When one or more lore counters are put onto + /// this Saga, if the number of lore counters on it was less than N and + /// became at least N, [effect]", and `parser::oracle_saga` emits exactly + /// that: a lore-counter threshold trigger on the Saga itself. So the chapter + /// numbers are exactly those thresholds. Empty for a non-Saga. /// - /// Lore-scoped per CR 714.2a: a Saga may also carry a thresholded trigger on + /// Lore-scoped per CR 714.2b: a Saga may also carry a thresholded trigger on /// some OTHER counter type, and that trigger is not a chapter ability. /// /// Structural scan of the Saga's own triggers — intrinsic to the card, not @@ -2935,22 +2937,27 @@ impl GameObject { .filter_map(|filter| filter.threshold) } - /// CR 714.1: Returns the final chapter number for a Saga, or None if not a Saga. + /// CR 714.2d: "A Saga's final chapter number is the greatest value among + /// chapter abilities it has." Returns `None` for a non-Saga. + /// + /// CR 714.2d also assigns a final chapter number of 0 to a Saga with no + /// chapter abilities; this returns `None` there too, because every caller + /// uses `None` to mean "not a Saga to begin with" and CR 714.3c / CR 714.4 + /// both exempt a Saga with no chapter abilities from the lore turn-based + /// action and the sacrifice. pub fn final_chapter_number(&self) -> Option { self.saga_chapter_numbers().max() } - /// CR 714.2a + CR 714.4: Identify one of this Saga's own chapter abilities + /// CR 714.2 + CR 714.2d: Identify one of this Saga's own chapter abilities /// by the exact trigger occurrence that produced it, returning /// `(chapter_number, final_chapter_number)`. /// - /// A chapter ability is not a distinct AST concept — it is a lore-counter - /// threshold trigger on the Saga itself (see `parser::oracle_saga`). This is - /// the single authority that maps a fired trigger occurrence back to its - /// printed chapter number, so callers never have to re-derive it from the - /// lore count (wrong under Read Ahead and multi-counter additions, which - /// cross several thresholds at once) or from the `"Chapter {n}"` description - /// string. + /// This is the single authority that maps a fired trigger occurrence back to + /// its printed chapter number, so callers never have to re-derive it from + /// the lore count (wrong under Read Ahead, and wrong for a multi-counter + /// addition, which per CR 714.2b crosses several thresholds at once) or from + /// the `"Chapter {n}"` description string. /// /// Returns `None` for a non-Saga, or for an occurrence that is not one of /// this permanent's lore-threshold triggers. diff --git a/crates/engine/src/game/log.rs b/crates/engine/src/game/log.rs index f5a8c0bafd..1b01527c28 100644 --- a/crates/engine/src/game/log.rs +++ b/crates/engine/src/game/log.rs @@ -152,7 +152,7 @@ fn importance(event: &GameEvent) -> LogImportance { | GameEvent::BecomesPlotted { .. } | GameEvent::StackPushed { .. } | GameEvent::StackResolved { .. } - // CR 714.2a: bookkeeping the engine publishes so meta-triggers can + // CR 714.2: bookkeeping the engine publishes so meta-triggers can // observe a chapter ability finishing; the chapter's own effects carry // the player-visible signal. | GameEvent::SagaChapterAbilityResolved { .. } @@ -300,7 +300,7 @@ fn tone(event: &GameEvent) -> LogTone { | GameEvent::LandPlayed { .. } | GameEvent::StackPushed { .. } | GameEvent::StackResolved { .. } - // CR 714.2a: neither good nor bad news on its own — the drain or token + // CR 714.2: neither good nor bad news on its own — the drain or token // the observing trigger produces is what carries tone. | GameEvent::SagaChapterAbilityResolved { .. } | GameEvent::Discarded { .. } @@ -423,7 +423,7 @@ fn should_exclude_event(event: &GameEvent, state: &GameState) -> bool { // StackPushed/StackResolved are low-signal bookkeeping — // the meaningful info is in SpellCast/AbilityActivated and EffectResolved GameEvent::StackPushed { .. } | GameEvent::StackResolved { .. } => true, - // CR 714.2a: the chapter-resolution notification exists so meta-triggers + // CR 714.2: the chapter-resolution notification exists so meta-triggers // can observe it; the player already saw the chapter ability itself // resolve. Same low-signal bookkeeping class as StackResolved. GameEvent::SagaChapterAbilityResolved { .. } => true, @@ -502,7 +502,7 @@ fn categorize(event: &GameEvent) -> LogCategory { | GameEvent::KeywordAbilityActivated { .. } | GameEvent::StackPushed { .. } | GameEvent::StackResolved { .. } - // CR 714.2a: a chapter ability finishing resolution is a stack event. + // CR 714.2: a chapter ability finishing resolution is a stack event. | GameEvent::SagaChapterAbilityResolved { .. } | GameEvent::SpellCountered { .. } => LogCategory::Stack, @@ -810,7 +810,7 @@ fn format_segments(event: &GameEvent, state: &GameState) -> Vec { vec![card_seg(state, *object_id), text(" resolves")] } - // CR 714.2a: filtered out by `is_low_signal` above — the chapter + // CR 714.2: filtered out by `is_low_signal` above — the chapter // ability's own resolution line already told the player what happened. GameEvent::SagaChapterAbilityResolved { .. } => vec![], diff --git a/crates/engine/src/game/public_state.rs b/crates/engine/src/game/public_state.rs index 0c04acd605..3a11525f24 100644 --- a/crates/engine/src/game/public_state.rs +++ b/crates/engine/src/game/public_state.rs @@ -415,7 +415,7 @@ pub fn mark_public_state_from_events(state: &mut GameState, events: &[GameEvent] | GameEvent::LandPlayed { .. } | GameEvent::StackPushed { .. } | GameEvent::StackResolved { .. } - // CR 714.2a: a notification consumed by triggers only; the chapter + // CR 714.2: a notification consumed by triggers only; the chapter // ability's own effects dirty whatever display state they touched. | GameEvent::SagaChapterAbilityResolved { .. } | GameEvent::GameOver { .. } diff --git a/crates/engine/src/game/stack.rs b/crates/engine/src/game/stack.rs index f6576b7386..4ddb5a4aa9 100644 --- a/crates/engine/src/game/stack.rs +++ b/crates/engine/src/game/stack.rs @@ -990,14 +990,14 @@ pub(crate) fn bind_resolution_scope( true } -/// CR 714.2a + CR 714.4: The Saga-chapter identity of a stack entry that is +/// CR 714.2 + CR 714.2d: The Saga-chapter identity of a stack entry that is /// about to resolve, or `None` if the entry is not a Saga chapter ability. /// /// Captured BEFORE the ability executes: a chapter ability may move its own /// Saga off the battlefield as its effect (Fable of the Mirror-Breaker III -/// exiles and returns it transformed; CR 714.4's sacrifice follows the final -/// chapter), and neither the chapter number nor the final chapter number can be -/// re-derived once the permanent is gone. +/// exiles and returns it transformed), and CR 714.4 sacrifices the Saga once its +/// final chapter ability has left the stack. Neither the chapter number nor the +/// final chapter number can be re-derived once the permanent is gone. struct ResolvingSagaChapter { saga_id: ObjectId, controller: PlayerId, @@ -1005,7 +1005,7 @@ struct ResolvingSagaChapter { final_chapter: u32, } -/// CR 714.2a: Classify an about-to-resolve stack entry as a Saga chapter +/// CR 714.2: Classify an about-to-resolve stack entry as a Saga chapter /// ability, via the exact trigger occurrence that fired rather than any /// re-derivation from the Saga's current lore count. fn resolving_saga_chapter(state: &GameState, entry: &StackEntry) -> Option { @@ -1015,11 +1015,19 @@ fn resolving_saga_chapter(state: &GameState, entry: &StackEntry) -> Option) { return; } - // CR 714.2a: Snapshot the Saga-chapter identity while the Saga is still + // CR 714.2: Snapshot the Saga-chapter identity while the Saga is still // reachable — the chapter ability's own effect may remove it. Only the // success path below publishes it; a fizzle (CR 608.2b) or a failed // intervening-if (CR 603.4) leaves the stack without resolving. @@ -2538,7 +2546,7 @@ pub fn resolve_top(state: &mut GameState, events: &mut Vec) { events.push(GameEvent::StackResolved { object_id: entry.id, }); - // CR 714.2a + CR 608.2: This is the only exit from `resolve_top` on which a + // CR 714.2 + CR 608.2: This is the only exit from `resolve_top` on which a // triggered ability actually RESOLVED — the fizzle, no-legal-target and // failed-intervening-if paths returned earlier, each pushing their own // `StackResolved`. Publishing the chapter-resolution event only here is what diff --git a/crates/engine/src/game/targeting.rs b/crates/engine/src/game/targeting.rs index 776ad25e00..e4498b2dca 100644 --- a/crates/engine/src/game/targeting.rs +++ b/crates/engine/src/game/targeting.rs @@ -1565,7 +1565,7 @@ pub(crate) fn extract_source_from_event( // trigger fires from; `source_id` is the permanent tapped for mana. GameEvent::TappedForMana { source_id, .. } => Some(*source_id), GameEvent::CounterAdded { object_id, .. } => Some(*object_id), - // CR 608.2k + CR 714.2a: "that Saga" in "each opponent loses X life … + // CR 608.2k + CR 714.2e: "that Saga" in "each opponent loses X life … // where X is that Saga's mana value" (Narci, Fable Singer) is an // untargeted back-reference to the object the trigger condition named — // the Saga whose chapter ability resolved. diff --git a/crates/engine/src/game/trigger_index.rs b/crates/engine/src/game/trigger_index.rs index 61205e92e9..a6cc959bb2 100644 --- a/crates/engine/src/game/trigger_index.rs +++ b/crates/engine/src/game/trigger_index.rs @@ -238,12 +238,13 @@ pub(crate) fn keys_from_trigger_def(def: &TriggerDefinition) -> (Keys, bool) { | TriggerMode::CounterAddedOnce | TriggerMode::CounterAddedAll | TriggerMode::CounterTypeAddedAll => push(TriggerEventKey::CounterAdded), - // CR 714.2a + CR 714.4: a Saga-chapter meta-trigger's match shape is - // dynamic — "the final chapter" is derived from the OBSERVED Saga's own - // trigger set, not from anything statically on this trigger. Route to - // `unclassified` (the documented safety net for dynamic shapes); the - // three printed cards in the class make the consult cost irrelevant. - TriggerMode::SagaChapterAbility { .. } => return (keys, true), + // CR 714.2d + CR 714.2e: a final-chapter meta-trigger's match shape is + // dynamic — the final chapter number is derived from the OBSERVED Saga's + // own chapter abilities, not from anything statically on this trigger. + // Route to `unclassified` (the documented safety net for dynamic + // shapes); the three printed cards in the class make the consult cost + // irrelevant. + TriggerMode::FinalSagaChapterAbility { .. } => return (keys, true), // CR 107.14: "Whenever you get one or more {E}" — energy uses the // player-counter event key, not the object-counter key. TriggerMode::CounterPlayerAddedAll => push(TriggerEventKey::PlayerCounterChanged), @@ -614,8 +615,9 @@ pub(crate) fn keys_from_event(event: &GameEvent, state: &GameState) -> Keys { GameEvent::DamagePrevented { .. } => push(TriggerEventKey::DamagePrevented), GameEvent::SpellCountered { .. } => {} GameEvent::CounterAdded { .. } => push(TriggerEventKey::CounterAdded), - // CR 714.2a: consumed only by `SagaChapterAbility { lifecycle: Resolved }` - // triggers, which live in the `unclassified` bucket. No key of its own. + // CR 714.2e: consumed only by + // `FinalSagaChapterAbility { lifecycle: Resolved }` triggers, which live + // in the `unclassified` bucket. No key of its own. GameEvent::SagaChapterAbilityResolved { .. } => {} GameEvent::Evolved { .. } => {} GameEvent::ObjectIntensified { .. } => {} diff --git a/crates/engine/src/game/trigger_matchers.rs b/crates/engine/src/game/trigger_matchers.rs index 2775243bdd..3515d8e47e 100644 --- a/crates/engine/src/game/trigger_matchers.rs +++ b/crates/engine/src/game/trigger_matchers.rs @@ -10,9 +10,7 @@ use crate::types::events::{GameEvent, PlayerActionKind}; use crate::types::game_state::{GameState, TriggerSourceContext}; use crate::types::identifiers::ObjectId; use crate::types::player::PlayerId; -use crate::types::triggers::{ - AbilityLifecyclePoint, PlaneswalkRole, SagaChapterScope, TriggerMode, -}; +use crate::types::triggers::{AbilityLifecyclePoint, PlaneswalkRole, TriggerMode}; use crate::types::zones::Zone; use super::triggers::TriggerMatcher; @@ -128,10 +126,10 @@ pub fn trigger_matcher(mode: TriggerMode) -> Option { // matcher that reads the `PlaneswalkRole` off the trigger's mode — `From` // and `To` bind the source to that endpoint, `Any` is source-independent. TriggerMode::Planeswalked { .. } => match_planeswalked, - // CR 714.2a + CR 714.4: "whenever the final chapter ability of a Saga - // you control triggers/resolves" — one matcher reads both axes off the + // CR 714.2e: "whenever the final chapter ability of a Saga you control + // triggers/resolves" — one matcher reads the lifecycle axis off the // mode. - TriggerMode::SagaChapterAbility { .. } => match_saga_chapter_ability, + TriggerMode::FinalSagaChapterAbility { .. } => match_saga_chapter_ability, // CR 904.9 / CR 701.32b: "When you set this scheme in motion" fires for // the scheme set in motion. TriggerMode::SetInMotion => match_set_in_motion, @@ -409,19 +407,17 @@ pub fn build_trigger_registry() -> HashMap { ] { r.insert(TriggerMode::Planeswalked { role }, match_planeswalked); } - // CR 714.2a + CR 714.4: one matcher for every (chapter, lifecycle) pair; it - // reads both axes off the trigger's mode. Each pair is a distinct registry - // key (both participate in `TriggerMode`'s Hash/Eq). - for chapter in [SagaChapterScope::Any, SagaChapterScope::Final] { - for lifecycle in [ - AbilityLifecyclePoint::Triggered, - AbilityLifecyclePoint::Resolved, - ] { - r.insert( - TriggerMode::SagaChapterAbility { chapter, lifecycle }, - match_saga_chapter_ability, - ); - } + // CR 714.2e: one matcher for each lifecycle point; it reads the axis off the + // trigger's mode. Each point is a distinct registry key (it participates in + // `TriggerMode`'s Hash/Eq). + for lifecycle in [ + AbilityLifecyclePoint::Triggered, + AbilityLifecyclePoint::Resolved, + ] { + r.insert( + TriggerMode::FinalSagaChapterAbility { lifecycle }, + match_saga_chapter_ability, + ); } // CR 904.9 / CR 701.32b / CR 701.33b: Archenemy scheme triggers r.insert(TriggerMode::SetInMotion, match_set_in_motion); @@ -1084,7 +1080,7 @@ fn count_matching_trigger_event_subjects( | GameEvent::StartingPlayerContest { .. } | GameEvent::Foretold { .. } | GameEvent::BecameForetold { .. } - // CR 714.2a: names a Saga, but chapter-ability meta-triggers are never + // CR 714.2: names a Saga, but chapter-ability meta-triggers are never // batched ("one or more" has no reading over chapter resolutions). | GameEvent::SagaChapterAbilityResolved { .. } | GameEvent::HiddenSearchViewed { .. } => 0, @@ -2183,7 +2179,7 @@ pub(super) fn match_counter_added( if !valid_player_matches(trigger, state, *actor, source_context) { return false; } - // CR 714.2a: Apply counter filter (type + optional threshold crossing). + // CR 714.2b: Apply counter filter (type + optional threshold crossing). if let Some(ref filter) = trigger.counter_filter { if filter.counter_type != *counter_type { return false; @@ -2228,22 +2224,22 @@ pub(super) fn match_counter_added( } } -/// CR 714.2a + CR 714.4: "Whenever [the final] chapter ability of a Saga you -/// control triggers/resolves" (Historian's Boon, Narci, Fable Singer, Tom -/// Bombadil). +/// CR 714.2e: "Whenever the final chapter ability of a Saga you control +/// triggers/resolves" (Historian's Boon, Narci, Fable Singer, Tom Bombadil). /// /// The observed Saga is constrained by the trigger's ordinary `valid_card` -/// filter ("a Saga you control"), matched with last-known information: a Saga -/// that reached its final chapter is sacrificed by CR 704.5s the moment its -/// chapter ability leaves the stack, and a chapter ability may remove the Saga -/// itself (Fable of the Mirror-Breaker III), so the permanent frequently no -/// longer exists when this trigger is collected. +/// filter ("a Saga you control"), matched with last-known information: CR 714.4 +/// sacrifices a Saga once its final chapter ability has left the stack, and a +/// chapter ability may remove the Saga itself (Fable of the Mirror-Breaker III), +/// so the permanent frequently no longer exists when this trigger is collected. /// /// The two lifecycle points read different events because they ARE different /// events (CR 603.2 vs CR 608.2): /// -/// * `Triggered` — chapter abilities have no event of their own; they *are* the -/// Saga's lore-counter threshold triggers, so the trigger event is the same +/// * `Triggered` — chapter abilities have no event of their own. CR 714.2b +/// defines a chapter symbol as "When one or more lore counters are put onto +/// this Saga, if the number of lore counters on it was less than N and became +/// at least N, [effect]", so the trigger event is the same /// `CounterAdded { Lore }` that `match_counter_added` consumes. /// * `Resolved` — `SagaChapterAbilityResolved`, published by `stack.rs` only on /// the path where a triggered ability genuinely finished resolving. @@ -2253,9 +2249,9 @@ pub(super) fn match_saga_chapter_ability( source_context: &TriggerSourceContext, state: &GameState, ) -> bool { - // The registry only routes `SagaChapterAbility` triggers here, but read the - // axes off the mode rather than assuming them. - let TriggerMode::SagaChapterAbility { chapter, lifecycle } = &trigger.mode else { + // The registry only routes `FinalSagaChapterAbility` triggers here, but read + // the lifecycle axis off the mode rather than assuming it. + let TriggerMode::FinalSagaChapterAbility { lifecycle } = &trigger.mode else { return false; }; @@ -2269,17 +2265,14 @@ pub(super) fn match_saga_chapter_ability( .. }, ) => { - if !valid_card_matches_with_lki(trigger, state, *saga_id, source_context) { - return false; - } - match chapter { - SagaChapterScope::Any => true, - SagaChapterScope::Final => resolved_chapter == final_chapter, - } + valid_card_matches_with_lki(trigger, state, *saga_id, source_context) + // CR 714.2e: the final chapter ability is the one whose chapter + // symbol carries the Saga's final chapter number (CR 714.2d). + && resolved_chapter == final_chapter } ( AbilityLifecyclePoint::Triggered, - // CR 714.2a: the chapter ability's own trigger event. `actor` (who + // CR 714.2b: the chapter ability's own trigger event. `actor` (who // placed the counter) is irrelevant — CR 714.3c's turn-based action // and any effect that adds lore both make chapter abilities trigger. GameEvent::CounterAdded { @@ -2295,10 +2288,10 @@ pub(super) fn match_saga_chapter_ability( if !valid_card_matches_with_lki(trigger, state, *object_id, source_context) { return false; } - // CR 714.2a: a chapter ability triggers when the lore count crosses - // its chapter number — `previous < n <= current`. The same crossing - // arithmetic `match_counter_added` performs for the Saga's own - // chapter triggers, evaluated here against the observed Saga. + // CR 714.2b: a chapter ability triggers when the lore count "was less + // than N and became at least N". The same crossing arithmetic + // `match_counter_added` performs for the Saga's own chapter triggers, + // evaluated here against the observed Saga's final chapter number. let Some(saga) = state.objects.get(object_id) else { return false; }; @@ -2308,20 +2301,10 @@ pub(super) fn match_saga_chapter_ability( .copied() .unwrap_or(0); let previous = current.saturating_sub(*count); - let crosses = |n: u32| previous < n && n <= current; - match chapter { - // CR 714.2c: a single addition can cross several chapter - // thresholds at once, triggering that many chapter abilities. - // This event-driven matcher fires once for such a batch. No - // printed card observes `Any` today (all three cards in the - // class say "the final chapter ability"), so the parser never - // produces this combination against a real card; the arm exists - // so the axis is total rather than silently absent. - SagaChapterScope::Any => saga.saga_chapter_numbers().any(crosses), - // A lore counter added to a Saga already past its final chapter - // (proliferate before CR 704.5s sacrifices it) crosses nothing. - SagaChapterScope::Final => saga.final_chapter_number().is_some_and(crosses), - } + // A lore counter added to a Saga already past its final chapter + // (proliferate before CR 714.4 sacrifices it) crosses nothing. + saga.final_chapter_number() + .is_some_and(|final_chapter| previous < final_chapter && final_chapter <= current) } _ => false, } @@ -2355,7 +2338,7 @@ pub(super) fn match_counter_removed( if !valid_card_matches(trigger, state, *object_id, source_context) { return false; } - // CR 310.12b + CR 714.2a-mirror: Apply counter filter (type + optional + // CR 310.12b + CR 714.2b-mirror: Apply counter filter (type + optional // "crossed zero" threshold). Used by the Siege victory trigger // "When the last defense counter is removed from this permanent". // A threshold of Some(0) means "fire only when the current count diff --git a/crates/engine/src/parser/oracle_nom/quantity.rs b/crates/engine/src/parser/oracle_nom/quantity.rs index 9fb7f54618..dd28127f83 100644 --- a/crates/engine/src/parser/oracle_nom/quantity.rs +++ b/crates/engine/src/parser/oracle_nom/quantity.rs @@ -4700,7 +4700,7 @@ fn parse_object_possessive_scope(input: &str) -> OracleResult<'_, ObjectScope> { value(ObjectScope::Target, tag("target creature's")), value(ObjectScope::Target, tag("target permanent's")), value(ObjectScope::EventSource, tag("that spell's")), - // CR 608.2k + CR 714.2a: "that Saga's mana value" (Narci, Fable Singer). + // CR 608.2k + CR 714.2e: "that Saga's mana value" (Narci, Fable Singer). // Same shape as the "that spell's" arm above and bound the same way: an // untargeted back-reference to the object the TRIGGER CONDITION named, // not a threaded target. The "that 's" arms below bind to diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 14127c9031..a8cbfdcd86 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -65,7 +65,7 @@ use crate::types::keywords::{Keyword, KeywordKind}; use crate::types::mana::{ManaColor, ManaType}; use crate::types::phase::Phase; use crate::types::triggers::{ - AbilityLifecyclePoint, AttackTargetFilter, PlaneswalkRole, SagaChapterScope, TriggerMode, + AbilityLifecyclePoint, AttackTargetFilter, PlaneswalkRole, TriggerMode, }; use crate::types::zones::Zone; use std::str::FromStr; @@ -9225,7 +9225,7 @@ pub(crate) fn parse_trigger_condition( return result; } - // CR 714.2a: "whenever the final chapter ability of a Saga you control + // CR 714.2e: "whenever the final chapter ability of a Saga you control // resolves". Dispatched before subject decomposition — the grammatical // subject of this clause is an ABILITY, not a permanent, so the generic // subject/event-verb path would mis-bind "chapter ability" as the object @@ -17717,16 +17717,23 @@ fn try_parse_discard_trigger( /// "Another" self-exclusion (e.g., Mazirek's "another permanent") is carried by /// `FilterProp::Another` from `parse_trigger_subject`; the runtime matcher enforces /// it via `FilterProp::Another` → `object_id != source.id` in `filter.rs`. -/// CR 714.2a + CR 714.4: "Whenever [the final] chapter ability of \ +/// CR 714.2e: "Whenever the final chapter ability of \ /// triggers/resolves" — Historian's Boon, Narci Fable Singer, Tom Bombadil. /// -/// The class varies over two independent axes, so each is one `alt()` composed -/// in sequence rather than an enumeration of phrasings: WHICH chapter ability is -/// observed (`the final chapter ability` / `a chapter ability`) and WHICH point -/// of its lifecycle fires the trigger (`triggers` / `resolves`). The observed -/// Saga flows through the shared `parse_trigger_subject` building block, so -/// "a Saga you control", a bare "a Saga", and an opponent-scoped subject all -/// work without any Saga-specific subject grammar here. +/// The one axis the printed class varies over is WHICH point of the observed +/// ability's lifecycle fires the trigger (`triggers` / `resolves`), so that is a +/// single `alt()`. The observed Saga flows through the shared +/// `parse_trigger_subject` building block, so "a Saga you control", a bare +/// "a Saga", and an opponent-scoped subject all work without any Saga-specific +/// subject grammar here. +/// +/// Deliberately requires "final": an unqualified "a chapter ability of …" +/// observer is NOT accepted, because CR 714.2b makes each chapter symbol its own +/// triggered ability and a single lore-counter addition can cross several +/// chapter numbers at once. Such an observer must fire once per crossed ability, +/// which the event-keyed matcher cannot express from one `CounterAdded`. +/// Accepting the grammar would mint a silently under-firing trigger; no printed +/// card needs it. fn try_parse_saga_chapter_ability_trigger(lower: &str) -> Option<(TriggerMode, TriggerDefinition)> { let (event, ()) = alt(( value((), tag::<_, _, OracleError<'_>>("whenever ")), @@ -17735,18 +17742,9 @@ fn try_parse_saga_chapter_ability_trigger(lower: &str) -> Option<(TriggerMode, T .parse(lower) .ok()?; - // "the final chapter ability of " / "a chapter ability of ". The article is - // optional because it is not part of the chapter-scope distinction. - let (subject_text, (_, chapter, _)) = ( - opt(alt(( - tag::<_, _, OracleError<'_>>("the "), - tag("a "), - tag("an "), - ))), - alt(( - value(SagaChapterScope::Final, tag("final chapter ability")), - value(SagaChapterScope::Any, tag("chapter ability")), - )), + let (subject_text, _) = ( + opt(tag::<_, _, OracleError<'_>>("the ")), + tag("final chapter ability"), tag(" of "), ) .parse(event) @@ -17769,11 +17767,11 @@ fn try_parse_saga_chapter_ability_trigger(lower: &str) -> Option<(TriggerMode, T return None; } - let mode = TriggerMode::SagaChapterAbility { chapter, lifecycle }; + let mode = TriggerMode::FinalSagaChapterAbility { lifecycle }; let mut def = make_base(); def.mode = mode.clone(); - // CR 714.1: the observed Saga is an ordinary trigger subject — the runtime - // matcher applies this filter to the Saga carried by the event. + // The observed Saga is an ordinary trigger subject — the runtime matcher + // applies this filter to the Saga carried by the event. def.valid_card = Some(filter); Some((mode, def)) } @@ -18827,11 +18825,12 @@ mod cost_x_totality_guard_tests { } } -/// CR 714.2a + CR 714.4: the Saga-chapter meta-trigger building block — +/// CR 714.2e: the final-chapter meta-trigger building block — /// "whenever [the final] chapter ability of \ triggers/resolves". /// -/// These exercise the two axes the combinator composes (chapter scope × -/// lifecycle point) and the shared subject grammar, not one card's printed line. +/// These exercise the lifecycle axis the combinator composes, the shared +/// subject grammar, and the deliberate refusal of an unqualified +/// chapter-ability clause — not one card's printed line. #[cfg(test)] mod saga_chapter_ability_trigger_tests { use super::parse_trigger_line; @@ -18839,7 +18838,7 @@ mod saga_chapter_ability_trigger_tests { use crate::types::ability::{ ControllerRef, ObjectScope, QuantityRef, TargetFilter, TypeFilter, }; - use crate::types::triggers::{AbilityLifecyclePoint, SagaChapterScope, TriggerMode}; + use crate::types::triggers::{AbilityLifecyclePoint, TriggerMode}; fn saga_filter(filter: Option<&TargetFilter>) -> (Vec, Option) { let Some(TargetFilter::Typed(typed)) = filter else { @@ -18857,8 +18856,7 @@ mod saga_chapter_ability_trigger_tests { ); assert_eq!( def.mode, - TriggerMode::SagaChapterAbility { - chapter: SagaChapterScope::Final, + TriggerMode::FinalSagaChapterAbility { lifecycle: AbilityLifecyclePoint::Resolved, } ); @@ -18881,8 +18879,7 @@ mod saga_chapter_ability_trigger_tests { ); assert_eq!( def.mode, - TriggerMode::SagaChapterAbility { - chapter: SagaChapterScope::Final, + TriggerMode::FinalSagaChapterAbility { lifecycle: AbilityLifecyclePoint::Triggered, } ); @@ -18895,21 +18892,24 @@ mod saga_chapter_ability_trigger_tests { ); } - /// The chapter axis is a real parameter, not a hardcoded "final": an - /// unqualified chapter ability parses rather than falling out of the - /// grammar. + /// CR 714.2b: an UNQUALIFIED chapter-ability observer must be refused, not + /// silently accepted as if it were the final-chapter one. Each chapter + /// symbol is its own triggered ability, so one lore-counter addition + /// crossing several chapter numbers triggers that many abilities; an + /// observer of all of them owes one firing per crossed ability, which this + /// event-keyed trigger family cannot express. Falling back to `Unknown` + /// keeps the clause honestly coverage-red instead of minting a trigger that + /// under-fires. #[test] - fn any_chapter_resolves() { + fn unqualified_chapter_ability_is_refused() { let def = parse_trigger_line( "Whenever a chapter ability of a Saga you control resolves, draw a card.", "Test", ); - assert_eq!( - def.mode, - TriggerMode::SagaChapterAbility { - chapter: SagaChapterScope::Any, - lifecycle: AbilityLifecyclePoint::Resolved, - } + assert!( + matches!(def.mode, TriggerMode::Unknown(_)), + "an unqualified chapter-ability clause must not mint a final-chapter trigger, got {:?}", + def.mode ); } @@ -18924,8 +18924,7 @@ mod saga_chapter_ability_trigger_tests { ); assert_eq!( def.mode, - TriggerMode::SagaChapterAbility { - chapter: SagaChapterScope::Final, + TriggerMode::FinalSagaChapterAbility { lifecycle: AbilityLifecyclePoint::Resolved, } ); diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index 9257c2629f..4faa123f9b 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -21593,7 +21593,7 @@ pub struct CounterTriggerFilter { pub counter_type: crate::types::counter::CounterType, /// If set, only fire when the count crosses this threshold: /// previous_count < threshold <= new_count. - /// Used by Saga chapter triggers (CR 714.2a). + /// Used by Saga chapter triggers (CR 714.2b). #[serde(default, skip_serializing_if = "Option::is_none")] pub threshold: Option, } @@ -21713,7 +21713,7 @@ pub struct TriggerDefinition { pub constraint: Option, #[serde(default)] pub condition: Option, - /// Optional filter for counter-related trigger modes (CR 714.2a). + /// Optional filter for counter-related trigger modes (CR 714.2b). #[serde(default, skip_serializing_if = "Option::is_none")] pub counter_filter: Option, /// CR 118.12: "Effect unless [player] pays {cost}" — tax trigger modifier. diff --git a/crates/engine/src/types/events.rs b/crates/engine/src/types/events.rs index 275e2762c4..aa1b767646 100644 --- a/crates/engine/src/types/events.rs +++ b/crates/engine/src/types/events.rs @@ -1053,27 +1053,30 @@ pub enum GameEvent { #[serde(default)] actor: PlayerId, }, - /// CR 714.2a + CR 608.2: A Saga's chapter ability finished resolving. + /// CR 714.2 + CR 608.2: A Saga's chapter ability finished resolving. /// - /// A chapter ability is not a distinct AST concept — it is the Saga's own - /// lore-counter threshold trigger (CR 714.2a). This event is the resolution - /// half of that lifecycle, which nothing else on the bus reports: - /// `StackResolved` is emitted for fizzles and failed intervening-ifs too, - /// and carries the stack entry id rather than the Saga. + /// A chapter ability is not a distinct AST concept — CR 714.2b defines a + /// chapter symbol as a lore-counter threshold trigger on the Saga itself. + /// This event is the resolution half of that ability's lifecycle, which + /// nothing else on the bus reports: `StackResolved` is emitted for fizzles + /// and failed intervening-ifs too, and carries the stack entry id rather + /// than the Saga. /// /// `chapter` and `final_chapter` are captured BEFORE the chapter ability /// executes, because a chapter ability may remove its own Saga from the - /// battlefield as its effect (CR 714.4 / Fable of the Mirror-Breaker III), - /// after which neither number could be re-derived. + /// battlefield as its effect (Fable of the Mirror-Breaker III), and CR 714.4 + /// sacrifices it as soon as the ability leaves the stack — after which + /// neither number could be re-derived. SagaChapterAbilityResolved { /// The Saga permanent whose chapter ability resolved. saga_id: ObjectId, /// CR 109.5: controller of the resolved chapter ability. controller: PlayerId, - /// CR 714.2a: the chapter number (lore threshold) that resolved. + /// CR 714.2b: the chapter number (lore threshold) that resolved. chapter: u32, - /// CR 714.4: the Saga's greatest chapter number. `chapter == - /// final_chapter` is what makes this the *final* chapter ability. + /// CR 714.2d: the greatest chapter number among this Saga's chapter + /// abilities. Per CR 714.2e, `chapter == final_chapter` is exactly what + /// makes this the Saga's *final* chapter ability. final_chapter: u32, }, /// Digital-only Alchemy (no CR entry): a card's intensity increased by diff --git a/crates/engine/src/types/triggers.rs b/crates/engine/src/types/triggers.rs index a39d671de0..f9b917114a 100644 --- a/crates/engine/src/types/triggers.rs +++ b/crates/engine/src/types/triggers.rs @@ -218,19 +218,6 @@ pub enum PlaneswalkRole { Any, } -/// CR 714.2a + CR 714.4: which chapter ability of a Saga a meta-trigger listens -/// for. A Saga's chapter abilities are its lore-counter threshold triggers -/// (CR 714.2a); the *final* chapter ability is the one whose chapter number is -/// the greatest among them (CR 714.4 — reaching it is what makes the Saga's -/// controller sacrifice it). -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] -pub enum SagaChapterScope { - /// Any chapter ability of the Saga. - Any, - /// CR 714.4: only the final (highest-numbered) chapter ability. - Final, -} - /// CR 603.2 + CR 608.2: the point in another ability's lifecycle that a /// meta-trigger observes. The two points are distinct events with distinct /// timing consequences: an ability that triggers may still be countered @@ -624,18 +611,27 @@ pub enum TriggerMode { /// `game::haunt::match_haunted_creature_dies`. HauntedCreatureDies, - /// CR 714.2a + CR 714.4: a meta-trigger on another permanent's Saga chapter - /// abilities — "whenever the final chapter ability of a Saga you control - /// resolves" (Narci, Fable Singer; Tom Bombadil) / "… triggers" - /// (Historian's Boon). Parameterized on the two independent axes the class - /// varies over: WHICH chapter ability is observed (`chapter`) and WHICH - /// point of that ability's lifecycle fires this trigger (`lifecycle`). + /// CR 714.2e: a meta-trigger on another permanent's FINAL chapter ability — + /// "whenever the final chapter ability of a Saga you control resolves" + /// (Narci, Fable Singer; Tom Bombadil) / "… triggers" (Historian's Boon). + /// CR 714.2e defines a Saga's final chapter ability as the chapter ability + /// whose chapter symbol carries its final chapter number (CR 714.2d). + /// + /// `lifecycle` is the one axis the printed class actually varies over. It is + /// deliberately NOT parameterized on *which* chapter ability is observed: + /// all three printed cards say "the final chapter ability", and an + /// unqualified "a chapter ability" observer could not be modeled correctly + /// here anyway. CR 714.2b makes each chapter symbol its own triggered + /// ability, so one lore-counter addition that crosses several chapter + /// numbers triggers that many chapter abilities — an observer of all of them + /// must fire once per crossed ability, which an event-keyed matcher reading + /// a single `CounterAdded` cannot express. Adding that scope needs an + /// occurrence-level chapter event first, not a wider enum. /// /// The Saga itself is constrained by the trigger's ordinary `valid_card` /// filter ("a Saga you control"), so no Saga-specific filter axis is needed /// here. - SagaChapterAbility { - chapter: SagaChapterScope, + FinalSagaChapterAbility { lifecycle: AbilityLifecyclePoint, }, diff --git a/crates/engine/tests/integration/narci_fable_singer_final_chapter_drain.rs b/crates/engine/tests/integration/narci_fable_singer_final_chapter_drain.rs index a45312f5a7..d73a57902c 100644 --- a/crates/engine/tests/integration/narci_fable_singer_final_chapter_drain.rs +++ b/crates/engine/tests/integration/narci_fable_singer_final_chapter_drain.rs @@ -1,4 +1,4 @@ -//! CR 714.2a + CR 714.4 + CR 608.2k — Narci, Fable Singer's final-chapter +//! CR 714.2e + CR 714.4 + CR 608.2k — Narci, Fable Singer's final-chapter //! drain, driven end-to-end through the production Saga pipeline. //! //! Oracle: `Whenever the final chapter ability of a Saga you control resolves, @@ -144,7 +144,7 @@ fn narci_drains_for_the_sagas_mana_value_when_its_final_chapter_resolves() { ); } -/// CR 714.4: a NON-final chapter ability resolving must not fire the trigger. +/// CR 714.2e: a NON-final chapter ability resolving must not fire the trigger. /// Without the chapter/final-chapter comparison this test drains on chapter I /// as well, which would double Narci's output on every multi-chapter Saga. #[test] From 42513342c08a39b6f048885f2a7bc764a36ffae0 Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Mon, 10 Aug 2026 22:01:51 -0500 Subject: [PATCH 3/5] fix(engine): record Saga chapter provenance instead of inferring it Addresses the two review findings previously deferred on #7224. Chapter abilities were being recognized by shape: a lore-counter threshold on a Saga. CR 714.2b does give a chapter symbol that shape, but the converse does not hold -- a lore threshold trigger a Saga acquired some other way is not a chapter ability, and counting it corrupts the final chapter number CR 714.2d defines and CR 714.4's sacrifice depends on. TriggerDefinition now carries `saga_chapter: Option`, the chapter symbol's Roman numeral, set only by the Saga parser -- the one place that has read an actual chapter symbol. `saga_chapter_numbers`, `saga_chapter_for_occurrence` and read-ahead's CR 714.2d derivation in synthesis all read that provenance instead of re-deriving from thresholds, so there is one model rather than three. Keying `saga_chapter_for_occurrence` on the occurrence keeps CR 714.2c's two chapter abilities printed on one line distinct; the History of Benalia IR snapshot shows that directly ("I, II --" now yields saga_chapter 1 and 2). Hand-built Saga fixtures in sba.rs, synthesis.rs and game_object.rs must now declare the provenance too. That is the point: a bare lore threshold no longer passes for a chapter ability anywhere. The second finding -- that publishing the resolution event before the settlement guard lets observers fire while a chapter ability is still resolving -- is answered with the test the review asked for rather than a refactor. `narci_does_not_drain_until_a_paused_final_chapter_finishes_resolving` gives the final chapter an optional-effect choice, asserts the drain has NOT landed while that prompt is open, and asserts it lands exactly once after it is answered. A `saw_optional` reach guard fails the test if the pause never happens, so it cannot pass vacuously. The ordering holds because a pending trigger cannot reach the stack until a player would receive priority, which follows completion of the in-resolution choice. Moving the emission would have required threading an event sink through `settle_finished_resolving_stack_entry` and every parked-resolution settle path, changing where the engine considers a resolution finished for `StackResolved` too. Co-Authored-By: Claude Opus 5 --- crates/engine/src/database/synthesis.rs | 21 +++-- crates/engine/src/game/game_object.rs | 67 +++++++-------- crates/engine/src/game/sba.rs | 12 +-- ...snapshot_tests__history_of_benalia_ir.snap | 3 + ...hot_tests__history_of_benalia_lowered.snap | 3 + crates/engine/src/parser/oracle_saga.rs | 5 ++ ...ne_snapshot_tests__pipeline_saga_card.snap | 5 +- crates/engine/src/types/ability.rs | 21 +++++ .../narci_fable_singer_final_chapter_drain.rs | 85 +++++++++++++++++++ 9 files changed, 170 insertions(+), 52 deletions(-) diff --git a/crates/engine/src/database/synthesis.rs b/crates/engine/src/database/synthesis.rs index ee82dc834f..6381b74c95 100644 --- a/crates/engine/src/database/synthesis.rs +++ b/crates/engine/src/database/synthesis.rs @@ -8994,16 +8994,12 @@ pub fn synthesize_read_ahead(face: &mut CardFace) { if !face.keywords.contains(&Keyword::ReadAhead) { return; } - // CR 714.2d: final chapter number = greatest lore-counter threshold among - // this Saga's chapter triggers. No chapter abilities → nothing to read ahead to. - let Some(final_chapter) = face - .triggers - .iter() - .filter_map(|t| t.counter_filter.as_ref()) - .filter(|f| f.counter_type == CounterType::Lore) - .filter_map(|f| f.threshold) - .max() - else { + // CR 714.2d: final chapter number = the greatest value among this Saga's + // chapter abilities, read from the chapter-symbol provenance the Saga parser + // records. Not inferred from lore thresholds: CR 714.2b gives a chapter + // symbol that shape, but a lore threshold trigger acquired some other way is + // not a chapter ability. No chapter abilities → nothing to read ahead to. + let Some(final_chapter) = face.triggers.iter().filter_map(|t| t.saga_chapter).max() else { return; }; @@ -22939,7 +22935,10 @@ mod devour_synthesis_tests { .counter_filter(CounterTriggerFilter { counter_type: CounterType::Lore, threshold: Some(n), - }), + }) + // CR 714.2: mirror what the Saga parser records — these + // fixtures stand in for real chapter symbols. + .saga_chapter(n), ); } face.replacements.push( diff --git a/crates/engine/src/game/game_object.rs b/crates/engine/src/game/game_object.rs index 4acf0e4c3d..94b18aa3dd 100644 --- a/crates/engine/src/game/game_object.rs +++ b/crates/engine/src/game/game_object.rs @@ -2911,19 +2911,19 @@ impl GameObject { self.owner == player && self.zone == Zone::Graveyard && self.is_represented_by_a_card() } - /// CR 714.2 + CR 714.2b: Every chapter number this Saga's chapter abilities - /// are keyed to. A chapter ability is not a distinct AST concept — CR 714.2b - /// defines a chapter symbol as "When one or more lore counters are put onto - /// this Saga, if the number of lore counters on it was less than N and - /// became at least N, [effect]", and `parser::oracle_saga` emits exactly - /// that: a lore-counter threshold trigger on the Saga itself. So the chapter - /// numbers are exactly those thresholds. Empty for a non-Saga. + /// CR 714.2: Every chapter number this Saga's chapter abilities are keyed + /// to, read from the chapter-symbol provenance the Saga parser records + /// (`TriggerDefinition::saga_chapter`). /// - /// Lore-scoped per CR 714.2b: a Saga may also carry a thresholded trigger on - /// some OTHER counter type, and that trigger is not a chapter ability. + /// Deliberately NOT inferred from lore-counter thresholds. CR 714.2b gives a + /// chapter symbol the shape of a lore threshold trigger, but the converse + /// does not hold: a lore threshold trigger a Saga acquired some other way is + /// not a chapter ability, and counting it would corrupt the final chapter + /// number that CR 714.2d defines and CR 714.4's sacrifice depends on. /// - /// Structural scan of the Saga's own triggers — intrinsic to the card, not - /// subject to functioning gates. `iter_all` is pub(crate). + /// Empty for a non-Saga. Structural scan of the Saga's own triggers — + /// intrinsic to the card, not subject to functioning gates. `iter_all` is + /// pub(crate). pub fn saga_chapter_numbers(&self) -> impl Iterator + '_ { self.card_types .subtypes @@ -2932,9 +2932,7 @@ impl GameObject { .then(|| self.trigger_definitions.iter_all()) .into_iter() .flatten() - .filter_map(|entry| entry.definition.counter_filter.as_ref()) - .filter(|filter| filter.counter_type == CounterType::Lore) - .filter_map(|filter| filter.threshold) + .filter_map(|entry| entry.definition.saga_chapter) } /// CR 714.2d: "A Saga's final chapter number is the greatest value among @@ -2953,14 +2951,15 @@ impl GameObject { /// by the exact trigger occurrence that produced it, returning /// `(chapter_number, final_chapter_number)`. /// - /// This is the single authority that maps a fired trigger occurrence back to - /// its printed chapter number, so callers never have to re-derive it from - /// the lore count (wrong under Read Ahead, and wrong for a multi-counter - /// addition, which per CR 714.2b crosses several thresholds at once) or from - /// the `"Chapter {n}"` description string. + /// Keyed on the occurrence, so CR 714.2c's two chapter abilities printed on + /// one line stay distinguishable even though they share that line. The + /// chapter number comes from the recorded chapter symbol, never re-derived + /// from the lore count (wrong under Read Ahead, and wrong for a + /// multi-counter addition, which per CR 714.2b crosses several thresholds at + /// once) nor from the `"Chapter {n}"` description string. /// /// Returns `None` for a non-Saga, or for an occurrence that is not one of - /// this permanent's lore-threshold triggers. + /// this permanent's chapter abilities. pub fn saga_chapter_for_occurrence( &self, occurrence: &TriggerDefinitionOccurrenceRef, @@ -2970,9 +2969,7 @@ impl GameObject { .trigger_definitions .iter_all() .find(|entry| &entry.occurrence == occurrence) - .and_then(|entry| entry.definition.counter_filter.as_ref()) - .filter(|filter| filter.counter_type == CounterType::Lore) - .and_then(|filter| filter.threshold)?; + .and_then(|entry| entry.definition.saga_chapter)?; Some((chapter, final_chapter)) } @@ -3692,24 +3689,24 @@ mod tests { ); obj.card_types.subtypes.push("Saga".to_string()); obj.trigger_definitions = vec![ - TriggerDefinition::new(TriggerMode::CounterAdded).counter_filter( - CounterTriggerFilter { + TriggerDefinition::new(TriggerMode::CounterAdded) + .counter_filter(CounterTriggerFilter { counter_type: CounterType::Lore, threshold: Some(1), - }, - ), - TriggerDefinition::new(TriggerMode::CounterAdded).counter_filter( - CounterTriggerFilter { + }) + .saga_chapter(1), + TriggerDefinition::new(TriggerMode::CounterAdded) + .counter_filter(CounterTriggerFilter { counter_type: CounterType::Lore, threshold: Some(2), - }, - ), - TriggerDefinition::new(TriggerMode::CounterAdded).counter_filter( - CounterTriggerFilter { + }) + .saga_chapter(2), + TriggerDefinition::new(TriggerMode::CounterAdded) + .counter_filter(CounterTriggerFilter { counter_type: CounterType::Lore, threshold: Some(3), - }, - ), + }) + .saga_chapter(3), ] .into(); assert_eq!(obj.final_chapter_number(), Some(3)); diff --git a/crates/engine/src/game/sba.rs b/crates/engine/src/game/sba.rs index 367bff7ab5..5033e4b8c9 100644 --- a/crates/engine/src/game/sba.rs +++ b/crates/engine/src/game/sba.rs @@ -4347,15 +4347,17 @@ mod tests { obj.card_types.core_types.push(CoreType::Enchantment); obj.card_types.subtypes.push("Saga".to_string()); obj.entered_battlefield_turn = Some(state.turn_number); - // Add chapter triggers so final_chapter_number() works + // CR 714.2: add chapter triggers so final_chapter_number() works. The + // `saga_chapter` provenance is what marks these as chapter abilities — + // a bare lore threshold is not one (see `saga_chapter_numbers`). for ch in 1..=final_chapter { obj.trigger_definitions.push( - TriggerDefinition::new(TriggerMode::CounterAdded).counter_filter( - CounterTriggerFilter { + TriggerDefinition::new(TriggerMode::CounterAdded) + .counter_filter(CounterTriggerFilter { counter_type: CounterType::Lore, threshold: Some(ch), - }, - ), + }) + .saga_chapter(ch), ); } id diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__history_of_benalia_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__history_of_benalia_ir.snap index a968cfcea9..fe8eb3c77b 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__history_of_benalia_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__history_of_benalia_ir.snap @@ -90,6 +90,7 @@ expression: "&ir" "counter_type": "lore", "threshold": 1 }, + "saga_chapter": 1, "batched": false }, "source_text": "I, II — Create a 2/2 white Knight creature token with vigilance." @@ -183,6 +184,7 @@ expression: "&ir" "counter_type": "lore", "threshold": 2 }, + "saga_chapter": 2, "batched": false }, "source_text": "I, II — Create a 2/2 white Knight creature token with vigilance." @@ -325,6 +327,7 @@ expression: "&ir" "counter_type": "lore", "threshold": 3 }, + "saga_chapter": 3, "batched": false }, "source_text": "III — Knights you control get +2/+1 until end of turn." diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__history_of_benalia_lowered.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__history_of_benalia_lowered.snap index 2bba96f67d..e6af63a398 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__history_of_benalia_lowered.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__history_of_benalia_lowered.snap @@ -71,6 +71,7 @@ expression: "&lowered" "counter_type": "lore", "threshold": 1 }, + "saga_chapter": 1, "batched": false }, { @@ -139,6 +140,7 @@ expression: "&lowered" "counter_type": "lore", "threshold": 2 }, + "saga_chapter": 2, "batched": false }, { @@ -197,6 +199,7 @@ expression: "&lowered" "counter_type": "lore", "threshold": 3 }, + "saga_chapter": 3, "batched": false } ], diff --git a/crates/engine/src/parser/oracle_saga.rs b/crates/engine/src/parser/oracle_saga.rs index 39f4a9b435..82563527c4 100644 --- a/crates/engine/src/parser/oracle_saga.rs +++ b/crates/engine/src/parser/oracle_saga.rs @@ -196,6 +196,11 @@ pub(crate) fn parse_saga_chapters(lines: &[&str], _card_name: &str) -> SagaChapt counter_type: crate::types::counter::CounterType::Lore, threshold: Some(n), }) + // CR 714.2: this trigger came from an actual chapter symbol, so + // record the numeral. Consumers that need "is this a chapter + // ability, and which one" read this rather than inferring it + // from the lore threshold above. + .saga_chapter(n) .execute(execute) .trigger_zones(vec![Zone::Battlefield]) .description(format!("Chapter {n}")); diff --git a/crates/engine/src/parser/snapshots/engine__parser__oracle__pipeline_snapshot_tests__pipeline_saga_card.snap b/crates/engine/src/parser/snapshots/engine__parser__oracle__pipeline_snapshot_tests__pipeline_saga_card.snap index 47987fd4b2..1a4e5402d1 100644 --- a/crates/engine/src/parser/snapshots/engine__parser__oracle__pipeline_snapshot_tests__pipeline_saga_card.snap +++ b/crates/engine/src/parser/snapshots/engine__parser__oracle__pipeline_snapshot_tests__pipeline_saga_card.snap @@ -1,5 +1,5 @@ --- -source: crates/engine/src/parser/oracle.rs +source: crates/engine/src/parser/oracle_pipeline_snapshot_tests.rs expression: result --- { @@ -71,6 +71,7 @@ expression: result "counter_type": "lore", "threshold": 1 }, + "saga_chapter": 1, "batched": false }, { @@ -137,6 +138,7 @@ expression: result "counter_type": "lore", "threshold": 2 }, + "saga_chapter": 2, "batched": false }, { @@ -187,6 +189,7 @@ expression: result "counter_type": "lore", "threshold": 3 }, + "saga_chapter": 3, "batched": false } ], diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index 4faa123f9b..0f45b9afed 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -21716,6 +21716,17 @@ pub struct TriggerDefinition { /// Optional filter for counter-related trigger modes (CR 714.2b). #[serde(default, skip_serializing_if = "Option::is_none")] pub counter_filter: Option, + /// CR 714.2 + CR 714.2a: The chapter symbol's Roman numeral, when this + /// trigger IS a Saga chapter ability. Provenance, not a second encoding of + /// the threshold: only the Saga parser sets it, and it is what distinguishes + /// a chapter ability from any other lore-counter threshold trigger a Saga + /// might carry. `None` for every non-chapter trigger. + /// + /// CR 714.2c ("{rN1}, {rN2}—[Effect]") yields one trigger per numeral, each + /// carrying its own chapter number, so two chapter abilities sharing a + /// printed line stay distinct here. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub saga_chapter: Option, /// CR 118.12: "Effect unless [player] pays {cost}" — tax trigger modifier. #[serde(default, skip_serializing_if = "Option::is_none")] pub unless_pay: Option, @@ -22242,6 +22253,7 @@ impl TriggerDefinition { constraint: None, condition: None, counter_filter: None, + saga_chapter: None, unless_pay: None, batched: false, die_sides: None, @@ -22340,6 +22352,14 @@ impl TriggerDefinition { self } + /// CR 714.2: Mark this trigger as the Saga chapter ability for `chapter`. + /// Only `parser::oracle_saga` may call this — it is the one place that has + /// read an actual chapter symbol. + pub fn saga_chapter(mut self, chapter: u32) -> Self { + self.saga_chapter = Some(chapter); + self + } + pub fn player_actions(mut self, actions: Vec) -> Self { self.player_actions = Some(actions); self @@ -26904,6 +26924,7 @@ mod tests { constraint: None, condition: None, counter_filter: None, + saga_chapter: None, unless_pay: None, batched: false, die_sides: None, diff --git a/crates/engine/tests/integration/narci_fable_singer_final_chapter_drain.rs b/crates/engine/tests/integration/narci_fable_singer_final_chapter_drain.rs index d73a57902c..488159027b 100644 --- a/crates/engine/tests/integration/narci_fable_singer_final_chapter_drain.rs +++ b/crates/engine/tests/integration/narci_fable_singer_final_chapter_drain.rs @@ -208,6 +208,91 @@ III — Create a 1/1 white Soldier creature token.", ); } +/// CR 608.2c: a final chapter ability that PAUSES mid-resolution for a choice +/// must not fire its observers until that resolution has actually finished. +/// +/// The chapter-resolution event is published next to the engine's own +/// `StackResolved`, before the settlement guard, so this pins the ordering +/// empirically rather than by assertion: the drain must not have landed while +/// the optional-effect prompt is still open, and must land exactly once after it +/// is answered. +#[test] +fn narci_does_not_drain_until_a_paused_final_chapter_finishes_resolving() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.with_library_top(P0, &["Forest", "Forest", "Forest", "Forest"]); + scenario.with_library_top(P1, &["Forest", "Forest", "Forest", "Forest"]); + + scenario + .add_creature(P0, "Narci, Fable Singer", 3, 3) + .as_legendary() + .from_oracle_text(NARCI_ORACLE); + + // Chapter II is the final chapter and pauses for an optional-effect choice + // during its own resolution. + let saga_id = scenario + .add_creature(P0, "Paused Test Saga", 0, 0) + .as_enchantment() + .with_subtypes(vec!["Saga"]) + .with_mana_cost(ManaCost::generic(3)) + .from_oracle_text( + "I — Create a 1/1 white Soldier creature token.\n\ +II — You may draw a card.", + ) + .id(); + scenario.with_counter(saga_id, CounterType::Lore, 1); + + let mut runner = scenario.build(); + let p1_life_before = runner.state().players[P1.0 as usize].life; + + park_for_next_p0_precombat_main(&mut runner); + runner.advance_to_phase(Phase::PreCombatMain); + runner.pass_both_players(); + runner.advance_to_phase(Phase::PreCombatMain); + assert_eq!(lore_count(&runner, saga_id), 2); + + // Walk the stack until the chapter ability's own optional choice is offered. + let mut saw_optional = false; + for _ in 0..64 { + if matches!(runner.state().waiting_for, WaitingFor::OrderTriggers { .. }) { + drain_order_triggers_with_identity(runner.state_mut()); + continue; + } + if let WaitingFor::OptionalEffectChoice { .. } = runner.state().waiting_for { + saw_optional = true; + assert_eq!( + runner.state().players[P1.0 as usize].life, + p1_life_before, + "the drain must not land while the final chapter ability is still resolving" + ); + runner + .act(GameAction::DecideOptionalEffect { accept: true }) + .expect("answer the chapter ability's optional draw"); + continue; + } + if runner.state().stack.is_empty() { + break; + } + if matches!(runner.state().waiting_for, WaitingFor::Priority { .. }) { + let _ = runner.act(GameAction::PassPriority); + let _ = runner.act(GameAction::PassPriority); + } else { + break; + } + } + + assert!( + saw_optional, + "reach guard: chapter II's optional draw must actually be offered, \ + otherwise this test never exercises a paused resolution" + ); + assert_eq!( + runner.state().players[P1.0 as usize].life - p1_life_before, + -3, + "after the paused chapter ability finishes, the drain lands exactly once" + ); +} + /// CR 603.2: the other end of the lifecycle axis — Historian's Boon observes the /// final chapter ability *triggering*, which is the Saga's own lore-counter /// threshold crossing, not a resolution. Narci's clause is identical except for From c0a1cf4c2cccf0abc3ccbceb51c2360ee9f25e9c Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Mon, 10 Aug 2026 23:29:58 -0500 Subject: [PATCH 4/5] fix(import): record chapter provenance on imported Saga chapters Follows the provenance change in the previous commit across the crate boundary. CI shard 2/4 caught manabrew-compat's Saga DTO test -- final_chapter_number returned None where it expected Some(3) -- because the previous commit was verified with `-p phase-engine` only, which is the wrong scope for a change to a shared type. Fixing that surfaced the more serious instance. `mtgish-import`'s saga converter is a PRODUCTION import path that builds chapter triggers from chapter ordinals, and it set only the lore threshold. Under the new model every Saga imported through it would have reported no final chapter number, so CR 714.4 would never sacrifice it, read-ahead (CR 702.155b) would find nothing to read ahead to, and a final-chapter observer would never see it. The converter has the ordinal in hand and simply was not recording it; it now does. Swept every counter_filter construction site in the workspace. The remaining ones build Time (CR 702.62a / CR 702.63a) and Defense (CR 310.12b) threshold triggers and correctly carry no chapter provenance -- which is exactly the distinction the field exists to make, and the one threshold-shape inference could not express. Also corrects CR 714.2a -> CR 714.2 on the converter's chapter-body comment: 714.2a defines only the Roman numeral, while 714.2 is what makes a chapter symbol a triggered ability. Note: `cargo test -p mtgish-import --lib` has two failures on this base (convert::action::tests::search_players_library_target_opponent_preserves_acquire_shape and convert::replacement::tests::would_deal_damage_fixed_actions_convert_to_typed_modifications). Both reproduce with this change reverted, neither touches Saga code, and no upstream commit since this branch's base touches those files. Co-Authored-By: Claude Opus 5 --- crates/manabrew-compat/src/lib.rs | 19 +++++++++++-------- crates/mtgish-import/src/convert/saga.rs | 8 +++++++- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/crates/manabrew-compat/src/lib.rs b/crates/manabrew-compat/src/lib.rs index 8fa9ce7fe8..f38c4a1898 100644 --- a/crates/manabrew-compat/src/lib.rs +++ b/crates/manabrew-compat/src/lib.rs @@ -6734,19 +6734,22 @@ mod tests { state.objects.get_mut(&class_id).unwrap().class_level = Some(2); let saga = state.objects.get_mut(&saga_id).unwrap(); saga.card_types.subtypes.push("Saga".to_string()); + // CR 714.2: `saga_chapter` is the chapter-symbol provenance that marks a + // trigger as a chapter ability; a bare lore threshold is not one, so + // `final_chapter_number` would report `None` without it. saga.trigger_definitions = vec![ - TriggerDefinition::new(TriggerMode::CounterAdded).counter_filter( - CounterTriggerFilter { + TriggerDefinition::new(TriggerMode::CounterAdded) + .counter_filter(CounterTriggerFilter { counter_type: CounterType::Lore, threshold: Some(1), - }, - ), - TriggerDefinition::new(TriggerMode::CounterAdded).counter_filter( - CounterTriggerFilter { + }) + .saga_chapter(1), + TriggerDefinition::new(TriggerMode::CounterAdded) + .counter_filter(CounterTriggerFilter { counter_type: CounterType::Lore, threshold: Some(3), - }, - ), + }) + .saga_chapter(3), ] .into(); diff --git a/crates/mtgish-import/src/convert/saga.rs b/crates/mtgish-import/src/convert/saga.rs index d9fcda3222..1e5e75276a 100644 --- a/crates/mtgish-import/src/convert/saga.rs +++ b/crates/mtgish-import/src/convert/saga.rs @@ -39,7 +39,7 @@ pub fn convert(chapters: &[SagaChapter]) -> ConvResult> { let mut out = Vec::new(); for chapter in chapters { let SagaChapter::SagaChapter(nums, actions) = chapter; - // CR 714.2a + CR 113.3a: Build the chapter body via the shared + // CR 714.2 + CR 113.3a: Build the chapter body via the shared // ActionsConversion pipeline so Modal / MayAction / MayCost / If / // Unless / IfElse / EachPlayerAction shapes lift through the same // ability-shaping code as a spell body. @@ -57,6 +57,12 @@ pub fn convert(chapters: &[SagaChapter]) -> ConvResult> { counter_type: EngineCounterType::Lore, threshold: Some(ordinal), }) + // CR 714.2: this ordinal IS a chapter symbol, so record the + // provenance. Without it the engine cannot tell these apart from + // any other lore threshold trigger, and `final_chapter_number` + // (CR 714.2d) — which CR 714.4's sacrifice and read-ahead both + // depend on — would report nothing for an imported Saga. + .saga_chapter(ordinal) .execute(exec.clone()) .trigger_zones(vec![Zone::Battlefield]) .description(format!("Chapter {ordinal}")); From 3e013fd42aaee760bbe82c640735c2d7725de14a Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Tue, 11 Aug 2026 00:53:47 -0500 Subject: [PATCH 5/5] fix(engine): bind the chapter-resolution event to its Saga incarnation Addresses the remaining review blocker on #7224. Both of the shapes I had considered were wrong, and the review named why: reading live state by storage id lets a re-entered Saga answer "that Saga's mana value", while guarding on an incarnation mismatch (the previous commit's fail-closed compromise) drops a firing that CR 113.7a says really did happen -- an already-triggered chapter ability resolves even after its Saga leaves. The fix needed no new machinery. TriggerSourceContext is already the engine's "complete event-time authority for a triggered source": captured when the chapter ability triggered, it pins the incarnation in `identity.reference` and carries that incarnation's `trigger_entries` and `lki`. The event now carries it instead of a raw saga_id, and every consumer reads from it: chapter numbers live object by id -> trigger_entries on the pinned incarnation (CR 714.2 / 714.2d) "a Saga you control" valid_card_matches_with_lki by id -> matches_target_filter_on_lki_snapshot on the pinned lki (CR 400.7) "that Saga's mana value" live -> LKI cache by id -> pinned lki.mana_value (CR 202.3) `resolving_saga_chapter` no longer consults `state.objects` at all, so the `is_current` guard is gone -- it was papering over reading the wrong source. Same-id confusion is now structurally excluded rather than guarded against. The blink regression discriminates on BOTH failure modes: the Saga re-enters at the same storage id with mana value 7 while the original was 3, so a live-state read drains 7, the old guard drains 0, and only the incarnation-bound read drains 3 exactly once. Also from this review round: - CR 608.2p is the rule this whole feature implements -- "Once all possible steps described in 608.2c-n are completed, any abilities that trigger when that spell or ability resolves trigger" -- and was never cited. It now annotates the event and its single emission site, and it settles the earlier question about publishing before the settlement guard by rule rather than by test alone. The paused-resolution test cites CR 608.2d for the mid-resolution choice; CR 608.2c (instruction order) described neither. - The manabrew Saga fixture gains a lore threshold with NO chapter provenance, at a threshold above the real final chapter. Without it the test passed under both the provenance contract and the threshold inference it replaced. Co-Authored-By: Claude Opus 5 --- client/src/adapter/types.ts | 6 +- crates/engine/src/game/quantity.rs | 26 ++++++ crates/engine/src/game/stack.rs | 87 ++++++++++-------- crates/engine/src/game/targeting.rs | 9 +- crates/engine/src/game/trigger_matchers.rs | 23 ++++- crates/engine/src/types/events.rs | 24 ++++- crates/engine/src/types/game_state.rs | 18 ++++ .../narci_fable_singer_final_chapter_drain.rs | 91 +++++++++++++++++-- crates/manabrew-compat/src/lib.rs | 10 ++ 9 files changed, 237 insertions(+), 57 deletions(-) diff --git a/client/src/adapter/types.ts b/client/src/adapter/types.ts index 05ee1431aa..05abf4a652 100644 --- a/client/src/adapter/types.ts +++ b/client/src/adapter/types.ts @@ -2463,7 +2463,11 @@ export type GameEvent = // CR 714.2: a Saga's chapter ability finished resolving. Bookkeeping the // engine publishes for meta-triggers (Narci, Fable Singer); non-visual, since // the chapter ability's own effects already animate. - | { type: "SagaChapterAbilityResolved"; data: { saga_id: ObjectId; controller: PlayerId; chapter: number; final_chapter: number } } + // `saga` is the engine's TriggerSourceContext for the exact Saga incarnation + // (CR 400.7). It is deliberately left unmodelled: this event is non-visual + // (see eventNormalizer) and the client never reads the payload, so declaring a + // partial shape here would assert a contract nothing checks. + | { type: "SagaChapterAbilityResolved"; data: { saga: unknown; controller: PlayerId; chapter: number; final_chapter: number } } | { type: "Discarded"; data: { player_id: PlayerId; object_id: ObjectId } } | { type: "EnduringStoryGained"; data: { player_id: PlayerId } } | { type: "DamageCleared"; data: { object_id: ObjectId } } diff --git a/crates/engine/src/game/quantity.rs b/crates/engine/src/game/quantity.rs index a3adc24415..f3d326793c 100644 --- a/crates/engine/src/game/quantity.rs +++ b/crates/engine/src/game/quantity.rs @@ -5813,6 +5813,23 @@ where /// CR 202.3: Resolve an object's mana value through the same ObjectScope axis /// used for power/toughness. Source scope falls back to LKI for objects that /// moved during resolution; target scope reads the selected object target. +/// CR 400.7 + CR 202.3: The mana value an event supplies for its OWN pinned +/// subject incarnation, when the current trigger event carries one. +/// +/// Only `SagaChapterAbilityResolved` pins a subject today. Its Saga is routinely +/// gone by the time an observer resolves — CR 714.4 sacrifices it the moment the +/// final chapter ability leaves the stack — and a re-entered Saga can occupy the +/// same storage id, so neither live state nor the id-keyed LKI cache can be +/// trusted to answer for the original. +fn event_source_mana_value_override(state: &GameState) -> Option { + match current_or_detection_trigger_event(state)? { + GameEvent::SagaChapterAbilityResolved { saga, .. } => { + Some(u32_to_i32_saturating(saga.lki.mana_value)) + } + _ => None, + } +} + fn resolve_object_mana_value( state: &GameState, scope: ObjectScope, @@ -5843,6 +5860,15 @@ fn resolve_object_mana_value( .map(|obj| u32_to_i32_saturating(obj.effective_mana_value())) .unwrap_or(0), ObjectScope::EventSource => { + // CR 400.7 + CR 202.3: an event that pins its own subject incarnation + // answers for that incarnation directly. Reading live state (or the + // id-keyed LKI cache) would let a re-entered permanent at the same + // storage id supply the value instead — Narci draining for the NEW + // Saga's mana value after a blink. Checked first, so the id-based + // fallback below only runs for events with no pinned subject. + if let Some(mana_value) = event_source_mana_value_override(state) { + return mana_value; + } let Some(object_id) = object_id_for_scope(state, ObjectScope::EventSource, ctx, targets) else { diff --git a/crates/engine/src/game/stack.rs b/crates/engine/src/game/stack.rs index 4ddb5a4aa9..4902c9b66b 100644 --- a/crates/engine/src/game/stack.rs +++ b/crates/engine/src/game/stack.rs @@ -10,7 +10,7 @@ use crate::types::events::GameEvent; use crate::types::game_state::{ AutoMayChoice, CastOfferKind, CastingVariant, ExileLink, ExileLinkKind, GameState, MayTriggerAutoChoiceKey, MayTriggerOrigin, PendingCounterPostAction, PendingSpellResolution, - StackEntry, StackEntryKind, StackPaidSnapshot, WaitingFor, + StackEntry, StackEntryKind, StackPaidSnapshot, TriggerSourceContext, WaitingFor, }; use crate::types::identifiers::{ObjectId, TriggerFiring}; use crate::types::player::PlayerId; @@ -992,44 +992,53 @@ pub(crate) fn bind_resolution_scope( /// CR 714.2 + CR 714.2d: The Saga-chapter identity of a stack entry that is /// about to resolve, or `None` if the entry is not a Saga chapter ability. -/// -/// Captured BEFORE the ability executes: a chapter ability may move its own -/// Saga off the battlefield as its effect (Fable of the Mirror-Breaker III -/// exiles and returns it transformed), and CR 714.4 sacrifices the Saga once its -/// final chapter ability has left the stack. Neither the chapter number nor the -/// final chapter number can be re-derived once the permanent is gone. struct ResolvingSagaChapter { - saga_id: ObjectId, + saga: TriggerSourceContext, controller: PlayerId, chapter: u32, final_chapter: u32, } -/// CR 714.2: Classify an about-to-resolve stack entry as a Saga chapter -/// ability, via the exact trigger occurrence that fired rather than any -/// re-derivation from the Saga's current lore count. -fn resolving_saga_chapter(state: &GameState, entry: &StackEntry) -> Option { - let StackEntryKind::TriggeredAbility { - source_id, ability, .. - } = &entry.kind - else { +/// CR 714.2 + CR 400.7: Classify an about-to-resolve stack entry as a Saga +/// chapter ability, reading everything from the trigger's own source context. +/// +/// Deliberately does NOT consult live state by `source_id`. `source_id` is +/// storage identity: a Saga that left and re-entered occupies the same id as a +/// different object, whose chapter abilities — and mana value — are not the ones +/// this ability triggered from. CR 113.7a lets that already-triggered chapter +/// ability resolve anyway, so reading live state would either report the wrong +/// Saga's numbers or (if guarded on incarnation) drop an occurrence that really +/// did resolve. +/// +/// `TriggerSourceContext` is the engine's existing answer to exactly this: it +/// was captured when the chapter ability triggered, pins the incarnation in +/// `identity.reference`, and carries that incarnation's `trigger_entries` and +/// `lki`. Both the chapter numbers below and every characteristic an observer +/// can later ask about therefore come from the right object by construction. +fn resolving_saga_chapter(entry: &StackEntry) -> Option { + let StackEntryKind::TriggeredAbility { ability, .. } = &entry.kind else { return None; }; - let definition_ref = ability.trigger_definition_ref.as_ref()?; - let saga = state.objects.get(source_id)?; - // CR 400.7: `source_id` is storage identity, not object identity. A Saga - // that left and re-entered can occupy the same id as a NEW object whose - // chapter abilities — and therefore whose chapter and final chapter numbers - // — are not the ones this ability triggered from. Fail closed on an - // incarnation mismatch: publishing numbers read off the wrong incarnation - // could misclassify a non-final chapter as final, or drain for the wrong - // mana value. Not firing in that exotic case is the recoverable error. - if !definition_ref.source.is_current(state) { - return None; - } - let (chapter, final_chapter) = saga.saga_chapter_for_occurrence(&definition_ref.occurrence)?; + let occurrence = &ability.trigger_definition_ref.as_ref()?.occurrence; + let saga = ability.trigger_source.as_ref()?; + + // CR 714.2: chapter numbers come from the chapter-symbol provenance on the + // source incarnation's own trigger entries, never from a live lore count. + let chapter = saga + .trigger_entries + .iter() + .find(|entry| &entry.occurrence == occurrence) + .and_then(|entry| entry.definition.saga_chapter)?; + // CR 714.2d: greatest chapter number among that same incarnation's chapter + // abilities. + let final_chapter = saga + .trigger_entries + .iter() + .filter_map(|entry| entry.definition.saga_chapter) + .max()?; + Some(ResolvingSagaChapter { - saga_id: *source_id, + saga: saga.clone(), controller: entry.controller, chapter, final_chapter, @@ -1139,7 +1148,7 @@ pub fn resolve_top(state: &mut GameState, events: &mut Vec) { // reachable — the chapter ability's own effect may remove it. Only the // success path below publishes it; a fizzle (CR 608.2b) or a failed // intervening-if (CR 603.4) leaves the stack without resolving. - let saga_chapter = resolving_saga_chapter(state, &entry); + let saga_chapter = resolving_saga_chapter(&entry); // Extract the resolved ability from the stack entry. `KeywordAction` is // handled by the early return above and never reaches this match. @@ -2546,15 +2555,17 @@ pub fn resolve_top(state: &mut GameState, events: &mut Vec) { events.push(GameEvent::StackResolved { object_id: entry.id, }); - // CR 714.2 + CR 608.2: This is the only exit from `resolve_top` on which a - // triggered ability actually RESOLVED — the fizzle, no-legal-target and - // failed-intervening-if paths returned earlier, each pushing their own - // `StackResolved`. Publishing the chapter-resolution event only here is what - // keeps "whenever the final chapter ability of a Saga you control resolves" - // (Narci, Fable Singer) from firing on a chapter ability that never did. + // CR 608.2p: "Once all possible steps described in 608.2c–n are completed, + // any abilities that trigger when that spell or ability resolves trigger." + // This is the only exit from `resolve_top` on which a triggered ability + // actually RESOLVED — the fizzle, no-legal-target and failed-intervening-if + // paths returned earlier, each pushing their own `StackResolved`. Publishing + // the chapter-resolution event only here is what keeps "whenever the final + // chapter ability of a Saga you control resolves" (Narci, Fable Singer) from + // firing on a chapter ability that never did. if let Some(chapter) = saga_chapter { events.push(GameEvent::SagaChapterAbilityResolved { - saga_id: chapter.saga_id, + saga: Box::new(chapter.saga), controller: chapter.controller, chapter: chapter.chapter, final_chapter: chapter.final_chapter, diff --git a/crates/engine/src/game/targeting.rs b/crates/engine/src/game/targeting.rs index e4498b2dca..235ffe83ad 100644 --- a/crates/engine/src/game/targeting.rs +++ b/crates/engine/src/game/targeting.rs @@ -1568,8 +1568,13 @@ pub(crate) fn extract_source_from_event( // CR 608.2k + CR 714.2e: "that Saga" in "each opponent loses X life … // where X is that Saga's mana value" (Narci, Fable Singer) is an // untargeted back-reference to the object the trigger condition named — - // the Saga whose chapter ability resolved. - GameEvent::SagaChapterAbilityResolved { saga_id, .. } => Some(*saga_id), + // the Saga whose chapter ability resolved. CR 400.7: this yields the id + // of the EXACT incarnation; callers that read a characteristic off it + // must prefer the event's own snapshot, because a re-entered Saga can + // occupy this same id (see `event_source_mana_value_override`). + GameEvent::SagaChapterAbilityResolved { saga, .. } => { + Some(saga.identity.reference.object_id) + } GameEvent::Evolved { object_id } => Some(*object_id), GameEvent::CounterRemoved { object_id, .. } => Some(*object_id), GameEvent::TokenCreated { object_id, .. } => Some(*object_id), diff --git a/crates/engine/src/game/trigger_matchers.rs b/crates/engine/src/game/trigger_matchers.rs index 3515d8e47e..ec1901697c 100644 --- a/crates/engine/src/game/trigger_matchers.rs +++ b/crates/engine/src/game/trigger_matchers.rs @@ -2259,16 +2259,29 @@ pub(super) fn match_saga_chapter_ability( ( AbilityLifecyclePoint::Resolved, GameEvent::SagaChapterAbilityResolved { - saga_id, + saga, chapter: resolved_chapter, final_chapter, .. }, ) => { - valid_card_matches_with_lki(trigger, state, *saga_id, source_context) - // CR 714.2e: the final chapter ability is the one whose chapter - // symbol carries the Saga's final chapter number (CR 714.2d). - && resolved_chapter == final_chapter + // CR 400.7: match "a Saga you control" against the SOURCE + // incarnation's own last-known characteristics, not against whatever + // now occupies its storage id. The Saga is routinely gone by now — + // CR 714.4 sacrifices it as soon as the final chapter ability leaves + // the stack — and may have been replaced by a re-entered copy. + let subject_matches = trigger.valid_card.as_ref().is_none_or(|filter| { + super::filter::matches_target_filter_on_lki_snapshot( + state, + saga.identity.reference.object_id, + &saga.lki, + filter, + &super::filter::FilterContext::from_trigger_source(source_context), + ) + }); + // CR 714.2e: the final chapter ability is the one whose chapter + // symbol carries the Saga's final chapter number (CR 714.2d). + subject_matches && resolved_chapter == final_chapter } ( AbilityLifecyclePoint::Triggered, diff --git a/crates/engine/src/types/events.rs b/crates/engine/src/types/events.rs index aa1b767646..f6955d8fe0 100644 --- a/crates/engine/src/types/events.rs +++ b/crates/engine/src/types/events.rs @@ -10,7 +10,7 @@ use super::ability::{ }; use super::card::PrintedCardRef; use super::card_type::{CardType, CoreType, Supertype}; -use super::game_state::ZoneChangeRecord; +use super::game_state::{TriggerSourceContext, ZoneChangeRecord}; use super::identifiers::{CardId, ObjectId, ObjectIncarnationRef, TrackedSetId}; use super::keywords::Keyword; use super::mana::ManaCost; @@ -1053,7 +1053,12 @@ pub enum GameEvent { #[serde(default)] actor: PlayerId, }, - /// CR 714.2 + CR 608.2: A Saga's chapter ability finished resolving. + /// CR 714.2 + CR 608.2p: A Saga's chapter ability finished resolving. + /// + /// CR 608.2p is the rule this event exists to serve: once every resolution + /// step is completed, abilities that trigger on that ability resolving + /// trigger. Nothing else on the bus reports that moment for a chapter + /// ability. /// /// A chapter ability is not a distinct AST concept — CR 714.2b defines a /// chapter symbol as a lore-counter threshold trigger on the Saga itself. @@ -1068,8 +1073,19 @@ pub enum GameEvent { /// sacrifices it as soon as the ability leaves the stack — after which /// neither number could be re-derived. SagaChapterAbilityResolved { - /// The Saga permanent whose chapter ability resolved. - saga_id: ObjectId, + /// CR 400.7 + CR 113.7a: The exact Saga incarnation whose chapter ability + /// resolved, with the characteristics it had when the ability triggered. + /// + /// The trigger's own source context, not a raw `ObjectId`, for the same + /// reason `ConniveSubject` carries a snapshot: a chapter ability already + /// on the stack still resolves after its Saga leaves and re-enters, and + /// the re-entered permanent can occupy the same storage id. A bare id + /// would let an observer's "that Saga" bind to the NEW incarnation and + /// read its mana value (CR 202.3); suppressing the event instead would + /// lose an occurrence that genuinely resolved. Carrying the context does + /// neither — `identity.reference` pins the incarnation and `lki` answers + /// every characteristic an observer can ask about. + saga: Box, /// CR 109.5: controller of the resolved chapter ability. controller: PlayerId, /// CR 714.2b: the chapter number (lore threshold) that resolved. diff --git a/crates/engine/src/types/game_state.rs b/crates/engine/src/types/game_state.rs index 20b7696d34..6b1ee0e89e 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -18420,6 +18420,24 @@ impl GameState { }) } + /// CR 400.7: Capture the exact incarnation-bound snapshot of an object, for + /// an event whose subject must survive that object leaving and re-entering + /// the battlefield at the same storage id. + /// + /// This is the general entry point; the capture body lives in + /// [`Self::capture_connive_subject`], which was the first caller to need it + /// (CR 701.50b/f) and remains a thin typed wrapper over the same snapshot. + /// There is deliberately no raw-`ObjectId` variant: a bare id cannot + /// distinguish the original incarnation from a re-entered one, which is the + /// whole reason this exists. + pub fn capture_event_object_snapshot( + &self, + object_id: ObjectId, + ) -> Option { + self.capture_connive_subject(object_id) + .map(|subject| subject.snapshot) + } + /// Builds the exact paused-delivery key from the replacement record before /// that record is consumed. Only a `ZoneChange` can belong to either /// logical zone-change owner. diff --git a/crates/engine/tests/integration/narci_fable_singer_final_chapter_drain.rs b/crates/engine/tests/integration/narci_fable_singer_final_chapter_drain.rs index 488159027b..bb4c795273 100644 --- a/crates/engine/tests/integration/narci_fable_singer_final_chapter_drain.rs +++ b/crates/engine/tests/integration/narci_fable_singer_final_chapter_drain.rs @@ -208,14 +208,17 @@ III — Create a 1/1 white Soldier creature token.", ); } -/// CR 608.2c: a final chapter ability that PAUSES mid-resolution for a choice -/// must not fire its observers until that resolution has actually finished. +/// CR 608.2d + CR 608.2p: a final chapter ability that PAUSES mid-resolution for +/// a choice must not fire its observers until that resolution has finished. /// -/// The chapter-resolution event is published next to the engine's own -/// `StackResolved`, before the settlement guard, so this pins the ordering -/// empirically rather than by assertion: the drain must not have landed while -/// the optional-effect prompt is still open, and must land exactly once after it -/// is answered. +/// CR 608.2d is the pause — a choice the effect offers is announced while +/// applying the effect, not earlier. CR 608.2p is the ordering this pins: "Once +/// all possible steps described in 608.2c–n are completed, any abilities that +/// trigger when that spell or ability resolves trigger." The chapter-resolution +/// event is published next to the engine's own `StackResolved`, before the +/// settlement guard, so this test establishes the rule empirically rather than by +/// assertion: the drain must not have landed while the optional-effect prompt is +/// still open, and must land exactly once after it is answered. #[test] fn narci_does_not_drain_until_a_paused_final_chapter_finishes_resolving() { let mut scenario = GameScenario::new(); @@ -293,6 +296,80 @@ II — You may draw a card.", ); } +/// CR 400.7 + CR 113.7a: the Saga leaves and RE-ENTERS at the same storage id +/// before its already-triggered final chapter ability resolves. +/// +/// CR 113.7a lets that ability resolve anyway, so the observer owes exactly one +/// firing — and CR 608.2k binds "that Saga" to the object the trigger condition +/// named, so X must be the ORIGINAL Saga's mana value. Both plausible shortcuts +/// fail this test: reading live state by storage id drains for the re-entered +/// Saga's mana value, and guarding on an incarnation mismatch drops the firing +/// altogether. +/// +/// The re-entry is simulated by bumping the incarnation and swapping the mana +/// cost in place, which is exactly the state a blink produces at this seam: same +/// `ObjectId`, new incarnation, different characteristics. +#[test] +fn narci_drains_for_the_original_saga_after_it_blinks_mid_resolution() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.with_library_top(P0, &["Forest", "Forest", "Forest", "Forest"]); + scenario.with_library_top(P1, &["Forest", "Forest", "Forest", "Forest"]); + + scenario + .add_creature(P0, "Narci, Fable Singer", 3, 3) + .as_legendary() + .from_oracle_text(NARCI_ORACLE); + + let saga_id = scenario + .add_creature(P0, "Test Saga", 0, 0) + .as_enchantment() + .with_subtypes(vec!["Saga"]) + .with_mana_cost(ManaCost::generic(3)) + .from_oracle_text(SAGA_ORACLE) + .id(); + scenario.with_counter(saga_id, CounterType::Lore, 1); + + let mut runner = scenario.build(); + let p0_life_before = runner.state().players[P0.0 as usize].life; + let p1_life_before = runner.state().players[P1.0 as usize].life; + + park_for_next_p0_precombat_main(&mut runner); + runner.advance_to_phase(Phase::PreCombatMain); + runner.pass_both_players(); + runner.advance_to_phase(Phase::PreCombatMain); + assert_eq!(lore_count(&runner, saga_id), 2); + + // The final chapter ability has triggered and is on the stack. Re-enter the + // Saga at the same id with a DIFFERENT mana value before it resolves; if the + // drain reads 7 instead of 3, it bound to the wrong incarnation. + assert!( + !runner.state().stack.is_empty(), + "reach guard: the final chapter ability must be on the stack before the blink" + ); + { + let saga = runner + .state_mut() + .objects + .get_mut(&saga_id) + .expect("Saga still present"); + saga.bump_incarnation(); + saga.mana_cost = ManaCost::generic(7); + } + + drain_stack(&mut runner); + + assert_eq!( + ( + runner.state().players[P0.0 as usize].life - p0_life_before, + runner.state().players[P1.0 as usize].life - p1_life_before, + ), + (3, -3), + "the drain must use the ORIGINAL Saga's mana value (3), not the re-entered one (7), \ + and must fire exactly once" + ); +} + /// CR 603.2: the other end of the lifecycle axis — Historian's Boon observes the /// final chapter ability *triggering*, which is the Saga's own lore-counter /// threshold crossing, not a resolution. Narci's clause is identical except for diff --git a/crates/manabrew-compat/src/lib.rs b/crates/manabrew-compat/src/lib.rs index f38c4a1898..b2ddfb2d2e 100644 --- a/crates/manabrew-compat/src/lib.rs +++ b/crates/manabrew-compat/src/lib.rs @@ -6750,6 +6750,16 @@ mod tests { threshold: Some(3), }) .saga_chapter(3), + // CR 714.2: a lore threshold WITHOUT chapter-symbol provenance is not + // a chapter ability. Its threshold is deliberately higher than the + // real final chapter, so this fixture fails if `final_chapter_number` + // ever regresses to inferring chapters from thresholds. + TriggerDefinition::new(TriggerMode::CounterAdded).counter_filter( + CounterTriggerFilter { + counter_type: CounterType::Lore, + threshold: Some(99), + }, + ), ] .into();