Skip to content

Add Talruum Piper#5131

Open
Spaceint wants to merge 6 commits into
phase-rs:mainfrom
Spaceint:card/talruum-piper
Open

Add Talruum Piper#5131
Spaceint wants to merge 6 commits into
phase-rs:mainfrom
Spaceint:card/talruum-piper

Conversation

@Spaceint

@Spaceint Spaceint commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds engine support for Talruum Piper ("All creatures with flying able to block this creature do so.").

Built for the class, not the single card: parameterizes the existing StaticMode::MustBeBlockedByAll unit variant into MustBeBlockedByAll { blockers: Option<TargetFilter> } — the filtered mass-forced-block (filtered Lure) class. None preserves the existing unfiltered Lure behavior byte-for-byte (Lure, Ochran Assassin, Prized Unicorn, Breaker of Armies); Some(filter) forces only creatures matching the filter to block. This mirrors the already-shipped sibling StaticMode::MustBeBlocked { by: Option<TargetFilter> } (the one-blocker requirement) exactly.

Cards unlocked by the class primitive:

  • Talruum PiperSome(creatures with flying) (permanent static)
  • Marble PriestSome(Walls) (permanent static)
  • You Look Upon the Tarrasque (mode 2) — Some(creatures your opponents control) (one-shot modal effect)

Both parser seams (one-shot effect + permanent static) share a single combinator-based two-slot grammar helper. Runtime enforcement gates the CR 509.1c "must block if able" requirement by the blocker filter, live-evaluated per candidate blocker.

Files changed

  • crates/engine/src/types/statics.rs
  • crates/engine/src/game/combat.rs
  • crates/engine/src/game/coverage.rs
  • crates/engine/src/game/static_abilities.rs
  • crates/engine/src/parser/oracle_effect/mod.rs
  • crates/engine/src/parser/oracle_effect/tests.rs
  • crates/engine/src/parser/oracle_static/evasion.rs
  • crates/engine/src/parser/oracle_static/mod.rs
  • crates/engine/src/parser/oracle_tests.rs
  • crates/engine/src/ai_support/filter.rs
  • crates/engine/tests/squirrel_perf_probe.rs

CR references

  • CR 509.1c — blocking requirements ("the defending player checks each creature they control" against must-block requirements). The blocker filter narrows which of the defending player's creatures carry the requirement; it does not change the requirement-maximization rule.

Track

Non-developer

This box (Windows) cannot generate client/public/card-data.json — the per-set MTGJSON fetches fail under Git Bash (curl: (3) URL rejected: Malformed input to a URL function), so cargo coverage, cargo semantic-audit, and the card-data integration suite are unavailable locally and are deferred to CI. To compensate, the full cargo test -p engine suite and the parser-combinator gate were run locally (results below).

LLM

Model: claude-opus-4-8
Thinking: high

Tier: Frontier

Gate A

$ ./scripts/check-parser-combinators.sh
$ echo $?
0

Exit 0 — no combinator-purity violations. (The script emits output only on violation; a clean run is silent.)

Anchored on

  • crates/engine/src/types/statics.rs:1556StaticMode::MustBeBlocked { by: Option<TargetFilter> }, the shipped one-blocker sibling; this PR applies the identical Option<TargetFilter> parameterization to MustBeBlockedByAll.
  • crates/engine/src/types/statics.rs:2753 — the MustBeBlocked:By({filter:?}) two-arm Display split; the new MustBeBlockedByAll Display mirrors it (None → unchanged label, Some → Debug form).
  • crates/engine/src/game/combat.rs:648 — the existing MustBeBlocked filtered enforcement loop's matches_target_filter(state, id, filter, &FilterContext::from_source(state, src_id)) conjunct; the new MustBeBlockedByAll enforcement reuses this exact filter-eval pattern.
  • crates/engine/src/game/coverage.rs:130 — the is_data_carrying_static arm for MustBeBlocked { .. }; MustBeBlockedByAll { .. } is added adjacent (both are now data-carrying, non-registry-keyed).

Verification

