Parse plural subtypes for Affinity (e.g. Elves → Elf) and add affinity test for Elves - #11
Open
keloide wants to merge 1 commit into
Open
Parse plural subtypes for Affinity (e.g. Elves → Elf) and add affinity test for Elves#11keloide wants to merge 1 commit into
keloide wants to merge 1 commit into
Conversation
…ion test Tier: Frontier Model: claude-opus-4-8 Thinking: High ### Motivation - Root cause: the affinity parameter fallback built a subtype by naively capitalizing and stripping a trailing `s`, which caused plural subtypes like "Elves" to be mis-canonicalized (e.g. produced the wrong subtype string) so affinity filters did not match battlefield Elves. - Intent: minimally fix parser/AST generation so MTG-native subtype pluralization (irregulars like "Elves") is canonicalized before producing the `Affinity` TypedFilter, without changing engine cost logic or other cards. - Scope constraint: only the affinity-type parsing fallback was changed and a focused regression test was added; unrelated systems were not refactored or modified. ### Description - Use the shared subtype parser to canonicalize affinity subtypes: `parse_affinity_type` now delegates to `crate::parser::oracle_util::parse_subtype(s)` and uses the parsed canonical subtype when it consumes the full parameter, falling back to the previous capitalization-only behavior only when the subtype parser does not match. - Added a focused unit test `affinity_for_elves_counts_controlled_elves` in `crates/engine/src/game/casting_tests.rs` that builds a Cantankerous Keepers-like object with `"Affinity for Elves"`, puts three Elf permanents onto the battlefield, and asserts the generic mana reduction and preservation of colored pips. - Files changed: `crates/engine/src/types/keywords.rs` (affinity parsing fallback) and `crates/engine/src/game/casting_tests.rs` (new regression test). ### Testing - Ran `cargo test -p phase-engine affinity_for_elves_counts_controlled_elves` and the new test passed. - Ran repository checks: `cargo fmt --all`, `./scripts/check-parser-combinators.sh` (Gate A PASS), and `cargo clippy --all-targets -- -D warnings`, all of which completed without failures. - Also ran `git diff --check` and targeted test runs; all automated checks listed above succeeded in this environment (note: fetching upstream/creating a remote PR was blocked by the environment network/proxy, so upstream merge-base/diff steps were not completed here).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Description
parse_affinity_typeto prefer the shared subtype parser by callingcrate::parser::oracle_util::parse_subtypeand require full-string consumption before accepting the parsed subtype.sstripping behavior (with the "Plains" exception).affinity_for_elves_counts_controlled_elvesincrates/engine/src/game/casting_tests.rsthat verifies affinity reduces the cost for controlled Elves and that the creature can be cast and resolves to the battlefield.Testing
cargo test -p engine, which included the newaffinity_for_elves_counts_controlled_elvestest, and all tests passed.Codex Task