CODEWITHSUNDEEP
javascriptjquery
user3348360
user3348360

Reputation: 27

Line in loop not executing

var games = [];
var numbers_to_letters = ["one", "two", "three", "four", "five", "six"]

function Game(){
    roll_animation();
}

$("#roll_dice").click(function(){
    games.push(new Game());
});

function roll_animation(){
    for (var i = 0; i < 100; i++){
        for (var j = 0; j < 5; j++){
            (function(ind){
                window.setTimeout(function(){
                    $(".btn.btn-die").addClass(numbers_to_letters[ind+1]).removeClass(numbers_to_letters[ind]);
                }, 20 * (ind + 1));
            })(j);
        }
        $(".btn.btn-die").addClass("one").removeClass("six");
    }
}

The last line in the first loop doesn't get executed unless I press the button again, resulting in the second loop only running once. (it needs a reset to work again) Why doesn't the last line always get executed?

Upvotes: 0

Views: 87

Answers (1)

jfriend00
jfriend00

Reputation: 707218

This appears to be an issue of asynchronous code in the setTimeout() function running some time later and thus not synchronized with the other code in the loops.

The line of code in question:

$(".btn.btn-die").addClass("one").removeClass("six");

is running just fine. But, because the setTimeout() calls are scheduled for execution some timer LATER, they all run after this above line runs. So this line of code runs and then the setTimeout() timers start firing and the effect of the above line is overwritten. This is what your code has instructed it to do.

If you slow the times down, you can see what is happening here in this demo of your codde: http://jsfiddle.net/jfriend00/tqYq7/

If, what you want is for the above line of code to run AFTER the last timer fires, then you need to write your code to do that instead of the way you have it written now. It's unclear to me what you're trying to do with this line of code anyway so I am not sure exactly what to recommend.

One might ask why you're setting 500 timers too, most of which are going to go off at the exact same time and run the exact same code? There are certainly more efficient ways to run something in a recurring fashion. If what you're really trying to do is to run the inner animation 100 times, one after the other, then you will have to code completely differently because what you're doing now is setting 100 copies of the same timers all scheduled to go off at the same time.

Here's a demo for a different way to program it that doesn't have the issues yours does.

Working demo: http://jsfiddle.net/jfriend00/Zsa8m/

var games = [];
var numbers_to_letters = ["one", "two", "three", "four", "five", "six"];

function Game(){
    roll_animation();
}

$("#roll_dice").click(function(){
    games.push(new Game());
});

function roll_animation(){
    var cntr = 0;

    function next() {
        var die = cntr % 6;
        var nextDie = (die + 1) % 6;
        $(".btn.btn-die")
            .addClass(numbers_to_letters[nextDie])
            .removeClass(numbers_to_letters[die]);
        ++cntr;
        // keep going as long as cntr < 100 and we aren't on number 1
        // so that the die always end on 1
        if (cntr < 100 || nextDie != 0) {
            setTimeout(next, 50);
        }
    }
    next();
}

You can obviously change the constants to suit your purposes.

Just for grins, here's a version that rolls randomly rather than sequentially: http://jsfiddle.net/jfriend00/NSW8U/

Upvotes: 1

Related Questions