fix: preserve colors_spent_to_cast for CR 603.4 Adamant ETBs (#5943)

[andriypolanski]

Closes #5943

Summary

Discord report: Emptiness evoked with {W}{W} or {B}{B} (via hybrid Evoke {W/B}{W/B}) does not fire the matching color-spent ETB.

The parser AST was already faithful — two ChangesZone ETBs gated by TriggerCondition::ManaColorSpent { minimum: 2 } plus the synthetic Evoke sac. The defect was runtime: after trigger collection succeeded (colors still present), clear_post_collection_transients zeroed colors_spent_to_cast on every object. CR 603.4 re-checks the intervening-if at resolution via ExactLive (permanent still on the battlefield), so Adamant-style ETBs silently did nothing.

Root cause

mana_spent_to_cast_amount was already treated as a durable cast fact (Hungry Graffalon / Topiary Lecturer), but colors_spent_to_cast was still cleared with the transient mana_spent_to_cast boolean. That asymmetric cleanup breaks every ManaColorSpent intervening-if that re-checks while the source is ExactLive — Evoke Emptiness is the reported instance of the Adamant / “if {C} was spent to cast it” class.

Changes

• crates/engine/src/game/triggers.rs — stop clearing colors_spent_to_cast in clear_post_collection_transients; document CR 603.4 / ExactLive rationale; unit test that ManaColorSpent survives the clear.
• crates/engine/src/game/game_object.rs — align field docs: color tally is durable like mana_spent_to_cast_amount.
• Integration — evoke Emptiness paying WW (GY return + sac) and BB (three −1/−1 + sac).

Test Plan

• cargo fmt --all -- --check
• cargo test -p engine --lib -- adamant_intervening_if_survives_post_collection_transient_clear
• cargo test -p engine --test integration -- emptiness_evoke

Summary by CodeRabbit

• Bug Fixes
  • Preserved durable per-color “colors spent to cast” during post-trigger and intervening-if checks, while clearing transient cast markers (e.g., “mana spent to cast”).
  • Ensured cast-payment provenance is cleared when permanents leave the battlefield and when spells leave the stack to non-battlefield zones, preventing stale re-entry behavior.
  • Fixed Emptiness evoke/spent ETBs for both white and black mana paths across resolve, counter, blink, and reanimate scenarios.
• Tests
  • Added regression integration coverage for Emptiness evoke “color-spent,” including blink and counter/re-entry cases.
  • Added a unit test for battlefield-exit clearing of per-color tallies.
• Documentation
  • Clarified “colors spent to cast” vs transient cast tracking semantics.

[superagent-security]

🚨 Contributor flagged. Click here for more info: Superagent Dashboard

[coderabbitai]

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: 72e0b046-c8ac-4753-a966-e84a98fb73b3

📥 Commits

Reviewing files that changed from the base of the PR and between d91ae94 and e532b54.

📒 Files selected for processing (4)

• crates/engine/src/game/game_object.rs
• crates/engine/src/game/triggers.rs
• crates/engine/src/game/zones.rs
• crates/engine/tests/integration/issue_5943_emptiness_evoke_color_spent.rs

🚧 Files skipped from review as they are similar to previous changes (2)

• crates/engine/src/game/game_object.rs
• crates/engine/src/game/triggers.rs

---

📝 Walkthrough

Walkthrough

The engine preserves per-color mana payment tallies after transient cleanup, clears them when objects leave relevant zones, and validates intervening-if checks plus Emptiness Evoke effects paid with white or black mana.

Changes

Mana color cast facts

Layer / File(s)	Summary
Preserve cast payment tallies crates/engine/src/game/game_object.rs, crates/engine/src/game/triggers.rs	Post-collection cleanup clears transient mana state but retains colors_spent_to_cast; documentation and tests cover the updated ManaColorSpent behavior.
Clear cast provenance at zone exits crates/engine/src/game/game_object.rs, crates/engine/src/game/zones.rs	A shared helper clears cast-payment provenance during battlefield exits and non-battlefield stack exits, with unit coverage for battlefield reset behavior.
Validate Emptiness Evoke effects crates/engine/tests/integration/issue_5943_emptiness_evoke_color_spent.rs, crates/engine/tests/integration/main.rs	Integration tests verify white- and black-mana Evoke effects and confirm stale color-spent state is cleared across blink, counter, and reanimation paths.

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

Possibly related PRs

• phase-rs/phase#6352: Modifies overlapping cast-payment stamp lifecycle and trigger-clearing logic.

Suggested reviewers: matthewevans

🚥 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 matches the main fix: preserving colors_spent_to_cast for CR 603.4 Adamant/Emptiness ETBs.
Linked Issues check	✅ Passed	The changes address #5943 by preserving cast-color provenance and adding tests for Emptiness with {W}{W}/{B}{B} evoke paths.
Out of Scope Changes check	✅ Passed	The added helper, cleanup updates, and tests all support the Emptiness cast-color tracking fix.
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.}

