Skip to content

Commit 81a020f

Browse files
committed
fix(engine): give each token battlefield entry its own CR 603.2c occurrence index
Token entries were emitted with `turn_zone_change_index` hardcoded to 0, so every token entry in a turn aliased onto occurrence #0. The CR 603.2c batched replay guard dedups on `(definition_ref, turn_zone_change_index)`, so a second same-turn token batch collided with the first and its batched trigger fire was swallowed. Route token births through `restrictions::record_zone_change`, the single authority that assigns the index and performs the CR 403.3 battlefield-entry bookkeeping. The three co-located `record_battlefield_entry` calls are deleted in the same change, since the recorder does it and a second call double-counts. This exposed a live/replay desync in the CR 733 resolved-commands journal: the live path now pushes a ledger record for a token birth, but `apply_resolved_token_creation` did not, so a journaled token birth followed by a same-turn zone change failed closed with `TurnRecordIndexMismatch`. The replayer now reconstructs the entry record and records it through the same authority, which also closes a pre-existing gap where replay skipped the CR 403.3 bookkeeping entirely. Reconstruction rather than a new command field, so the serialized journal surface is unchanged. The same applier also skipped token ability injection. It now dispatches on the body variant, mirroring the live seam: a Copy body gets `finalize_copied_token` + the predefined-only injector (CR 707.2 + CR 702.33a strip cast-only characteristics off a token copy); a Spec body gets the catalog injector. Known ceiling, documented at the seam: the live journal point precedes entry counters, `attach_to`, and injection, so a replayed record can still differ from live for counter-carrying and Role/Aura token births. Closing those requires moving the live record point and is out of scope here. Note: `apply_resolved_token_creation` has no production caller yet — this is replay fidelity ahead of wiring, exercised by the cr733 suites. Verification (measured): cargo test -p engine 21975 passed / 0 failed / 15 ignored revert-probe: deleting the body dispatch fails 2 tests; replacing it with a blanket injector fails exactly 1, so both arms are separately load-bearing. Assisted-by: ClaudeCode:claude-opus-4.8
1 parent baa6eb6 commit 81a020f

11 files changed

Lines changed: 1218 additions & 46 deletions

