fix(engine): preserve mana-spent snapshots across spell ETB (#1156)

[kiannidev]

Summary

• Snapshot and restore mana_spent_source_snapshots, colors_spent_to_cast, and related cast-payment fields in CastLinkSnapshot during stack-to-battlefield delivery.
• Ensures Coin of Mastery ETB replacements can count artifact-source mana spent to cast the entering creature.

Fixes #1156

Test plan

• cargo test -p engine --lib artifact_mana_spent_on_self
• Cast a creature using Treasure mana with Coin of Mastery on the battlefield and verify extra +1/+1 counters

Made with Cursor

[gemini-code-assist]

Code Review

This pull request updates CastLinkSnapshot in crates/engine/src/game/zone_pipeline.rs to include fields for tracking details about the mana spent to cast, such as mana_spent_to_cast, mana_spent_to_cast_amount, colors_spent_to_cast, and mana_spent_source_snapshots. These fields are also correctly copied and restored within the deliver_replaced_zone_change function. As there were no review comments provided, I have no feedback to evaluate.

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]

[MED] This does not demonstrate a fix for the Coin of Mastery failure, and the added snapshot fields appear to be at the wrong/no-op seam. Evidence: crates/engine/src/game/zone_pipeline.rs:1558 now snapshots/restores the mana-spent fields around zones::move_to_zone, but GameObject::reset_for_battlefield_entry clears cast provenance/kicker/convoke at crates/engine/src/game/game_object.rs:1336-1352 and does not clear mana_spent_to_cast_amount, colors_spent_to_cast, or mana_spent_source_snapshots. The Coin-style ETB counter quantity is resolved before the move in extract_etb_counters_from_effect using QuantityContext { entering: Some(object_id), ... } (crates/engine/src/game/replacement.rs:3988-4018), and source-qualified mana reads the entering object snapshot list at crates/engine/src/game/quantity.rs:789-802; those snapshots are already populated during mana payment at crates/engine/src/game/casting.rs:10188-10220. The PR also changes only zone_pipeline.rs, so there is no fail-on-revert regression for issue #1156. Why it matters: this can merge a no-op while the reported “mana from an artifact source spent to cast it” path remains broken or unproven. Suggested fix: first add a regression that fails on origin/main for Coin of Mastery (or an equivalent Moved replacement using CastManaSpentMetric::FromSource { source_filter: Artifact }) through the real cast/payment/replacement path, then put the fix at the seam that makes that test pass; if the delivery snapshot is genuinely necessary, the test should fail when these four new CastLinkSnapshot fields are removed.

Confidence: high from code inspection; contradictory evidence would be a regression test that fails on origin/main and passes only with this CastLinkSnapshot expansion.

[kiannidev]

Thanks for the review — you were right that the CastLinkSnapshot mana-spent expansion is a no-op seam.

What I verified

• extract_etb_counters_from_effect resolves CastManaSpentMetric::FromSource { Artifact } against the entering spell via QuantityContext { entering: Some(object_id), ... } before zones::move_to_zone.
• pay_mana_cost already records mana_spent_source_snapshots at payment time (casting.rs ~10188–10220).
• reset_for_battlefield_entry clears cast provenance/kicker/convoke but does not clear mana_spent_source_snapshots (or mana_spent_to_cast_amount).
• Reverting the four CastLinkSnapshot fields does not change behavior.

What changed in 90ebcba

• Reverted the zone_pipeline.rs CastLinkSnapshot mana-spent snapshot/restore.
• Added integration tests driving cast → stack → resolve → ETB replacement with Treasure-tagged pool mana (issue_1156_coin_of_mastery.rs).
• Added a replacement-layer unit test artifact_mana_spent_on_self_resolves_against_entering_object mirroring Coin’s ChangeZone + FromSource { Artifact } shape.

Test plan

• cargo test -p engine --test integration coin_of_mastery
• cargo test -p engine --lib artifact_mana_spent_on_self_resolves

These pass on origin/main without the CastLinkSnapshot expansion, which matches the analysis: the correct seam is payment-time snapshot recording + pre-move replacement quantity resolution, not post-delivery snapshot restore.

[matthewevans]

VERDICT: approve-pending-CI

Re-reviewed follow-up head 90ebcba05b539cec62cbb756f3b0a9bc04490933, then pushed maintainer fixup 9ff322b68cf886ee84cb9b43db2b81c3886e6406 to move the new unit-test imports to module scope.

Prior blocker status: ADDRESSED. The wrong-seam CastLinkSnapshot implementation was removed. The remaining diff is regression coverage: the integration tests drive the real GameScenario cast → stack commit → resolve → ETB replacement path with artifact-tagged pool mana, and the replacement-layer unit test covers the CastManaSpentMetric::FromSource { Artifact } quantity against the entering object snapshot. CR citations for 122.6/122.6a, 614.1c, and 601.2h check out against docs/MagicCompRules.txt.

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

[matthewevans]

Approved after re-review of the current head: the wrong-seam CastLinkSnapshot path was removed, the artifact-mana regression now exercises the real cast/commit/resolve flow, the required category labels are present, and GitHub checks are green at 64a818bb1dee42fc2cbc2abf4f420b9f274d071a.

[kiannidev]

@matthewevans Re: the zone-pipeline snapshot concern — agreed that expanding CastLinkSnapshot at move_to_zone was the wrong seam. Follow-up on this branch reverted that path and added regression coverage through the existing cast/payment/replacement route instead. CI is green on the latest run; please take another look when you have a moment.