Skip to content

Relics + Events + Rewards: 30 event generators, boss relic skip#7

Open
JackSwitzer wants to merge 1 commit intomainfrom
work/relics-events-rewards
Open

Relics + Events + Rewards: 30 event generators, boss relic skip#7
JackSwitzer wants to merge 1 commit intomainfrom
work/relics-events-rewards

Conversation

@JackSwitzer
Copy link
Owner

@JackSwitzer JackSwitzer commented Feb 4, 2026

Summary

  • Add 30+ event choice generators for model traversability
  • Add SkipBossRelicAction for explicit boss relic skip
  • Fix conftest.py path issue for worktrees
  • Various event handler improvements

Event Generators Added

  • Act 1: Sssserpent, Mushrooms, ShiningLight, DeadAdventurer, WingStatue
  • Act 2: Addict, Augmenter, BackToBasics, Beggar, CursedTome, ForgottenAltar, Ghosts, Nest, Vampires
  • Act 3: Falling, MoaiHead, MysteriousSphere, SecretPortal, SensoryStone, TombOfLordRedMask, WindingHalls
  • Special: AccursedBlacksmith, BonfireElementals, Designer, FaceTrader, FountainOfCleansing, TheJoust, TheLab, Nloth, WeMeetAgain, WomanInBlue

Test Results

4193 tests passing

Files Changed

  • packages/engine/handlers/event_handler.py (+30 generators)
  • packages/engine/handlers/reward_handler.py (SkipBossRelicAction)
  • tests/conftest.py (path fix)

🤖 Generated with Claude Code


Note

Medium Risk
Adds a new boss-reward action (SkipBossRelicAction) and sentinel chosen_index=-1, which could affect downstream logic that assumes a boss relic is always selected. Event choice generation is largely additive but touches many event IDs and availability rules.

Overview
Expands event choice coverage by adding ~30 new EVENT_CHOICE_GENERATORS entries across Acts 1–3 and special one-time events, making more events return structured choices (including deck-dependent options for Falling).

Updates boss reward flow to support explicitly skipping boss relic selection via SkipBossRelicAction, tracking skips with BossRelicChoices.chosen_index = -1 (plus is_skipped/__repr__ updates) and exposing execute_action as an alias of handle_action.

Fixes test setup portability by deriving the repo root dynamically in tests/conftest.py instead of using a hardcoded absolute path.

Written by Cursor Bugbot for commit f902c0f. This will update automatically on new commits. Configure here.

- Fix conftest.py path issue causing imports from main repo instead of worktree
- Add ~30 event choice generators for previously missing events:
  - Act 1: Sssserpent, Mushrooms, ShiningLight, DeadAdventurer, WingStatue
  - Act 2: Addict, Augmenter, BackToBasics, Beggar, CursedTome, ForgottenAltar,
           Ghosts, Nest, Vampires
  - Act 3: Falling, MoaiHead, MysteriousSphere, SecretPortal, SensoryStone,
           TombOfLordRedMask, WindingHalls
  - Special: AccursedBlacksmith, BonfireElementals, Designer, FaceTrader,
             FountainOfCleansing, TheJoust, TheLab, Nloth, WeMeetAgain, WomanInBlue
- Add SkipBossRelicAction for explicit boss relic skip
- Add execute_action alias for API compatibility with handle_action
- Extend BossRelicChoices with is_skipped property for skip tracking

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@JackSwitzer
Copy link
Owner Author

PR #7 Code Review: Events + Rewards - Java Parity Audit

Critical Issues

1. Vampires Event - Missing Blood Vial Choice

  • Python: 2 choices (Accept, Refuse->Fight)
  • Java: 2-3 choices depending on Blood Vial ownership
    • Option 0: Accept (lose max HP, gain Bites)
    • Option 1 (conditional): Trade Blood Vial (no HP loss, gain Bites)
    • Option 2: Refuse (leave WITHOUT fighting!)
  • Fix: Add Blood Vial trade option, change Refuse to NOT trigger combat

