Critical bug fix for issue where Groundball A with runner on first would fail to execute a double play after game recovery from database. Root cause: current_on_base_code field was not recalculated during state recovery, defaulting to 0 (empty bases) even when runners were on base. This caused runner advancement logic to select Result 1 (batter out, runners hold) instead of Result 2 (double play). Changes: - Added calculate_on_base_code() helper method to GameState model - Updated _prepare_next_play() to use helper (eliminates duplication) - Fixed state recovery to calculate current_on_base_code from runners - Fixed X-Check G1 mapping (was GROUNDBALL_B, should be GROUNDBALL_A) - Added 5 regression tests to prevent recurrence Testing: - All 359 unit tests passing - New regression tests verify fix and demonstrate bug scenario - Tested in network dev environment - double plays now work correctly Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
153 lines
4.7 KiB
Markdown
153 lines
4.7 KiB
Markdown
# Bug Fix: Double Play Not Working After Game Recovery
|
|
|
|
**Issue**: [Gitea #3](https://git.manticorum.com/cal/strat-gameplay-webapp/issues/3)
|
|
|
|
**Reported By**: User during test game
|
|
**Date**: 2025-02-07
|
|
|
|
## Problem Description
|
|
|
|
When a user rejoined a game (after the game state was evicted from memory and recovered from the database), rolling a Groundball A with 0 outs and a runner on first base incorrectly resulted in:
|
|
|
|
- ❌ Runner on first out at second (correct)
|
|
- ❌ Batter **safe** at first (WRONG - should be out)
|
|
- ❌ **1 out recorded** (WRONG - should be 2 outs)
|
|
|
|
**Expected Behavior**: Double play - runner out at second, batter out at first, 2 outs recorded.
|
|
|
|
## Root Cause
|
|
|
|
The `current_on_base_code` field in `GameState` is a **snapshot field** that stores the base situation as a bit field (1=first, 2=second, 4=third, 7=loaded). This field is used by the runner advancement logic to determine which result to apply.
|
|
|
|
During normal gameplay, this field is set by `_prepare_next_play()` in the game engine. However, during **state recovery** from the database, it was not being recalculated, leaving it at the default value of `0` (empty bases).
|
|
|
|
This caused the runner advancement logic to select:
|
|
- **Result 1** (batter out, runners hold) instead of
|
|
- **Result 2** (double play at second and first)
|
|
|
|
## Fix Summary
|
|
|
|
### 1. Created Helper Method (PR #1)
|
|
|
|
Added `calculate_on_base_code()` method to `GameState` model to centralize the bit field calculation:
|
|
|
|
**File**: `backend/app/models/game_models.py`
|
|
|
|
```python
|
|
def calculate_on_base_code(self) -> int:
|
|
"""
|
|
Calculate on-base code from current runner positions.
|
|
|
|
Returns bit field where:
|
|
- Bit 0 (value 1): runner on first
|
|
- Bit 1 (value 2): runner on second
|
|
- Bit 2 (value 4): runner on third
|
|
- Value 7: bases loaded (1 + 2 + 4)
|
|
"""
|
|
code = 0
|
|
if self.on_first:
|
|
code |= 1
|
|
if self.on_second:
|
|
code |= 2
|
|
if self.on_third:
|
|
code |= 4
|
|
return code
|
|
```
|
|
|
|
### 2. Updated Game Engine (PR #1)
|
|
|
|
Refactored `_prepare_next_play()` to use the helper method:
|
|
|
|
**File**: `backend/app/core/game_engine.py` (line ~1014)
|
|
|
|
```python
|
|
# Before:
|
|
state.current_on_base_code = 0
|
|
if state.on_first:
|
|
state.current_on_base_code |= 1
|
|
if state.on_second:
|
|
state.current_on_base_code |= 2
|
|
if state.on_third:
|
|
state.current_on_base_code |= 4
|
|
|
|
# After:
|
|
state.current_on_base_code = state.calculate_on_base_code()
|
|
```
|
|
|
|
### 3. Fixed State Recovery (PR #1)
|
|
|
|
Added calculation of `current_on_base_code` at the end of state recovery:
|
|
|
|
**File**: `backend/app/core/state_manager.py` (line ~608)
|
|
|
|
```python
|
|
# CRITICAL FIX: Calculate current_on_base_code from actual runner positions
|
|
# This field is used by runner advancement logic to determine double plays
|
|
# Without this, recovered games default to on_base_code=0 (empty bases)
|
|
# even when runners are on base, breaking double play logic
|
|
state.current_on_base_code = state.calculate_on_base_code()
|
|
logger.debug(f"Recovery: Set current_on_base_code = {state.current_on_base_code}")
|
|
```
|
|
|
|
### 4. Fixed X-Check G1 Mapping (PR #2)
|
|
|
|
Also discovered and fixed a separate bug where X-Check G1 results were incorrectly mapped to GROUNDBALL_B instead of GROUNDBALL_A:
|
|
|
|
**File**: `backend/app/core/play_resolver.py` (line ~1450)
|
|
|
|
```python
|
|
# Before:
|
|
"G1": PlayOutcome.GROUNDBALL_B, # WRONG
|
|
|
|
# After:
|
|
"G1": PlayOutcome.GROUNDBALL_A, # Fixed: G1 is fast grounder (double play capable)
|
|
```
|
|
|
|
## Testing
|
|
|
|
### Regression Test Added
|
|
|
|
**File**: `backend/tests/unit/core/test_recovery_double_play_fix.py`
|
|
|
|
- ✅ Test verify `calculate_on_base_code()` for all base combinations
|
|
- ✅ Test recovered state with runner on first executes double play
|
|
- ✅ Test demonstrates bug behavior without fix
|
|
|
|
### Test Results
|
|
|
|
```
|
|
359 unit tests passing
|
|
5 new regression tests passing
|
|
```
|
|
|
|
## Impact
|
|
|
|
**Before Fix**:
|
|
- Games recovered from database would have incorrect on_base_code (always 0)
|
|
- Double plays would not work correctly after rejoining a game
|
|
- Force outs and other runner advancement scenarios also affected
|
|
|
|
**After Fix**:
|
|
- State recovery correctly calculates on_base_code from runner positions
|
|
- Double plays work correctly regardless of whether state was recovered
|
|
- All runner advancement logic functions properly after rejoin
|
|
|
|
## Related Issues
|
|
|
|
- This also fixes any other runner advancement issues that depend on `current_on_base_code` being accurate
|
|
- Prevents similar issues with Result 4 (force out), Result 10 (home-to-first DP), etc.
|
|
|
|
## Deployment Notes
|
|
|
|
No database migration required. The fix is purely in the application logic.
|
|
|
|
Users who experienced this bug should:
|
|
1. Refresh their browser after deployment
|
|
2. Previous games will work correctly after next state recovery
|
|
|
|
---
|
|
|
|
**Fixed**: 2025-02-07
|
|
**Tested**: ✅ All unit tests passing
|
|
**Status**: Ready for deployment
|