Non-developer track — cargo coverage / cargo semantic-audit / gen-card-data.sh deferred to CI (card-data.json cannot be generated on this box; see Track). Compensating local checks, all clean:

  • cargo fmt --all — clean
  • ./scripts/check-parser-combinators.sh — exit 0, no violations
  • cargo check -p engine and cargo check -p engine --tests — clean, 0 warnings
  • cargo test -p engine15175 lib + 1758 parser pass / 0 fail (6 ignored); all other engine test binaries 0 fail
  • 13 new/updated tests pass, with revert-discrimination empirically confirmed on both seams: deleting the combat is_none_or(matches_target_filter) conjunct fails the runtime discriminator parsed_talruum_piper_flying_lure_forces_only_fliers; reverting the parser filter slot fails the Some(filter) reach-guards.

Discriminating tests added:

  • parsed_talruum_piper_flying_lure_forces_only_fliers (combat.rs) — parses the real Oracle line, then drives validate_blockers: with a flying and a non-flying defender both able, []→Err, [(non_flier, attacker)]→Err, [(flier, attacker)]→Ok (the non-flier is not forced). Reach-guarded by asserting the extracted filter is Some(flying) matching the flier but not the non-flier.
  • mass_forced_block_filtered_opponents_control (oracle_effect/tests.rs) — one-shot seam; asserts Some(creatures your opponents control) at both the StaticDefinition and the AddStaticMode construction sites.
  • Parser shape tests for Talruum Piper (flying) and Marble Priest (Walls); regression test that unfiltered Lure/Ochran Assassin still parse to blockers: None; two-disjoint-lures, tapped-only-match, non-matching-idle, and both-shapes-data-carrying tests.

Scope Expansion

None.

Validation Failures

None.

CI Failures

None.

Parameterize StaticMode::MustBeBlockedByAll with an optional blocker
TargetFilter so the engine supports filtered mass-forced-block (filtered
Lure): "All <creature-filter> able to block <subject> do so". None
preserves the existing unfiltered Lure; Some(filter) forces only matching
creatures. Covers Talruum Piper (flying), Marble Priest (Walls), and You
Look Upon the Tarrasque (opponents-control). CR 509.1c.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Spaceint Spaceint requested a review from matthewevans as a code owner July 5, 2026 07:34
@github-actions github-actions Bot added the needs-maintainer AI-contribution PR requires human triage (Non-dev track or unresolved gaps) label Jul 5, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements filtered lures in accordance with CR 509.1c by parameterizing the StaticMode::MustBeBlockedByAll enum variant with an optional TargetFilter. The parser has been updated to extract blocker filters (such as 'creatures with flying' for Talruum Piper or 'Walls' for Marble Priest), and the blocker validation logic in combat.rs now correctly enforces that only qualifying creatures are compelled to block. Comprehensive unit tests have been added to verify the new functionality. No review comments were provided, and the implementation is highly idiomatic and conforms to the repository's architectural guidelines, so I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@matthewevans matthewevans left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MED] One-shot filtered lures lose the spell controller before combat evaluates controller-relative blocker filters. Evidence: crates/engine/src/parser/oracle_effect/tests.rs:17556 asserts the one-shot form parses creatures your opponents control as ControllerRef::Opponent, crates/engine/src/game/layers.rs:5023 installs the AddStaticMode as a recipient StaticDefinition, and crates/engine/src/game/combat.rs:1647 later rebuilds the filter context from that recipient source id. Why it matters: if the spell targets a creature controlled by someone other than the spell controller, your opponents is evaluated relative to the target creature's controller, so the wrong creatures are compelled to block. Suggested fix: preserve/snapshot the originating ability controller for controller-relative blocker filters when materializing AddStaticMode, and add a runtime test that targets an opponent-controlled creature.

[LOW] Required parse-diff evidence is not present yet for this engine/parser PR. Evidence: the PR currently changes parser seams at crates/engine/src/parser/oracle_static/evasion.rs:611 and crates/engine/src/parser/oracle_effect/mod.rs:7979, but the issue comments do not yet contain a <!-- coverage-parse-diff --> sticky for head 6b680a33c01c41fb8be19202d11f130b2ec592ad. Why it matters: the card-level gained/lost/changed parse coverage cannot be audited against the claimed Talruum Piper/Marble Priest scope. Suggested fix: let the card-data/parse-diff job complete and address any unexpected card-level changes before approval.

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown

Parse changes introduced by this PR · 3 card(s), 6 signature(s) (baseline: main 11eb20f500b3)

