Parse counted "you had N or more [type] enter this turn" conditions (Park Heights Pegasus)#5087
Conversation
The condition combinator's "you had ..." branch hardcoded a count of 1 and
required a leading article, so the counted threshold surface — "you had two
or more creatures enter the battlefield under your control this turn" (Park
Heights Pegasus) — failed to parse. Downstream, the trailing effect gate was
dropped entirely and the card drew a card on every combat-damage trigger,
unconditionally instead of only when two or more creatures had entered.
Teach parse_entered_this_turn the counted "N or more [type] enter ... this
turn" form by extracting a shared parse_or_more_entered_count combinator used
by both the bare past-tense surface ("N or more creatures entered ...") and
the "you had"-auxiliary present-tense surface ("you had N or more creatures
enter ..."). Both denote the same CR 400.7 this-turn ETB tally, so the count
and suffix are the only axes that vary. The combinator emits
QuantityComparison { EnteredThisTurn{creature/You} >= N }, which the effect
pipeline already bridges onto the draw as AbilityCondition::QuantityCheck and
the runtime already evaluates — no new engine plumbing required.
Tests: a building-block condition test asserting the GE 2 threshold, plus a
runtime revert-probe (crates/engine/tests) confirming the parsed draw is
suppressed when only one creature entered this turn (the dropped condition
would draw unconditionally) and fires when two entered.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request resolves an issue where the parser dropped the 'two or more' condition under the 'you had' auxiliary (e.g., on Park Heights Pegasus), causing the effect to trigger unconditionally. It introduces a shared helper function parse_or_more_entered_count in condition.rs to correctly parse 'N or more [type] ' into a greater-than-or-equal-to (GE) threshold comparison. Additionally, a unit test and a comprehensive integration regression test suite have been added to verify both the parsing and runtime behavior of this condition. There are no review comments, and the changes are architecturally sound and compliant with the repository style guide.
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
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
[MED] The new parser path uses the live-board EnteredThisTurn quantity instead of the existing battlefield-entry history authority. Evidence: crates/engine/src/parser/oracle_nom/condition.rs:6258 builds QuantityRef::EnteredThisTurn, whose resolver only counts objects still in Zone::Battlefield with entered_battlefield_turn == Some(state.turn_number) (crates/engine/src/game/quantity.rs:2530). The repo already has QuantityRef::BattlefieldEntriesThisTurn for “who had N or more [type] enter ... this turn,” backed by battlefield_entries_this_turn snapshots that count entries even after the object leaves (crates/engine/src/types/ability.rs:4809). Why it matters: Park Heights Pegasus should still see a creature that entered under your control this turn even if it died, bounced, or was sacrificed before the combat-damage draw condition resolves; the current implementation incorrectly suppresses the draw in that legal game state. Suggested fix: lower the counted “you had N or more [type] enter ... this turn” condition to the battlefield-entry snapshot quantity/condition authority and add a regression where one of the two entering creatures has left the battlefield before the draw condition resolves.
Also, the parser-diff sticky is still absent on this head; please let the current CI produce <!-- coverage-parse-diff --> before this is reconsidered.
Parse changes introduced by this PR · 14 card(s), 7 signature(s) (baseline: main
|
…rn threshold
Per review: the counted "you had N or more [type] enter the battlefield under
your control this turn" condition must count entries even after the object has
left. The prior implementation used QuantityRef::EnteredThisTurn, whose
resolver only counts objects still on the battlefield with
entered_battlefield_turn == current turn, so Park Heights Pegasus would wrongly
suppress the draw when one of the entering creatures died, bounced, or was
sacrificed before the combat-damage draw resolved.
Lower the shared parse_or_more_entered_count combinator (both the past-tense
"N or more ... entered ..." and the present-tense "you had N or more ... enter
..." surfaces) to QuantityRef::BattlefieldEntriesThisTurn { player:
PlayerScope::Controller, filter } — the battlefield_entries_this_turn snapshot
authority (CR 403.3 + CR 608.2h) that persists across the object leaving.
PlayerScope::Controller scopes the tally to "under your control", so the type
filter carries no controller of its own (mirroring the existing "who had N or
more ... enter" for-each authority).
Tests: update the two counted-condition unit assertions to the snapshot
authority, and add a runtime regression where both entering creatures have
already left the battlefield before the draw resolves — the draw still fires
(it would incorrectly fail under the live-board EnteredThisTurn count).
|
Thanks — good catch, fixed in Authority switched to the battlefield-entry snapshot. The counted This applies to both counted surfaces the shared combinator serves — the present-tense Regression added ( The parser-diff sticky should populate once this head's CI finishes (it was still mid-run at the previous review). |
|
Reviewed the updated The previous code concern looks resolved: the counted I am still holding this from approval/enqueue for process proof, not for CI: the PR body is missing the required AI-contributor proof sections ( |
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
Reviewed current head fb2e8344bd5377f2baed5ccd9cf85fc042068b2f.
The previous runtime concern looks addressed: the counted you had N or more ... enter ... this turn path now uses the battlefield-entry snapshot authority, and the runtime regression covers entered permanents that have already left before the condition resolves.
I still need one cleanup before approval. The new CR annotations cite the wrong rule for the historical look-back behavior. In crates/engine/src/parser/oracle_nom/condition.rs and the Park Heights Pegasus runtime test, the comments describe objects still counting after leaving the battlefield but cite CR 608.2h; in the current rules that look-back exception is CR 608.2i, while the battlefield-zone rule should be cited separately. Please update the annotations to cite the correct rule pair for historical ETB membership, and keep CR 608.2h only where the code is specifically relying on information read during resolution.
Also please refresh the PR body so the proof matches the current implementation; it still describes the old EnteredThisTurn lowering.
2 participants
Problem
Park Heights Pegasus (in
docs/parser-misparse-backlog.md):The trailing effect gate is an effect-level conditional (CR 608.2c) on a CR 400.7 this-turn ETB threshold. It was being dropped entirely: the card drew a card on every combat-damage trigger, unconditionally, instead of only when two or more creatures had entered under your control that turn.
Root cause
parse_entered_this_turn(oracle_nom/condition.rs) — the combinator behindparse_inner_condition, the single authority for game-state conditions — handled the"you had ..."surface only through a subject path that hardcoded a count of 1 and required a leading article (a/an/another). The counted"you had two or more creatures enter ..."surface matched neither the article path nor the bare past-tense"N or more [type] entered ..."branch, so it returnedErr, the trailingifnever re-homed, and the draw resolved unconditionally.Fix
Extract a shared
parse_or_more_entered_countcombinator and use it for both surfaces:"N or more creatures entered the battlefield under your control this turn""you had"-auxiliary present-tense —"you had N or more creatures enter the battlefield under your control this turn"Both denote the same CR 400.7 this-turn ETB tally, so count and suffix are the only axes that vary — one combinator, no duplication (the past-tense branch previously inlined this logic).
The combinator emits
StaticCondition::QuantityComparison { EnteredThisTurn{creature/You} >= N }. The rest of the pipeline was already in place:static_condition_to_ability_conditionbridgesQuantityComparison → AbilityCondition::QuantityCheck,strip_suffix_conditionalre-homes the trailingifonto the draw effect,QuantityCheckandQuantityRef::EnteredThisTurn.So no new engine plumbing — this is a pure parser-completeness fix that unlocks the whole "you had N or more [type] enter … this turn" class, not just this card.
Tests
condition.rs):test_you_had_two_or_more_enter_this_turnasserts the counted surface yieldsEnteredThisTurn{creature/You} >= 2(not the singular>= 1the article-only path produced).crates/engine/tests/park_heights_pegasus_draw_condition_runtime.rs): parses the real draw clause through the productionparse_effect_chainpath and resolves it under P0.>= 2gate suppresses the draw. Verified this test fails without the fix (the dropped condition draws unconditionally —left: 1, right: 0).CR
FilterProp::EnteredThisTurn/QuantityRef::EnteredThisTurnannotations).if) clause on an effect.