Basic tic tac toe game in HTML isn't working for all winning moves

Question

So I wanted to just make a quick tic tac toe in HTML. I threw this together and now it only works when the first row wins. It doesn't like my tie check, or any other possible winning positions. It is fine with either X or O. I tried changing the order of the win states, which changes which win state is accepted. But IDK why it won't allow any of the others. I also tried changing [a,b,c] to a var instead of const, but that didn't fix it.

const board = document.querySelector('.board');
let turn = 'X';
board.addEventListener('click', e => {
  const square = e.target;

  if (square.innerText) {
    return; // already clicked, do nothing!
  }
  square.innerText = turn;
  if (turn === 'X') {
    square.classList.add('x');
    turn = 'O';
  } else {
    square.classList.add('o');
    turn = 'X';
  }
  checkWin(); // check for a win after each turn!
});

function checkWin() {
  const squares = document.querySelectorAll('.square');

  // winning combinations (3 in a row) 
  const winningCombos = [
    [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 top left to bottom right 
    [2, 4, 6] // diagonal top right to bottom left 
  ];

  for (let i = 0; i < winningCombos.length; i++) { // loop through all winning combos 

    const [a, b, c] = winningCombos[i]; // destructure the current combo into 3 variables 

    if (squares[a].innerText && squares[a].innerText === squares[b].innerText && squares[a].innerText === squares[c].innerText) { // check if all 3 squares have the same innerText (X or O) 

      alert(`
Player $ {
  squares[a].innerText
}
wins!`); // alert the winner! 

      board.removeEventListener('click', () => {}); // remove the click event listener so no more moves can be made 

      break; // break out of the loop since we found a winner! 

    } else if (squares.every(s => s.innerText)) { // check if all squares have been clicked (filled with X or O) and there is no winner yet 

      alert('It\'s a draw!'); // alert that it's a draw! 

      board.removeEventListener('click', () => {}); // remove the click event listener so no more moves can be made 

      break; // break out of the loop since it's a draw! 

    }

  }

}

.board {
  display: flex;
  flex-wrap: wrap;
  width: 300px;
  height: 300px;
  border: 1px solid #000;
}

.square {
  width: 100px;
  height: 100px;
  display: flex;
  align-items: center;
  justify-content: center;
  font-size: 48px;
  font-weight: bold;
  cursor: pointer;
}

.x {
  color: #f00;
}

.o {
  color: #00f;
}

<div class="board">
  <div class="square" data-index="0"></div>
  <div class="square" data-index="1"></div>
  <div class="square" data-index="2"></div>
  <div class="square" data-index="3"></div>
  <div class="square" data-index="4"></div>
  <div class="square" data-index="5"></div>
  <div class="square" data-index="6"></div>
  <div class="square" data-index="7"></div>
  <div class="square" data-index="8"></div>
</div>

score 1 · Accepted Answer · answered Jan 02 '23 at 01:17

1

Be sure to check your console for errors.

squares is not a vanilla JavaScript array, but a NodeList. This means that you don't have access to Array.prototype.every().

Try this instead:

const squares = Array.from(document.querySelectorAll('.square'));

answered Jan 02 '23 at 01:17

Nathan Wiles

841
10
30

score 0 · Answer 2 · answered Jan 02 '23 at 03:35

Here is alternative; set the square grid to well, a grid CSS and inserted fixed spaces initially in them.

You can use square.dataset.player instead of the innerText - makes formatting a bit better perhaps but also removes logic from the visual. To iterate the nodes for the squares we can use squares.forEach((sq) => { Note I also did away with the removal of the click handler and set them to only fire once instead - see code comment. We can also filter nodes if we make an array of them with a spread operator i.e.: [...squares] and check for that dataset value.

const board = document.querySelector('.board');
const squares = board.querySelectorAll('.square');
// just me not liking the alert
const result = document.querySelector('#result');
// setup the variables
let turn = 'X';
let iWon = false;
const winningCombos = [
  [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 top left to bottom right 
  [2, 4, 6] // diagonal top right to bottom left 
];

// just set the turn
function setTurn(square) {
  square.innerText = turn;
  // use the dataset to make formatting better
  square.dataset.player = turn;
  if (turn === 'X') {
    square.classList.add('x');
    turn = 'O';
  } else {
    square.classList.add('o');
    turn = 'X';
  }
}

// check check the result (a bit verbose)
function checkCombo(combo, square) {
  // I went with your a b c but not really needed
  const a = 0;
  const b = 1;
  const c = 2;

  const val = square.innerText;
  const matchA = squares[combo[a]].innerText === val;
  const matchB = squares[combo[b]].innerText === val;
  const matchC = squares[combo[c]].innerText === val;
  return matchA && matchB && matchC;
}

function checkWin(event) {
  // if someone won already do nothing
  if (iWon) return;
  const square = event.target;
  setTurn(square);
  // loop through the win combo's
  for (let i = 0; i < winningCombos.length; i++) {
    const win = checkCombo(winningCombos[i], square);
    if (win) {
      iWon = win;
      break;
    }
  }
  // filter squares to see if they all have been clicked
  // the [...squares] makes an array of the nodes so we can filter them
  const squaresWithText = [...squares]
    .filter(square => square.dataset.player);
  const allClicked = squaresWithText.length == [...squares].length;
  // set the result or if no winner and not all clicked set to next player
  result.innerText = iWon ? `Player ${square.innerText} wins!` : allClicked ? "It's a draw!" : `Your turn ${turn}`;
}

// set a click event handler to fire ONCE per square
squares.forEach((sq) => {
  sq.addEventListener('click', checkWin, {
    capture: true, //displatch this first
    once: true // only called once
  });
});

.board {
  display: grid;
  grid-template-columns: repeat(3, 1fr);
  width: 300px;
  height: 300px;
  border: 1px solid #000;
}

.square {
  display: flex;
  align-items: center;
  justify-content: center;
  font-size: 3rem;
  font-weight: bold;
  cursor: pointer;
  border: dotted 2px green;
}

.x {
  color: #f00;
}

.o {
  color: #00f;
}

#result {
  margin: 1rem;
}

<div class="board">
  <div class="square" data-index="0">&nbsp;</div>
  <div class="square" data-index="1">&nbsp;</div>
  <div class="square" data-index="2">&nbsp;</div>
  <div class="square" data-index="3">&nbsp;</div>
  <div class="square" data-index="4">&nbsp;</div>
  <div class="square" data-index="5">&nbsp;</div>
  <div class="square" data-index="6">&nbsp;</div>
  <div class="square" data-index="7">&nbsp;</div>
  <div class="square" data-index="8">&nbsp;</div>
</div>
<div id="result"></div>

Basic tic tac toe game in HTML isn't working for all winning moves

2 Answers2

Linked