Closed
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThis PR introduces comprehensive development guidelines across project rule files, refactors the mini-game demo with character selection and loading phases, adds new UI components for intro and game screens, implements dynamic character texturing, and refines component type signatures. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant IntroScreen
participant GameState as Game State<br/>(useGame)
participant LoadingSystem as Loading System<br/>(useLoadingPhase)
participant GameUI
User->>LoadingSystem: Assets loading...
LoadingSystem->>GameState: setReady() on complete
GameState->>IntroScreen: phase = "ready"
IntroScreen->>User: Show character selection
User->>IntroScreen: Left/Right/Enter keys
IntroScreen->>GameState: setSelectedCharacterIndex(idx)
GameState->>IntroScreen: Update visual state
User->>IntroScreen: Press Play (Enter)
IntroScreen->>GameState: start()
GameState->>GameUI: phase = "playing"
GameUI->>User: Display timer & restart
GameUI->>User: Show active controls
User->>GameUI: Play game...
User->>GameUI: Game ends
GameUI->>GameState: end()
GameState->>IntroScreen: phase = "ended"
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
.cursor/rules/project-conventions.mdc (1)
6-6: Consider separating meta-instructions from conventions.The emoji directive is about AI chat behavior rather than a project convention. Mixing meta-instructions with technical conventions in the same section may reduce clarity.
📝 Suggested organization improvement
Consider adding a separate section header for AI behavior configuration:
+# AI Chat Configuration Start every chat with "🤖". +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.cursor/rules/project-conventions.mdc at line 6, The emoji directive "Start every chat with '🤖'." is a meta-instruction and should be moved out of the technical conventions list: create a new section header such as "AI behavior" or "Meta-instructions" and relocate that directive under it (remove it from the conventions section), update any surrounding headers or TOC if present, and keep the conventions section focused on technical/project rules only.app/routes/demos/12-mini-game/player/index.tsx (1)
15-17: Consider usingfor...ofto avoid lint noise.While the static analysis warning is technically a false positive (since
useTexture.preload()returns void), usingfor...ofis more explicit and silences the lint rule.♻️ Proposed refactor
-CHARACTER_IDS.forEach((id) => - useTexture.preload(`/models/player/${id}/base-color.png`) -); +for (const id of CHARACTER_IDS) { + useTexture.preload(`/models/player/${id}/base-color.png`); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/demos/12-mini-game/player/index.tsx` around lines 15 - 17, Replace the Array.prototype.forEach usage with an explicit for...of loop to call useTexture.preload for each id so the lint rule is silenced; locate the block that iterates over CHARACTER_IDS and change the iteration over CHARACTER_IDS that currently calls useTexture.preload(`/models/player/${id}/base-color.png`) inside .forEach to a for (const id of CHARACTER_IDS) loop that invokes useTexture.preload for each id.app/routes/demos/12-mini-game/player/player-types.ts (1)
1-3: Consider derivingPlayerIDfromCHARACTER_IDSto prevent drift.The array and union type are defined separately and could become inconsistent if one is modified without the other. Using
as constensures type safety and single source of truth.♻️ Proposed refactor
-export const CHARACTER_IDS = ["hungry", "angry", "shocked", "dizzy"]; - -export type PlayerID = "angry" | "dizzy" | "hungry" | "shocked"; +export const CHARACTER_IDS = ["hungry", "angry", "shocked", "dizzy"] as const; + +export type PlayerID = (typeof CHARACTER_IDS)[number];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/demos/12-mini-game/player/player-types.ts` around lines 1 - 3, CHARACTER_IDS and PlayerID are duplicated sources of truth and can drift; make CHARACTER_IDS a readonly tuple by adding `as const` and derive PlayerID from it using an indexed type (PlayerID = typeof CHARACTER_IDS[number]) so the union is generated from the array; update exports to keep CHARACTER_IDS and PlayerID exported and remove the manual union to ensure a single source of truth.app/routes/demos/12-mini-game/use-game.tsx (1)
30-45: Consider validating index bounds insetSelectedCharacterIndex.The setter accepts any number without validation. While the UI constrains input to valid indices, a bounds check would provide defensive protection.
🛡️ Optional: Add bounds validation
- setSelectedCharacterIndex: (index: number) => set({ selectedCharacterIndex: index }), + setSelectedCharacterIndex: (index: number) => set((state) => { + // Optionally import CHARACTER_IDS.length or pass max as param + if (index >= 0) { + return { selectedCharacterIndex: index }; + } + return {}; + }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/demos/12-mini-game/use-game.tsx` around lines 30 - 45, The setSelectedCharacterIndex setter currently accepts any number; update it to validate bounds before updating state by reading current characters/character count from the store (use set((state) => { ... }) so you can access state), then: if index is within 0 and characters.length-1 set selectedCharacterIndex, otherwise either clamp to valid range or ignore the set (return {}), and ensure you still return a partial state object like { selectedCharacterIndex: validIndex } when updating; reference the setSelectedCharacterIndex function and the store's characters/selectedCharacterIndex symbols when making the change.app/routes/demos/12-mini-game/interface/intro/index.tsx (1)
42-54: Consider adding accessible labels for character buttons.The character buttons use images with
alt={characterId}, but addingaria-labeloraria-pressedwould improve screen reader experience for the selection state.♿ Optional: Improve accessibility
<button key={characterId} className={`${styles.characterButton} ${selectedCharacterIndex === index ? styles.selected : ""}`} onClick={() => handleCharacterSelect(index)} disabled={phase === "loading"} + aria-label={`Select ${characterId} character`} + aria-pressed={selectedCharacterIndex === index} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/demos/12-mini-game/interface/intro/index.tsx` around lines 42 - 54, The character selection buttons lack accessible selection state and labels; update the button rendered in the map (the element using className styles.characterButton and onClick -> handleCharacterSelect) to include an explicit aria-label (e.g., a human-readable label derived from characterId) and an aria-pressed attribute set to (selectedCharacterIndex === index) so screen readers know which is selected; keep the existing disabled={phase === "loading"} and the image with className styles.characterPreview, and ensure the aria-label text is descriptive enough for accessibility.app/routes/demos/12-mini-game/interface/game-ui/index.tsx (1)
30-34: Consider consolidating keyboard control subscriptions.Five separate
useKeyboardControlscalls create five independent subscriptions. A single selector can reduce this overhead slightly.♻️ Optional consolidation
- const forward = useKeyboardControls((state) => state.forward); - const backward = useKeyboardControls((state) => state.backward); - const leftward = useKeyboardControls((state) => state.leftward); - const rightward = useKeyboardControls((state) => state.rightward); - const jump = useKeyboardControls((state) => state.jump); + const { forward, backward, leftward, rightward, jump } = useKeyboardControls( + (state) => ({ + forward: state.forward, + backward: state.backward, + leftward: state.leftward, + rightward: state.rightward, + jump: state.jump, + }) + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/demos/12-mini-game/interface/game-ui/index.tsx` around lines 30 - 34, Multiple separate subscriptions (useKeyboardControls) for forward, backward, leftward, rightward, and jump create unnecessary overhead; replace the five calls with a single call to useKeyboardControls that selects an object (or array) containing all keys, e.g. useKeyboardControls(state => ({ forward: state.forward, backward: state.backward, leftward: state.leftward, rightward: state.rightward, jump: state.jump })), then destructure that object into forward, backward, leftward, rightward, jump where they are used so the rest of the component references the consolidated values from one subscription.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/routes/demos/12-mini-game/index.tsx`:
- Line 37: Rename the control key or update state access so the keyboard control
name matches usage: change the control entry { name: "jump/confirm", keys:
["Space", "Enter"] } to { name: "jump", keys: ["Space", "Enter"] } so
useKeyboardControls creates state.jump, or alternatively update all usages (in
the Game UI component and the usePlayerControls hook) to access the control via
bracket notation state["jump/confirm"] (and corresponding destructuring from
useKeyboardControls) so the property name matches exactly.
- Around line 10-11: The import currently references a misleading CSS module
("import \"./styles.module.css\"") while the file only contains global rules;
rename the file from styles.module.css to styles.css and update the import to
"import \"./styles.css\"" so it reflects a regular global stylesheet (no CSS
module binding needed); ensure any other references to styles.module.css in this
component are updated accordingly.
In `@app/routes/demos/12-mini-game/interface/game-ui/index.tsx`:
- Around line 8-25: In updateTimer, guard against undefined startTime/endTime
from useGame.getState() before doing arithmetic: check that state.startTime (and
state.endTime when used) are finite numbers (e.g., typeof === "number" and
isFinite) and if not, set elapsedTime to 0 or return early and write a safe
default like "0.00" to timeRef; update the branches for phase === "playing" and
phase === "ended" to validate the operands before subtracting so you never
propagate NaN to timeRef.current.textContent.
In `@app/routes/demos/12-mini-game/player/index.tsx`:
- Around line 21-23: The component reads selectedCharacterIndex from useGame and
directly indexes CHARACTER_IDS which can crash if the index is out of bounds;
add defensive bounds validation before computing characterId/texture: clamp or
wrap selectedCharacterIndex into the valid range (0..CHARACTER_IDS.length-1)
inside the component (use the symbols selectedCharacterIndex, CHARACTER_IDS, and
the derived characterId/texture variables) so characterId =
CHARACTER_IDS[validatedIndex] and texture = useTexture(...) always receive a
valid id; keep existing behavior for valid indexes but guard against future
misuse of setSelectedCharacterIndex.
In `@app/routes/demos/12-mini-game/styles.module.css`:
- Around line 2-8: The `@font-face` block uses an unnecessary quoted font-family
name; update the font-family declaration in the `@font-face` rule by removing the
quotes around "Fredoka" (i.e., change font-family: "Fredoka"; to font-family:
Fredoka;) so the rule conforms to Stylelint expectations and avoids the lint
error while leaving the rest of the `@font-face` properties (src, font-weight,
font-stretch, font-display) unchanged.
In `@app/routes/home/index.tsx`:
- Line 37: Update the description string in app/routes/home/index.tsx (the JSX
prop named description) to correct the grammar and improve readability; replace
"In this site there are the demos made for the Three JS Journey Course before
reaching the React Three Fiber Chapter." with a clearer sentence such as "This
site contains the demos made for the Three JS Journey Course before reaching the
React Three Fiber chapter."
---
Nitpick comments:
In @.cursor/rules/project-conventions.mdc:
- Line 6: The emoji directive "Start every chat with '🤖'." is a
meta-instruction and should be moved out of the technical conventions list:
create a new section header such as "AI behavior" or "Meta-instructions" and
relocate that directive under it (remove it from the conventions section),
update any surrounding headers or TOC if present, and keep the conventions
section focused on technical/project rules only.
In `@app/routes/demos/12-mini-game/interface/game-ui/index.tsx`:
- Around line 30-34: Multiple separate subscriptions (useKeyboardControls) for
forward, backward, leftward, rightward, and jump create unnecessary overhead;
replace the five calls with a single call to useKeyboardControls that selects an
object (or array) containing all keys, e.g. useKeyboardControls(state => ({
forward: state.forward, backward: state.backward, leftward: state.leftward,
rightward: state.rightward, jump: state.jump })), then destructure that object
into forward, backward, leftward, rightward, jump where they are used so the
rest of the component references the consolidated values from one subscription.
In `@app/routes/demos/12-mini-game/interface/intro/index.tsx`:
- Around line 42-54: The character selection buttons lack accessible selection
state and labels; update the button rendered in the map (the element using
className styles.characterButton and onClick -> handleCharacterSelect) to
include an explicit aria-label (e.g., a human-readable label derived from
characterId) and an aria-pressed attribute set to (selectedCharacterIndex ===
index) so screen readers know which is selected; keep the existing
disabled={phase === "loading"} and the image with className
styles.characterPreview, and ensure the aria-label text is descriptive enough
for accessibility.
In `@app/routes/demos/12-mini-game/player/index.tsx`:
- Around line 15-17: Replace the Array.prototype.forEach usage with an explicit
for...of loop to call useTexture.preload for each id so the lint rule is
silenced; locate the block that iterates over CHARACTER_IDS and change the
iteration over CHARACTER_IDS that currently calls
useTexture.preload(`/models/player/${id}/base-color.png`) inside .forEach to a
for (const id of CHARACTER_IDS) loop that invokes useTexture.preload for each
id.
In `@app/routes/demos/12-mini-game/player/player-types.ts`:
- Around line 1-3: CHARACTER_IDS and PlayerID are duplicated sources of truth
and can drift; make CHARACTER_IDS a readonly tuple by adding `as const` and
derive PlayerID from it using an indexed type (PlayerID = typeof
CHARACTER_IDS[number]) so the union is generated from the array; update exports
to keep CHARACTER_IDS and PlayerID exported and remove the manual union to
ensure a single source of truth.
In `@app/routes/demos/12-mini-game/use-game.tsx`:
- Around line 30-45: The setSelectedCharacterIndex setter currently accepts any
number; update it to validate bounds before updating state by reading current
characters/character count from the store (use set((state) => { ... }) so you
can access state), then: if index is within 0 and characters.length-1 set
selectedCharacterIndex, otherwise either clamp to valid range or ignore the set
(return {}), and ensure you still return a partial state object like {
selectedCharacterIndex: validIndex } when updating; reference the
setSelectedCharacterIndex function and the store's
characters/selectedCharacterIndex symbols when making the change.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpublic/models/player/angry/base-color.pngis excluded by!**/*.pngpublic/models/player/dizzy/base-color.pngis excluded by!**/*.pngpublic/models/player/hungry/base-color.pngis excluded by!**/*.pngpublic/models/player/shocked/base-color.pngis excluded by!**/*.png
📒 Files selected for processing (21)
.cursor/rules/project-conventions.mdc.windsurf/rules/development-workflow.md.windsurf/rules/implementation-behavior.md.windsurf/rules/project-setup.md.windsurf/rules/styling.md.windsurf/rules/threejs-conventions.mdapp/routes/demos/12-mini-game/index.tsxapp/routes/demos/12-mini-game/interface/game-ui/index.tsxapp/routes/demos/12-mini-game/interface/index.tsxapp/routes/demos/12-mini-game/interface/intro/index.tsxapp/routes/demos/12-mini-game/interface/intro/use-intro-keyboard.tsapp/routes/demos/12-mini-game/interface/styles.module.cssapp/routes/demos/12-mini-game/main-scene.tsxapp/routes/demos/12-mini-game/player/index.tsxapp/routes/demos/12-mini-game/player/player-types.tsapp/routes/demos/12-mini-game/styles.module.cssapp/routes/demos/12-mini-game/use-game.tsxapp/routes/demos/12-mini-game/use-loading-phase.tsapp/routes/home/index.tsxapp/sketched-components/label/label.tsxapp/sketched-components/leva-panel/styles.module.css
💤 Files with no reviewable changes (1)
- app/sketched-components/leva-panel/styles.module.css
Comment on lines
+10
to
+11
| import "./styles.module.css"; | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat app/routes/demos/12-mini-game/styles.module.cssRepository: gianzamboni/three-js-react
Length of output: 365
🏁 Script executed:
# Find other CSS and CSS module imports to see the project's pattern
fd -e css -e module.css . app/routes --exec head -n 1 {} \; | head -20Repository: gianzamboni/three-js-react
Length of output: 194
🏁 Script executed:
# Search for CSS imports across the project
rg "import.*\.css" --max-count=10Repository: gianzamboni/three-js-react
Length of output: 2145
🏁 Script executed:
# Check the vite/build config to understand CSS module handling
fd -e config -e mjs -e ts \( -name "vite.*" -o -name "webpack.*" -o -name "tsconfig.*" \) | head -5Repository: gianzamboni/three-js-react
Length of output: 300
Rename styles.module.css to styles.css for clarity and consistency.
The file contains only global styles (@font-face and :root variables) with no scoped class names, so the .module.css extension is misleading. Throughout the codebase, CSS modules with scoped classes use import styles from ... binding, while global styles use regular .css files. Rename this to styles.css and import as import "./styles.css" to match the project convention.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/routes/demos/12-mini-game/index.tsx` around lines 10 - 11, The import
currently references a misleading CSS module ("import \"./styles.module.css\"")
while the file only contains global rules; rename the file from
styles.module.css to styles.css and update the import to "import
\"./styles.css\"" so it reflects a regular global stylesheet (no CSS module
binding needed); ensure any other references to styles.module.css in this
component are updated accordingly.
| { name: "leftward", keys: ["ArrowLeft", "KeyA"] }, | ||
| { name: "rightward", keys: ["ArrowRight", "KeyD"] }, | ||
| { name: "jump", keys: ["Space"] }, | ||
| { name: "jump/confirm", keys: ["Space", "Enter"] }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how jump is accessed in GameUI and related files
rg -n "useKeyboardControls.*jump|state\.jump|jump.*confirm" app/routes/demos/12-mini-game/Repository: gianzamboni/three-js-react
Length of output: 378
🏁 Script executed:
#!/bin/bash
# Let's also find where useKeyboardControls is defined/imported
rg -n "useKeyboardControls" app/routes/demos/12-mini-game/ -A 2 -B 2Repository: gianzamboni/three-js-react
Length of output: 2984
🏁 Script executed:
#!/bin/bash
# Check the main index.tsx file to see the keyboard controls definition
cat -n app/routes/demos/12-mini-game/index.tsx | head -50Repository: gianzamboni/three-js-react
Length of output: 1651
🏁 Script executed:
#!/bin/bash
# Check the GameUI component to see how it uses keyboard state
cat -n app/routes/demos/12-mini-game/interface/game-ui/index.tsxRepository: gianzamboni/three-js-react
Length of output: 2739
Fix keyboard control name mismatch: rename "jump/confirm" to "jump" or update state access to use state["jump/confirm"].
The control is named "jump/confirm" in the keyboard controls map, but both app/routes/demos/12-mini-game/interface/game-ui/index.tsx (line 34) and app/routes/demos/12-mini-game/player/use-player-controls.ts (line 37) access it as state.jump. Since useKeyboardControls creates state properties based on control names, accessing state.jump will return undefined. Either rename the control to "jump" to match all usage, or update the state access to use bracket notation like state["jump/confirm"].
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/routes/demos/12-mini-game/index.tsx` at line 37, Rename the control key
or update state access so the keyboard control name matches usage: change the
control entry { name: "jump/confirm", keys: ["Space", "Enter"] } to { name:
"jump", keys: ["Space", "Enter"] } so useKeyboardControls creates state.jump, or
alternatively update all usages (in the Game UI component and the
usePlayerControls hook) to access the control via bracket notation
state["jump/confirm"] (and corresponding destructuring from useKeyboardControls)
so the property name matches exactly.
Comment on lines
+8
to
+25
| function updateTimer(timeRef: RefObject<HTMLDivElement | null>) { | ||
| const state = useGame.getState(); | ||
|
|
||
| let elapsedTime = 0; | ||
|
|
||
| if (state.phase === "playing") { | ||
| elapsedTime = Date.now() - state.startTime; | ||
| } else if (state.phase === "ended") { | ||
| elapsedTime = state.endTime - state.startTime; | ||
| } | ||
|
|
||
| elapsedTime /= 1000; | ||
| const parsedElapsedTime = elapsedTime.toFixed(2); | ||
|
|
||
| if (timeRef.current) { | ||
| timeRef.current.textContent = parsedElapsedTime; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Consider guarding against undefined time values.
If startTime or endTime are undefined when their respective phases are active (e.g., due to a state management bug), the arithmetic would produce NaN, displaying "NaN" in the timer UI.
🛡️ Proposed defensive fix
function updateTimer(timeRef: RefObject<HTMLDivElement | null>) {
const state = useGame.getState();
let elapsedTime = 0;
if (state.phase === "playing") {
- elapsedTime = Date.now() - state.startTime;
+ elapsedTime = state.startTime ? Date.now() - state.startTime : 0;
} else if (state.phase === "ended") {
- elapsedTime = state.endTime - state.startTime;
+ elapsedTime = state.startTime && state.endTime
+ ? state.endTime - state.startTime
+ : 0;
}
elapsedTime /= 1000;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/routes/demos/12-mini-game/interface/game-ui/index.tsx` around lines 8 -
25, In updateTimer, guard against undefined startTime/endTime from
useGame.getState() before doing arithmetic: check that state.startTime (and
state.endTime when used) are finite numbers (e.g., typeof === "number" and
isFinite) and if not, set elapsedTime to 0 or return early and write a safe
default like "0.00" to timeRef; update the branches for phase === "playing" and
phase === "ended" to validate the operands before subtracting so you never
propagate NaN to timeRef.current.textContent.
Comment on lines
+21
to
+23
| const selectedCharacterIndex = useGame((state) => state.selectedCharacterIndex); | ||
| const characterId = CHARACTER_IDS[selectedCharacterIndex]; | ||
| const texture = useTexture(`/models/player/${characterId}/base-color.png`); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how selectedCharacterIndex is managed in the game store
rg -n -A 10 'selectedCharacterIndex' app/routes/demos/12-mini-game/use-game.tsxRepository: gianzamboni/three-js-react
Length of output: 1077
🏁 Script executed:
#!/bin/bash
# Find and examine CHARACTER_IDS definition
fd -e tsx -e ts app/routes/demos/12-mini-game | xargs grep -l 'CHARACTER_IDS'Repository: gianzamboni/three-js-react
Length of output: 493
🏁 Script executed:
#!/bin/bash
# Check the full use-game.tsx file to understand store initialization
wc -l app/routes/demos/12-mini-game/use-game.tsxRepository: gianzamboni/three-js-react
Length of output: 115
🏁 Script executed:
#!/bin/bash
# Search for CHARACTER_IDS definition
rg -n 'CHARACTER_IDS' app/routes/demos/12-mini-game/ --type tsx --type tsRepository: gianzamboni/three-js-react
Length of output: 97
🏁 Script executed:
#!/bin/bash
# Check the full player component file
cat -n app/routes/demos/12-mini-game/player/index.tsx | head -40Repository: gianzamboni/three-js-react
Length of output: 1557
🏁 Script executed:
#!/bin/bash
# Find and examine player-types file
fd player-types app/routes/demos/12-mini-game/player/Repository: gianzamboni/three-js-react
Length of output: 122
🏁 Script executed:
#!/bin/bash
# Search for CHARACTER_IDS with context
rg -n 'CHARACTER_IDS' app/routes/demos/12-mini-game/player/ -A 5 -B 2Repository: gianzamboni/three-js-react
Length of output: 2267
🏁 Script executed:
#!/bin/bash
# Find all calls to setSelectedCharacterIndex
rg -n 'setSelectedCharacterIndex' app/routes/demos/12-mini-game/ -B 3 -A 3Repository: gianzamboni/three-js-react
Length of output: 3250
🏁 Script executed:
#!/bin/bash
# Get the full use-intro-keyboard.ts to see what transform does
cat -n app/routes/demos/12-mini-game/interface/intro/use-intro-keyboard.tsRepository: gianzamboni/three-js-react
Length of output: 1885
🏁 Script executed:
#!/bin/bash
# Get the full intro/index.tsx to see how handleCharacterSelect is used
cat -n app/routes/demos/12-mini-game/interface/intro/index.tsxRepository: gianzamboni/three-js-react
Length of output: 2666
Add bounds validation for selectedCharacterIndex.
While all current call sites properly constrain the index (keyboard navigation uses modulo arithmetic, UI buttons iterate over CHARACTER_IDS), the setSelectedCharacterIndex setter accepts any number without validation. Adding a defensive fallback in the component prevents potential issues if the setter is misused in the future.
🛡️ Proposed defensive fix
const selectedCharacterIndex = useGame((state) => state.selectedCharacterIndex);
- const characterId = CHARACTER_IDS[selectedCharacterIndex];
+ const characterId = CHARACTER_IDS[selectedCharacterIndex] ?? CHARACTER_IDS[0];
const texture = useTexture(`/models/player/${characterId}/base-color.png`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const selectedCharacterIndex = useGame((state) => state.selectedCharacterIndex); | |
| const characterId = CHARACTER_IDS[selectedCharacterIndex]; | |
| const texture = useTexture(`/models/player/${characterId}/base-color.png`); | |
| const selectedCharacterIndex = useGame((state) => state.selectedCharacterIndex); | |
| const characterId = CHARACTER_IDS[selectedCharacterIndex] ?? CHARACTER_IDS[0]; | |
| const texture = useTexture(`/models/player/${characterId}/base-color.png`); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/routes/demos/12-mini-game/player/index.tsx` around lines 21 - 23, The
component reads selectedCharacterIndex from useGame and directly indexes
CHARACTER_IDS which can crash if the index is out of bounds; add defensive
bounds validation before computing characterId/texture: clamp or wrap
selectedCharacterIndex into the valid range (0..CHARACTER_IDS.length-1) inside
the component (use the symbols selectedCharacterIndex, CHARACTER_IDS, and the
derived characterId/texture variables) so characterId =
CHARACTER_IDS[validatedIndex] and texture = useTexture(...) always receive a
valid id; keep existing behavior for valid indexes but guard against future
misuse of setSelectedCharacterIndex.
Comment on lines
+2
to
+8
| @font-face { | ||
| font-family: "Fredoka"; | ||
| src: url("/fonts/Fredoka/Fredoka-VariableFont_wdth,wght.ttf") format("truetype"); | ||
| font-weight: 300 700; | ||
| font-stretch: 75% 125%; | ||
| font-display: swap; | ||
| } |
Contributor
There was a problem hiding this comment.
Remove unnecessary quotes around font-family name to satisfy Stylelint.
Stylelint flags the quotes around "Fredoka" as unnecessary. Single-word font names without special characters don't require quotes.
🔧 Proposed fix
`@font-face` {
- font-family: "Fredoka";
+ font-family: Fredoka;
src: url("/fonts/Fredoka/Fredoka-VariableFont_wdth,wght.ttf") format("truetype");
font-weight: 300 700;
font-stretch: 75% 125%;
font-display: swap;
}🧰 Tools
🪛 Stylelint (17.3.0)
[error] 3-3: Unexpected quotes around "Fredoka" (font-family-name-quotes)
(font-family-name-quotes)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/routes/demos/12-mini-game/styles.module.css` around lines 2 - 8, The
`@font-face` block uses an unnecessary quoted font-family name; update the
font-family declaration in the `@font-face` rule by removing the quotes around
"Fredoka" (i.e., change font-family: "Fredoka"; to font-family: Fredoka;) so the
rule conforms to Stylelint expectations and avoids the lint error while leaving
the rest of the `@font-face` properties (src, font-weight, font-stretch,
font-display) unchanged.
| link="https://three.gianfrancozamboni.com.ar/" | ||
| title="Three JS Journey without React" | ||
| description="This are the demos made for the Three JS Journey Course before reaching the React Three Fiber Chapter." | ||
| description="In this site there are the demos made for the Three JS Journey Course before reaching the React Three Fiber Chapter." |
Contributor
There was a problem hiding this comment.
Minor grammar issue: "In this site" → "On this site"
The phrase "In this site there are" is grammatically awkward. Consider:
- description="In this site there are the demos made for the Three JS Journey Course before reaching the React Three Fiber Chapter."
+ description="On this site are the demos made for the Three JS Journey Course before reaching the React Three Fiber Chapter."Or for better readability:
"This site contains the demos made for the Three JS Journey Course before reaching the React Three Fiber chapter."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| description="In this site there are the demos made for the Three JS Journey Course before reaching the React Three Fiber Chapter." | |
| description="On this site are the demos made for the Three JS Journey Course before reaching the React Three Fiber Chapter." |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/routes/home/index.tsx` at line 37, Update the description string in
app/routes/home/index.tsx (the JSX prop named description) to correct the
grammar and improve readability; replace "In this site there are the demos made
for the Three JS Journey Course before reaching the React Three Fiber Chapter."
with a clearer sentence such as "This site contains the demos made for the Three
JS Journey Course before reaching the React Three Fiber chapter."
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Refactor
Style