diff --git a/crates/engine/src/game/ability_utils.rs b/crates/engine/src/game/ability_utils.rs index 2e975479f3..523e37243c 100644 --- a/crates/engine/src/game/ability_utils.rs +++ b/crates/engine/src/game/ability_utils.rs @@ -9127,6 +9127,78 @@ mod tests { assert!(matches!(err, EngineError::ActionNotAllowed(_))); } + // Nettling Imp / Norritt / Arcum's Whistle class: "target creature the active + // player has controlled continuously since the beginning of the turn". Proves + // BOTH predicates gate legality independently — the active-player controller + // scope AND the continuity flag — via three fixtures, only one of which is a + // legal target. Positive full-set assertion (not a bare negative) so neither + // hostile fixture can be excluded vacuously by an upstream short-circuit. + #[test] + fn build_target_slots_active_player_controlled_continuously_since_turn_began() { + use crate::types::ability::{ControllerRef, FilterProp}; + use crate::types::zones::Zone; + + let mut state = GameState::new_two_player(42); + // Active player (CR 102.1) is PlayerId(0) at game start. + assert_eq!(state.active_player, PlayerId(0)); + + // Legal: active player's control, no summoning sickness (create_object + // does not set the flag — a "pre-existing" battlefield creature). + let continuous_active = create_object( + &mut state, + CardId(1), + PlayerId(0), + "Continuous Active".to_string(), + Zone::Battlefield, + ); + // Illegal via continuity: active player's control, but summoning-sick. + let fresh_active = create_object( + &mut state, + CardId(2), + PlayerId(0), + "Fresh Active".to_string(), + Zone::Battlefield, + ); + // Illegal via controller: continuous control, but by the non-active player. + let continuous_nonactive = create_object( + &mut state, + CardId(3), + PlayerId(1), + "Continuous Nonactive".to_string(), + Zone::Battlefield, + ); + for creature in [continuous_active, fresh_active, continuous_nonactive] { + state + .objects + .get_mut(&creature) + .unwrap() + .card_types + .core_types + .push(CoreType::Creature); + } + state.objects.get_mut(&fresh_active).unwrap().summoning_sick = true; + + let ability = ResolvedAbility::new( + Effect::TargetOnly { + target: TargetFilter::Typed( + TypedFilter::creature() + .controller(ControllerRef::ActivePlayer) + .properties(vec![FilterProp::ControlledContinuouslySinceTurnBegan]), + ), + }, + vec![], + ObjectId(900), + PlayerId(0), + ); + + let slots = build_target_slots(&state, &ability).expect("target slots should build"); + assert_eq!(slots.len(), 1); + assert_eq!( + slots[0].legal_targets, + vec![TargetRef::Object(continuous_active)] + ); + } + #[test] fn build_target_slots_uses_prior_player_targets_for_relative_controller_filters() { use crate::types::ability::ControllerRef; diff --git a/crates/engine/src/parser/oracle_target.rs b/crates/engine/src/parser/oracle_target.rs index 87985af567..08b2bf1959 100644 --- a/crates/engine/src/parser/oracle_target.rs +++ b/crates/engine/src/parser/oracle_target.rs @@ -5434,6 +5434,40 @@ fn parse_ownership_or_controller_suffix( } return own_ctrl_offset + (own_ctrl.len() - rest.len()); } + // CR 102.1 + CR 302.6 + CR 508.1a: "the active player has controlled + // continuously since the beginning of the turn" is a target-selection + // relative clause (Nettling Imp / Norritt / Arcum's Whistle) — distinct + // from `parse_controller_suffix`'s past-tense LOOK-BACK arm ("the active + // player controlled", CR 608.2i, used by look-back aggregates over + // objects that may have since left the battlefield). This clause instead + // restricts a LIVE battlefield target: it must both (a) currently be + // controlled by the active player and (b) have been so controlled + // without interruption since that player's turn began — the same + // continuity test CR 508.1a uses to gate which creatures may attack. + // Both facts are pushed together: `*controller` pins the live + // ActivePlayer scope, and `FilterProp::ControlledContinuouslySinceTurnBegan` + // pins the continuity predicate (runtime-evaluated in game/filter.rs as + // `!obj.summoning_sick`, CR 302.6's summoning-sickness flag). Only the + // ActivePlayer subject is recognized — no card in the corpus was found + // using a "you've controlled/you have controlled continuously..." form + // for this clause, so that variant is not built ahead of a card that + // needs it. The clause is sequenced from three composable atoms — subject + // ("the active player"), verb ("has controlled"), and continuity tail + // ("continuously since the beginning of the turn") — rather than one + // verbatim tag, mirroring `parse_continuity_exemption_clause` (oracle.rs) + // and the "owned by" tuple idiom immediately above. + let active_player_continuity: nom::IResult<&str, (), OracleError<'_>> = ( + tag("the active player"), + tag(" has controlled"), + tag(" continuously since the beginning of the turn"), + ) + .map(|_| ()) + .parse(own_ctrl); + if let Ok((rest, ())) = active_player_continuity { + *controller = Some(ControllerRef::ActivePlayer); + properties.push(FilterProp::ControlledContinuouslySinceTurnBegan); + return own_ctrl_offset + (own_ctrl.len() - rest.len()); + } let (ctrl, ctrl_len) = parse_controller_suffix(text, ctx).map_or((None, 0), |(ctrl, len)| (Some(ctrl), len)); @@ -7991,6 +8025,26 @@ mod tests { assert_eq!(rest, ""); } + // Nettling Imp / Norritt / Arcum's Whistle class: verbatim clause from + // Nettling Imp's real Oracle text. Building-block test — isolates the new + // controller+continuity arm from the pre-existing "non-Wall" type-filter + // handling (already covered elsewhere). + #[test] + fn parse_target_active_player_controlled_continuously_since_turn_began() { + let (f, rest) = parse_target( + "target creature the active player has controlled continuously since the beginning of the turn", + ); + assert_eq!( + f, + TargetFilter::Typed( + TypedFilter::creature() + .controller(ControllerRef::ActivePlayer) + .properties(vec![FilterProp::ControlledContinuouslySinceTurnBegan]) + ) + ); + assert_eq!(rest.trim(), ""); + } + #[test] fn saddled_type_phrase_parses_as_target() { // CR 702.171b: a "saddled " selector must carry FilterProp::IsSaddled diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 0e02241b28..63c4c23356 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -531,6 +531,7 @@ mod mycoloth_upkeep_trigger; mod mystic_forge_regression; mod narset_jeskai_waymaster_draw_spells_cast; mod necrodominance_pay_any_life_draw; +mod nettling_imp_continuity_target; mod ninjutsu_cluster; mod nix_counter_no_mana_spent; mod ob_nixilis_captive_kingpin_life_loss; diff --git a/crates/engine/tests/integration/nettling_imp_continuity_target.rs b/crates/engine/tests/integration/nettling_imp_continuity_target.rs new file mode 100644 index 0000000000..44894c6954 --- /dev/null +++ b/crates/engine/tests/integration/nettling_imp_continuity_target.rs @@ -0,0 +1,141 @@ +//! Nettling Imp / Norritt / Arcum's Whistle continuity target clause — PR #5463 +//! fix round, addressing the maintainer's "test doesn't cover the actual +//! regression" finding. +//! +//! The two pre-existing tests are each insufficient alone: +//! * `parse_target_active_player_controlled_continuously_since_turn_began` +//! (oracle_target.rs) only checks the isolated `parse_target` AST shape — it +//! never reaches target selection. +//! * `build_target_slots_active_player_controlled_continuously_since_turn_began` +//! (ability_utils.rs) hand-constructs the `FilterProp`, bypassing the parser +//! entirely — reverting the new parser arm leaves it green. +//! +//! This end-to-end test closes the gap. It parses Nettling Imp's REAL Oracle +//! text through the top-level `parse_oracle_text` entry point and drives the +//! resulting PARSER-PRODUCED `AbilityDefinition` through the production +//! `build_resolved_from_def` -> `build_target_slots` path (the same path used by +//! the live activation flow). If the decomposed continuity arm in +//! `parse_ownership_or_controller_suffix` is reverted or broken, the parsed +//! filter degrades to non-Wall-creature-only (no `ActivePlayer` controller, no +//! `ControlledContinuouslySinceTurnBegan` property), so `build_target_slots` +//! would then wrongly admit the summoning-sick and non-active-player fixtures — +//! failing the exact `legal_targets` assertion below. + +use engine::game::ability_utils::{build_resolved_from_def, build_target_slots}; +use engine::game::zones::create_object; +use engine::parser::oracle::parse_oracle_text; +use engine::types::ability::{ControllerRef, FilterProp, TargetFilter, TargetRef}; +use engine::types::card_type::CoreType; +use engine::types::game_state::GameState; +use engine::types::identifiers::CardId; +use engine::types::zones::Zone; +use engine::types::PlayerId; + +/// Verbatim Scryfall Oracle text for Nettling Imp. +const NETTLING_IMP_ORACLE: &str = "{T}: Choose target non-Wall creature the active player has controlled continuously since the beginning of the turn. That creature attacks this turn if able. Destroy it at the beginning of the next end step if it didn't attack this turn. Activate only during an opponent's turn, before attackers are declared."; + +#[test] +fn nettling_imp_parsed_continuity_filter_restricts_target_slots_end_to_end() { + // --- Parse the real card through the real top-level entry point. --- + let parsed = parse_oracle_text( + NETTLING_IMP_ORACLE, + "Nettling Imp", + &[], + &["Creature".to_string()], + &["Imp".to_string()], + ); + assert_eq!( + parsed.abilities.len(), + 1, + "expected the single {{T}} activated ability" + ); + let ability_def = &parsed.abilities[0]; + + // Positive reach-guard: prove the parse produced the FULL continuity filter + // (active-player controller + continuity property), not the degraded + // non-Wall-creature-only fallback. This is the exact seam the Finding 1 + // decomposition parses; if that arm regresses, both facts vanish here and + // the slot assertion below would no longer be a meaningful discriminator. + let parsed_filter = ability_def + .effect + .target_filter() + .expect("Nettling Imp surfaces a target-selection filter"); + let TargetFilter::Typed(typed) = parsed_filter else { + panic!("expected a Typed target filter, got {parsed_filter:?}"); + }; + assert_eq!( + typed.controller, + Some(ControllerRef::ActivePlayer), + "continuity clause must pin the ActivePlayer controller scope" + ); + assert!( + typed + .properties + .contains(&FilterProp::ControlledContinuouslySinceTurnBegan), + "continuity clause must carry ControlledContinuouslySinceTurnBegan, got {:?}", + typed.properties + ); + + // --- Three fixtures; only one is a legal target. --- + let mut state = GameState::new_two_player(42); + // CR 102.1: the active player is PlayerId(0) at game start. + assert_eq!(state.active_player, PlayerId(0)); + + // Legal: active player's control, no summoning sickness (create_object does + // not set the flag — a "pre-existing" battlefield creature). + let continuous_active = create_object( + &mut state, + CardId(1), + PlayerId(0), + "Continuous Active".to_string(), + Zone::Battlefield, + ); + // Illegal via continuity: active player's control, but summoning-sick. + let fresh_active = create_object( + &mut state, + CardId(2), + PlayerId(0), + "Fresh Active".to_string(), + Zone::Battlefield, + ); + // Illegal via controller: continuous control, but by the non-active player. + let continuous_nonactive = create_object( + &mut state, + CardId(3), + PlayerId(1), + "Continuous Nonactive".to_string(), + Zone::Battlefield, + ); + for creature in [continuous_active, fresh_active, continuous_nonactive] { + state + .objects + .get_mut(&creature) + .unwrap() + .card_types + .core_types + .push(CoreType::Creature); + } + state.objects.get_mut(&fresh_active).unwrap().summoning_sick = true; + + // The Nettling Imp source itself, controlled by the active player. + let imp = create_object( + &mut state, + CardId(4), + PlayerId(0), + "Nettling Imp".to_string(), + Zone::Battlefield, + ); + + // --- Drive the PARSED ability through the production target-slot builder. --- + let resolved = build_resolved_from_def(ability_def, imp, PlayerId(0)); + let slots = build_target_slots(&state, &resolved).expect("target slots should build"); + assert_eq!(slots.len(), 1, "single target selection slot"); + // Revert-failing assertion: with the continuity arm reverted the parsed + // filter is non-Wall-creature-only, so this set would also contain + // `fresh_active` and `continuous_nonactive`. + assert_eq!( + slots[0].legal_targets, + vec![TargetRef::Object(continuous_active)], + "only the continuously-active-player-controlled creature is a legal target" + ); +} diff --git a/docs/parser-misparse-backlog.md b/docs/parser-misparse-backlog.md index f2e742e514..56e3e5c07c 100644 --- a/docs/parser-misparse-backlog.md +++ b/docs/parser-misparse-backlog.md @@ -3,8 +3,8 @@ Consolidated from 50 per-batch clustering passes over the whole card database. Synonymous per-batch clusters were merged into canonical root causes, their card lists unioned and deduped, and ranked by total card appearances (largest first). - **Canonical root causes:** 30 -- **Distinct cards implicated:** 4778 -- **Total card appearances across root causes:** 4812 (a card may appear under more than one root cause when it exhibits multiple distinct misparses) +- **Distinct cards implicated:** 4776 +- **Total card appearances across root causes:** 4810 (a card may appear under more than one root cause when it exhibits multiple distinct misparses) This is the prioritized "fix N root causes → unlock M cards" backlog: the top handful of root causes account for the majority of broken cards. @@ -12,7 +12,7 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top | # | Root cause | # cards | Fix hint (where it likely lives) | |---|------------|--------:|----------------------------------| -| 1 | Relative-clause / filter restriction on target dropped | 752 | oracle_target.rs / game/filter.rs — extend TargetFilter property extraction for trailing relative clauses | +| 1 | Relative-clause / filter restriction on target dropped | 750 | oracle_target.rs / game/filter.rs — extend TargetFilter property extraction for trailing relative clauses | | 2 | Dropped intervening-if / gating condition (condition: null) | 606 | oracle_nom/condition.rs parse_inner_condition — trigger/static parsers must delegate condition extraction here | | 3 | Anaphor bound to wrong referent | 404 | oracle_quantity.rs context-ref resolution + game/ability_utils.rs forward_result wiring | | 4 | Conjoined / chained second effect clause dropped | 387 | oracle.rs effect-chain composition — split on 'and'/'then'/sentence boundaries and build sub_ability chain | @@ -47,7 +47,7 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top ## Full card lists per root cause -### 1. Relative-clause / filter restriction on target dropped (752 cards) +### 1. Relative-clause / filter restriction on target dropped (750 cards) **Signature.** TargetFilter/affected emitted with empty or missing properties; a trailing restrictive clause (type, subtype, color, mana value, zone, combat/temporal/control predicate, exclusion) is silently dropped, over-broadening the filter. @@ -496,7 +496,6 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top - Necrotic Plague - Needle Drop - Netcaster Spider -- Nettling Imp - Neural Network - Niambi, Esteemed Speaker - Nick Fury, Agent of S.H.I.E.L.D. @@ -504,7 +503,6 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top - Nightshade Seer - Nimble Obstructionist - Noctis, Heir Apparent -- Norritt - Not of This World - Oath of Druids - Oath of Ghouls