Skip to content

fix(parser): batch Grim Hireling combat-damage trigger (#1152)#3739

Merged
matthewevans merged 4 commits into
phase-rs:mainfrom
kiannidev:fix/issue-1152-grim-hireling
Jun 18, 2026
Merged

fix(parser): batch Grim Hireling combat-damage trigger (#1152)#3739
matthewevans merged 4 commits into
phase-rs:mainfrom
kiannidev:fix/issue-1152-grim-hireling

Conversation

@kiannidev

@kiannidev kiannidev commented Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Set batched = true on DamageDoneOnceByController triggers parsed from "whenever one or more creatures you control deal combat damage to a player".

Test plan

  • cargo test -p engine --lib grim_hireling
  • cargo test -p engine --lib trigger_one_or_more_creatures_you_control

Fixes #1152

@kiannidev @cursoragent
fix(parser): mark one-or-more combat damage triggers as batched (phas…
86f8b39 
…e-rs#1152)

Grim Hireling and similar triggers use CR 603.2c batched semantics when multiple creatures deal combat damage in the same step.

Co-authored-by: Cursor <cursoragent@cursor.com>
@kiannidev kiannidev requested a review from matthewevans as a code owner June 18, 2026 18:31
@gemini-code-assist

gemini-code-assist Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Note

Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported.

kiannidev and others added 2 commits June 18, 2026 21:07
@kiannidev @cursoragent
style(parser): format Grim Hireling trigger batch change for CI
38afdb6 
Co-authored-by: Cursor <cursoragent@cursor.com>
@kiannidev @cursoragent
fix(ci): refresh Tauri Cargo.lock for locked compile check
325367f 
Co-authored-by: Cursor <cursoragent@cursor.com>
@matthewevans

matthewevans commented Jun 18, 2026

Copy link
Copy Markdown
Member

VERDICT: approve-pending-CI

Reviewed head 325367f246878aaf19bbfe743a5c771aeb2e4b40 via the GitHub API diff.

The parser change is at the right seam for this bug: the existing DamageDoneOnceByController arm already recognizes the Grim Hireling / Professional Face-Breaker class, and the missing part was the CR 603.2c batched trigger flag. The new assertions are fail-on-revert for that parser output, and the existing aggregate combat-damage matcher code already carries the filtered source set for this trigger mode.

Holding formal approval/enqueue until CI is green and the head passes the normal repush guard.

@matthewevans matthewevans added the bug Bug fix label Jun 18, 2026
@matthewevans matthewevans self-assigned this Jun 18, 2026
matthewevans
matthewevans approved these changes Jun 18, 2026
View reviewed changes

@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.

Approved after green CI and repush guard at head 325367f.

@matthewevans matthewevans added this pull request to the merge queue Jun 18, 2026
@matthewevans matthewevans removed their assignment Jun 18, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jun 18, 2026
@matthewevans matthewevans enabled auto-merge June 18, 2026 20:43
@matthewevans matthewevans added this pull request to the merge queue Jun 18, 2026
Merged via the queue into phase-rs:main with commit f34ee0e Jun 18, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@matthewevans matthewevans matthewevans approved these changes

Assignees

No one assigned

Labels

bug Bug fix

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bunch of Errors resolving mutiple triggers from Grim Hireling — Game is freezing up and throwing bunch of errors when t…

2 participants

@kiannidev @matthewevans