Conversation
Add comprehensive effect implementations for all Ironclad cards: Effects implemented: - Simple stat modifications: gain_strength, gain_temp_strength, reduce_enemy_strength, double_strength, double_block - Energy effects: gain_2_energy, lose_hp_gain_energy, lose_hp_gain_energy_draw - HP loss: lose_hp - Card generation/manipulation: add_copy_to_discard, shuffle_wound_into_draw, shuffle_dazed_into_draw, add_wounds_to_hand, add_burn_to_discard, add_random_attack_cost_0, put_card_from_discard_on_draw, return_exhausted_card_to_hand, copy_attack_or_power - Draw effects: draw_then_no_draw, exhaust_to_draw, draw_then_put_on_draw - Exhaust effects: exhaust_random_card, exhaust_non_attacks_gain_block, exhaust_all_non_attacks, exhaust_hand_damage_per_card - Conditional damage: damage_equals_block, damage_per_strike, strength_multiplier, increase_damage_on_use, random_enemy_x_times, if_vulnerable_draw_and_energy, damage_all_heal_unblocked, if_fatal_gain_max_hp - Status applications: apply_weak_all, apply_vulnerable_1_all, apply_weak_and_vulnerable, apply_weak_and_vulnerable_all, gain_strength_if_enemy_attacking - X-cost: damage_all_x_times - Power effects: block_not_lost, gain_vulnerable_gain_energy_per_turn, start_turn_lose_hp_draw, skills_cost_0_exhaust, draw_on_exhaust, draw_on_status, block_on_exhaust, damage_on_status_curse, when_attacked_deal_damage, gain_strength_each_turn, end_turn_gain_block, end_turn_damage_all_lose_hp, gain_strength_on_hp_loss, damage_random_on_block, gain_block_per_attack, play_attacks_twice, play_top_card, gain_energy_on_exhaust_2_3 Power triggers added: - Berserk: energy at start of turn - Rage: block per attack - DoubleTap: play attacks twice - NoDraw: remove at end of turn - Corruption, Barricade: state flags Tests: 97 new tests covering all Ironclad card stats and effects Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
PR #3 Code Review: Ironclad Cards - Java Parity AuditCritical Issues1. Berserk - Wrong Trigger Hook
2. Rupture - Incorrect Trigger Condition
3. Limit Break - Doesn't Handle Negative Strength
Medium Issues
Review by Claude Opus 4.5 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 6 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| def rage_on_attack(ctx: PowerContext) -> None: | ||
| """Rage: Gain Block when playing an Attack card.""" | ||
| from ..content.cards import ALL_CARDS, CardType | ||
| card_id = ctx.trigger_data.get("card_id", "") |
There was a problem hiding this comment.
Rage and DoubleTap power triggers always fail silently
High Severity
The rage_on_attack and double_tap_on_attack functions access ctx.trigger_data.get("card_id", "") to get the card ID, but the trigger is called with {"card": card} where card is a Card object. Since "card_id" is never in trigger_data, card_id will always be an empty string, causing the condition base_id in ALL_CARDS to always be False. Both the Rage (block per attack) and DoubleTap (play attack twice) effects will never activate.
Additional Locations (1)
| # Extra damage = strength * (multiplier - 1) since base strength is already applied | ||
| extra_damage = strength * (multiplier - 1) | ||
| if ctx.target and extra_damage > 0: | ||
| ctx.deal_damage_to_enemy(ctx.target, extra_damage) |
There was a problem hiding this comment.
Heavy Blade ignores negative strength penalty
Medium Severity
The strength_multiplier effect only applies extra damage when extra_damage > 0. When the player has negative strength, extra_damage will be negative and the function does nothing. In Slay the Spire, Heavy Blade's strength multiplier applies in both directions - negative strength results in reduced damage (multiplied penalty), which this implementation doesn't handle.
|
|
||
| import pytest | ||
| import sys | ||
| sys.path.insert(0, '/Users/jackswitzer/Desktop/SlayTheSpireRL') |
There was a problem hiding this comment.
Test file contains hardcoded local machine path
Medium Severity
The test file contains a hardcoded absolute path sys.path.insert(0, '/Users/jackswitzer/Desktop/SlayTheSpireRL') that will only work on one developer's machine. This will cause test failures on CI systems and other developers' environments where this path doesn't exist.
| def gain_energy_on_exhaust_2_3(ctx: EffectContext) -> None: | ||
| """Sentinel - If exhausted, gain 2/3 energy.""" | ||
| # This effect is tracked on the card, triggers when exhausted | ||
| ctx.extra_data["sentinel_energy"] = 3 if ctx.is_upgraded else 2 |
There was a problem hiding this comment.
Sentinel card effect never grants energy when exhausted
High Severity
The gain_energy_on_exhaust_2_3 effect for Sentinel only stores a value in ctx.extra_data["sentinel_energy"] but never actually grants the energy. There's no corresponding onExhaust trigger handler that reads this value and calls ctx.gain_energy(). The effect sets up data that is never consumed, making Sentinel's "gain energy when exhausted" ability completely non-functional.
| # Mark for auto-play and exhaust | ||
| if not hasattr(ctx.state, 'cards_to_auto_play'): | ||
| ctx.state.cards_to_auto_play = [] | ||
| ctx.state.cards_to_auto_play.append((card, True)) # True = exhaust after |
There was a problem hiding this comment.
Havoc effect removes card but never plays it
High Severity
The play_top_card effect (Havoc) pops a card from the draw pile and appends it to ctx.state.cards_to_auto_play, but this list is never read or processed anywhere in the codebase. The card is permanently removed from the draw pile but never actually played or exhausted - it simply vanishes. This causes card loss during gameplay.
| # Mark card as cost 0 this turn | ||
| if not hasattr(ctx.state, 'cost_0_this_turn'): | ||
| ctx.state.cost_0_this_turn = [] | ||
| ctx.state.cost_0_this_turn.append(card_id) |
There was a problem hiding this comment.
Infernal Blade marks cost-0 but nothing applies it
Medium Severity
The add_random_attack_cost_0 effect (Infernal Blade) adds a random attack to hand and appends its ID to ctx.state.cost_0_this_turn, but this list is never read anywhere in the codebase to actually modify card costs. The card is added to hand at its normal cost instead of costing 0 as intended.
1. Berserk - Change from onEnergyRecharge to atStartOfTurn hook for Java parity (Java: BerserkPower.atStartOfTurn grants energy) 2. Rupture - Fix trigger condition to check is_self_damage instead of source=="card" (Java: info.owner == this.owner - triggers on ANY self-damage) 3. Limit Break - Allow doubling negative strength (Java: doubles any non-zero strength including negative) 4. Body Slam - Add calculate_card_damage() method to apply full damage pipeline (Java: uses calculateCardDamage() which applies Strength/Weak/Vuln/Stance) 5. Corruption - Implement onCardDraw (set Skill cost to 0) and onUseCard (exhaust Skills) (Java: setCostForTurn(-9) on draw, action.exhaustCard=true on use) Note: Spot Weakness was verified correct - already checks only target's intent. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Summary
Test Results
599 tests passing
Files Changed
🤖 Generated with Claude Code
Note
Medium Risk
Large, mostly additive changes to combat/effect logic (new Ironclad effects plus
Rage/DoubleTap/Berserk/NoDrawtriggers) that can subtly affect turn sequencing and state handling. New tests also include a hardcoded localsys.pathinjection that could break in CI/environments outside the author’s machine.Overview
Adds a large set of Ironclad card effect implementations to
effects/cards.py, covering Strength/Block manipulation, self-damage and energy gain, card generation/movement, exhaust interactions, conditional damage (e.g., block/strike/Strength scaling), and X-cost/AoE behaviors, plus anIRONCLAD_CARD_EFFECTSmapping and expanded attack-ID detection used for playability checks (e.g.,Clash).Extends
registry/powers.pywith Ironclad power triggers, including per-turn energy forBerserk,Ragereset + on-attack block gain,DoubleTapmarking attacks to replay, and end-of-turn cleanup forNoDraw.Introduces
tests/test_ironclad_cards.pywith broad coverage of Ironclad card definitions/effects wiring and basic registry invariants.Written by Cursor Bugbot for commit ec37fc3. This will update automatically on new commits. Configure here.