Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,6 @@ export default [
index('routes/_index.tsx'),
route(':lang', 'routes/$lang.tsx'),
route(':lang/:codeLanguage/play', 'routes/$lang.$codeLanguage.play.tsx'),
route('result/create', 'routes/result.create.tsx'),
route('result/:id', 'routes/result.$id.tsx'),
] satisfies RouteConfig;
83 changes: 80 additions & 3 deletions app/routes/$lang.$codeLanguage.play.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useState, useEffect, useCallback } from 'react';
import { redirect } from 'react-router';
import { redirect, useFetcher, useNavigate } from 'react-router';
import type { Route } from './+types/$lang.$codeLanguage.play';
import type { SupportedLanguage } from '../locales';
import { t } from '../locales';
Expand All @@ -16,7 +16,8 @@ type GameState = {
score: number;
combo: number;
remainingSeconds: number; // 60 → 0
solvedIssueIds: string[]; // IDs of issues that have been found
solvedIssueIds: string[]; // IDs of issues that have been found in current problem
allSolvedIssueIds: string[]; // IDs of all issues found across all problems
tappedLines: Record<string, number[]>; // Map of problem ID to tapped lines
problemCount: number; // Number of problems solved
usedProblemIds: string[]; // IDs of problems that have been used
Expand Down Expand Up @@ -118,6 +119,8 @@ function selectNextProblemWithLevelAdvance(
*/
export default function Play({ loaderData }: Route.ComponentProps) {
const { lang, codeLanguage } = loaderData;
const fetcher = useFetcher();
const navigate = useNavigate();

// Initialize game state
const [gameState, setGameState] = useState<GameState>(() => {
Expand All @@ -129,6 +132,7 @@ export default function Play({ loaderData }: Route.ComponentProps) {
combo: 0,
remainingSeconds: 60,
solvedIssueIds: [],
allSolvedIssueIds: [],
tappedLines: {},
problemCount: 0,
usedProblemIds: firstProblem ? [firstProblem.id] : [],
Expand All @@ -139,6 +143,7 @@ export default function Play({ loaderData }: Route.ComponentProps) {
type: 'correct' | 'wrong' | 'complete';
text: string;
} | null>(null);
const [error, setError] = useState<string | null>(null);

// Timer countdown
useEffect(() => {
Expand Down Expand Up @@ -168,6 +173,61 @@ export default function Play({ loaderData }: Route.ComponentProps) {
}
}, [feedbackMessage]);

// Save score when game ends
useEffect(() => {
const scoreSaved = fetcher.state === 'idle' && fetcher.data?.success;

if (gameEnded && !scoreSaved && fetcher.state === 'idle') {
// Calculate total issues across all problems
const allProblems = getProblems(codeLanguage, 1)
.concat(getProblems(codeLanguage, 2))
.concat(getProblems(codeLanguage, 3));

const usedProblems = allProblems.filter((p) =>
gameState.usedProblemIds.includes(p.id)
);

const totalIssues = usedProblems.reduce(
(sum, problem) => sum + problem.issues.length,
0
);

const issuesFound = gameState.allSolvedIssueIds.length;
const accuracy = totalIssues > 0 ? issuesFound / totalIssues : 0;

// Prepare result data
const resultData = {
score: gameState.score,
issuesFound: issuesFound,
totalIssues: totalIssues,
accuracy: accuracy,
uiLanguage: lang,
codeLanguage: codeLanguage,
};

// Submit to result/create action
fetcher.submit(
{ payload: JSON.stringify(resultData) },
{ method: 'post', action: '/result/create' }
);
}
}, [gameEnded, fetcher.state, fetcher.data, gameState, codeLanguage, lang, fetcher]);

Comment on lines +176 to +215
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

Guard score-saving effect to avoid endless resubmits on failure

When /result/create returns { success: false, error: ... } (or any falsy success), fetcher.state goes back to 'idle' with fetcher.data set, so:

  • scoreSaved is false, and
  • if (gameEnded && !scoreSaved && fetcher.state === 'idle') becomes true again,

causing the effect to immediately re-call fetcher.submit. This will loop indefinitely on persistent failures, spamming your D1 endpoint and re-triggering the error UI.

You already surface errors nicely in the second effect and in the game-over UI, so it’s better to auto-submit only once and let the user decide if they want to retry.

One simple fix is to treat “has we ever tried to save?” as a separate guard derived from fetcher.state/fetcher.data, and only auto-submit when there has been no prior attempt:

   // Save score when game ends
   useEffect(() => {
-    const scoreSaved = fetcher.state === 'idle' && fetcher.data?.success;
-
-    if (gameEnded && !scoreSaved && fetcher.state === 'idle') {
+    const scoreSaved = !!fetcher.data?.success;
+    const hasAttemptedSave = fetcher.state !== 'idle' || fetcher.data != null;
+
+    if (gameEnded && !scoreSaved && !hasAttemptedSave) {
       // Calculate total issues across all problems
       const allProblems = getProblems(codeLanguage, 1)
         .concat(getProblems(codeLanguage, 2))
         .concat(getProblems(codeLanguage, 3));
@@
       // Submit to result/create action
       fetcher.submit(
         { payload: JSON.stringify(resultData) },
         { method: 'post', action: '/result/create' }
       );
     }
-  }, [gameEnded, fetcher.state, fetcher.data, gameState, codeLanguage, lang, fetcher]);
+  }, [gameEnded, fetcher.state, fetcher.data, gameState, codeLanguage, lang, fetcher]);

This keeps your existing success navigation and error handling intact, but prevents uncontrolled retries. If you later want a manual “Retry saving score” button, it can call fetcher.submit directly without changing this effect.

Also applies to: 216-229, 344-380


I'll verify the review comment by examining the actual codebase. Let me start by inspecting the file and understanding the complete context.

Now let me examine the actual file and related code sections.


🏁 Script executed:

#!/bin/bash
# Examine the actual file to verify the code matches the review comment
cat "app/routes/\$lang.\$codeLanguage.play.tsx" | head -230 | tail -80

Length of output: 2451


Let me examine the other locations mentioned in the review.


🏁 Script executed:

#!/bin/bash
# Check lines 344-380 mentioned in "Also applies to"
sed -n '340,380p' "app/routes/\$lang.\$codeLanguage.play.tsx"

Length of output: 1651


Let me check the /result/create action handler to confirm the failure response format:


🏁 Script executed:

#!/bin/bash
# Find the result/create action file
fd -type f -name "*.tsx" -o -name "*.ts" | xargs rg -l "result/create|export.*action" | head -20

Length of output: 288


🏁 Script executed:

#!/bin/bash
# Examine the result.create action handler
cat "app/routes/result.create.tsx"

Length of output: 2232


Fix the infinite loop in score submission on API failure

The infinite loop bug is confirmed and real. When /result/create fails and throws an error, fetcher.data becomes { error: '...' } and fetcher.state becomes 'idle'. Since the current logic checks only fetcher.state === 'idle' && fetcher.data?.success, the condition remains true when success is missing/falsy, triggering an immediate resubmit. This cycles indefinitely, spamming the D1 endpoint.

The proposed fix correctly guards against this by tracking whether a save attempt has ever been made, allowing the action to submit once and letting error handling (via the second effect) take over.

Apply the suggested diff to lines 176-215 as shown in the review comment.

🤖 Prompt for AI Agents
In app/routes/$lang.$codeLanguage.play.tsx around lines 176-215, the effect
resubmits score repeatedly when the API returns an error because it only checks
fetcher.state and fetcher.data?.success; fix by adding a one-time guard (e.g., a
useRef or boolean state like saveAttempted) initialized false, change the effect
condition to require !saveAttempted (so it only submits once), set saveAttempted
to true immediately when calling fetcher.submit, and keep the rest of the logic
(calculating totalIssues, accuracy, and submitting payload) the same so
subsequent failures are handled by the existing error-handling effect rather
than causing infinite resubmits.

// Navigate to result page when score is saved
useEffect(() => {
if (fetcher.data && fetcher.state === 'idle') {
const data = fetcher.data as { success?: boolean; id?: string; error?: string };

if (data.success && data.id) {
navigate(`/result/${data.id}`);
} else if (data.error) {
setError(data.error);
} else if (!data.success) {
setError(lang === 'ja' ? 'スコアの保存に失敗しました' : 'Failed to save score');
}
}
}, [fetcher.data, fetcher.state, navigate, lang]);

/**
* Handle line tap
*/
Expand Down Expand Up @@ -199,6 +259,7 @@ export default function Play({ loaderData }: Route.ComponentProps) {
const newCombo = prev.combo + 1;
const scoreGain = calculateScore(hitIssue.score, newCombo);
const newSolvedIssueIds = [...prev.solvedIssueIds, hitIssue.id];
const newAllSolvedIssueIds = [...prev.allSolvedIssueIds, hitIssue.id];

setFeedbackMessage({
type: 'correct',
Expand All @@ -210,6 +271,7 @@ export default function Play({ loaderData }: Route.ComponentProps) {
score: prev.score + scoreGain,
combo: newCombo,
solvedIssueIds: newSolvedIssueIds,
allSolvedIssueIds: newAllSolvedIssueIds,
tappedLines: newTappedLines,
};
} else {
Expand Down Expand Up @@ -279,6 +341,8 @@ export default function Play({ loaderData }: Route.ComponentProps) {

// Show game over screen
if (gameEnded) {
const isSaving = fetcher.state === 'submitting' || fetcher.state === 'loading';

return (
<>
<Header currentLang={lang} />
Expand All @@ -302,8 +366,21 @@ export default function Play({ loaderData }: Route.ComponentProps) {
: `Solved ${gameState.problemCount} problems`}
</div>
</div>

{isSaving && (
<div className="text-sm text-slate-600 dark:text-slate-400 animate-pulse">
{lang === 'ja' ? 'スコアを保存中...' : 'Saving score...'}
</div>
)}

{error && (
<div className="text-sm text-red-600 dark:text-red-400 bg-red-50 dark:bg-red-900/20 rounded-md p-3">
{error}
</div>
)}

<button
onClick={() => window.location.href = `/${lang}`}
onClick={() => (window.location.href = `/${lang}`)}
className="w-full py-3 text-lg font-semibold rounded-md bg-sky-500 text-white hover:bg-sky-600 active:bg-sky-700 transition"
>
{lang === 'ja' ? 'ホームへ戻る' : 'Back to Home'}
Expand Down
Loading