fix(engine): resolve enters_under for any player ControllerRef (CR 110.2a)

[galuis116]

Summary

Fixes the engine side of #2813 (CR 110.2a). resolve_enters_under_player (game/effects/change_zone.rs) only mapped ControllerRef::You to a concrete PlayerId and returned Err("not yet supported") for every other variant. So any effect that puts a card onto the battlefield under a player other than the ability controller — "under their control" (each-player), "under target player's control", "that permanent's controller puts ..." — failed mid-resolution. This is a silent class gap: the card parses (the enters_under carrier is produced), so coverage counted it "supported," but Effect::ChangeZone / Effect::ChangeZoneAll errored at runtime.

Affected class: each-player / target-player mass put-into-play and reanimation (Tempting Wurm, Show and Tell, Hypergenesis, Warp World, Scrambleverse, "under target player's control" reanimation, …).

Change

• game/filter.rs — make controller_ref_player (the single authority for ControllerRef -> PlayerId) pub(crate).
• game/effects/change_zone.rs — resolve_enters_under_player now delegates to controller_ref_player, threading state + ability, so every player reference resolves consistently: You, ScopedPlayer (CR 115.10 / 608.2c), TargetPlayer (CR 109.4), ParentTargetController, etc. Both call sites (ChangeZone, ChangeZoneAll) updated.
• None still means the owner's control. A ControllerRef that genuinely cannot resolve to a single player in this context (e.g. bare Opponent in multiplayer with no target) keeps the explicit Err — no silent guessing, exhaustive match, no new engine variant.

Tests

• enters_under_target_player_puts_card_under_chosen_player — card enters under the chosen TargetPlayer, not the ability controller.
• enters_under_scoped_player_uses_iterating_player — under player_scope, the card enters under the scoped (iterating) player.
• The pre-existing change_zone_all_strict_fails_on_unsupported_enters_under_controller_ref still passes, confirming the strict-fail invariant is preserved for genuinely-unresolvable refs.
• Full engine suite green (11,234 passed); clippy -D warnings and fmt clean.

Fixes #2813

[gemini-code-assist]

Code Review

This pull request updates the zone-change resolution logic to support resolving arbitrary ControllerRef overrides (such as TargetPlayer and ScopedPlayer) via the canonical controller_ref_player resolver, aligning with CR 110.2a. It also adds corresponding unit tests. The review feedback points out a missing mandatory CR annotation comment on a rules-touching line of engine code in resolve_all, violating rule R6 of the style guide.

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.

[gemini-code-assist]

CR 110.2a: The call to resolve_enters_under_player in resolve_all is a rules-touching line of engine code but lacks a mandatory CR annotation comment. Adding a CR 110.2a comment here ensures compliance with the strict R6 rule of the style guide.

                // CR 110.2a: Resolve the controller-override reference to a concrete PlayerId.
                resolve_enters_under_player(state, ability, "ChangeZoneAll", enters_under.as_ref())?

References

1. R6. CR annotations are mandatory and verified. Every rules-touching line of engine code must carry a comment of the form CR <number>: <description>. ^(link)

[matthewevans]

Review: PR #2817 — resolve enters_under for any player ControllerRef (CR 110.2a)

Verdict: APPROVE (correct seam, idiomatic, real fix). One MED context clarification on end-to-end class reach; one refutation of the Gemini comment. Reviewed via API diff + worktree off origin/main (no gh pr diff).

(a) Single-authority delegation + both call sites — CORRECT

