Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions crates/engine/src/game/ability_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
54 changes: 54 additions & 0 deletions crates/engine/src/parser/oracle_target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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 <type>" selector must carry FilterProp::IsSaddled
Expand Down
1 change: 1 addition & 0 deletions crates/engine/tests/integration/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
141 changes: 141 additions & 0 deletions crates/engine/tests/integration/nettling_imp_continuity_target.rs
Original file line number Diff line number Diff line change
@@ -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"
);
}
10 changes: 4 additions & 6 deletions docs/parser-misparse-backlog.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@
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.

## Ranked root causes

| # | 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 |
Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -496,15 +496,13 @@ 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.
- Night Nurse, Healer of Heroes
- Nightshade Seer
- Nimble Obstructionist
- Noctis, Heir Apparent
- Norritt
- Not of This World
- Oath of Druids
- Oath of Ghouls
Expand Down
Loading