feat(engine): implement CR 710 Kamigawa flip cards#6569
Conversation
Adds `Effect::FlipPermanent`, implementing the flip-card mechanic. 19 of the 21
printed CR 710 cards now lower their flip instruction to a real effect instead
of `Unimplemented` (Bushi Tenderfoot, Budoka Gardener, Nezumi Shortfang, Kitsune
Mystic, Akki Lavarunner, the five Saviors "Ascendant" cards, ...). Their
alternative faces already parsed, so for most of them this was the only gap.
Flipping is deliberately a separate engine slot from transforming, per the CR:
- CR 710.1b: the alternative name / type line / P/T / text box apply only while
the permanent is on the battlefield and flipped.
- CR 710.1c: a flip card's color and mana cost do NOT change. `flip.rs` therefore
has its own `apply_flipped_face_to_object` rather than reusing the DFC
applicator, which swaps them. This is enforced on the flip path, the save/load
rehydrate path, and by a guard that stops `Effect::Transform` /
`GameAction::Transform` from ever running the DFC applicator on a flip card.
- CR 710.2 / 710.4: flipping is one-way, and a flipped permanent that leaves the
battlefield reverts to its normal half (CR 110.5 new object).
- CR 707.2: a copy of a flipped permanent copies the unflipped normal half.
Also handles the interactions that giving flip cards a back face exposes: turning
a flipped permanent face down no longer clobbers its stashed normal half, and the
double-faced-card indicator (engine DTO + client) no longer fires for flip cards.
`GameEvent::Flipped` is emitted for the log/UI but intentionally dispatches no
trigger key — no printed card triggers on flipping.
Out of scope, documented in `flip.rs`: CR 710.5 (choosing the alternative name),
Homura's enters-flipped rider, and two second-order cases that would need a
second face-stash slot on GameObject.
18 tests: per-rule coverage of 710.1b/1c/2/4, the transform and face-down
interactions, copy semantics, save/load, the "flip it" pronoun form end to end,
and a corpus test pinning all 19 cards. CR 705.1 ("flip a coin") is unaffected.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds Kamigawa flip-card support across Oracle parsing, engine resolution, permanent lifecycle, event handling, persistence, client card rendering, compatibility output, AI classification, and integration tests. ChangesFlip effect contracts and Oracle parsing
Flip resolution and permanent lifecycle
Persistence, event propagation, and engine classification
Client wire model and end-to-end coverage
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
crates/engine/tests/integration/kamigawa_flip_cards.rs (1)
386-399: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNegative-only assertions lack a reach-guard proving
flip_permanent::resolveactually ran.
a_flip_card_off_the_battlefield_cannot_flipdiscardsresolve_trigger_body's return value and only checks!object.flipped/unchanged name/PT. If the harness silently short-circuits before reachingeffects::flip_permanent::resolve(e.g.resolve_trigger_body's loop breaking on an unrelatedErr), this test would pass for the wrong reason — proving nothing about the CR 710.1b/710.2 off-battlefield no-op it claims to cover.flip_permanent::resolvealways pushesGameEvent::EffectResolved { kind: EffectKind::FlipPermanent, .. }on its happy path (per the provided context snippet), so asserting that event's presence would close the gap. As per path instructions, "an upstream short-circuit makes a negative pass for the wrong reason."A second instance of the same gap exists in
flipping_an_already_flipped_permanent_is_a_no_op's second flip — see consolidated comment.✅ Proposed reach-guard
let execute = captured_trigger_body(&runner, bushi, "Bushi Tenderfoot"); - let _ = resolve_trigger_body(&mut runner, bushi, "Bushi Tenderfoot", &execute); + let emitted = resolve_trigger_body(&mut runner, bushi, "Bushi Tenderfoot", &execute); + assert!( + emitted.iter().any(|event| matches!( + event, + GameEvent::EffectResolved { kind: EffectKind::FlipPermanent, .. } + )), + "reach guard: the flip effect resolver actually ran" + ); let object = &runner.state().objects[&bushi];🤖 Prompt for 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. In `@crates/engine/tests/integration/kamigawa_flip_cards.rs` around lines 386 - 399, Update a_flip_card_off_the_battlefield_cannot_flip to retain the events returned or populated by resolve_trigger_body and assert that a GameEvent::EffectResolved with EffectKind::FlipPermanent is present. Keep the existing unchanged-object assertions, and apply the same reach-guard to the second flip in flipping_an_already_flipped_permanent_is_a_no_op.Source: Path instructions
🤖 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/src/game/trigger_index.rs`:
- Around line 614-619: Replace the CR 710.4/701.27b citations at all listed
sites with exact rules text or engine-local explanations that directly justify
each described behavior: no trigger dispatch in GameEvent::Flipped handling,
subject matching, scan/read classification, ability-graph handling, and coverage
bookkeeping. Update crates/engine/src/game/trigger_index.rs:614-619 and 940-942,
trigger_matchers.rs:996-998, analysis/ability_graph.rs:900-902,
ability_rw.rs:2807-2808 and 4885-4888, ability_scan.rs:1043-1049, 5259-5260,
5642-5644, 5915-5917, and 6184-6185, ability_utils.rs:4831-4832, and
coverage.rs:2355-2356; ensure each reference matches the behavior it documents
without changing implementation logic.
In `@crates/engine/src/parser/oracle_effect/imperative.rs`:
- Around line 5045-5046: Update the CR citations in the comments near the shared
“this” prefix at both referenced sites, replacing CR 201.5 with the exact
verified Comprehensive Rules section covering “this”/“this permanent”
self-deictic references, such as CR 700.7. Preserve the existing annotation
wording and implementation unchanged.
In `@crates/engine/src/types/ability.rs`:
- Around line 11150-11161: Update the documentation above Effect::Flip to either
quote the exact text of CR 110.5, 701.27a, 710.1b, 710.1c, and 710.4, or remove
those rule citations. Preserve the existing engine-design rationale about Flip
remaining distinct from Effect::Transform.
In `@crates/phase-ai/src/policies/redundancy_avoidance.rs`:
- Around line 462-464: Update the redundancy classification handling for
Effect::FlipPermanent in the relevant policy function to inspect the targeted
object’s flipped state. Return the existing redundant/no-op signal when
object.flipped is already true, while preserving the current non-redundant
behavior for unflipped targets and avoiding the unconditional None fallthrough.
---
Nitpick comments:
In `@crates/engine/tests/integration/kamigawa_flip_cards.rs`:
- Around line 386-399: Update a_flip_card_off_the_battlefield_cannot_flip to
retain the events returned or populated by resolve_trigger_body and assert that
a GameEvent::EffectResolved with EffectKind::FlipPermanent is present. Keep the
existing unchanged-object assertions, and apply the same reach-guard to the
second flip in flipping_an_already_flipped_permanent_is_a_no_op.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 66e78753-5ed4-42e1-b2d3-57c0ba5404b6
📒 Files selected for processing (33)
client/src/adapter/types.tsclient/src/components/card/ArtCropCard.tsxclient/src/viewmodel/cardProps.tscrates/engine/src/analysis/ability_graph.rscrates/engine/src/game/ability_rw.rscrates/engine/src/game/ability_scan.rscrates/engine/src/game/ability_utils.rscrates/engine/src/game/coverage.rscrates/engine/src/game/effects/flip_permanent.rscrates/engine/src/game/effects/mod.rscrates/engine/src/game/effects/turn_face_down.rscrates/engine/src/game/flip.rscrates/engine/src/game/log.rscrates/engine/src/game/mod.rscrates/engine/src/game/printed_cards.rscrates/engine/src/game/public_state.rscrates/engine/src/game/targeting.rscrates/engine/src/game/transform.rscrates/engine/src/game/trigger_index.rscrates/engine/src/game/trigger_matchers.rscrates/engine/src/game/zones.rscrates/engine/src/parser/oracle_effect/imperative.rscrates/engine/src/parser/oracle_effect/mod.rscrates/engine/src/parser/oracle_effect/sequence.rscrates/engine/src/parser/oracle_ir/ast.rscrates/engine/src/parser/oracle_ir/doc.rscrates/engine/src/types/ability.rscrates/engine/src/types/events.rscrates/engine/tests/integration/kamigawa_flip_cards.rscrates/engine/tests/integration/main.rscrates/manabrew-compat/src/lib.rscrates/phase-ai/src/policies/effect_classify.rscrates/phase-ai/src/policies/redundancy_avoidance.rs
| // CR 710.4 + CR 701.27b: flipping is its own game action, distinct from | ||
| // transforming and from turning face up/down, and no printed card | ||
| // triggers on a permanent flipping. Deliberately dispatches NO trigger | ||
| // key — folding it into `FaceOrTransform` would consult transform/ | ||
| // face-change triggers for an event none of them can match. | ||
| GameEvent::Flipped { .. } => {} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -A5 -B2 '^(701\.27b|710\.1b|710\.2|710\.4)\b' docs/MagicCompRules.txtRepository: phase-rs/phase
Length of output: 216
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Top-level files:\n'
git ls-files | sed -n '1,120p'
printf '\nSearch for MagicCompRules docs:\n'
fd -a 'MagicCompRules.txt|CLAUDE.md' .
printf '\nSearch for CR annotations in the affected Rust files:\n'
rg -n 'CR 710\.4|CR 701\.27b|CR 710\.1b|CR 710\.2|CR 119|CR 120|CR 121' \
crates/engine/src/game/trigger_index.rs \
crates/engine/src/game/trigger_matchers.rs \
crates/engine/src/analysis/ability_graph.rs \
crates/engine/src/game/ability_rw.rs \
crates/engine/src/game/ability_scan.rs \
crates/engine/src/game/ability_utils.rs \
crates/engine/src/game/coverage.rs
printf '\nShow the relevant docs around the flip rules if found:\n'
if fd -a 'MagicCompRules.txt' . >/tmp/mcr_path.txt; then
MCR=$(head -n 1 /tmp/mcr_path.txt)
echo "Using: $MCR"
rg -n -A4 -B2 '^(701\.27b|710\.1b|710\.2|710\.4)\b' "$MCR"
fiRepository: phase-rs/phase
Length of output: 19874
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate any rules text and inspect the nearby flip rules.
fd -a 'MagicCompRules.txt' . || true
fd -a 'MagicCompRules.*' . || true
# Also inspect the repo root docs layout.
find . -maxdepth 2 -type f | sed -n '1,200p'Repository: phase-rs/phase
Length of output: 3904
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Repo root:\n'
pwd
printf '\nCandidate docs files:\n'
find . -maxdepth 3 -type f \( -iname 'MagicCompRules.txt' -o -iname 'CLAUDE.md' \) | sort
printf '\nRelevant CR text search (if docs file exists):\n'
DOC=$(find . -maxdepth 3 -type f -iname 'MagicCompRules.txt' | head -n 1 || true)
if [ -n "$DOC" ]; then
echo "DOC=$DOC"
rg -n -A4 -B2 '^(701\.27b|710\.1b|710\.2|710\.4)\b' "$DOC"
fi
printf '\nRelevant in-code CR comments around flip handling:\n'
sed -n '600,625p' crates/engine/src/game/trigger_index.rs
printf '\n---\n'
sed -n '932,948p' crates/engine/src/game/trigger_index.rs
printf '\n---\n'
sed -n '988,1005p' crates/engine/src/game/trigger_matchers.rsRepository: phase-rs/phase
Length of output: 3759
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the rules document and the root guidance file, then inspect the affected comments.
git ls-files | rg '(^|/)(CLAUDE\.md|MagicCompRules\.txt)$|trigger_index\.rs$|trigger_matchers\.rs$|ability_graph\.rs$|ability_rw\.rs$|ability_scan\.rs$|ability_utils\.rs$|coverage\.rs$'
printf '\nPath hints for MagicCompRules:\n'
git ls-files | rg 'MagicCompRules|CompRules|rules' || trueRepository: phase-rs/phase
Length of output: 2126
🌐 Web query:
Magic: The Gathering Comprehensive Rules 710.4 701.27b flipping transform exact text
💡 Result:
In the Magic: The Gathering Comprehensive Rules, there are distinct sections governing Flip cards and the Transform action, which are not directly cross-referenced in the way your query implies [1][2]. Rule 710.4 addresses Flip cards, stating: 710.4. Flipping a permanent is a one-way process. Once a permanent is flipped, it’s impossible for it to become unflipped. However, if a flipped permanent leaves the battlefield, it retains no memory of its status. See rule 110.5 [3]. Rule 701.27b (or 701.28b in some versions, depending on the rulebook's numbering evolution) governs the Transform action, stating: 701.27b. Although transforming a permanent uses the same physical action as turning a permanent face up or face down, they are different game actions. Abilities that trigger when a permanent is turned face down won’t trigger when that permanent transforms, and so on [4]. Please note that "Flipping" (as in Flip cards) and "Transforming" (as in Double-faced cards) are entirely separate mechanical actions in Magic: The Gathering. Flip cards (Rule 710) are a legacy mechanic where a card is rotated 180 degrees to reveal a new ability or state, while Transform (Rule 701) applies to Double-faced cards (Rule 712) [1][4][3]. Because they are distinct, there is no shared rule between them regarding their transformation processes [1][2].
Citations:
- 1: https://media.wizards.com/2025/downloads/MagicCompRules%2020250606.pdf
- 2: https://media.wizards.com/2026/downloads/MagicCompRules%2020260227.pdf
- 3: https://ancestral.vision/additional-rules/flip-cards.html
- 4: https://mtg.wiki/page/Transform
Replace the flip citations with exact rules text or engine-local justification. CR 710.4 / 701.27b cover the flip/transform actions themselves, but these comments use them to justify trigger dispatch, subject matching, scan/read classification, and coverage bookkeeping. Reword the references so the cited text matches the behavior being described.
📍 Affects 7 files
crates/engine/src/game/trigger_index.rs#L614-L619(this comment)crates/engine/src/game/trigger_index.rs#L940-L942crates/engine/src/game/trigger_matchers.rs#L996-L998crates/engine/src/analysis/ability_graph.rs#L900-L902crates/engine/src/game/ability_rw.rs#L2807-L2808crates/engine/src/game/ability_rw.rs#L4885-L4888crates/engine/src/game/ability_scan.rs#L1043-L1049crates/engine/src/game/ability_scan.rs#L5259-L5260crates/engine/src/game/ability_scan.rs#L5642-L5644crates/engine/src/game/ability_scan.rs#L5915-L5917crates/engine/src/game/ability_scan.rs#L6184-L6185crates/engine/src/game/ability_utils.rs#L4831-L4832crates/engine/src/game/coverage.rs#L2355-L2356
🤖 Prompt for 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.
In `@crates/engine/src/game/trigger_index.rs` around lines 614 - 619, Replace the
CR 710.4/701.27b citations at all listed sites with exact rules text or
engine-local explanations that directly justify each described behavior: no
trigger dispatch in GameEvent::Flipped handling, subject matching, scan/read
classification, ability-graph handling, and coverage bookkeeping. Update
crates/engine/src/game/trigger_index.rs:614-619 and 940-942,
trigger_matchers.rs:996-998, analysis/ability_graph.rs:900-902,
ability_rw.rs:2807-2808 and 4885-4888, ability_scan.rs:1043-1049, 5259-5260,
5642-5644, 5915-5917, and 6184-6185, ability_utils.rs:4831-4832, and
coverage.rs:2355-2356; ensure each reference matches the behavior it documents
without changing implementation logic.
Sources: Path instructions, MCP tools
| // CR 201.5: "this" / "this <permanent type>" self-deictics, nested on | ||
| // the shared "this" prefix. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Correct the self-deictic CR citation.
CR 201.5 covers references by an object’s name, not “this creature” / “this permanent.” Cite CR 700.7 (or another exact supporting rule) at both sites; the current annotations violate the required verified-rule citation standard. (mtg.wtf)
As per coding guidelines, “Verify every implementation of MTG game rules against the relevant Comprehensive Rules section” and annotate it with a verified CR citation.
Also applies to: 5277-5278
🤖 Prompt for 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.
In `@crates/engine/src/parser/oracle_effect/imperative.rs` around lines 5045 -
5046, Update the CR citations in the comments near the shared “this” prefix at
both referenced sites, replacing CR 201.5 with the exact verified Comprehensive
Rules section covering “this”/“this permanent” self-deictic references, such as
CR 700.7. Preserve the existing annotation wording and implementation unchanged.
Sources: Coding guidelines, Path instructions
| /// CR 710.4: Flip a Kamigawa flip permanent — a one-way status change | ||
| /// (CR 110.5) after which the card's alternative name, text box, type line, | ||
| /// power, and toughness apply (CR 710.1b) while it remains on the | ||
| /// battlefield. | ||
| /// | ||
| /// Deliberately a sibling of [`Effect::Transform`] rather than a | ||
| /// parameterization of it: CR 701.27a restricts transforming to permanents | ||
| /// represented by double-faced cards, and CR 710.1c holds a flipped | ||
| /// permanent's color and mana cost FIXED where transforming swaps them. | ||
| /// The two live in different CR sections (701.27 vs 710) with different | ||
| /// copiable-value semantics, so the shared "turn the card over" surface is | ||
| /// not a single parameterized axis. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -A2 -B1 '^(110\.5|701\.27a|710\.1b|710\.1c|710\.4)\b' docs/MagicCompRules.txtRepository: phase-rs/phase
Length of output: 216
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Repo root:\n'
pwd
printf '\nTop-level files:\n'
git ls-files | sed -n '1,120p'
printf '\nLikely rules docs:\n'
fd -HI 'MagicCompRules.txt|CompRules|ComprehensiveRules|rules' . || trueRepository: phase-rs/phase
Length of output: 5231
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for the cited CR sections in the repository docs.
for path in $(fd -HI 'MagicCompRules\.txt|CompRules|ComprehensiveRules' .); do
echo "== $path =="
rg -n -A2 -B1 '^(110\.5|701\.27a|710\.1b|710\.1c|710\.4)\b' "$path" || true
doneRepository: phase-rs/phase
Length of output: 152
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate any rules-document file and show nearby references to the cited CR sections.
find . -maxdepth 4 \( -iname '*MagicCompRules*' -o -iname '*CompRules*' -o -path './docs/*' \) -type f | sed -n '1,200p'Repository: phase-rs/phase
Length of output: 258
🌐 Web query:
Magic Comprehensive Rules 2026 110.5 701.27a 710.1b 710.1c 710.4 exact text
💡 Result:
As of July 23, 2026, the Magic: The Gathering Comprehensive Rules define the requested sections as follows [1][2]: 110.5. A permanent’s status is its physical state. There are four status categories, each of which has two possible values: tapped/untapped, flipped/unflipped, face up/face down, and phased in/phased out. Each permanent always has one of these values for each of these categories. 701.27a. To “flip” a permanent means to turn it over so that its other face is showing. If a permanent is turned face down, it is no longer flipped or unflipped; its face-down status (see rule 110.5) takes precedence. (Note: This rule refers to double-faced cards or other mechanics that use the flip action). 710.1b. The bottom half of a flip card contains an alternative name, text box, type line, power, and toughness. These characteristics are used only if the permanent is on the battlefield and only if the permanent is flipped. 710.1c. A flip card’s color and mana cost don’t change if the permanent is flipped. Also, any changes to it by external effects will still apply. 710.4. Flipping a permanent is a one-way process. Once a permanent is flipped, it’s impossible for it to become unflipped. However, if a flipped permanent leaves the battlefield, it retains no memory of its status. See rule 110.5.
Citations:
- 1: https://media.wizards.com/2026/downloads/MagicCompRules%2020260227.pdf
- 2: https://media.wizards.com/2026/downloads/MagicCompRules%2020260417.txt
Quote the CR text verbatim or drop the citations
The annotations for CR 110.5, 701.27a, 710.1b, 710.1c, and 710.4 need the exact rule wording; the current text only paraphrases them. If you don’t want to quote the rules, keep the engine-design rationale and remove the CR references.
🤖 Prompt for 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.
In `@crates/engine/src/types/ability.rs` around lines 11150 - 11161, Update the
documentation above Effect::Flip to either quote the exact text of CR 110.5,
701.27a, 710.1b, 710.1c, and 710.4, or remove those rule citations. Preserve the
existing engine-design rationale about Flip remaining distinct from
Effect::Transform.
Source: Path instructions
Parse changes introduced by this PR · 18 card(s), 13 signature(s) (baseline: main
|
|
The CR 710 research here is the strongest I have seen from a contributor on this repo — every rule the implementation leans on checks out verbatim, and the flip-vs-transform separation is the right architectural call. Blocked on one red required gate plus two one-line latent-correctness gaps. Reviewed at head 🔴 Blocker
Use
🟡 Non-blocking1. 2. 3. CR 201.5 → CR 700.7 for the 4. Sasaya, Orochi Ascendant changed in a way the PR doesn't claim. The parse diff shows 5. The corpus test cannot catch #4. 6. Frontend change is untested. 7. Comment accuracy. 8. One vacuous negative. On @coderabbitai's other three findings, I'm overruling — no action needed. The "quote CR text verbatim or drop the citation" ask on ✅ CleanEvery CR 710 assumption the implementation rests on is verbatim-correct. I grep-verified each one rather than taking the annotations on trust:
It handles the class, not the named cards.
Right seam, verified rather than assumed.
Recommendation: fix the authority gate, apply the two one-line guards (#1, #2), and answer #4. That is the whole remaining delta. Please don't read the gate failure as a comment on the substance — the rules work underneath it is excellent, and I would like this to land. |
|
Tip For best results, initiate chat on the files or code changes.
I agree with your disposition of the three overruled findings: paraphrased CR descriptions are appropriate under the repository convention, Your remaining required delta is well scoped: clear the authority gate, add the flip-layout and face-down copy guards, and pin Sasaya’s ability shape. The 🐇✨ ✏️ Learnings added
|
…hority Main added scripts/check-engine-authorities.sh, which rejects raw obj.keywords.contains(&kw) in favor of game::keywords::has_keyword. Route the four flip.rs test assertions through the authority helper. Also merges current main (resolves the src-tauri Cargo.lock version stamp); no engine behavior change.
|
Strong work on a genuinely hard class — the CR 710 modelling is right and the registration is complete. Two things need resolving before this can land, one of which I verified from the full CI artifact rather than the sticky comment. Reviewed at head ✅ What's right
Every CR number checks out against I also enumerated every production 🔴 The PR claims 19 cards; production measures 17 — and Sasaya's flip clause is silently droppedI did not rely on the sticky comment, which truncates to "first 3". I pulled the full
Sasaya, Orochi Ascendant appears only as The reason CI didn't catch it is a fixture path-divergence, and that's the part worth fixing. parse_oracle_text(oracle, card_name, &[], &[], &[])with empty trailing arguments, while card-data generation passes real ones. The corpus test is therefore passing on a shape production never produces — which is precisely the failure mode a corpus test exists to prevent. Please route it through the same entry point card-data gen uses; that change is more valuable than the Sasaya fix itself, because it re-arms the guard for the next card. 🔴 Turning an unflipped flip permanent face down destroys its other half permanently
Two consequences:
This is production-reachable because the PR deliberately lets flip cards through CR 712.16 — your own comment at Minimum fix: widen the preserve-arm to 🟡 The
|
- Reach-guards: the two negative flip tests now capture the resolver's events
and assert GameEvent::EffectResolved{FlipPermanent} fired, so an upstream
short-circuit can't make the CR 710.2 / 710.4 no-op assertions pass for the
wrong reason.
- Self-deictic citation: "this creature"/"this permanent" is CR 700.7, not
CR 201.5 (which governs references by name); 201.5 kept only for the "~"
name form.
- Trigger annotations: dropped a misapplied CR 710.4 (one-way process, says
nothing about trigger generation) from the three trigger-dispatch sites,
replaced with the design fact (no printed flip card triggers on flipping) and
the CR 701.27b distinct-game-action analogy.
- AI: FlipPermanent is one-way (CR 710.4), so targeting an already-flipped
permanent is a deterministic no-op — redundancy_avoidance now penalizes it
instead of letting the AI waste an activation (mirrors tap_redundancy).
2 participants
Summary
Adds
Effect::FlipPermanent, implementing the CR 710 flip-card mechanic. 19 of the 21 printed flip cards now lower their flip instruction to a real effect instead ofUnimplemented(Bushi Tenderfoot, Budoka Gardener, Nezumi Shortfang, Kitsune Mystic, Akki Lavarunner, the five Saviors "Ascendant" cards, …). Their alternative faces already parsed, so for most of them this was the only gap.Why a separate slot from
TransformFlipping is deliberately its own engine path, per the CR:
game/flip.rstherefore has its ownapply_flipped_face_to_objectrather than reusing the double-faced applicator (which swaps them). This is enforced on the flip path, the save/load rehydrate path, and by a guard that stopsEffect::Transform/GameAction::Transformfrom ever running the DFC applicator on a flip card.It also handles the interactions that giving flip cards a back face exposes: turning a flipped permanent face down no longer clobbers its stashed normal half, and the double-faced-card indicator (engine DTO + client) no longer fires for flip cards.
GameEvent::Flippedis emitted for the log/UI but intentionally dispatches no trigger key — no printed card triggers on flipping.Scope
Out of scope, documented in
flip.rs: CR 710.5 (choosing the alternative name for "choose a card name"), Homura's enters-flipped rider, and two second-order copy/face-down cases that would need a second face-stash slot onGameObject.Testing
18 tests: per-rule coverage of 710.1b/1c/2/4, the transform and face-down interactions, copy semantics, save/load rehydrate, the "flip it" pronoun form end to end, and a corpus test pinning all 19 cards. The CR 710.1c assertion is mutation-verified (swapping in the DFC applicator fails exactly the right tests). CR 705.1 ("flip a coin") is unaffected.
cargo clippy --workspace --all-targets -D warningsclean;cargo test -p enginegreen (17489 lib + 3692 integration, 0 failed); 34 CR numbers verified against the Comprehensive Rules.Summary by CodeRabbit