Skip to content

Add Plargg and Nassari - #55

Open
Whovencroft wants to merge 9 commits into
mainfrom
card/plargg-and-nassari
Open

Add Plargg and Nassari#55
Whovencroft wants to merge 9 commits into
mainfrom
card/plargg-and-nassari

Conversation

@Whovencroft

@Whovencroft Whovencroft commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Summary

Adds engine support for Plargg and Nassari.

Closes the previously-unsupported choose gap on the upkeep trigger's middle sentence — "An opponent chooses a nonland card exiled this way." — and, per review, makes the full trigger rules-correct end-to-end:

  1. Parser (anaphor): try_parse_choose_exiled_anaphor's "exiled this way" referent only supported the untyped chain-tracked-set form ("choose a card exiled this way") and fell through honestly on a typed qualifier. The typed form ("a nonland card exiled this way") now lowers to the same linked-exile shape as the Koh, the Face Stealer "exiled with ~" arm: ChooseFromZone { zone: Exile, zone_owner: AllOwners, filter: And { Typed, ExiledBySource } } (CR 607.2a linkage; CR 400.1 shared-zone scan). The card-type qualifier now also accepts negated non<type> forms (CR 205.2b), and an "an opponent chooses" prefix (→ Chooser::Opponent, CR 608.2d) mirrors parse_choose_anaphoric's existing convention. Per review, the speculative "target opponent chooses" tag was dropped in 76cf4d1 — no printed card pairs it with an exiled-this-way anaphor, and collapsing a targeted chooser to the generic opponent would be wrong.
  2. Parser (subject layer): strip_subject_clause peels "an opponent" and deconjugates the predicate to "choose …", losing the chooser. lower_subject_predicate_ast now rebinds the bare single-opponent subject onto ChooseFromZone's chooser before the honesty gate; typed or property-carrying subjects still fall through to Effect::unimplemented. Per review 3, the rebind is additionally gated on subject.target.is_none() — "target opponent chooses …" (Forgotten Lore, Shrouded Lore) binds the chooser to a chosen target slot that Chooser::Opponent cannot represent, so the targeted form keeps its honest fall-through (see Claimed parse impact).
  3. Parser + runtime ("up to two spells" bound — review HIGH, 0559e93): "cast up to N spells from among the [other] cards exiled this way without paying their mana costs" no longer lowers to CastFromZone permission grants (per-object, no shared budget — the printed bound was silently dropped). It now routes to Effect::FreeCastFromZones, the counted during-resolution free-cast window (Invoke Calamity machinery), with count: N — matching ruling 2021-04-16 that Plargg's spells are cast during the ability's resolution. FreeCastFromZones gains a Zone::Exile arm scoped to the resolving source's exile links (CR 607.2a) plus a cast-mode land guard (CR 305.1), and the source is threaded through CastOfferKind::FreeCastWindow and the re-offer loop. "The other cards" rewrites FilterProp::Another → Not(InTrackedSet(0)) over the prior choose's published picks (CR 608.2c), mirroring rewrite_filter_prop_another_to_tracked_set; uncounted forms (Etali, Primal Conqueror's "cast any number") keep their existing lowering.
  4. Runtime (multiplayer chooser — review HIGH, 0559e93): resolving a ChooseFromZone with chooser: Opponent and two or more live opponents now pauses on a new typed resolution-time step — WaitingFor::ChooseFromZoneOpponentChooser / GameAction::ChooseZoneOpponentChooser — so the controller picks which opponent makes the choice, per the card's release notes ("you choose which opponent gets to choose"). Mirrors the ClashChooseOpponent pattern end-to-end (pause raise, resume handler, AI candidate enumeration, decision-kind grouping, stable action ordering, scenario naming, manabrew-compat conversion, server payload guard, and the frontend: adapter unions, handler registry, ZoneOpponentChooserModal mounted in GamePage, i18n keys in all seven locales — c88ad0e). With exactly one live opponent the picker is skipped and the choice goes directly to that opponent.
  5. Runtime (candidate pool): resolve_candidate_cards returned tracked-set-derived candidate pools raw, ignoring the effect's own card filter — the player-scope completion publishes every card the exile clause moved (lands included) as the chain tracked set, so the opponent was offered the lands too. A typed filter (Some(...)) now narrows tracked-set and explicit-target pools through matches_target_filter (CR 608.2d); filter: None keeps the raw set, so the untyped "choose one of them" tracked-set anaphors are unchanged. Surfaced by the runtime regression test on this PR's first CI run.
  6. Runtime (resolution-scoped "exiled this way" — review 2 HIGH): TargetFilter::ExiledBySource reads the source's complete live linked-exile ledger, so a linked nonland card left in exile by a previous upkeep (declined window) would wrongly re-enter the next resolution's free-cast offer. The ChooseFromZoneChoice answer handler now forwards the choose's full offered pool — THIS resolution's typed exile batch, derived from the chain tracked set the exile clause published — as the FreeCastFromZones continuation's object targets; the resolver reads it as a concrete member_pool, and eligible_candidates intersects the exile-zone scan with it BEFORE the filter's Not(InTrackedSet) chosen-card exclusion (CR 607.2a). The pool is carried on CastOfferKind::FreeCastWindow and ResolutionCastSuccessAction::FreeCastOfferRemaining so the re-offer loop stays confined too, and is redacted like candidates for non-controller viewers (CR 400.2). Empty means no batch restriction — Invoke Calamity's chooseless graveyard/hand window is untouched, as is Etali's uncounted permission-grant path. The same seam fixes a defect the c88ad0e CI run exposed (CI Failures item 5): chain_references_tracked_set could not see the chosen-card exclusion — FilterProp::Not { InTrackedSet } nested inside the typed leg of an And — so the choose head never published the chosen card as the fresh chain set, the InTrackedSet(0) sentinel stayed bound to the exile-batch set, and Not over the batch emptied the window entirely. A property-level membership walker (filter_properties_reference_tracked_membership, mirroring the existing quantity walker) now feeds the FreeCastFromZones detection arm (CR 608.2c).

An earlier interim seam (binding the unchosen complement in the ChooseFromZoneChoice handler) is reverted in 0559e93 — the tracked-set complement filter in seam 3 supersedes it structurally, so continuation semantics for every other card class are untouched.

Files changed

  • crates/engine/src/parser/oracle_effect/imperative.rs
  • crates/engine/src/parser/oracle_effect/mod.rs
  • crates/engine/src/parser/oracle_effect/tests.rs
  • crates/engine/src/game/engine_resolution_choices.rs
  • crates/engine/src/game/effects/choose_from_zone.rs
  • crates/engine/src/game/effects/exile_from_top_until.rs
  • crates/engine/src/game/effects/free_cast_from_zones.rs
  • crates/engine/src/game/effects/mod.rs
  • crates/engine/src/game/casting_costs.rs
  • crates/engine/src/game/visibility.rs
  • crates/engine/src/game/scenario.rs
  • crates/engine/src/ai_support/candidates.rs
  • crates/engine/src/ai_support/mod.rs
  • crates/engine/src/types/game_state.rs
  • crates/engine/src/types/actions.rs
  • crates/engine/src/types/ability.rs
  • crates/engine/src/types/action_stable_order.rs
  • crates/manabrew-compat/src/lib.rs
  • crates/phase-ai/src/decision_kind.rs
  • crates/phase-ai/src/search.rs
  • crates/server-core/src/game_action_payload_guard.rs
  • client/src/adapter/types.ts
  • client/src/game/waitingForRegistry.ts
  • client/src/pages/GamePage.tsx
  • client/src/components/modal/ZoneOpponentChooserModal.tsx
  • client/src/components/modal/tests/ZoneOpponentChooserModal.test.tsx
  • client/src/i18n/locales/{de,en,es,fr,it,pl,pt}/game.json

CR references

  • CR 101.4 (APNAP — superseded for the chooser pick by the release-notes-mandated controller decision)
  • CR 205.2b (non<type> card-type qualifier)
  • CR 305.1 (land cards can never be cast — the window's land guard)
  • CR 400.1 (exile is a shared zone — ownership must not gate the pool)
  • CR 400.2 (hidden-zone information — the member pool is redacted for non-controller viewers alongside the candidates)
  • CR 601.2 ("up to N" counted cast bound)
  • CR 607.2a ("exiled this way" is a linked-ability reference to the exiling instruction, scoped to THIS resolution's batch — not the source's lifetime ledger)
  • CR 608.2c (effect resolution — "the other cards" complement)
  • CR 608.2d ("an opponent chooses" delegates the resolution-time selection; the controller decides which opponent when the text doesn't say)

Implementation method (required)

Method: /engine-implementer

Track

Non-developer

LLM

Model: manus-1.5
Thinking: high

Verification

  • Required checks ran clean, or the exact CI-owned alternative is stated below.
  • Gate A output below is for the current committed head.
  • Final review-impl below is clean for the current committed head.
  • Both anchors cite existing analogous code at the same seam.
  • ./scripts/check-parser-combinators.sh upstream/mainGate G PASS + Gate A PASS on the committed round-3 head 92c56e7 (the rebind gate and constraint gate are new conditionals inside existing arms — no new combinator dispatch).
  • Frontend, run locally on the round-3 head: pnpm run type-check — clean; eslint on the touched files — 0 errors; vitest run — full suite executed (262 files / 2236 tests): 255 files passed outright and the 7 sandbox-timeout files (unrelated menu/lobby/deck suites hitting the 5s cap while a parallel Rust compile saturated the VM) all pass in isolation, 63/63.
  • rustfmt --check — clean on every Rust file this branch touches (verified locally on the round-3 head).
  • Non-developer track: cargo test -p engine (including the nine new tests below), cargo clippy-strict, cargo coverage, and cargo semantic-audit are delegated to the CI-owned checks on this PR — the ephemeral sandbox was OOM-reset during local compile attempts, so full local test execution could not complete.
  • New tests on this PR: parse_choose_nonland_card_exiled_this_way, parse_opponent_chooses_nonland_card_exiled_this_way, parse_choose_card_exiled_this_way_still_tracked_set (imperative.rs), plargg_and_nassari_full_trigger_chain_choose_then_cast_others (tests.rs — asserts the FreeCastFromZones{count: 2} tail with the Not(InTrackedSet) rewrite), search_for_survivors_opponent_chooses_at_random_in_your_graveyard and target_opponent_chooses_in_your_graveyard_keeps_honest_fall_through (tests.rs — round-3 parse-impact pins), plargg_opponent_chooses_nonland_hit_then_cast_pool_is_the_other_hits (exile_from_top_until.rs — 4-player, apply()-driven through all three pauses: opponent-picker → zone choice → capped FreeCastWindow with the unchosen-only pool and remaining_casts == 2), plargg_two_player_skips_the_opponent_picker_pause (2-player regression: no picker pause with a single opponent), plargg_second_resolution_excludes_previous_resolutions_leftovers (exile_from_top_until.rs — review-2 two-upkeep regression: the full chain resolves twice against the same source; upkeep 1's window is declined leaving a linked nonland card in exile — and asserted still exile-linked — and upkeep 2's choice pool AND free-cast window are asserted to exclude it), and ZoneOpponentChooserModal.test.tsx (frontend: registry registration, ChooseZoneOpponentChooser dispatch, and engine-supplied render order).

Gate A

Gate A PASS head=92c56e77aa506a228ce0a53b74e2f062c2acc5b2 base=209324a701dce9d3ea183f79d9380032d00706ea (Gate G PASS on the same run)

Anchored on

  • crates/engine/src/parser/oracle_effect/imperative.rs:4040 — Koh, the Face Stealer's "exiled with ~" arm: the existing FromZone { Exile, AllOwners, And { Typed, ExiledBySource } } lowering the typed "exiled this way" referent now reuses (CR 607.2a + CR 400.1).
  • crates/engine/src/game/effects/clash.rs — ClashChooseOpponent: the existing typed resolution-time "controller picks an opponent" pause/resume seam the new ChooseFromZoneOpponentChooser step mirrors across every registration site.
  • crates/engine/src/game/effects/free_cast_from_zones.rs — Invoke Calamity's counted free-cast window: the existing "up to count" stop-early offer loop the Plargg cast tail now rides, extended with the exile-links zone arm.

Final review-impl

Final review-impl PASS head=92c56e7 (round-3 review fixes + picker continuation parking over cb332a5)

  • Full-diff pass against CLAUDE.md and the review-impl checklist: no string-method dispatch, no match-on-string-literal arms, no chained if let Ok(tag) blocks, no hand-built Effect::Unimplemented literals in the diff.
  • Seam check: typed-anaphor lowering sits in the existing anaphor combinator (not a new dispatch path); the chooser rebind sits at the subject/predicate lowering seam where every other subject rebind lives; the opponent-picker pause sits in choose_from_zone's single chooser-resolution authority with its resume in the one interactive-choice handler; the counted-cast reroute sits in try_parse_cast_effect ahead of the uncounted arm it specializes.
  • Test discrimination: the 4-player runtime test drives the real pipeline (resolve_ability_chainWaitingFor::ChooseFromZoneOpponentChooserapply(ChooseZoneOpponentChooser)WaitingFor::ChooseFromZoneChoiceapply(SelectCards)WaitingFor::CastOffer{FreeCastWindow}) and fails if the picker pause, the chooser delegation, the typed-anaphor filter, the cap, or the complement rewrite is reverted. The 2-player test pins the no-picker fast path. The untyped-referent regression guard pins End-Blaze-style impulse reductions to the chain tracked set.

Claimed parse impact

  • Plargg and Nassari
  • Author of Shadows (middle sentence "Choose a nonland card exiled this way." — same typed exiled-this-way anaphor; the card's remaining clauses are unchanged)
  • Search for Survivors (review 2/3 MED — claimed and pinned): its sentence "An opponent chooses a card at random in your graveyard." rides the subject-layer chooser rebind (Summary seam 2) — strip_subject_clause peels the bare untargeted "an opponent" subject, the predicate lowers through the existing random graveyard choose arm, and the rebind restores Chooser::Opponent — hence its + ChooseFromZone (count=1, zone=graveyard) parse-diff signature. Pinned by search_for_survivors_opponent_chooses_at_random_in_your_graveyard (tests.rs), asserting chooser, zone, and CardSelectionMode::Random.
  • Forgotten Lore and Shrouded Lore (review 3 MED — deliberately reverted, not claimed): their sentence is "Target opponent chooses …". parse_target lowers "target opponent" to the same bare opponent filter as "an opponent", so the rebind originally caught them too — but the subject carries target: Some(_): the chooser is a chosen target slot, and Chooser::Opponent cannot represent that binding (the controller would wrongly pick the chooser at resolution). The rebind is now gated on subject.target.is_none(), restoring both cards to the honest Effect::Unimplemented fall-through — the same deliberate rejection the exiled-this-way anaphor keeps for its targeted form (76cf4d1). Pinned by target_opponent_chooses_in_your_graveyard_keeps_honest_fall_through (tests.rs). The CI parse diff on this head should list exactly three cards.

Validation Failures

None.

CI Failures

Six red runs during iteration, each root-caused and fixed on this branch.

  1. Rust lint — two rustfmt diffs in test/handler code introduced after a sandbox reset dropped the local cargo fmt pass. Fixed in 5578e01 by applying the exact rustfmt output.
  2. Rust tests (both shards) — E0433: WaitingFor missing from the runtime test's local import list. Fixed in de0e127.
  3. Rust tests (shard 2/2) — the new runtime regression test failed: the opponent's candidate pool contained all six exiled cards (lands included) because resolve_candidate_cards ignored the effect filter for chain-tracked-set pools. Fixed in 7aaadbd (Summary seam 5); shard 1, Card data, and all other checks were already green on the same run, and no pre-existing test changed behavior.
  4. Review-round head 0559e93 — three failures, all fixed in c88ad0e: (a) Rust tests (both shards) — E0063: the pre-existing FreeCastWindow visibility test constructor was missing the new source field; (b) Rust lint — two rustfmt diffs in the new runtime tests; (c) Frontend — the boundary-guardrail tests require every new engine WaitingFor/GameAction variant to be mirrored in the client unions with a registered UI handler, so ZoneOpponentChooserModal was wired end-to-end (adapter types, registry, GamePage mount, i18n keys in all locales, modal test).
  5. Head c88ad0e, Rust tests (shard 2/2) — the 4-player runtime test ran for the first time (both prior shards had died at the E0063 compile error) and exposed a real defect: after the opponent's pick, the free-cast window was silently skipped (Priority instead of CastOffer). Root cause: chain_references_tracked_set walks whole-filter TrackedSet variants but Plargg's chosen-card exclusion is FilterProp::Not { InTrackedSet } nested inside the typed leg of an And, so the choose head never published the chosen card as the fresh chain set; the InTrackedSet(0) sentinel stayed bound to the exile-batch set and Not over the batch emptied the window. Fixed by adding a property-level membership walker (filter_properties_reference_tracked_membership, mirroring the existing quantity walker) to the FreeCastFromZones detection arm.
  6. Head cb332a5, Rust tests (shard 2/2) — the 4-player test still failed with the same symptom (Priority instead of CastOffer) while the two-upkeep and 2-player regressions passed, isolating the defect to the picker path: WaitingFor::ChooseFromZoneOpponentChooser was never registered in waits_for_resolution_choice, so when the picker pause was raised the chain machinery treated the choose head as not paused and resolved the FreeCastFromZones sub-ability inline — before any pick existed — leaving nothing parked for the answer handlers to drive. Fixed by registering the variant alongside ChooseFromZoneChoice; the picker now parks the free-cast continuation exactly like the direct 2-player path (item 1 of the round-3 review response hardens the same handler's empty-pool drain).

Review response (matthewevans, round 1, CHANGES_REQUESTED)

  • HIGH — controller picks the choosing opponent: implemented in 0559e93 as the typed ChooseFromZoneOpponentChooser resolution step (Summary seam 4); the 4-player test drives the pick and the 2-player test pins the skip path.
  • HIGH — "up to two spells" bound: implemented in 0559e93 via the FreeCastFromZones reroute (Summary seam 3); the cap is asserted (remaining_casts == 2) in the 4-player test and the parser chain test.
  • MED — targeted chooser: fixed in 76cf4d1 — the speculative "target opponent chooses" tag was dropped from the new prefix; no printed card pairs it with an exiled-this-way anaphor.

Review response (matthewevans, round 2, CHANGES_REQUESTED)

  • HIGH — frontend wiring for the new pause: already landed in c88ad0e (crossed with the review): adapter unions, waitingForRegistry registration, ZoneOpponentChooserModal mounted in GamePage, i18n keys in all seven locales, and the modal dispatch test. Frontend CI is green on that head's checks locally (guardrails + parity suites).
  • HIGH — "exiled this way" must scope to THIS resolution: implemented as the concrete member-pool thread (Summary seam 6). The choose's offered pool — already resolution-scoped because it derives from the chain tracked set the exile clause published within the same resolution — is forwarded through the choose → free-cast continuation and intersected in eligible_candidates before the Not(InTrackedSet) chosen-card exclusion; the re-offer loop carries it via FreeCastOfferRemaining, and the visibility layer redacts the pool to placeholders exactly like candidates (CR 400.2, asserted in the extended visibility test). The requested two-upkeep regression (plargg_second_resolution_excludes_previous_resolutions_leftovers) resolves the full chain twice: upkeep 1's window is declined leaving a linked nonland card in exile, and upkeep 2 asserts both the opponent's choice pool and the free-cast window exclude the stale leftover while offering exactly the new batch. The same round also fixes the shard-2 defect CI exposed on c88ad0e (CI Failures item 5): the tracked-set consumption detection now sees the property-level Not(InTrackedSet) exclusion, so the chosen card is published as the fresh chain set and the window opens over the correct pool.
  • MED — parse-diff reconciliation: the three unclaimed cards (Forgotten Lore, Shrouded Lore, Search for Survivors) are now claimed and justified in "Claimed parse impact" — they ride the subject-layer chooser rebind (seam 2), the same delegated-chooser grammar family, with their remaining clauses parsing via pre-existing arms. (Superseded in round 3: the Lores are deliberately reverted — see below.)

Review response (matthewevans, round 3, CHANGES_REQUESTED)

  • HIGH — empty member-pool continuation drops priority: fixed. The ChooseZoneOpponentChooser answer arm's empty-pool drain discarded player and called resume_pending_continuation_if_priority while waiting_for was still the picker pause — the drain is gated on WaitingFor::Priority, so the state wedged. The arm now binds player and calls set_priority(state, player) before resuming, mirroring the adjacent ChooseClashOpponent arm's exact sequence.
  • HIGH — counted free-cast arm drops a parsed cast constraint: fixed. try_parse_counted_free_cast_from_exiled_this_way's caller now gates the reroute on constraint.is_none()Effect::FreeCastFromZones has no constraint channel, so a constrained counted form (none printed today) falls through to the strict handling instead of silently discarding the restriction.
  • MED — modal must render engine order: fixed. ZoneOpponentChooserModal no longer re-sorts candidates by client seatOrder; it renders the engine-supplied candidate order verbatim (the engine already emits opponents in turn order from the controller — that game ordering belongs in the engine). The modal test now pins the render order against a deliberately shuffled input.
  • MED — capability registry: fixed. local.zone-opponent-chooser-unsupported is registered in manabrew-compat's UNSUPPORTED_PROTOCOL_CAPABILITIES (18 → 19 entries) with the well-formedness test updated.
  • MED — parse-diff reconciliation with tests: resolved via the subject.target.is_none() gate plus two pinning parser tests — Search for Survivors claimed, the Lores deliberately reverted (see Claimed parse impact). Expected parse diff on this head: exactly three cards.
  • (CodeRabbit nitpick) the two-upkeep regression now also asserts the declined card remains exile-linked to the source after upkeep 1, pinning that the exclusion comes from resolution scoping rather than link pruning.

…igger

The middle sentence "An opponent chooses a nonland card exiled this
way" was Unimplemented{choose}: the typed exiled-this-way anaphor fell
through honestly, the "an opponent" subject lost the chooser, and the
ChooseFromZoneChoice continuation bound the pick (not the complement)
onto the follow-up cast.

- imperative.rs: typed "exiled this way" arm of
  try_parse_choose_exiled_anaphor lowers to the Koh-shape
  FromZone { Exile, AllOwners, And { Typed, ExiledBySource } }
  (CR 607.2a linkage; CR 400.1 shared-zone scan); chooser prefix
  alternation ("an opponent chooses" -> Chooser::Opponent, CR 608.2d);
  non<type> qualifier support (CR 205.2b).
- mod.rs: lower_subject_predicate_ast rebinds the bare "an opponent"
  subject onto ChooseFromZone's chooser before the honesty gate.
- engine_resolution_choices.rs: an "other cards" cast continuation
  (ExiledBySource + FilterProp::Another) receives the UNCHOSEN
  complement of the pick (CR 608.2c); chosen-consumer continuations
  (End-Blaze Epiphany) are untouched.

Tests: two anaphor unit tests + untyped-referent regression guard
(imperative.rs), full three-sentence chain shape (tests.rs), and a
three-player runtime test driving the opponent's pick and asserting
zero-cost permissions land only on the other nonland hits
(exile_from_top_until.rs).

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0dd2600f54

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

filter: TargetFilter::And {
filters: vec![
TargetFilter::Typed(TypedFilter::new(tf)),
TargetFilter::ExiledBySource,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Bind typed exiled-this-way choices to the current set

When a typed exiled this way choice is lowered to a durable ExiledBySource scan, it is no longer scoped to the objects exiled by this resolving instruction. For example, a later Plargg upkeep can offer nonland cards that the same source exiled on previous upkeeps and left in exile; for an Author of Shadows-style chain, a preceding tracked-set producer can also cause ChooseFromZone to read the published set before applying this new nonland filter. This needs to bind a current-resolution filtered set (or filter tracked-set candidates) rather than the source's lifetime linked-exile pool.

AGENTS.md reference: AGENTS.md:L14-L14

Useful? React with 👍 / 👎.

Comment on lines +3847 to +3848
cont.chain.targets =
unchosen.iter().map(|&id| TargetRef::Object(id)).collect();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Enforce the two-spell cap before granting permissions

In a four-player Plargg game, the opponent chooses one nonland hit and this branch then forwards every unchosen hit as CastFromZone targets; CastFromZone stamps a separate zero-cost permission on each target and has no count field, so the controller can cast all three remaining hits even though the card says "up to two spells." The continuation needs to carry/enforce that limit before granting permissions instead of binding the full unchosen complement.

AGENTS.md reference: AGENTS.md:L14-L14

Useful? React with 👍 / 👎.

@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown

Parse changes introduced by this PR · 3 card(s), 3 signature(s) (baseline: main 1b11783fae63)

🟢 Added (2 signatures)

  • 2 cards · ➕ ability/ChooseFromZone · added: ChooseFromZone (count=1, zone=exile)
    • Affected (first 3): Author of Shadows, Plargg and Nassari
  • 1 card · ➕ ability/ChooseFromZone · added: ChooseFromZone (count=1, zone=graveyard)
    • Affected (first 3): Search for Survivors

🔴 Removed (1 signature)

  • 3 cards · ➖ ability/choose · removed: choose
    • Affected (first 3): Author of Shadows, Plargg and Nassari, Search for Survivors

10 card(s) had Oracle-text changes (errata/reprint) — excluded as non-parser.
New cards in head: 5.

…is-way anaphor

'target opponent chooses' must bind the chooser to the chosen target slot;
collapsing it into Chooser::Opponent loses target identity, and no printed
card pairs the targeted form with an 'exiled this way' anaphor. Accept only
'an opponent chooses' and let the targeted form fall through honestly.

Addresses the CodeRabbit review finding on PR phase-rs#6320.
…st window for 'up to N spells ... exiled this way'

Addresses the CHANGES_REQUESTED review on phase-rs#6320:

HIGH-1 (multiplayer chooser): resolving a ChooseFromZone with
chooser: Opponent and 2+ live opponents now pauses on the new typed
WaitingFor::ChooseFromZoneOpponentChooser so the CONTROLLER picks which
opponent makes the choice (Plargg release notes), mirroring the
ClashChooseOpponent pattern. With exactly one live opponent the picker
is skipped. Registered across engine, ai_support, phase-ai, scenario,
action stable order, manabrew-compat, and the server payload guard.

HIGH-2 (up-to-two bound): 'cast up to N spells from among the [other]
cards exiled this way without paying their mana costs' no longer lowers
to CastFromZone permission grants (no shared budget); it now routes to
Effect::FreeCastFromZones - the during-resolution counted window
(Invoke Calamity machinery) - matching ruling 2021-04-16 that the
spells are cast during resolution. FreeCastFromZones gains a Zone::Exile
arm scoped to the resolving source's exile links, a cast-mode land
guard, and source threading through the CastOffer window and re-offer
loop. 'the other cards' rewrites Another -> Not(InTrackedSet(0)) over
the prior choose's published picks.

The unchosen-complement binding in the ChooseFromZoneChoice handler is
reverted (superseded by the tracked-set complement filter).

Tests: 4-player runtime test drives all three pauses (picker ->
opponent zone choice -> capped FreeCastWindow with the unchosen-only
pool); 2-player regression asserts the picker pause is skipped; parser
chain test updated to the FreeCastFromZones tail.

Copy link
Copy Markdown
Owner Author

CI re-check: maintainer-review fixes pushed in 0559e93 (opponent-picker pause + counted free-cast window).

Whovencroft added 2 commits July 22, 2026 06:51
…nent-chooser step

- client: mirror WaitingFor::ChooseFromZoneOpponentChooser and
  GameAction::ChooseZoneOpponentChooser in the adapter unions, register the
  variant in HANDLED_WAITING_FOR_TYPES, and wire ZoneOpponentChooserModal
  (ClashOpponentModal pattern) into GamePage with i18n keys in all locales
- engine: add the new 'source' field to the FreeCastWindow test constructor
  in visibility.rs (E0063 on both test shards)
- engine: apply rustfmt to the two flagged sites in exile_from_top_until.rs
…resolution's exile batch

Review round 2 (matthewevans, CHANGES_REQUESTED) + shard-2 CI defect on c88ad0e:

- Thread THIS resolution's concrete member pool (the choose's full offered
  pool, derived from the chain tracked set the exile clause published) through
  the ChooseFromZoneChoice -> FreeCastFromZones continuation as object targets;
  eligible_candidates intersects the exile scan with it BEFORE the
  Not(InTrackedSet) chosen-card exclusion (CR 607.2a), so a linked nonland
  card left in exile by a previous upkeep is never re-offered. Carried on
  CastOfferKind::FreeCastWindow and FreeCastOfferRemaining for the re-offer
  loop; redacted like candidates for non-controller viewers (CR 400.2).

- Fix the c88ad0e shard-2 failure (window skipped, Priority instead of
  CastOffer): chain_references_tracked_set could not see the chosen-card
  exclusion nested as FilterProp::Not { InTrackedSet } inside the typed leg
  of an And, so the chosen card was never published as the fresh chain set
  and the InTrackedSet(0) sentinel stayed bound to the exile-batch set,
  emptying the window. Add a property-level membership walker
  (filter_properties_reference_tracked_membership) to the FreeCastFromZones
  detection arm (CR 608.2c).

- Add the requested two-upkeep regression
  (plargg_second_resolution_excludes_previous_resolutions_leftovers): upkeep 1
  declines the window leaving a linked leftover in exile; upkeep 2 asserts
  both the opponent's choice pool and the free-cast window exclude it.

- Extend the FreeCastWindow visibility test to pin member_pool redaction.

Copy link
Copy Markdown
Owner Author

Review round 2 addressed in cb332a5 (PR body updated with full details).

HIGH — resolution-scoped "exiled this way": implemented as a concrete member-pool thread through the choose → free-cast continuation, exactly as suggested. The ChooseFromZoneChoice answer handler forwards the choose's full offered pool — THIS resolution's typed exile batch, derived from the chain tracked set the exile clause published within the same resolution — as the FreeCastFromZones continuation's object targets. The resolver reads it as member_pool, and eligible_candidates intersects the exile-zone scan with it before the Not(InTrackedSet) chosen-card exclusion (CR 607.2a). The pool is carried on CastOfferKind::FreeCastWindow and ResolutionCastSuccessAction::FreeCastOfferRemaining so the re-offer loop stays confined, and is redacted to placeholders for non-controller viewers exactly like candidates (CR 400.2, asserted in the extended visibility test). An empty pool means no batch restriction, so Invoke Calamity's chooseless graveyard/hand window and Etali's uncounted permission-grant path are untouched. The requested two-upkeep regression (plargg_second_resolution_excludes_previous_resolutions_leftovers) resolves the full chain twice against the same source: upkeep 1's window is declined leaving a linked nonland card in exile, and upkeep 2 asserts both the opponent's choice pool and the free-cast window exclude the stale leftover while offering exactly the new batch.

HIGH — frontend wiring: landed in c88ad0e (crossed with the review): adapter WaitingFor/GameAction unions, waitingForRegistry registration, a localized ZoneOpponentChooserModal (ClashOpponentModal pattern) mounted in GamePage, i18n keys in all seven locales, and the browser-facing dispatch test. The Frontend CI check is green on that head.

Shard-2 failure on c88ad0e (window skipped, Priority instead of CastOffer): root-caused and fixed in cb332a5. chain_references_tracked_set walks whole-filter TrackedSet variants, but the chosen-card exclusion here is FilterProp::Not { InTrackedSet } nested inside the typed leg of an And — so the choose head never published the chosen card as the fresh chain set, the InTrackedSet(0) sentinel stayed bound to the exile-batch set, and Not over the batch emptied the window. A property-level membership walker (filter_properties_reference_tracked_membership, mirroring the existing quantity walker filter_properties_reference_tracked_quantity) now feeds the FreeCastFromZones detection arm (CR 608.2c).

MED — parse-diff reconciliation: the three previously unclaimed cards (Forgotten Lore, Shrouded Lore, Search for Survivors) are now claimed and justified in the PR body's "Claimed parse impact": they ride the subject-layer chooser rebind (seam 2) — the same delegated-chooser grammar family ("an opponent chooses a card in your graveyard") — with their remaining clauses parsing via pre-existing arms.

…estore, targeted-chooser honesty gate

- Register WaitingFor::ChooseFromZoneOpponentChooser in waits_for_resolution_choice
  so the picker pause parks the FreeCastFromZones continuation instead of
  resolving it inline before any pick exists (CI shard-2: Priority instead of
  CastOffer on the 4-player path).
- Restore priority (set_priority) before draining the parked continuation on the
  picker answer arm's empty-pool path, mirroring ChooseClashOpponent (review HIGH).
- Gate the counted free-cast reroute on constraint.is_none() so a parsed cast
  constraint is never silently dropped (review HIGH).
- Gate the subject-layer chooser rebind on subject.target.is_none(): 'target
  opponent chooses …' (Forgotten/Shrouded Lore) binds the chooser to a chosen
  target slot Chooser::Opponent cannot represent — keep the honest fall-through
  (review MED); pin both directions with parser tests, claiming Search for
  Survivors' untargeted form.
- Render the engine-supplied candidate order in ZoneOpponentChooserModal instead
  of re-sorting by client seatOrder (review MED); pin with a shuffled-input test.
- Register local.zone-opponent-chooser-unsupported in manabrew-compat's
  capability registry, 18→19 entries (review MED).
- Two-upkeep regression also asserts the declined card stays exile-linked
  (resolution scoping, not link pruning).

Copy link
Copy Markdown
Owner Author

Round-3 review addressed in 92c56e7 (all five items, plus the outstanding shard-2 CI failure root-caused on this head).

HIGH — empty member-pool continuation drops priority: fixed. The ChooseZoneOpponentChooser answer arm's empty-pool drain discarded player and called resume_pending_continuation_if_priority while waiting_for was still the picker pause — the drain is gated on WaitingFor::Priority, so the state wedged. The arm now binds player and calls set_priority(state, player) before resuming, mirroring the adjacent ChooseClashOpponent arm's exact sequence.

HIGH — counted free-cast arm drops a parsed cast constraint: fixed. The reroute to try_parse_counted_free_cast_from_exiled_this_way is now gated on constraint.is_none()Effect::FreeCastFromZones has no constraint channel, so a constrained counted form falls through to the strict handling instead of silently discarding the restriction.

MED — modal must render engine order: fixed. ZoneOpponentChooserModal no longer re-sorts candidates by client seatOrder; it renders the engine-supplied order verbatim (the engine already emits opponents in turn order from the controller). The modal test pins the render order against a deliberately shuffled input.

MED — capability registry: fixed. local.zone-opponent-chooser-unsupported is registered in manabrew-compat's UNSUPPORTED_PROTOCOL_CAPABILITIES (18 → 19 entries), well-formedness test updated.

MED — parse-diff reconciliation with tests: resolved by tightening the subject-layer rebind rather than claiming all three cards. "Target opponent chooses …" (Forgotten Lore, Shrouded Lore) lowers "target opponent" to the same bare opponent filter as "an opponent", but the subject carries target: Some(_) — a chosen target slot Chooser::Opponent cannot represent, the same collapse the exile-anaphor arm deliberately rejects (76cf4d1). The rebind is now gated on subject.target.is_none(), reverting both Lores to the honest Effect::Unimplemented fall-through, while Search for Survivors' untargeted "An opponent chooses a card at random in your graveyard." is claimed. Both directions are pinned by new parser tests (search_for_survivors_opponent_chooses_at_random_in_your_graveyard, target_opponent_chooses_in_your_graveyard_keeps_honest_fall_through). Expected parse diff on this head: exactly three cards (Plargg and Nassari, Author of Shadows, Search for Survivors).

Shard-2 CI failure (4-player test, Priority instead of CastOffer): root-caused as a second defect on the picker path — WaitingFor::ChooseFromZoneOpponentChooser was never registered in waits_for_resolution_choice, so the chain machinery treated the choose head as not paused and resolved the FreeCastFromZones sub-ability inline before any pick existed, leaving nothing parked for the answer handlers to drive. Registered alongside ChooseFromZoneChoice; the picker path now parks the free-cast continuation exactly like the direct 2-player path. (CI Failures item 6 in the PR body.)

(CodeRabbit nitpick): the two-upkeep regression additionally asserts the declined card remains exile-linked to the source after upkeep 1, pinning that upkeep 2's exclusion comes from resolution scoping rather than link pruning.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant