Skip to content
Open
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
3 changes: 3 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
"minigames",
"rgba",
"Shorthair",
"Tetris",
"Tetromino",
"tetrominos",
"tseslint",
"Zäåö"
],
Expand Down
2 changes: 2 additions & 0 deletions src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Route, HashRouter as Router, Routes } from "react-router-dom";
const Home = React.lazy(() => import("./pages/Home"));
const HangmanPage = React.lazy(() => import("./pages/Hangman"));
const MemoryPage = React.lazy(() => import("./pages/Memory"));
const TetrisPage = React.lazy(() => import("./pages/Tetris"));

function App() {
return (
Expand All @@ -14,6 +15,7 @@ function App() {
<Route path="/" element={<Home />} />
<Route path="/hangman" element={<HangmanPage />} />
<Route path="/memory" element={<MemoryPage />} />
<Route path="/tetris" element={<TetrisPage />} />
</Routes>
</div>
</Router>
Expand Down
20 changes: 20 additions & 0 deletions src/components/Tetris/GameBoard.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
.game-board {
display: grid;
grid-template-rows: repeat(20, 20px);
grid-template-columns: repeat(10, 20px);
Comment on lines +3 to +4

Choose a reason for hiding this comment

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

The grid dimensions are defined with fixed pixel values (20px), which limits the responsiveness and scalability of the game board. Consider using relative units like vw, vh, or percentages to make the grid responsive and adaptable to different screen sizes.

For example, you could modify the grid definition to use viewport units:

.game-board {
  grid-template-rows: repeat(20, 1vw);
  grid-template-columns: repeat(10, 1vw);
}

gap: 1px;
}

.row {
display: contents;
}

.cell {
width: 20px;
height: 20px;
background-color: #ddd;
}

.cell.filled {
background-color: #333;
}
24 changes: 24 additions & 0 deletions src/components/Tetris/GameBoard.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import "./GameBoard.css";

interface GameBoardProps {
board: number[][];
}

const GameBoard: React.FC<GameBoardProps> = ({ board }) => {
return (
<div className="game-board">
{board.map((row, rowIndex) => (
<div key={rowIndex} className="row">
{row.map((cell, cellIndex) => (
<div
key={cellIndex}
className={`cell ${cell ? "filled" : ""}`}
Comment on lines +14 to +15

Choose a reason for hiding this comment

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

Using cellIndex as a key for cells might lead to issues with unnecessary re-renders or DOM manipulations if the board structure changes (e.g., rows are added or removed), as the indices might not uniquely identify a cell over re-renders.

Recommended Solution: Use a more stable identifier for the key, such as a combination of rowIndex and cellIndex (e.g., `key={
owIndex-cellIndex}"). This ensures that each cell maintains a unique identifier across re-renders, improving React's ability to manage DOM updates efficiently.

></div>
))}
</div>
))}
Comment on lines +10 to +19

Choose a reason for hiding this comment

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

The use of nested .map() functions for rendering the game board can lead to performance issues, especially for larger boards or frequent updates. Each .map() call creates a new function instance, which can be costly in terms of performance.

Recommended Solution: Consider using a memoization technique or React's useMemo hook to cache the board's rendered output unless the board prop changes. This can prevent unnecessary re-renders and improve performance.

</div>
);
};

export default GameBoard;
110 changes: 110 additions & 0 deletions src/components/Tetris/Tetris.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
import React, { useCallback, useEffect, useState } from "react";
import { checkCollision, createBoard, getRandomTetromino } from "./utils";

import GameBoard from "./GameBoard";
import Tetromino from "./Tetromino";

interface Tetromino {
shape: number[][];
color: string;
}

// TODO: does not work
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (complexity): Consider extracting the game logic into a custom hook to separate it from the rendering logic.

The component's complexity can be reduced by:

  1. Extracting game logic into a custom hook
  2. Removing unnecessary useCallback wrappers
  3. Simplifying state management

Example implementation:

// useTetrisGame.ts
export function useTetrisGame() {
  const [board, setBoard] = useState(createBoard());
  const [tetromino, setTetromino] = useState<Tetromino>(getRandomTetromino());
  const [position, setPosition] = useState({ x: 3, y: 0 });
  const [gameOver, setGameOver] = useState(false);

  function moveTetromino(direction: number) {
    const newPosition = { ...position, x: position.x + direction };
    if (!checkCollision(tetromino.shape, board, newPosition)) {
      setPosition(newPosition);
    }
  }

  function dropTetromino() {
    const newPosition = { ...position, y: position.y + 1 };
    if (!checkCollision(tetromino.shape, board, newPosition)) {
      setPosition(newPosition);
      return;
    }

    const newBoard = [...board];
    tetromino.shape.forEach((row, y) => {
      row.forEach((cell, x) => {
        if (cell !== 0) {
          newBoard[position.y + y][position.x + x] = cell;
        }
      });
    });

    setBoard(newBoard);
    setTetromino(getRandomTetromino());
    setPosition({ x: 3, y: 0 });

    if (checkCollision(tetromino.shape, newBoard, { x: 3, y: 0 })) {
      setGameOver(true);
    }
  }

  return {
    board,
    tetromino,
    position,
    gameOver,
    moveTetromino,
    dropTetromino,
    // ... other methods
  };
}

// Tetris.tsx
const Tetris: React.FC = () => {
  const game = useTetrisGame();

  useEffect(() => {
    const handleKeyDown = (event: KeyboardEvent) => {
      switch (event.key) {
        case "ArrowLeft": game.moveTetromino(-1); break;
        case "ArrowRight": game.moveTetromino(1); break;
        case "ArrowDown": game.dropTetromino(); break;
        case "ArrowUp": game.rotateTetromino(); break;
      }
    };
    window.addEventListener("keydown", handleKeyDown);
    return () => window.removeEventListener("keydown", handleKeyDown);
  }, [game]);

  return (
    <div className="game">
      {game.gameOver ? <div className="game-over">Game Over</div> : null}
      <GameBoard board={game.board} />
      <Tetromino shape={game.tetromino.shape} position={game.position} />
    </div>
  );
};

This approach:

  • Separates game logic from rendering
  • Removes unnecessary callback wrapping
  • Makes the component's responsibility clear
  • Improves testability of game logic

const Tetris: React.FC = () => {
const [board, setBoard] = useState(createBoard());
const [tetromino, setTetromino] = useState<Tetromino>(getRandomTetromino());
const [position, setPosition] = useState({ x: 3, y: 0 });
const [gameOver, setGameOver] = useState(false);

const moveTetromino = useCallback(
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: Add bounds checking to prevent pieces from moving off the board edges

The current collision detection only checks for collisions with other pieces and the bottom. You should also check if position.x + tetromino width is within the board bounds.

(direction: number) => {
const newPosition = { ...position, x: position.x + direction };
if (!checkCollision(tetromino.shape, board, newPosition)) {
setPosition(newPosition);
}
},
[position, tetromino.shape, board],
);

const dropTetromino = useCallback(() => {
const newPosition = { ...position, y: position.y + 1 };
if (!checkCollision(tetromino.shape, board, newPosition)) {
setPosition(newPosition);
} else {
// Lock the tetromino and generate a new one
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: Implement row clearing mechanics when a row is completely filled

After locking a piece, check for and remove any completed rows, then shift all rows above downward.

const newBoard = [...board];
tetromino.shape.forEach((row, y) => {
row.forEach((cell, x) => {
if (cell !== 0) {
newBoard[position.y + y][position.x + x] = cell;
}
});
});
Comment on lines +35 to +42

Choose a reason for hiding this comment

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

The operation const newBoard = [...board]; on line 35 only creates a shallow copy of the board array. Since board is an array of arrays, this means the inner arrays are not copied, and modifying newBoard will mutate the original board state. This can lead to bugs and performance issues.

Recommended Solution:
Use a deep copy method to ensure that the inner arrays are also copied. You can achieve this by using map to copy each inner array:

const newBoard = board.map(innerArray => [...innerArray]);

setBoard(newBoard);
setTetromino(getRandomTetromino());
setPosition({ x: 3, y: 0 });

if (checkCollision(tetromino.shape, newBoard, { x: 3, y: 0 })) {
setGameOver(true);
}
Comment on lines +47 to +49

Choose a reason for hiding this comment

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

The game over check is performed after setting a new tetromino, which might not be the most efficient or safest point to perform this check. This could potentially lead to a scenario where the game over state is set but the board state is not correctly updated to reflect this, leading to inconsistencies in the UI and game logic.

Recommended Solution:
Consider checking for game over conditions before setting the new tetromino. This way, you can ensure that all game state updates are consistent and reflect the actual state of the game:

if (checkCollision(tetromino.shape, board, { x: 3, y: 0 })) {
  setGameOver(true);
} else {
  setTetromino(getRandomTetromino());
  setPosition({ x: 3, y: 0 });
}

}
}, [position, tetromino.shape, board]);

const rotateTetromino = useCallback(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Implement wall kick behavior when rotating pieces near walls

When a rotation would cause a collision, try shifting the piece left or right to make it fit - this is standard Tetris behavior known as wall kicks.

const rotatedTetromino = tetromino.shape
.map((_, index) => tetromino.shape.map((col) => col[index]))
.reverse();

const newPosition = { ...position };

if (!checkCollision(rotatedTetromino, board, newPosition)) {
setTetromino({
...tetromino,
shape: rotatedTetromino,
});
}
}, [tetromino, position, board]);

useEffect(() => {
const handleKeyDown = (event: KeyboardEvent) => {
switch (event.key) {
case "ArrowLeft":
moveTetromino(-1);
break;
case "ArrowRight":
moveTetromino(1);
break;
case "ArrowDown":
dropTetromino();
break;
case "ArrowUp":
rotateTetromino();
break;
}
};

window.addEventListener("keydown", handleKeyDown);

return () => {
window.removeEventListener("keydown", handleKeyDown);
};
}, [dropTetromino, moveTetromino, rotateTetromino]);

useEffect(() => {
const interval = setInterval(() => {
dropTetromino();
}, 1000);

return () => clearInterval(interval);
}, [dropTetromino]);

return (
<div className="game">
{gameOver ? <div className="game-over">Game Over</div> : null}
<GameBoard board={board} />
<Tetromino shape={tetromino.shape} position={position} />
</div>
);
};

export default Tetris;
31 changes: 31 additions & 0 deletions src/components/Tetris/Tetromino.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import React from "react";

interface TetrominoProps {
shape: number[][];
position: { x: number; y: number };
}

const Tetromino: React.FC<TetrominoProps> = ({ shape, position }) => {
return (
<div
style={{
position: "absolute",
top: position.y * 20,
left: position.x * 20,
Comment on lines +13 to +14

Choose a reason for hiding this comment

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

The positioning of the Tetromino uses hardcoded multipliers (20) for the top and left styles. This approach reduces the flexibility of the component, especially if the size of the cells needs to be adjusted or made responsive. Consider defining these values as props or using a scaling factor that can be adjusted based on the component's environment.

Recommended Solution:
Add a new prop, e.g., scale, and use it to calculate the position:

const scale = props.scale || 20; // default to 20 if not provided
const style = useMemo(() => ({
  position: 'absolute',
  top: position.y * scale,
  left: position.x * scale
}), [position.x, position.y, scale]);

}}
Comment on lines +11 to +15

Choose a reason for hiding this comment

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

Using inline styles in React components can lead to performance issues because the style object is recreated on every render. This can cause unnecessary re-renders of the DOM elements. Consider using useMemo for the style object or moving the styles to a CSS class to prevent this issue.

Recommended Solution:

const style = useMemo(() => ({
  position: 'absolute',
  top: position.y * 20,
  left: position.x * 20
}), [position.x, position.y]);

Then use this style in your component.

>
{shape.map((row, rowIndex) => (
<div key={rowIndex}>
{row.map((cell, cellIndex) => (
<div
key={cellIndex}
className={`cell ${cell ? "filled" : ""}`}
></div>
))}
</div>
))}
</div>
);
};

export default Tetromino;
83 changes: 83 additions & 0 deletions src/components/Tetris/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
export const checkCollision = (
tetromino: number[][],
board: number[][],
position: { x: number; y: number },
): boolean => {
for (let y = 0; y < tetromino.length; y++) {
for (let x = 0; x < tetromino[y].length; x++) {
if (tetromino[y][x] !== 0 && (!board[position.y + y] ||
!board[position.y + y][position.x + x] ||
board[position.y + y][position.x + x] !== 0)) {
return true;
}

}
}
return false;
};

export const createBoard = (): number[][] => {
return Array.from({ length: 20 }, () => Array(10).fill(0));
};

export const TETROMINOS = {
0: { shape: [[0]], color: "0, 0, 0" },
I: { shape: [[1, 1, 1, 1]], color: "80, 227, 230" },
J: {
shape: [
[1, 0, 0],
[1, 1, 1],
],
color: "36, 95, 223",
},
L: {
shape: [
[0, 0, 1],
[1, 1, 1],
],
color: "223, 173, 36",
},
O: {
shape: [
[1, 1],
[1, 1],
],
color: "223, 217, 36",
},
S: {
shape: [
[0, 1, 1],
[1, 1, 0],
],
color: "48, 211, 56",
},
T: {
shape: [
[0, 1, 0],
[1, 1, 1],
],
color: "132, 61, 198",
},
Z: {
shape: [
[1, 1, 0],
[0, 1, 1],
],
color: "227, 78, 78",
},
};

export const getRandomTetromino = (): { shape: number[][]; color: string } => {
const tetrominos: Array<keyof typeof TETROMINOS> = [
"I",
"J",
"L",
"O",
"S",
"T",
"Z",
];
const randTetromino =
tetrominos[Math.floor(Math.random() * tetrominos.length)];
return TETROMINOS[randTetromino];
Comment on lines +81 to +82

Choose a reason for hiding this comment

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

The getRandomTetromino function uses Math.random() for generating a random index, which is suitable for non-security-critical applications like games. However, it's important to be aware that Math.random() does not provide cryptographic security and can be predictable. This is generally not an issue for game mechanics but could be relevant if the randomness affects competitive gameplay or if used in a security-sensitive context.

Note: No change is necessary for the current context unless the application requirements specify higher security for random number generation.

};
1 change: 1 addition & 0 deletions src/pages/Home.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Link } from "react-router-dom";
const games = [
{ name: "Hangman", path: "/hangman" },
{ name: "Memory", path: "/memory" },
{ name: "Tetris", path: "/tetris" },
];

function Home() {
Expand Down
7 changes: 7 additions & 0 deletions src/pages/Tetris.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import Tetris from "../components/Tetris/Tetris";

function TetrisPage() {
return <Tetris />;
}

export default TetrisPage;
Loading