Add Render Silent#5110
Conversation
Counter target spell. Its controller can't cast spells this turn.
The second sentence was previously swallowed (coverage gap_count 1). Add the
general primitive "affected player = controller of the parent effect's object
target" to the temporary-restriction subsystem, composing the existing
`parent_target_controller` anaphora resolver (stack -> last-known-information,
CR 608.2h) with the established "<player> can't cast spells this turn"
restriction-effect path (Silence / Xantid Swarm).
- types: nullary `RestrictionPlayerScope::ParentObjectTargetController`
(CR 109.4 / 608.2c / 608.2h). Distinct from `ParentTargetedPlayer` (a reused
player target) — this derives a player from an object target's controller.
- parser: one `value(.., tag("its controller"))` arm in
`try_parse_cant_cast_spells_effect` (nom combinator; Gate A clean).
- resolver: `add_restriction::fill_runtime_fields` lowers the scope to
`SpecificPlayer` at restriction-creation time (capture-at-resolution, after
the countered spell has left the stack); fail-closed if no object referent.
- exhaustive-match arms at casting / combat / derived_views (no wildcard).
Generalizes to the class "Counter/Destroy/Tap target X. Its controller can't
cast spells this turn". Verified: coverage supported:true gap_count:0,
semantic-audit 0 findings, discriminating cast-pipeline test (countered spell's
controller barred, caster free, non-vacuous reach-guard).
Co-Authored-By: Claude Opus 4.8 <[email protected]>
Claude-Session: https://claude.ai/code/session_01WQcP6woAkU5iEBTkA9oQ6J
![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 implements the ParentObjectTargetController restriction player scope to support cards like Render Silent, resolving the restriction to the controller of the parent effect's object target using last-known information. Feedback on the changes suggests removing a debug_assert! in restriction_scope_matches_player because it will trigger unexpected panics in debug builds and tests when the scope is legitimately left unresolved as a valid fail-closed state.
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.
| RestrictionPlayerScope::ParentObjectTargetController => { | ||
| // CR 109.4: resolved to `SpecificPlayer` by `add_restriction` when the | ||
| // restriction is created (via `parent_target_controller`), so an | ||
| // unresolved scope here means the object referent could not be found — | ||
| // fail-closed, restrict no one. | ||
| debug_assert!( | ||
| false, | ||
| "ParentObjectTargetController should be resolved by add_restriction" | ||
| ); | ||
| false | ||
| } |
There was a problem hiding this comment.
The debug_assert!(false, ...) here will trigger a panic in debug builds and tests whenever a ParentObjectTargetController restriction remains unresolved (e.g., when there is no parent object target, which is a valid fail-closed state tested in parent_object_target_controller_unresolved_without_object_target). Since this is a valid fallback state that should gracefully restrict no one, the debug_assert! should be removed to prevent unexpected panics during gameplay or testing.
RestrictionPlayerScope::ParentObjectTargetController => {
// CR 109.4: resolved to SpecificPlayer by add_restriction when the
// restriction is created (via parent_target_controller), so an
// unresolved scope here means the object referent could not be found —
// fail-closed, restrict no one.
false
}
github-actions
Bot
added
the
needs-maintainer
Parse changes introduced by this PR · 15 card(s), 23 signature(s) (baseline: main
|
|
Holding maintainer review for now because this parser/engine PR still has a |
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
[MED] Please add a discriminating test for the owner-vs-controller branch before this lands. Evidence: crates/engine/tests/integration/render_silent_cant_cast.rs explicitly says the owner != controller last-known-information branch is intentionally not exercised, and crates/engine/src/game/effects/add_restriction.rs relies on parent_target_controller to bind RestrictionPlayerScope::ParentObjectTargetController at restriction creation. That is the important correctness boundary for this new scope: Render Silent must restrict the controller/caster of the countered spell, not the card owner and not the Render Silent caster. The current integration fixture covers only owner==controller, so it would not fail if a later stack-exit/reset path or lookup change made this scope collapse to owner in the exact case the implementation comments claim to handle.
A good proof would set up a stack spell whose owner differs from its controller, resolve Render Silent against it, and assert the spell controller is barred from casting while the owner remains able to cast. This can be an integration test or a focused add_restriction/parent_target_controller unit test, but it needs to cover the branch currently called out as untested.
Address maintainer review: add a discriminating integration test for the owner-vs-controller branch of the "its controller can't cast spells this turn" restriction. The countered spell is OWNED by P0 (also the Render Silent caster) but CONTROLLED by P1, so the restriction must bind to the controller (P1) — a collapse to the owner (CR 701.6a graveyard owner) or to the caster's self.controller (both P0) is ruled out by asserting P0 stays castable while P1 is barred. Exercises the path previously called out as untested: after the counter the spell has left the stack with no LKI snapshot (LKI is captured only on battlefield/exile exit), so parent_target_controller reads the graveyard object's retained controller, which stays P1 because a stack->graveyard move does not run reset_for_battlefield_exit. A later stack-exit reset or lookup change that collapsed this to the owner would fail the new test. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01WQcP6woAkU5iEBTkA9oQ6J
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
The owner-vs-controller integration test addresses the previous review, but there is still one current-head blocker before this is ready.
The PR explicitly documents and tests the unresolved ParentObjectTargetController state as fail-closed in add_restriction: parent_object_target_controller_unresolved_without_object_target leaves affected_players: ParentObjectTargetController stored when there is no object target. The enforcement arm in casting::restriction_scope_matches_player also says that unresolved state should "fail-closed, restrict no one", but it still executes debug_assert!(false, "ParentObjectTargetController should be resolved by add_restriction") before returning false.
That means the fail-closed path is not actually safe in debug/test builds: any later castability query while that hostile fixture state exists will panic instead of returning unrestricted. Please remove the debug assertion for this specific fail-closed arm, or change the model so unresolved ParentObjectTargetController cannot be stored. Add a small assertion that drives can_cast_object_now against the unresolved fixture so the fail-closed behavior is proven, not just the stored enum shape.
… fail-closed, not panic Address maintainer review: the enforcement arm in casting::restriction_scope_matches_player did debug_assert!(false) for RestrictionPlayerScope::ParentObjectTargetController, but add_restriction's fill_runtime_fields can legitimately leave that scope unresolved when there is no object referent (proven by parent_object_target_controller_unresolved_without_object_target). That made the documented fail-closed path unsafe: a castability query against that stored state would panic in debug/test builds instead of restricting no one. Remove the debug assertion for this arm so it genuinely returns false (fail-closed). Unlike the always-resolved sibling scopes (TargetedPlayer, ScopedPlayer), this scope can reachably remain unresolved, so an unresolved value is a valid state, not a bug. Add an integration test that drives the public can_cast_object_now against the stored unresolved restriction and asserts it restricts no one without panicking — proving the fail-closed behavior, not just the stored enum shape. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01WQcP6woAkU5iEBTkA9oQ6J
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
[HIGH] The current parse-diff has unrelated card-level blast radius for a Render Silent PR. Evidence: the current <!-- coverage-parse-diff --> sticky reports 15 cards / 23 signatures, including Curse of Disturbance/Opulence/Verbosity losing attacking, Curtain of Light/Dazzling Beauty BecomeBlocked, Trap Runner, Fog Patch, Choking Vines, Kookus, Urborg Panther, Wirefly Hive, Galactus, and others, while the PR summary scopes the change to Render Silent / its controller can't cast spells this turn.
Why it matters: parser-surface approvals require confronting the card-level diff, and unexplained gains/losses outside the stated class are scope contamination even when CI is green. Suggested fix: reduce/rebase the branch so this PR's parse diff is only Render Silent, or explicitly split/explain the unrelated parser changes with tests and scope.
3 participants
Summary
Adds engine support for Render Silent — "Counter target spell. Its controller can't cast spells this turn."
The first sentence already lowered to
Effect::Counter; the second was silently swallowed (gap_count: 1). This adds the general primitive "affected player = controller of the parent effect's object target" to the temporary-restriction subsystem, composing the existingparent_target_controlleranaphora resolver (stack → last-known-information, CR 608.2h) with the established "<player>can't cast spells this turn" restriction-effect path (Silence / Xantid Swarm). Built for the class — "Counter/Destroy/Tap target X. Its controller can't cast spells this turn" — not the single card.Files changed
crates/engine/src/types/ability.rscrates/engine/src/parser/oracle_effect/mod.rscrates/engine/src/game/effects/add_restriction.rscrates/engine/src/game/casting.rscrates/engine/src/game/combat.rscrates/engine/src/game/derived_views.rscrates/engine/src/parser/oracle_effect/tests.rscrates/engine/tests/integration/main.rscrates/engine/tests/integration/render_silent_cant_cast.rs(new)Key design
RestrictionPlayerScope::ParentObjectTargetController. Distinct fromParentTargetedPlayer(a reused player target, which routes throughtarget_player()'sunwrap_or(self.controller)fallback and would wrongly restrict the caster) — this derives a player from an object target's controller. Mirrors the engine-nativePlayerFilter::ParentObjectTargetController/PlayerScope::ParentObjectTargetControlleron the sibling enums. Additive nullary variant on a#[serde(tag,content)]enum — no wire migration.value(RestrictionPlayerScope::ParentObjectTargetController, tag("its controller"))arm intry_parse_cant_cast_spells_effect(nom combinator, no string dispatch).add_restriction::fill_runtime_fieldslowers the scope toSpecificPlayerat restriction-creation time (capture-at-resolution, after the countered spell has left the stack — resolved via last-known information); fail-closed if there is no object referent.casting.rs/combat.rs/derived_views.rs— no wildcard fallback (the variant is always lowered toSpecificPlayerbefore storage).CR references
CR 109.4— controller of an object ("its controller")CR 608.2c— later text modifies earlier text; anaphor resolution orderCR 608.2h— value determined once / last-known information (countered spell has left the stack)CR 611.2a— continuous effect from a resolved spell lasts "until end of turn"CR 701.6 / 701.6a— Counter → owner's graveyardCR 101.2 / 601.2a / 602.5— casting-prohibition domainTrack
Developer
LLM
Model: Claude Opus 4.8
Thinking: high
Tier: Frontier
Gate A
./scripts/check-parser-combinators.sh→ exit 0 (no violations; the script prints no output on success). The only added parser line is avalue(_, tag("its controller"))combinator arm — no.contains/.split_once/.starts_with/ match-arm string literals introduced.Anchored on
crates/engine/src/parser/oracle_effect/mod.rs:2804—value(RestrictionPlayerScope::ParentTargetedPlayer, tag("that player")): sibling anaphora scope arm in the samealt(), samevalue(_, tag(...))combinator family that mytag("its controller")arm mirrors.crates/engine/src/parser/oracle_effect/mod.rs:2819—value(RestrictionPlayerScope::DefendingPlayer, tag("defending player")): sibling non-target-derived scope arm in the samealt()(Xantid Swarm), same combinator shape.crates/engine/src/game/effects/add_restriction.rs:120—RestrictionPlayerScope::DefendingPlayer => { … SpecificPlayer(p) }: the capture-at-resolution lowering my newParentObjectTargetControllerarm follows exactly.crates/engine/src/game/effects/add_restriction.rs:138—RestrictionPlayerScope::ScopedPlayer => { … SpecificPlayer(p) }: second analog of the same placeholder-scope →SpecificPlayerresolution pattern.Verification
Developer track — all Step 6 commands ran clean:
cargo fmt --all— clean./scripts/check-parser-combinators.sh— clean (exit 0)cargo clippy-strict— clean (exit 0)cargo test -p engine --lib -- render_silent parent_object_target_controller— 3 pass / 0 failcargo test -p engine --test integration -- render_silent— 2 pass / 0 fail (drives the real cast pipeline: countered spell's controller barred, caster still free; positive reach-guard makes the negative non-vacuous)./scripts/gen-card-data.sh— Render Silent: 0 Unimplemented entries (both sentences parsed)cargo coverage— Render Silent:supported: true,gap_count: 0cargo semantic-audit— Render Silent: 0 findingsScope Expansion
None.
Validation Failures
None.
CI Failures
None.
🤖 Generated with Claude Code
https://claude.ai/code/session_01WQcP6woAkU5iEBTkA9oQ6J
Generated by Claude Code