Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
26 changes: 26 additions & 0 deletions PR_6499.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
Closes #6499

## Summary

Discord report: **Flickering Ward** could not stay attached after choosing a color.

Oracle: `Enchanted creature has protection from the chosen color. This effect doesn't remove this Aura.`

Choosing white (the Aura's color) grants protection from white. Without the CR 702.16n rider, SBA CR 704.5m / CR 702.16c puts the Aura in the graveyard — which matches the report.

## Root cause

Coverage claimed the protection grant was supported, but the parser deliberately dropped `"This effect doesn't remove this Aura"` as inert prose so the keyword would parse. The exemption was never modeled, so runtime treated the grant like ordinary protection.

## Changes

- **`ProtectionDoesNotRemove`** on `StaticDefinition` — `Source` / `Auras` / `ControlledAttachmentsAlreadyAttached` (CR 702.16n / 702.16p class).
- **Parser** — recover the rider onto the continuous static (`parse_protection_does_not_remove`); still strip it from the keyword leg so `Protection(ChosenColor)` parses cleanly. Source matches both `"this Aura"` and post-normalization `~`.
- **Runtime** — `attachment_illegality` walks continuous protection grants and honors per-grant exemptions (other protection instances still apply).
- **Tests** — parse Source exemption; choose white → Aura stays attached; printed protection without rider still blocks.

## Test Plan

- [ ] `cargo fmt --all -- --check`
- [ ] `cargo test -p engine --lib -- protection_chosen_color_drops_trailing_this_aura`
- [ ] `cargo test -p engine --test integration -- flickering_ward`
2 changes: 2 additions & 0 deletions crates/engine/src/ai_support/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4809,6 +4809,7 @@ mod tests {
source_controller: None,
source_object: None,
bypass_beneficiary: None,
protection_does_not_remove: None,
};
obj.static_definitions = vec![def].into();
}
Expand Down Expand Up @@ -4931,6 +4932,7 @@ mod tests {
source_controller: None,
source_object: None,
bypass_beneficiary: None,
protection_does_not_remove: None,
};
obj.static_definitions = vec![def].into();
}
Expand Down
4 changes: 4 additions & 0 deletions crates/engine/src/game/casting_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3467,6 +3467,7 @@ fn granted_freerunning_static_surfaces_freerunning_variant() {
source_controller: None,
source_object: None,
bypass_beneficiary: None,
protection_does_not_remove: None,
};
obj.static_definitions = vec![def].into();
}
Expand Down Expand Up @@ -11960,6 +11961,7 @@ fn x_cost_max_accounts_for_granted_affinity_exceeding_fixed_generic() {
source_controller: None,
source_object: None,
bypass_beneficiary: None,
protection_does_not_remove: None,
}]
.into();
}
Expand Down Expand Up @@ -14737,6 +14739,7 @@ fn witherbloom_grants_affinity_to_instant_and_sorcery_spells() {
source_controller: None,
source_object: None,
bypass_beneficiary: None,
protection_does_not_remove: None,
};
obj.static_definitions = vec![def].into();
}
Expand Down Expand Up @@ -14854,6 +14857,7 @@ fn add_witherbloom_affinity_source(state: &mut GameState, player: PlayerId) -> O
source_controller: None,
source_object: None,
bypass_beneficiary: None,
protection_does_not_remove: None,
}]
.into();
}
Expand Down
12 changes: 12 additions & 0 deletions crates/engine/src/game/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12178,6 +12178,7 @@ mod tests {
source_controller: None,
source_object: None,
bypass_beneficiary: None,
protection_does_not_remove: None,
}],
duration: Some(Duration::UntilEndOfTurn),
target: None,
Expand Down Expand Up @@ -12225,6 +12226,7 @@ mod tests {
source_controller: None,
source_object: None,
bypass_beneficiary: None,
protection_does_not_remove: None,
}],
duration: Some(Duration::UntilEndOfTurn),
target: None,
Expand Down Expand Up @@ -13360,6 +13362,7 @@ mod tests {
source_controller: None,
source_object: None,
bypass_beneficiary: None,
protection_does_not_remove: None,
});

assert!(audit_card_lines(oracle, &face).is_empty());
Expand Down Expand Up @@ -13394,6 +13397,7 @@ mod tests {
source_controller: None,
source_object: None,
bypass_beneficiary: None,
protection_does_not_remove: None,
});

assert!(audit_card_lines(oracle, &face).is_empty());
Expand Down Expand Up @@ -13426,6 +13430,7 @@ mod tests {
source_controller: None,
source_object: None,
bypass_beneficiary: None,
protection_does_not_remove: None,
});

