Skip to content

fix(engine): Mockingbird copy ceiling uses cast-payment stamp (#6440) - #6737

Merged
matthewevans merged 2 commits into
phase-rs:mainfrom
claytonlin1110:fix/6440-mockingbird-chord
Jul 28, 2026
Merged

fix(engine): Mockingbird copy ceiling uses cast-payment stamp (#6440)#6737
matthewevans merged 2 commits into
phase-rs:mainfrom
claytonlin1110:fix/6440-mockingbird-chord

Conversation

@claytonlin1110

@claytonlin1110 claytonlin1110 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Mockingbird-class AmountSpentToCastSource copy ceilings now read the entering object's mana_spent_to_cast_amount via the same objects→liminal dual lookup used for source identity (CR 400.7d).
  • Uncast put-onto-battlefield (Chord of Calling) yields Some(0) instead of unconstrained None, so MV 2+ creatures are no longer legal copy targets.
  • PendingSpellResolution.actual_mana_spent is no longer consulted for this ceiling (hostile unit fixture proves Chord's payment cannot override the stamp).

Closes #6440

Test plan

  • cargo fmt --all
  • Unit: issue_6440_* (4) + typed_copy_limit_zero_stamp_is_some_zero_not_unconstrained
  • Integration: issue_6440_mockingbird_* (chord uncast, cast path, clone sibling, parse reach-guard)
  • CI clippy / test-engine

Validation Failures

Plan review completed clean via isolated agents (2 rounds). Implementation and /review-impl subagents could not be spawned (API usage limit); implementation was done in the orchestrator thread against the approved plan. Isolated implementation review did not run — please give the PR an extra human/architecture pass before merge.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed copy effects that rely on mana spent to cast a creature so uncast entries are constrained to a maximum mana value of zero (instead of being treated as unconstrained).
    • Copy mana-value ceilings are now taken from the entering object’s cast-payment “stamp,” including when the stamp is zero and for liminal entries; Clone without a limit remains unrestricted.
  • Tests

    • Added regression coverage for Mockingbird via Chord of Calling (uncast vs cast), and for Clone behavior, plus a new unit test covering the zero-stamp ceiling behavior.

…rs#6440)

Resolve AmountSpentToCastSource ceilings from the entering object's
mana_spent_to_cast_amount (objects/liminal dual lookup), including Some(0)
when never cast. Stop reading PendingSpellResolution.actual_mana_spent so
Chord put-into-play cannot open an unconstrained MV filter.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5830df47-80a8-4ccd-8a0d-701a500babab

📥 Commits

Reviewing files that changed from the base of the PR and between 155828c and 7b552b9.

📒 Files selected for processing (1)
  • crates/engine/tests/integration/main.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/engine/tests/integration/main.rs

📝 Walkthrough

Walkthrough

Mockingbird copy targeting now uses the entering object’s cast-payment stamp, including zero for uncast entries, instead of unrelated spell-resolution mana. Unit and integration tests cover cast, uncast, liminal, Clone, and parsing behavior.

Changes

Copy ceiling correction

Layer / File(s) Summary
Copy ceiling semantics
crates/engine/src/ai_support/copy.rs
Documentation and unit coverage define Some(0) as the constrained result for an AmountSpentToCastSource limit with a zero stamp.
Replacement-effect stamp propagation
crates/engine/src/game/engine_replacement.rs
apply_post_replacement_effect extracts the entering object’s cast-payment stamp, including liminal objects, and uses it for BecomeCopy target limits.
Mockingbird integration coverage
crates/engine/tests/integration/issue_6440_mockingbird_uncast_copy_ceiling.rs, crates/engine/tests/integration/main.rs
Integration tests cover uncast and cast Mockingbird behavior, Clone’s unconstrained behavior, oracle parsing, and test-module registration.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

Suggested reviewers: matthewevans

Sequence Diagram(s)

sequenceDiagram
  participant apply_post_replacement_effect
  participant EnteringObject
  participant copy_target_mana_value_ceiling
  participant CopyTargetChoice
  apply_post_replacement_effect->>EnteringObject: read mana_spent_to_cast_amount
  EnteringObject-->>apply_post_replacement_effect: return cast-payment stamp
  apply_post_replacement_effect->>copy_target_mana_value_ceiling: calculate BecomeCopy max_mana_value
  copy_target_mana_value_ceiling-->>apply_post_replacement_effect: return Some(stamp) or None
  apply_post_replacement_effect->>CopyTargetChoice: apply copy target limit
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main fix for Mockingbird's copy ceiling bug.
Linked Issues check ✅ Passed The changes directly address #6440 by using the entering object's cast-payment stamp and adding regression coverage for the Chord of Calling case.
Out of Scope Changes check ✅ Passed The added documentation, internal rename, and tests all support the reported Mockingbird copy-ceiling fix and do not appear unrelated.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

Parse changes introduced by this PR

✓ No card-parse changes detected.

@matthewevans matthewevans self-assigned this Jul 28, 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.

Current-head maintainer review complete. The replacement seam derives the Mockingbird ceiling from the entering object’s cast provenance, while the registered Chord → uncast/cast/recast regression covers the lifecycle and leaves copy behavior unchanged. No further findings.

@matthewevans matthewevans added the bug Bug fix label Jul 28, 2026
@matthewevans
matthewevans added this pull request to the merge queue Jul 28, 2026
@matthewevans matthewevans removed their assignment Jul 28, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Jul 28, 2026
@matthewevans matthewevans self-assigned this Jul 28, 2026
Co-authored-by: Clayton <claytonlin1110@gmail.com>

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

Current-head maintainer re-review complete. This port contains current main plus both independent integration module registrations; it does not alter the Mockingbird lifecycle behavior or its regression coverage. No further findings.

@matthewevans
matthewevans enabled auto-merge July 28, 2026 20:03
@matthewevans matthewevans removed their assignment Jul 28, 2026
@matthewevans
matthewevans added this pull request to the merge queue Jul 28, 2026
Merged via the queue into phase-rs:main with commit 1604a6f Jul 28, 2026
15 checks passed
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.

Mockingbird — If I put mockingbird into play off of chord of calling, I can copy creatures with mv 2.

2 participants