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
24 changes: 24 additions & 0 deletions crates/engine/src/game/engine_resolution_choices.rs
Original file line number Diff line number Diff line change
Expand Up @@ -616,9 +616,33 @@ fn park_search_observer_triggers(
events: &[GameEvent],
events_before_drain: usize,
) -> ResolutionChoiceOutcome {
// CR 603.2 + CR 603.3b + CR 701.23: park the search's post-put/shuffle
// observer events for the next priority checkpoint. The ZoneChanged events
// for cards put onto the battlefield by the search's ChangeZone delivery
// were ALREADY collected (once) by the zone-change pipeline's segment /
// settlement collections (`append_and_collect_logical_zone_trigger_segment`
// / `complete_logical_zone_trigger_collection`). Those same `ZoneChanged`
// occurrences sit in `events[events_before_drain..]`, so collecting this
// whole slice again would DOUBLE-fire every entrant's observer triggers
// (landfall, ETB observers) — a single land entry fires landfall twice.
//
// Mirror the generic post-priority scan's `deferred_logical_zone_events`
// guard (engine_priority.rs): drop any `ZoneChanged` occurrence whose event
// is already represented in `state.deferred_triggers`. Other event kinds
// (EffectResolved, Shuffle, PlayerPerformedAction, ...) remain eligible so
// their own observers are still parked.
let retained_zone_events: Vec<_> = state
.deferred_triggers
.iter()
.flat_map(|context| context.trigger_events.iter())
.filter(|event| matches!(event, GameEvent::ZoneChanged { .. }))
.collect();
let trigger_events: Vec<GameEvent> = events[events_before_drain..]
.iter()
.filter(|ev| !matches!(ev, GameEvent::PhaseChanged { .. }))
.filter(|ev| {
!matches!(ev, GameEvent::ZoneChanged { .. }) || !retained_zone_events.contains(ev)
})
.cloned()
.collect();
if !trigger_events.is_empty() {
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 @@ -1158,6 +1158,7 @@ mod sarkhan_dragon_ascendant_behold;
mod scarblade_malice_delayed_dies_762;
mod scry_choice_not_clobbered_by_triggers;
mod scry_substituted_draw_per_card_replacement;
mod search_landfall_double_fire_repro;
mod secret_of_bloodbending_control_window;
mod she_hulk_wallbreaker_becomes_blocked_4599;
mod shelob_repro_token;
Expand Down
155 changes: 155 additions & 0 deletions crates/engine/tests/integration/search_landfall_double_fire_repro.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
//! Repro for the reported double-fire: casting a search tutor that puts a land
//! onto the battlefield (Prishe's Wanderings / Nature's Lore pattern) fires each
//! landfall observer trigger twice for the single land entry.
//!
//! A landfall permanent ("Whenever a land enters the battlefield under your
//! control, <effect>") must produce EXACTLY ONE trigger per land entry.

use engine::game::scenario::{GameScenario, P0};
use engine::types::mana::ManaColor;
use engine::types::phase::Phase;
use engine::types::zones::Zone;

const LANDFALL_ORACLE: &str =
"Whenever a land enters the battlefield under your control, draw a card.";

const NATURES_LORE_ORACLE: &str =
"Search your library for a Forest card, put that card onto the battlefield, then shuffle.";

fn seed_forest_on_library_top(
runner: &mut engine::game::scenario::GameRunner,
) -> engine::types::identifiers::ObjectId {
use engine::types::card_type::CoreType;
let card_id = engine::types::identifiers::CardId(runner.state().next_object_id);
let id = engine::game::zones::create_object(
runner.state_mut(),
card_id,
P0,
"Forest".to_string(),
Zone::Library,
);
let obj = runner.state_mut().objects.get_mut(&id).unwrap();
obj.card_types.core_types.push(CoreType::Land);
obj.base_card_types = obj.card_types.clone();
obj.card_types.subtypes.push("Forest".to_string());
runner.state_mut().players[P0.0 as usize]
.library
.insert(0, id);
id
}

#[test]
fn landfall_fires_once_per_land_etb_from_search_tutor() {
let mut scenario = GameScenario::new();
scenario.at_phase(Phase::PreCombatMain);
scenario.add_basic_land(P0, ManaColor::Green);

// A landfall permanent on the battlefield.
scenario
.add_creature_from_oracle(P0, "Landfall Scout", 1, 1, LANDFALL_ORACLE)
.id();

let natures_lore = scenario
.add_spell_to_hand_from_oracle(P0, "Nature's Lore", false, NATURES_LORE_ORACLE)
.id();

let mut runner = scenario.build();
let forest = seed_forest_on_library_top(&mut runner);
runner.cast(natures_lore).search_first_legal().resolve();

// The land entered the battlefield exactly once.
assert_eq!(
runner.state().objects[&forest].zone,
Zone::Battlefield,
"Nature's Lore must put the searched Forest onto the battlefield"
);

// Drain the search tutor to a priority window so deferred triggers settle.
runner.advance_until_stack_empty();

// Exactly ONE landfall trigger must have fired for the single land entry.
let landfall_count = runner
.state()
.deferred_triggers
.iter()
.filter(|ctx| {
ctx.pending
.description
.as_deref()
.unwrap_or("")
.contains("Whenever a land enters")
})
.count();
assert_eq!(
landfall_count,
1,
"landfall must fire exactly once per land ETB; deferred=[{:?}]",
runner
.state()
.deferred_triggers
.iter()
.map(|c| c.pending.description.clone().unwrap_or_default())
.collect::<Vec<_>>()
);
}

/// The reported shape: TWO separate landfall permanents on the battlefield and
/// one land entering via a search tutor. Each landfall permanent must fire
/// exactly ONE trigger for the single land — two total, never four (the
/// double-fire reported for Sazh's Chocobo + Bird token).
#[test]
fn two_landfall_sources_fire_once_each_for_single_search_land() {
let mut scenario = GameScenario::new();
scenario.at_phase(Phase::PreCombatMain);
scenario.add_basic_land(P0, ManaColor::Green);

scenario
.add_creature_from_oracle(P0, "Landfall Scout", 1, 1, LANDFALL_ORACLE)
.id();
scenario
.add_creature_from_oracle(P0, "Landfall Warden", 2, 2, LANDFALL_ORACLE)
.id();
Comment on lines +106 to +111

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Assert each landfall source ID.

Lines 106 and 109 discard the source IDs. The aggregate count at lines 129-140 can pass if one source fires twice and the other source does not fire. Retain both IDs. Assert that the matching deferred triggers contain each ID exactly once.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/engine/tests/integration/search_landfall_double_fire_repro.rs` around
lines 106 - 111, Retain the IDs returned by both add_creature_from_oracle calls
for “Landfall Scout” and “Landfall Warden” instead of discarding them. Update
the deferred-trigger assertions around the aggregate count to verify each
retained source ID appears exactly once, preventing one source from firing twice
while the other does not.

Source: Path instructions


let natures_lore = scenario
.add_spell_to_hand_from_oracle(P0, "Nature's Lore", false, NATURES_LORE_ORACLE)
.id();

let mut runner = scenario.build();
let forest = seed_forest_on_library_top(&mut runner);
runner.cast(natures_lore).search_first_legal().resolve();
runner.advance_until_stack_empty();

// The land entered the battlefield exactly once.
assert_eq!(
runner.state().objects[&forest].zone,
Zone::Battlefield,
"Nature's Lore must put the searched Forest onto the battlefield"
);

let landfall_count = runner
.state()
.deferred_triggers
.iter()
.filter(|ctx| {
ctx.pending
.description
.as_deref()
.unwrap_or("")
.contains("Whenever a land enters")
})
.count();
assert_eq!(
landfall_count,
2,
"two landfall sources must fire exactly once each for the single land ETB; deferred=[{:?}]",
runner
.state()
.deferred_triggers
.iter()
.map(|c| (
c.pending.source_id,
c.pending.description.clone().unwrap_or_default()
))
.collect::<Vec<_>>()
);
}
Loading