Meika
Meika

Reputation: 11

Function firing multiple times

could someone help me out with this piece of Javascript?

I am trying to make some sort of "whack-a-mole" game, and this is what I came up with; I set up a way to keep track of the score by adding 1 (score++) every time the user clicks on the picture that pops up. My problem is that the code runs the function more times than needed—for example, if I click on the first image that pops up, the function to add +1 to the score fires once, if I click on the second, the function fires twice, threee times on the third, etc...

What am I doing wrong?

//gid
const grid = document.querySelector('.grid');
//score display value
const scoreValue = document.querySelector('#scoreValue');
//score
let score = 0;

const timer = setInterval(() => {

    //output random number
    let output = Math.floor(Math.random() * 16);
    //select hole
    let hole = document.getElementById(output);

    hole.innerHTML = '<img src="img/kiseki.png" alt=""></img>';
    
    setTimeout(() => {
        hole.innerHTML = '';
    }, 2000);

    grid.addEventListener('click', e => {
        if (e.target.tagName === "IMG") {
            score++;
            scoreValue.textContent = score;
            console.log(score);
            hole.innerHTML = '';
        }
    });
}, 4000);

Upvotes: 0

Views: 841

Answers (3)

Mateus Martins
Mateus Martins

Reputation: 442

Since you're ading a new eventListener every time the interval runs, so in order to solve your problem, just add it once, before starting the setInterval that pops your moles.

Example code:


const grid = document.querySelector('.grid');
const scoreValue = document.querySelector('#scoreValue');
const newMoleTimer = 4000;
const moleTimeout = 2000
let score = 0;
let hole;

grid.addEventListener('click', e => {
  if (e.target.tagName === "IMG") {
    score++;
    scoreValue.textContent = score;
    if(hole) hole.innerHTML = '';
  }
});
    

const timer = setInterval(() => {
    let output = Math.floor(Math.random() * 16);
    hole = document.getElementById(output);

    hole.innerHTML = '<img src="img/kiseki.png" alt=""></img>';
    
    setTimeout(() => {
        hole.innerHTML = '';
    }, moleTimeout);
}, newMoleTimer);

*updated code according to @Meika commentary

Upvotes: 1

Lawrence Cherone
Lawrence Cherone

Reputation: 46660

Rewrite, a mole is a DOM element, attach the click event to it on load then, in the game timer you only need to pick a random mole and toggle a class, within the click event you can check for that class, if it is there then the mole must be showing, add a score.

For example:

const moles = document.querySelectorAll('.grid .mole')
const hitScore = document.querySelector('.score .hit')
const missScore = document.querySelector('.score .miss')
const gameOver = document.querySelector('.gameover')

let score = {
  hit: 0,
  miss: 0
}

// assign clicks to all moles
moles.forEach((elm) => {
  elm.addEventListener('click', e => {
    if (e.target.classList.contains('show')) {
      hitScore.textContent = ++score.hit
      e.target.classList.remove('show')
    }
  })
})

// game timer
const timer = setInterval(() => {

  // get random mole element
  const randMole = moles[Math.floor(Math.random() * moles.length)]

  // check if has class, i.e miss
  if (randMole.classList.contains('show')) {
    missScore.textContent = ++score.miss
  }

  // toggle show
  randMole.classList.toggle('show')

  // 5 misses and game over
  if (score.miss >= 5) {
    clearInterval(timer)
    gameOver.style.display = 'block'
  }
}, 1000)
.grid {
  width: 310px;
  height: 310px;
  background-image: url(https://i.imgur.com/s6lUgud.png);
  position: relative
}

.mole {
  position: absolute;
  width: 100px;
  height: 100px
}

.mole.show {
  background-image: url(https://i.imgur.com/uScpWV4.png);
  background-repeat: no-repeat;
  background-size: 48px 51px;
  background-position: center
}

.mole:nth-of-type(1) {
  top: 0;
  left: 0
}

.mole:nth-of-type(2) {
  top: 0;
  left: 108px
}

.mole:nth-of-type(3) {
  top: 0;
  left: 214px
}

.mole:nth-of-type(4) {
  top: 100px;
  left: 0
}

.mole:nth-of-type(5) {
  top: 100px;
  left: 108px
}

.mole:nth-of-type(6) {
  top: 100px;
  left: 214px
}

.mole:nth-of-type(7) {
  top: 200px;
  left: 0px
}

.mole:nth-of-type(8) {
  top: 200px;
  left: 107px
}

.mole:nth-of-type(9) {
  top: 200px;
  left: 214px
}

.gameover {
  display: none;
  color: red
}
<div class="score">
  <strong>Score:</strong> Hit:
  <span class="hit">0</span> Miss:
  <span class="miss">0</span>
</div>
<div class="gameover">Game Over</div>
<div class="grid">
  <div class="mole"></div>
  <div class="mole"></div>
  <div class="mole"></div>
  <div class="mole"></div>
  <div class="mole"></div>
  <div class="mole"></div>
  <div class="mole"></div>
  <div class="mole"></div>
  <div class="mole"></div>
</div>

Upvotes: 0

chrwahl
chrwahl

Reputation: 13161

You need to separate the eventlistener from the settimer function.

In this example I created div elements with a color. Only blue color score and can only score one point pr. timer.

//gid
const grid = document.querySelector('#grid');
//score display value
const scoreValue = document.querySelector('#scoreValue');
//score
let score = 0;

grid.addEventListener('click', e => {
  if (e.target.score) {
    score++;
    scoreValue.textContent = score;
    e.target.score = false;
  }
});

const timer = setInterval(() => {

  //output random number
  let output = 1 + Math.floor(Math.random() * 3);
  //select hole

  let hole = document.querySelector(`div.box:nth-child(${output})`)
  hole.classList.add('blue');
  hole.score = true;

  setTimeout(() => {
    hole.classList.remove('blue');
    hole.score = false;
  }, 1000);


}, 2000);
div#grid {
display: flex;
}
div.box {
  width: 100px;
  height: 100px;
  border: thin solid black;
  background-color: red;
}

div.blue {
  background-color: blue;
}
<div id="grid">
  <div class="box"></div>
  <div class="box"></div>
  <div class="box"></div>
  <div class="box"></div>
</div>
<div id="scoreValue"></div>

Upvotes: 0

Related Questions