[matthewevans]

Changes requested — the current-head fix retains cast colors across a new-object boundary.

🔴 Blocker

clear_post_collection_transients now deliberately preserves colors_spent_to_cast so the original ETB's intervening-if can re-check it. That is the right lifetime for the original object, but GameObject::reset_for_battlefield_exit does not clear the tally (crates/engine/src/game/game_object.rs:2291), while the zone-move path calls that reset for every battlefield exit (crates/engine/src/game/zones.rs:350-351). A creature cast with {W}{W}, then blinked or returned without being cast again, can therefore enter with the old white tally and incorrectly satisfy a later ManaColorSpent ETB.

Please clear the cast-color tally at the battlefield-exit/new-object boundary (alongside the other cast-entry provenance), while retaining it through the original trigger-resolution window. Add a registered runtime regression that casts with the qualifying color, leaves the battlefield, re-enters without a new cast, and proves the color-spent condition does not fire.

The two existing Emptiness tests cover direct cast-to-ETB behavior only; they do not exercise the required reset boundary. CI/CodeRabbit and the engine parse-diff are also not yet settled for this head, so they remain independent merge gates.

Recommendation: update this PR with the exit-boundary reset and regression test, then request re-review.

[github-actions]

Parse changes introduced by this PR

✓ No card-parse changes detected.

[andriypolanski]

