fix(engine): Coalition Victory only wins if you control a land of each basic type and a creature of each color

[ultrahighsuper]

Problem

Coalition Victory (Time Spiral) reads:

You win the game if you control a land of each basic land type and a creature of each color.

The trailing "if …" condition was not parsed, so the spell reduced to a bare Effect::WinTheGame (condition: null) — casting Coalition Victory won the game immediately, regardless of the board.

Fix

Teach the condition parser the two "of each …" clauses and compose them. parse_control_conditions now recognizes "you control a land of each basic land type and a creature of each color" and yields:

And[ QuantityComparison(BasicLandTypeCount{You} >= 5),
     QuantityComparison(DistinctColorsAmongPermanents{creatures you control} >= 5) ]

reusing the existing domain (BasicLandTypeCount) and distinct-color (DistinctColorsAmongPermanents) quantities — no new condition types. The two sub-clauses are factored into standalone combinators (parse_land_of_each_basic_land_type, parse_creature_of_each_color) so they cover every "of each basic land type" / "of each color" clause, not just this card (build-for-the-class). WinTheGame is already trailing-condition-eligible, so the parsed condition attaches to the spell and is checked once on resolution.

• CR 104.2b: an effect may state that a player wins the game.
• CR 608.2h: the game-state information the condition needs is determined once, as the spell resolves.

Tests (runtime, real cast pipeline)

• coalition_victory_does_not_win_without_the_board — casting with an empty board does not win (condition unmet). Fails on the old bare-WinTheGame parse, which won unconditionally (revert-probed locally: the assertion fails when the combinator arm is removed).
• coalition_victory_wins_with_full_domain_and_all_colors — controlling a land of each basic type (Plains/Island/Swamp/Mountain/Forest) and a creature of each color (W/U/B/R/G), casting wins the game.

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[matthewevans]

[MED] The new Coalition Victory comments cite the wrong Comprehensive Rules for this effect-based win condition. Evidence: crates/engine/src/parser/oracle_nom/condition.rs:2855, crates/engine/src/parser/oracle_nom/condition.rs:2872, crates/engine/src/parser/oracle_nom/condition.rs:2889, and crates/engine/src/parser/oracle_nom/condition.rs:2912 cite CR 603.4, which docs/MagicCompRules.txt defines only for triggered abilities with intervening “if” clauses; crates/engine/src/parser/oracle_nom/condition.rs:2889 and crates/engine/src/parser/oracle_nom/condition.rs:2912 also cite CR 104.2a, but that rule is last-player-standing while CR 104.2b is the effect-states-you-win rule. Why it matters: this repo requires verified CR annotations, and wrong rule citations create false evidence around rules-sensitive parser/engine code. Suggested fix: remove CR 603.4 here and cite the effect/resolution rules that actually apply, e.g. CR 104.2b plus CR 608.2c/CR 608.2h for evaluating the conditional game-state check during resolution.

[github-actions]

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

1 card(s) · ability/WinTheGame · field conditional: you control you control land Plains and you control you control land Island and you control you control land Swamp and … → basic land types among lands you control ≥ 5 and # of colors among you control creature ≥ 5

Examples: Coalition Victory

1 card(s) had Oracle-text changes (errata/reprint) — excluded as non-parser.

[ultrahighsuper]

Thanks — you're right, and I verified against docs/MagicCompRules.txt:

• 104.2a is "A player still in the game wins the game if that player's opponents have all left" (last-player-standing) — not this effect. 104.2b ("An effect may state that a player wins the game") is the correct rule.
• 603.4 is the intervening-"if" rule, which by its own text applies only to an "if" immediately following a trigger condition; Coalition Victory is a spell, so it does not apply. 608.2h ("If an effect requires information from the game … the answer is determined only once, when the effect is applied") is the rule for evaluating the win condition as the spell resolves.

Updated in 05b6a39bb:

• Win-condition sites (parse_control_land_each_basic_and_creature_each_color + the dispatch arm) now cite CR 104.2b + CR 608.2h.
• The "of each basic land type" / "of each color" sub-combinators keep only their subject-matter rules (CR 205.3i, CR 105.2) and drop the erroneous CR 603.4.

Comment-only change; no logic or test behavior changed.

[matthewevans]

Reviewed current head 05b6a39bb73bdaec85c80e439d0e318a5539cda2. The CR citation blocker from the prior review is addressed, the parse-diff is scoped to Coalition Victory, and I did not find a remaining implementation blocker.