• The bug is real on origin/main: resolve_enters_under_player matched only Some(ControllerRef::You) and Err'd every other variant (change_zone.rs:59-76 @ main). Confirmed by reading git show origin/main:....
• controller_ref_player in filter.rs:650 is the genuine single authority for ControllerRef → Option<PlayerId> — an exhaustive match over all 9 variants, already used at filter.rs:2833 and :3104. Delegating to it is exactly the seam the issue ([Bug]: enters_under only supports ControllerRef::You — "under that/their/target player's control" put-onto-battlefield errors at resolution (CR 110.2a) #2813) prescribed.
• Call args (state, ability.source_id, Some(ability.controller), Some(ability), cref) match the existing-caller convention (source.id, source.controller, source.ability, c). You → source_controller, ScopedPlayer → scoped_player_or_controller (reads ability.scoped_player, filter.rs:588), TargetPlayer → ability.targets first Player ref. Context threading is correct.
• Both call sites updated consistently: ChangeZone (change_zone.rs:189) and ChangeZoneAll (:832). The pub(crate) lift (filter.rs:650) is the minimal-correct exposure (crate-internal, not pub).

(b) Strict-fail invariant — PRESERVED

• controller_ref_player returns None for Opponent and genuinely-unresolvable contexts; the delegation maps None → Err(InvalidParam) via ok_or_else, so no silent guessing. Match is exhaustive (None/Some), no wildcard.
• Pre-existing change_zone_all_strict_fails_on_unsupported_enters_under_controller_ref (change_zone.rs:4620) still holds: it uses ControllerRef::Opponent, and its assertions check msg.contains("CR 110.2a") / "ChangeZoneAll" / "Opponent" — all satisfied by the rewritten error string. No brittle exact-message assertion.

(c) CR annotations — ALL VERIFIED against docs/MagicCompRules.txt

• CR 110.2a (line 618): "If an effect instructs a player to put an object onto the battlefield, that object enters the battlefield under that player's control unless the effect states otherwise." Exactly the rule implemented. ✓
• CR 109.4 (control, line 594) ✓; CR 115.10 / 115.10b ("you" doesn't indicate control of non-targeted player, line 884/888) ✓; CR 608.2c (controller follows instructions, line 2789) ✓. No hallucinated numbers.

(d) Test discrimination — DISCRIMINATING, drive real pipeline

• Both new tests call resolve(&mut state, &ability, ...) (the production resolver), not hand-constructed state. enters_under_player flows through ctx.enters_under_player → execute_zone_move → controller_override on the proposed zone-change event (zone_pipeline.rs:884), and they assert state.objects[&obj_id].controller.
• enters_under_target_player...: controller PlayerId(0), target Player(PlayerId(1)), asserts controller == PlayerId(1). On origin/main this hits Some(other) => Err, so .unwrap() panics — the test fails on main, passes with fix. ✓
• enters_under_scoped_player...: controller PlayerId(0), set_scoped_player_recursive(PlayerId(1)), asserts == PlayerId(1). Same fail-on-main/pass-with-fix discrimination. ✓

(e) Relationship to #2803 — #2817 SUPERSEDES #2803's enters-under change

• Fix targeting and controller assignment bugs #2803 (Kelvinchen03, changes-requested) fixed the same Tempting Wurm bug by adding an origin param to resolve_enters_under_player and applying the override only for Hand→Battlefield moves (per its own description). That is a wrong-seam discriminator that flips a default for a card-shape rather than resolving the ControllerRef. fix(engine): resolve enters_under for any player ControllerRef (CR 110.2a) #2817 fixes it at the correct seam (delegate to the canonical resolver), making Fix targeting and controller assignment bugs #2803's enters-under hack unnecessary and incorrect. They would conflict (both edit resolve_enters_under_player's signature/body). Recommendation: land fix(engine): resolve enters_under for any player ControllerRef (CR 110.2a) #2817's enters-under fix; drop Fix targeting and controller assignment bugs #2803's change_zone.rs change (its other two fixes — Grenzo Grenzo, Havoc Raiser: trigger targets the player who dealt damage instead of the player who received it #2346, Aven Interrupter Aven Interrupter: ETB trigger exiles itself instead of the target spell #2351 — are independent and out of scope here).

Findings

[MED] End-to-end card class is not actually unblocked by this PR alone — the fix is latent. Evidence: the parser only ever emits enters_under: Some(ControllerRef::You) or None (oracle_effect/lower.rs:3228 and the alt at :3284-3292 recognizes only " under your control" / owner phrases / empty). No parser path emits ScopedPlayer/TargetPlayer/ParentTargetController into enters_under, and execute_zone_move defaults to owner's control when controller_override is None (zone_pipeline.rs:884). Why it matters: the PR/issue framing implies cards like Tempting Wurm / Show and Tell / Hypergenesis are fixed, but until the parser is extended to emit the non-You variant for "under their/that/target player's control", those cards still don't flow through the new resolver path (the new tests construct the ResolvedAbility directly). The resolver fix is correct and is the right prerequisite — strict-fail keeps the gap visible — but it is the engine half of a two-part fix. Suggested: land as-is (the issue explicitly scoped this as the resolver/engine side); track the parser emission of non-You enters_under as the follow-up that actually delivers the card class.

[LOW — refute Gemini] Gemini asks for a CR 110.2a comment on the resolve_all call site (change_zone.rs:832). Not warranted: resolve_enters_under_player itself carries the CR 110.2a doc block (:64-71) and the ChangeZone call site is annotated (:186-188); the call is plumbing into an already-annotated rules function, not a fresh rules-touching line. Refute, don't echo.

Three lenses

• Correct seam: ✓ delegates to the designated single authority; removes a 2-arm hand-roll.
• Idiomatic: ✓ exhaustive match, no bool/new variant, minimal pub(crate) lift, reuses controller_ref_player, CR-verified.
• Value: ✓ real fix for a silent runtime class gap with discriminating tests and preserved strict-fail — though full card-class impact awaits the parser follow-up (MED above).

Note: Tilt runs against origin/main, not this branch, so its test-engine/clippy green does not cover the PR; analysis above is from reading the worktree diff. Tests are card-data-independent (create_object synthetic cards).

[natefinch]

Review: PR #2817 — resolve enters_under for any player ControllerRef (CR 110.2a)

Independent review (API diff + read of origin/main working tree; gh pr diff avoided). Confirms the real fix and the seam; one MED caveat on actual card-class reach; Gemini nit refuted.

Seam + idiom — CORRECT

• The bug is real: on origin/main, resolve_enters_under_player matched only Some(ControllerRef::You) and Errd every other variant (change_zone.rs:67-76). Delegating to controller_ref_player (filter.rs:650) — the exhaustive 9-variant single authority already used at filter.rs:2833/:3104 — is exactly the right seam.
• Call args (state, ability.source_id, Some(ability.controller), Some(ability), cref) match the existing-caller convention (source.id, source.controller, source.ability, c). You to source_controller, ScopedPlayer to scoped_player_or_controller (reads ability.scoped_player, filter.rs:588), TargetPlayer to the first Player ref in ability.targets. Threading verified.
• Both call sites updated (ChangeZone :189, ChangeZoneAll :832); pub(crate) is the minimal exposure. Exhaustive None/Some match, no wildcard, no new bool/variant.

Strict-fail invariant — PRESERVED

• controller_ref_player returns None for Opponent/unresolvable contexts; the delegation maps None to Err(InvalidParam) via ok_or_else, so no silent guessing. The new message still contains CR 110.2a + effect name + Opponent, so BOTH pre-existing strict-fail tests hold — change_zone_all_strict_fails_... (:4522, asserts ChangeZoneAll+Opponent) and resolver_strict_fails_on_opponent_... (:5885, asserts Opponent). Neither asserts the old "not yet supported" wording.

Tests — DISCRIMINATING

• Both new tests drive the production resolve(...) and assert state.objects[&obj_id].controller. On origin/main the non-You variants hit Some(other) => Err, so .unwrap() panics, i.e. fail-on-main / pass-with-fix. The ScopedPlayer test correctly seeds via set_scoped_player_recursive.

CR annotations — VERIFIED

• CR 110.2a resolves in docs/MagicCompRules.txt and describes "enters the battlefield under that player's control unless the effect states otherwise" — exactly this code. CR 109.4 / 115.10 in the test docs check out. No hallucinated numbers.

Findings

[MED] Fix is latent end-to-end — no parser path emits a non-You enters_under. Evidence: every emission site sets Some(ControllerRef::You) only when the clause contains "under your control", else None (oracle_effect/sequence.rs:2983, :3074; same shape in oracle_trigger.rs:21885). No path emits TargetPlayer/ScopedPlayer/ParentTargetController, and the new tests construct ResolvedAbility directly. Why it matters: the title/issue framing implies cards entering "under target/their/that player's control" are unblocked, but none flow through the new resolver until the parser is extended. Suggested fix: land as the engine half (correct prerequisite; strict-fail keeps the gap visible) and track parser emission of the non-You enters_under variant as the follow-up that actually delivers the card class.

[LOW — refute Gemini] Gemini asks for a CR 110.2a comment on the resolve_all call site (:832). Not warranted: resolve_enters_under_player itself carries the CR 110.2a doc block and the ChangeZone site is annotated; the resolve_all line is plumbing into an already-annotated rules function, not a fresh rules-touching line. Refute, do not echo.

VERDICT: approve with comments

[matthewevans]

Maintainer sign-off: delegates enters_under resolution to the controller_ref_player single authority (exhaustive 9-variant, CR 110.2a); bug confirmed live on main; two independent approvals; Gemini nit refuted with evidence.