Skip to content

Recognize "there are no [type] on the battlefield" condition (Sarcomancy, Call to the Grave) - #4917

Merged
matthewevans merged 2 commits into
phase-rs:mainfrom
e11734937-beep:feat/no-permanents-on-battlefield-condition
Jul 2, 2026
Merged

Recognize "there are no [type] on the battlefield" condition (Sarcomancy, Call to the Grave)#4917
matthewevans merged 2 commits into
phase-rs:mainfrom
e11734937-beep:feat/no-permanents-on-battlefield-condition

Conversation

@e11734937-beep

Copy link
Copy Markdown
Contributor

Summary

Triggers/conditions of the form "there are no [type] on the battlefield" were dropped — the intervening-if parsed to condition: null, so cards like Sarcomancy ("At the beginning of your upkeep, if there are no Zombies on the battlefield, ~ deals 1 damage to you"), Drop of Honey, Porphyry Nodes, and Task Mage Assembly ("when there are no creatures on the battlefield, sacrifice ~") lost their gating condition.

The engine already lowered the subject-first form "no [type] are on the battlefield" (Call to the Grave) to ObjectCount(<filter>) == 0; only the existential there-form was missing.

Change

Extend parse_no_on_battlefield (crates/engine/src/parser/oracle_nom/condition.rs) to recognize both surface forms with one shared output:

  • "there are no <type> on the battlefield" (existential there-form) — new
  • "no <type> are on the battlefield" (subject-first) — existing

Both lower to StaticCondition::QuantityComparison { ObjectCount(<filter>) == 0 }. A prefix alt selects the surface form and its trailing anchor; <type> is parsed via the shared parse_type_phrase, so a subtype ("Zombies"), a card type ("lands"), or a token phrase ("Reflection tokens") scopes the emptiness check exactly. No controller restriction — "no creatures" counts any player's matching permanents (CR 603.4 intervening-if; CR 110.1).

Nom combinators only (alt/value/tag/take_until); no new condition type.

Tests

Added recognizer tests covering: the there-form ("there are no creatures / Zombies on the battlefield"), the subject-first form, card-type ("no lands"), the battlefield anchor requirement, and a guard that "there are no" is not confused with a count condition. All assert the ObjectCount == 0 lowering with the correct filter and no controller restriction.

Fails-before (there-form → dropped condition) / passes-after confirmed. Green: rustfmt, parser-combinator (Rule Zero) gate, clippy -D warnings, full engine suite, and cargo coverage (net gain, 0 regressions).

…ce condition

Extend parse_no_on_battlefield to handle the existential there-form
("there are no Zombies on the battlefield" - Sarcomancy; "there are no
creatures on the battlefield" - Drop of Honey, Porphyry Nodes, Task Mage
Assembly) alongside the existing subject-first form ("no creatures are on
the battlefield" - Call to the Grave). Both lower to the same
QuantityComparison(ObjectCount(<filter>) == 0) (CR 603.4 intervening-if +
CR 110.1); the <type> is parsed via parse_type_phrase so subtypes, card
types, and token phrases scope the emptiness check. No controller
restriction - any players matching permanents count. Previously the
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

Parse changes introduced by this PR

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

@matthewevans matthewevans self-assigned this Jul 2, 2026

@matthewevans matthewevans left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maintainer review: parser condition fix is scoped and idiomatic. It extends the existing battlefield-absence condition parser rather than adding a sibling shape, keeps the output as the existing ObjectCount == 0 predicate, and the parse-diff is limited to the expected Sarcomancy / Task Mage Assembly condition gains.

@matthewevans matthewevans added the bug Bug fix label Jul 2, 2026
@matthewevans
matthewevans enabled auto-merge July 2, 2026 16:30
@matthewevans matthewevans removed their assignment Jul 2, 2026
@matthewevans
matthewevans added this pull request to the merge queue Jul 2, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 2, 2026
@matthewevans matthewevans self-assigned this Jul 2, 2026
@matthewevans
matthewevans added this pull request to the merge queue Jul 2, 2026
@matthewevans matthewevans removed their assignment Jul 2, 2026
Merged via the queue into phase-rs:main with commit 25b2a8a Jul 2, 2026
11 checks passed
@e11734937-beep

Copy link
Copy Markdown
Contributor Author

Hi @matthewevans — quick labeling note on this one: it landed tagged only bug, but it adds a new parser capability (recognizing "there are no [type] on the battlefield" as a condition — Sarcomancy, Call to the Grave), which seems to be the same class as the card/condition adds you tagged enhancement/quality (e.g. #4812, #4900, #4906). Would enhancement be the right fit here? Totally your call — thanks for the guidance on labeling earlier.

@e11734937-beep

Copy link
Copy Markdown
Contributor Author

Hello @matthewevans,

Thanks for merging these, and for the careful reviews. A calibration question on the quality label so I can aim future PRs at it: the triage applied it to some of these minimal-churn parser fixes (#4812, #4903) but not to others of similar scope — e.g. #4804 (+65/−4, definite-article reveal), #4839 (+82/−0, shared-quality), and this one (#4917, a focused condition recognizer). Per the label's description ("high-quality minimal to no-churn PRs") they read similar to me.

Is there a distinction I'm missing, or just triage variance? Happy to match whatever bar you'd prefer. Thanks!

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

Labels

bug Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants