Javascript Question posted in
A very good W3school tutorial can be found here.

Javascript – Vanilla JS TicTacToe Win Logic

I’ve been messing around with some vanilla JS, having gotten too used to frameworks, and was building a simple TicTacToe game, and I am stuck on a certain piece of the win logic and switch players logic. Here is the code (hopefully it’s not too long):

const GameBoard = (() => {
  const board = Array(9).fill("");
  const getBoard = () => board;
  const resetBoard = () => board.fill("");

  const setBoard = (symbol, index) =>
    board.forEach(() => (board[index] === "" ? (board[index] = symbol) : ""));

  const checkForWinner = () => {
    const winningCombinations = [
      [0, 1, 2], // Top row
      [3, 4, 5], // Middle row
      [6, 7, 8], // Bottom row
      [0, 3, 6], // Left column
      [1, 4, 7], // Middle column
      [2, 5, 8], // Right column
      [0, 4, 8], // Diagonal from top-left to bottom-right
      [2, 4, 6], // Diagonal from top-right to bottom-left
    ];

    for (const combo of winningCombinations) {
      const [a, b, c] = combo;

      if (
        getBoard()[a] &&
        getBoard()[a] === getBoard()[b] &&
        getBoard()[a] === getBoard()[c]
      ) {
        return true;
      }
    }

    return false;
  };

  return { checkForWinner, getBoard, resetBoard, setBoard };
})();

const players = () => {
  const activePlayers = [
    {
      name: "Player 1 (X)",
      symbol: "X",
      active: true,
    },
    {
      name: "Player 2 (O)",
      symbol: "O",
      active: false,
    },
  ];

  const getActivePlayer = () => activePlayers.find((player) => player.active);
  const switchPlayers = () =>
    activePlayers.forEach((player) => (player.active = !player.active));

  return { getActivePlayer, switchPlayers };
};

(() => {
  const gameBoardContainer = document.querySelector("#game-board");
  const { checkForWinner, getBoard, setBoard } = GameBoard;
  const { getActivePlayer, switchPlayers } = players();
  let button;

  for (let i = 0; i < getBoard().length; i++) {
    button = document.createElement("button");
    gameBoardContainer.appendChild(button);
  }

  document.querySelectorAll("button").forEach((button, index) => {
    button.addEventListener("click", () => {
      if (getBoard()[index] === "" && !checkForWinner()) {
        setBoard(getActivePlayer().symbol, index);
        button.textContent = getActivePlayer().symbol;
        switchPlayers();
      }

      if (checkForWinner()) {
        console.log(`${getActivePlayer().name} has won!`);
        return;
      }
    });
  });
})();
<div id="game-board"></div>

My issue is specifically in the addEventListener of the button elements. The logic to check for a winner seems to work, but for some reason, when I am console.logging the active player winner, it actually logs the incorrect user for the win. If I have all X, then it logs that O is the winner, and vice versa. It’s something to do with the switchPlayers functionality I’m assuming, but not sure exactly where it’s failing and why.

Anyone offer any thoughts? Thank you…!

Tags:

2

Answers


  1. Chosen as BEST ANSWER
    0

    @gwcoffey - yep, that was the exact issue, thanks! Except I found a little bit of a cleaner way to solve the issue, which just involves changing the switchPlayers functionality like so:

    document.querySelectorAll("button").forEach((button, index) => {
    button.addEventListener("click", () => {
      if (getBoard()[index] === "" && !checkForWinner()) {
        setBoard(getActivePlayer().symbol, index);
        button.textContent = getActivePlayer().symbol;
      }

      if (checkForWinner()) {
        console.log(`${getActivePlayer().name} has won!`);
        return;
      }

      switchPlayers();
    });
  });

    where switchPlayers is just moved out of the conditional checks, and allows the switch to happen, and stops when the conditions are met. This seems to work from my testing, and I try to avoid nested conditionals in this regard, if possible. But thanks for answering...!


  2. 0

    The problem is that after a player makes a legal move, you call switchPlayers(). This happens before you then check for and report the winner. The relevant code is here:

    if (getBoard()[index] === "" && !checkForWinner()) {
   setBoard(getActivePlayer().symbol, index);
   button.textContent = getActivePlayer().symbol;
   switchPlayers(); // when X plays a wining move you switch to O here
}

if (checkForWinner()) {
   console.log(`${getActivePlayer().name} has won!`);
   return;
}

    There are probably lots of ways to solve this, like set the current player to the winner during checkForWinner() or restructure your logic a bit so you only call checkForWinner() once.

    But the fix that involves the fewest changes to your code is to not call switchPlayers() after a win, which is what I’ve done in the snippet below.

    const GameBoard = (() => {
  const board = Array(9).fill("");
  const getBoard = () => board;
  const resetBoard = () => board.fill("");

  const setBoard = (symbol, index) =>
    board.forEach(() => (board[index] === "" ? (board[index] = symbol) : ""));

  const checkForWinner = () => {
    const winningCombinations = [
      [0, 1, 2], // Top row
      [3, 4, 5], // Middle row
      [6, 7, 8], // Bottom row
      [0, 3, 6], // Left column
      [1, 4, 7], // Middle column
      [2, 5, 8], // Right column
      [0, 4, 8], // Diagonal from top-left to bottom-right
      [2, 4, 6], // Diagonal from top-right to bottom-left
    ];

    for (const combo of winningCombinations) {
      const [a, b, c] = combo;

      if (
        getBoard()[a] &&
        getBoard()[a] === getBoard()[b] &&
        getBoard()[a] === getBoard()[c]
      ) {
        return true;
      }
    }

    return false;
  };

  return { checkForWinner, getBoard, resetBoard, setBoard };
})();

const players = () => {
  const activePlayers = [
    {
      name: "Player 1 (X)",
      symbol: "X",
      active: true,
    },
    {
      name: "Player 2 (O)",
      symbol: "O",
      active: false,
    },
  ];

  const getActivePlayer = () => activePlayers.find((player) => player.active);
  const switchPlayers = () =>
    activePlayers.forEach((player) => (player.active = !player.active));

  return { getActivePlayer, switchPlayers };
};

(() => {
  const gameBoardContainer = document.querySelector("#game-board");
  const { checkForWinner, getBoard, setBoard } = GameBoard;
  const { getActivePlayer, switchPlayers } = players();
  let button;

  for (let i = 0; i < getBoard().length; i++) {
    button = document.createElement("button");
    gameBoardContainer.appendChild(button);
  }

  document.querySelectorAll("button").forEach((button, index) => {
    button.addEventListener("click", () => {
      if (getBoard()[index] === "" && !checkForWinner()) {
        setBoard(getActivePlayer().symbol, index);
        button.textContent = getActivePlayer().symbol;
        if (!checkForWinner()) {
          switchPlayers();
        }
      }

      if (checkForWinner()) {
        console.log(`${getActivePlayer().name} has won!`);
        return;
      }
    });
  });
})();
    #game-board {
  background: black;
  display: grid;
  grid-template-columns: repeat(3, 1fr);
  grid-template-rows: repeat(3, 1fr);
  gap: 5px;
  width: 300px;
  height: 300px;
}

button {
  border: none;
  background: white;
  font-size: 48pt;
  font-family: monospace;
}
    <!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <link rel="stylesheet" href="styles.css" />
    <title>Tic Tac Toe</title>
  </head>
  <body>
    <div id="game-board"></div>
    <script src="script.js"></script>
  </body>
</html>
    Login or Signup to reply.
Please signup or login to give your own answer.

Back To Top
Search