Skip to content

Commit 8fd7f36

Browse files
jduffeyjduffey
authored
Roulette bugs galore (#44)
* bugs docs * Fix critical roulette bugs: infinite loops, zero display, simulation intervals, JSX, and stale state - Fix infinite update loop in CompletionsCounter by removing completionsCount from useEffect dependencies - Fix formattedChainNumber to display 0 values correctly in PlayerInfo, HouseInfo, and CompletionsCounter - Fix invalid JSX <br /> tags in PlayerInfo and HouseInfo - Fix stale state bug in SpinButton by computing display flag locally - Remove duplicate value in NumberHitGameCounter outline breakpoints - Rewrite simulatePlayingGame.js to use a single interval and proper cleanup, preventing runaway timers and memory leaks All changes address issues documented in ROULETTE_BUGS.md and improve performance, user experience, and code quality. * Remove unused shouldDisplayExtraMessage state variable from SpinButton - Remove unused state variable that was left over from refactoring - Fixes ESLint no-unused-vars warning - Component logic remains unchanged, just cleaner code
1 parent 9a67625 commit 8fd7f36

File tree

11 files changed

+488
-27
lines changed

11 files changed

+488
-27
lines changed

‎ROULETTE_BUGS.md‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
# Current Roulette Bugs
2+
3+
This document lists all currently unresolved bugs in the roulette codebase as of the latest review.
4+
5+
## Critical Bugs
6+
7+
### 1. Infinite Update Loops in `CompletionsCounter`
8+
**File**: `src/components/roulette/CompletionsCounter.js` (lines 18-23)
9+
10+
**Issue**: The `useEffect` includes `completionsCount` in its dependency array while also updating that same state, causing infinite re-renders and unnecessary API calls.
11+
12+
```javascript
13+
useEffect(() => {
14+
setTimeout(async () => {
15+
const count = await getPlayerNumberCompletionSetsCounter(props.playerAddress);
16+
setCompletionsCount(count);
17+
}, 1000);
18+
}, [props.playerAddress, completionsCount]); // ❌ completionsCount causes infinite loop
19+
```
20+
21+
**Fix**: Remove `completionsCount` from the dependency array:
22+
```javascript
23+
}, [props.playerAddress]); // ✅ Only depend on player address
24+
```
25+
26+
### 2. Uncontrolled Intervals in Simulation Script
27+
**File**: `src/common/simulatePlayingGame.js` (lines 14-39)
28+
29+
**Issue**: Nested `setInterval` calls without clearing them, creating runaway timers that accumulate over time.
30+
31+
```javascript
32+
setInterval(() => {
33+
let betsPlaced = 0;
34+
setInterval(() => { // ❌ Inner interval never cleared
35+
if (betsPlaced++ >= NUMBER_OF_BETS_TO_PLACE) return;
36+
// ... betting logic
37+
}, SECONDS_BETWEEN_BET_PLACEMENTS * 1000);
38+
// ... spin logic
39+
}, (NUMBER_OF_BETS_TO_PLACE + 1) * SECONDS_BETWEEN_BET_PLACEMENTS * 1000);
40+
```
41+
42+
**Fix**: Use a single interval or clear the inner interval after completion.
43+
44+
## Functional Bugs
45+
46+
### 3. `formattedChainNumber` Treats Zero as "Loading..."
47+
**Files**:
48+
- `src/components/roulette/PlayerInfo.js` (lines 1-6)
49+
- `src/components/roulette/HouseInfo.js` (lines 6-11)
50+
- `src/components/roulette/CompletionsCounter.js` (lines 6-11)
51+
52+
**Issue**: The falsy check `chainNumber ? ... : "Loading..."` treats `0` as falsy, causing legitimate zero values to display as "Loading...".
53+
54+
```javascript
55+
const formattedChainNumber = (chainNumber, decimals) => {
56+
return chainNumber
57+
? parseFloat(chainNumber)
58+
.toLocaleString('en-US', { minimumFractionDigits: decimals, maximumFractionDigits: decimals })
59+
: "Loading..."; // ❌ 0 is falsy, so shows "Loading..."
60+
}
61+
```
62+
63+
**Fix**: Check for `null` or `undefined` explicitly:
64+
```javascript
65+
return chainNumber !== null && chainNumber !== undefined
66+
? parseFloat(chainNumber)
67+
.toLocaleString('en-US', { minimumFractionDigits: decimals, maximumFractionDigits: decimals })
68+
: "Loading...";
69+
```
70+
71+
### 4. Stale State in `SpinButton`
72+
**File**: `src/components/roulette/SpinButton.js` (lines 10-18)
73+
74+
**Issue**: The `useEffect` uses `shouldDisplayExtraMessage` in its dependency array while also setting it, potentially causing stale state issues.
75+
76+
```javascript
77+
useEffect(() => {
78+
setShouldDisplayExtraMessage(!props.hasABetBeenPlaced || props.wheelIsSpinning);
79+
setZIndex(shouldDisplayExtraMessage ? 1 : -1); // ❌ Uses stale state
80+
setColor(shouldDisplayExtraMessage ? '#999999' : 'inherit'); // ❌ Uses stale state
81+
// ...
82+
}, [shouldDisplayExtraMessage, props.hasABetBeenPlaced, props.wheelIsSpinning]);
83+
```
84+
85+
**Fix**: Compute the flag locally and use it consistently:
86+
```javascript
87+
useEffect(() => {
88+
const shouldDisplay = !props.hasABetBeenPlaced || props.wheelIsSpinning;
89+
setShouldDisplayExtraMessage(shouldDisplay);
90+
setZIndex(shouldDisplay ? 1 : -1);
91+
setColor(shouldDisplay ? '#999999' : 'inherit');
92+
// ...
93+
}, [props.hasABetBeenPlaced, props.wheelIsSpinning]);
94+
```
95+
96+
## UI/Display Bugs
97+
98+
### 5. Invalid JSX Tags with Leading Spaces
99+
**Files**:
100+
- `src/components/roulette/HouseInfo.js` (line 39)
101+
- `src/components/roulette/PlayerInfo.js` (lines 15, 20, 25)
102+
103+
**Issue**: Contains `< br />` with leading space instead of `<br />`, producing invalid JSX.
104+
105+
```javascript
106+
House Balance
107+
< br /> {/* ❌ Invalid JSX */}
108+
{formattedChainNumber(houseBalance, 3)}
109+
```
110+
111+
**Fix**: Replace with proper JSX:
112+
```javascript
113+
House Balance
114+
<br /> {/* ✅ Correct JSX */}
115+
{formattedChainNumber(houseBalance, 3)}
116+
```
117+
118+
### 6. Duplicate Value in Game Counter Outline
119+
**File**: `src/components/roulette/NumberHitGameCounter.js` (line 12)
120+
121+
**Issue**: The outline breakpoints array contains a duplicate `220` value, which appears to be accidental.
122+
123+
```javascript
124+
const outline = [40, 50, 60, 70, 80, 90, 100, 110, 120, 130, 140, 150, 160, 170, 180, 190, 200, 210, 220, 220, 230, 240, 250, 260, 270, 280, 290, 300].includes(i) ? "1px solid black" : "none";
125+
// ^^^ ❌ Duplicate
126+
```
127+
128+
**Fix**: Remove the duplicate `220`:
129+
```javascript
130+
const outline = [40, 50, 60, 70, 80, 90, 100, 110, 120, 130, 140, 150, 160, 170, 180, 190, 200, 210, 220, 230, 240, 250, 260, 270, 280, 290, 300].includes(i) ? "1px solid black" : "none";
131+
```
132+
133+
## Previously Fixed Bugs
134+
135+
The following bugs have been resolved in recent updates:
136+
137+
✅ **Event listener leaks in `MostRecentSpinResults` and `SpinResult`** - Fixed with proper `useEffect` cleanup
138+
✅ **Event listener accumulation in `Roulette.js`** - Fixed with proper listener removal
139+
✅ **Infinite update loops in `NumbersHitTracker`** - Fixed by switching to event-driven updates
140+
✅ **Hard-coded contract address in `initializeChain.js`** - Fixed to use dynamic contract addresses
141+
142+
## Recommendations
143+
144+
1. **Priority 1**: Fix the infinite loop in `CompletionsCounter` as it causes performance issues
145+
2. **Priority 2**: Fix the `formattedChainNumber` zero bug as it affects user experience
146+
3. **Priority 3**: Fix the JSX formatting issues for cleaner code
147+
4. **Priority 4**: Address the simulation script intervals for better resource management
148+
5. **Priority 5**: Fix the stale state issue in `SpinButton` for more predictable behavior
149+
6. **Priority 6**: Remove the duplicate value in the game counter for cleaner code

0 commit comments

Comments
 (0)