feat(engine): journal CR 707.2 copy-token births as resolved commands - #6648
Merged
Conversation
matthewevans
enabled auto-merge
July 26, 2026 04:38
Contributor
|
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 (6)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughCopy-token creation now records structured resolved bodies, shares materialization between live creation and replay, journals liminal and direct births, and rejects replay for deferred post-birth modifications. Integration tests cover exact replay, folded exceptions, and fail-closed behavior. ChangesResolved copy-token creation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CopyTokenOf
participant materialize_token_copy_body
participant resolved_rules_journal
participant apply_resolved_token_creation
CopyTokenOf->>materialize_token_copy_body: Materialize copy-token body
CopyTokenOf->>resolved_rules_journal: Record ResolvedTokenBody::Copy
resolved_rules_journal->>apply_resolved_token_creation: Replay token creation
apply_resolved_token_creation->>materialize_token_copy_body: Materialize replayed body
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/effects/token.rs`:
- Around line 1093-1117: Use the recorded entry turn when materializing replayed
tokens instead of the live state.turn_number. Update both the
ResolvedTokenBody::Spec and ResolvedTokenBody::Copy calls to pass the command’s
recorded turn field, alongside the existing entry_timestamp, so
reset_for_battlefield_entry preserves the original summoning-sickness state.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 227db667-dc22-4d6c-879b-992956ec8c00
📒 Files selected for processing (6)
crates/engine/src/game/effects/token.rscrates/engine/src/game/effects/token_copy.rscrates/engine/src/types/resolved_commands.rscrates/engine/tests/integration/cr733_resolved_copy_token_creation.rscrates/engine/tests/integration/cr733_resolved_token_creation.rscrates/engine/tests/integration/main.rs
Parse changes introduced by this PR✓ No card-parse changes detected. |
Copy tokens were the last unjournaled token birth. Both production copy
seams now resolve-and-journal through the same command the ordinary
CR 111.1 birth uses, and replay materializes them from the record.
Parameterize, don't proliferate — copy extends ResolvedTokenCreationCommand
rather than adding a sibling variant. The journaled axis is object creation
(CR 111.1: "Some effects put tokens onto the battlefield") for BOTH bodies:
an object came into existence and its id and CR 613.7d timestamp were drawn.
CR 707.2 governs only how the body was computed upstream of that seam, so it
parameterizes the payload instead of forking the family, and the axis stays
inside a single CR section as the categorical-boundary rule requires. The
shipped scope comment on that struct already prescribed exactly this.
spec + token_image_ref -> body: ResolvedTokenBody::{Spec, Copy}
One shared body authority per shape, so resolve and replay cannot drift:
materialize_token_copy_body is now called by the liminal build, the direct
create_object path, and the applier. That also removes a duplicated keyword
grant and a redundant loyalty seed (install_copiable_values_as_base already
installs values.loyalty, which is what copied_loyalty was).
Fail-closed on the case that cannot be replayed exactly. The direct seam
applies its CR 707.9 exceptions AFTER the birth via apply_token_modifications
-- pausable, state-level, and with no resolved family yet -- so those births
record DeferredToUnjournaledSeam and the applier returns
UnreplayableCopyModifications instead of installing a body silently missing
them. An absent entry is detectable; a present-but-wrong one is not. The
refusal is greppable and disappears when that seam gets its own family.
all_creature_types is recorded rather than re-read at replay: changeling
(CR 702.73a) and type-changing effects mutate it, and remove_subtype_set
consults the live list.
The entry turn is now recorded too, for BOTH body arms. The applier used to
read state.turn_number live when stamping entered_battlefield_turn, which
backs CR 302.6 summoning sickness. This is NOT a shipped defect: turns.rs
resets resolved_rules_journal (:947) inside start_next_turn, after the
turn_number bump (:813), so the journal can only hold current-turn entries
and a replay always observes the recording turn. But that comment calls
itself a "P1 retention policy (not a CR rule)" that holds only "until the
CR 733 settlement consumer defines the real retention window" -- so the
command should be self-contained rather than resting on a policy documented
as provisional. Widening retention would otherwise make a replayed creature
attack when its summoning sickness should forbid it, silently, with no
compile error.
Which seam a copy takes is decided by its TARGET, not by its own text.
Applied Geometry's exceptions all parse to SetPower/SetToughness/AddSubtype/
AddType (all liminal-immediate) and its "Put six +1/+1 counters on it" parses
as a separate effect, so copying a plain permanent takes the liminal seam
(Folded) while copying one that itself enters with counters makes
etb_counters non-empty and takes the direct seam (Deferred). All three body
arms are covered by production-path tests on that basis.
Meld is NOT in this family. finish_meld_entry reuses the existing component
object's id and moves it through the ordinary zone pipeline -- it draws no
id, sets no is_token, and materializes nothing -- so an applier whose whole
contract is materializing a subject cannot own it.
The LiminalEntry premise that blocked this unit was wrong: it already carries
object: GameObject plus copy_resume/spec_resume, so no field was added and
no serialized contract changed. Verified copy births are the ONLY production
liminal births -- spec_resume is never set to Some, and the other
LiminalEntry constructors are under #[cfg(test)].
matthewevans
force-pushed
the
cr733/copy-token-births
branch
from
July 26, 2026 06:29
a5ff2a9 to
13ab7c6
Compare
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.
Copy tokens were the last unjournaled token birth. Both production copy
seams now resolve-and-journal through the same command the ordinary
CR 111.1 birth uses, and replay materializes them from the record.
Parameterize, don't proliferate — copy extends ResolvedTokenCreationCommand
rather than adding a sibling variant. The journaled axis is object creation
(CR 111.1: "Some effects put tokens onto the battlefield") for BOTH bodies:
an object came into existence and its id and CR 613.7d timestamp were drawn.
CR 707.2 governs only how the body was computed upstream of that seam, so it
parameterizes the payload instead of forking the family, and the axis stays
inside a single CR section as the categorical-boundary rule requires. The
shipped scope comment on that struct already prescribed exactly this.
spec + token_image_ref -> body: ResolvedTokenBody::{Spec, Copy}
One shared body authority per shape, so resolve and replay cannot drift:
materialize_token_copy_body is now called by the liminal build, the direct
create_object path, and the applier. That also removes a duplicated keyword
grant and a redundant loyalty seed (install_copiable_values_as_base already
installs values.loyalty, which is what copied_loyalty was).
Fail-closed on the case that cannot be replayed exactly. The direct seam
applies its CR 707.9 exceptions AFTER the birth via apply_token_modifications
-- pausable, state-level, and with no resolved family yet -- so those births
record DeferredToUnjournaledSeam and the applier returns
UnreplayableCopyModifications instead of installing a body silently missing
them. An absent entry is detectable; a present-but-wrong one is not. The
refusal is greppable and disappears when that seam gets its own family.
all_creature_types is recorded rather than re-read at replay: changeling
(CR 702.73a) and type-changing effects mutate it, and remove_subtype_set
consults the live list.
Which seam a copy takes is decided by its TARGET, not by its own text.
Applied Geometry's exceptions all parse to SetPower/SetToughness/AddSubtype/
AddType (all liminal-immediate) and its "Put six +1/+1 counters on it" parses
as a separate effect, so copying a plain permanent takes the liminal seam
(Folded) while copying one that itself enters with counters makes
etb_counters non-empty and takes the direct seam (Deferred). All three body
arms are covered by production-path tests on that basis.
Meld is NOT in this family. finish_meld_entry reuses the existing component
object's id and moves it through the ordinary zone pipeline -- it draws no
id, sets no is_token, and materializes nothing -- so an applier whose whole
contract is materializing a subject cannot own it.
The LiminalEntry premise that blocked this unit was wrong: it already carries
object: GameObject plus copy_resume/spec_resume, so no field was added and
no serialized contract changed. Verified copy births are the ONLY production
liminal births -- spec_resume is never set to Some, and the other
LiminalEntry constructors are under #[cfg(test)].
Follow-up rider (NOT fixed here): apply_resolved_token_creation reads
state.turn_number live rather than recording it, so a replay could observe a
different value once turn advancement is journaled.
Summary by CodeRabbit