diff --git a/crates/engine/src/game/layers.rs b/crates/engine/src/game/layers.rs index 185747d46a..3fa67da3ae 100644 --- a/crates/engine/src/game/layers.rs +++ b/crates/engine/src/game/layers.rs @@ -2120,25 +2120,26 @@ pub fn evaluate_layers(state: &mut GameState) { // Step 2: Apply copy effects first so copied static abilities exist before later layers. let mut zone_cache = LayerZoneObjectCache::default(); let mut started_effect_sets = StartedContinuousEffectSets::new(); - let copy_effects = gather_active_effects_for_layer(state, Layer::Copy); - let ordered_copy = order_active_continuous_effects(Layer::Copy, ©_effects, state); - for effect in &ordered_copy { - apply_continuous_effect( - state, - effect, - &mut abilities_suppressed, - &mut zone_cache, - &mut started_effect_sets, - ); - } - if crate::game::stickers::apply_battlefield_name_and_ability_stickers(state, &bf_ids) { - // Sticker ability text is appended after the top-of-pass reset/copy - // application, so a sticker can turn a non-generator into a continuous - // static source mid-pass. Refresh the generator index before the main - // gather so those sticker-granted statics participate in this pass - // without broadening the non-sticker top-of-pass rebuild contract. - crate::types::game_state::StaticSourceIndex::rebuild_from_state(state); - } + // Narrowed by payload rather than by "did layer 1a run": `CopyValues` carries + // the whole copiable static set in the modification, so the payload answers + // both questions 1a needs with no plumbing and no battlefield rescan. They are + // two different questions and are asked separately — `copy_grants_continuous_static` + // for the flag returned here (does the recipient become a generator layers 2-7 + // must see?) and the narrower `copy_grants_copy_layer_static` for continuing + // 1a's own discovery loop. A clone of a vanilla creature — a permanent-duration + // TCE that outlives the clone — therefore pays nothing extra at all, and a + // clone of an ordinary lord pays one rebuild here rather than a second + // gather-and-rebuild generation inside 1a. Only a copy-granted COPY ability + // pays for a generation, which is the point of an index whose stated job is + // keeping per-flush work off `|battlefield|`. + let copy_added_generator = apply_copy_sublayer_to_fixed_point( + state, + &mut abilities_suppressed, + &mut zone_cache, + &mut started_effect_sets, + ); + let stickers_applied = + crate::game::stickers::apply_battlefield_name_and_ability_stickers(state, &bf_ids); // CR 613.2b + CR 708.2a + CR 708.10: Layer 1b. After Layer-1a copiable effects // (copy per CR 707, merge per CR 730) are applied, re-set each face-down @@ -2152,6 +2153,39 @@ pub fn evaluate_layers(state: &mut GameState) { } } + // Both producers say the same thing: layer 1 can turn a non-generator into a + // continuous static source mid-pass, and the top-of-pass index was built from + // the just-reset BASE definitions, which predate that. Refresh the generator + // index before the main gather so those statics participate in this pass, + // without broadening the top-of-pass rebuild contract for boards that have + // neither. + // + // CR 613.2a + CR 613.2c: a copy effect can hand a permanent a static ability + // its copiable base does not carry (an embalm token copying a Painter's + // Servant). After all of layer 1 has been applied the object's characteristics + // ARE its copiable values, so a static granted there must generate effects for + // layers 2-7 of THIS pass, not the next one. + // + // Sticker ability text is likewise appended after the top-of-pass reset/copy + // application. + // + // Placed below the 1b face-down reseed so the rebuild observes the whole of + // layer 1, which is the invariant CR 613.2c actually states. 1b only ever + // clears statics (`apply_face_down_creature_characteristics` clears live and + // base alike), so ordering the two the other way is inert today — but only via + // that non-local fact, not via the rule cited here. + // + // `apply_copy_sublayer_to_fixed_point` already rebuilt mid-1a whenever a + // generation of copies added a COPY-LAYER generator, but that rebuild is scoped + // to 1a's own discovery loop and predates 1b by construction. This one is the + // layer-1 + // exit rebuild the CR 613.2c invariant asks for; on the rare board that pays + // both it is one extra O(battlefield) walk, the same order as the Step-1 reset + // that already runs unconditionally. + if copy_added_generator || stickers_applied { + crate::types::game_state::StaticSourceIndex::rebuild_from_state(state); + } + // Step 3: Gather active continuous effects after layer 1 is applied. let mut effects_by_layer = gather_active_continuous_effects(state); crate::game::stickers::append_battlefield_pt_sticker_effects(state, &mut effects_by_layer); @@ -3471,6 +3505,47 @@ fn prepare_incremental_flush( { return None; } + // CR 613.2a + CR 613.2c: a copy effect applied on this path can hand a + // recipient a continuous static ability its copiable base does not carry, and + // after layer 1 that static is part of the board layers 2-7 derive from. + // + // The incremental arm cannot absorb that. `entered_object_blocks_incremental` + // (1) escalates for an entrant that arrives sourcing a continuous effect, + // precisely because effects here are re-applied only to `recipient_ids` — + // pre-existing permanents are never reset, so a board-wide anthem a recipient + // acquires would never reach the objects it must apply to. A copy that grants + // a static reaches the same state one step later, so it takes the same exit. + // Escalating BEFORE any copy is applied also keeps the generator set of the + // index built above true for the whole of `apply_layers_incremental`. + // + // Reached by filter match, not only by name: the copy set below is applied + // with `apply_continuous_effect_to(state, effect, &recipient_ids, ..)`, which + // restricts an ordinary board-wide `affected_filter` to the recipients rather + // than requiring `TargetFilter::SpecificObject`. Neither + // `active_effects_force_incremental_escalation` (magnitude / affected-set + // population sensitivity) nor the source-side guard above sees that case: the + // copy's source is a pre-existing permanent and its filter counts nothing. + // + // That same `restrict_to` intersection is why the guard asks + // `effect_can_reach_incremental_recipients` rather than firing on the mere + // presence of a copy: a `SelfRef` / `SpecificObject` copy that names a + // PRE-EXISTING permanent — the ordinary shape, every resolved clone on the + // board carries one — applies to the empty set on this path, so it can hand no + // recipient anything, and escalating for it would put every board that has + // ever cast a clone back on the O(|battlefield|) full pass this module exists + // to avoid. A copy that does name a recipient still escalates; so does any + // copy whose affected set is a predicate, since that set is not decidable + // here. + // + // O(active-effect-count), and zero on the overwhelmingly common board with no + // copy effect at all. + if active_effects.iter().any(|effect| { + effect.layer == Layer::Copy + && effect_can_reach_incremental_recipients(effect, &recipient_ids) + && copy_grants_continuous_static(&effect.modification) + }) { + return None; + } Some(PreparedIncrementalFlush { recipient_ids, @@ -3799,17 +3874,372 @@ fn incremental_recipient_ids( recipients } +/// The single object an effect's `affected_filter` provably names, when it names +/// one by id at all. `None` means the filter is a PREDICATE over the board +/// (`Typed`, `And`, `Or`, ...) whose membership can only be decided by evaluating +/// it against every object — which neither caller below does, so each supplies +/// its own conservative answer for that case. +/// +/// An id-naming filter is exactly the case where the affected set is known +/// without a board scan. (No CR annotation: this decides nothing about the rules, +/// it only reports what a filter's shape already tells us.) +fn effect_names_single_affected_object(effect: &ActiveContinuousEffect) -> Option { + match &effect.affected_filter { + TargetFilter::SelfRef => Some(effect.source_id), + TargetFilter::SpecificObject { id } => Some(*id), + _ => None, + } +} + +/// Is this effect provably CONFINED to the incremental recipients — i.e. can the +/// restricted pass reproduce it in full? Unknown affected sets answer `false` +/// (escalate on doubt): a board-wide filter also touches pre-existing objects +/// that the incremental arm never resets. fn effect_is_restricted_to_incremental_recipients( effect: &ActiveContinuousEffect, recipient_ids: &BTreeSet, ) -> bool { - match &effect.affected_filter { - TargetFilter::SelfRef => recipient_ids.contains(&effect.source_id), - TargetFilter::SpecificObject { id } => recipient_ids.contains(id), + effect_names_single_affected_object(effect).is_some_and(|id| recipient_ids.contains(&id)) +} + +/// Can this effect REACH at least one incremental recipient? The dual question to +/// [`effect_is_restricted_to_incremental_recipients`], and deliberately not the +/// same predicate: the two agree on an id-naming filter (a set of one is confined +/// iff it is reached) but their conservative answers for an unknown affected set +/// are OPPOSITE, because both must escalate on doubt and doubt sits on different +/// sides of the two questions. +fn effect_can_reach_incremental_recipients( + effect: &ActiveContinuousEffect, + recipient_ids: &BTreeSet, +) -> bool { + effect_names_single_affected_object(effect).is_none_or(|id| recipient_ids.contains(&id)) +} + +/// CR 613.2a + CR 613.2c: does applying this layer-1 copy modification hand its +/// recipient a continuous static ability — i.e. can it turn a non-generator into a +/// `StaticSourceIndex` generator mid-pass? +/// +/// Shares the index's own classification predicate +/// (`static_source_index::defs_source_continuous_effect`, the body of +/// `object_sources_continuous_effect`): a generator is an object carrying a +/// `StaticMode::Continuous` def. `CopyValues` carries its whole copiable static +/// set in the modification payload and `apply_copiable_values` assigns that set +/// wholesale, so the answer is exact — no id plumbing out of the apply path, and +/// no battlefield rescan. +/// +/// The six variants below are exactly `ContinuousModification::layer()`'s +/// `Layer::Copy` set; every other variant belongs to layers 2-7 and is filtered +/// out by both callers before they ask. +fn copy_grants_continuous_static(modification: &ContinuousModification) -> bool { + match modification { + ContinuousModification::CopyValues { values, .. } => { + crate::game::static_source_index::defs_source_continuous_effect( + &values.static_definitions, + ) + } + // CR 707.9b: name-only override; never writes `static_definitions`. + ContinuousModification::SetName { .. } => false, + // CR 707.2c: parse-time marker whose apply arm is an explicit no-op. The + // real copy is the latched `CopyValues` TCE, which sits in this same + // bucket and is classified on its own payload. + ContinuousModification::CopyChosen => false, + // CR 707.9a: trigger-set retention only. + ContinuousModification::RetainPrintedTriggerFromSource { .. } => false, + // CR 707.9a: single-ability retention. Its apply arm pushes one + // `AbilityDefinition` read from the source's `base_abilities` onto + // `obj.abilities` and touches no other set — `static_definitions` is + // structurally out of its reach, so the Sakashima-class single-ability + // retain provably adds no generator. + ContinuousModification::RetainPrintedAbilityFromSource { .. } => false, + // CR 707.9a: the unbounded retain genuinely merges the source's + // `base_static_definitions` back onto the copy, so it can add a + // generator. Answered `true` without inspecting the retained set, which + // lives on the live source rather than in this payload. + ContinuousModification::RetainAllOtherAbilitiesFromSource => true, + // Layers 2-7. `ContinuousModification` spans all eleven layers in one + // enum, so this arm is required for totality; it is EXACT rather than a + // fallback guess — no modification outside layer 1 writes copied + // `static_definitions`, and both callers filter to `Layer::Copy` first. + // Deliberately does not consult `other.layer()`: six of that method's + // arms are `unreachable!()` panics (`AddCounterOnEnter`, + // `SetStartingLoyalty`, `RemoveManaCost`, and the three + // combat-assignment variants), so the guard that was meant to make this + // arm safe could abort inside itself. _ => false, } } +/// CR 613.2a: can applying this copy modification produce ANOTHER generation of +/// sublayer 1a — i.e. does its payload grant a continuous static that itself +/// generates a LAYER-1 effect? +/// +/// Strictly narrower than [`copy_grants_continuous_static`], and the two are not +/// interchangeable. That one asks "does the recipient become a +/// `StaticSourceIndex` generator?", which is the right question for the layer-1 +/// exit rebuild that feeds layers 2-7 and for the incremental-flush guard. +/// Cloning an ordinary lord answers it yes — an anthem is a continuous static — +/// but an anthem is a layer-7c effect and can never appear in a `Layer::Copy` +/// gather however many times 1a re-runs. Continuing the loop on that answer buys +/// a `StaticSourceIndex::rebuild_from_state` plus a board-wide +/// `collect_shared_active_continuous_effects` for a generation that is +/// guaranteed to find nothing, on the very common board that merely contains a +/// cloned lord. Only a copy-LAYER static can extend the chain. +/// +/// Asks [`ContinuousModification::is_copy_layer`] rather than `layer()`: these +/// are payload modifications that have never been through the gather filter, and +/// six of `layer()`'s arms are `unreachable!()` panics. +fn copy_grants_copy_layer_static(modification: &ContinuousModification) -> bool { + match modification { + ContinuousModification::CopyValues { values, .. } => values + .static_definitions + .iter() + // CR 604.2 + CR 613.1: a STATIC ability is what creates a continuous + // effect here, so only a `Continuous` def counts. (CR 611.2, which + // `defs_source_continuous_effect` cites for the same test, is the + // resolution-generated case; the answer is identical, the rule for a + // static is 604.2.) + .filter(|def| def.mode == StaticMode::Continuous) + .flat_map(|def| def.modifications.iter()) + // `is_copy_layer` is total over `Layer::Copy`, so it answers `true` for + // a nested `CopyValues` too. That sub-case is unreachable, for the same + // reason `apply_continuous_effect`'s `"CopyValues must originate from a + // transient continuous effect"` expect is sound: every construction of + // `CopyValues` installs it through `add_transient_continuous_effect` + // (`become_copy::apply_precomputed_copy_values`, + // `merge::install_merge_layer_effect`) and card data never authors one, + // so no `StaticDefinition` — and hence no `CopiableValues` snapshot of + // one — can hold it. The two seams therefore disagree about whether the + // state is possible — this one buys a discovery generation for it, that + // one panics on it — and the disagreement is deliberate: the expect is + // where the invariant is enforced, so a future reachable case is a + // construction-site bug to fix there, not a case to quietly admit by + // special-casing it out of this totality. + .any(ContinuousModification::is_copy_layer), + // CR 707.9a: the unbounded retain merges the LIVE source's + // `base_static_definitions`, which are not in this payload to inspect, so + // it is answered conservatively — as in `copy_grants_continuous_static`. + // Over-answering costs at most one extra generation, which then gathers + // nothing fresh and exits. + ContinuousModification::RetainAllOtherAbilitiesFromSource => true, + // The remaining `Layer::Copy` variants write no `static_definitions` at + // all (`SetName` is name-only, `CopyChosen`'s apply arm is a no-op, and + // the two single-item retains push one ability / one trigger), and every + // layers-2-7 variant is out of both callers' `Layer::Copy` filter. + _ => false, + } +} + +/// CR 613.2a + CR 613.2c: apply sublayer 1a to a fixed point. +/// +/// A copy effect can hand a permanent a static ability its copiable base does not +/// carry, and CR 613.2c says that after all of layer 1 has been applied the +/// object's characteristics ARE its copiable values. When that granted static is +/// itself a copy effect, it belongs to THIS sublayer — so gathering layer 1 once +/// is not enough: the sublayer has to be iterated until no new copy effect +/// appears. +/// +/// Ordering is preserved ACROSS generations, not just within one: each generation +/// re-orders and re-applies the whole started set (CR 613.2 timestamp order, +/// CR 613.8a dependency order), never just the newly discovered tail. That +/// matters because `depends_on` sorts every `CopyValues` ahead of the other +/// layer-1 modifications, so a copy discovered late can legitimately need to +/// apply before one already applied. Re-application is safe, and CR 707.2c is +/// why: the copiable values a copy effect grants are fixed the first time it +/// starts to apply, so re-applying a started effect re-applies that same locked +/// snapshot. Mechanically, `CopyValues` / `SetName` assign wholesale from the +/// payload, the retains dedupe per item, `CopyChosen` is a no-op, and +/// `started_effect_sets` hands an already-started effect back its original +/// affected set (CR 613.6). Nothing is ever pruned from the started set for the +/// same reason: an effect whose provenance slot has since been overwritten still +/// applies the values it locked in, so dropping it would be the CR 707.2c +/// violation, not keeping it. +/// +/// Termination. The loop's identity ([`CopySublayerEffectId`]) is provenance, +/// never payload, so the space it draws from is finite and does not grow during +/// the pass: +/// +/// - The battlefield object set is fixed for the duration of layer 1 — nothing +/// in sublayer 1a creates, destroys or moves an object — so the `Static` and +/// `GrantedStatic` arms of `ContinuousEffectGroupKey` range over a fixed set of +/// `ObjectIncarnationRef`s (`incarnation` changes only on a zone change), and +/// the `Transient` arm over the fixed id set of +/// `state.transient_continuous_effects`. +/// - `definition_index` and `mod_index` are bounded by the largest static set any +/// object can hold during the pass, and no arm can mint fresh ones without +/// bound: `apply_copiable_values` ASSIGNS `obj.static_definitions` wholesale +/// from the payload's snapshot rather than appending, +/// `RetainAllOtherAbilitiesFromSource` merges the source's +/// `base_static_definitions` behind a per-item `contains` dedupe, and every +/// other `Layer::Copy` arm writes no static at all. Each payload is itself a +/// snapshot taken before the pass began, so the reachable static sets are fixed +/// too. +/// +/// Each iteration therefore either inserts at least one identity into the +/// monotonically growing `started_ids` — bounded above by that finite space — or +/// gathers nothing fresh and returns. So the loop terminates, with no generation +/// cap needed to force it. +/// +/// Returns whether any applied copy added a `StaticSourceIndex` generator, which +/// the caller needs for the layer-1 exit rebuild that feeds layers 2-7. +fn apply_copy_sublayer_to_fixed_point( + state: &mut GameState, + abilities_suppressed: &mut HashSet, + zone_cache: &mut LayerZoneObjectCache, + started_effect_sets: &mut StartedContinuousEffectSets, +) -> bool { + let mut started: Vec = Vec::new(); + let mut started_ids: HashSet = HashSet::new(); + let mut copy_added_generator = false; + + loop { + // `insert` returns `false` for an identity already started, so this both + // selects the fresh effects and records them in one O(1)-per-effect pass. + let fresh: Vec = + gather_active_effects_for_layer(state, Layer::Copy) + .into_iter() + .filter(|effect| started_ids.insert(copy_sublayer_effect_id(state, effect))) + .collect(); + if fresh.is_empty() { + // Fixed point: this generation's gather produced nothing the + // previous ones had not already started applying. + return copy_added_generator; + } + + copy_added_generator |= fresh + .iter() + .any(|effect| copy_grants_continuous_static(&effect.modification)); + // Loop continuation is the NARROW question: only a copy-layer static can + // show up in the next `Layer::Copy` gather. A cloned lord answers + // `copy_grants_continuous_static` yes and still cannot extend the chain. + let fresh_added_copy_generator = fresh + .iter() + .any(|effect| copy_grants_copy_layer_static(&effect.modification)); + + let reapplying = !started.is_empty(); + started.extend(fresh); + let ordered = order_active_continuous_effects(Layer::Copy, &started, state); + if reapplying { + // 1a re-runs from the top, so its display attribution is re-derived + // from the top too — otherwise a re-applied effect would be listed + // twice, in stale order. Only the Copy bucket exists yet: Step 1 + // cleared the table and no later layer has run. + for (_, attribution) in state.attribution.iter_mut() { + attribution.by_layer.remove(&Layer::Copy); + } + } + for effect in &ordered { + apply_continuous_effect( + state, + effect, + abilities_suppressed, + zone_cache, + started_effect_sets, + ); + } + + if !fresh_added_copy_generator { + // Nothing this generation can put a copy-layer static on the board, + // so the next gather would return the same set. Stop without paying + // the O(battlefield) rebuild. If a NON-copy generator was added, the + // returned flag makes the caller do the layer-1 exit rebuild instead. + return copy_added_generator; + } + // The index was built from definitions that predate these copies; refresh + // it so the next gather can see a copy-granted copy ability. + crate::types::game_state::StaticSourceIndex::rebuild_from_state(state); + } +} + +/// CR 707.2c: identity of a layer-1 copy effect for the fixed-point loop. +/// +/// PROVENANCE ONLY — never the payload. CR 707.2c: "If a static ability generates +/// a continuous effect that's a copy effect, the copiable values that effect +/// grants are determined only at the time that effect first starts to apply." So +/// re-reading a provenance that has already started is the SAME effect granting +/// the SAME locked values; a rewritten payload does not mint a new one. Keying on +/// the payload instead both contradicts that rule and +/// makes the loop non-convergent by construction — two permanents that rewrite +/// each other's static sets would produce payload-distinct identities forever, +/// which is exactly what a generation cap used to have to paper over. +/// +/// Built on [`ContinuousEffectGroupKey`], this file's canonical "which effect is +/// this" answer, rather than a hand-rolled tuple. Two reasons: it is what +/// `started_effect_sets` already keys on, so the loop and `apply_continuous_effect` +/// can no longer disagree about whether an effect has started (a disagreement is a +/// CR 613.6 bug — the loop calls an effect new while the applier hands it back an +/// already-locked affected set); and it distinguishes +/// `GrantedStatic { grant_origin, recipient }` occurrences that a raw +/// `(source_id, def_index, transient_id)` tuple collides on. That second reason is +/// forward-looking here, not load-bearing: `GrantedStatic` is live in the layers +/// that share this key, but reaching it from THIS loop needs a +/// `GrantStaticAbility` whose inner definition is copy-layer, and no card prints +/// one (see the pool argument below). Sharing the key is still the right call — +/// the alternative is a second identity that can drift from the one +/// `apply_continuous_effect` enforces. +/// +/// The three provenances are not equally precise, and this identity claims no more +/// than each one carries. `Transient { continuous_effect_id }` names an allocated +/// id and `GrantedStatic { grant_origin, recipient }` names a grant origin — +/// nothing in a layer pass can make either denote a different ability. +/// `Static { source, definition_index }` names a POSITION in +/// `obj.static_definitions`, and [`apply_copiable_values`] assigns that vector +/// wholesale from the copy payload. So slot `i` could in principle denote a +/// different static ability in generation N+1 than in generation N, and the +/// newcomer would be filtered out as already-started while the effect it displaced +/// kept re-applying. +/// +/// That board is not constructible today, and the residual is accepted rather than +/// closed: `ContinuousEffectGroupKey` is the shared key `started_effect_sets` uses +/// in every layer, so de-positioning its `Static` arm is a change to all seven +/// layers rather than to this loop. Reaching the hazard needs a `Layer::Copy` +/// effect that is `Static`-keyed, and no producer makes one: +/// +/// - Both engine construction sites of a `Layer::Copy` modification — +/// `become_copy::apply_precomputed_copy_values` and +/// `merge::install_merge_layer_effect` — install through +/// `add_transient_continuous_effect`, so the effect is `Transient`-keyed. +/// - `expand_granted_static_effects` sets `def_index: None`, so a granted +/// copy-layer static is `GrantedStatic`-keyed. +/// - Card data is the only other producer of `StaticDefinition`s, and in the +/// generated pool every copy-layer modification that sits inside one at all sits +/// inside a `GenericEffect` payload — Awakening of Vitu-Ghazi, Tenth District +/// Hero, The Curse of Fenric, The Irencrag, all `SetName` — which +/// `effects::effect` resolves through `register_transient_effect`. No card +/// carries a copy-layer modification in its printed `static_abilities`, so no +/// route into an object's `static_definitions` can carry one either: the copy +/// payload ([`apply_copiable_values`]), the `GrantStaticAbility` graft, and the +/// `RetainPrintedAbilityFromSource` graft all replay card-data statics. +/// +/// The first card to print a copy-layer static ability directly — rather than +/// creating its copy effect from a resolving spell or ability — is the trigger to +/// revisit this. +/// +/// `mod_index` rides alongside because the group key deliberately drops it — every +/// modification of one definition shares one CR 613.6 affected set — while this +/// loop must re-apply EACH modification of a multi-modification copy definition +/// (`CopyValues` + its `SetName` exception, say), not just the first. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +struct CopySublayerEffectId { + /// `None` only for a synthetic effect carrying no static, transient or grant + /// provenance at all (the Ring emblem, stickers). No synthetic producer emits + /// a `Layer::Copy` modification today, so this is unreachable here; carrying + /// `source_id` keeps the identity total instead of resting on that fact. + group: Option, + source_id: ObjectId, + mod_index: usize, +} + +fn copy_sublayer_effect_id( + state: &GameState, + effect: &ActiveContinuousEffect, +) -> CopySublayerEffectId { + CopySublayerEffectId { + group: continuous_effect_group_key(state, effect), + source_id: effect.source_id, + mod_index: effect.mod_index, + } +} + /// Incremental layer re-derivation for a set of freshly-entered objects. /// /// Mirrors the PER-OBJECT subset of `evaluate_layers` for `entered_ids` only: @@ -3837,9 +4267,18 @@ fn apply_layers_incremental(state: &mut GameState, prepared: PreparedIncremental // static-source index rebuild and shared active-effect collection. // Step 2: Copy effects first (Layer 1), restricted to recipient objects. + // + // `effect_can_reach_incremental_recipients` drops a copy that provably names a + // NON-recipient — the ordinary resolved-clone shape. `apply_continuous_effect_to` + // would skip every one of its objects at the `restrict_to` test anyway, so this + // changes no state; what it buys is an accurate `copy_effects.is_empty()` below, + // which is what decides whether this flush pays a second board-wide gather. let copy_effects: Vec = active_effects .iter() - .filter(|effect| effect.layer == Layer::Copy) + .filter(|effect| { + effect.layer == Layer::Copy + && effect_can_reach_incremental_recipients(effect, &recipient_ids) + }) .cloned() .collect(); let ordered_copy = order_active_continuous_effects(Layer::Copy, ©_effects, state); @@ -3857,6 +4296,36 @@ fn apply_layers_incremental(state: &mut GameState, prepared: PreparedIncremental let recipient_vec: Vec = recipient_ids.iter().copied().collect(); let stickers_changed = crate::game::stickers::apply_battlefield_name_and_ability_stickers(state, &recipient_vec); + // CR 613.2a + CR 613.2c: deliberately no copy disjunct on this rebuild, unlike + // the full pass. A copy applied here could only add a generator by landing on + // a recipient, and it cannot: `recipient_ids` is `entered_ids` alone, because + // an attached entrant — the one way a pre-existing host joins the set — is + // rejected outright by `entered_object_blocks_incremental` guard (3); and a + // copy TCE naming a fresh entrant can only have been installed by that same + // entry, which called `layers_dirty.mark_full()` (`game_state.rs`, + // `apply_resolved_continuous_effect`) and so made this flush a full pass. + // + // That argument covers only a copy that NAMES a recipient. A pre-existing copy + // effect also reaches a recipient by plain filter match, because + // `apply_continuous_effect_to` restricts an ordinary board-wide + // `affected_filter` to `recipient_ids` rather than requiring + // `TargetFilter::SpecificObject`. What rules that case out is the escalation + // guard in `prepare_incremental_flush`: any active copy that can REACH a + // recipient and whose payload grants a continuous static sends the whole flush + // to `evaluate_layers`. The copies it lets through are exactly those confined + // to a named non-recipient, which the `copy_effects` filter above then drops — + // so either way no copy surviving to this line can turn a recipient into a + // `StaticSourceIndex` generator. The index built there still names exactly the + // right sources and a rebuild would find the identical set. (It would also be + // the wrong repair anyway: a guard that fans a copy-granted static over + // `recipient_ids` alone would not agree with the full pass, which derives it + // board-wide.) + // + // The re-collect below is not dead weight, though. A copy changes its + // recipients' characteristics, and a pre-existing generator's + // `GrantStaticAbility` fan-out is computed per matching recipient when that + // generator is visited — same generator set, different collected effects. So + // the effect set must be re-read after the copy even though the index need not. let active_effects = if stickers_changed { // Incremental resets clear the entered/attached recipients back to base, // so retained stickers must be re-applied before the restricted main @@ -20348,6 +20817,544 @@ mod tests { ); } + /// CR 613.2a + CR 613.2c: an entry-incremental flush must ESCALATE while a + /// copy effect whose payload carries a continuous static is active. + /// + /// The incremental arm applies every active effect through + /// `apply_continuous_effect_to(state, effect, &recipient_ids, ..)`, which + /// NARROWS an ordinary board-wide `affected_filter` down to the entrants — so + /// a PRE-EXISTING copy effect reaches a fresh entrant by FILTER MATCH, never + /// having to name it with `TargetFilter::SpecificObject`. The entrant would + /// then hold a continuous static (here an anthem) that the incremental arm + /// can never fan out over the pre-existing board, because pre-existing + /// objects are not reset — the same state `entered_object_blocks_incremental` + /// already escalates for, reached one step later. Neither existing guard sees + /// it: the entrant's own BASE carries no static, and the copy's source is a + /// pre-existing permanent whose filter counts nothing. + /// + /// REVERT-PROBE (discriminating, RUN): delete the `Layer::Copy` guard at the + /// end of `prepare_incremental_flush` ⇒ `layers_incremental == 1` and + /// `layers_full_eval == 0`, and this test fails on the branch assertions. + #[test] + fn entry_incremental_escalates_when_a_live_copy_payload_carries_a_static() { + let mut state = setup(); + let player = PlayerId(0); + let template = make_creature(&mut state, "Template", 2, 2, player); + let mut copied_values = intrinsic_copiable_values(&state.objects[&template]); + Arc::make_mut(&mut copied_values.static_definitions).push( + StaticDefinition::continuous() + .affected(TargetFilter::Typed( + TypedFilter::creature().controller(ControllerRef::You), + )) + .modifications(vec![ContinuousModification::AddPower { value: 1 }]), + ); + // Board-wide `affected`, NOT `SpecificObject`: this is the shape that + // reaches an object which did not exist when the effect was registered. + let caster = make_creature(&mut state, "Caster", 1, 1, player); + state.add_transient_continuous_effect( + caster, + player, + Duration::Permanent, + TargetFilter::Typed(TypedFilter::creature().controller(ControllerRef::You)), + vec![ContinuousModification::CopyValues { + values: Box::new(copied_values), + display_source: crate::game::game_object::DisplaySource::Card, + printed_ref: None, + token_image_ref: None, + }], + None, + ); + evaluate_layers(&mut state); + + let entrant = make_creature(&mut state, "Entrant", 1, 1, player); + // Reach-guard asked of the guard ITSELF, not of a neighbouring field: this + // is the exact predicate `prepare_incremental_flush` consults first, and it + // reads the LIVE `static_definitions`. If it answered true the escalation + // below would be its doing and this test would stop probing the copy guard. + assert!( + !entered_object_blocks_incremental(&state, &state.objects[&entrant]), + "the entrant must be an ordinary entry, or the existing \ + `entered_object_blocks_incremental` guard would escalate instead" + ); + + crate::game::perf_counters::reset(); + state.layers_dirty = LayersDirty::EnteredObjects([entrant].into()); + flush_layers(&mut state); + let counters = crate::game::perf_counters::snapshot(); + + assert_eq!( + counters.layers_full_eval, 1, + "a live copy effect whose payload grants a continuous static must force \ + a full pass" + ); + assert_eq!( + counters.layers_incremental, 0, + "the incremental arm must not run for this entry" + ); + // POSITIVE reach-guard: the copy really did reach the entrant by filter + // match, so the escalation above is not vacuous. + assert!( + !state.objects[&entrant].static_definitions.is_empty(), + "the board-wide copy must have handed the entrant the payload's static" + ); + } + + /// The other side of that guard: a copy effect that provably names a + /// PRE-EXISTING permanent must NOT escalate. This is the ordinary resolved + /// clone — every Clone / Vizier copy on a board carries a `SpecificObject` + /// (or `SelfRef`) filter naming one permanent — so escalating on the mere + /// PRESENCE of a copy would drop every board that has ever resolved one back + /// onto the O(|battlefield|) full pass this module exists to avoid. + /// + /// It is sound because the incremental arm applies copies through + /// `apply_continuous_effect_to(state, effect, &recipient_ids, ..)`: a filter + /// naming a non-recipient intersects `recipient_ids` to the empty set, so it + /// can hand the entrant nothing, and the clone's own copy-granted static — + /// already live on it from the earlier full pass, never reset here — is picked + /// up by the top-of-pass `StaticSourceIndex` rebuild like any other generator. + /// The differential assertion below is what pins that: the entrant must come + /// out of the incremental flush wearing the CLONE's anthem. + /// + /// REVERT-PROBE (discriminating, RUN): drop + /// `effect_can_reach_incremental_recipients` from the guard ⇒ + /// `layers_full_eval == 1` / `layers_incremental == 0` and the two branch + /// assertions fail. + #[test] + fn entry_incremental_stays_incremental_for_a_copy_naming_a_pre_existing_object() { + let mut state = setup(); + let player = PlayerId(0); + let template = make_creature(&mut state, "Template", 2, 2, player); + let mut copied_values = intrinsic_copiable_values(&state.objects[&template]); + Arc::make_mut(&mut copied_values.static_definitions).push( + StaticDefinition::continuous() + .affected(TargetFilter::Typed( + TypedFilter::creature().controller(ControllerRef::You), + )) + .modifications(vec![ContinuousModification::AddPower { value: 1 }]), + ); + // The clone exists BEFORE the flush under test, and the copy effect names + // it — the shape a resolved clone actually has on a live board. + let clone = make_creature(&mut state, "Clone", 1, 1, player); + let caster = make_creature(&mut state, "Caster", 1, 1, player); + state.add_transient_continuous_effect( + caster, + player, + Duration::Permanent, + TargetFilter::SpecificObject { id: clone }, + vec![ContinuousModification::CopyValues { + values: Box::new(copied_values), + display_source: crate::game::game_object::DisplaySource::Card, + printed_ref: None, + token_image_ref: None, + }], + None, + ); + evaluate_layers(&mut state); + + // POSITIVE reach-guards, both on the live fields the guard consults: the + // copy really did land, so `copy_grants_continuous_static` answers TRUE for + // this effect and the only thing keeping the flush incremental is the reach + // test. Without this the assertions below would pass for the wrong reason. + assert!( + !state.objects[&clone].static_definitions.is_empty(), + "the clone must be carrying the payload's continuous static, or the \ + guard's copy branch is never exercised" + ); + + let entrant = make_creature(&mut state, "Entrant", 1, 1, player); + // Same reach-guard as the escalating sibling, asked of the predicate itself + // (it reads the LIVE `static_definitions`, not the base). + assert!( + !entered_object_blocks_incremental(&state, &state.objects[&entrant]), + "the entrant must be an ordinary entry, or the incremental arm would be \ + refused for a reason unrelated to the copy guard" + ); + + crate::game::perf_counters::reset(); + state.layers_dirty = LayersDirty::EnteredObjects([entrant].into()); + flush_layers(&mut state); + let counters = crate::game::perf_counters::snapshot(); + + assert_eq!( + counters.layers_incremental, 1, + "a copy confined to a pre-existing permanent cannot reach the entrant, \ + so the flush must stay incremental" + ); + assert_eq!( + counters.layers_full_eval, 0, + "no full pass may be forced by a copy the entrant is out of reach of" + ); + // CR 613.2c: after layer 1 the clone's copied static is part of the board, + // so its anthem must reach the entrant on THIS path — 1/1 printed, +1/+0 + // from the clone. + assert_eq!( + ( + state.objects[&entrant].power, + state.objects[&entrant].toughness + ), + (Some(2), Some(1)), + "the entrant must still receive the clone's copy-granted anthem" + ); + } + + /// CR 613.2a: `copy_grants_continuous_static` answers over the COMPLETE + /// `Layer::Copy` modification set, and its `_ => false` arm is exact rather + /// than a fallback guess. Every variant here is also asserted to report + /// `Layer::Copy` (and the layers-2-7 sample NOT to), which pins the claim the + /// catch-all rests on. `layer()` itself is deliberately not consulted by the + /// function under test: six of its arms are `unreachable!()` panics, so the + /// `debug_assert_eq!(other.layer(), Layer::Copy)` guard this replaced could + /// abort inside itself — `AddCounterOnEnter` below is one such variant and is + /// answered without ever asking for its layer. + #[test] + fn copy_grants_continuous_static_covers_every_copy_layer_variant() { + let mut state = setup(); + let player = PlayerId(0); + let donor = make_creature(&mut state, "Donor", 2, 2, player); + let plain_values = intrinsic_copiable_values(&state.objects[&donor]); + assert!( + plain_values.static_definitions.is_empty(), + "the vanilla donor must carry no static, or the negative case below \ + would not be a negative case" + ); + let mut anthem_values = plain_values.clone(); + Arc::make_mut(&mut anthem_values.static_definitions).push( + StaticDefinition::continuous() + .modifications(vec![ContinuousModification::AddPower { value: 1 }]), + ); + // Same shape, but the payload's static is a LAYER-1 one — the only thing + // that can extend sublayer 1a's discovery chain. + let mut copy_static_values = plain_values.clone(); + Arc::make_mut(&mut copy_static_values.static_definitions).push( + StaticDefinition::continuous().modifications(vec![ContinuousModification::SetName { + name: "Renamed".into(), + }]), + ); + let copy_of = |values: CopiableValues| ContinuousModification::CopyValues { + values: Box::new(values), + display_source: crate::game::game_object::DisplaySource::Card, + printed_ref: None, + token_image_ref: None, + }; + + // The whole `Layer::Copy` set: (modification, adds ANY generator, adds a + // COPY-LAYER generator). The two columns are the two different questions + // layer 1 asks, and the anthem row is where they must disagree. + let copy_layer_cases = [ + // Payload-exact: the copiable static set travels inside the + // modification, so no live-source read and no battlefield rescan. + // An anthem makes its recipient a generator for layers 2-7 but can + // never appear in a `Layer::Copy` gather, so it must NOT buy a + // discovery generation. + (copy_of(anthem_values), true, false), + (copy_of(copy_static_values), true, true), + (copy_of(plain_values), false, false), + // CR 707.9a: merges the source's whole printed set, INCLUDING its + // `base_static_definitions`, onto the recipient (see the apply arm). + // The live source is not in the payload, so both answers are + // conservative. + ( + ContinuousModification::RetainAllOtherAbilitiesFromSource, + true, + true, + ), + // CR 707.9a: pushes ONE `AbilityDefinition` onto `obj.abilities`; + // `static_definitions` is structurally out of reach. + ( + ContinuousModification::RetainPrintedAbilityFromSource { + source_ability_index: 0, + }, + false, + false, + ), + // CR 707.9a: pushes one trigger; likewise never a static. + ( + ContinuousModification::RetainPrintedTriggerFromSource { + source_trigger_index: 0, + }, + false, + false, + ), + // CR 707.9b: name-only override. + ( + ContinuousModification::SetName { + name: "Renamed".into(), + }, + false, + false, + ), + // CR 707.2c: parse-time marker whose apply arm is an explicit no-op. + (ContinuousModification::CopyChosen, false, false), + ]; + for (modification, expected_any, expected_copy_layer) in ©_layer_cases { + assert_eq!( + modification.layer(), + Layer::Copy, + "{modification:?} must be a layer-1 modification for this table to be \ + the complete `Layer::Copy` set" + ); + // Pins the panic-free companion to `layer()` over the same set — the + // invariant `is_copy_layer`'s doc claims. + assert!( + modification.is_copy_layer(), + "{modification:?} is in the `Layer::Copy` set, so `is_copy_layer` \ + must agree with `layer()`" + ); + assert_eq!( + copy_grants_continuous_static(modification), + *expected_any, + "wrong generator answer for {modification:?}" + ); + assert_eq!( + copy_grants_copy_layer_static(modification), + *expected_copy_layer, + "wrong copy-layer generator answer for {modification:?}" + ); + } + + // Layers 2-7 fall to the catch-all. `AddCounterOnEnter` is one of the six + // variants whose `layer()` is `unreachable!()`, so it is exactly the input + // that made the replaced `debug_assert_eq!(other.layer(), ..)` unsafe — + // and exactly why `is_copy_layer` exists to be asked instead. + for modification in [ + ContinuousModification::AddCounterOnEnter { + counter_type: CounterType::Plus1Plus1, + count: QuantityExpr::Fixed { value: 1 }, + if_type: None, + }, + ContinuousModification::AddPower { value: 1 }, + ] { + assert!( + !modification.is_copy_layer(), + "{modification:?} is not a layer-1 modification" + ); + assert!( + !copy_grants_continuous_static(&modification), + "{modification:?} is not a layer-1 modification and must not claim to \ + add a generator" + ); + assert!( + !copy_grants_copy_layer_static(&modification), + "{modification:?} is not a layer-1 modification and must not buy a \ + discovery generation" + ); + } + } + + /// CR 707.2c + CR 613.2c: sublayer 1a runs to a FIXED POINT on the exact + /// construction the deleted `MAX_COPY_SUBLAYER_GENERATIONS = 8` named — "two + /// permanents that rewrite each other's static sets". Reaching the assertions + /// at all IS the termination evidence; the assertions are the CR-correctness + /// evidence. + /// + /// Each half of the pair is copied from a donor whose copiable values carry a + /// copy-LAYER static (CR 707.2), so BOTH second-generation effects exist only + /// because the first generation applied, in both directions at once. The board + /// is synthetic in the sense that no printed pair does exactly this, but every + /// piece is a real mechanism: a latched `CopyValues` TCE (the only way a copy + /// effect can originate — `apply_continuous_effect` requires `transient_id` for + /// it) whose payload carries a `SetName` copy exception. + /// + /// The two copy-granted renames land on neutral watchers rather than on the + /// pair itself, ON PURPOSE. Two permanents copying each other is a CR 613.8b + /// dependency LOOP, so the engine correctly discards the dependency edges and + /// falls back to timestamp order, under which each half's older static applies + /// before the newer `CopyValues` overwrites its name. That is a separate rule + /// from the one under test, and asserting on the pair's own names would pin + /// 613.8b's tie-break instead of 1a's fixed point. + /// + /// REVERT-PROBE (discriminating, run): make the loop return after its first + /// generation ⇒ both watchers keep their printed names and both rename + /// assertions fail, while the copy assertions stay green. + #[test] + fn mutually_copying_permanents_reach_a_layer_one_fixed_point() { + let mut state = setup(); + let player = PlayerId(0); + let left = make_creature(&mut state, "Left", 1, 1, player); + let right = make_creature(&mut state, "Right", 1, 1, player); + let watcher_l = make_creature(&mut state, "Watcher L", 1, 1, player); + let watcher_r = make_creature(&mut state, "Watcher R", 1, 1, player); + let donor_p = make_creature(&mut state, "Donor P", 4, 4, player); + let donor_q = make_creature(&mut state, "Donor Q", 5, 5, player); + + // Donor payloads: each carries a copy-layer static that renames a watcher. + let renames = |state: &GameState, donor: ObjectId, target: ObjectId, name: &str| { + let mut values = intrinsic_copiable_values(&state.objects[&donor]); + Arc::make_mut(&mut values.static_definitions).push( + StaticDefinition::continuous() + .affected(TargetFilter::SpecificObject { id: target }) + .modifications(vec![ContinuousModification::SetName { + name: name.to_string(), + }]), + ); + values + }; + let p_values = renames(&state, donor_p, watcher_l, "Marked by right"); + let q_values = renames(&state, donor_q, watcher_r, "Marked by left"); + + let mut copy_onto = |source: ObjectId, target: ObjectId, values: CopiableValues| { + state.add_transient_continuous_effect( + source, + player, + Duration::Permanent, + TargetFilter::SpecificObject { id: target }, + vec![ContinuousModification::CopyValues { + values: Box::new(values), + display_source: crate::game::game_object::DisplaySource::Card, + printed_ref: None, + token_image_ref: None, + }], + None, + ); + }; + // `left` copies donor P onto `right`; `right` copies donor Q onto `left`. + copy_onto(left, right, p_values); + copy_onto(right, left, q_values); + + // POSITIVE reach-guards, on the live field the top-of-pass reset writes and + // `for_each_static_effect_source` gathers: neither permanent carries a + // PRINTED static, so every second-generation effect below can only exist + // because generation 1's copy put it there. + assert!( + state.objects[&left].base_static_definitions.is_empty() + && state.objects[&right].base_static_definitions.is_empty(), + "neither half of the pair may carry a printed static, or the renames \ + would not depend on the copies at all" + ); + + evaluate_layers(&mut state); + + assert!( + !state.objects[&left].static_definitions.is_empty() + && !state.objects[&right].static_definitions.is_empty(), + "each copy must have handed its recipient the donor's copy-layer static, \ + or there is no second generation to reach a fixed point over" + ); + // CR 707.2: each half took its donor's copiable P/T. + assert_eq!( + (state.objects[&right].power, state.objects[&right].toughness), + (Some(4), Some(4)), + "the right half must be a copy of donor P" + ); + assert_eq!( + (state.objects[&left].power, state.objects[&left].toughness), + (Some(5), Some(5)), + "the left half must be a copy of donor Q" + ); + // Generation 2, both directions at once: each half's copy-granted static + // reached its watcher inside this same layer-1 pass. + assert_eq!( + state.objects[&watcher_l].name, "Marked by right", + "the copy-granted static on the RIGHT half must apply inside the same \ + layer-1 pass that applied the copy" + ); + assert_eq!( + state.objects[&watcher_r].name, "Marked by left", + "the copy-granted static on the LEFT half must apply inside the same \ + layer-1 pass that applied the copy" + ); + } + + /// CR 613.2a: `copy_grants_continuous_static` is the WRONG question for + /// CONTINUING sublayer 1a's discovery loop. An anthem makes its recipient a + /// `StaticSourceIndex` generator, which layers 2-7 must see, but it can never + /// appear in a `Layer::Copy` gather — so answering the broad question at the + /// continuation site charged every board holding a cloned lord for an extra + /// index rebuild plus an extra board-wide + /// `collect_shared_active_continuous_effects`, inside the module whose stated + /// job is keeping per-flush work off `|battlefield|`. + /// + /// Three boards differing ONLY in the copy payload's static set, measured in + /// board-wide gathers. + /// + /// REVERT-PROBE (discriminating, run): put `copy_grants_continuous_static` + /// back at the loop-continuation site ⇒ the anthem board costs one gather more + /// than the vanilla board and the first assertion fails. + #[test] + fn only_a_copy_layer_payload_buys_a_layer_one_discovery_generation() { + /// Returns (board-wide gathers during the pass, statics landed on the + /// recipient). + fn probe(payload_static: Option) -> (usize, usize) { + let mut state = setup(); + let player = PlayerId(0); + let template = make_creature(&mut state, "Template", 2, 2, player); + let recipient = make_creature(&mut state, "Recipient", 1, 1, player); + let caster = make_creature(&mut state, "Caster", 1, 1, player); + let mut values = intrinsic_copiable_values(&state.objects[&template]); + assert!( + values.static_definitions.is_empty(), + "the template must be vanilla so the payload static is the only variable" + ); + if let Some(def) = payload_static { + Arc::make_mut(&mut values.static_definitions).push(def); + } + state.add_transient_continuous_effect( + caster, + player, + Duration::Permanent, + TargetFilter::SpecificObject { id: recipient }, + vec![ContinuousModification::CopyValues { + values: Box::new(values), + display_source: crate::game::game_object::DisplaySource::Card, + printed_ref: None, + token_image_ref: None, + }], + None, + ); + reset_active_effect_collection_count(); + evaluate_layers(&mut state); + ( + active_effect_collection_count(), + state.objects[&recipient].static_definitions.len(), + ) + } + + let board_wide = + || TargetFilter::Typed(TypedFilter::creature().controller(ControllerRef::You)); + let (vanilla_gathers, vanilla_statics) = probe(None); + let (anthem_gathers, anthem_statics) = probe(Some( + StaticDefinition::continuous() + .affected(board_wide()) + .modifications(vec![ContinuousModification::AddPower { value: 1 }]), + )); + // Same shape, but a `Layer::Copy` modification — the only kind that can + // show up in the next gather. + let (copy_layer_gathers, copy_layer_statics) = probe(Some( + StaticDefinition::continuous() + .affected(board_wide()) + .modifications(vec![ContinuousModification::SetName { + name: "Renamed".into(), + }]), + )); + + // POSITIVE reach-guards: the payload static really landed on the recipient + // in both static cases, so the gather counts below are not comparing two + // boards where the copy did nothing. + assert_eq!( + vanilla_statics, 0, + "the vanilla control must land no static" + ); + assert_eq!( + anthem_statics, 1, + "the anthem payload must reach the recipient" + ); + assert_eq!( + copy_layer_statics, 1, + "the copy-layer payload must reach the recipient" + ); + + assert_eq!( + anthem_gathers, vanilla_gathers, + "cloning an ordinary lord must buy no layer-1 discovery generation: an \ + anthem is a layer-7c effect and cannot appear in a `Layer::Copy` gather" + ); + assert_eq!( + copy_layer_gathers, + vanilla_gathers + 1, + "a copy-LAYER payload static must buy exactly one discovery generation" + ); + } + /// Shared reach-guard: run ONLY the guard from a Clean baseline and assert it /// forces a full escalation (the `mark_layers_full` the guarded site would /// otherwise call unconditionally). diff --git a/crates/engine/src/game/static_source_index.rs b/crates/engine/src/game/static_source_index.rs index 95e9ab6094..38ceea223b 100644 --- a/crates/engine/src/game/static_source_index.rs +++ b/crates/engine/src/game/static_source_index.rs @@ -64,9 +64,17 @@ use super::game_object::GameObject; /// That is the soundness foundation — the index over-includes (condition-failing /// statics) but never under-includes a real source. pub(crate) fn object_sources_continuous_effect(obj: &GameObject) -> bool { - obj.static_definitions - .iter_all() - .any(|def| def.mode == StaticMode::Continuous) + defs_source_continuous_effect(obj.static_definitions.as_slice()) +} + +/// The predicate above, over a bare definition slice. Layer 1 has to ask the same +/// question of a set of definitions that is not (yet) on any object — the copiable +/// static set inside a `CopyValues` payload — to decide whether applying that copy +/// turns its recipient into a generator mid-pass (`layers.rs`, +/// `copy_grants_continuous_static`). Sharing one body makes the two answers +/// impossible to drift apart. +pub(crate) fn defs_source_continuous_effect(defs: &[crate::types::StaticDefinition]) -> bool { + defs.iter().any(|def| def.mode == StaticMode::Continuous) } impl StaticSourceIndex { diff --git a/crates/engine/src/types/layers.rs b/crates/engine/src/types/layers.rs index 702fec6fc8..57259f7ad8 100644 --- a/crates/engine/src/types/layers.rs +++ b/crates/engine/src/types/layers.rs @@ -77,6 +77,29 @@ impl Layer { } impl ContinuousModification { + /// CR 613.2a: whether this modification is applied in layer 1 (copy). + /// + /// Panic-free companion to [`Self::layer`]. Six of `layer`'s arms are + /// `unreachable!()` — `AddCounterOnEnter`, `SetStartingLoyalty`, + /// `RemoveManaCost` and the three combat-assignment variants are consumed at + /// copy resolution and never layered — so asking an arbitrary modification + /// (one read out of a `CopyValues` payload, say) for its layer can abort. + /// This answers the only layer question sublayer 1a needs without that + /// hazard. The variant list is exactly `layer`'s `Layer::Copy` set; + /// `copy_grants_continuous_static_covers_every_copy_layer_variant` + /// (`game/layers.rs`) pins the two together. + pub fn is_copy_layer(&self) -> bool { + matches!( + self, + ContinuousModification::CopyValues { .. } + | ContinuousModification::CopyChosen + | ContinuousModification::SetName { .. } + | ContinuousModification::RetainPrintedTriggerFromSource { .. } + | ContinuousModification::RetainPrintedAbilityFromSource { .. } + | ContinuousModification::RetainAllOtherAbilitiesFromSource + ) + } + /// Returns the appropriate Layer for this modification type. pub fn layer(&self) -> Layer { match self { diff --git a/crates/engine/tests/integration/vizier_of_many_faces_embalm_copy_panic_5278.rs b/crates/engine/tests/integration/vizier_of_many_faces_embalm_copy_panic_5278.rs index fe55e047c5..9b6842a064 100644 --- a/crates/engine/tests/integration/vizier_of_many_faces_embalm_copy_panic_5278.rs +++ b/crates/engine/tests/integration/vizier_of_many_faces_embalm_copy_panic_5278.rs @@ -30,7 +30,10 @@ //! `Duration::Permanent` transient), the accept case panics again. use engine::game::scenario::{GameRunner, GameScenario, P0}; -use engine::types::ability::{ContinuousModification, Effect, TargetRef}; +use engine::types::ability::{ + ContinuousModification, Effect, FilterProp, StaticDefinition, TargetFilter, TargetRef, + TypeFilter, TypedFilter, +}; use engine::types::actions::GameAction; use engine::types::game_state::WaitingFor; use engine::types::mana::{ManaColor, ManaType, ManaUnit}; @@ -118,6 +121,71 @@ fn build_scenario() -> ( (runner, vizier, bear) } +/// A lord whose ONLY ability is a board-wide static. Copying it is what hands +/// the Embalm token a static ability that its copiable base — a 0/0 Clone with +/// no static of its own — does not carry. +const LORD_ORACLE: &str = "Other creatures you control get +1/+1."; + +/// Same shape as [`build_scenario`], except the creature the token will copy is +/// a lord and a third, vanilla creature is present to receive the anthem. +/// Returns `(runner, vizier, lord, vanilla)`. +fn build_lord_scenario() -> ( + GameRunner, + engine::types::identifiers::ObjectId, + engine::types::identifiers::ObjectId, + engine::types::identifiers::ObjectId, +) { + build_lord_scenario_with(None) +} + +/// [`build_lord_scenario`], plus an extra static definition on the lord's PRINTED +/// base — so it is part of the lord's copiable values (CR 707.2) and rides along +/// onto the Embalm token when the token copies the lord. +fn build_lord_scenario_with( + extra_lord_static: Option, +) -> ( + GameRunner, + engine::types::identifiers::ObjectId, + engine::types::identifiers::ObjectId, + engine::types::identifiers::ObjectId, +) { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + + let vizier = scenario + .add_creature_to_graveyard(P0, "Vizier of Many Faces", 0, 0) + .with_mana_cost(engine::types::mana::ManaCost::Cost { + generic: 3, + shards: vec![engine::types::mana::ManaCostShard::Blue], + }) + .from_oracle_text_with_keywords(&["Embalm"], VIZIER_ORACLE) + .id(); + + // 1/1 lord and a 2/2 vanilla: the two anthem instances (original + copy) + // are each worth +1/+1, so the vanilla's toughness alone distinguishes + // "one anthem applied" (3/3) from "both applied" (4/4). + let mut lord_builder = scenario.add_creature(P0, "Bear Umbra Lord", 1, 1); + lord_builder.from_oracle_text(LORD_ORACLE); + if let Some(static_def) = extra_lord_static { + lord_builder.with_static_definition(static_def); + } + let lord = lord_builder.id(); + let vanilla = scenario.add_creature(P0, "Grizzly Bears", 2, 2).id(); + + let mut runner = scenario.build(); + add_mana( + &mut runner, + &[ + ManaType::Blue, + ManaType::Blue, + ManaType::Colorless, + ManaType::Colorless, + ManaType::Colorless, + ], + ); + (runner, vizier, lord, vanilla) +} + /// Activate the Embalm ability and drain any mana-payment prompt so the token /// is created and parked on its first entry-choice prompt. fn activate_embalm(runner: &mut GameRunner, vizier: engine::types::identifiers::ObjectId) { @@ -149,6 +217,61 @@ fn activate_embalm(runner: &mut GameRunner, vizier: engine::types::identifiers:: panic!("Embalm activation never reached an entry choice"); } +/// Drive a token parked on its entry choices all the way onto the battlefield as +/// a copy of `copy_target`: accept the enter-as-copy replacement, name the target, +/// then let the stack settle. Returns the token's `ObjectId`. +fn resolve_embalm_copy_of( + runner: &mut GameRunner, + copy_target: engine::types::identifiers::ObjectId, +) -> engine::types::identifiers::ObjectId { + // Bounded, not `loop`: the `Priority` arm acts on every trip, so an engine + // change that stops surfacing `CopyTargetChoice` would spin here forever and + // hang CI instead of failing. Same bound as `activate_embalm`. + let mut chosen = None; + for _ in 0..64 { + match runner.state().waiting_for.clone() { + WaitingFor::ReplacementChoice { .. } => { + runner + .act(GameAction::ChooseReplacement { index: 0 }) + .expect("accept enter-as-copy replacement"); + } + WaitingFor::CopyTargetChoice { + source_id, + valid_targets, + .. + } => { + assert!( + valid_targets.contains(©_target), + "the intended copy target must be legal, got {valid_targets:?}" + ); + runner + .act(GameAction::ChooseTarget { + target: Some(TargetRef::Object(copy_target)), + }) + .expect("choose copy target"); + chosen = Some(source_id); + break; + } + WaitingFor::Priority { .. } => { + runner.act(GameAction::PassPriority).expect("pass priority"); + } + other => panic!("unexpected waiting_for during entry: {other:?}"), + } + } + let token = chosen.expect("the Embalm token never reached its copy-target choice"); + + for _ in 0..16 { + match runner.state().waiting_for.clone() { + WaitingFor::Priority { .. } if runner.state().stack.is_empty() => break, + WaitingFor::Priority { .. } => { + runner.act(GameAction::PassPriority).expect("pass priority"); + } + _ => break, + } + } + token +} + #[test] fn embalm_copy_of_bear_survives_and_carries_stamped_exceptions() { let (mut runner, vizier, _bear) = build_scenario(); @@ -156,7 +279,11 @@ fn embalm_copy_of_bear_survives_and_carries_stamped_exceptions() { // Accept the enter-as-copy replacement, then pick the Bear to copy. This is // the exact sequence that panicked before the fix. - let token = 'drive: loop { + // Bounded, not `loop`, for the same reason as `resolve_embalm_copy_of`: the + // `Priority` arm acts on every trip, so an engine change that stops surfacing + // `CopyTargetChoice` would spin here forever and hang CI instead of failing. + let mut chosen = None; + for _ in 0..64 { match runner.state().waiting_for.clone() { WaitingFor::ReplacementChoice { .. } => { runner @@ -176,14 +303,16 @@ fn embalm_copy_of_bear_survives_and_carries_stamped_exceptions() { target: Some(TargetRef::Object(target)), }) .expect("choose copy target (Bear)"); - break 'drive source_id; + chosen = Some(source_id); + break; } WaitingFor::Priority { .. } => { runner.act(GameAction::PassPriority).expect("pass priority"); } other => panic!("unexpected waiting_for during entry: {other:?}"), } - }; + } + let token = chosen.expect("the Embalm token never reached its copy-target choice"); // Drain any residual priority so SBAs run. for _ in 0..16 { @@ -271,7 +400,9 @@ fn embalm_copy_declined_enters_as_zero_zero_and_dies() { // Decline the enter-as-copy replacement (index 1 = decline on an optional // replacement). The token stays a copy of Vizier (0/0) with the Embalm // exceptions already stamped at creation, then dies to CR 704.5f. - let token = loop { + // Bounded for the same CI-hang reason as the drive loop above. + let mut declined = None; + for _ in 0..64 { match runner.state().waiting_for.clone() { WaitingFor::ReplacementChoice { candidates, .. } => { // Positive reach-guard: the enter-as-copy replacement really @@ -292,14 +423,16 @@ fn embalm_copy_declined_enters_as_zero_zero_and_dies() { runner .act(GameAction::ChooseReplacement { index: 1 }) .expect("decline enter-as-copy replacement"); - break entering; + declined = Some(entering); + break; } WaitingFor::Priority { .. } => { runner.act(GameAction::PassPriority).expect("pass priority"); } other => panic!("unexpected waiting_for before decline: {other:?}"), } - }; + } + let token = declined.expect("the Embalm token never reached its enter-as-copy choice"); // Drain to let SBAs run. for _ in 0..16 { @@ -325,3 +458,168 @@ fn embalm_copy_declined_enters_as_zero_zero_and_dies() { "a declined 0/0 Embalm copy of Vizier must die to SBA, not remain on the battlefield" ); } + +/// CR 613.2a + CR 613.2c: layer 1a applies copy effects, and after all of layer 1 +/// has been applied the object's characteristics ARE its copiable values. A static +/// ability a permanent has ONLY because of a copy effect is therefore part of the +/// board that layers 2–7 of that SAME pass must be derived from — it does not wait +/// for the next evaluation. +/// +/// The generator index that decides which permanents are scanned for statics is +/// built at the top of the pass, from the just-reset BASE definitions. An Embalm +/// token's base is Vizier's (a 0/0 Clone with no static at all), so before the fix +/// the copied anthem was invisible to the very pass that applied the copy, and it +/// stayed invisible on every later pass too — each one resets to that same base +/// first. The engine therefore ran a copy of a lord as a permanent with no ability. +/// +/// REVERT-PROBE (discriminating, RUN): drop `copy_added_generator ||` from the +/// post-layer-1 `StaticSourceIndex::rebuild_from_state` guard in `evaluate_layers` +/// ⇒ the vanilla creature holds at 3/3 (only the ORIGINAL lord's anthem applies) +/// and this test fails on both P/T assertions, while the two tests above stay +/// green (neither copies a permanent that carries a static ability). +#[test] +fn a_copy_granted_static_ability_applies_in_the_pass_that_applied_the_copy() { + let (mut runner, vizier, lord, vanilla) = build_lord_scenario(); + activate_embalm(&mut runner, vizier); + let token = resolve_embalm_copy_of(&mut runner, lord); + + // POSITIVE reach-guards: the copy really happened AND it really is the copy + // that put a static ability on the token, so the P/T assertions below cannot + // pass for the wrong reason (a token that copied nothing has no anthem to + // contribute, and a token whose base already carried the anthem would not + // exercise the layer-1 binding at all). + let token_obj = runner + .state() + .objects + .get(&token) + .expect("the Embalm token must be on the battlefield"); + assert_eq!( + token_obj.name, "Bear Umbra Lord", + "the token must be a copy of the chosen lord" + ); + assert!( + !token_obj.static_definitions.is_empty(), + "the copy must have given the token the lord's static ability" + ); + assert!( + token_obj.base_static_definitions.is_empty(), + "the token's own copiable base must carry no static, or its anthem would not \ + depend on the layer-1 copy effect and this test would stop exercising the \ + binding it is written for" + ); + + // Two anthems now apply to the vanilla creature: the original lord's and the + // token's copied one. Before the fix only the original was gathered. + assert_eq!( + ( + runner.state().objects[&vanilla].power, + runner.state().objects[&vanilla].toughness + ), + (Some(4), Some(4)), + "the vanilla 2/2 must get +1/+1 from BOTH the original lord and its copy" + ); + assert_eq!( + ( + runner.state().objects[&lord].power, + runner.state().objects[&lord].toughness + ), + (Some(2), Some(2)), + "the original 1/1 lord must get +1/+1 from the token's copied anthem \ + (\"other creatures\", so its own anthem does not pump it)" + ); + // The third recipient class, and the only one where the two anthem instances + // must disagree about the SAME object: the token is excluded from the anthem + // it copied (`FilterProp::Another`) and pumped by the original lord's, so a + // bug that swapped which instance excludes which recipient would move this + // total while leaving the two above plausible. + assert_eq!( + ( + runner.state().objects[&token].power, + runner.state().objects[&token].toughness + ), + (Some(2), Some(2)), + "the 1/1 token must get +1/+1 from the ORIGINAL lord's anthem only — its \ + own copied anthem says \"other creatures\" and cannot pump itself" + ); +} + +/// CR 613.2a + CR 613.2c: sublayer 1a must reach a FIXED POINT before layer 2. +/// Applying a copy effect can hand its recipient a static ability that ITSELF +/// generates a layer-1 continuous effect (CR 707.2c), and CR 613.2c says the +/// characteristics standing after layer 1 finishes ARE the object's copiable +/// values — so that second-generation copy effect belongs to the same layer 1, +/// not to the next pass. +/// +/// Board: SYNTHETIC, and deliberately so. No printed card carries a board-wide +/// `SetName` static — on real cards `SetName` shows up as a copy EXCEPTION, +/// paired with `CopyValues` inside `additional_modifications` (Vizier's own +/// Embalm line is one). What is being tested here is the building block, not a +/// card: `SetName` is the only `Layer::Copy` modification with an observable that +/// no later layer can also write, which is what makes the assertion below +/// attribute the change to sublayer 1a and nothing else. The mechanism it stands +/// in for — a copy handing its recipient a static that is itself a layer-1 effect +/// — is real and is what CR 707.2c is about. +/// +/// So: the lord's printed base carries "each OTHER creature is named Faceless +/// Reflection" (`FilterProp::Another` excludes the source from its own effect). +/// The Embalm token copies the lord (CR 707.2), which makes the TOKEN a second +/// source of that same static — and the token's instance, unlike the lord's, +/// does cover the lord. So `lord.name` flips if and only if sublayer 1a ran a +/// second generation: the lord's own instance can never rename the lord, and no +/// later layer applies `SetName` (it is a `Layer::Copy` modification). +/// +/// REVERT-PROBE (discriminating, RUN): cap `apply_copy_sublayer_to_fixed_point` +/// at a single generation (return right after the first apply) ⇒ the vanilla is +/// still renamed by generation 1 (the lord's own instance) but the lord keeps its +/// printed name, and this test fails on the `lord.name` assertion alone. +#[test] +fn a_copy_granted_layer_one_static_applies_inside_the_same_layer_one_pass() { + const RENAMED: &str = "Faceless Reflection"; + let renamer = StaticDefinition::continuous() + .affected(TargetFilter::Typed( + TypedFilter::new(TypeFilter::Creature).properties(vec![FilterProp::Another]), + )) + .modifications(vec![ContinuousModification::SetName { + name: RENAMED.to_string(), + }]); + let (mut runner, vizier, lord, vanilla) = build_lord_scenario_with(Some(renamer)); + activate_embalm(&mut runner, vizier); + let token = resolve_embalm_copy_of(&mut runner, lord); + + // POSITIVE reach-guards: the copy really delivered a LAYER-1 static onto the + // token, and the token's own copiable base carries none — so the rename + // asserted below cannot come from anywhere but the copy effect. + let token_obj = runner + .state() + .objects + .get(&token) + .expect("the Embalm token must be on the battlefield"); + assert!( + token_obj.base_static_definitions.is_empty(), + "the token's own copiable base must carry no static, or the second \ + generation this test probes would not depend on the copy at all" + ); + assert!( + token_obj.static_definitions.as_slice().iter().any(|sd| sd + .modifications + .iter() + .any(|m| matches!(m, ContinuousModification::SetName { .. }))), + "the copy must have handed the token the lord's copy-LAYER static, or the \ + second generation could not exist: {:?}", + token_obj.static_definitions.as_slice() + ); + + // Generation 1 evidence: the lord's own instance renames every OTHER creature. + assert_eq!( + runner.state().objects[&vanilla].name, + RENAMED, + "the lord's own layer-1 static must rename the vanilla creature" + ); + // Generation 2 evidence: only the token's copied instance can rename the lord. + assert_eq!( + runner.state().objects[&lord].name, + RENAMED, + "the copy-granted layer-1 static must be applied inside the SAME layer-1 \ + pass that applied the copy" + ); +}