Skip to content

Fetch lands like Polluted Delta were showing a mana choice due to thi… - #123

Merged
matthewevans merged 5 commits into
mainfrom
fix/fetch-land-mana-choice
Apr 28, 2026
Merged

Fetch lands like Polluted Delta were showing a mana choice due to thi…#123
matthewevans merged 5 commits into
mainfrom
fix/fetch-land-mana-choice

Conversation

@colin-riddell

@colin-riddell colin-riddell commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

Fix for polluted delta showing mana ability.
Also affected:

  • Fetchlands — Polluted Delta, Misty Rainforest, Wooded Foothills, Flooded Strand, Bloodstained Mire, Verdant Catacombs, Marsh Flats, Arid Mesa, Scalding Tarn,
    Windswept Heath, Prismatic Vista, Fabled Passage, Evolving Wilds, Terramorphic Expanse, plus all the new MH3-style ones (Cabaretti Courtyard, Brokers Hideout, etc.)
  • Tap-for-an-effect lands — Bazaar of Baghdad, Maze of Ith, Diamond Valley, Glacial Chasm, Ice Floe, Island of Wak-Wak, Safe Haven, Sorrow's Path, Thawing Glaciers
  • Static-only lands — Urborg Tomb of Yawgmoth, Yavimaya Cradle of Growth, Urza's Saga, The Tabernacle at Pendrell Vale, Halls of Mist
  • Counter-based — Dark Depths, Eye of Ugin

The following no longer shows a mana choice, and actually no choice at all.

Tapping it goes directly to searching up the land.

Screenshot 2026-04-26 at 21 40 49

@colin-riddell
colin-riddell marked this pull request as ready for review April 26, 2026 20:43
@matthewevans

Copy link
Copy Markdown
Member

Still looking into this. I think this change alone has some unintended side-effects. Thank you for also checking this out!

…mana actions

Pushes mana-tappability decision out of the frontend and into ai_support so
fetchlands no longer surface a phantom mana choice. The flat legal_actions
list stays free of mana actions (auto-pass + AI search tree), while the
per-object grouping exposes engine-emitted mana actions for board interaction.

- ai_support: activatable_object_mana_actions reuses
  mana_sources::activatable_land_mana_options for lands so subtype/explicit
  abilities flow through one authority. Single-option lands collapse to
  TapLandForMana; multi-option lands emit ActivateAbility per ability_index.
  Coverage extended to ManaPayment and UnlessPayment waiting states.
- client: GameBoard/PermanentCard iterate legalActionsByObject and ask the
  shared isManaObjectAction predicate instead of inferring tappability from
  card_types/has_mana_ability/summoning_sickness/tapped state.
- Doc updates on LegalActionsResult, gameStore, and GameAction::is_mana_ability
  describing the deliberate flat-vs-grouped asymmetry.
