CODEWITHSUNDEEP
javascriptjquerysettimeout
oliver_siegel
oliver_siegel

Reputation: 1676

Why does this setTimeout(); function not work like I think it should?

I want to make a matching card kind of game, and reveal 2 cards at a time after clicking on them. if they are the same, they'll become inactive, if they are not the same, they'll just be covered again.

Here is an implementation of this where the cards are uncovered 1.5 seconds after the click

$(document).on("click", "div.card.covered", function(){    
    if( $('div.card.uncovered').length < 2 ){
        uncover_card(this);
    }       
    if( $('div.card.uncovered').length == 2 ){
        var uncovered_cards = new Array();          
        $('div.card.uncovered').each(function(){
            uncovered_cards.push( $(this).attr("class") );
        });

        if( uncovered_cards[0] == uncovered_cards[1]){
            setTimeout(function(){
                $('div.card.uncovered').addClass("matched");
                $('div.card.uncovered').removeClass("uncovered");
                //uncover_card(this);
            },1000);                
        }else{
            setTimeout(function(){
                $('div.card.uncovered').addClass("covered");
                $('div.card.uncovered').removeClass("uncovered");
                //uncover_card(this);
            },1000);
        }
    }
});

http://jsfiddle.net/Lf8cE/2/

if you do this SLOWLY it will work as expected.

however if you click on 2, and then click on a third one like crazy while waiting for them to become inactive or cover up again, this third card will shortly after the pair gets changed, also be changed to the same state as the pair.

Why does this bug happen?!

Upvotes: 1

Views: 83

Answers (3)

James Montagne
James Montagne

Reputation: 78650

The issue, as alluded to by @KevinB in the comments is that each time you click you are adding an additional timeout. When the first timeout fires, it removes the uncovered class from the two cards. Now, because you are clicking quickly, you uncover another card. However, your 2nd (or 3rd, 4th etc.) timeout then fires and acts upon any uncovered cards, thereby affecting this newly uncovered card.

The easiest fix would be to move your 2nd if inside of the first so that it only fires once.

if( $('div.card.uncovered').length < 2 )
{
    uncover_card(this);

    if( $('div.card.uncovered').length == 2 )
    {
        var uncovered_cards = new Array();

        $('div.card.uncovered').each(function(){
            uncovered_cards.push( $(this).attr("class") );
        });

        if( uncovered_cards[0] == uncovered_cards[1])
        {
            setTimeout(function(){
                $('div.card.uncovered').addClass("matched");
                $('div.card.uncovered').removeClass("uncovered");
                //uncover_card(this);
            },1000);

        }
        else
        {
            setTimeout(function(){
                $('div.card.uncovered').addClass("covered");
                $('div.card.uncovered').removeClass("uncovered");
                //uncover_card(this);
            },1000);
        }
    }
}

http://jsfiddle.net/Lf8cE/4/

Upvotes: 1

Ryan H Lewis
Ryan H Lewis

Reputation: 624

It's happening because you are adding the class "matched" to every card that matches the div.card.uncovered selector. This would include any card clicked in the meantime. If you cached your first selection of "uncovered" elements, then you could just operate on them once the matching works out. Here's a working snippet:

    var uncovered_cards = [];
    var els = $('div.card.uncovered');
    els.each(function(){
        uncovered_cards.push( $(this).attr("class") );
    });

    if( uncovered_cards[0] == uncovered_cards[1]){
        setTimeout(function(){
            els.addClass("matched");
            els.removeClass("uncovered");
            //uncover_card(this);
        },1000);            
    }

Upvotes: 0

Luke Willis
Luke Willis

Reputation: 8580

The issue here is with the setTimeout function.

Basically, at the end of the timeout interval, you are looking for all cards with the uncovered class and either covering them or matching them. By clicking on the third card "like crazy" you are managing to set the third card as uncovered before setTimeout is able to query for $('div.card.uncovered'), and thus it is matching/covering all three of the uncovered cards.

The solution is to match/cover the two cards by reference instead of querying for them again at the end of the timeout.

Something like this:

setTimeout(function(){
    uncovered_cards[0].addClass("matched");
    uncovered_cards[0].removeClass("uncovered");

    uncovered_cards[1].addClass("matched");
    uncovered_cards[1].removeClass("uncovered");
},1000);

Please Note: I have not tested the above code, it may not work as is. It only serves to demonstrate the concept.

Upvotes: 0

Related Questions