Could you please let me know how to remove Contribution trust flagged mark?

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/engine/tests/integration/issue_5943_emptiness_evoke_color_spent.rs`:
- Around line 146-163: The test’s blink sequence bypasses the replacement-aware
zone-change pipeline by directly calling zones::move_to_zone and
process_triggers. Update the test around the emptiness blink to use a production
effect or GameAction that emits ProposedEvent::ZoneChange for exile and return,
then let the engine collect replacements and triggers through its normal
execution path while preserving the colors_spent_to_cast assertions.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 78411397-88dd-47da-8a42-a3a96bc66ced

📥 Commits

Reviewing files that changed from the base of the PR and between 9a0493b and d58fdcf.

📒 Files selected for processing (2)

• crates/engine/src/game/game_object.rs
• crates/engine/tests/integration/issue_5943_emptiness_evoke_color_spent.rs

[matthewevans]

Changes requested — the battlefield-exit repair is correct, but cast-color provenance still leaks across a stack-to-nonbattlefield new-object boundary.

🔴 Blocker

[HIGH] colors_spent_to_cast is cleared only on a battlefield exit, not when a cast spell leaves the stack without becoming that permanent. Evidence: crates/engine/src/game/triggers.rs:4637-4641 preserves the tally in every zone while clearing only cast_from_zone outside battlefield/stack; crates/engine/src/game/zones.rs:339-352 invokes the new reset only for from == Zone::Battlefield; and game_object.rs:2327-2333 therefore cannot clear stack-to-graveyard/exile survivors. CR 400.7 says an object that changes zones is a new object with no memory of its prior existence; CR 400.7d only preserves cast-cost information for the permanent the spell becomes. Why it matters: a {W}{W} Emptiness countered to a graveyard and later returned without being cast retains the old tally and can incorrectly pass its white-spent ETB condition. Suggested fix: give cast-payment provenance one lifecycle authority that preserves it only for the stack spell and the permanent it becomes, clears it on stack-to-nonbattlefield transitions as well as battlefield exits, and add a countered-then-reanimated runtime regression.

🟡 Non-blocking

[MED] The blink regression reaches the raw delivery helper rather than a replacement-aware zone-change production entry. Evidence: crates/engine/tests/integration/issue_5943_emptiness_evoke_color_spent.rs:146-163 calls zones::move_to_zone and process_triggers directly; the unresolved CodeRabbit thread identifies the same gap. Why it matters: the test proves the reset helper but not its behavior through a real proposed zone-change/replacement path. Suggested fix: drive the blink through an existing production effect or ZoneMoveRequest pipeline while retaining the post-reentry no-trigger assertion.

Recommendation: extend the provenance lifecycle and its production-path regression, then request re-review.

[matthewevans]

Changes requested — the current head does not address the previously reported cast-provenance boundary.

🔴 Blocker

[HIGH] colors_spent_to_cast still survives a spell leaving the stack without becoming its permanent. Evidence: crates/engine/src/game/triggers.rs:4637-4641 preserves the tally for every zone, while crates/engine/src/game/game_object.rs:2327-2333 clears it only from reset_for_battlefield_exit; the only current-head delta is the test's explicit normal-cast choice. Why it matters: a {W}{W} Emptiness countered to a graveyard and later returned can inherit its prior cast colors, contrary to CR 400.7; CR 400.7d's exception is only for the permanent the spell becomes. Suggested fix: make cast-payment provenance clear on stack-to-nonbattlefield transitions as well as battlefield exits, and cover countered-then-reanimated behavior.

[HIGH] The blink regression still calls raw zones::move_to_zone and process_triggers directly. Evidence: crates/engine/tests/integration/issue_5943_emptiness_evoke_color_spent.rs:146-163; the current CodeRabbit finding identifies the same path. Why it matters: replacements and normal zone-change trigger collection are not exercised. Suggested fix: drive exile and return through the replacement-aware production zone-change pipeline.

Recommendation: implement both lifecycle and production-path regressions, then request re-review.

[matthewevans]

Blocking findings

1. colors_spent_to_cast is still only cleared by reset_for_battlefield_exit, which is called when from == Zone::Battlefield. A spell carries the tally while on the stack, so a countered spell moved Stack → graveyard/exile and later put onto the battlefield can still satisfy the Adamant ETB without being cast for that entry. CR 400.7 applies to the zone change, not only battlefield exits. Clear cast-bound state at the stack-to-nonbattlefield boundary too, and add a production-pipeline regression that counters the originally paid spell and reanimates/returns it; the current blink test only covers Battlefield → Exile → Battlefield.

2. The integration regression invokes engine::game::zones::move_to_zone directly for both blink legs. That validates the zone helper, but not the spell/effect resolution path the card behavior depends on. Please drive the blink through a real resolving effect/ability so the test would catch a future pipeline bypass.

The current head contains no contributor commit after the prior requested-changes review; this re-review is for the merge-main head update. The existing blocks remain unresolved.

[matthewevans]

Closing as superseded by #6352.

Both PRs close #5943. The current head here fixes the original ETB and zone-exit cases, but it still clones colors_spent_to_cast into spell copies: copy_spell.rs clones the source GameObject and clears cast_from_zone without clearing the color-payment tally. A copied qualifying spell can therefore satisfy ManaColorSpent, contrary to CR 707.10.

#6352 covers the same reported defect and resets cast-payment stamps at the spell-copy birth sites as well as the zone-identity boundaries, with regressions for those paths. Keeping one implementation avoids two competing lifecycle changes for the same issue.