Skip to content
Open
Show file tree
Hide file tree
Changes from 4 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 @@ -2865,6 +2865,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 @@ -4388,6 +4391,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
17 changes: 17 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 @@ -5359,6 +5370,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 @@ -5761,6 +5773,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 @@ -6040,6 +6053,9 @@ fn effect_resolution_choice_freedom(e: &Effect) -> ResolutionChoiceFreedom {
| Effect::GainActivatedAbilitiesOfTarget { .. }
| Effect::ChooseCard { .. }
| Effect::PutCounter { .. }
// CR 122.1 + CR 603.2c: may prompt for Aragorn's "up to one target"
// and may enter the CR 614 replacement pipeline — classify MayPrompt.
| Effect::ReproduceEventCounters { .. }
| Effect::PutCounterAll { .. }
| Effect::MultiplyCounter { .. }
| Effect::DoublePT { .. }
Expand Down Expand Up @@ -6313,6 +6329,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 @@ -2590,6 +2590,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 @@ -4151,6 +4151,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)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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 @@ -4998,6 +5001,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 @@ -5175,6 +5179,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 @@ -8872,6 +8877,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 @@ -8937,6 +8943,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
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
Loading
Loading