3

I'm building a simon game. And after each round the player should see the moves he must play in the next round. So i created a function showMoves which flashes the square he has to play. The problem is that the function is not showing anything. Can anyone tell me what did i miss?

// the effect
function flasher(index) {
     $(moves[index]).fadeIn(50).fadeOut(50).fadeIn(50).fadeOut(50).fadeIn(100);
}
var interval2;
// show the moves that supposed to be played
function showMoves() {
    for (var i = 0; i < moves; i++) {
        if (i === 0) {
            interval2 = setTimeout(flasher(i), 1000);
        } else {
            interval2 = setTimeout(flasher(i), (i+1) * 1000);
        }
     }
 }
kumardeepakr3
  • 395
  • 6
  • 16
Hesham
  • 31
  • 1
  • 1
    you should pass function reference.`setTimeout(function(){flasher(i);},1000);` – Madhawa Priyashantha Nov 27 '16 at 10:11
  • 1
    @FastSnail, you'll still have [this situation](http://stackoverflow.com/questions/1451009/javascript-infamous-loop-issue), `i` variable will be the same in all calls to `flasher` – Max Koretskyi Nov 27 '16 at 10:19
  • 1
    `1000 === (0 + 1) * 1000`; Beyond the larger issue at hand, these branches are redundant. – Oka Nov 27 '16 at 11:31

3 Answers3

1

setTimeout accepts a function as a first parameter. I assume that by calling flasher you tried to avoid this situation. In you case, this should be done like this:

  function showMoves() {
    for (var i = 0; i < moves; i++) {
      if (i === 0) {
        interval2 = setTimeout(function(i) {return function() {flasher(i)}}(i), 1000);
      } else {
        interval2 = setTimeout(function(i) {return function() {flasher(i)}}(i), (i+1) * 1000);
      }
    }
  }
Community
  • 1
  • 1
Max Koretskyi
  • 101,079
  • 60
  • 333
  • 488
0

The setTimeout and setInterval are a little diffrent than we think about them. They are save some event on specified times that will be fired in its times. Because of this they has a problem with loops:

for(i=0;i<3;i++)
{
setTimeout(function(){alert(i)}, i*1000);
}

after ending the loop the browser has 3 jobs to do:

alert(i) after 1 second
alert(i) after 2 seconds
alert(i) after 3 seconds

But what is the value of 'i'. If you are in c# programming after ending the loop 'i' will be disposed and we have not that. But javascript does not dispose 'i' and we have it yet. So the browser set the current value for i that is 3. because when 'i' reaches to 3 loop goes end. Therefor Your browser do this:

alert(3) after 1 second
 alert(3) after 2 seconds
 alert(3) after 3 seconds

That is not what we want. But if change the above code to this:

for(i=0;i<3;i++){
      (function (index)
            {
                setTimeout(function () { alert(index); }, i * 1000);
            })(i);
}

We will have:

alert(0) after 1 second
alert(1) after 2 seconds
alert(2) after 3 seconds

So as Maximus said you mast make the browser to get value of i currently in loop. in this way:

setTimeout(function(i) {return function() {flasher(i)}}(i), (i+1) * 1000);

i does not leave out until end of loop and must be get value just now.

Farzin Kanzi
  • 3,380
  • 2
  • 21
  • 23
0

What I can derive from your code is that moves is an array, but you're using it as if it's an integer in the for loop. And that's why nothing happens at all.

Replace:

for (var i = 0; i < moves; i++) {

With:

for (var i = 0; i < moves.length; i++) {

And you should see things happening.

But you will notice flasher is called immediately, without timeout. And that's because the result of flasher is set to be called, instead of flasher itself.
Other answers here suggest using an wrapper function, but this requires workarounds to correctly pass the index to the function called by setTimeout. So assuming that it doesn't have to run in IE8 and below, the following is the most concise solution:

setTimeout(flasher.bind(null, i), (i+1) * 1000)

Full working example:

var moves = [1, 2, 3, 4];

function flasher(index) {
    console.log('move', moves[index]); 
}
var interval2;
// show the moves that supposed to be played
function showMoves() {
    for (var i = 0; i < moves.length; i++) {
        interval2 = setTimeout(flasher.bind(null, i), (i+1) * 1000);
    }
 }
 
 showMoves()
Koen.
  • 25,449
  • 7
  • 83
  • 78