Skip to content

Fix recursive gambling issue causing multiple "Go Back" selections - resolves #22#29

Merged
dmccoystephenson merged 2 commits intomainfrom
copilot/fix-ed1b91bc-20a9-4a51-9e54-ce58430cdf33
Oct 2, 2025
Merged

Fix recursive gambling issue causing multiple "Go Back" selections - resolves #22#29
dmccoystephenson merged 2 commits intomainfrom
copilot/fix-ed1b91bc-20a9-4a51-9e54-ce58430cdf33

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Sep 21, 2025

Problem

Players were required to select "Go Back" multiple times when exiting the gambling feature after changing their bet. This occurred because the changeBet() method in the tavern was calling gamble() recursively, creating nested gambling loops.

Root Cause

In src/location/tavern.py, the gambling flow worked as follows:

  1. Player chooses "Gamble" from tavern menu
  2. Player chooses "Change Bet"
  3. changeBet() method processes the bet amount
  4. Problem: changeBet() calls self.gamble() recursively
  5. This creates a nested gambling loop inside the original gambling loop
  6. When the player wants to exit, they must press "Back" once for each nested level

Solution

Primary Fix: Removed the recursive self.gamble() call from the changeBet() method. Instead of creating nested loops, the method now simply sets the bet amount and updates the prompt, then returns control to the main gambling loop.

Before:

def changeBet(self, prompt):
    # ... input handling ...
    if self.amount <= self.player.money:
        self.currentBet = self.amount
        self.currentPrompt.text = f"What will the dice land on? Current Bet: ${self.currentBet}"
        self.gamble()  # ❌ Recursive call creates nested loops

After:

def changeBet(self, prompt):
    # ... input handling ...
    if self.amount <= self.player.money:
        self.currentBet = self.amount
        self.currentPrompt.text = f"What will the dice land on? Current Bet: ${self.currentBet}"
        # ✅ Return to main loop instead of recursing

Additional Fix: Corrected invalid error handling that was calling a non-existent self.deposit() method, replacing it with proper prompt updates.

Testing

  • All 57 existing tests continue to pass
  • Added new test test_changeBet_no_recursive_gamble to verify the fix
  • Manual testing confirms players now only need one "Back" selection to exit gambling
  • Error handling verified for invalid input and insufficient funds

Impact

  • User Experience: Players can now exit gambling with a single "Back" selection as expected
  • Code Quality: Eliminated problematic recursion pattern while maintaining all functionality
  • Compatibility: No breaking changes - all existing behavior preserved except for the bug fix

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Copilot AI changed the title [WIP] @Stephenson-Software/FishE/issues/22 Fix recursive gambling issue causing multiple "Go Back" selections - resolves #22 Sep 21, 2025
@dmccoystephenson dmccoystephenson marked this pull request as ready for review September 21, 2025 16:43
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a recursive gambling issue where players had to select "Go Back" multiple times when exiting the gambling feature after changing their bet. The root cause was a recursive call to gamble() within the changeBet() method that created nested gambling loops.

  • Removed recursive self.gamble() call from changeBet() method
  • Fixed invalid error handling that called non-existent self.deposit() method
  • Added test coverage to verify the fix prevents recursive gambling calls

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/location/tavern.py Removed recursive gamble() call and fixed error handling in changeBet() method
tests/location/test_tavern.py Added test to verify changeBet() no longer calls gamble() recursively

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +120 to +126
with MagicMock() as mock_input:
mock_input.return_value = '50'

# Temporarily replace the built-in input function
import builtins
original_input = builtins.input
builtins.input = mock_input
Copy link

Copilot AI Sep 21, 2025

Choose a reason for hiding this comment

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

The with MagicMock() as mock_input: context manager is not actually managing the mocking of builtins.input. Consider using unittest.mock.patch instead for cleaner and more reliable input mocking.

Copilot uses AI. Check for mistakes.
mock_input.return_value = '50'

# Temporarily replace the built-in input function
import builtins
Copy link

Copilot AI Sep 21, 2025

Choose a reason for hiding this comment

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

The import statement should be placed at the top of the file with other imports rather than inside the test function.

Copilot uses AI. Check for mistakes.
@dmccoystephenson dmccoystephenson merged commit 397f937 into main Oct 2, 2025
@dmccoystephenson dmccoystephenson deleted the copilot/fix-ed1b91bc-20a9-4a51-9e54-ce58430cdf33 branch October 2, 2025 06:13
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.

3 participants