Wrong CSS class being applied on rerun of a function

Viewed 82

I'm developing a point-and-click game where a co-ordinate is presented to the user and they then have to click the corresponding co-ordinate.

I've styled it so that if they're correct, a class is added that puts a green border around their score. If they're incorrect a different class is added which puts a red border around their score. This works the first time the game is played. The problem is that every subsequent time it is played it only applies the red border regardless of whether it is correct or not.

I'm confused because it is still tallying the score correctly - if you click the right square then you'll still score a point, but it applies the wrong class.

Here's a link to my codepen: https://codepen.io/jacobc1596/pen/yLNwQZR

Here is what I consider the relevant code:

function startGame() {

    board.style.pointerEvents = 'all';
    target.innerHTML = randomSquare;
    gameTime()

    document.querySelectorAll('.square').forEach(item => {
        item.addEventListener('click', event => {
            if(item.id == randomSquare) {
                score++
                tries++
                scoreOutput.innerHTML = score;
                randomSquare = rndSq(squareset);
                target.innerHTML = randomSquare;
                scoreOutput.classList.add('correct'); //adds 'correct' class
                scoreOutput.classList.remove('incorrect'); //removes 'incorrect' class
            } else {
                tries++;
                // scoreDisplay.innerHTML = score;
                randomSquare = rndSq(squareset);
                target.innerHTML = randomSquare;
                scoreOutput.classList.remove('correct'); //removes 'correct' class
                scoreOutput.classList.add('incorrect'); //adds 'incorrect' class
            };
        })
    })
};

//Reset Game (runs when the game timer runs out)
function reset() {
    tries=0;
    score=0;
    target.innerHTML = '';
    strt.style.visibility = "visible";
    rst.style.visibility = 'hidden';
    board.style.pointerEvents = 'none'; 

    //to remove whatever class was last applied before game finish.
    scoreOutput.classList.remove('incorrect');
    scoreOutput.classList.remove('correct');
    scoreOutput.innerHTML = '';
}

//End Game
function end() {
    scoreDisplay.innerHTML = "Time's Up! You scored " + score + " points!"
    reset();
}
.correct {
    border:6px solid green;
    border-radius: 50%;
}

.incorrect {
    border:6px solid red;
    border-radius: 50%;
}
2 Answers

Everytime you start a game, you add event listeners to all the squares:

function startGame() {

board.style.pointerEvents = 'all';
target.innerHTML = randomSquare;
gameTime()

document.querySelectorAll('.square').forEach(item => {
    item.addEventListener('click', event => {     //////   <<<<  HERE

The second time you run the game, in the clicked square there are 2 listeners.

The first one runs ok, as expected. But changes the randomSquare value.

The second event will report the failure, because now the clicked square is no longer the randomSquare

When you have run the game 100 times, you have 6400 listeners !!!!

First way: remove any listeners (if exist) before you attach a new one

function onClick(event) {
    const item = event.target;
    if (item.id == randomSquare) {
        console.log("correct", item);
        score++;
        tries++;
        scoreOutput.innerHTML = score;
        randomSquare = rndSq(squareset);
        target.innerHTML = randomSquare;
        scoreOutput.classList.add('correct');
        scoreOutput.classList.remove('incorrect');
    } else {
        console.log("incorrect", item);
        tries++;
        // scoreDisplay.innerHTML = score;
        randomSquare = rndSq(squareset);
        target.innerHTML = randomSquare;
        scoreOutput.classList.remove('correct');
        scoreOutput.classList.add('incorrect');
    };
}
function startGame() {
    console.log("startGame");
    //To make the board active
    board.style.pointerEvents = 'all';
    //First Target
    target.innerHTML = randomSquare;
    //Start Game timer
    gameTime();
    document.querySelectorAll('.square').forEach(item => {
        item.removeEventListener('click', onClick);
        item.addEventListener('click', onClick);
    })
};

Or the second way, attach listeners only one time:

document.querySelectorAll('.square').forEach(item => {
    item.addEventListener('click', onClick);
})
function startGame() {
    console.log("startGame");
    //To make the board active
    board.style.pointerEvents = 'all';
    //First Target
    target.innerHTML = randomSquare;
    //Start Game timer
    gameTime();
};
Related