fix(engine): implement Mirror Match copy-blocker tokens (#1597)#1735
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements the CopyTokenBlockingAttacker effect to support cards like Mirror Match, which copy attacking creatures and put those copies onto the battlefield blocking them. It adds the necessary combat bookkeeping, effect resolution, parser integration, and import conversions. Feedback highlights two compilation issues violating Rule R7 (Persistent-container hot fields), where standard Vec types and operations are incorrectly used instead of im::Vector and push_back for creatures_blocked_this_turn and last_created_token_ids.
| .or_default() | ||
| .push(blocker_id); | ||
| // CR 509.1a tracking: record the blocker for per-turn "blocked this turn" queries. | ||
| state.creatures_blocked_this_turn.push(blocker_id); |
There was a problem hiding this comment.
[HIGH] Use push_back on persistent vector creatures_blocked_this_turn. Evidence: crates/engine/src/game/combat.rs:347.
Why it matters: GameState persistent containers use im::Vector which does not support .push(), so this will cause a compilation error and violates rule R7.
Suggested fix: Use push_back instead of push.
| state.creatures_blocked_this_turn.push(blocker_id); | |
| state.creatures_blocked_this_turn.push_back(blocker_id); |
References
- R7. Persistent-container hot fields: Hot GameState zones use im (15.x persistent vectors) so clones are O(log n) structural shares. Writes must use im::Vector methods (push_back/pop_back/iter_mut), not std Vec's push/pop/values_mut. (link)
| let mut all_created: Vec<ObjectId> = Vec::new(); | ||
| for attacker_id in attacker_ids { | ||
| // CR 707.2: create one token copy of this specific attacker, delegating | ||
| // to the single-authority token-copy resolver so copiable values, | ||
| // predefined abilities, and ETB events are handled identically to every | ||
| // other copy-token effect. | ||
| let copy_effect = Effect::CopyTokenOf { | ||
| target: TargetFilter::Any, | ||
| owner: owner.clone(), | ||
| source_filter: Some(TargetFilter::SpecificObject { id: attacker_id }), | ||
| enters_attacking: false, | ||
| tapped: false, | ||
| count: QuantityExpr::Fixed { value: 1 }, | ||
| extra_keywords: vec![], | ||
| additional_modifications: vec![], | ||
| }; | ||
| let copy_ability = | ||
| ResolvedAbility::new(copy_effect, vec![], ability.source_id, ability.controller); | ||
| crate::game::effects::token_copy::resolve(state, ©_ability, events)?; | ||
|
|
||
| // CR 506.3e + CR 509.1g: each fresh copy is put onto the battlefield | ||
| // blocking the attacker it copied. `last_created_token_ids` holds exactly | ||
| // this attacker's single copy at this point in the loop. | ||
| for token_id in state.last_created_token_ids.clone() { | ||
| combat::place_blocking(state, token_id, attacker_id); | ||
| all_created.push(token_id); | ||
| } | ||
| } | ||
|
|
||
| // CR 603.7c + CR 701.36a: republish the full created-token set so a composed | ||
| // "exile those tokens at end of combat" delayed trigger snapshots all copies | ||
| // via `TargetFilter::LastCreated` rather than just the last loop iteration's. | ||
| state.last_created_token_ids = all_created; |
There was a problem hiding this comment.
[HIGH] Use im::Vector for all_created to match last_created_token_ids. Evidence: crates/engine/src/game/effects/copy_token_blocking.rs:55-87.
Why it matters: state.last_created_token_ids is an im::Vector persistent container, so assigning a standard Vec to it violates rule R7 and causes a compilation error.
Suggested fix: Initialize all_created as im::Vector::new() and use push_back.
let mut all_created = im::Vector::new();
for attacker_id in attacker_ids {
// CR 707.2: create one token copy of this specific attacker, delegating
// to the single-authority token-copy resolver so copiable values,
// predefined abilities, and ETB events are handled identically to every
// other copy-token effect.
let copy_effect = Effect::CopyTokenOf {
target: TargetFilter::Any,
owner: owner.clone(),
source_filter: Some(TargetFilter::SpecificObject { id: attacker_id }),
enters_attacking: false,
tapped: false,
count: QuantityExpr::Fixed { value: 1 },
extra_keywords: vec![],
additional_modifications: vec![],
};
let copy_ability =
ResolvedAbility::new(copy_effect, vec![], ability.source_id, ability.controller);
crate::game::effects::token_copy::resolve(state, ©_ability, events)?;
// CR 506.3e + CR 509.1g: each fresh copy is put onto the battlefield
// blocking the attacker it copied. `last_created_token_ids` holds exactly
// this attacker's single copy at this point in the loop.
for token_id in state.last_created_token_ids.clone() {
combat::place_blocking(state, token_id, attacker_id);
all_created.push_back(token_id);
}
}
// CR 603.7c + CR 701.36a: republish the full created-token set so a composed
// "exile those tokens at end of combat" delayed trigger snapshots all copies
// via `TargetFilter::LastCreated` rather than just the last loop iteration's.
state.last_created_token_ids = all_created;References
- R7. Persistent-container hot fields: Hot GameState zones use im (15.x persistent vectors) so clones are O(log n) structural shares. Writes must use im::Vector methods (push_back/pop_back/iter_mut), not std Vec's push/pop/values_mut. (link)
Mirror Match did nothing in-game: the tokens that should copy each
attacker and block it were never created, so the attackers connected
with the player. The engine had no way to put a copy token onto the
battlefield blocking the attacker it copies.
Add `Effect::CopyTokenBlockingAttacker { source_filter, owner }`: for
each attacking creature matched by `source_filter`, create a token
that's a copy of it (delegating to the single-authority token-copy
resolver) and put that token onto the battlefield blocking the attacker
it copies (CR 506.3e + CR 509.1g). The created tokens are republished to
`last_created_token_ids` so the composed "exile those tokens at end of
combat" delayed trigger (CR 603.7c) captures them via
`TargetFilter::LastCreated`.
- New `combat::place_blocking` establishes the block per CR 506.3e
(only when the attacker is attacking the blocker's controller / a
planeswalker or battle they control) and CR 509.1h (attacker becomes
blocked), without firing "whenever ~ blocks" triggers (CR 509.3a).
- Native Oracle parser: a whole-card recognizer in `parse_effect_chain`
lowers Mirror Match's text to the new effect plus the delayed exile.
- mtgish-import converter: lower
`ForEachPermanentCreateTokensWithFlags` + `EntersBlockingAttacker`,
the `IsAttackingAPlayerOrPlaneswalkerTheyControl` filter, and the
`TheCreatedTokens` anaphor.
Closes phase-rs#1597
- creatures_blocked_this_turn is a HashSet: use insert(), not push() - apply rustfmt canonical formatting to new combat/resolver/parser code
4596f76 to
630c5f3
Compare
|
Hi, @matthewevans , |
matthewevans
left a comment
There was a problem hiding this comment.
Maintainer review: current head is clean. Gemini findings are stale against this branch; focused copy-token-blocking and parser tests pass after a local merge of origin/main.
Summary
Fixes #1597 — Mirror Match did nothing in-game: the tokens that should copy each attacker and block it were never created, so the attackers connected with the player ("did not make the blockers and save the enemy").
The engine had no way to put a copy token onto the battlefield blocking the attacker it copies. This PR adds that primitive and teaches both the native Oracle parser (the runtime card-data source) and the
mtgish-importconverter to produce it.What changed
Engine primitive —
Effect::CopyTokenBlockingAttacker { source_filter, owner }source_filter, create a token that's a copy of it (delegating to the single-authoritytoken_copyresolver) and put that token onto the battlefield blocking the attacker it copies (CR 506.3e + CR 509.1g).last_created_token_idsso the composed "exile those tokens at end of combat" delayed trigger (CR 603.7c) snapshots them viaTargetFilter::LastCreated.combat::place_blocking— establishes the block per CR 506.3e (only when the attacker is attacking the blocker's controller, or a planeswalker/battle they control) and CR 509.1h (attacker becomes blocked), without firing "whenever ~ blocks" triggers (CR 509.3a — a creature put onto the battlefield blocking doesn't trigger those).Native Oracle parser — a whole-card recognizer in
parse_effect_chainlowers Mirror Match's text ("For each creature attacking you or a planeswalker you control, create a token that's a copy of that creature and that's blocking that creature. Exile those tokens at end of combat.") to the new effect plus the delayed end-of-combat exile. This is the path that feeds runtimecard-data.json.mtgish-importconverter — lowersForEachPermanentCreateTokensWithFlags+EntersBlockingAttacker, theIsAttackingAPlayerOrPlaneswalkerTheyControlfilter, and theTheCreatedTokensanaphor, so the second-source-of-truth pipeline covers the card too.Rules references
Tests
copy_token_blocking::tests::copies_and_blocks_each_attacker— resolver creates one copy per attacker and blocks each.oracle_effect::tests::effect_mirror_match_copy_blockers_and_delayed_exile— native parser lowers the full card to the effect + delayed exile.