I’m still holding approval/enqueue because this PR is missing the current AI-CONTRIBUTOR proof/template sections that the maintainer review loop requires before queueing. Please fill in the template sections for files changed / track / LLM disclosure / verification, including the exact test evidence you already summarized here. Once that proof is in the PR body, this can be reconsidered without another code-path blocker from me.

[matthewevans]

Current-head review on 82e1e7a3a142eafb45897cc33b377720739db738:

I rechecked the latest push. The implementation still looks scoped and compositional: it parses the Coalition Victory condition by composing reusable “land of each basic land type” and “creature of each color” condition helpers, and the runtime regression covers both the false board and the satisfied board through the cast pipeline.

I am still holding approval/enqueue because the AI-contributor proof/template sections are missing and checks were not complete for this exact head when reviewed. Please add the current-head summary/files/verification/LLM disclosure and let CI finish.

[ultrahighsuper]

Friendly nudge: the CHANGES_REQUESTED on the CR annotations was addressed in 82e1e7a3a (win-condition sites now cite CR 104.2b + CR 608.2h; the of each … sub-combinators keep CR 205.3i/CR 105.2 and drop the erroneous CR 603.4). All 11 checks are green and the branch is rebased current/mergeable-clean. Could you re-review when you have a moment? Happy to adjust further if anything else stands out.

[matthewevans]

Reviewed current head 82e1e7a3a142eafb45897cc33b377720739db738.

The prior CR blocker is addressed: the Coalition Victory win condition cites the effect-based win and resolution-time information rules, and the of each condition helpers keep their color/basic-land-type CR grounding. The implementation is scoped to the condition parser/runtime seam and composes the existing conditional WinTheGame path rather than adding card-specific resolution logic.

Parse-diff evidence is scoped to Coalition Victory only, changing the WinTheGame.conditional field from absent to the expected “basic land types among lands you control >= 5 and colors among creatures you control >= 5” condition. The runtime regression covers the false board and satisfied board through the cast pipeline. The PR body still has template gaps, but the current comments and green current-head checks provide enough concrete proof for this maintainer review.

[matthewevans]

[MED] This PR is now stale and would add duplicate/narrow Coalition Victory parser machinery beside the broader implementation already on current main. Evidence: current origin/main already handles the shared you control a <noun> of each <dimension> seam in crates/engine/src/parser/oracle_effect/conditions.rs and has cast-pipeline coverage in crates/engine/tests/coalition_victory_win_condition.rs; this PR still adds a separate Coalition-specific path in crates/engine/src/parser/oracle_nom/condition.rs plus a weaker duplicate runtime test. Why it matters: merging this as-is would leave two parallel authorities for the same Oracle class, and the PR was behind enough that its raw diff would also drop unrelated current-main parser/test work. Suggested fix: do not queue this branch as-is; close/supersede it with the current-main implementation, or rebuild it on current main with only a demonstrably non-duplicative regression.

I disabled auto-merge to prevent this stale branch from entering the queue while the current-main overlap is resolved.

[matthewevans]

[MED] This head still adds a second, narrower Coalition Victory condition parser beside the broader implementation already on main. Evidence: this PR adds parse_land_of_each_basic_land_type, parse_creature_of_each_color, and parse_control_land_each_basic_and_creature_each_color in crates/engine/src/parser/oracle_nom/condition.rs, while current origin/main already owns the same phrase class in crates/engine/src/parser/oracle_effect/conditions.rs via parse_you_control_of_each_condition_text / parse_of_each_clause, including the permanent of each color sibling. Why it matters: merging this would leave duplicate parser machinery for one card-specific route, and the new runtime test is covered by main's stronger crates/engine/tests/coalition_victory_win_condition.rs cases. Suggested fix: close/rebase this PR to drop the duplicate parser/test additions, or rework only if it extends the existing oracle_effect/conditions.rs of-each recognizer without duplicating it.

Evidence sources checked: current PR head 31ca2e56aacde096dcdf77821e0690981db01b7c, local git diff origin/main...HEAD, the parse-diff sticky showing only Coalition Victory, and current origin/main files. Confidence: high. Self-challenge: this would be contradicted by a production path that requires StaticCondition parsing for the same Oracle phrase and is not served by the existing AbilityCondition trailing-if recognizer; I did not find such a path in this PR's cast-pipeline test or current main's parser tests.

[matthewevans]

Closing this due to overlap of functionality in main.