Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/phase-ai/src/policies/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ mod redundancy_avoidance;
pub mod registry;
mod sacrifice_land_protection;
mod sacrifice_value;
mod self_bounce_target;
mod self_cost;
mod self_cost_value;
mod self_protection_classify;
Expand Down
3 changes: 3 additions & 0 deletions crates/phase-ai/src/policies/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,8 @@ pub enum PolicyId {
GraveyardTypes,
CrewTiming,
CombatWithdrawal,
/// CR 608.2c: "return a land you control" self-bounce target choice.
SelfBounceTarget,
}

/// Coarse routing kind for a candidate decision. Each policy declares which
Expand Down Expand Up @@ -396,6 +398,7 @@ impl Default for PolicyRegistry {
Box::new(PayoffPolicy::new(&REANIMATOR_PAYOFF)),
Box::new(PayoffPolicy::new(&BLINK_PAYOFF)),
Box::new(LoopShortcutPolicy),
Box::new(super::self_bounce_target::SelfBounceTargetPolicy),
];
let mut by_kind: HashMap<DecisionKind, Vec<usize>> = HashMap::new();
for (idx, policy) in policies.iter().enumerate() {
Expand Down
146 changes: 146 additions & 0 deletions crates/phase-ai/src/policies/self_bounce_target.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
//! Self-bounce target selection — when a "return a land you control" ETB forces
//! the AI to return one of its own lands, pick the least-costly one and NEVER
//! the land that just entered (which loops).
//!
//! ## The defect this closes (#4730)
//!
//! Azorius Chancery and the rest of the Ravnica/MOM bounce-land ("Karoo") cycle
//! enter and their ETB returns "a land you control" to hand (CR 608.2c, chosen
//! at resolution). No policy scored that choice, so the AI kept returning the
//! bounce-land it had just played — putting it back in hand to be replayed and
//! bounced again, the tempo loop the reporter observed. `land_sequencing` fixed
//! the *sequencing* half and its own doc-comment deferred this half verbatim:
//! "the AI should return the least-useful land, never the just-played
//! bounce-land — that needs its own target-selection policy."
//!
//! ## Heuristic (building block, not a card fix)
//!
//! The choice surfaces as a `WaitingFor::EffectZoneChoice` returning battlefield
//! lands you control to hand; each candidate is a `SelectCards` selection.
//! Detection is structural (no card names) — it covers every "return a land you
//! control" bouncer. Each land in the selection is scored by how little
//! returning it costs:
//! * the ability's own source — the just-played bounce-land — takes a strong
//! penalty, so it is chosen only when it is the sole eligible land (which
//! breaks the loop);
//! * an already-tapped land is preferred (its mana is spent this turn, so
//! replaying it next turn costs the least tempo);
//! * an untapped land is mildly penalized (returning it forfeits mana still
//! available this turn) but still beats looping the bounce-land.

use engine::types::ability::EffectKind;
use engine::types::actions::GameAction;
use engine::types::card_type::CoreType;
use engine::types::game_state::{GameState, WaitingFor};
use engine::types::identifiers::ObjectId;
use engine::types::player::PlayerId;
use engine::types::zones::Zone;

use crate::features::DeckFeatures;

use super::context::PolicyContext;
use super::registry::{DecisionKind, PolicyId, PolicyReason, PolicyVerdict, TacticalPolicy};

/// Penalty for returning the ability's own source — the just-played bounce-land.
/// Returning it re-buys the land drop for nothing and loops, so it must lose to
/// any other land the pool offers.
pub(crate) const RETURN_SOURCE_PENALTY: f64 = -3.0;
/// Bonus for returning an already-tapped land: its mana is spent this turn, so
/// replaying it next turn costs the least tempo.
pub(crate) const RETURN_TAPPED_BONUS: f64 = 1.0;
/// Penalty for returning an untapped land: doing so forfeits mana still
/// available this turn. Milder than the source penalty — an untapped non-source
/// land is still a better return than looping the bounce-land.
pub(crate) const RETURN_UNTAPPED_PENALTY: f64 = -0.5;
Comment on lines +44 to +54

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Use the registry’s PolicyVerdict band helpers rather than raw score constants.

The literal -3.0, 1.0, and -0.5 bypass the required verdict-band contract, making this policy’s influence unstable when composed with other tactical policies. Map the three outcomes to the established penalty/bonus helpers.

As per path instructions, “New tactical policies must use the PolicyVerdict band helpers, not raw sentinel scores.”

Also applies to: 140-144

🤖 Prompt for 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.

In `@crates/phase-ai/src/policies/self_bounce_target.rs` around lines 44 - 54,
Replace the raw score values in RETURN_SOURCE_PENALTY, RETURN_TAPPED_BONUS, and
RETURN_UNTAPPED_PENALTY with the corresponding PolicyVerdict penalty and bonus
band helpers. Preserve the existing outcome mapping: source return uses the
penalty band, tapped-land return uses the bonus band, and untapped non-source
return uses the penalty band.

Source: Path instructions


pub struct SelfBounceTargetPolicy;

impl SelfBounceTargetPolicy {
/// Desirability of returning `land_id` to hand; higher = better to bounce.
pub(crate) fn return_desirability(
state: &GameState,
land_id: ObjectId,
source_id: ObjectId,
) -> f64 {
// CR 608.2c: never bounce the permanent that generated the choice — that
// is the tempo loop this policy exists to break.
if land_id == source_id {
return RETURN_SOURCE_PENALTY;
}
match state.objects.get(&land_id) {
Some(land) if land.tapped => RETURN_TAPPED_BONUS,
Some(_) => RETURN_UNTAPPED_PENALTY,
None => 0.0,
}
}
}

impl TacticalPolicy for SelfBounceTargetPolicy {
fn id(&self) -> PolicyId {
PolicyId::SelfBounceTarget
}

fn decision_kinds(&self) -> &'static [DecisionKind] {
// `WaitingFor::EffectZoneChoice` routes through the ActivateAbility
// catch-all bucket (see `decision_kind::classify`).
&[DecisionKind::ActivateAbility]
}

fn activation(
&self,
_features: &DeckFeatures,
_state: &GameState,
_player: PlayerId,
) -> Option<f32> {
// Universal; the verdict's bounce-choice guard self-gates.
// activation-constant: self-bounce target choice, universal.
Some(1.0)
}

fn verdict(&self, ctx: &PolicyContext<'_>) -> PolicyVerdict {
let na = || PolicyVerdict::neutral(PolicyReason::new("self_bounce_target_na"));

let GameAction::SelectCards { cards } = &ctx.candidate.action else {
return na();
};
// CR 608.2c: a resolution-time "return a land you control to hand"
// choice — from the battlefield, to hand, scoped to the AI.
let WaitingFor::EffectZoneChoice {
player,
source_id,
effect_kind: EffectKind::ChangeZone,
zone: Zone::Battlefield,
destination: Some(Zone::Hand),
cards: pool,
..
} = &ctx.decision.waiting_for
Comment on lines +108 to +116

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Exclude land-return cost payments from this resolution policy.

is_cost_payment is ignored, so any activated-ability cost that returns a controlled land to hand receives this source/tapped heuristic. Gate on is_cost_payment: false and add a neutral regression test; this policy is intended for resolution-time bounce effects.

Proposed guard
             destination: Some(Zone::Hand),
             cards: pool,
+            is_cost_payment: false,
             ..

As per path instructions, policies must be composable building blocks for the intended class rather than unrelated selections.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let WaitingFor::EffectZoneChoice {
player,
source_id,
effect_kind: EffectKind::ChangeZone,
zone: Zone::Battlefield,
destination: Some(Zone::Hand),
cards: pool,
..
} = &ctx.decision.waiting_for
let WaitingFor::EffectZoneChoice {
player,
source_id,
effect_kind: EffectKind::ChangeZone,
zone: Zone::Battlefield,
destination: Some(Zone::Hand),
cards: pool,
is_cost_payment: false,
..
} = &ctx.decision.waiting_for
🤖 Prompt for 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.

In `@crates/phase-ai/src/policies/self_bounce_target.rs` around lines 108 - 116,
Update the WaitingFor::EffectZoneChoice pattern in the self-bounce resolution
policy to bind and require is_cost_payment: false, excluding cost-payment land
returns while preserving resolution-time bounce handling. Add a neutral
regression test covering a controlled land returned to hand as an
activated-ability cost.

Source: Path instructions

else {
return na();
};
if *player != ctx.ai_player {
return na();
}
// Restrict to the "return a LAND you control" class: every eligible
// object is a land the AI controls. A value self-bounce of a creature
// (blink, save-from-removal) is intentionally left untouched.
let all_own_lands = pool.iter().all(|id| {
ctx.state.objects.get(id).is_some_and(|o| {
o.controller == ctx.ai_player && o.card_types.core_types.contains(&CoreType::Land)
})
});
if pool.is_empty() || !all_own_lands || cards.is_empty() {
return na();
}

let delta: f64 = cards
.iter()
.map(|&id| Self::return_desirability(ctx.state, id, *source_id))
.sum();

PolicyVerdict::score(
delta,
PolicyReason::new("self_bounce_target")
.with_fact("returns_source", i64::from(cards.contains(source_id))),
)
}
}
1 change: 1 addition & 0 deletions crates/phase-ai/src/policies/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ pub mod mulligan_input_lint;
pub mod poison;
pub mod reanimator_payoff;
pub mod score_contract_lint;
pub mod self_bounce_target;
Loading
Loading