@matthewevans
matthewevans enabled auto-merge (squash) April 28, 2026 01:03
@matthewevans
matthewevans disabled auto-merge April 28, 2026 01:17
@matthewevans
matthewevans enabled auto-merge (squash) April 28, 2026 01:17
@matthewevans
matthewevans merged commit cbfa215 into main Apr 28, 2026
3 checks passed
@matthewevans
matthewevans deleted the fix/fetch-land-mana-choice branch April 28, 2026 01:25
matthewevans added a commit to nishu-builder/phase that referenced this pull request Jul 14, 2026
…ts exit condition (phase-rs#5797)

Ruling rider 1 on task phase-rs#122. Gate G already enforced the exact-set property; what
it did not do was say WHY each survivor is on the list, WHO closes it, or WHEN the
gate retires. A ratchet whose entries carry no rationale decays into a blessing —
the next reader sees a passing gate and a magic number, not a work-list.

Each of the 16 permissive router calls is now enumerated with its file:line, its
defect class, and the task that owns it (phase-rs#123):

  - priority 0 / 1b (:3720,:3725,:3789) commit on Some(vec![]), which
    extract_granted_keyword_list returns on MTGJSON metadata ALONE — the
    parameterized Oracle line is never parsed.
  - the flashback split (:4673,:4682) is NON-ATOMIC: it advances when only one
    half parsed.
  - priority 8f (:5154) is the worst: Class-A, `i += 1; continue;` OUTSIDE both
    `if let Some` blocks, so an unparseable line is consumed with no keyword and
    no Unimplemented — and it dispatches on a bare alt((tag("kicker"), …)) with
    no word-boundary guard.
  - suspend/specialize/buyback/escalate/commander-ninjutsu/p13-residual/d20
    (:5092,:5102,:5170,:5186,:5521,:5554,:5646) are advance-on-partial.
  - the two classifiers (:2161,:2167,:2205) gate ROUTING decisions on a permissive
    parse. Step 5 item 11: a classifier may probe with the STRICT parser; it may
    not call a helper that discards the remainder.

DEFINITION OF DONE is now stated in the gate itself: entries are DELETED as each
router entry converts, and when every EXPECTED_* reaches 0 this becomes the plain
"no permissive symbol in a router context" gate Plan 02 step 7 actually asks for,
taking SKILL.md §3a's migration-status paragraph with it. The down-drift failure
message now names phase-rs#123 and requires the floor to be lowered in the SAME commit, so
a partial migration cannot land while the gate still advertises the old count.

Exact-set property re-witnessed in BOTH directions after the edit:
  add a permissive call to a scratch oracle.rs  (13 -> 14)  => EXIT 1
  migrate one away without lowering the floor   (13 -> 12)  => EXIT 1, names phase-rs#123
  restored                                                  => EXIT 0
oracle.rs byte-identical afterwards. No production code touched.

Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>
parthmishra pushed a commit to parthmishra/phase that referenced this pull request Jul 14, 2026
…gn invariants (phase-rs#5816)

The add-* skills lagged five invariants that have landed on main, and one of
them (add-static-ability) stated the opposite of what the engine now does.

- add-static-ability: continuous effects apply to OBJECTS (CR 613.1), not only
  battlefield permanents. New "Zone Boundary" section: the layer pass
  materializes keywords ONLY into the zones it also resets
  ({Battlefield, Hand, Stack} — layer_pass_materializes_keywords); library /
  graveyard / exile are owned by off_zone_characteristics and computed on
  demand. A grant into an unowned zone is a silent no-op that still reports as
  supported (the PR phase-rs#5803 defect class).
- add-keyword: document the strict keyword router (PR phase-rs#5813 / task phase-rs#123).
  Router slots and routing classifiers must parse through
  parse_router_keyword_line / _list / _fragment (all-consuming); the permissive
  parse_granted_keyword_fragment / extract_granted_keyword_list are for
  EMBEDDED GRANT contexts only. A line with an unconsumed semantic tail must
  DECLINE, not commit. Gate G in scripts/check-parser-combinators.sh is the
  whole-file invariant. New router-registry checklist item
  (is_keyword_cost_line + ROUTER_KEYWORD_CASES set-equality test).
- add-trigger: new "Purged Source" section (CR 608.2h). LKI look-back via
  subject_filter_matches_with_lki / source_controller_or_lki; attachments do
  NOT survive on the live object (CR 704.5m/n sever them — only
  LKISnapshot::attachments holds the pre-sever set) while attack/block history
  DOES (durable id-keyed ledgers on GameState). Names the non-vacuity vector:
  a ceased-to-exist token (CR 111.7), not a printed creature.
- casting-stack-conditions + add-engine-effect: the three X carriers
  (GameObject::cost_x_paid = CR 107.3m cast-X, GameState::announced_source_x =
  CR 107.3a/d announce-X, ResolvedAbility::chosen_x = the published channel,
  CR 107.3i) and publish_announced_x as the single publish authority.
  Re-measuring X at resolution is rules-wrong (CR 107.3c), not merely late.
- add-replacement-effect: cross-line "instead" lowers to a BRANCH
  (ClauseDisposition::ReplaceMeaning → else_ability), never two independent
  effects — CR 614.1a + CR 614.6, the phase-rs#44/phase-rs#79 double-execution class.
- project-reference: card-export measurement hazards — export-cards rewrites
  the tracked parser input oracle-subtypes.json (non-idempotent),
  gen-card-data dirties known-tokens.toml, and the export is nondeterministic
  on ~20 faces (the noise floor of any whole-pool ledger).
- card-test: gen-test-fixture.py --check verifies COVERAGE only (a key-set
  comparison), never the stored parse VALUES — a parser change can leave the
  integration fixture stale while the gate stays green.

Also fixes stale anchors found while verifying the self-check blocks (all five
now pass; two were red before this change):
- add-static-ability / add-keyword cited crates/engine/src/parser/oracle_static.rs,
  which is a directory now (oracle_static/mod.rs, oracle_static/grammar.rs,
  oracle_static/keyword_grant.rs).
- casting-stack-conditions cited pay_and_push and pay_ability_cost in casting.rs;
  they live in casting_costs.rs and costs.rs.
- add-replacement-effect cited "MTG Rule 614.16" for as-enters choices; 614.16
  is the token/counter-creation replacement rule. The correct citations are
  CR 614.1c + CR 614.12a.

Every symbol grep-verified against the tree at 5f4fbcd; every CR number
grep-verified against docs/MagicCompRules.txt. scripts/check-skill-doc.sh and
Gate G both pass.

Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants