Fix Fable chapter III returning as Saga with lore counter instead of transforming (#2425)

[kiannidev]

Summary

• Fixes #2425: Fable of the Mirror-Breaker chapter III ("Exile this Saga, then return it to the battlefield transformed") now enters as Reflection of Kiki-Jiki instead of the Saga front face with a fresh lore counter.
• Root cause: enter_transformed is a silent no-op when GameObject::back_face is unset — the saga ETB lore-counter replacement then fires on the front face.
• Adds populate_back_face_if_dfc() and calls it during object creation (add_real_card, create_object_from_card_face_with_db) in addition to the existing rehydrate_game_from_card_db path.

Test plan

• cargo test -p engine fable_chapter
• cargo test -p engine --lib populate_back_face_attaches
• cargo test -p engine --lib fable_chapter_three_exiles
• In-game: cast Fable, advance to chapter III — permanent should flip to Reflection of Kiki-Jiki (no lore counter, not a Saga)

[gemini-code-assist]

Code Review

This pull request resolves an issue where dual-faced cards (DFCs) failed to return to the battlefield transformed (such as Fable of the Mirror-Breaker's Chapter III) by extracting and reusing the back-face hydration logic into populate_back_face_if_dfc. This helper is now integrated into deck loading, scenario setup, and game rehydration, supported by new parser and integration tests. Feedback on these changes highlights a violation of Rule R6 regarding missing mandatory CR annotations on the layout match arms, and suggests a more robust approach of using printed_ref_from_face(card_face) instead of reading from the mutable obj.printed_ref.

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.

[gemini-code-assist]

[MEDIUM] Missing specific CR annotations on layout match arms. Evidence: crates/engine/src/game/printed_cards.rs:920-925. Why it matters: Rule R6 requires every rules-touching line of engine code to carry a comment of the form CR : . Suggested fix: Add the specific CR comments back to the match arms.

            // CR 715: Adventure half available at cast time
            CardLayout::Adventure(_, back) => Some((LayoutKind::Adventure, back)),
            // CR 712: Transform / Modal DFC / Meld / Omen back face
            CardLayout::Transform(_, back) => Some((LayoutKind::Transform, back)),
            CardLayout::Modal(_, back) => Some((LayoutKind::Modal, back)),
            CardLayout::Meld(_, back) => Some((LayoutKind::Meld, back)),
            CardLayout::Omen(_, back) => Some((LayoutKind::Omen, back)),
            // CR 702: Prepare (Strixhaven) — face b is the prepare spell
            CardLayout::Prepare(_, back) => Some((LayoutKind::Prepare, back)),

References

1. Rule R6: Every rules-touching line of engine code must carry a comment of the form CR : . ^(link)

[gemini-code-assist]

[MEDIUM] Use printed_ref_from_face instead of obj.printed_ref. Evidence: crates/engine/src/game/printed_cards.rs:934-936. Why it matters: Relying on mutable or potentially cleared object state is less robust than using the immutable card_face reference directly. Suggested fix: Use printed_ref_from_face(card_face) to get the printed reference.

Suggested change

	obj.printed_ref
	.as_ref()
	.and_then(|printed_ref| db.get_other_face_by_printed_ref(printed_ref))
	printed_ref_from_face(card_face)
	.as_ref()
	.and_then(|printed_ref| db.get_other_face_by_printed_ref(printed_ref))

[matthewevans]

🤖 Architecture Review (automated)

Verdict: ✅ Approve w/ nits

Seam: PASS — extracting the duplicate-prone inline back_face hydration into a single populate_back_face_if_dfc authority and reusing it across rehydrate / scenario_db is exactly the building-block pattern.
Idiomatic: PASS — clean match on CardLayout, let-else early returns, or_else fallback; saga parser change is import-only (no string-matching). No bool flags, no verbatim Oracle matches.
Value: Covers the CLASS of transform/MDFC/meld/omen/adventure/prepare DFCs that need back_face for enter_transformed, not just Fable; the regression tests are pipeline tests (drive resolve_ability_chain), not shape tests.
Reconciled with existing reviews:

• Gemini chore: update coverage stats and badges #1 (MEDIUM, missing per-arm CR annotations on printed_cards.rs match arms): CONFIRMED — the original block carried // CR 715 (Adventure), // CR 712 (Transform/Modal/Meld/Omen), // CR 702.xxx (Prepare) per-arm; the extraction (printed_cards.rs:919-926) dropped them, keeping only the function-level /// CR 712: doc. Real per the CR-annotation mandate, but pure-refactor loss with partial doc compensation → NIT, not MEDIUM.
• Gemini chore: update coverage stats and badges #2 (use printed_ref_from_face(card_face) instead of obj.printed_ref): REFUTED as a correctness issue. At every callsite obj.printed_ref is set by apply_card_face_to_object to exactly printed_ref_from_face(card_face) (printed_cards.rs:131), and the rehydrate path fetches card_face from obj.printed_ref — the two are equivalent here. It's a pre-existing pattern unchanged by this PR; a style preference, not a bug.

Findings

• [NIT] crates/engine/src/game/printed_cards.rs:919-926 — restore the per-arm CR comments lost in the extraction (// CR 715 Adventure, // CR 712 Transform/Modal/Meld/Omen, Prepare). The function /// CR 712: doc only covers Transform; the 715/Prepare specificity is gone.
• [NIT] crates/engine/src/game/deck_loading.rs:193 — create_object_from_card_face_with_db has zero callers (the wiring went through scenario_db calling populate_back_face_if_dfc directly instead). Unused pub helper added for a hypothetical caller; drop it or wire it in to avoid dead surface area.

CR numbers verified against docs/MagicCompRules.txt: 712.14a (DFC put onto battlefield "transformed" enters back-face-up) ✅, 714.3a (Saga lore-counter ETB intrinsic / 614.1c replacement) ✅, 715 (Adventurer cards) ✅. No AI/perf/frontend hot paths touched.

[matthewevans]

Pushed maintainer-nit follow-up in acf05fd3b.

What changed:

• Restored the per-layout CR annotations in populate_back_face_if_dfc using verified rule numbers: CR 712 for DFC layouts, CR 715 for adventurer cards, and CR 722 for preparation cards.
• Removed the unused create_object_from_card_face_with_db helper instead of keeping dead public surface area.

Focused review:

• Seam remains printed_cards::populate_back_face_if_dfc, the single back-face hydration authority from this PR.
• No production behavior changed beyond removing an unused helper; this resolves the maintainer review nits only.

Verification:

• cargo fmt --all: passed
• git diff --check: passed
• Pre-commit parser combinator gate: passed
• Tilt clippy / test-engine / card-data stayed queued and timed out locally, so GitHub CI is the broad gate for this pushed head.

[matthewevans]

Maintainer sign-off: reviewed acf05fd. The two review nits are addressed: verified CR comments restored at the layout match arms, and the unused public helper was removed. Architecture remains centered on populate_back_face_if_dfc.