fix(parser): route bare "you don't own" ownership suffix (Agent of Treachery)#3435
Open
galuis116 wants to merge 1 commit into
Open
fix(parser): route bare "you don't own" ownership suffix (Agent of Treachery)#3435galuis116 wants to merge 1 commit into
galuis116 wants to merge 1 commit into
Conversation
…eachery)
"At the beginning of your end step, if you control three or more
permanents you don't own, draw three cards." dropped the intervening-if:
parse_ownership_or_controller_suffix only recognized the "but don't own"
family (which requires a controller already set), so the bare
"you don't own" suffix was left unconsumed, parse_control_count_ge
returned a non-empty remainder, and try_extract_intervening discarded the
whole condition — the trigger drew three cards every end step
unconditionally.
Add a bare "you don't own" / "you do not own" arm to
parse_ownership_or_controller_suffix that pushes the existing
FilterProp::Owned { controller: Opponent } (runtime-evaluated as
owner != controller, i.e. "you don't own it"). It does not set the
controller — ownership is independent of control (CR 109.4); for
"you control N permanents you don't own" the controller is supplied
upstream by inject_controller_you, composing to
Typed(Permanent, controller: You, [Owned{Opponent}]). The end-step
trigger now carries QuantityComparison(count >= 3) and is gated per
CR 603.4. Parser-only; no new engine variant; the "but don't own"
path (Thieving Amalgam family) is untouched.
CR 108.3, CR 109.4. Adds parser + combinator tests asserting the
condition is carried (not dropped) and a no-regression test for the
"but don't own" shape.
Fixes phase-rs#3304
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3304.Agent of Treachery — "At the beginning of your end step, if you control three or more permanents you don't own, draw three cards." — drew three cards every end step, because the intervening-if condition was silently dropped (the trigger parsed with
condition: null).Root cause
parse_ownership_or_controller_suffixonly recognized the "but don't own" family (which additionally requires a controller already set). The bare "you don't own" suffix had no arm, soparse_type_phrase("permanents you don't own")left "you don't own" unconsumed →parse_control_count_gereturned a non-empty remainder →try_extract_intervening's boundary check failed → the entire condition was discarded.Fix (parser-only; no new engine variant)
Add a bare
"you don't own"/"you do not own"arm toparse_ownership_or_controller_suffixthat pushes the existingFilterProp::Owned { controller: Opponent }— runtime-evaluated asowner != controller, i.e. "you don't own it". The arm deliberately does not set the controller (ownership is independent of control, CR 109.4); for "you control N permanents you don't own" the controller is supplied upstream byinject_controller_you, composing toTyped(Permanent, controller: You, [Owned{Opponent}, InZone Battlefield])= "permanents you control but don't own".The end-step trigger now carries
QuantityComparison(ObjectCount{…} >= 3)and is gated at detection and resolution per CR 603.4 — so it draws only when the controller controls ≥3 permanents they don't own. The"but don't own"path (Thieving Amalgam family) is a separate code path and is untouched.Runtime
The fix consumes only previously-orphaned text; it also lets
Sentinel of Lost Lore/ similar "card you don't own …" object phrases consume the suffix (strictly additive — the parallel"you own"arm already exists).Tests
agent_of_treachery_end_step_draw_is_gated_by_not_owned_count— the end-step trigger'sconditionisSome(QuantityComparison{ GE, Fixed 3, ObjectCount{ filter } })(notNone), with the filterTyped(Permanent, controller: You)carryingFilterProp::Owned{ Opponent }; the execute body still draws; no parse warnings.parse_control_count_ge_permanents_you_dont_own— combinator unit (both "don't"/"do not"), empty remainder,Owned{Opponent}+controller: You.parse_type_phrase_but_dont_own_shape_unchanged_by_bare_arm— no-regression: "creature you control but don't own" still yields the unchangedAnd[Typed(You), Not(Owned{You})]shape (proves the bare arm is additive).(Reverting the new arm makes the first two tests fail with the exact dropped-condition signature; the no-regression test still passes.)
Verification
cargo +nightly test -p engine --lib "own": 297 passed;"dont_own": 8 passed;--test integration: 1065 passed (no fixture diff).cargo +nightly clippy -p engine --lib -- -D warnings: clean.rustfmt --check: clean. Parser-combinator gate: clean (the new arm uses onlytag/alt).CR references grep-verified against
docs/MagicCompRules.txt: 108.3 (owner), 109.4 (control ≠ ownership), 603.4 (intervening-if).