fix(engine): retain tracked delayed target pins

[matthewevans]

Fixes #7100.

Rebased replacement for #7283. When a delayed ChangeZone is upgraded to a tracked-set mass move, retain ParentTarget/slot identity through binding and evaluate its member predicate against the delayed trigger's current incarnation pin. This prevents a later incarnation from being moved while retaining the original member path.

Verification: cargo fmt --all; parser combinator and PreLowered pre-commit gates. Per workspace policy, no cargo build/test was run in this worktree.

Summary by CodeRabbit

• Bug Fixes
  • Fixed delayed zone-change effects that return tracked objects from exile to the battlefield.
  • Preserved parent-object targeting information during delayed effects, including nested filters.
  • Prevented delayed effects from incorrectly affecting newer incarnations of previously targeted objects.
  • Improved target validation for mass zone changes involving tracked sets.
• Tests
  • Added regression coverage for delayed returns and object-incarnation targeting.

[coderabbitai]

Warning

Review limit reached

@matthewevans, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 7 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 8736704f-5eda-4fd6-a3db-fa9532f9f4ca

📥 Commits

Reviewing files that changed from the base of the PR and between 2728351 and 13beec3.

📒 Files selected for processing (5)

• crates/engine/src/game/ability_utils.rs
• crates/engine/src/game/effects/change_zone.rs
• crates/engine/src/game/effects/delayed_trigger.rs
• crates/engine/src/game/effects/mod.rs
• crates/engine/src/game/engine.rs

📝 Walkthrough

Walkthrough

The change preserves ParentTarget references during delayed tracked-set conversion, captures nested references in ChangeZoneAll, and validates target incarnations during mass zone changes. Regression tests cover stale incarnations and delayed exile-to-battlefield returns.

Changes

Tracked-set delayed zone changes

Layer / File(s)	Summary
Preserve ParentTarget filters crates/engine/src/game/effects/delayed_trigger.rs, crates/engine/src/game/effects/mod.rs, crates/engine/src/game/effects/delayed_trigger.rs	Tracked-set conversion preserves ParentTarget and ParentTargetSlot filters. Parent-target detection now traverses nested TrackedSetFiltered filters.
Validate ChangeZoneAll incarnations crates/engine/src/game/effects/change_zone.rs, crates/engine/src/game/filter.rs, crates/engine/src/game/ability_utils.rs, crates/engine/src/game/engine.rs	ChangeZoneAll binds delayed parent references and validates current incarnations or immediate delayed-exile successors. Regression tests cover stale targets and delayed returns. Source-coordinate pins are updated.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

• phase-rs/phase#7099: Introduced delayed-trigger target-incarnation pinning extended by this change.
• phase-rs/phase#7257: Modified delayed tracked-set and ChangeZoneAll handling.
• phase-rs/phase#7283: Covered related ChangeZoneAll, ParentTarget, and TrackedSetFiltered behavior.

Suggested labels: bug

Suggested reviewers: lgray, kiannidev, andriypolanski

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the primary fix: retaining delayed tracked target pins.
Linked Issues check	✅ Passed	The changes preserve ParentTarget identities and validate current incarnations for delayed tracked-set moves, addressing issue #7100.
Out of Scope Changes check	✅ Passed	All changes support delayed tracked-set target pinning and related source-location updates; no unrelated code changes are evident.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ship/fix-engine-retain-tracked-delayed-target-pins-rebased

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Generated for head 13beec36ff685f3a46551d0b1edc85d9a1b7ea44.

Parse changes introduced by this PR

✓ No card-parse changes detected.

[coderabbitai]

Actionable comments posted: 3

🤖 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/ability_utils.rs`:
- Around line 2097-2103: The ChangeZoneAll exemption currently skips target
validation for all filters, including real player targets. In the ChangeZoneAll
branch near validated.targets.clone(), apply the exemption only when
filter_refs_parent_target(target) (or an equivalent existing helper) is true;
otherwise run targeting::validate_targets_for_ability so illegal departed-player
targets are removed. Add a negative test covering an illegal
TargetFilter::Player target alongside the existing delayed-parent snapshot test.

In `@crates/engine/src/game/effects/change_zone.rs`:
- Around line 209-267: Replace the leaf-binding logic in
bind_delayed_parent_target_filter with the shared concrete_parent_target_filter
helper from delayed_trigger.rs. Extract or expose that helper so both
ParentTarget and ParentTargetSlot { index } use the same empty-set and
out-of-range semantics, including returning Any where required. Keep only
recursive handling of TrackedSetFiltered, And, Or, and Not local to
bind_delayed_parent_target_filter.
- Around line 269-288: Update target_pin_is_current_or_delayed_exile_successor
to bind the +1 incarnation allowance to the exact parent exile occurrence,
rather than accepting any later same-ID incarnation; use the available
tracked-object/ThisWayCause data or extend it to retain that occurrence. Ensure
delayed-exile successors are accepted only when tied to the parent exile event,
and add a regression test covering an unrelated intervening zone change.

🪄 Autofix

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: 27fe7ddb-4348-4a42-a694-08ee3d014d81

📥 Commits

Reviewing files that changed from the base of the PR and between d38f364 and 2728351.

📒 Files selected for processing (6)

• crates/engine/src/game/ability_utils.rs
• crates/engine/src/game/effects/change_zone.rs
• crates/engine/src/game/effects/delayed_trigger.rs
• crates/engine/src/game/effects/mod.rs
• crates/engine/src/game/engine.rs
• crates/engine/src/game/filter.rs

[matthewevans]

Review follow-up for the current head 5221672b6b:

• The ChangeZoneAll player-target revalidation is in 471efd95eb.
• The parent-target leaf binding now delegates to delayed_trigger::concrete_parent_target_filter; local code only traverses filter containers.
• I verified the requested delayed-exile “intervening zone change” case against the identity model. Every real zone change advances the single GameObject::bump_incarnation counter, making the parent exile uniquely pin + 1; any later zone change is pin + 2 or higher and is rejected. No duplicate event ledger is required.

I could not attach these as inline replies because the account already has a pending review on this PR; this comment records the current-head disposition without modifying that review.