feat(parser): compound-quantified dual-subject become effect (CR 611.3)

[claytonlin1110]

Summary

Mirrors the structural pattern from #5219 (compound "all <X> and all <Y>" subject distribution) on the effect layer for become-verb compounds:

• Factors peel_compound_all_quantified_conjuncts into static_helpers.rs as the shared " and all " seam authority
• Refactors parse_compound_all_subjects_filter / parse_compound_all_subjects_land_filter in type_change.rs to reuse the peeler
• Adds try_parse_compound_all_subjects_become_clause dispatched before try_parse_subject_become_clause

Fixes Nightcreep misparsing: all creatures become black and all lands become Swamps now emits two static_abilities (Creature → SetColor(Black), Land → SetBasicLandType(Swamp)) instead of dropping the land conjunct into description.

Closes #5377

Test plan

• cargo fmt --all — clean
• cargo check -p engine — clean
• cargo test -p engine --lib compound_all_subjects_become — blocked locally by Windows link.exe; CI should verify

[gemini-code-assist]

Code Review

This pull request implements robust support for player-scope hexproof and shroud (CR 702.11c, CR 702.18a), introducing a single-authority targeting check that correctly handles Two-Headed Giant teammates. It also refactors compound-subject keyword-grant parsing to decompose player-applicable keywords into separate object-half and player-half static definitions, and adds a shared helper to parse compound-quantified dual-subject become effects. Feedback on the changes highlights a high-severity bug in the parser where matching "become " using tag fails when the verb is "becomes ", which can be resolved by using alt to match both forms.

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.

[gemini-code-assist]

[HIGH] tag("become ") will fail to match when the verb is becomes because "becomes " does not start with "become ". Additionally, avoid swallowing parsing errors with .ok()? as it masks critical bugs; instead, propagate the parser error directly.

Evidence: crates/engine/src/parser/oracle_effect/subject.rs:634-636.

Why it matters: This breaks the parser's explicit support for the singular "becomes" verb form, causing any compound clause containing "becomes" to fail parsing. Swallowing errors with .ok()? makes debugging parser failures extremely difficult.

Suggested fix: Use nom::branch::alt to match either "become " or "becomes ", and propagate the error instead of converting it to an Option with .ok().

        nom::branch::alt::<_, _, _, OracleError<'_>>((tag("become "), tag("becomes ")))
            .parse(predicate_lower.as_str())?;

References

1. Avoid swallowing all errors (e.g., using .ok()) when only specific errors are expected to be handled or ignored. Propagate unexpected errors (such as invariant violations or invalid actions) to prevent masking critical bugs.

[github-actions]

Parse changes introduced by this PR

Baseline pending for 236a05639a6f3d7da2c2e0cdd4580f3a51eaf0d8 — this populates once main publishes its coverage snapshot (a few minutes after that commit landed).

[matthewevans]

[MED] The new compound-become parser accepts becomes in the split but then rejects that same verb before building the clause. Evidence: crates/engine/src/parser/oracle_effect/subject.rs:626 accepts " becomes ", but crates/engine/src/parser/oracle_effect/subject.rs:634 immediately requires tag("become "); the added coverage at crates/engine/src/parser/oracle_effect/tests.rs:2236 only exercises plural "become". Why it matters: any singular/third-person conjunct that reaches this handler falls back instead of producing the intended per-conjunct static, so the new building block is narrower than its own grammar claims. Suggested fix: accept both become and becomes at the validation/build boundary, and add a test with a becomes conjunct.

[matthewevans]

[MED] Player hexproof uses the source object's stored controller instead of the targeting call's controller. Evidence: crates/engine/src/game/targeting.rs:16 carries source_controller, but the player-target branch calls player_cannot_be_targeted_by(state, player.id, source_id) without it at crates/engine/src/game/targeting.rs:204, and crates/engine/src/game/static_abilities.rs:1436 recomputes the controller from state.objects[source_id]; the repo already has an explicit override path at crates/engine/src/game/targeting.rs:57. Why it matters: when the ability/spell controller differs from the source object record, player hexproof can evaluate the wrong opponent relationship and allow an illegal player target. Suggested fix: pass the authoritative source_controller into player_cannot_be_targeted_by from all player-target enumeration sites and use that for the CR 702.11c opponent check.

[matthewevans]

[MED] Player hexproof uses the source object's stored controller instead of the targeting call's controller. Evidence: crates/engine/src/game/targeting.rs:16 carries source_controller, but the player-target branch calls player_cannot_be_targeted_by(state, player.id, source_id) without it at crates/engine/src/game/targeting.rs:204, and crates/engine/src/game/static_abilities.rs:1436 recomputes the controller from state.objects[source_id]; the repo already has an explicit override path at crates/engine/src/game/targeting.rs:57. Why it matters: when the ability/spell controller differs from the source object record, player hexproof can evaluate the wrong opponent relationship and allow an illegal player target. Suggested fix: pass the authoritative source_controller into player_cannot_be_targeted_by from all player-target enumeration sites and use that for the CR 702.11c opponent check.

[matthewevans]

Maintainer sign-off: current head addresses the become/becomes and source-controller targeting blockers; parser capability is at the right seam with discriminating coverage.