fix(engine): model Ward paid with player counters (The Serpent Society, #6640) - #6844
fix(engine): model Ward paid with player counters (The Serpent Society, #6640)#6844galuis116 wants to merge 1 commit into
Conversation
The Serpent Society's "Ward—Get five poison counters" had no representation:
WardCost lacked a player-counter form, so the oracle parser fell through to the
mana fallback and lowered it to WardCost::Mana(generic 0). An opponent targeting
the creature paid nothing and the spell resolved for free.
Add WardCost::GetPlayerCounters { kind, count }, parameterized over
PlayerCounterKind so it covers the whole class (poison/rad/experience/ticket),
not one card:
- Parser (oracle_keyword): parse "get N <kind> counters" as a ward cost,
reusing the same parse_number + parse_player_counter_kind combinators the
imperative "get N poison counters" effect uses, and strip the parenthetical
reminder text first (CR 702.21a + CR 122.1).
- ward_cost_to_ability_cost (triggers): map it to an EffectCost wrapping
GivePlayerCounter, mirroring the existing "unless you take N damage" /
"unless its controller draws" punisher shape (CR 118.12).
- Unless-payment (engine_payment_choices): add the GivePlayerCounter arm to the
EffectCost payer path so paying re-targets the effect to the payer and adds the
counters (CR 122.1 + CR 104.3d).
- AI: can_pay_ward_cost treats it as always payable (a player can always receive
counters); anti_self_harm scores poison/rad as self-harm scaled by count and
experience/ticket as harmless.
Tests: parser unit test (word/digit count, reminder stripping, whole class) plus
runtime regressions in serpent_society_ward_poison_6640.rs — paying gives exactly
five poison counters and leaves the spell on the stack; declining counters it.
Closes phase-rs#6640
📝 WalkthroughWalkthroughThis change adds typed player-counter Ward costs. It parses poison and rad counter costs, resolves payments for the targeting player, adds integration coverage, and updates AI Ward-cost evaluation. ChangesPlayer-counter Ward support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant TargetingPlayer
participant WardTrigger
participant PaymentChoice
participant PlayerCounterResolver
participant TargetedSpell
TargetingPlayer->>WardTrigger: target creature
WardTrigger->>PaymentChoice: request player-counter payment
PaymentChoice->>PlayerCounterResolver: give counters to targeting player
PlayerCounterResolver-->>TargetingPlayer: update counter total
PaymentChoice-->>TargetedSpell: preserve spell if paid
PaymentChoice-->>TargetedSpell: counter spell if declined
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
galuis116
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/engine_payment_choices.rs`:
- Around line 1183-1205: Update the Effect::GivePlayerCounter Ward-cost branch
to resolve through costs::pay_ability_cost_for_resolution instead of directly
calling effects::player_counter::resolve. Propagate its Paid, Failed, and Paused
outcomes, ensuring prevented counters mark payment as failed while
replacement-choice waits return the paused action result.
In `@crates/engine/src/game/triggers.rs`:
- Around line 223-229: Update the WardCost::GetPlayerCounters conversion to
perform a checked u32-to-i32 conversion before constructing QuantityExpr::Fixed;
reject the WardCost input when the count exceeds i32::MAX instead of allowing a
wrapping negative value, while preserving valid counter handling.
In `@crates/phase-ai/src/policies/anti_self_harm.rs`:
- Around line 804-810: Make Ward-cost evaluation apply to every target type, not
only creatures: move the shared handling around the creature/noncreature split
in the Ward resolution flow, or invoke it from both branches, so noncreature
targets also run can_pay_ward_cost and ward_counter_severity for
WardCost::GetPlayerCounters. Add a regression covering a noncreature Ward
target.
- Around line 825-827: The WardCost handling must preserve every component of
compound costs. In crates/phase-ai/src/policies/anti_self_harm.rs at lines
825-827, update the WardCost severity logic to recursively score each component
instead of assigning WardCost::Compound a fixed value; in
crates/phase-ai/src/policies/strategy_helpers.rs at lines 783-786, ensure
compound costs are recursively converted into AbilityCost::Composite rather than
using only the first cost, and update the corresponding runtime conversion in
the triggers logic so prompts and payments include all components.
- Around line 51-60: Update ward_counter_severity to accept the payer’s current
counter total and calculate poison/rad severity from current total plus incoming
count, capping it at 3.0 so 9+1 and 5+5 receive maximum severity. Update its
callers to pass player_counter(kind), and add target-selection coverage for both
cases.
🪄 Autofix (Beta)
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: c747f3bb-222a-4c0e-b72c-fdebe21fa1c0
📒 Files selected for processing (8)
crates/engine/src/game/engine_payment_choices.rscrates/engine/src/game/triggers.rscrates/engine/src/parser/oracle_keyword.rscrates/engine/src/types/keywords.rscrates/engine/tests/integration/main.rscrates/engine/tests/integration/serpent_society_ward_poison_6640.rscrates/phase-ai/src/policies/anti_self_harm.rscrates/phase-ai/src/policies/strategy_helpers.rs
| // CR 702.21a + CR 122.1 + CR 118.12: "Ward—Get N <kind> counters" | ||
| // (The Serpent Society). The payer pays by receiving N player | ||
| // counters. Re-target the effect to the payer (a declared Player | ||
| // target) and resolve it through the player-counter handler, the | ||
| // same punisher shape as the DealDamage/Draw arms above. | ||
| Effect::GivePlayerCounter { .. } => { | ||
| let mut counter_ability = pending_effect.as_ref().clone(); | ||
| counter_ability.effect = *effect.clone(); | ||
| counter_ability.targets = vec![TargetRef::Player(player)]; | ||
| counter_ability.unless_pay = None; | ||
| counter_ability.sub_ability = None; | ||
| if let Err(e) = | ||
| effects::player_counter::resolve(state, &counter_ability, events) | ||
| { | ||
| return Err(EngineError::InvalidAction(format!("{e:?}"))); | ||
| } | ||
| if matches!( | ||
| state.waiting_for, | ||
| WaitingFor::ReplacementChoice { .. } | ||
| ) { | ||
| return Ok(action_result(events, state.waiting_for.clone())); | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -t f '^player_counter\.rs$' crates/engine | while IFS= read -r file; do
ast-grep outline "$file" --items all
rg -n -C 10 'GivePlayerCounter|fn resolve|ReplacementChoice|counter_kind|controller|targets' "$file"
done
rg -n -C 8 'pay_ability_cost_for_resolution|PaymentOutcome|EffectCost' \
crates/engine/src/game crates/engine/src/typesRepository: phase-rs/phase
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- payment branch ---'
sed -n '1128,1220p' crates/engine/src/game/engine_payment_choices.rs
printf '%s\n' '--- resolution cost authority ---'
fd -t f '^costs\.rs$' crates/engine | while IFS= read -r file; do
ast-grep outline "$file" --items all
rg -n -C 12 'enum PaymentOutcome|pay_ability_cost_for_resolution|EffectCost|GivePlayerCounter|supported_at_resolution|resolve_effect' "$file"
done
printf '%s\n' '--- counter event and replacement semantics ---'
rg -n -C 12 'enum CounterPlacement|AddCounter|actor:|ReplacementResult::Prevented|add_player_counter_with_replacement' \
crates/engine/src/types crates/engine/src/game/effects crates/engine/src/game/replacement.rsRepository: phase-rs/phase
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- payment branch ---'
sed -n '1140,1215p' crates/engine/src/game/engine_payment_choices.rs
printf '%s\n' '--- cost files ---'
fd -t f 'costs\.rs$' crates/engine
printf '%s\n' '--- resolution authority symbols ---'
rg -n -C 8 'enum PaymentOutcome|fn pay_ability_cost_for_resolution|EffectCost|GivePlayerCounter|supported_at_resolution' \
crates/engine/src/game/costs.rs
printf '%s\n' '--- counter placement type ---'
rg -n -C 8 'enum CounterPlacement|AddCounter' crates/engine/src/types/proposed_event.rs
printf '%s\n' '--- focused player-counter tests ---'
sed -n '780,875p' crates/engine/src/game/effects/proliferate.rs
sed -n '1,190p' crates/engine/src/game/effects/player_counter.rsRepository: phase-rs/phase
Length of output: 38512
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- replacement actor usage ---'
rg -n -C 10 'CounterPlacement::Player|placement\.actor|actor.*CounterPlacement|counter.*actor' \
crates/engine/src/game/replacement.rs crates/engine/src/game/engine_replacement.rs \
crates/engine/src/types
printf '%s\n' '--- unless-payment entry and completion ---'
rg -n -C 14 'surface_unless_payment|WaitingFor::UnlessPayment|cost_payment_failed_flag|payment_failed' \
crates/engine/src/game/engine_payment_choices.rs crates/engine/src/game/effects
printf '%s\n' '--- resolution cost match around EffectCost ---'
sed -n '1260,1375p' crates/engine/src/game/costs.rs
sed -n '1745,1822p' crates/engine/src/game/costs.rsRepository: phase-rs/phase
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- replacement matching for AddCounter ---'
rg -n -C 6 'AddCounter|CounterPlacement::Player|actor' \
crates/engine/src/game/replacement.rs
printf '%s\n' '--- exact resolution cost contract ---'
sed -n '644,725p' crates/engine/src/game/costs.rs
sed -n '1300,1365p' crates/engine/src/game/costs.rs
sed -n '1750,1820p' crates/engine/src/game/costs.rs
printf '%s\n' '--- exact unless-payment failure mapping ---'
sed -n '620,675p' crates/engine/src/game/engine_payment_choices.rs
sed -n '1210,1245p' crates/engine/src/game/engine_payment_choices.rsRepository: phase-rs/phase
Length of output: 50370
Route player-counter Ward costs through the payment authority.
add_player_counter_with_replacement returns true for ReplacementResult::Prevented, and resolve returns Ok(()). This leaves payment_failed false, so Ward’s primary effect resolves when the payer did not receive the required counters. Return Paid, Failed, or Paused through costs::pay_ability_cost_for_resolution, including prevention and replacement-choice handling.
🤖 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/engine/src/game/engine_payment_choices.rs` around lines 1183 - 1205,
Update the Effect::GivePlayerCounter Ward-cost branch to resolve through
costs::pay_ability_cost_for_resolution instead of directly calling
effects::player_counter::resolve. Propagate its Paid, Failed, and Paused
outcomes, ensuring prevented counters mark payment as failed while
replacement-choice waits return the paused action result.
Sources: Coding guidelines, Path instructions
| WardCost::GetPlayerCounters { kind, count } => AbilityCost::EffectCost { | ||
| effect: Box::new(crate::types::ability::Effect::GivePlayerCounter { | ||
| counter_kind: *kind, | ||
| count: QuantityExpr::Fixed { | ||
| value: *count as i32, | ||
| }, | ||
| target: TargetFilter::Player, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 6 'GetPlayerCounters|value:\s*\*count as i32' crates/engine/src crates/engine/testsRepository: phase-rs/phase
Length of output: 7227
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- WardCost and quantity definitions ---'
rg -n -C 8 'enum WardCost|GetPlayerCounters|enum QuantityExpr|struct QuantityExpr|parse_number' crates/engine/src/types crates/engine/src/parser crates/engine/src/game
printf '%s\n' '--- Relevant conversion and quantity consumers ---'
rg -n -C 8 'QuantityExpr::Fixed|value:\s*.*as i32|\.value\b' crates/engine/src | head -n 500
printf '%s\n' '--- Parser number implementation and tests ---'
rg -n -C 12 'fn parse_number|parse_number\s*=|parse_number\.' crates/engine/src/parser crates/engine/srcRepository: phase-rs/phase
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- WardCost definition ---'
rg -n -A 35 -B 8 'enum WardCost' crates/engine/src/types/keywords.rs
printf '%s\n' '--- QuantityExpr definition ---'
rg -n -A 45 -B 8 'enum QuantityExpr' crates/engine/src/types/ability.rs crates/engine/src/types
printf '%s\n' '--- Ward parser ---'
sed -n '720,785p' crates/engine/src/parser/oracle_keyword.rs
printf '%s\n' '--- Ward conversion ---'
sed -n '190,240p' crates/engine/src/game/triggers.rs
printf '%s\n' '--- Number parser definition ---'
rg -n -A 35 -B 10 'parse_number' crates/engine/src/parser/nom_primitives.rs crates/engine/src/parserRepository: phase-rs/phase
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Candidate type files ---'
git ls-files 'crates/engine/src/types/*' | sed -n '1,120p'
printf '%s\n' '--- WardCost definition ---'
rg -n -A 18 -B 5 'enum WardCost' crates/engine/src/types
printf '%s\n' '--- QuantityExpr definition ---'
rg -l 'enum QuantityExpr' crates/engine/src/types | while read -r f; do
printf '\nFILE: %s\n' "$f"
rg -n -A 35 -B 5 'enum QuantityExpr' "$f"
done
printf '%s\n' '--- Ward parser ---'
sed -n '740,775p' crates/engine/src/parser/oracle_keyword.rs
printf '%s\n' '--- Ward conversion ---'
sed -n '205,235p' crates/engine/src/game/triggers.rs
printf '%s\n' '--- Number parser declarations only ---'
rg -n -m 20 -A 18 -B 8 'parse_number' crates/engine/src/parser/nom_primitives.rsRepository: phase-rs/phase
Length of output: 8996
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Files defining or exporting parse_number ---'
rg -l 'parse_number' crates/engine/src/parser | sort
printf '%s\n' '--- parse_number definitions and nearby code ---'
for f in $(rg -l 'parse_number' crates/engine/src/parser | sort); do
matches=$(rg -n 'parse_number' "$f" | head -n 12)
if [ -n "$matches" ]; then
printf '\nFILE: %s\n%s\n' "$f" "$matches"
fi
done
printf '%s\n' '--- Numeric parser tests or explicit bounds ---'
rg -n -C 5 'u32::MAX|i32::MAX|try_from\(.*parse_number|parse_number.*u32|parse_number.*overflow|number word' crates/engine/src/parserRepository: phase-rs/phase
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Exact digit parser ---'
sed -n '20,70p' crates/engine/src/parser/oracle_nom/primitives.rs
printf '%s\n' '--- Exact English-number parser bounds ---'
sed -n '67,130p' crates/engine/src/parser/oracle_nom/primitives.rs
printf '%s\n' '--- Wrapper remainder handling ---'
sed -n '341,365p' crates/engine/src/parser/oracle_util.rs
printf '%s\n' '--- Ward parser call site and tests ---'
rg -n -A 12 -B 8 'GetPlayerCounters|ward.*get|get [^"]* counters' crates/engine/src/parser/oracle_keyword.rs
printf '%s\n' '--- Standalone conversion probe ---'
python3 - <<'PY'
values = [0, 5, 2_147_483_647, 2_147_483_648, 4_294_967_295]
for value in values:
signed = value if value <= 2_147_483_647 else value - 2**32
print(f"{value} -> {signed}")
PYRepository: phase-rs/phase
Length of output: 9604
Reject out-of-range ward counter counts.
parse_number accepts u32, but line 227 casts it to i32. Counts above i32::MAX wrap to negative quantities. Use a checked conversion and reject invalid WardCost input before constructing QuantityExpr::Fixed.
🤖 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/engine/src/game/triggers.rs` around lines 223 - 229, Update the
WardCost::GetPlayerCounters conversion to perform a checked u32-to-i32
conversion before constructing QuantityExpr::Fixed; reject the WardCost input
when the count exceeds i32::MAX instead of allowing a wrapping negative value,
while preserving valid counter handling.
Source: Path instructions
| /// CR 702.21a + CR 104.3d: Self-harm severity of a "Ward—Get N <kind> counters" | ||
| /// cost. Poison and rad counters are harmful (poison at ten loses the game), so | ||
| /// severity scales with the count; experience and ticket counters are | ||
| /// beneficial, so paying them is not self-harm. | ||
| fn ward_counter_severity(kind: PlayerCounterKind, count: u32) -> f64 { | ||
| match kind { | ||
| PlayerCounterKind::Poison | PlayerCounterKind::Rad => (count as f64 / 2.0).min(3.0), | ||
| PlayerCounterKind::Experience | PlayerCounterKind::Ticket => 0.0, | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- candidate files ---'
fd -i 'anti_self_harm|phase-ai|MagicCompRules|main.rs' . | head -80
printf '%s\n' '--- symbol references ---'
rg -n -C 4 'ward_counter_severity|GetPlayerCounters|PlayerCounterKind|poison' crates/phase-ai crates/engine/tests/integration docs/MagicCompRules.txt 2>/dev/null | head -240
printf '%s\n' '--- source outline ---'
if [ -f crates/phase-ai/src/policies/anti_self_harm.rs ]; then
ast-grep outline crates/phase-ai/src/policies/anti_self_harm.rs
fiRepository: phase-rs/phase
Length of output: 30555
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- anti_self_harm.rs: helper and scoring ---'
cat -n crates/phase-ai/src/policies/anti_self_harm.rs | sed -n '1,135p;560,735p;1035,1245p'
printf '%s\n' '--- rules document candidates ---'
find . -type f -iname '*MagicCompRules*' -o -type f -iname '*CLAUDE.md' | sort
printf '%s\n' '--- CR references in tracked files ---'
rg -n -C 2 '104\.3d|704\.5c|ten or more poison|ten poison|poison counters' --glob '*.txt' --glob '*.md' --glob '*.rs' . | head -220
printf '%s\n' '--- Ward-related symbols and tests ---'
rg -n -C 5 'Ward|ward|GetPlayerCounters|counter.*severity|target selection' crates/phase-ai/src crates/phase-ai/tests crates/engine/tests 2>/dev/null | head -260Repository: phase-rs/phase
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- exact helper references ---'
rg -n -C 12 'ward_counter_severity|GetPlayerCounters|WardCost' crates/phase-ai/src/policies/anti_self_harm.rs crates/phase-ai/src/policies crates/phase-ai/src/strategy_helpers.rs crates/engine/src/types/keywords.rs crates/engine/src/types/ability.rs 2>/dev/null
printf '%s\n' '--- Ward payment helpers ---'
rg -n -C 10 'can_pay_ward_cost|WardCounter|PlayerCounter' crates/phase-ai/src crates/engine/src | head -260
printf '%s\n' '--- focused tests mentioning counter effects ---'
rg -n -C 8 'plus_counter_is_beneficial|minus_counter_is_harmful|player_counter|Poison|ward' crates/phase-ai/src/policies/anti_self_harm.rs
printf '%s\n' '--- CLAUDE.md relevant rules ---'
rg -n -C 3 'counter|Ward|CR|typed|exhaustive|phase-ai' CLAUDE.md
printf '%s\n' '--- repository tracking and rule-file status ---'
git ls-files | rg '(^|/)(MagicCompRules\.txt|CLAUDE\.md)$|phase-ai/src/policies/anti_self_harm.rs'Repository: phase-rs/phase
Length of output: 48199
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- Ward scoring context ---'
cat -n crates/phase-ai/src/policies/anti_self_harm.rs | sed -n '740,842p'
printf '%s\n' '--- player counter accessor ---'
cat -n crates/engine/src/types/player.rs | sed -n '45,75p;245,272p'
printf '%s\n' '--- anti-self-harm counter tests ---'
rg -n -C 18 'plus_counter_is_beneficial|minus_counter_is_harmful|ward_counter|GetPlayerCounters|PlayerCounterKind::Poison' crates/phase-ai/src/policies/anti_self_harm.rs
printf '%s\n' '--- target/cast candidate routing ---'
rg -n -C 8 'AntiSelfHarm|ChooseTarget|SelectTargets|CastSpell|score_pre_cast' crates/phase-ai/src/policies/registry.rs crates/phase-ai/src/policies/context.rs crates/phase-ai/src/policies/anti_self_harm.rs | head -240
printf '%s\n' '--- deterministic threshold probe ---'
python3 - <<'PY'
def current(count):
return min(count / 2.0, 3.0)
for existing, incoming in ((0, 1), (9, 1), (5, 5)):
print(f"existing={existing}, incoming={incoming}, total={existing+incoming}, current_severity={current(incoming)}")
PYRepository: phase-rs/phase
Length of output: 35149
🌐 Web query:
Magic Comprehensive Rules poison counters ten or more player loses CR 704.5c 104.3d
💡 Result:
According to the Magic: The Gathering Comprehensive Rules, a player who has ten or more poison counters loses the game [1][2]. This is enforced as a state-based action, which means the player loses the game the next time a player would receive priority [1][2]. This rule is explicitly stated in two sections of the Comprehensive Rules: 1. Rule 104.3d: "If a player has ten or more poison counters, that player loses the game the next time a player would receive priority. (This is a state-based action. See rule 704.)" [1][2]. 2. Rule 704.5c: This rule defines the state-based action regarding poison counters [3][4]. While the specific text of 704.5c may vary slightly in presentation across editions, it functions in conjunction with 104.3d to trigger the loss of the game upon reaching the threshold [3][4]. Note that for specific variants like Two-Headed Giant, this rule is superseded by alternative rules (such as 704.6b, which sets the threshold at fifteen poison counters for a team) [3][4].
Citations:
- 1: https://media.wizards.com/2026/downloads/MagicCompRules%2020260417.txt
- 2: https://media.wizards.com/2026/downloads/MagicCompRules%2020260417.pdf
- 3: https://ancestral.vision/additional-rules/state-based-actions.html
- 4: https://mtg.wiki/page/Poison_counter
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- policy score bands and Ward penalty ---'
rg -n -C 5 'CRITICAL_MAX|ward_cost_penalty_base|default_ward_cost_penalty_base' crates/phase-ai/src/policies crates/phase-ai/src/config.rs
printf '%s\n' '--- target scoring test helpers and nearby tests ---'
rg -n -C 12 'fn make_target_selection_ctx|fn make_mutate_target_selection_ctx|score_target_object|target.*ward|ward.*target' crates/phase-ai/src/policies/anti_self_harm.rs
printf '%s\n' '--- Ward counter engine behavior ---'
rg -n -C 8 'GetPlayerCounters|add_player_counters|player_counter\(' crates/engine/src | head -180Repository: phase-rs/phase
Length of output: 50370
Include the payer’s current counter total in Ward severity.
ward_counter_severity scores only the incoming count. Under CR 104.3d, 9 existing poison counters plus 1 incoming counter causes a loss and must receive the strongest severity (3.0). The 5+5 case must receive the same severity instead of the current 2.5. Pass player_counter(kind) into the helper and cover both cases through target selection.
🤖 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/anti_self_harm.rs` around lines 51 - 60, Update
ward_counter_severity to accept the payer’s current counter total and calculate
poison/rad severity from current total plus incoming count, capping it at 3.0 so
9+1 and 5+5 receive maximum severity. Update its callers to pass
player_counter(kind), and add target-selection coverage for both cases.
Source: Learnings
| // CR 702.21a + CR 104.3d: receiving poison/rad counters is | ||
| // real self-harm scaled by count (ten poison loses the | ||
| // game); experience/ticket counters are beneficial, so no | ||
| // penalty for getting them. | ||
| WardCost::GetPlayerCounters { kind, count } => { | ||
| ward_counter_severity(*kind, *count) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Evaluate player-counter Ward costs for noncreature targets.
The new arm executes only inside the creature-only branch that starts at Line 733. An artifact, enchantment, or planeswalker with Keyword::Ward therefore bypasses both can_pay_ward_cost and the counter severity penalty. The noncreature path treats that Ward cost as free.
Move shared Ward-cost evaluation before the creature/noncreature split, or invoke it from both branches. Add a noncreature Ward target-selection regression.
As per path instructions, evaluate generalized Ward behavior instead of only the current creature case.
🤖 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/anti_self_harm.rs` around lines 804 - 810, Make
Ward-cost evaluation apply to every target type, not only creatures: move the
shared handling around the creature/noncreature split in the Ward resolution
flow, or invoke it from both branches, so noncreature targets also run
can_pay_ward_cost and ward_counter_severity for WardCost::GetPlayerCounters. Add
a regression covering a noncreature Ward target.
Source: Path instructions
| WardCost::GetPlayerCounters { kind, count } => { | ||
| ward_counter_severity(*kind, *count) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 8 'WardCost::Compound|GetPlayerCounters|ward_cost_to_ability_cost|can_pay_ward_cost' \
crates/engine/src crates/phase-ai/srcRepository: phase-rs/phase
Length of output: 34248
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- WardCost and AbilityCost definitions ---'
rg -n -C 12 'enum WardCost|Compound\(|enum AbilityCost|unless_pay|EffectCost' \
crates/engine/src/types crates/engine/src/game
printf '%s\n' '--- Unless-payment resolution and payment dispatch ---'
rg -n -C 16 'unless_pay|AbilityCost::Compound|resolve.*cost|pay.*cost|EffectCost' \
crates/engine/src/game crates/engine/src/typesRepository: phase-rs/phase
Length of output: 50371
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- AbilityCost definition ---'
rg -n -A 120 -B 8 'pub enum AbilityCost|enum AbilityCost' crates/engine/src/types
printf '%s\n' '--- WardCost definition ---'
rg -n -A 45 -B 8 'pub enum WardCost|enum WardCost' crates/engine/src/types/keywords.rs
printf '%s\n' '--- Ward conversion and unless-pay construction ---'
sed -n '185,255p' crates/engine/src/game/triggers.rs
sed -n '3625,3685p' crates/engine/src/game/triggers.rs
printf '%s\n' '--- Runtime unless-pay interceptor ---'
rg -n -C 20 'unless_pay' crates/engine/src/game/effects/mod.rs crates/engine/src/game/effects.rs crates/engine/src/gameRepository: phase-rs/phase
Length of output: 50371
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- Definition files ---'
rg -l 'pub enum AbilityCost|enum AbilityCost' crates/engine/src
rg -l 'pub enum WardCost|enum WardCost' crates/engine/src
printf '%s\n' '--- Exact definition locations ---'
rg -n -m 5 'pub enum AbilityCost|enum AbilityCost|pub enum WardCost|enum WardCost' crates/engine/src
printf '%s\n' '--- Conversion and runtime locations ---'
rg -n -m 30 'fn ward_cost_to_ability_cost|WardCost::Compound|AbilityCost::Composite|unless_pay' \
crates/engine/src/game/triggers.rs crates/engine/src/game/effects/mod.rs crates/engine/src/game/effects.rs crates/engine/src/gameRepository: phase-rs/phase
Length of output: 45286
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- AbilityCost variants ---'
sed -n '8380,8475p' crates/engine/src/types/ability.rs
printf '%s\n' '--- WardCost variants ---'
sed -n '520,565p' crates/engine/src/types/keywords.rs
printf '%s\n' '--- Unless-payment setup ---'
sed -n '8935,9110p' crates/engine/src/game/effects/mod.rs
printf '%s\n' '--- Unless-cost normalization ---'
sed -n '11805,11880p' crates/engine/src/game/effects/mod.rs
printf '%s\n' '--- AI compound handling ---'
sed -n '785,840p' crates/phase-ai/src/policies/anti_self_harm.rs
sed -n '736,798p' crates/phase-ai/src/policies/strategy_helpers.rsRepository: phase-rs/phase
Length of output: 26014
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
triggers = Path("crates/engine/src/game/triggers.rs").read_text()
anti = Path("crates/phase-ai/src/policies/anti_self_harm.rs").read_text()
helpers = Path("crates/phase-ai/src/policies/strategy_helpers.rs").read_text()
keywords = Path("crates/engine/src/types/keywords.rs").read_text()
checks = {
"engine compound conversion uses only first component":
"WardCost::Compound(costs)" in triggers
and "costs.first()" in triggers
and "ward_cost_to_ability_cost(first)" in triggers,
"AI affordability requires every compound component":
"WardCost::Compound(costs) => costs" in helpers
and ".all(|cost| can_pay_ward_cost(ctx, cost, warded))" in helpers,
"AI severity uses fixed nested-compound fallback":
"WardCost::Compound(_) => 2.0" in anti,
"WardCost documents compound costs as conjoined":
"multiple costs that must all be paid" in keywords,
}
for name, result in checks.items():
print(f"{name}: {result}")
assert result, name
PY
printf '%s\n' '--- Exact engine conversion ---'
sed -n '232,250p' crates/engine/src/game/triggers.rs
printf '%s\n' '--- Composite unless-payment branch ---'
sed -n '9080,9135p' crates/engine/src/game/effects/mod.rs
printf '%s\n' '--- WardCost compound construction sites ---'
rg -n -C 4 'WardCost::Compound' crates/engine/src crates/phase-ai/srcRepository: phase-rs/phase
Length of output: 8593
Preserve all compound Ward-cost components at runtime.
crates/engine/src/game/triggers.rs converts WardCost::Compound with costs.first(), so the runtime prompts and pays only the first sub-cost. This conflicts with strategy_helpers.rs, which requires every component, and with anti_self_harm.rs, which scores every component.
- Convert compound costs recursively into
AbilityCost::Composite. - Replace
WardCost::Compound(_) => 2.0with recursive severity scoring.
📍 Affects 2 files
crates/phase-ai/src/policies/anti_self_harm.rs#L825-L827(this comment)crates/phase-ai/src/policies/strategy_helpers.rs#L783-L786
🤖 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/anti_self_harm.rs` around lines 825 - 827, The
WardCost handling must preserve every component of compound costs. In
crates/phase-ai/src/policies/anti_self_harm.rs at lines 825-827, update the
WardCost severity logic to recursively score each component instead of assigning
WardCost::Compound a fixed value; in
crates/phase-ai/src/policies/strategy_helpers.rs at lines 783-786, ensure
compound costs are recursively converted into AbilityCost::Composite rather than
using only the first cost, and update the corresponding runtime conversion in
the triggers logic so prompts and payments include all components.
Source: Path instructions
Parse changes introduced by this PR✓ No card-parse changes detected. |
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
5 verified findings block merge: the counter Ward path can accept an unpaid cost, loses compound components, and leaves material AI cases unpriced.
🔴 Blocker
[HIGH] A prevented player-counter event is accepted as a paid Ward cost. Evidence: crates/engine/src/game/engine_payment_choices.rs:1188 resolves GivePlayerCounter and reaches the successful-payment path; crates/engine/src/game/effects/player_counter.rs:53 reports ReplacementResult::Prevented as success. Why it matters: with a “players can’t get counters” replacement, choosing to pay leaves the targeting spell on the stack even though the required counters were never received. Suggested fix: give this payment path an explicit paid/failed/paused result (or extend the payment authority for this exact effect), treat prevention as failed payment, and add a Ward-plus-counter-prevention cast-pipeline regression.
[HIGH] Compound Ward can contain the new counter cost, but runtime charges only its first component. Evidence: crates/engine/src/parser/oracle_keyword.rs:686 constructs WardCost::Compound; crates/engine/src/game/triggers.rs:240 lowers only costs.first(); crates/engine/src/game/engine_payment_choices.rs:986 rejects mixed composite payments. Why it matters: a Ward such as {2}, Get five poison counters is represented as supported while a payer can satisfy only one required part. Suggested fix: either preserve this shape as unsupported or implement all-or-none sequenced composite payment including the counter component; add a compound-cost regression.
🟡 Non-blocking
[MED] Player-counter Ward is priced only for creature targets. Evidence: crates/phase-ai/src/policies/anti_self_harm.rs:733 encloses the only Ward-pricing loop at :790 in the creature branch, while the noncreature branch begins at :888 without it. Why it matters: targeting a warded artifact, enchantment, or planeswalker treats poison/rad Ward as free. Suggested fix: evaluate shared Ward pricing outside the creature split and add a noncreature target-selection regression.
[MED] Poison Ward severity ignores the payer’s existing poison total. Evidence: crates/phase-ai/src/policies/anti_self_harm.rs:55 scores only the incoming count; crates/engine/src/types/player.rs:255 exposes the live counter total. Why it matters: paying Get one poison counter at nine poison is immediately lethal but receives the smallest counter-cost penalty. Suggested fix: compute poison severity from existing plus incoming counters, keep rad semantics separate, and cover the 9+1 target-selection case.
[LOW] Counter Ward counts can wrap negative during lowering. Evidence: crates/engine/src/parser/oracle_nom/primitives.rs:24 parses a u32, while crates/engine/src/game/triggers.rs:227 casts it with as i32. Why it matters: a parsed count above i32::MAX resolves as zero and creates a free Ward payment. Suggested fix: reject or preserve as unimplemented an out-of-range count before constructing QuantityExpr::Fixed.
Recommendation: request changes. Please address the two engine-payment blockers and add discriminating regressions for counter prevention, compound payment, noncreature Ward pricing, and lethal existing-poison state before re-review.
|
CI note: the only non-green checks are the two AI gates ("Paired-seed AI gate" and "Decision-cost perf gate"), both cancelled (not failed). The cancellation is the pre-existing, seed-dependent Improvise panic tracked in #6837 � not a regression from this PR. Evidence:
Every substantive check is green: Rust lint (fmt, clippy, parser gate), both Rust test shards, Rust (fmt/clippy/test/coverage-gate), Card data (generate/validate/coverage), Frontend, Lobby worker, WASM, and Tauri.
|
|
Changes requested — the current head is unchanged and still blocked. Current head
The current diff also still leaves the reported noncreature Ward AI path and current-poison severity gaps. The cancelled non-required AI gates do not alter this disposition; the blocking engine findings must be fixed with discriminating regressions before re-review. See the existing detailed review: #6844 (review) |
2 participants
Problem
The Serpent Society's "Ward�Get five poison counters" had no representation in the engine.
WardCostlacked a player-counter form, so the oracle parser fell through to the mana fallback and lowered the cost toWardCost::Mana(generic 0). An opponent targeting the creature paid nothing and the spell resolved for free (#6640).Fix
Add
WardCost::GetPlayerCounters { kind, count }, parameterized overPlayerCounterKindso it covers the whole class (poison / rad / experience / ticket), not just the one card:oracle_keyword): parse"get N <kind> counters"as a ward cost, reusing the sameparse_number+parse_player_counter_kindcombinators the imperative "get N poison counters" effect already uses, and strip the parenthetical reminder text first. (CR 702.21a + CR 122.1)ward_cost_to_ability_cost(triggers): map it to anEffectCostwrappingGivePlayerCounter, mirroring the existing "unless you take N damage" / "unless its controller draws" punisher shape. (CR 118.12)engine_payment_choices): add theGivePlayerCounterarm to theEffectCostpayer path so paying re-targets the effect to the payer and adds the counters. (CR 122.1 + CR 104.3d)can_pay_ward_costtreats it as always payable (a player can always receive counters);anti_self_harmscores poison/rad as self-harm scaled by count, and experience/ticket as harmless.Why this shape
Rather than a poison-only special case, the variant is parameterized over the existing
PlayerCounterKindenum, and payment is expressed through the existingEffectCost+GivePlayerCounterbuilding blocks � the same mechanism the damage/draw punisher wards already use. No new cost primitive or payment machinery.Tests
"get 2 rad counters") to prove it's built for the class.serpent_society_ward_poison_6640.rsdriving the real cast pipeline:Verified locally: parser test, both integration tests,
cargo fmt, andclippy -p phase-engine --testsare clean. (phase-ai could not be built in my local Windows toolchain � missingdlltool.exeforwindows-sys� but the AI changes are two match arms + a small helper; CI's Linux build covers them.)Closes #6640
Summary by CodeRabbit
New Features
Bug Fixes
AI Improvements