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
4 changes: 4 additions & 0 deletions crates/engine/src/analysis/ability_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,10 @@ fn effect_projection(effect: &Effect) -> Projection {
| Effect::DoublePT { .. }
| Effect::DoublePTAll { .. }
| Effect::MoveCounters { .. }
// CR 122.1 + CR 603.2c: the reproduced counter kind is event-derived (not
// statically known), so it projects onto no fixed resource axis — like
// `MoveCounters`, it is Unmodeled.
| Effect::ReproduceEventCounters { .. }
| Effect::Animate { .. }
| Effect::ReturnAsAura { .. }
| Effect::RegisterBending { .. }
Expand Down
15 changes: 15 additions & 0 deletions crates/engine/src/game/ability_rw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2868,6 +2868,9 @@ fn legacy_effect(x: &Effect) -> bool {
| Effect::GrantCastingPermission { target, .. }
| Effect::AddTargetReplacement { target, .. }
| Effect::DiscardCard { target, .. }
// CR 122.1 + CR 603.2c: only the reproduction target carries a legacy tag;
// the per-kind magnitude is a plain enum with no batch-prompt semantics.
| Effect::ReproduceEventCounters { target, .. }
| Effect::Animate { target, .. } => legacy_target_filter(target),

Effect::PutOnTopOrBottom { target, chooser } => {
Expand Down Expand Up @@ -4391,6 +4394,18 @@ fn rw_effect(
}
(p, sc)
}
// CR 122.1 + CR 603.2c + CR 608.2h: writes ObjectCounters on the target;
// the reproduced kind+count multiset is read from the triggering event
// batch (`state.current_trigger_events`) — a live event-context read, not
// a read of any object's counter map.
Effect::ReproduceEventCounters {
target,
per_kind_count: _,
} => {
let (mut p, sc) = obj(StateKind::ObjectCounters, target);
p.merge(reads_event_live());
(p, sc)
}
Effect::Bolster { count } => {
let mut p = ext_write(StateKind::ObjectCounters);
// Untargeted external counter write ⇒ census Any (fail-closed, §2).
Expand Down
14 changes: 14 additions & 0 deletions crates/engine/src/game/ability_scan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -991,6 +991,17 @@ fn scan_effect(x: &Effect, mode: ScanMode) -> Axes {
acc = acc.or(scan_target_filter(target, target_ctx, mode));
acc
}
// CR 122.1 + CR 603.2c: the per-kind magnitude is event-derived (not a
// `QuantityExpr`), so only the reproduction target is scanned; mirrors
// `MultiplyCounter`.
Effect::ReproduceEventCounters {
target,
per_kind_count: _,
} => {
let mut acc = Axes::NONE;
acc = acc.or(scan_target_filter(target, target_ctx, mode));
acc
}
Effect::Animate { .. } => Axes::CONSERVATIVE,
Effect::ReturnAsAura { .. } => Axes::CONSERVATIVE,
Effect::RegisterBending { kind: _ } => Axes::NONE,
Expand Down Expand Up @@ -5379,6 +5390,7 @@ fn effect_target_ctx(e: &Effect, mode: ScanMode) -> FilterReadContext {
| Effect::HideawayConceal { .. }
| Effect::ChooseCard { .. }
| Effect::PutCounter { .. }
| Effect::ReproduceEventCounters { .. }

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.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Classify ReproduceEventCounters as LiveBoardCensus, matching MultiplyCounter.

resolve_reproduce_event_counters resolves its recipients through resolve_defined_or_targets (crates/engine/src/game/effects/counters.rs). That is the same resolver cited at lines 5275-5278 as the reason MultiplyCounter is census-tagged: when ability.targets.is_empty() it mass-scans battlefield_phased_in_ids(). No static field on ReproduceEventCounters discriminates the announced-single mode from the mass mode, so the file's own fail-closed rule puts the whole variant in the census group.

Placing it in the relax group can mint a false loop-firewall certificate for a reproduction effect that enumerates a growing battlefield.

The mirror entry at line 5776 must move with it; census_partition_agrees_with_effect_target_ctx requires the two partitions to stay byte-identical.

🛡️ Proposed classification move
         | Effect::MultiplyCounter { .. }
+        //   CR 122.1 + CR 603.2c: `resolve_reproduce_event_counters` resolves its
+        //     recipients through the same `resolve_defined_or_targets` mass-scan as
+        //     MultiplyCounter when `ability.targets.is_empty()` ⇒ fail-closed census.
+        | Effect::ReproduceEventCounters { .. }

Remove the corresponding entry from the SnapshotOrEvent group at line 5373 and from the BoundedOrNoPopulation group at line 5776, adding a matching census entry in effect_census_role.

🤖 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/src/game/ability_scan.rs` at line 5373, Move
ReproduceEventCounters out of the SnapshotOrEvent and BoundedOrNoPopulation
classifications, and add it to the LiveBoardCensus and corresponding census
partition in effect_census_role. Keep both classification partitions
byte-identical so census_partition_agrees_with_effect_target_ctx remains valid.

| Effect::DoublePT { .. }
| Effect::MoveCounters { .. }
| Effect::Animate { .. }
Expand Down Expand Up @@ -5781,6 +5793,7 @@ fn effect_census_role(e: &Effect) -> CensusRole {
| Effect::HideawayConceal { .. }
| Effect::ChooseCard { .. }
| Effect::PutCounter { .. }
| Effect::ReproduceEventCounters { .. }
| Effect::DoublePT { .. }
| Effect::MoveCounters { .. }
| Effect::Animate { .. }
Expand Down Expand Up @@ -6019,6 +6032,7 @@ pub(crate) fn effect_is_randomness_bearing(e: &Effect) -> bool {
| Effect::GainActivatedAbilitiesOfTarget { .. }
| Effect::ChooseCard { .. }
| Effect::PutCounter { .. }
| Effect::ReproduceEventCounters { .. }
| Effect::PutCounterAll { .. }
| Effect::MultiplyCounter { .. }
| Effect::DoublePT { .. }
Expand Down
7 changes: 7 additions & 0 deletions crates/engine/src/game/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2609,6 +2609,13 @@ fn effect_details(effect: &Effect) -> Vec<(String, String)> {
));
d.push(("target".into(), fmt_target(target)));
}
Effect::ReproduceEventCounters {
target,
per_kind_count,
} => {
d.push(("reproduce counters".into(), format!("{per_kind_count:?}")));
d.push(("target".into(), fmt_target(target)));
}
Effect::RemoveCounter {
counter_type,
count,
Expand Down
107 changes: 104 additions & 3 deletions crates/engine/src/game/effects/counters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ use crate::game::game_object::GameObject;
use crate::game::replacement::{self, ReplacementResult};
use crate::types::ability::{
AbilityTag, CounterMoveSelection, CounterTransferMode, DelayedTriggerCondition, Duration,
Effect, EffectError, EffectKind, QuantityExpr, ResolvedAbility, TargetChoiceTiming,
TargetFilter, TargetRef,
Effect, EffectError, EffectKind, EventCounterReproductionCount, QuantityExpr, ResolvedAbility,
TargetChoiceTiming, TargetFilter, TargetRef,
};
#[cfg(test)]
use crate::types::counter::parse_counter_type;
Expand Down Expand Up @@ -891,6 +891,9 @@ pub(crate) fn apply_counter_addition(
object_id,
counter_type,
count,
// CR 122.1 + CR 603.2c: record who placed the counters so actor-gated
// "whenever you/an opponent put counters" triggers can match.
actor,
});
}

Expand Down Expand Up @@ -1473,7 +1476,8 @@ fn emit_evolved_event_for_counter_addition(
GameEvent::CounterAdded {
object_id: added_to,
counter_type: CounterType::Plus1Plus1,
count
count,
..
} if *added_to == object_id && *count > 0
)
});
Expand All @@ -1482,6 +1486,98 @@ fn emit_evolved_event_for_counter_addition(
}
}

/// CR 122.1 + CR 603.2c + CR 608.2h: Reproduce onto the effect's target(s) the
/// counters that the triggering counter-placement event just put onto the
/// recipient creature ("put the same number and kind of counters" / "put one of
/// each of those kinds of counters"). The kind→count multiset is read from
/// `state.current_trigger_events` — which, under the per-recipient firing model
/// (`matching_counter_added_events_by_recipient`), holds exactly one recipient's
/// `GameEvent::CounterAdded` occurrences (one per kind placed on it). Unlike
/// `resolve_move` this reads the DELTA the event placed, not the recipient's
/// total counter map. The multiset is snapshotted from the firing's events
/// (CR 608.2h), so later changes to the recipient's counters don't affect it.
pub fn resolve_reproduce_event_counters(
state: &mut GameState,
ability: &ResolvedAbility,
events: &mut Vec<GameEvent>,
) -> Result<(), EffectError> {
let per_kind_count = match &ability.effect {
Effect::ReproduceEventCounters { per_kind_count, .. } => *per_kind_count,
_ => return Ok(()),
};

// Fold the firing's `CounterAdded` occurrences into a kind→count multiset,
// preserving first-seen kind order for deterministic placement/event order.
let mut reproduced: Vec<(CounterType, u32)> = Vec::new();
for event in &state.current_trigger_events {
let GameEvent::CounterAdded {
counter_type,
count,
..
} = event
else {
continue;
};
// CR 122.1: "one of each of those kinds" (PerKind) ignores the event's
// per-kind magnitude; "the same number and kind" (SameNumber) reproduces
// exactly what the event placed, summing repeated kinds.
let amount = match per_kind_count {
EventCounterReproductionCount::SameNumber => *count,
EventCounterReproductionCount::PerKind(n) => n,
};
if amount == 0 {
continue;
}
match reproduced.iter_mut().find(|(kind, _)| kind == counter_type) {
Some((_, existing)) => match per_kind_count {
// SameNumber sums repeated kinds; PerKind is a flat per-kind
// count, so a repeated kind stays at `n` (already recorded).
EventCounterReproductionCount::SameNumber => *existing += amount,
EventCounterReproductionCount::PerKind(_) => {}
},
None => reproduced.push((counter_type.clone(), amount)),
}
}

if reproduced.is_empty() {
events.push(GameEvent::EffectResolved {
kind: EffectKind::from(&ability.effect),
source_id: ability.source_id,
subject: None,
});
return Ok(());
}

let targets = resolve_defined_or_targets(state, ability);
let additions: Vec<PendingCounterAddition> = targets
.into_iter()
.flat_map(|obj_id| {
reproduced.iter().map(move |(kind, amount)| {
object_counter_addition(ability.controller, obj_id, kind.clone(), *amount)
})
})
.collect();

let completion =
PendingEffectResolved::new(EffectKind::from(&ability.effect), ability.source_id);
for (index, addition) in additions.iter().cloned().enumerate() {
if !apply_object_counter_addition(state, addition, events) {
// CR 614: a replacement choice paused placement — stash the rest so
// the continuation drains them after the choice resolves.
stash_pending_counter_additions(state, additions[index + 1..].to_vec(), completion);
return Ok(());
}
}

events.push(GameEvent::EffectResolved {
kind: EffectKind::from(&ability.effect),
source_id: ability.source_id,
subject: None,
});

Ok(())
}

/// CR 122.1: Place counters on all battlefield objects matching a filter (no targeting).
pub fn resolve_add_all(
state: &mut GameState,
Expand Down Expand Up @@ -1747,6 +1843,10 @@ fn resolve_defined_or_targets(
let target_spec = match &ability.effect {
Effect::MultiplyCounter { target, .. }
| Effect::RemoveCounter { target, .. }
// CR 122.1 + CR 603.2c: reproduction targets exactly like `PutCounter` —
// `SelfRef` short-circuits to the source (Captain Marvel), a real target
// falls through to the chosen-target return (Aragorn).
| Effect::ReproduceEventCounters { target, .. }
| Effect::PutCounter { target, .. } => Some(target),
_ => None,
};
Expand Down Expand Up @@ -4449,6 +4549,7 @@ mod tests {
object_id,
counter_type: CounterType::Plus1Plus1,
count: 2,
..
} if *object_id == dest_id
)));
}
Expand Down
10 changes: 10 additions & 0 deletions crates/engine/src/game/effects/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4173,6 +4173,9 @@ pub fn resolve_effect(
}
Effect::ChooseCard { .. } => choose_card::resolve(state, ability, events),
Effect::PutCounter { .. } => counters::resolve_add(state, ability, events),
Effect::ReproduceEventCounters { .. } => {
counters::resolve_reproduce_event_counters(state, ability, events)
}
Effect::PutCounterAll { .. } => counters::resolve_add_all(state, ability, events),
Effect::MultiplyCounter { .. } => counters::resolve_multiply(state, ability, events),
Effect::DoublePT { .. } => pump::resolve_double_pt(state, ability, events),
Expand Down Expand Up @@ -5076,6 +5079,7 @@ fn affected_objects_from_events(
Effect::PutCounter { .. }
| Effect::PutCounterAll { .. }
| Effect::MultiplyCounter { .. }
| Effect::ReproduceEventCounters { .. }
| Effect::MoveCounters { .. } => events
.iter()
.filter_map(|event| match event {
Expand Down Expand Up @@ -5253,6 +5257,7 @@ fn mandatory_parent_effect_performed(effect: &Effect, events: &[GameEvent]) -> b
Effect::PutCounter { .. }
| Effect::PutCounterAll { .. }
| Effect::MultiplyCounter { .. }
| Effect::ReproduceEventCounters { .. }
| Effect::MoveCounters { .. } => events
.iter()
.any(|event| matches!(event, GameEvent::CounterAdded { .. })),
Expand Down Expand Up @@ -8974,6 +8979,7 @@ fn resolve_chain_body(
state.push_optional_effect_frame(OptionalEffectFrame {
ability: Box::new(ability_with_event_context_targets(state, ability)),
trigger_event: state.current_trigger_event.clone(),
trigger_events: state.current_trigger_events.clone(),
trigger_match_count: state.current_trigger_match_count,
});
state.waiting_for = WaitingFor::OpponentMayChoice {
Expand Down Expand Up @@ -9039,6 +9045,10 @@ fn resolve_chain_body(
// optional ("may") trigger's effect resolves `TriggeringPlayer` and
// other event-context refs exactly as a non-optional trigger would.
trigger_event: state.current_trigger_event.clone(),
// CR 603.2c + CR 608.2: capture the PLURAL event batch in lockstep so
// a "you may" reproduction (Captain Marvel, Apex Avenger) folds every
// `CounterAdded` occurrence when the decision resumes.
trigger_events: state.current_trigger_events.clone(),
// CR 603.2c + CR 608.2: mirror the batched-trigger subject count so a
// "you may" sub-ability of a batched trigger (Ur-Dragon's optional
// permanent-from-hand sub-effect) resumes with the same
Expand Down
1 change: 1 addition & 0 deletions crates/engine/src/game/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15551,6 +15551,7 @@ mod stage2_injector_tests {
state.push_optional_effect_frame(crate::types::resolution::OptionalEffectFrame {
ability: Box::new(optional),
trigger_event: None,
trigger_events: Vec::new(),
trigger_match_count: None,
});
state.waiting_for = WaitingFor::OptionalEffectChoice {
Expand Down
15 changes: 15 additions & 0 deletions crates/engine/src/game/engine_payment_choices.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ pub(super) fn handle_optional_effect_choice(
let OptionalEffectFrame {
ability,
trigger_event: pending_event,
trigger_events: pending_events,
trigger_match_count: pending_count,
} = frame;
let choice = if accept {
Expand All @@ -74,6 +75,12 @@ pub(super) fn handle_optional_effect_choice(
// `TriggeringPlayer` and other event-context refs resolve correctly.
let previous_trigger_event = state.current_trigger_event.clone();
state.current_trigger_event = pending_event;
// CR 603.2c + CR 608.2: restore the PLURAL batched-trigger event list
// too — an effect that folds the whole event batch (e.g.
// `Effect::ReproduceEventCounters` reading every `CounterAdded`
// occurrence) must see all occurrences, not just the singular event.
let previous_trigger_events = std::mem::take(&mut state.current_trigger_events);
state.current_trigger_events = pending_events;
Comment on lines +78 to +83

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

Add a regression test for a non-empty trigger batch.

The changed fixtures pass trigger_events: Vec::new(). They cannot detect a regression that drops the plural CounterAdded batch. Add a production-pipeline test that pauses a plural optional trigger, accepts it, and verifies that Effect::ReproduceEventCounters observes every event.

As per path instructions, tests must drive the production pipeline and prove the failure path; an empty trigger_events fixture does not cover batched-context restoration.

🤖 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/src/game/engine_payment_choices.rs` around lines 78 - 83, Add a
production-pipeline regression test near the trigger handling covered by the
changed restoration logic, using a non-empty plural CounterAdded batch. Pause
the plural optional trigger, accept it through the normal pipeline, and assert
that Effect::ReproduceEventCounters observes every event; do not rely on empty
trigger_events fixtures.

Source: Path instructions

// CR 603.2c + CR 608.2: mirror restoration of the batched-trigger
// subject count so a `QuantityRef::EventContextAmount` resolved during
// the resumed sub-ability reads the same "that many" the pre-pause
Expand All @@ -83,6 +90,7 @@ pub(super) fn handle_optional_effect_choice(
let result =
effects::resolve_optional_effect_decision(state, *ability, choice, events, 1);
state.current_trigger_event = previous_trigger_event;
state.current_trigger_events = previous_trigger_events;
state.current_trigger_match_count = previous_trigger_match_count;
result.map_err(|e| EngineError::InvalidAction(format!("{e:?}")))?;
} else if state.pending_trigger.as_ref().is_some_and(|t| {
Expand Down Expand Up @@ -2033,6 +2041,7 @@ mod tests {
state.push_optional_effect_frame(OptionalEffectFrame {
ability: Box::new(optional),
trigger_event: None,
trigger_events: Vec::new(),
trigger_match_count: None,
});
state.waiting_for = WaitingFor::OptionalEffectChoice {
Expand Down Expand Up @@ -2063,6 +2072,7 @@ mod tests {
state.push_optional_effect_frame(OptionalEffectFrame {
ability: Box::new(optional),
trigger_event: None,
trigger_events: Vec::new(),
trigger_match_count: None,
});
state.waiting_for = WaitingFor::OptionalEffectChoice {
Expand Down Expand Up @@ -2099,6 +2109,7 @@ mod tests {
state.push_optional_effect_frame(OptionalEffectFrame {
ability: Box::new(optional),
trigger_event: None,
trigger_events: Vec::new(),
trigger_match_count: None,
});
state.waiting_for = WaitingFor::OptionalEffectChoice {
Expand Down Expand Up @@ -2132,6 +2143,7 @@ mod tests {
state.push_optional_effect_frame(OptionalEffectFrame {
ability: Box::new(optional),
trigger_event: None,
trigger_events: Vec::new(),
trigger_match_count: None,
});
state.waiting_for = WaitingFor::OptionalEffectChoice {
Expand Down Expand Up @@ -2163,6 +2175,7 @@ mod tests {
state.push_optional_effect_frame(OptionalEffectFrame {
ability: Box::new(optional),
trigger_event: None,
trigger_events: Vec::new(),
trigger_match_count: None,
});
state.waiting_for = WaitingFor::OptionalEffectChoice {
Expand Down Expand Up @@ -2193,6 +2206,7 @@ mod tests {
state.push_optional_effect_frame(OptionalEffectFrame {
ability: Box::new(optional),
trigger_event: None,
trigger_events: Vec::new(),
trigger_match_count: None,
});
state.waiting_for = WaitingFor::OptionalEffectChoice {
Expand Down Expand Up @@ -2223,6 +2237,7 @@ mod tests {
state.push_optional_effect_frame(OptionalEffectFrame {
ability: Box::new(optional),
trigger_event: None,
trigger_events: Vec::new(),
trigger_match_count: None,
});
state.waiting_for = WaitingFor::OptionalEffectChoice {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1834,6 +1834,7 @@ fn optional_effect_choice_accept_preserves_nested_effect_zone_choice_continuatio
state.push_optional_effect_frame(crate::types::OptionalEffectFrame {
ability: Box::new(ability),
trigger_event: None,
trigger_events: Vec::new(),
trigger_match_count: None,
});
state.waiting_for = WaitingFor::OptionalEffectChoice {
Expand Down Expand Up @@ -1884,6 +1885,7 @@ fn opponent_may_choice_accept_preserves_nested_effect_zone_choice_continuation()
state.push_optional_effect_frame(crate::types::OptionalEffectFrame {
ability: Box::new(ability),
trigger_event: None,
trigger_events: Vec::new(),
trigger_match_count: None,
});
state.waiting_for = WaitingFor::OpponentMayChoice {
Expand Down
Loading
Loading