Medium Issues

  • Falling Event: Java always shows 3 choices (disabled for missing types), Python hides them
  • Sensory Stone: Java has 3 choices (0/5/10 damage for 1/2/3 cards), Python has 2 with different logic
  • Winding Halls: Choice generator only exposes 2 of 3 choices
  • Mind Bloom: Third option should change based on floor number (1-40 vs 41-50)

Verified Correct

  • Boss Relic Skip Behavior (SkipBossRelicAction works correctly)
  • Reward Handler modifiers (Gold Tooth, Golden Idol, White Beast Statue, Sozu)
  • All reward action types (ClaimGold, ClaimPotion, PickCard, ClaimRelic, etc.)

Review by Claude Opus 4.5

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 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.

run_state: 'RunState'
) -> List[EventChoice]:
"""Fountain of Cleansing: Drink or Leave."""
has_curses = any(c.id in handler.CURSE_CARDS or c.id in handler.UNREMOVABLE_CURSES for c in run_state.deck)
Copy link

Choose a reason for hiding this comment

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

Fountain event shows wrong text for unremovable curses

Medium Severity

The _get_fountain_of_cleansing_choices function uses or to check for curses: c.id in handler.CURSE_CARDS or c.id in handler.UNREMOVABLE_CURSES. This makes has_curses true even when the player only has unremovable curses (like AscendersBane). However, the actual handler at line 2584 only removes curses that are in CURSE_CARDS AND NOT in UNREMOVABLE_CURSES. This means the choice text will say "[Drink] Remove a curse" when the player has only unremovable curses, but drinking will remove nothing. The check should use and c.id not in handler.UNREMOVABLE_CURSES or simply check only CURSE_CARDS.

Fix in Cursor Fix in Web

EventChoice(index=0, name="help", text="[Help] Give gold for relic"),
EventChoice(index=1, name="steal", text="[Steal] Gain Shame curse"),
EventChoice(index=2, name="leave", text="[Leave]"),
]
Copy link

Choose a reason for hiding this comment

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

Addict event choices have incorrect indices and labels

High Severity

The _get_addict_choices function has choices that don't match the handler behavior. The handler at _handle_addict expects: index 0 = pay gold for relic, index 1 = refuse (gain Shame curse only), index 2 = rob (get relic + Shame). But the choice generator provides: index 0 = "help" (matches), index 1 = "steal" with text about gaining Shame (but handler just refuses, no stealing), index 2 = "leave" (but handler actually robs/steals). The "[Leave]" choice will actually steal a relic and give a curse, which is completely misleading and potentially game-breaking.

Fix in Cursor Fix in Web

@JackSwitzer
Copy link
Owner Author

Correction: PR #7 Events Review - Retracted Claims

After deeper audit against Java source:

❌ RETRACTED: Sensory Stone

My review claimed Python has "2 choices with different logic" - this is INCORRECT. Python correctly implements all 3 choices:

  • Option 0: 1 card, no damage
  • Option 1: 2 cards, 5 damage
  • Option 2: 3 cards, 10 damage

New finding: Python documents A15+ damage modifiers (5/8, 10/15) but Java uses fixed 5/10 values with no ascension scaling.

❌ RETRACTED: Winding Halls

My review claimed "only exposes 2 of 3 choices" - this is INCORRECT. Python correctly exposes all 3 choices:

  1. Embrace Madness (damage + Madness)
  2. Retrace Steps (heal + Writhe)
  3. Press On (max HP loss)

✅ CONFIRMED CORRECT:

  • Vampires: Missing Blood Vial trade option, Refuse incorrectly triggers combat
  • Falling: UI difference (Python hides unavailable vs Java disables)
  • Mind Bloom: Third option floor-dependent branching

Correction by Claude Opus 4.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant