-
-
Notifications
You must be signed in to change notification settings - Fork 155
fix(engine): route the debug face-state write through the face authorities (#7539) #7540
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
matthewevans
merged 2 commits into
phase-rs:main
from
cuinhellcat:fix/debug-turn-face-up
Aug 18, 2026
+156
−2
Merged
Changes from 1 commit
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
161 changes: 161 additions & 0 deletions
161
crates/engine/tests/integration/issue_7539_debug_turn_face_up.rs
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,161 @@ | ||
| //! Regression for GitHub issue #7539 — the sandbox `Turn Face Up` action must | ||
| //! RESTORE the stored face, not just clear the flag. | ||
| //! | ||
| //! CR 708.2a: a face-down permanent is a 2/2 creature with no name, no mana | ||
| //! cost, no creature types and no abilities. Its real characteristics live in | ||
| //! `back_face` until it is turned face up. CR 702.37e: the morph effect ends | ||
| //! and the permanent "regains its normal characteristics". Clearing `face_down` | ||
| //! alone | ||
| //! leaves the vanilla 2/2 installed, so the tool appears to do nothing. | ||
| //! | ||
| //! Same class as #3284 / #3290, where the debug `transformed` write was routed | ||
| //! through `transform::transform_permanent` by #3684. The `face_down` write in | ||
| //! the same match arm was never carried over. | ||
|
|
||
| use engine::game::scenario::{GameScenario, P0}; | ||
| use engine::types::actions::{DebugAction, GameAction}; | ||
| use engine::types::events::GameEvent; | ||
| use engine::types::mana::{ManaCost, ManaCostShard}; | ||
| use engine::types::zones::Zone; | ||
|
|
||
| /// A creature card in hand with a real mana cost, so CR 701.40b can derive the | ||
| /// turn-face-up cost from the stored face. | ||
| fn board() -> ( | ||
| engine::game::scenario::GameRunner, | ||
| engine::types::identifiers::ObjectId, | ||
| ) { | ||
| let mut scenario = GameScenario::new(); | ||
| let id = scenario | ||
| .add_creature_to_hand(P0, "Hidden Bear", 3, 3) | ||
| .with_mana_cost(ManaCost::Cost { | ||
| shards: vec![ManaCostShard::Green], | ||
| generic: 1, | ||
| }) | ||
| .id(); | ||
| let mut runner = scenario.build(); | ||
| runner.state_mut().debug_mode = true; | ||
|
|
||
| let mut events = Vec::new(); | ||
| engine::game::morph::play_face_down(runner.state_mut(), P0, id, &mut events) | ||
| .expect("the card is played face down"); | ||
|
|
||
| let obj = &runner.state().objects[&id]; | ||
| assert!(obj.face_down, "setup: the permanent is face down"); | ||
| assert_eq!(obj.zone, Zone::Battlefield); | ||
| assert_eq!(obj.name, "", "CR 708.2a: a face-down permanent has no name"); | ||
| assert_eq!(obj.base_power, Some(2), "CR 708.2a: it is a 2/2"); | ||
|
|
||
| (runner, id) | ||
| } | ||
|
|
||
| /// The defect: the tool must produce the real card, and it must produce the | ||
| /// event the turn-face-up triggers observe. | ||
| #[test] | ||
| fn the_sandbox_turn_face_up_restores_the_stored_face() { | ||
| let (mut runner, id) = board(); | ||
|
|
||
| let result = runner | ||
| .act(GameAction::Debug(DebugAction::SetFaceState { | ||
| object_id: id, | ||
| face_down: Some(false), | ||
| transformed: None, | ||
| flipped: None, | ||
| })) | ||
| .expect("the debug turn-face-up runs"); | ||
|
|
||
| let obj = &runner.state().objects[&id]; | ||
| assert!(!obj.face_down); | ||
| assert_eq!(obj.name, "Hidden Bear", "the stored face is restored"); | ||
| assert_eq!( | ||
| (obj.base_power, obj.base_toughness), | ||
| (Some(3), Some(3)), | ||
| "with its printed power and toughness, not the CR 708.2a 2/2" | ||
| ); | ||
|
|
||
| // The discriminating assertion. A flag-only write also leaves `face_down` | ||
| // false, so the flag alone cannot tell the two implementations apart — the | ||
| // restored characteristics and this event can. `TurnedFaceUp` is what the | ||
| // "when this is turned face up" triggers and the | ||
| // "as ~ is turned face up" replacement key on; without it the tool changes a | ||
| // flag and the game never learns anything happened. | ||
| assert!( | ||
| result.events.iter().any( | ||
| |event| matches!(event, GameEvent::TurnedFaceUp { object_id, .. } if *object_id == id) | ||
| ), | ||
| "the turn-face-up event must reach the triggers, got {:?}", | ||
| result.events | ||
| ); | ||
| } | ||
|
|
||
| /// The other direction, and the reason it belongs in the same fix: turning a | ||
| /// permanent face down must SNAPSHOT its face, or the permanent keeps its name | ||
| /// and printed P/T while claiming to be face down — and `back_face` stays empty, | ||
| /// so it can never be turned back up. The round trip is the assertion. | ||
| #[test] | ||
| fn the_sandbox_turn_face_down_snapshots_the_real_face_and_the_round_trip_closes() { | ||
| let mut scenario = GameScenario::new(); | ||
| let id = scenario | ||
| .add_creature(P0, "Open Bear", 4, 4) | ||
| .with_mana_cost(ManaCost::Cost { | ||
| shards: vec![ManaCostShard::Green], | ||
| generic: 2, | ||
| }) | ||
| .id(); | ||
| let mut runner = scenario.build(); | ||
| runner.state_mut().debug_mode = true; | ||
|
|
||
| let face_down = |runner: &mut engine::game::scenario::GameRunner, down: bool| { | ||
| runner | ||
| .act(GameAction::Debug(DebugAction::SetFaceState { | ||
| object_id: id, | ||
| face_down: Some(down), | ||
| transformed: None, | ||
| flipped: None, | ||
| })) | ||
| .expect("the debug face-state write runs") | ||
| }; | ||
|
|
||
| face_down(&mut runner, true); | ||
| let obj = &runner.state().objects[&id]; | ||
| assert!(obj.face_down); | ||
| assert_eq!(obj.name, "", "CR 708.2a: no name while face down"); | ||
| assert_eq!( | ||
| (obj.base_power, obj.base_toughness), | ||
| (Some(2), Some(2)), | ||
| "CR 708.2a: a 2/2, not the printed 4/4" | ||
| ); | ||
| assert!( | ||
| obj.back_face.is_some(), | ||
| "the real face is stashed, which is what makes the way back possible" | ||
| ); | ||
|
|
||
| face_down(&mut runner, false); | ||
| let obj = &runner.state().objects[&id]; | ||
| assert!(!obj.face_down); | ||
| assert_eq!(obj.name, "Open Bear"); | ||
| assert_eq!((obj.base_power, obj.base_toughness), (Some(4), Some(4))); | ||
| } | ||
|
|
||
| /// Counter-direction: an object with no stored face keeps the plain flag write, | ||
| /// so the arm stays a debug tool for states the rules cannot reach. | ||
| #[test] | ||
| fn a_permanent_without_a_stored_face_keeps_the_plain_flag_write() { | ||
| let mut scenario = GameScenario::new(); | ||
| let id = scenario.add_creature(P0, "Ordinary Bear", 2, 2).id(); | ||
| let mut runner = scenario.build(); | ||
| runner.state_mut().debug_mode = true; | ||
| runner.state_mut().objects.get_mut(&id).unwrap().face_down = true; | ||
|
|
||
| runner | ||
| .act(GameAction::Debug(DebugAction::SetFaceState { | ||
| object_id: id, | ||
| face_down: Some(false), | ||
| transformed: None, | ||
| flipped: None, | ||
| })) | ||
| .expect("the debug write runs"); | ||
|
|
||
| let obj = &runner.state().objects[&id]; | ||
| assert!(!obj.face_down); | ||
| assert_eq!(obj.name, "Ordinary Bear"); | ||
| } |
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
Repository: phase-rs/phase
Length of output: 49893
🏁 Script executed:
Repository: phase-rs/phase
Length of output: 50371
🏁 Script executed:
Repository: phase-rs/phase
Length of output: 27548
🏁 Script executed:
Repository: phase-rs/phase
Length of output: 50370
🏁 Script executed:
Repository: phase-rs/phase
Length of output: 50371
🌐 Web query:
Magic Comprehensive Rules 2026 CR 613.7f permanent turns face up or face down timestamp CR 712.16💡 Result:
In the Magic: The Gathering Comprehensive Rules (as of August 2026), these rules govern the timestamping of permanents and restrictions on turning double-faced permanents face down: 613.7f states that a permanent receives a new timestamp each time it turns face up or face down [1][2][3]. This ensures that the game can correctly determine the order of continuous effects acting on that permanent, as effects with later timestamps are generally applied after those with earlier ones [2]. 712.16 addresses double-faced permanents. It specifies that melded permanents and other double-faced permanents cannot be turned face down [4][5][6]. If a spell or ability attempts to turn a double-faced permanent face down, nothing happens [4][5][6]. Recent updates (as of June 2026) clarified that this restriction also applies to permanents with a double-faced component that are merged (such as through the mutate mechanic), ensuring that these permanents do not interact in ways that would create unintended results within the layer system [4].
Citations:
🏁 Script executed:
Repository: phase-rs/phase
Length of output: 402
Route battlefield turn-face-down through an in-place authority.
apply_face_down_entry_profileis an entry-time helper. It does not allocate a timestamp or reject double-faced permanents. This branch can leave the timestamp unchanged and turn a battlefield Transform, Modal, or Meld permanent face down, contrary to CR 613.7f and CR 712.16. Reuse the resolver’s guards and base-face snapshot logic, then apply the profile and allocate the timestamp only after validation. Add regression tests for both cases.🤖 Prompt for AI Agents
Sources: Coding guidelines, Path instructions, MCP tools