diff --git a/crates/engine/src/game/elimination.rs b/crates/engine/src/game/elimination.rs index 1fc3a7cfc7..45254f8ba8 100644 --- a/crates/engine/src/game/elimination.rs +++ b/crates/engine/src/game/elimination.rs @@ -920,7 +920,6 @@ fn do_eliminate( // player leaving the game, so the consult is skipped while the // unconditional primitive guards still run (PLAN ยง3). exile_owned_objects_on_player_left_game(state, player, events); - retire_trigger_grants_owned_by(state, player); crate::game::planechase::finish_player_left_game_handoff(state, planar_handoff, events); state.auto_pass.remove(&player); @@ -1176,18 +1175,6 @@ fn abandon_change_zone_family_for_controller(state: &mut GameState, player: Play }; } -/// CR 800.4a: The trigger-grant registry is object-local serialized state. -/// Once its owner has left, no active producer may survive to a later layer -/// reconciliation; preserve the monotonic allocator while retiring all active -/// instances so no occurrence is resurrected. -fn retire_trigger_grants_owned_by(state: &mut GameState, player: PlayerId) { - for (_, object) in state.objects.iter_mut() { - if object.owner == player { - object.trigger_occurrence_state.retire_all_grants(); - } - } -} - /// CR 104.2a: A player wins if all opponents have left. CR 104.3g: A team loses if all members have lost. /// /// Check if the game should end. Game ends when 1 or fewer living players/teams remain. diff --git a/crates/engine/src/game/ledger.rs b/crates/engine/src/game/ledger.rs index 20887870bf..89bdebc2f0 100644 --- a/crates/engine/src/game/ledger.rs +++ b/crates/engine/src/game/ledger.rs @@ -514,3 +514,57 @@ pub fn apply_resolved_ledger_edit( fn history_len(len: usize) -> Result { u32::try_from(len).map_err(|_| ResolvedLedgerEditReplayInvariantError::CounterOverflow) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::game::game_object::GameObject; + use crate::types::ability::{TriggerDefinition, TriggerDefinitionOccurrenceRef, TriggerEntry}; + use crate::types::identifiers::ObjectId; + use crate::types::player::PlayerId; + use crate::types::resolved_commands::{ResolvedCommandOrdinal, RulesExecutionNodeRef}; + use crate::types::triggers::TriggerMode; + use crate::types::zones::Zone; + use crate::types::CardId; + + #[test] + fn max_times_replay_resolves_recipient_key() { + let object_id = ObjectId(1); + let mut state = GameState::new_two_player(42); + let mut object = GameObject::new( + object_id, + CardId(1), + PlayerId(0), + "Granted trigger".to_string(), + Zone::Battlefield, + ); + let entry = TriggerEntry::new( + TriggerDefinitionOccurrenceRef::Printed { + base_set: object.trigger_base_set_instance, + printed_index: 0, + }, + TriggerDefinition::new(TriggerMode::Attacks), + ); + let trigger = object.trigger_definition_ref(&entry); + object.trigger_definitions.push(entry); + state.objects.insert(object_id, object); + state + .trigger_fire_counts_this_turn + .insert(trigger.clone(), 2); + + let command = ResolvedLedgerEditCommand { + edit: ResolvedLedgerEdit::TriggerFired { + trigger: trigger.clone(), + edit: ResolvedTriggerLedgerEdit::MaxTimesPerTurn { expected_old: 2 }, + }, + cause: RulesExecutionNodeRef::Proposal(ResolvedCommandOrdinal(0)), + }; + + apply_resolved_ledger_edit(&mut state, &command).expect("legacy replay resolves grant key"); + assert_eq!( + state.trigger_fire_counts_this_turn.get(&trigger).copied(), + Some(3) + ); + assert_eq!(state.trigger_fire_counts_this_turn.len(), 1); + } +} diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index 9257c2629f..e0aa1848b4 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -92,25 +92,21 @@ mod trigger_occurrence_tests { fn identical_grants_from_distinct_producers_remain_distinct_entries() { let definition = TriggerDefinition::new(TriggerMode::Attacks); let mut state = TriggerOccurrenceState::default(); + let first_producer = TriggerGrantProducerKey::Granted { + origin: static_origin(), + output_index: 0, + }; + let second_producer = TriggerGrantProducerKey::Granted { + origin: TriggerProducerOrigin::Transient { + continuous_effect_id: 19, + modification_index: 0, + }, + output_index: 0, + }; let entries = state .reconcile_trigger_entries(vec![ - ( - TriggerGrantProducerKey::Granted { - origin: static_origin(), - output_index: 0, - }, - definition.clone(), - ), - ( - TriggerGrantProducerKey::Granted { - origin: TriggerProducerOrigin::Transient { - continuous_effect_id: 19, - modification_index: 0, - }, - output_index: 0, - }, - definition, - ), + (first_producer.clone(), definition.clone()), + (second_producer.clone(), definition), ]) .unwrap(); assert_eq!(entries.len(), 2); @@ -179,31 +175,6 @@ mod trigger_occurrence_tests { .0; assert_eq!(instance, TriggerGrantInstanceRef(1)); } - - #[test] - fn abandoning_a_recipient_retires_grants_without_rewinding_the_allocator() { - let producer = TriggerGrantProducerKey::Granted { - origin: static_origin(), - output_index: 0, - }; - let mut state = TriggerOccurrenceState::default(); - let first = state - .reconcile_grant_instances(vec![(producer.clone(), ())]) - .unwrap()[0] - .0; - - state.retire_all_grants(); - assert_eq!(state.active_grants().count(), 0); - - let replacement = state - .reconcile_grant_instances(vec![(producer, ())]) - .unwrap()[0] - .0; - assert!( - replacement.0 > first.0, - "an abandoned recipient must not resurrect a retired grant generation" - ); - } } /// CR 400.1 + CR 608.2c: Which player's zone supplies cards for a direct @@ -21817,7 +21788,7 @@ pub struct CopyEffectInstanceRef { /// Payload-free identity of the continuous-effect occurrence which produced a /// Layer-6 trigger candidate. -#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] #[serde(tag = "type", content = "data")] pub enum TriggerProducerOrigin { Static { @@ -21836,7 +21807,7 @@ pub enum TriggerProducerOrigin { /// This is deliberately independent of `TriggerDefinition`: byte-identical /// grants from distinct producers remain independently functioning abilities /// (CR 113.2c). -#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] #[serde(tag = "type", content = "data")] pub enum TriggerGrantProducerKey { KeywordCompanion { @@ -21972,7 +21943,10 @@ impl<'de> Deserialize<'de> for TriggerEntry { TriggerEntryWire::IdentityBearing { occurrence, definition, - } => Ok(Self::new(occurrence, definition)), + } => Ok(Self { + occurrence, + definition, + }), // A later GameState normalization validates this only for a // provable printed/base slot. Keeping the marker here preserves the // distinction instead of guessing copied/granted provenance from @@ -22156,13 +22130,6 @@ impl TriggerOccurrenceState { .retain(|active| live_instances.contains(&active.instance)); } - /// Retires every active producer while preserving the monotonic allocator. - /// A player-left-game transition abandons the recipient permanently; a - /// future allocation must never resurrect one of its former grants. - pub fn retire_all_grants(&mut self) { - self.active_grants.clear(); - } - pub fn active_grants( &self, ) -> impl Iterator { diff --git a/crates/engine/src/types/game_state.rs b/crates/engine/src/types/game_state.rs index 20b7696d34..4d2805b759 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -224,6 +224,33 @@ mod tuple_key_map { } } +#[cfg(test)] +mod legacy_trigger_definition_ref_map { + use super::*; + + pub fn serialize( + map: &HashMap, + serializer: S, + ) -> Result + where + S: serde::Serializer, + { + let mut entries: Vec<_> = map.iter().collect(); + entries.sort_unstable_by_key(|(key, _)| *key); + entries.serialize(serializer) + } + + pub fn deserialize<'de, D>( + deserializer: D, + ) -> Result, D::Error> + where + D: serde::Deserializer<'de>, + { + Vec::<(TriggerDefinitionRef, u32)>::deserialize(deserializer) + .map(|entries| entries.into_iter().collect()) + } +} + /// Serde adapter for trigger occurrence ledgers. JSON object keys must be /// strings, while a `TriggerDefinitionRef` is structured identity; encode the /// map as an explicit entry list rather than flattening or guessing a key. @@ -22479,6 +22506,7 @@ mod tests { use crate::types::ability::{ AbilityDefinition, AbilityKind, Effect, PostReplacementContinuation, QuantityExpr, ResolvedAbility, TargetFilter, TriggerBaseSetInstanceRef, TriggerDefinitionOccurrenceRef, + TriggerEntry, TriggerGrantInstanceRef, }; use crate::types::deterministic_serde::test_support::ReverseBuildHasher; use crate::types::identifiers::{ @@ -22494,7 +22522,7 @@ mod tests { #[derive(Serialize)] struct TriggerRefFixture<'a> { - #[serde(serialize_with = "trigger_definition_ref_map::serialize")] + #[serde(serialize_with = "legacy_trigger_definition_ref_map::serialize")] values: &'a HashMap, } @@ -22604,7 +22632,7 @@ mod tests { .expect("trigger-ref fixture should serialize"), r#"{"values":[[{"source":{"object_id":7,"incarnation":3},"occurrence":{"type":"Printed","data":{"base_set":1,"printed_index":0}}},10],[{"source":{"object_id":7,"incarnation":3},"occurrence":{"type":"Printed","data":{"base_set":1,"printed_index":1}}},11],[{"source":{"object_id":7,"incarnation":3},"occurrence":{"type":"Printed","data":{"base_set":1,"printed_index":2}}},12]]}"# ); - let trigger_round_trip = trigger_definition_ref_map::deserialize( + let trigger_round_trip = legacy_trigger_definition_ref_map::deserialize( &mut serde_json::Deserializer::from_str( r#"[[{"source":{"object_id":7,"incarnation":3},"occurrence":{"type":"Printed","data":{"base_set":1,"printed_index":2}}},12],[{"source":{"object_id":7,"incarnation":3},"occurrence":{"type":"Printed","data":{"base_set":1,"printed_index":0}}},10]]"#, ), @@ -28403,6 +28431,60 @@ mod tests { )); } + #[test] + fn game_state_deserialize_preserves_legacy_grant_fire_counts_by_recipient() { + let object_id = ObjectId(993); + let second_object_id = ObjectId(994); + let mut state = GameState::new_two_player(42); + let mut object = GameObject::new( + object_id, + CardId(993), + PlayerId(0), + "Legacy granted trigger".to_string(), + Zone::Battlefield, + ); + let entry = TriggerEntry::new( + TriggerDefinitionOccurrenceRef::Granted { + grant_instance: TriggerGrantInstanceRef(1), + }, + TriggerDefinition::new(crate::types::triggers::TriggerMode::Attacks), + ); + let definition = object.trigger_definition_ref(&entry); + object.trigger_definitions.push(entry); + state.objects.insert(object_id, object); + state + .trigger_fire_counts_this_turn + .insert(definition.clone(), 2); + + let mut second_object = GameObject::new( + second_object_id, + CardId(994), + PlayerId(0), + "Second legacy granted trigger".to_string(), + Zone::Battlefield, + ); + let second_entry = TriggerEntry::new( + TriggerDefinitionOccurrenceRef::Granted { + grant_instance: TriggerGrantInstanceRef(1), + }, + TriggerDefinition::new(crate::types::triggers::TriggerMode::Attacks), + ); + let second_definition = second_object.trigger_definition_ref(&second_entry); + second_object.trigger_definitions.push(second_entry); + state.objects.insert(second_object_id, second_object); + state + .trigger_fire_counts_this_turn + .insert(second_definition.clone(), 3); + + let snapshot = serde_json::to_value(state).expect("serialize fixture state"); + let restored: GameState = + serde_json::from_value(snapshot).expect("recipient-specific ledger counts restore"); + assert_eq!( + restored.trigger_fire_counts_this_turn, + HashMap::from([(definition, 2), (second_definition, 3),]) + ); + } + #[test] fn game_state_deserialize_rejects_unproven_legacy_trigger_payload() { let object_id = ObjectId(992); diff --git a/crates/engine/src/types/resolved_commands.rs b/crates/engine/src/types/resolved_commands.rs index ac4e65dcdc..20198c9397 100644 --- a/crates/engine/src/types/resolved_commands.rs +++ b/crates/engine/src/types/resolved_commands.rs @@ -3252,7 +3252,7 @@ pub(crate) fn ledger_edit_is_invalid(edit: &ResolvedLedgerEdit) -> bool { || *resulting_first_card_drawn_this_turn != expected_first } ResolvedLedgerEdit::TriggerFired { - edit: ResolvedTriggerLedgerEdit::MaxTimesPerTurn { expected_old }, + edit: ResolvedTriggerLedgerEdit::MaxTimesPerTurn { expected_old, .. }, .. } => *expected_old == u32::MAX, ResolvedLedgerEdit::TriggerFired { .. } diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 83160706a4..86fcdc1c4c 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -1170,6 +1170,7 @@ mod momir_token_firebreathing_duration; mod moon_girl_second_draw_base_pt; mod mox_diamond_discard_cost_2853; mod multi_source_each_power_damage; +mod nadu_lavaspur_boots_max_times; mod najeela_extra_combat_grant_2898; mod no_top_level_test_binaries; mod oblivions_hunger_conditional_draw; diff --git a/crates/engine/tests/integration/nadu_lavaspur_boots_max_times.rs b/crates/engine/tests/integration/nadu_lavaspur_boots_max_times.rs new file mode 100644 index 0000000000..c568ce3b7f --- /dev/null +++ b/crates/engine/tests/integration/nadu_lavaspur_boots_max_times.rs @@ -0,0 +1,116 @@ +//! Regression for Nadu's granted MaxTimesPerTurn trigger through Lavaspur Boots. +//! +//! Nadu grants the targeting trigger to each creature separately, and each +//! recipient's ability owns its own "twice each turn" limit. Each Equip +//! activation below uses the production targeting and trigger-collection pipeline. + +use engine::game::layers::evaluate_layers; +use engine::game::scenario::{GameScenario, P0}; +use engine::types::ability::{ContinuousModification, TriggerConstraint}; +use engine::types::mana::{ManaType, ManaUnit}; +use engine::types::phase::Phase; +use std::sync::Arc; + +const NADU_ORACLE: &str = "Flying\nCreatures you control have \"Whenever this creature becomes the target of a spell or ability, reveal the top card of your library. If it's a land card, put it onto the battlefield. Otherwise, put it into your hand. This ability triggers only twice each turn.\""; +const LAVASPUR_BOOTS_ORACLE: &str = + "Equipped creature gets +1/+0 and has haste and ward {1}.\nEquip {1}"; + +#[test] +fn nadu_granted_trigger_has_independent_max_times_caps_per_target() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.with_mana_pool( + P0, + (0..9) + .map(|_| { + ManaUnit::new( + ManaType::Colorless, + engine::types::identifiers::ObjectId(0), + false, + vec![], + ) + }) + .collect(), + ); + scenario.with_library_top( + P0, + &[ + "Forest", "Island", "Mountain", "Plains", "Swamp", "Forest", "Island", "Mountain", + "Plains", + ], + ); + + let nadu = scenario + .add_creature_from_oracle(P0, "Nadu, Winged Wisdom", 3, 4, NADU_ORACLE) + .id(); + let first_target = scenario.add_vanilla(P0, 1, 1); + let second_target = scenario.add_vanilla(P0, 1, 1); + let third_target = scenario.add_vanilla(P0, 1, 1); + let boots = scenario + .add_creature(P0, "Lavaspur Boots", 0, 0) + .as_artifact() + .with_subtypes(vec!["Equipment"]) + .from_oracle_text(LAVASPUR_BOOTS_ORACLE) + .id(); + + let mut runner = scenario.build(); + { + let nadu_object = runner.state_mut().objects.get_mut(&nadu).unwrap(); + for static_definition in Arc::make_mut(&mut nadu_object.base_static_definitions) { + for modification in &mut static_definition.modifications { + if let ContinuousModification::GrantTrigger { trigger } = modification { + trigger.constraint = Some(TriggerConstraint::MaxTimesPerTurn { max: 2 }); + } + } + } + nadu_object.static_definitions = (*nadu_object.base_static_definitions).clone().into(); + } + evaluate_layers(runner.state_mut()); + assert_eq!( + runner.state().objects[&boots].abilities.len(), + 1, + "Lavaspur Boots must expose its Equip ability" + ); + for target in [first_target, second_target, third_target] { + assert!( + runner.state().objects[&target] + .trigger_definitions + .as_slice() + .iter() + .any(|entry| matches!( + entry.definition.constraint, + Some(TriggerConstraint::MaxTimesPerTurn { max: 2 }) + )), + "Nadu must grant its targeting trigger with MaxTimesPerTurn=2" + ); + } + for target in [ + first_target, + second_target, + third_target, + first_target, + second_target, + third_target, + first_target, + second_target, + third_target, + ] { + runner.activate(boots, 0).target_object(target).resolve(); + } + + let counts = &runner.state().trigger_fire_counts_this_turn; + assert_eq!( + counts.values().sum::(), + 6, + "Nadu's granted trigger must fire twice for each creature targeted by Equip" + ); + assert_eq!( + counts.len(), + 3, + "each recipient must own an independent MaxTimesPerTurn ledger entry" + ); + assert!( + counts.values().all(|count| *count == 2), + "each recipient's granted trigger must retain two uses" + ); +}