crates/engine/src/game/effects/incubate.rs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,8 @@ pub fn resolve(
9696
// token (escalates to a full pass if it sources effects, carries
9797
// counters, etc.).
9898
crate::game::layers::mark_layers_entered(state, obj_id);
99-
crate::game::restrictions::record_battlefield_entry(state, obj_id);
99+
// CR 403.3 battlefield-entry bookkeeping is done by `record_zone_change` below —
100+
// recording it here too would double-count `battlefield_entries_this_turn`.
100101
crate::game::restrictions::record_token_created(state, obj_id);
101102

102103
// CR 603.6a: The Incubator token enters the battlefield as a zone change
@@ -109,14 +110,21 @@ pub fn resolve(
109110
// triggers (issue #4238). Mirrors
110111
// `token.rs::apply_create_token_after_replacement_with_created_ids` and
111112
// `conjure.rs`'s identical fix for the same bug class.
112-
let zone_change_record = state
113+
//
114+
// CR 400.7 + CR 603.2c: route the record through `restrictions::record_zone_change` — the
115+
// single authority that assigns this turn's zone-change index — and write the assigned index
116+
// back onto the emitted record. `snapshot_for_zone_change` leaves it at its `0` placeholder,
117+
// and the batched zone-change replay guard (`triggers.rs`) dedups on
118+
// `(definition_ref, turn_zone_change_index)` read off the EVENT, so an unrouted record aliases
119+
// this Incubator onto occurrence `0` and a `batched: true` ETB trigger that already fired for
120+
// another entry this turn is swallowed. Same shape as `merge.rs` and `token.rs`.
121+
let mut zone_change_record = state
113122
.objects
114123
.get(&obj_id)
115124
.expect("incubator token was just created")
116125
.snapshot_for_zone_change(obj_id, None, Zone::Battlefield);
117-
state
118-
.zone_changes_this_turn
119-
.push_back(zone_change_record.clone());
126+
zone_change_record.turn_zone_change_index =
127+
crate::game::restrictions::record_zone_change(state, zone_change_record.clone());
120128
events.push(GameEvent::ZoneChanged {
121129
object_id: obj_id,
122130
from: None,

crates/engine/src/game/effects/token.rs

Lines changed: 89 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -950,7 +950,8 @@ pub(crate) fn apply_create_token_after_replacement_with_created_ids(
950950
// continuous effect / carries counters / etc., or if any active effect
951951
// reads board population.
952952
crate::game::layers::mark_layers_entered(state, obj_id);
953-
crate::game::restrictions::record_battlefield_entry(state, obj_id);
953+
// CR 403.3 battlefield-entry bookkeeping is done by `record_zone_change` inside
954+
// `push_committed_token_entry_events` below — recording it here too double-counts.
954955
crate::game::restrictions::record_token_created(state, obj_id);
955956

956957
// CR 303.4 + CR 303.7: A Role/Aura token created "attached to" a host
@@ -980,25 +981,16 @@ pub(crate) fn apply_create_token_after_replacement_with_created_ids(
980981
// Battlefield }` so every ETB trigger matcher (Elvish Vanguard, Soul
981982
// Warden, Panharmonicon) fires for tokens through the same code path
982983
// used for normal battlefield entry. The accompanying `TokenCreated`
983-
// event is preserved below for token-specific consumers (animation,
984-
// logging, `LastCreated` target filters).
985-
let zone_change_record = state
986-
.objects
987-
.get(&obj_id)
988-
.expect("token just created")
989-
.snapshot_for_zone_change(obj_id, None, Zone::Battlefield);
990-
events.push(GameEvent::ZoneChanged {
991-
object_id: obj_id,
992-
from: None,
993-
to: Zone::Battlefield,
994-
record: Box::new(zone_change_record),
995-
});
996-
997-
events.push(GameEvent::TokenCreated {
998-
object_id: obj_id,
999-
name: spec.characteristics.display_name.clone(),
1000-
source_id: spec.source_id,
1001-
});
984+
// event is emitted for token-specific consumers (animation, logging,
985+
// `LastCreated` target filters). Single authority for both, and for the
986+
// CR 400.7 zone-change index the batched replay guard keys on.
987+
push_committed_token_entry_events(
988+
state,
989+
obj_id,
990+
spec.characteristics.display_name.clone(),
991+
spec.source_id,
992+
events,
993+
);
1002994

1003995
// CR 603.7: Tokens with a limited duration get a delayed sacrifice trigger.
1004996
// Used by Mobilize and similar keywords that create temporary attacking tokens.
@@ -1122,6 +1114,60 @@ pub fn apply_resolved_token_creation(
11221114
state.objects.insert(object_id, object);
11231115
// allow-raw-zone: replay materializes a token birth, which has no from-zone move (CR 111.1 + CR 614.12).
11241116
zones::add_to_zone(state, object_id, Zone::Battlefield, command.owner);
1117+
// CR 111.3 + CR 111.10: a token's abilities come from the creating effect
1118+
// and the predefined/catalog tables, NOT from the body the command carries,
1119+
// so the body alone materializes a Treasure with no "{T}, Sacrifice this
1120+
// token: Add one mana of any color." Both live paths inject after
1121+
// materializing and before their entry snapshot (Spec: this file, above the
1122+
// `push_committed_token_entry_events` call; Copy: `token_copy.rs`'s
1123+
// `finalize_copied_token` + `inject_predefined_token_abilities`), so replay
1124+
// does the same here, per body variant. The dispatch mirrors
1125+
// `finalize_committed_liminal_token_entry_from_action`'s
1126+
// `LiminalTokenAbilityInjection` match arm-for-arm — a blanket
1127+
// `inject_resolved_token_abilities` would be wrong for the Copy body, whose
1128+
// live authority uses the predefined-only injector after
1129+
// `finalize_copied_token`'s CR 707.2 cast-only strip.
1130+
match &command.body {
1131+
ResolvedTokenBody::Copy { copy, .. } => {
1132+
super::token_copy::finalize_copied_token(state, copy.source_id, object_id);
1133+
inject_predefined_token_abilities(state, object_id);
1134+
}
1135+
ResolvedTokenBody::Spec { .. } => inject_resolved_token_abilities(state, object_id),
1136+
}
1137+
// CR 400.7 + CR 403.3: the resolve path records the birth through
1138+
// `restrictions::record_zone_change` (`push_committed_token_entry_events`),
1139+
// which appends to this turn's zone-change ledger and assigns the entry's
1140+
// index. Replay must record the same entry: the ledger length IS the index
1141+
// allocator, so a birth that records nothing leaves every later replayed
1142+
// zone change one short of its recorded `turn_zone_change_index` and
1143+
// `apply_resolved_zone_change` fails closed on `TurnRecordIndexMismatch`.
1144+
// The record is reconstructed from the materialized object rather than
1145+
// carried on the command: it is a projection of state this applier has
1146+
// already installed.
1147+
//
1148+
// KNOWN CEILING — two record-visible classes the reconstruction cannot
1149+
// reproduce, both because the LIVE journal point (`record_token_creation`,
1150+
// in the resolve path above) runs BEFORE the live mutations and before the
1151+
// live snapshot, so no call site inside THIS applier can close them; they
1152+
// would need the live journal-record point moved:
1153+
// (i) `spec.enter_with_counters` — the live snapshot's
1154+
// `trigger_source_context.lki.counters` (and P/T, if the counter's
1155+
// layer bump landed first) carry the entry counters. Counters replay
1156+
// through their own `ObjectCounter` command, journaled AFTER this
1157+
// birth, so the reconstructed record here has none.
1158+
// (ii) `spec.attach_to` (Role/Aura tokens) — `record.attached_to`. Same
1159+
// reason: attachment replays through the Attachment family.
1160+
// A third class, predefined/catalog ability injection contributing
1161+
// `record.trigger_definitions`, IS closed — by the injection dispatch
1162+
// directly above, which runs before this snapshot exactly as the live paths
1163+
// do. Storing the live record on the command would not close (i) or (ii)
1164+
// either, for the same ordering reason, so it was not done.
1165+
let entry_record = state
1166+
.objects
1167+
.get(&object_id)
1168+
.expect("the token was materialized above")
1169+
.snapshot_for_zone_change(object_id, None, Zone::Battlefield);
1170+
crate::game::restrictions::record_zone_change(state, entry_record);
11251171
// CR 111.1: replay must not hand the same id out again to a later allocation.
11261172
state.next_object_id = state.next_object_id.max(command.resulting_next_object_id);
11271173
// CR 613.7d: the birth drew an entry timestamp alongside the object id, and
@@ -1475,15 +1521,6 @@ pub(crate) fn continue_liminal_copy_token_batch_after_counter_pause(
14751521
)
14761522
}
14771523

1478-
pub(crate) fn commit_liminal_token_entry_with_event_emission(
1479-
state: &mut GameState,
1480-
event: ProposedEvent,
1481-
events: &mut Vec<GameEvent>,
1482-
entry_events: TokenEntryEventEmission,
1483-
) -> bool {
1484-
commit_liminal_token_entry_with_post_actions(state, event, events, entry_events, Vec::new())
1485-
}
1486-
14871524
pub(crate) fn commit_liminal_token_entry_with_post_actions(
14881525
state: &mut GameState,
14891526
event: ProposedEvent,
@@ -1662,7 +1699,8 @@ pub(crate) fn finalize_committed_liminal_token_entry_from_action(
16621699
}
16631700
}
16641701
crate::game::layers::mark_layers_entered(state, object_id);
1665-
crate::game::restrictions::record_battlefield_entry(state, object_id);
1702+
// CR 403.3 battlefield-entry bookkeeping is done by `record_zone_change` inside
1703+
// `push_committed_token_entry_events` below — recording it here too double-counts.
16661704
crate::game::restrictions::record_token_created(state, object_id);
16671705

16681706
if enters_attacking {
@@ -1710,15 +1748,33 @@ pub(crate) fn finalize_committed_liminal_token_entry_from_action(
17101748
true
17111749
}
17121750

1751+
/// CR 603.6a + CR 400.7: emit a token's battlefield-entry events, recording the entry through
1752+
/// [`crate::game::restrictions::record_zone_change`] — the single authority that assigns this
1753+
/// turn's zone-change index and performs the CR 403.3 battlefield-entry bookkeeping.
1754+
///
1755+
/// The index matters: `GameObject::snapshot_for_zone_change` leaves
1756+
/// `turn_zone_change_index` at its `0` placeholder for the recorder to overwrite, and the
1757+
/// CR 603.2c batched zone-change replay guard (`triggers.rs`) dedups on
1758+
/// `(definition_ref, turn_zone_change_index)`. A token entry that never reached the recorder
1759+
/// therefore shipped index `0` on the wire, so a SECOND same-turn token batch collided with the
1760+
/// first and its batched trigger fire was swallowed.
1761+
///
1762+
/// Callers must NOT also call `record_battlefield_entry` — `record_zone_change` does it, and a
1763+
/// second call double-counts `battlefield_entries_this_turn`.
17131764
pub(crate) fn push_committed_token_entry_events(
1714-
state: &GameState,
1765+
state: &mut GameState,
17151766
object_id: ObjectId,
17161767
name: String,
17171768
source_id: ObjectId,
17181769
events: &mut Vec<GameEvent>,
17191770
) {
1720-
if let Some(token) = state.objects.get(&object_id) {
1721-
let zone_change_record = token.snapshot_for_zone_change(object_id, None, Zone::Battlefield);
1771+
let entry = state
1772+
.objects
1773+
.get(&object_id)
1774+
.map(|token| token.snapshot_for_zone_change(object_id, None, Zone::Battlefield));
1775+
if let Some(mut zone_change_record) = entry {
1776+
zone_change_record.turn_zone_change_index =
1777+
crate::game::restrictions::record_zone_change(state, zone_change_record.clone());
17221778
events.push(GameEvent::ZoneChanged {
17231779
object_id,
17241780
from: None,

crates/engine/src/game/effects/token_copy.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -835,6 +835,14 @@ pub(crate) fn apply_copy_token_after_replacement_with_created_ids(
835835
}
836836

837837
// CR 111.10: Predefined token abilities for known subtypes (Treasure, Food, etc.).
838+
//
839+
// PAIRED WITH THE REPLAY ARM at `token::apply_resolved_token_creation`'s
840+
// `ResolvedTokenBody::Copy` match arm, which must call the same
841+
// predefined-only injector. Unlike the liminal path — where one
842+
// `copy_resume.is_some()` predicate drives both the live and journaled
843+
// matches, so a divergence fails to compile — this branch is coupled to
844+
// replay by convention only. Switching it to the catalog-wide
845+
// `inject_resolved_token_abilities` would silently desync replay from live.
838846
super::token::inject_predefined_token_abilities(state, token_id);
839847
// Battlefield entry of a copy token: request an incremental re-derive
840848
// for just this token. `flush_layers` escalates to a full pass when

crates/engine/src/game/engine_replacement.rs

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1774,11 +1774,35 @@ pub(super) fn handle_copy_target_choice(
17741774
}));
17751775
}
17761776
}
1777-
if !super::effects::token::commit_liminal_token_entry_with_event_emission(
1777+
// CR 403.3 + CR 603.6a: the commit applies the token's enter-with-counters, which can
1778+
// PAUSE on a CR 616.1 ordering choice between two AddCounter replacements. On that pause
1779+
// the only stashed post-action is the entry finalization, and its `Suppress` emission mode
1780+
// means the finalize tail neither emits the entry events nor (since the entry record is now
1781+
// written by `record_zone_change` inside `push_committed_token_entry_events`) records the
1782+
// entry at all — the token would enter invisibly. Hand the emit down as a post-finalize
1783+
// action so the paused path still performs the entry EMIT the unpaused one performs below.
1784+
// Only the emit: on a pause this function returns at the `commit_liminal_token_entry_*`
1785+
// call below, so the unpaused tail's CR 614.12a `BecomeCopy` chain,
1786+
// `finish_copy_target_choice_entry`, and the copy continuation do not run on that route.
1787+
// That abandonment is pre-existing and is not what this hand-down addresses. Dropped
1788+
// unused when the commit does not pause.
1789+
let paused_entry_emit: Vec<PendingCounterPostAction> = entry_events
1790+
.clone()
1791+
.map(
1792+
|(name, event_source_id)| PendingCounterPostAction::EmitCommittedCopyTokenEntry {
1793+
object_id: source_id,
1794+
name,
1795+
source_id: event_source_id,
1796+
},
1797+
)
1798+
.into_iter()
1799+
.collect();
1800+
if !super::effects::token::commit_liminal_token_entry_with_post_actions(
17781801
state,
17791802
resume_event,
17801803
events,
17811804
TokenEntryEventEmission::Suppress,
1805+
paused_entry_emit,
17821806
) {
17831807
return Ok(state.waiting_for.clone());
17841808
}

crates/engine/src/game/restrictions.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -379,9 +379,9 @@ pub(crate) fn battlefield_entry_record_for(
379379
subtypes: obj.card_types.subtypes.clone(),
380380
supertypes: obj.card_types.supertypes.clone(),
381381
colors: obj.color.clone(),
382-
// CR 403.3: snapshot the object's keywords at entry time. This is the
383-
// printed/base + counter-granted keyword set (pre-layer; see the field doc
384-
// on BattlefieldEntryRecord.keywords for the documented Layer-6 limitation).
382+
// CR 403.3: snapshot the object's keywords at entry time — whatever the layer
383+
// state is at the caller's record point (pre-flush for most entries, post-flush
384+
// for an attached token). See the field doc on `BattlefieldEntryRecord.keywords`.
385385
keywords: obj.keywords.clone(),
386386
controller: obj.controller,
387387
}

crates/engine/src/types/game_state.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1719,10 +1719,15 @@ pub struct BattlefieldEntryRecord {
17191719
/// with flying entered this turn") evaluate via the CR 603.10 last-known-state
17201720
/// against entry-time characteristics (like the existing core_types/colors
17211721
/// snapshots). KNOWN LIMITATION: this captures the object's keywords at record
1722-
/// time, which is BEFORE the layer system re-evaluates (layers are only marked
1723-
/// dirty, not recomputed, at zone-change). Printed flyers and keyword-counter /
1724-
/// intrinsic flyers are counted; a creature granted flying ONLY by a Layer-6
1725-
/// continuous effect (e.g. an anthem) at the moment it enters is NOT counted.
1722+
/// time, which for most entries is BEFORE the layer system re-evaluates (layers
1723+
/// are only marked dirty, not recomputed, at zone-change). Printed flyers and
1724+
/// keyword-counter / intrinsic flyers are counted; a creature granted flying ONLY
1725+
/// by a Layer-6 continuous effect (e.g. an anthem) at the moment it enters is NOT
1726+
/// counted. EXCEPTION — a token created attached to a host (Role/Aura tokens):
1727+
/// `effects::attach::attach_to` runs `mark_layers_full` + `flush_layers`, and the
1728+
/// token path records its entry AFTER the attach, so that sub-path's snapshot IS
1729+
/// post-flush and does see Layer-6 grants. Not a defect: the paired
1730+
/// `ZoneChangeRecord` has always been taken post-attach, so the two ledgers agree.
17261731
#[serde(default, skip_serializing_if = "Vec::is_empty")]
17271732
pub keywords: Vec<Keyword>,
17281733
pub controller: PlayerId,

crates/engine/tests/integration/combo_infinite_pile.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3076,6 +3076,52 @@ fn opponents_etb_life_gainer_does_not_suppress_your_axis() {
30763076
);
30773077
}
30783078

3079+
/// R4-C1 COMBINED GATE (4a + 4b together; per-commit green is explicitly insufficient).
3080+
///
3081+
/// The same ETB life gainer as `batched_and_replay_routes_converge_on_the_same_life_total`, but
3082+
/// with `batched: true` — the "Whenever ONE OR MORE creatures you control enter, you gain 1 life"
3083+
/// shape (CR 603.2c). Collapsing at N now produces N SEPARATE same-turn token batches (one per
3084+
/// replayed cycle), so the total is right only if BOTH fixes hold:
3085+
///
3086+
/// * 4a (route): the ETB-sourced axis must take the concrete replay. Reverting it re-introduces
3087+
/// the batched `Life` on top of the real entries ⇒ amplified over-count.
3088+
/// * 4b (index): each replayed cycle's entry must carry its OWN zone-change index. Reverting it
3089+
/// leaves every entry on the `0` placeholder, so `batched_zone_change_already_collected` keys
3090+
/// all N batches to `(def, 0)` and only the FIRST fires ⇒ the trigger-count assertion fails.
3091+
///
3092+
/// Both revert-probes were RUN; observed values are in the assertion messages.
3093+
#[test]
3094+
fn combined_batched_etb_gainer_fires_once_per_replayed_cycle() {
3095+
const N: u32 = 5;
3096+
let mut state: GameState = serde_json::from_str(&OFFER_STATE)
3097+
.expect("the real 4p offer dump must deserialize into the current GameState");
3098+
strip_life_conditional_cost_static(&mut state);
3099+
let mut batched_gainer = innkeeper_etb_life_trigger(&state);
3100+
batched_gainer.batched = true;
3101+
let host = create_life_gainer(&mut state, P0, "Grafted Batched Innkeeper");
3102+
graft_trigger(&mut state, host, batched_gainer);
3103+
3104+
drive_all_accept_n(&mut state, N);
3105+
assert_eq!(
3106+
route_labels(&state, P0),
3107+
vec!["DriveSequence".to_string()],
3108+
"4a: a batched ETB life gainer still routes the axis to the concrete replay"
3109+
);
3110+
3111+
let life_before = life_of(&state, P0);
3112+
let minted = collapse_at(&mut state, N);
3113+
3114+
// POSITIVE reach-guard: N cycles really replayed.
3115+
assert_eq!(minted, N as usize, "the replay minted one token per cycle");
3116+
// DISCRIMINATOR (needs BOTH fixes): N distinct same-turn batches ⇒ N fires ⇒ +N.
3117+
// revert 4b ⇒ all N batches collide on `(def, 0)` ⇒ +1. revert 4a ⇒ batched Life on top ⇒ >N.
3118+
assert_eq!(
3119+
life_of(&state, P0) - life_before,
3120+
N as i32,
3121+
"each replayed cycle is its OWN batch and fires once (revert 4b ⇒ 1, revert 4a ⇒ more)"
3122+
);
3123+
}
3124+
30793125
/// NON-`GainLife` LIFE SOURCE (CR 732.2a + CR 603.6a + CR 702.15b): the Terror-of-the-Peaks
30803126
/// shape — an ETB *damage* trigger on a permanent with LIFELINK. The life axis is just as
30813127
/// ETB-sourced as Soul Warden's, but it never passes through `Effect::GainLife`: it reaches

0 commit comments

Comments
 (0)