let findings = audit_card_lines(oracle, &face);
Expand Down Expand Up @@ -13576,6 +13581,7 @@ mod tests {
source_controller: None,
source_object: None,
bypass_beneficiary: None,
protection_does_not_remove: None,
});

assert!(
Expand Down Expand Up @@ -13608,6 +13614,7 @@ mod tests {
source_controller: None,
source_object: None,
bypass_beneficiary: None,
protection_does_not_remove: None,
});

assert!(
Expand Down Expand Up @@ -13650,6 +13657,7 @@ mod tests {
source_controller: None,
source_object: None,
bypass_beneficiary: None,
protection_does_not_remove: None,
});

let gaps = card_face_gaps(&face);
Expand Down Expand Up @@ -13683,6 +13691,7 @@ mod tests {
source_controller: None,
source_object: None,
bypass_beneficiary: None,
protection_does_not_remove: None,
});

let gaps = card_face_gaps(&face);
Expand Down Expand Up @@ -13718,6 +13727,7 @@ mod tests {
source_controller: None,
source_object: None,
bypass_beneficiary: None,
protection_does_not_remove: None,
});

let gaps = card_face_gaps(&face);
Expand Down Expand Up @@ -13759,6 +13769,7 @@ mod tests {
source_controller: None,
source_object: None,
bypass_beneficiary: None,
protection_does_not_remove: None,
});
}

Expand Down Expand Up @@ -13927,6 +13938,7 @@ mod tests {
source_controller: None,
source_object: None,
bypass_beneficiary: None,
protection_does_not_remove: None,
});

