feat: Implement game logic with random problem selection #5
Conversation
- Add game route at /$lang/$codeLanguage/play (e.g., /ja/javascript/play) - Implement complete game state management (score, combo, timer, level progression) - Add line tap handling with correct/incorrect feedback - Implement combo multiplier system (1x, 1.2x, 1.5x, 2.0x) - Add random problem selection within each level (1→2→3) - Implement 60-second countdown timer - Add skip functionality with bonus for completing all issues - Create game over screen with final score display - Update Welcome component with code language selection and navigation - Update BASIC.md documentation with new route structure Game features: - Level progression: starts at level 1, advances to 2, then stays at 3 - Random problem selection from available problems in each level - Visual feedback for tapped lines (correct=green, wrong=red) - Real-time score, combo, and timer display - Issues found counter per problem
WalkthroughAdds a language- and code-language-aware Play route at Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Welcome
participant Router
participant PlayLoader as Play Route (Loader)
participant PlayUI as Game Component
participant GameState as Game Logic
User->>Welcome: choose language & codeLanguage
User->>Welcome: click Start
Welcome->>Router: navigate to /{lang}/{codeLanguage}/play
Router->>PlayLoader: invoke loader with params
rect rgb(200,220,255)
Note over PlayLoader: validate params\n(redirect if invalid)
PlayLoader-->>PlayUI: return loader data (lang, codeLanguage)
end
PlayUI->>GameState: init (level=1, timer=60s)
GameState->>GameState: selectRandomProblem(excludeIds)
rect rgb(220,240,220)
loop Game loop
User->>PlayUI: tap line / skip
PlayUI->>GameState: handleTap or handleSkip
GameState-->>PlayUI: updated state (score, combo, solved, next problem)
PlayUI-->>User: show feedback
GameState->>GameState: decrement timer each second
end
end
alt timer == 0
PlayUI->>User: render game-over screen
User->>PlayUI: click Home
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
- Add /:lang/:codeLanguage/play route to routes.ts - This fixes the issue where the Start button didn't navigate to the game
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
BASIC.md (1)
105-113: Docs updates align with implementation; consider syncing directory names
- The added note that “レベル内のデータはランダムに選択” and the
/$lang/$codeLanguage/playroute description are consistent with the currentselectRandomProblemimplementation and the new play route file.- The updated route table correctly reflects the language‑aware URLs being used by the loader and
Welcomescreen.- As a minor clean‑up (not blocking), the doc still refers to
src/routes/src/problemswhile the code lives underapp/routes/app/problems; updating those paths when convenient would avoid confusion for new contributors.Also applies to: 350-357
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
BASIC.md(2 hunks)app/routes/$lang.$codeLanguage.play.tsx(1 hunks)app/welcome/welcome.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/welcome/welcome.tsx (2)
app/locales/index.ts (1)
SupportedLanguage(8-8)app/problems/index.ts (1)
CodeLanguageOrAll(6-6)
app/routes/$lang.$codeLanguage.play.tsx (3)
app/problems/index.ts (4)
Problem(20-26)CodeLanguageOrAll(6-6)getProblems(58-66)calculateScore(83-95)app/locales/index.ts (2)
SupportedLanguage(8-8)t(21-24)app/components/Header.tsx (1)
Header(8-67)
🔇 Additional comments (1)
app/welcome/welcome.tsx (1)
1-2: Import is already consistent with the rest of the codebase; no action neededVerification confirms that
useNavigatefrom'react-router'in welcome.tsx matches the pattern throughout the app—Header.tsxand all other components also import from'react-router', not'react-router-dom'. The import is correct and needs no change.The optional a11y suggestion (wiring the
<label>to the<select>viahtmlFor/id) remains a minor improvement if you wish to address it, but the code is functionally sound as-is.
| // Initialize game state | ||
| const [gameState, setGameState] = useState<GameState>(() => { | ||
| const firstProblem = selectRandomProblem(codeLanguage, 1); | ||
| return { | ||
| currentProblem: firstProblem, | ||
| currentLevel: 1, | ||
| score: 0, | ||
| combo: 0, | ||
| remainingSeconds: 60, | ||
| solvedIssueIds: [], | ||
| tappedLines: {}, | ||
| problemCount: 0, | ||
| }; | ||
| }); |
There was a problem hiding this comment.
Timer continues while showing “No problems available”; short-circuit the countdown in that case
When there’s no currentProblem (e.g., no problems for the selected level/language, or after exhausting all problems), the component renders the “No problems available.” screen, but the timer effect continues to run and eventually flips the UI to the Game Over screen. That’s a bit confusing: once you tell the user there are no problems, the game effectively should be “over” and not counting down.
You can short‑circuit the timer when there is no current problem:
- useEffect(() => {
- if (gameEnded) return;
+ useEffect(() => {
+ if (gameEnded || !gameState.currentProblem) return;
@@
- }, [gameEnded]);
+ }, [gameEnded, gameState.currentProblem]);This keeps the “No problems available” message stable instead of morphing into Game Over after 60 seconds.
Also applies to: 94-110, 263-285
🤖 Prompt for AI Agents
In app/routes/$lang.$codeLanguage.play.tsx around lines 70-83 (and similarly at
94-110 and 263-285), the timer effect continues running even when currentProblem
is null causing the "No problems available" screen to later flip to Game Over;
update the timer logic to early return or not start the countdown when
gameState.currentProblem is falsy (and also guard any effects that transition to
Game Over) so the interval/timeout is only created when a problem exists, and
clear any existing interval when currentProblem becomes null to keep the "No
problems available" view stable.
problemCount semantics cause “0 problems solved” on timeout; adjust initialization/update
As implemented, problemCount starts at 0 and is only incremented inside handleSkip. If the timer expires while the player is still on the first problem (never pressed Skip), the Game Over screen will report 0 problems solved, which is misleading for users who have actually worked on one problem.
You can make problemCount represent “problems attempted” more accurately by:
- Initializing it to
1when a first problem exists. - Incrementing it only when a new problem is actually loaded.
For example:
- const [gameState, setGameState] = useState<GameState>(() => {
- const firstProblem = selectRandomProblem(codeLanguage, 1);
- return {
- currentProblem: firstProblem,
- currentLevel: 1,
- score: 0,
- combo: 0,
- remainingSeconds: 60,
- solvedIssueIds: [],
- tappedLines: {},
- problemCount: 0,
- };
- });
+ const [gameState, setGameState] = useState<GameState>(() => {
+ const firstProblem = selectRandomProblem(codeLanguage, 1);
+ return {
+ currentProblem: firstProblem,
+ currentLevel: 1,
+ score: 0,
+ combo: 0,
+ remainingSeconds: 60,
+ solvedIssueIds: [],
+ tappedLines: {},
+ // Count the first problem as “attempted” if it exists
+ problemCount: firstProblem ? 1 : 0,
+ };
+ });
@@
- setGameState((prev) => {
+ setGameState((prev) => {
// Advance level: 1 → 2 → 3 (max)
const nextLevel = Math.min(prev.currentLevel + 1, 3);
const nextProblem = selectRandomProblem(codeLanguage, nextLevel, usedProblemIds);
@@
- if (nextProblem) {
- setUsedProblemIds((prevIds) => [...prevIds, nextProblem.id]);
- }
-
- return {
- ...prev,
- currentProblem: nextProblem,
- currentLevel: nextLevel,
- score: prev.score + bonusScore,
- solvedIssueIds: [],
- problemCount: prev.problemCount + 1,
- };
+ if (nextProblem) {
+ setUsedProblemIds((prevIds) => [...prevIds, nextProblem.id]);
+ return {
+ ...prev,
+ currentProblem: nextProblem,
+ currentLevel: nextLevel,
+ score: prev.score + bonusScore,
+ solvedIssueIds: [],
+ problemCount: prev.problemCount + 1,
+ };
+ }
+
+ // No further problems: keep currentCount, just apply bonus + mark as cleared
+ return {
+ ...prev,
+ currentProblem: null,
+ currentLevel: nextLevel,
+ score: prev.score + bonusScore,
+ solvedIssueIds: [],
+ };
});
}, [gameEnded, gameState.currentProblem, gameState.solvedIssueIds, codeLanguage, usedProblemIds]);Also applies to: 188-223, 226-261
🤖 Prompt for AI Agents
In app/routes/$lang.$codeLanguage.play.tsx around lines 70-83, the
gameState.problemCount is initialized to 0 which yields “0 problems solved” if
the timer expires on the first problem; change the initialization to set
problemCount to 1 when a firstProblem exists, and then update the logic where
new problems are loaded (also review and adjust the handlers in ranges 188-223
and 226-261) so that problemCount is incremented whenever a new problem is
actually loaded/attempted (not only on handleSkip) — ensure both
skip/next/problem-load paths increment problemCount consistently and the Game
Over screen reads problemCount as “problems attempted.”
| const handleLineTap = useCallback( | ||
| (lineNumber: number) => { | ||
| if (gameEnded || !gameState.currentProblem) return; | ||
|
|
||
| const problemId = gameState.currentProblem.id; | ||
| const alreadyTapped = gameState.tappedLines[problemId]?.includes(lineNumber) || false; | ||
|
|
||
| // Ignore if already tapped | ||
| if (alreadyTapped) return; | ||
|
|
||
| // Find if this line has an issue | ||
| const issues = gameState.currentProblem.issues; | ||
| const hitIssue = issues.find( | ||
| (issue) => | ||
| issue.lines.includes(lineNumber) && !gameState.solvedIssueIds.includes(issue.id) | ||
| ); | ||
|
|
||
| setGameState((prev) => { | ||
| const newTappedLines = { | ||
| ...prev.tappedLines, | ||
| [problemId]: [...(prev.tappedLines[problemId] || []), lineNumber], | ||
| }; | ||
|
|
||
| if (hitIssue) { | ||
| // Correct! Increase score and combo | ||
| const newCombo = prev.combo + 1; | ||
| const scoreGain = calculateScore(hitIssue.score, newCombo); | ||
| const newSolvedIssueIds = [...prev.solvedIssueIds, hitIssue.id]; | ||
|
|
||
| setFeedbackMessage({ | ||
| type: 'correct', | ||
| text: `+${scoreGain} (${t('label.combo', lang)}: ${newCombo}x)`, | ||
| }); | ||
|
|
||
| return { | ||
| ...prev, | ||
| score: prev.score + scoreGain, | ||
| combo: newCombo, | ||
| solvedIssueIds: newSolvedIssueIds, | ||
| tappedLines: newTappedLines, | ||
| }; | ||
| } else { | ||
| // Wrong! Lose 1 point and reset combo | ||
| setFeedbackMessage({ | ||
| type: 'wrong', | ||
| text: `-1`, | ||
| }); | ||
|
|
||
| return { | ||
| ...prev, | ||
| score: Math.max(0, prev.score - 1), | ||
| combo: 0, | ||
| tappedLines: newTappedLines, | ||
| }; | ||
| } | ||
| }); | ||
| }, | ||
| [gameEnded, gameState.currentProblem, gameState.tappedLines, gameState.solvedIssueIds, lang] | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
State in handleLineTap mixes outer gameState reads with functional updates; consider tightening to prev only
handleLineTap correctly handles hit/miss logic, combo, and scoring, and integrates with calculateScore as specified. However, it reads gameState.currentProblem, gameState.tappedLines, and gameState.solvedIssueIds outside the setGameState functional updater, then also uses prev inside. This pattern works today but slightly increases the risk of stale reads if the function grows or concurrent interactions (future transitions, etc.) are added.
You can make it more robust by deriving everything from prev inside the updater:
- const handleLineTap = useCallback(
- (lineNumber: number) => {
- if (gameEnded || !gameState.currentProblem) return;
-
- const problemId = gameState.currentProblem.id;
- const alreadyTapped = gameState.tappedLines[problemId]?.includes(lineNumber) || false;
-
- // Ignore if already tapped
- if (alreadyTapped) return;
-
- // Find if this line has an issue
- const issues = gameState.currentProblem.issues;
- const hitIssue = issues.find(
- (issue) =>
- issue.lines.includes(lineNumber) && !gameState.solvedIssueIds.includes(issue.id)
- );
-
- setGameState((prev) => {
+ const handleLineTap = useCallback(
+ (lineNumber: number) => {
+ if (gameEnded) return;
+
+ setGameState((prev) => {
+ if (!prev.currentProblem) return prev;
+
+ const problemId = prev.currentProblem.id;
+ const alreadyTapped =
+ prev.tappedLines[problemId]?.includes(lineNumber) ?? false;
+ if (alreadyTapped) return prev;
+
+ const issues = prev.currentProblem.issues;
+ const hitIssue = issues.find(
+ (issue) =>
+ issue.lines.includes(lineNumber) && !prev.solvedIssueIds.includes(issue.id)
+ );
+
const newTappedLines = {
...prev.tappedLines,
[problemId]: [...(prev.tappedLines[problemId] || []), lineNumber],
};
@@
- setFeedbackMessage({
+ setFeedbackMessage({
type: 'correct',
text: `+${scoreGain} (${t('label.combo', lang)}: ${newCombo}x)`,
});
@@
- } else {
+ } else {
@@
- setFeedbackMessage({
+ setFeedbackMessage({
type: 'wrong',
text: `-1`,
});
@@
- });
- },
- [gameEnded, gameState.currentProblem, gameState.tappedLines, gameState.solvedIssueIds, lang]
+ });
+ },
+ [gameEnded, lang]
);This keeps all game‑state decisions inside a single, consistent snapshot (prev) and simplifies the dependency list.
Also applies to: 287-377
🏁 Script executed:
#!/bin/bash
# Quick sanity check: ensure no other handlers in this file mix outer state and functional updates.
rg -n "setGameState" app/routes/\$lang.\$codeLanguage.play.tsx -n -C3Length of output: 1077
---
🏁 Script executed:
```shell
#!/bin/bash
# Get context around the second location (lines 287-377) to see if it has the same pattern
sed -n '280,380p' app/routes/\$lang.\$codeLanguage.play.tsx
Length of output: 4130
---
🏁 Script executed:
```shell
#!/bin/bash
# Get the handler containing the setGameState call around line 206
sed -n '190,250p' app/routes/\$lang.\$codeLanguage.play.tsx
Length of output: 2173
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the exact function definition that contains the setGameState at line 206
sed -n '170,210p' app/routes/\$lang.\$codeLanguage.play.tsx
Length of output: 1149
---
🏁 Script executed:
```shell
#!/bin/bash
# Check what's actually in the 287-377 range (from the review's "Also applies to")
sed -n '287,377p' app/routes/\$lang.\$codeLanguage.play.tsx | head -50
Length of output: 2184
Correct line reference: the mixing pattern also exists in handleSkip (lines 186–229), not 287–377
Lines 287–377 are JSX render statements that read gameState externally; this is normal React rendering and not the mixing antipattern. The actual second handler sharing the same issue is handleSkip (lines 186–229):
const handleSkip = useCallback(() => {
if (gameEnded || !gameState.currentProblem) return;
const currentProblemIssues = gameState.currentProblem.issues;
const allIssuesSolved = currentProblemIssues.every((issue) =>
gameState.solvedIssueIds.includes(issue.id)
);
// ...
setGameState((prev) => {
// uses prev, not outer gameState
});
}, [gameEnded, gameState.currentProblem, gameState.solvedIssueIds, ...]);
Both handleLineTap and handleSkip read outer gameState before invoking setGameState, then also use prev inside the updater. Apply the same refactoring to both: move all state logic inside the updater and reduce the dependency list.
🤖 Prompt for AI Agents
In app/routes/$lang.$codeLanguage.play.tsx around lines 125–183 and 186–229,
both handleLineTap and handleSkip read from the outer gameState (e.g.,
currentProblem, tappedLines, solvedIssueIds) before calling setGameState and
also use the updater's prev, causing a stale/mixing antipattern; move all reads
and computations that depend on gameState into the setGameState updater (use
prev exclusively), avoid accessing gameState.* outside the updater (capture
gameEnded and lang from closure if needed), and then shrink each useCallback
dependency array to only the truly external values (e.g., gameEnded and lang or
any pure callbacks like calculateScore) so the handlers no longer depend on the
whole gameState object.
- Fix runtime error 'does not provide an export named' - Add 'import type' for SupportedLanguage in all components - Add 'import type' for CodeLanguageOrAll and Problem types - This ensures types are only used at compile time, not runtime
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/routes/$lang.$codeLanguage.play.tsx (2)
254-254: Prefernavigateoverwindow.location.hreffor SPA navigation.Using
window.location.hreftriggers a full page reload, which discards React state and reinitializes the app. Sincereact-router'suseNavigateis already imported for use elsewhere (seeapp/welcome/welcome.tsx), consider using it here for client-side navigation.Apply this approach to both locations:
+ const navigate = useNavigate(); + // In game over screen (line 254): - onClick={() => window.location.href = `/${lang}`} + onClick={() => navigate(`/${lang}`)} // In no-problem screen (line 278): - onClick={() => window.location.href = `/${lang}`} + onClick={() => navigate(`/${lang}`)}If a full reload is intentional to reset all state, document that decision with a comment.
Also applies to: 278-278
27-45: Loader validation is functional; consider deriving valid languages from a type or constant.The hardcoded array on line 36 duplicates the
CodeLanguageOrAlltype structure. If the valid set changes, both must be updated.You could derive the validation set from the type or centralize it in
app/problems/index.ts:// In app/problems/index.ts: export const VALID_CODE_LANGUAGES = ['all', 'javascript', 'php', 'ruby', 'java', 'dart'] as const; // In loader: import { VALID_CODE_LANGUAGES } from '../problems'; if (!VALID_CODE_LANGUAGES.includes(codeLanguage)) { throw redirect(`/${lang}`); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/components/Header.tsx(1 hunks)app/routes/$lang.$codeLanguage.play.tsx(1 hunks)app/routes/$lang.tsx(1 hunks)app/welcome/welcome.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/welcome/welcome.tsx (2)
app/locales/index.ts (1)
SupportedLanguage(8-8)app/problems/index.ts (1)
CodeLanguageOrAll(6-6)
app/routes/$lang.$codeLanguage.play.tsx (4)
app/problems/index.ts (4)
Problem(20-26)CodeLanguageOrAll(6-6)getProblems(58-66)calculateScore(83-95)app/routes/$lang.tsx (1)
loader(24-35)app/locales/index.ts (2)
SupportedLanguage(8-8)t(21-24)app/components/Header.tsx (1)
Header(9-68)
🔇 Additional comments (5)
app/routes/$lang.tsx (1)
3-3: LGTM! Type-only import is idiomatic.Converting to a type-only import is correct since
SupportedLanguageis used exclusively in type positions. This clarifies intent and can improve bundle size.app/components/Header.tsx (1)
2-3: LGTM! Correct separation of type and value imports.Splitting the import to use type-only for
SupportedLanguageand value import forsaveLanguageis the correct pattern and aligns with TypeScript best practices.app/welcome/welcome.tsx (2)
1-13: LGTM! State management and navigation are correctly implemented.The addition of controlled state for
codeLanguageand programmatic navigation viauseNavigateis well-structured. The navigation path/${lang}/${codeLanguage}/playcorrectly matches the route introduced in this PR.
50-61: LGTM! Controlled select component is properly implemented.The select element correctly binds
valueto state and updates viaonChange. The code language options align with the validation in the play route loader.app/routes/$lang.$codeLanguage.play.tsx (1)
97-112: Note: Previous review comments on this file remain relevant.Past reviews have flagged:
- Timer continuing when no problem is available (lines 97–112): The countdown runs even when
currentProblemisnull, causing the "No problems available" screen to flip to Game Over after 60 seconds.problemCountinitialization at 0 (lines 73–85): If the timer expires on the first problem, the Game Over screen shows "0 problems solved," which may confuse users.- State mixing in
handleLineTapandhandleSkip(lines 127–185, 190–226): Both handlers read from the outergameStatebefore callingsetGameState((prev) => ...), creating a stale-read antipattern. Moving all reads inside the updater and relying solely onprevwould improve robustness.These are not re-raised here to avoid duplication, but addressing them would strengthen the implementation.
Based on learnings
Also applies to: 127-185, 190-226
Game features:
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.