1 card(s) · static/MustBeBlockedByAll:By(Typed(TypedFilter { type_filters: [Creature], controller:… · added: MustBeBlockedByAll:By(Typed(TypedFilter { type_filters: [Creature], controller: None, properties: [WithKeyword { value: Flying }] })) (affects=self)

Examples: Talruum Piper

1 card(s) · ability/MustBeBlockedByAll:By(Typed(TypedFilter { type_filters: [Creature], controller:… · added: MustBeBlockedByAll:By(Typed(TypedFilter { type_filters: [Creature], controller: Some(Opponent), properties: [] })) (affects=parent target, duration=until end o…

Examples: You Look Upon the Tarrasque

1 card(s) · static/MustBeBlockedByAll:By(Typed(TypedFilter { type_filters: [Subtype("Wall")], cont… · added: MustBeBlockedByAll:By(Typed(TypedFilter { type_filters: [Subtype("Wall")], controller: None, properties: [] })) (affects=self)

Examples: Marble Priest

1 card(s) · ability/block · removed: block

Examples: You Look Upon the Tarrasque

1 card(s) · ability/effect_structure · removed: effect_structure

Examples: Marble Priest

1 card(s) · ability/static_structure · removed: static_structure

Examples: Talruum Piper

@matthewevans

Copy link
Copy Markdown
Member

The parse-diff sticky has now posted for head 6b680a33c01c41fb8be19202d11f130b2ec592ad. I checked it against the PR scope: the additions are the expected filtered MustBeBlockedByAll signatures for Talruum Piper, Marble Priest, and You Look Upon the Tarrasque, with the corresponding old unsupported/block structure signatures removed.

That resolves the parse-diff evidence gap from my review. The remaining blocker is still the MED controller-provenance issue in the formal review: the one-shot creatures your opponents control path needs to preserve the spell controller when combat later evaluates the controller-relative blocker filter.

AI Contributor and others added 2 commits July 5, 2026 17:04
One-shot filtered mass-forced-block effects (You Look Upon the
Tarrasque) graft the MustBeBlockedByAll static onto the target creature
via AddStaticMode, which dropped the spell controller so combat resolved
a controller-relative blocker filter ("creatures your opponents control")
relative to the target's controller instead of the caster's. Snapshot the
originating ability controller onto StaticDefinition.source_controller at
graft time (mirrors ReplacementDefinition.source_controller) and anchor the
combat filter context via from_source_with_controller. Covers the sibling
MustBeBlocked { by } seam too. CR 611.2c, CR 109.5, CR 509.1c.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Spaceint

Spaceint commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Both points addressed in 0e940a943 (+ merge 6500fa4fd to current main):

[MED] one-shot controller-relative lures lost the spell controller — fixed.
Root cause exactly as you diagnosed: the one-shot form grafts the MustBeBlockedByAll static onto the target creature via AddStaticMode, and combat rebuilt the filter context from that recipient's source id (combat.rs), so creatures your opponents control resolved relative to the target's controller (CR 109.5).

Fix — snapshot the originating ability controller at graft time:

  • Added StaticDefinition.source_controller: Option<PlayerId> (#[serde(default, skip_serializing_if = "Option::is_none")]), mirroring the existing ReplacementDefinition.source_controller anchor pattern.
  • At the AddStaticMode materialization (layers.rs), when the granted mode carries a controller-relative blocker filter, bake effect.controller onto the grafted definition — the same shape as the adjacent resolve_static_mode_chosen_color / chosen_color snapshot. CR 611.2c fixes the reference at inception.
  • Combat's two must-be-blocked requirement iterators now thread the anchor and evaluate via FilterContext::from_source_with_controller(src_id, anchor) (falling back to from_source when None, so permanent lures — Talruum Piper, Marble Priest — are byte-for-byte unchanged). The sibling MustBeBlocked { by } seam is covered by the same plumbing.

Multiplayer-correct: ControllerRef::Opponent stays a set predicate (every non-caster player), it is not collapsed to a single player.

New runtime test (your requested case): parsed_tarrasque_one_shot_lure_evaluates_opponents_from_caster_seat — a 3-player game where P0 casts the Tarrasque one-shot lure targeting P1's creature, driven through the real parse_effect → resolve → evaluate_layers → validate_blockers pipeline. It asserts the grafted static's source_controller == Some(P0), that only the caster's opponents' creatures are compelled (P0's own creature is not), and validate_blockers(&state, &[]).is_ok() — which fails on revert to the from_source(target) behavior.

[LOW] parse-diff evidence — thanks, confirmed resolved on your follow-up: the sticky shows only the three intended cards (Talruum Piper / Marble Priest / You Look Upon the Tarrasque) gaining the filtered MustBeBlockedByAll signatures with the old unsupported/structure signatures removed, no collateral.

Verification (Non-developer track, Windows — coverage/semantic-audit deferred to CI): merged current upstream/main, cargo fmt clean, cargo check -p engine + --tests clean, cargo clippy -p engine --all-targets 0 warnings, full cargo test -p engine green (177 binaries / 0 fail), and the filtered-lure + new controller-anchor tests pass.

🤖 Addressed by Claude Code

…terals

CI builds the full workspace; phase-ai constructs full StaticDefinition
literals in its feature/policy tests that need the new field.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@matthewevans matthewevans left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MED] Distinct one-shot lure anchors can still collapse when the same mode is already installed on the target. Evidence: crates/engine/src/game/layers.rs:5081 snapshots the installing player into def.source_controller, but the install guard at crates/engine/src/game/layers.rs:5084 only checks sd.mode == resolved_mode; StaticDefinition::source_controller is the field combat later consumes at crates/engine/src/game/combat.rs:1024 and crates/engine/src/game/combat.rs:1043. Why it matters: in multiplayer, if two players cast the same controller-relative one-shot lure on the same creature, the second effect has the same MustBeBlockedByAll { blockers: ... } mode but a different caster anchor, and it is silently dropped, so one player’s required-blocker set is never enforced. Suggested fix: include the full installed static definition, including source_controller, in the idempotency check for AddStaticMode, and add a runtime test with two different casters applying the same controller-relative lure to one target.

AI Contributor and others added 2 commits July 6, 2026 05:52
The AddStaticMode graft deduped on mode only, so two different casters
installing the same controller-relative lure mode on one permanent collapsed
to a single static — the second caster's source_controller anchor (and thus
its CR 509.1c required-blocker set) was silently dropped. Compare the full
grafted StaticDefinition (including source_controller) so distinct-caster
anchors coexist; same-caster re-application across layer passes still dedups.
CR 611.2c, CR 509.1c.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Spaceint

Spaceint commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

Addressed in d5a1752f0 (+ merged current main).

[MED] distinct one-shot lure anchors collapsing in the AddStaticMode dedup — fixed.
Exactly as you described: the graft's idempotency guard keyed on sd.mode == resolved_mode, so a second caster's same-mode controller-relative lure on the same permanent was dropped along with its distinct source_controller anchor. The guard now compares the full grafted StaticDefinition (sd == &def, which includes source_controller):

  • Two different casters → different anchors → both statics coexist, and combat's must_be_blocked_by_all_requirements_for_attacker yields each as a separate CR 509.1c requirement with its own FilterContext (per-anchor). CR 611.2c — each resolution-generated continuous effect locks its own anchor at inception, so they must not merge.
  • Same caster re-applied across layer passes → identical def → still dedups (no grant multiplication). Non-lure grants (source_controller: None) are unaffected.

Test (your requested case): two_casters_same_lure_on_one_target_each_anchor_enforced — 3-player game, P0 and P2 each resolve the Tarrasque lure targeting the same P1 creature through the real parse_effect → resolve → evaluate_layers pipeline. It asserts (1) two distinct source_controller anchors {Some(P0), Some(P2)} coexist on the target, and (2) behaviorally that P0's own creature (an opponent of P2 but not of P0) is compelled to block — which is only true if P2's anchor survived. I confirmed it FAILS when the guard is reverted to sd.mode == resolved_mode, and it's reach-guarded (the creature can legally block; assigning it makes the declaration legal) so the negative isn't vacuous.

Verification (merged current main, Non-developer track — coverage/semantic-audit on CI): cargo fmt clean, cargo check -p engine clean, cargo clippy --workspace --exclude phase-tauri --all-targets --features engine/proptest -- -D warnings clean, full cargo test -p engine green (178 binaries / 0 fail).

🤖 Addressed by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-maintainer AI-contribution PR requires human triage (Non-dev track or unresolved gaps)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants