Skip to content

Test/5329 mana echoes shares type regression#5532

Merged
matthewevans merged 2 commits into
phase-rs:mainfrom
andriypolanski:test/5329-mana-echoes-shares-type-regression
Jul 10, 2026
Merged

Test/5329 mana echoes shares type regression#5532
matthewevans merged 2 commits into
phase-rs:mainfrom
andriypolanski:test/5329-mana-echoes-shares-type-regression

Conversation

@andriypolanski

Copy link
Copy Markdown
Contributor

Summary

Mana Echoes ("Whenever a creature enters, you may add an amount of {C} equal to the number of creatures you control that share a creature type with it.") was reported as producing zero mana. Investigation on current main shows the full stack path already works when:

  1. Parser binds SharesQuality to TargetFilter::TriggeringSource (not ParentTarget).
  2. Optional resume stashes pending_optional_trigger_event and restores it on DecideOptionalEffect { accept: true }.
  3. all_creature_types is populated (deck load / test seed) so CreatureType sharing resolves.
    No engine logic changes were required. This PR adds regression coverage so a future regression in parser binding, optional trigger-event plumbing, or shares-type counting is caught immediately.

Root cause of the original report (triage)

Layer Finding
Parser AST SharesQuality { reference: TriggeringSource } on ObjectCount filter
Stack + optional Trigger event round-trips through optional pause; mana lands on accept
Quantity / filter object_count_matching_ids returns correct count with current_trigger_event set
Integration harness foot-gun Test helper cloned base_card_types before subtypes were pushed, which broke subtype-based matching in some paths

Changes

Integration tests (crates/engine/tests/integration/issue_5329_mana_echoes.rs)

  • Parse assertion: shares-type reference is TriggeringSource.
  • Runtime positive: three Goblins on board + Goblin ETB → 3 colorless after accepting optional trigger.
  • Runtime zero case: subtypeless entering creature + Goblin on board → 0 colorless (no shared creature type).
  • Helper fixes: priority loop passes until OptionalEffectChoice (no pass_both_players fallback); enter_creature_from_hand syncs base_card_types after subtypes.

Unit tests

  • filter.rs: game_scenario_mana_echoes_shares_type_count_with_triggering_source, mana_echoes_optional_stack_pause_preserves_shares_type_count
  • Existing: object_count_shares_creature_type_with_triggering_source_for_mana_echoes, colorless_production_counts_creatures_sharing_type_with_triggering_source

Registration

  • mod issue_5329_mana_echoes; in crates/engine/tests/integration/main.rs

Verification

cargo fmt --all
cargo test -p engine mana_echoes
cargo test -p engine game_scenario_mana_echoes mana_echoes_optional_stack object_count_shares_creature colorless_production_counts

Test plan

  • cargo test -p engine mana_echoes — 3 integration + lib unit tests green
  • Manual: Mana Echoes on battlefield, accept optional on creature ETB, mana pool increases by matching creature count
  • Manual: decline optional — no mana added

Closes #5329

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@github-actions

Copy link
Copy Markdown

Parse changes introduced by this PR

✓ No card-parse changes detected.

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

Thanks for the #5329 regression — the shape (engine fix in mana.rs/filter.rs + a registered integration test) is the right one. Two mechanical blockers before I can review the logic:

  1. One-line compile error. crates/engine/src/game/effects/mana.rs:2351 imports use crate::types::identifiers::{CardId, ObjectId}; but ObjectId is unused. Under the workspace's -D warnings, an unused import is a hard compile error — that's why all three Rust gates (lint, both test shards) fail with could not compile engine (lib test) due to 1 previous error, not the test itself. Drop ObjectId from that use (keep CardId) and the build clears.

  2. Branch is BEHIND main. Please rebase onto origin/main (or use the update-branch button) so CI runs against current main.

Once it's green I'll do the full architecture + rules pass on the shares-a-creature-type counting in filter.rs (CR 202.2 / the Mana Echoes trigger). Holding until then.

andriypolanski and others added 2 commits July 10, 2026 22:44
Unused import fails under -D warnings and blocked all Rust CI gates.

Co-authored-by: Cursor <cursoragent@cursor.com>
@andriypolanski
andriypolanski force-pushed the test/5329-mana-echoes-shares-type-regression branch from 7fcb3c5 to 480994a Compare July 10, 2026 22:52

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

Re-reviewed on head 480994ae: both mechanical blockers from the prior head are resolved — the -D warnings compile abort is gone (mana.rs import is CardId-only) and mod issue_5329_mana_echoes; is registered in integration/main.rs (not inert). This is clean test-only regression coverage (534 additions, 0 removals; all engine-file hunks under mod tests; parse_diff reports no card-parse changes), locking in the #5329 fix that already works on main. Coverage is discriminating: positive (3 Goblins → 3 colorless), negative (subtypeless entering creature → 0), the optional-stack-pause preservation case, and SharesQuality bound to TriggeringSource (the actual regression axis, not ParentTarget). Full CI rollup is settled green. Approving and enqueuing. Thanks for the thorough regression net.

@matthewevans matthewevans added the bug Bug fix label Jul 10, 2026
@matthewevans
matthewevans added this pull request to the merge queue Jul 10, 2026
Merged via the queue into phase-rs:main with commit 3c7e58a Jul 10, 2026
13 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.

Mana Echoes adds no mana — Pretty much just that.

2 participants