Fix Scythecat Cub

[traemyn]

Summary

Fixes Scythecat Cub's second landfall resolution so the explicitly chosen creature's +1/+1 counters are doubled instead of resolving against the entering land.

Files changed

• crates/engine/src/game/effects/counters.rs
• crates/engine/tests/integration/main.rs
• crates/engine/tests/integration/scythecat_cub_second_resolution.rs

Track

Developer

LLM

Model: GitHub Copilot (canonical id not exposed)
Tier: Frontier
Thinking: high

Implementation method (required)

Method: /engine-implementer

Note

Any change to crates/engine/ game logic — parser, effects, resolver,
targeting, rules behavior — is expected to go through /engine-implementer.
The "not used" box is for changes that genuinely fall outside that scope.

CR references

• CR 608.2c — later text in a resolving ability can modify the meaning of earlier text; the chained counter effect must retain the parent instruction's chosen object.

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 — PASS

• cargo clippy -p phase-engine --all-targets -- -D warnings — PASS

• cargo test -p phase-engine --test integration scythecat_cub_second_resolution::scythecat_cub_doubles_counters_on_second_landfall_resolution -- --exact --nocapture — PASS (1 test)

• cargo test -p phase-engine --lib effects::counters -- --nocapture — PASS (62 tests)

• cargo test -p phase-engine — started successfully; manually interrupted during long-running unrelated loop tests, with no test failure reported; CI should run the complete suite.

• cargo coverage — tool built, but card-level coverage could not run because data/card-data.json is absent in this dedicated worktree; CI-owned alternative is the generated card-data pipeline.

• cargo semantic-audit — tool built, but the audit could not run because client/public/card-data.json is absent in this dedicated worktree; CI-owned alternative is the generated card-data pipeline.

Gate A

Gate A PASS head=35e45338b37f7d953e8652ced0cb8616720ddcb2 base=97591656218103d8e8c7315725b24cfe64645dd4

Anchored on

• crates/engine/src/game/effects/counters.rs:1914 — existing ParentTarget branch distinguishes chooser-only bookkeeping from an absent object target.
• crates/engine/src/game/effects/counters.rs:1929 — existing batched ParentTarget resolution uses the same has_object_target guard and shared target-resolution vocabulary.

Final review-impl

Final review-impl PASS head=35e45338b37f7d953e8652ced0cb8616720ddcb2

Claimed parse impact

None.

Scope Expansion

None.

Validation Failures

None.

CI Failures

• Full cargo test -p phase-engine was not completed locally because long-running unrelated loop tests were manually interrupted; no failure was observed before interruption.
• cargo coverage and cargo semantic-audit require generated data/card-data.json and client/public/card-data.json, which are absent from this dedicated worktree; CI must run the generated card-data pipeline before these audits.

Summary by CodeRabbit

• Bug Fixes

  • Fixed chained counter effects so explicitly selected creature targets resolve correctly.
  • Corrected Scythecat Cub’s second landfall resolution so it properly doubles the counters placed on the selected creature.
  • Improved consistency when resolving repeated abilities that reference the same target.

• Tests

  • Added integration coverage for repeated landfall resolutions, target validation, shared ability behavior, and resulting counter totals.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 8e40a22b-7296-4e42-a0ae-16deb2f41d59

📥 Commits

Reviewing files that changed from the base of the PR and between 35e4533 and 668d930.

📒 Files selected for processing (1)

• crates/engine/tests/integration/scythecat_cub_second_resolution.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• crates/engine/tests/integration/scythecat_cub_second_resolution.rs

---

📝 Walkthrough

Walkthrough

The counter-effect resolver now propagates explicitly selected live object targets for chained ParentTarget effects. A new integration test verifies that successive Scythecat Cub landfall resolutions increase Lotus Cobra’s counter total from one to two and then four.

Changes

Counter target propagation

Layer / File(s)	Summary
Live object ParentTarget resolution crates/engine/src/game/effects/counters.rs	ParentTarget counter effects now prioritize selected live object targets and exclude player-only bookkeeping targets.
Scythecat Cub integration coverage crates/engine/tests/integration/scythecat_cub_second_resolution.rs, crates/engine/tests/integration/main.rs	The registered integration test validates Lotus Cobra as a target, checks counter totals of two and four after successive resolutions, and confirms both triggers use the printed ability index.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• phase-rs/phase#6310: Both changes update resolve_defined_or_targets for ParentTarget counter resolution.
• phase-rs/phase#6346: Both changes preserve explicitly selected targets across chained abilities.
• phase-rs/phase#7099: Both changes resolve ParentTarget effects through live object targets in counters.rs.

Suggested labels: bug

Suggested reviewers: matthewevans

🚥 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 identifies the main change: fixing Scythecat Cub’s second landfall resolution.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.}

[coderabbitai]

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/tests/integration/scythecat_cub_second_resolution.rs`:
- Around line 62-80: Update the test setup to seed Lotus Cobra with one +1/+1
counter before the first resolve_cub_landfall call. Adjust the first assertion
to expect two counters and the second assertion to expect four, ensuring the
second resolution verifies doubling rather than another single-counter
placement.

🪄 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: d8ea55b3-432a-46b3-876e-90b74e72a2e5

📥 Commits

Reviewing files that changed from the base of the PR and between 8879b91 and 35e4533.

📒 Files selected for processing (3)

• crates/engine/src/game/effects/counters.rs
• crates/engine/tests/integration/main.rs
• crates/engine/tests/integration/scythecat_cub_second_resolution.rs

[github-actions]

Generated for head 668d9300c1f46f67af7243da26f162fc82dfeb5c.

Parse changes introduced by this PR

✓ No card-parse changes detected.

[matthewevans]

Maintainer review is complete for head 668d9300c1f46f67af7243da26f162fc82dfeb5c.

The maintainer test fix resolves CodeRabbit’s finding: seeding the target at one +1/+1 counter and asserting 2 then 4 makes the second landfall multiplication regression discriminating. The logic review is otherwise clean.

Required CI is still running, and the coverage-parse-diff sticky comment is still bound to the prior head. I will resume approval and merge-queue handling after required checks and a current-head parse-diff result are available.

[matthewevans]

Approved — current head is ready for the merge queue.

Verified 668d9300c1f46f67af7243da26f162fc82dfeb5c: the counter resolver now uses the existing propagated live-object target authority for a chained ParentTarget, ahead of event-context fallback; the integration regression drives land play, target selection, stack resolution, and distinguishes the second-resolution multiplication (1 → 2 → 4). Required CI is green and the current-head parse-diff reports no card-parse changes. CodeRabbit's prior test-discrimination finding is resolved on this head.