Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion client/src/adapter/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,10 @@ export interface MeldSelection {
// engine surfaces on the declare-attackers/blockers waiting payloads for
// display-only badges + Confirm gating. `#[serde(tag = "kind")]` in the engine.
export type CombatRequirement =
| { kind: "MustAttack"; players: PlayerId[]; sources?: ObjectId[] }
// CR 506.3: `defenders` spans the whole defender category — players,
// planeswalkers, and battles — so a planeswalker-directed lure (Gideon Jura's
// "+2") surfaces the same way a player-directed one does.
| { kind: "MustAttack"; defenders: AttackTarget[]; sources?: ObjectId[] }
| { kind: "MustBlock"; sources?: ObjectId[]; attackers?: ObjectId[] }
| { kind: "CantAttack"; sources?: ObjectId[] }
| { kind: "CantBlock"; sources?: ObjectId[] };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ describe("ActionButton", () => {
valid_attacker_ids: [100],
valid_attack_targets: [target],
valid_attack_targets_by_attacker: { "100": [target] },
attacker_constraints: { "100": { kind: "MustAttack", players: [] } },
attacker_constraints: { "100": { kind: "MustAttack", defenders: [] } },
},
};
useGameStore.setState({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe("useAttackRequirements", () => {
data: {
player: 0,
valid_attacker_ids: [100],
attacker_constraints: { "100": { kind: "MustAttack", players: [] } },
attacker_constraints: { "100": { kind: "MustAttack", defenders: [] } },
},
});

Expand Down Expand Up @@ -85,7 +85,7 @@ describe("useAttackRequirements", () => {
data: {
player: 0,
valid_attacker_ids: [100],
attacker_constraints: { "100": { kind: "MustAttack", players: [], sources: [200] } },
attacker_constraints: { "100": { kind: "MustAttack", defenders: [], sources: [200] } },
},
});
const r = renderHook(() => useAttackRequirements());
Expand Down Expand Up @@ -221,7 +221,7 @@ describe("AttackRequirementBadges source attribution (display-only)", () => {
player: 0,
valid_attacker_ids: [objectId],
attacker_constraints: {
[String(objectId)]: { kind: "MustAttack", players: [], sources },
[String(objectId)]: { kind: "MustAttack", defenders: [], sources },
},
},
});
Expand Down
8 changes: 4 additions & 4 deletions crates/engine/src/ai_support/candidates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6327,7 +6327,7 @@ mod tests {
}

/// Regression (multi-requirement residual): a creature directed by
/// `MustAttackPlayer` (CR 508.1b) alongside a goaded creature (CR 701.15b)
/// `MustAttackDefender` (CR 508.1b) alongside a goaded creature (CR 701.15b)
/// also forces a mixed-target declaration. The greedy forced-legal candidate
/// must steer the directed creature onto its *required* player regardless of
/// `valid_attack_targets` ordering. A goad-only target pick would land the
Expand Down Expand Up @@ -6364,8 +6364,8 @@ mod tests {
.unwrap()
.static_definitions
.push(StaticDefinition::new(
crate::types::statics::StaticMode::MustAttackPlayer {
player: PlayerId(1).into(),
crate::types::statics::StaticMode::MustAttackDefender {
defender: PlayerId(1).into(),
},
));
let goaded = make_creature(&mut state, 2);
Expand Down Expand Up @@ -6398,7 +6398,7 @@ mod tests {
});
assert!(
has_legal,
"forced assignment must direct the MustAttackPlayer creature to its required player"
"forced assignment must direct the MustAttackDefender creature to its required player"
);
}

Expand Down
2 changes: 1 addition & 1 deletion crates/engine/src/ai_support/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1218,7 +1218,7 @@ impl LegalityPoisonGates {
StaticMode::CantAttack
| StaticMode::CantAttackOrBlock
| StaticMode::MustAttack
| StaticMode::MustAttackPlayer { .. }
| StaticMode::MustAttackDefender { .. }
| StaticMode::Goaded
| StaticMode::MustAttackAwayFromSource
| StaticMode::CanAttackWithDefender
Expand Down
68 changes: 67 additions & 1 deletion crates/engine/src/analysis/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4059,6 +4059,28 @@ fn static_mode_references_growing_class(mode: &crate::types::statics::StaticMode
// analogue of `modification_grants_growing_cost_keyword`).
StaticMode::CastWithKeyword { keyword } => kw_reads(keyword),

// CR 508.1d + CR 604.1: the required defender splits by whether it is a
// resolution-time SNAPSHOT or a LIVE class.
//
// `Fixed`/`Permanent` are frozen ids (a `PlayerId`, an
// `ObjectIncarnationRef`) — nothing is re-derived from the board, so they
// are genuinely read-free.
//
// `Matching` is not. `combat::must_attack_defender_directives_for_creature`
// re-evaluates its `PlayerFilter` against live player state at EVERY
// declare-attackers step (Galactus's "opponent with the most life among
// your opponents"), so the class it names can change as the board does and
// a cached analysis result can go stale. Fail closed on ANY filter rather
// than enumerating `PlayerFilter`'s variants — the same doctrine the
// `activator_filter` site above states at length, and for the same reason:
// enumerating here would silently assert something about every future
// variant.
StaticMode::MustAttackDefender { defender } => match defender {
crate::types::statics::RequiredDefender::Fixed { .. }
| crate::types::statics::RequiredDefender::Permanent { .. } => false,
crate::types::statics::RequiredDefender::Matching { .. } => true,
},

// Non-cost (or fixed-cost) variants — read-free, listed exhaustively (NO `_`).
// `ReduceActionCost`/`DefilerCostReduction` carry only a fixed generic
// reduction; `CantPayCost` is a payment PROHIBITION, not a payable cost; the
Expand Down Expand Up @@ -4090,7 +4112,6 @@ fn static_mode_references_growing_class(mode: &crate::types::statics::StaticMode
| StaticMode::CantLoseLife
| StaticMode::PlayerProtection(..)
| StaticMode::MustAttack
| StaticMode::MustAttackPlayer { .. }
| StaticMode::MustBlock
| StaticMode::MustBlockAttacker { .. }
| StaticMode::CantDraw { .. }
Expand Down Expand Up @@ -5366,6 +5387,51 @@ fn ability_has_per_game_activation_gate(state: &GameState, key: &(ObjectId, usiz

#[cfg(test)]
mod tests {

/// CR 508.1d + CR 604.1: only a LIVE required-defender class is a growing-class
/// read. `Fixed`/`Permanent` are frozen ids and stay read-free; `Matching`
/// carries a `PlayerFilter` that
/// `combat::must_attack_defender_directives_for_creature` re-evaluates against
/// live player state at every declare-attackers step (Galactus), so a cached
/// analysis result can go stale and the scan must fail closed.
///
/// The two halves are paired deliberately: the `false` arms are what make the
/// `true` arm meaningful, since a blanket `=> true` would also pass it.
#[test]
fn must_attack_defender_reads_growing_class_only_for_a_live_class() {
use crate::types::ability::PlayerFilter;
use crate::types::identifiers::{ObjectId, ObjectIncarnationRef};
use crate::types::player::PlayerId;
use crate::types::statics::{RequiredDefender, StaticMode};

// Frozen snapshots — nothing is re-derived from the board.
assert!(
!static_mode_references_growing_class(&StaticMode::MustAttackDefender {
defender: RequiredDefender::Fixed {
player: PlayerId(1)
},
}),
"a snapshotted player id reads nothing"
);
assert!(
!static_mode_references_growing_class(&StaticMode::MustAttackDefender {
defender: RequiredDefender::Permanent {
permanent: ObjectIncarnationRef::of(ObjectId(7), 1),
},
}),
"a snapshotted permanent pin reads nothing"
);

// A live class — re-evaluated against player state, so fail closed.
assert!(
static_mode_references_growing_class(&StaticMode::MustAttackDefender {
defender: RequiredDefender::Matching {
filter: PlayerFilter::Opponent,
},
}),
"a live defender CLASS is re-evaluated against the board and must fail closed"
);
}
use super::*;
use crate::game::game_object::GameObject;
use crate::types::ability::TriggerDefinitionRef;
Expand Down
16 changes: 8 additions & 8 deletions crates/engine/src/database/encore_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ fn encore_activation_creates_attacking_haste_copy_per_opponent() {
);

// CR 702.141a + CR 508.1d: token must attack the opponent (PlayerId(1)) this
// turn — a transient MustAttackPlayer requirement bound to the token.
// turn — a transient MustAttackDefender requirement bound to the token.
let must_attack = state
.transient_continuous_effects
.iter()
Expand All @@ -207,8 +207,8 @@ fn encore_activation_creates_attacking_haste_copy_per_opponent() {
must_attack.modifications.iter().any(|m| matches!(
m,
ContinuousModification::AddStaticMode {
mode: StaticMode::MustAttackPlayer {
player: RequiredDefender::Fixed { player },
mode: StaticMode::MustAttackDefender {
defender: RequiredDefender::Fixed { player },
},
} if *player == PlayerId(1)
)),
Expand Down Expand Up @@ -241,7 +241,7 @@ fn dt_targets_contain(dt: &crate::types::game_state::DelayedTrigger, id: ObjectI

/// CR 702.141a end-to-end (THREE players — the per-opponent case the dedicated
/// resolver exists for): activating Encore creates one haste-bearing copy token
/// PER opponent, each bound via `MustAttackPlayer` to the *distinct* opponent it
/// PER opponent, each bound via `MustAttackDefender` to the *distinct* opponent it
/// was created for, all collected into one next-end-step sacrifice — and that
/// sacrifice, when the end step arrives, actually removes both tokens.
#[test]
Expand Down Expand Up @@ -289,7 +289,7 @@ fn encore_three_player_one_token_per_opponent_then_sacrificed_at_end_step() {
);

// Each token is a haste-bearing copy bound to a DISTINCT opponent's
// `MustAttackPlayer` requirement (the whole reason the dedicated resolver
// `MustAttackDefender` requirement (the whole reason the dedicated resolver
// exists — generic ForceAttack can't bind "that opponent").
let mut bound_opponents = BTreeSet::new();
for &token in &tokens {
Expand All @@ -308,14 +308,14 @@ fn encore_three_player_one_token_per_opponent_then_sacrificed_at_end_step() {
ce.modifications.iter().find_map(|m| match m {
ContinuousModification::AddStaticMode {
mode:
StaticMode::MustAttackPlayer {
player: RequiredDefender::Fixed { player },
StaticMode::MustAttackDefender {
defender: RequiredDefender::Fixed { player },
},
} => Some(*player),
_ => None,
})
})
.expect("token must carry a MustAttackPlayer requirement");
.expect("token must carry a MustAttackDefender requirement");
bound_opponents.insert(player);
}
let expected: BTreeSet<PlayerId> = [PlayerId(1), PlayerId(2)].into_iter().collect();
Expand Down
24 changes: 21 additions & 3 deletions crates/engine/src/game/ability_rw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2219,6 +2219,9 @@ fn legacy_controller_ref(x: &ControllerRef) -> bool {
// CR 102.1: the active player is a game-defined role read live, not a
// frozen event-context tag.
| ControllerRef::ActivePlayer
// CR 109.4 + CR 611.2: a frozen player id is a plain literal read, not
// an event-context tag.
| ControllerRef::SpecificPlayer { .. }
| ControllerRef::EnchantedPlayer => false,
}
}
Expand Down Expand Up @@ -2570,6 +2573,9 @@ fn member_bound_controller_ref(x: &ControllerRef) -> bool {
// CR 102.1: the active player is a game-defined role read live from
// `state.active_player`, not per-source member-bound storage.
| ControllerRef::ActivePlayer
// CR 109.4 + CR 611.2: a frozen player id is a plain literal read, not
// an event-context tag.
| ControllerRef::SpecificPlayer { .. }
| ControllerRef::DefendingPlayer => false,
}
}
Expand Down Expand Up @@ -3261,11 +3267,14 @@ fn legacy_effect(x: &Effect) -> bool {
}
Effect::ForceAttack {
target,
required_player,
required_defender,
duration,
// A static single-vs-mass discriminant (CR 115.1), never a legacy
// target/duration seam of its own.
scope: _,
} => {
legacy_target_filter(target)
|| legacy_target_filter(required_player)
|| legacy_target_filter(required_defender)
|| legacy_duration(duration)
}

Expand Down Expand Up @@ -5479,8 +5488,11 @@ fn rw_effect(
| Effect::SolveCase => (ext_write(StateKind::Other), None),
Effect::ForceAttack {
target,
required_player: _,
required_defender: _,
duration,
// A static single-vs-mass discriminant (CR 115.1): reads nothing and
// writes nothing, so it contributes no read/write profile.
scope: _,
} => {
let mut p = ext_write(StateKind::Other);
flag_legacy_write_target(&mut p, target);
Expand Down Expand Up @@ -6620,6 +6632,9 @@ fn rw_player_scope(x: &PlayerScope) -> RwProfile {
| PlayerScope::Opponent { .. }
| PlayerScope::RecipientController
| PlayerScope::AnyTurn
// CR 611.2 + CR 514.2: duration-timing-only, like `AnyTurn` — never reached
// from a value/quantity/player-selection position.
| PlayerScope::SpecificPlayer { .. }
| PlayerScope::DefendingPlayer => RwProfile::empty(),
}
}
Expand All @@ -6645,6 +6660,9 @@ fn rw_controller_ref(x: &ControllerRef) -> RwProfile {
// CR 102.1: a live read of `state.active_player` — no sibling-mutable
// state, empty RW profile (mirrors `DefendingPlayer`).
| ControllerRef::ActivePlayer
// CR 109.4 + CR 611.2: a frozen player id is a plain literal read, not
// an event-context tag.
| ControllerRef::SpecificPlayer { .. }
// resolution-local (ResolvedAbility.chosen_players)
| ControllerRef::ChosenPlayer { .. } => RwProfile::empty(),
}
Expand Down
14 changes: 12 additions & 2 deletions crates/engine/src/game/ability_scan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1203,12 +1203,16 @@ fn scan_effect(x: &Effect, mode: ScanMode) -> Axes {
}
Effect::ForceAttack {
target,
required_player,
required_defender,
duration,
// A static single-vs-mass discriminant (CR 115.1) — no event, sibling,
// or projected-resource axis; the filters it selects between are
// classified below.
scope: _,
} => {
let mut acc = Axes::NONE;
acc = acc.or(scan_target_filter(target, target_ctx, mode));
acc = acc.or(scan_target_filter(required_player, target_ctx, mode));
acc = acc.or(scan_target_filter(required_defender, target_ctx, mode));
acc = acc.or(scan_duration(duration, mode));
acc
}
Expand Down Expand Up @@ -4204,6 +4208,9 @@ fn scan_player_scope(x: &PlayerScope) -> Axes {
// CR 513.1: turn-agnostic end-step deadline reached via the
// `UntilNextStepOf` duration walk — a pure timing referent, no axes.
PlayerScope::AnyTurn => Axes::NONE,
// CR 611.2: a frozen literal id — reads no event, sibling, or projected
// resource.
PlayerScope::SpecificPlayer { .. } => Axes::NONE,
}
}

Expand Down Expand Up @@ -4238,6 +4245,9 @@ fn scan_controller_ref(x: &ControllerRef) -> Axes {
ControllerRef::EnchantedPlayer => Axes::NONE,
// CR 102.1: a live read of `state.active_player` — no event/sibling axis.
ControllerRef::ActivePlayer => Axes::NONE,
// CR 109.4 + CR 611.2: a frozen literal id — reads no event, sibling, or
// projected resource.
ControllerRef::SpecificPlayer { .. } => Axes::NONE,
}
}

Expand Down
13 changes: 12 additions & 1 deletion crates/engine/src/game/ability_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3437,7 +3437,16 @@ fn mass_all_target_filter(effect: &Effect) -> Option<&TargetFilter> {
target,
..
}
| Effect::DoublePTAll { target, .. } => Some(target),
| Effect::DoublePTAll { target, .. }
// CR 508.1d + CR 109.4: the mass forced-attack population (Gideon Jura's
// "creatures that player controls"). Listed here — not just excluded from
// `target_filter()` — so its `ControllerRef::TargetOpponent` still
// surfaces the COMPANION PLAYER slot the ability genuinely targets.
| Effect::ForceAttack {
scope: EffectScope::All,
target,
..
} => Some(target),
_ => None,
}
}
Expand Down Expand Up @@ -3985,6 +3994,8 @@ pub(crate) fn collect_player_targets(
// CR 102.1 + CR 109.4: the active player, resolvable directly
// (unlike the fail-closed DefendingPlayer arm above).
Some(ControllerRef::ActivePlayer) => p.id == state.active_player,
// CR 109.4 + CR 611.2: a snapshotted id, resolvable directly.
Some(ControllerRef::SpecificPlayer { id }) => p.id == *id,
None => true,
})
.map(|p| p.id)
Expand Down
Loading
Loading