assert!(
Expand Down
1 change: 1 addition & 0 deletions crates/engine/src/game/dungeon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,7 @@ pub fn room_effects(
source_controller: None,
source_object: None,
bypass_beneficiary: None,
protection_does_not_remove: None,
}],
triggers: Vec::new(),
},
Expand Down
177 changes: 176 additions & 1 deletion crates/engine/src/game/effects/attach.rs
Original file line number Diff line number Diff line change
Expand Up @@ -700,18 +700,193 @@ pub(crate) fn attachment_illegality(
// being attached to the protected permanent.
// CR 702.16d: Protection from a quality prevents Equipment or Fortifications
// of that quality from being attached to the protected permanent.
// CR 702.16n / CR 702.16p: A protection grant that says "this effect doesn't
// remove …" does not make matching attachments illegal via *that* instance
// (Flickering Ward / Ward cycle / Benevolent Blessing). Other instances of
// protection from the same quality still apply normally.
if let (Some(host), Some(attachment)) = (
state.objects.get(&host_id),
state.objects.get(&attachment_id),
) {
if crate::game::keywords::protection_prevents_from(host, attachment) {
if protection_blocks_attachment(state, host_id, attachment_id, host, attachment) {
return Some(AttachIllegality::Protection);
}
}

None
}

/// CR 702.16c/d + CR 702.16n/p: True when some protection instance on `host`
/// matches `attachment` and is not exempted for that attachment.
fn protection_blocks_attachment(
state: &GameState,
host_id: ObjectId,
attachment_id: ObjectId,
host: &crate::game::game_object::GameObject,
attachment: &crate::game::game_object::GameObject,
) -> bool {
use crate::types::ability::ContinuousModification;
use crate::types::keywords::Keyword;
use crate::types::statics::StaticMode;

// CR 702.16: Printed / base protection on the host has no 702.16n rider —
// it always blocks matching attachments.
for kw in &host.base_keywords {
if let Keyword::Protection(ref pt) = kw {
if crate::game::keywords::source_matches_protection_target(pt, host, attachment) {
return true;
}
}
}

// Continuous grants: each matching protection instance blocks unless its
// StaticDefinition/TCE carries a CR 702.16n/p exemption covering this
// attachment.
let mut any_matching_grant = false;
for (source_obj, def) in crate::game::functioning_abilities::battlefield_active_statics(state) {
if !matches!(def.mode, StaticMode::Continuous) {
continue;
}
let source_id = source_obj.id;
let affected = def.affected.clone().unwrap_or(TargetFilter::Any);
let ctx = FilterContext::from_source(state, source_id);
if !matches_target_filter(state, host_id, &affected, &ctx) {
continue;
}
for modification in &def.modifications {
let ContinuousModification::AddKeyword {
keyword: Keyword::Protection(pt),
} = modification
else {
continue;
};
let resolved = resolve_protection_target_for_grant(state, source_id, pt);
let Some(resolved) = resolved else {
continue;
};
if !crate::game::keywords::source_matches_protection_target(&resolved, host, attachment)
{
continue;
}
any_matching_grant = true;
if !protection_grant_exempts_attachment(
state,
host_id,
attachment_id,
source_id,
def.protection_does_not_remove.as_ref(),
) {
return true;
}
}
}

// Transient continuous protection grants (e.g. Mother of Runes) — no
// StaticDefinition rider today; treat as always-blocking when they match.
for tce in &state.transient_continuous_effects {
let ctx = FilterContext::from_source(state, tce.source_id);
if !matches_target_filter(state, host_id, &tce.affected, &ctx) {
continue;
}
for modification in &tce.modifications {
let ContinuousModification::AddKeyword {
keyword: Keyword::Protection(pt),
} = modification
else {
continue;
};
let resolved = resolve_protection_target_for_grant(state, tce.source_id, pt);
let Some(resolved) = resolved else {
continue;
};
if crate::game::keywords::source_matches_protection_target(&resolved, host, attachment)
{
// Transients currently carry no 702.16n rider field.
return true;
}
}
}

// If host.keywords still match (granted protection present) but we found no
// continuous grant — fall back to the pre-exemption query so we never open
// a hole when grant discovery misses a path.
if !any_matching_grant && crate::game::keywords::protection_prevents_from(host, attachment) {
return true;
}

false
}

/// CR 702.16 + CR 105.4: Resolve `ChosenColor` / `ChosenCardType` against the
/// granting source before matching the attachment (mirrors layer bake-in).
fn resolve_protection_target_for_grant(
state: &GameState,
source_id: ObjectId,
pt: &crate::types::keywords::ProtectionTarget,
) -> Option<crate::types::keywords::ProtectionTarget> {
use crate::types::keywords::ProtectionTarget;
match pt {
ProtectionTarget::ChosenColor => state
.objects
.get(&source_id)
.and_then(|src| src.chosen_color())
.map(ProtectionTarget::Color),
ProtectionTarget::ChosenCardType => state
.objects
.get(&source_id)
.and_then(|src| src.chosen_card_type())
.and_then(|ct| ct.protection_quality_str())
.map(|quality| ProtectionTarget::CardType(quality.to_string())),
other => Some(other.clone()),
}
}

/// CR 702.16n / CR 702.16p: Does this protection grant's exemption rider cover
/// `attachment_id` on `host_id`?
fn protection_grant_exempts_attachment(
state: &GameState,
host_id: ObjectId,
attachment_id: ObjectId,
grant_source_id: ObjectId,
exemption: Option<&crate::types::ability::ProtectionDoesNotRemove>,
) -> bool {
use crate::types::ability::ProtectionDoesNotRemove;

let Some(exemption) = exemption else {
return false;
};
let Some(attachment) = state.objects.get(&attachment_id) else {
return false;
};
match exemption {
// CR 702.16n: "this effect doesn't remove this Aura"
ProtectionDoesNotRemove::Source => attachment_id == grant_source_id,
// CR 702.16n: "this effect doesn't remove Auras"
ProtectionDoesNotRemove::Auras => attachment
.card_types
.subtypes
.iter()
.any(|s| s.eq_ignore_ascii_case("Aura")),
// CR 702.16p: already-attached Auras/Equipment you control
ProtectionDoesNotRemove::ControlledAttachmentsAlreadyAttached => {
let Some(source) = state.objects.get(&grant_source_id) else {
return false;
};
let is_aura_or_equipment = attachment
.card_types
.subtypes
.iter()
.any(|s| s.eq_ignore_ascii_case("Aura") || s.eq_ignore_ascii_case("Equipment"));
let controlled_by_source_controller = attachment.controller == source.controller;
let already_on_host = matches!(
attachment.attached_to,
Some(AttachTarget::Object(h)) if h == host_id
);
is_aura_or_equipment && controlled_by_source_controller && already_on_host
}
}
}

/// CR 301.5 + CR 303.4 + CR 701.3a: True unless `host_id` is forbidden by a
/// positive "can be attached only to {filter}" restriction on `attachment_id`.
///
Expand Down
1 change: 1 addition & 0 deletions crates/engine/src/game/effects/create_emblem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ mod tests {
source_controller: None,
source_object: None,
bypass_beneficiary: None,
protection_does_not_remove: None,
}
}

Expand Down
1 change: 1 addition & 0 deletions crates/engine/src/parser/oracle_modal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1234,6 +1234,7 @@ fn lower_as_enters_anchor_word_modal(
source_controller: None,
source_object: None,
bypass_beneficiary: None,
protection_does_not_remove: None,
};
result.statics.push(placeholder);
}
Expand Down
Loading
Loading