Fix Emperor of Bones delayed sacrifice regression#5865
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds an integration test for Emperor of Bones (Issue #1515) to verify that it correctly grants haste to, and later sacrifices, the creature returned from its linked exile set. The feedback suggests refactoring the test setup to use the idiomatic GameScenario::add_creature_to_exile helper instead of manual object creation.
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.
| let mut runner = scenario.build(); | ||
| let returned = { | ||
| let state = runner.state_mut(); | ||
| let returned = create_object( | ||
| state, | ||
| CardId(900), | ||
| P0, | ||
| "Linked Gravebeast".to_string(), | ||
| Zone::Exile, | ||
| ); | ||
| let object = state | ||
| .objects | ||
| .get_mut(&returned) | ||
| .expect("exiled card exists"); | ||
| object | ||
| .card_types | ||
| .core_types | ||
| .push(engine::types::card_type::CoreType::Creature); | ||
| object.base_card_types = object.card_types.clone(); | ||
| object.power = Some(3); | ||
| object.toughness = Some(3); | ||
| object.base_power = Some(3); | ||
| object.base_toughness = Some(3); | ||
| state.exile_links.push(ExileLink { | ||
| exiled_id: returned, | ||
| source_id: emperor, | ||
| kind: ExileLinkKind::TrackedBySource, | ||
| }); | ||
| returned | ||
| }; |
There was a problem hiding this comment.
The test setup for the exiled creature can be simplified by using the GameScenario::add_creature_to_exile helper. This is more idiomatic for the codebase, as it uses existing test building blocks instead of lower-level object manipulation with create_object. This also makes the test more concise and readable.
let returned = scenario.add_creature_to_exile(P0, "Linked Gravebeast", 3, 3).id();
let mut runner = scenario.build();
runner.state_mut().exile_links.push(ExileLink {
exiled_id: returned,
source_id: emperor,
kind: ExileLinkKind::TrackedBySource,
});References
- The project's primary architectural goal is to favor idiomatic approaches that use existing building blocks. The
GameScenariotype is a building block for tests, and its helpers likeadd_creature_to_exileshould be preferred over manual object creation withcreate_objectfor setting up test state. (link)
There was a problem hiding this comment.
Addressed in 2639ac4.
Added GameScenario::add_creature_to_exile beside the existing hand/graveyard creature helpers and updated the Emperor regression test to use it instead of manually creating the exiled object.
Parse changes introduced by this PR✓ No card-parse changes detected. |
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
Approved after current-head implementation review.
3 participants
Summary
Fixes the Emperor of Bones returned-creature binding regression by adding an integration test that resolves its linked-exile counter-trigger body and verifies the returned creature, not Emperor, receives haste and the delayed sacrifice. Closes #1515.
Files changed
CR references
None added or touched.
Implementation method (required)
Method: not-applicable - regression-test and test-harness helper only; no production rules behavior changed.
Track
Developer
LLM
Model: codex-5
Thinking: high
Verification
Required checks ran clean, or the exact CI-owned alternative is stated below.
Gate A output below is for the current committed head.
Final review-impl below is clean for the current committed head.
Both anchors cite existing analogous code at the same seam.
cargo fmt --all -- --check- PASScargo test -p engine --test integration issue_1515_emperor_of_bones -- --nocapture- PASS: 1 passed; 0 failed; 3134 filtered out.cargo test -p engine --test integration no_top_level_test_binaries -- --nocapture- PASS: 2 passed; 0 failed; 3133 filtered out../scripts/check-parser-combinators.sh- PASS.tilt get uiresource clippy- unavailable in isolated worktree:Error: No tilt apiserver found: tilt-default; direct scoped Rust checks used.Gate A
Gate G PASS (router/grant architecture: strict router vs permissive grant boundary intact)
Gate A PASS head=2639ac4ad3fc1a1eceea25544df274acb167d03b base=c0d7e0f3a5d65c22b475491f0556a2655777980f
Anchored on
GameScenario::add_creature_to_graveyardtest builder for staging creature cards outside the battlefield.Final review-impl
Final review-impl PASS head=2639ac4ad3fc1a1eceea25544df274acb167d03b
Claimed parse impact
None.
Validation Failures
None.
CI Failures
None.