Simple memory game doesn't update its state. Bad game loop?

Question

I'm trying to make a simple (or so I thought) memory game. Unfortunately it does not update state of cards when user clicks on them. I'm running out of ideas, probably because it's my first javascript game. I suppose there is a problem with game loop. Could anyone at least point me in the right direction and help me understand what needs to be changed/rewritten?

//HTML5 Memory Game implementation

//main variables
var cards = [1,2,3,4,5,6,7,8,1,2,3,4,5,6,7,8];
var exposed = [makeArray("false",16)];
var first_card = 0;
var second_card = 0;
var moves = 0;
var WIDTH = 800;
var HEIGHT = 100;
var state = 0;
var mouseX = 0;
var mouseY = 0;

//creating canvas
var canvas = document.createElement("canvas");
var ctx = canvas.getContext("2d");
canvas.width = WIDTH;
canvas.height = HEIGHT;
document.getElementById("game").appendChild(canvas);

//filling empty array with number,character,object
function makeArray(value, length) {
    var newArray = [];
    var i = 0;
    while (i<length) {
        newArray[i] = value;
        i++;
    }
    return newArray;
}

//shuffling algorithm
function shuffle(array) {
    var copy = [];
    var n = array.length;
    var i;

    while (n) {
        i = Math.floor(Math.random() * n--);
        copy.push(array.splice(i, 1)[0]);
    }

    return copy;
}

//where user clicks
function getClickPosition(event) {
    var X = event.pageX - canvas.offsetLeft;
    var Y = event.pageY - canvas.offsetTop;
    return mouse = [X, Y];
}

//read click position
function readPos(event) {
    mousePos = getClickPosition(event);
    mouseX = mousePos[0];
    mouseY = mousePos[1];
}

//initializing
function init() {
    state = 0;
    moves = 0;
    exposed = [makeArray("false",16)];
    cards = shuffle(cards);
}

//drawing cards
function draw() {
    ctx.clearRect(0, 0, WIDTH, HEIGHT);
    for (var i in cards) {
        if (exposed[i] === true) {
            ctx.fillStyle = "rgb(250, 250, 250)";
            ctx.font = "50px Courier New";
            ctx.fillText(cards[i], (i*50+12), 65);
        } else {
            ctx.strokeStyle = "rgb(250, 0, 0)";
            ctx.fillStyle = "rgb(0, 0, 250)";
            ctx.fillRect(i*50, 0, 50, 100);
            ctx.strokeRect(i*50, 0, 50, 100);
        }
    }
};

//update cards
function update() {
    if (exposed[parseInt(mouseX / 50)] === false) {
        if (state == 0) {
            state = 1;
            first_card = parseInt(mouseX / 50);
            exposed[parseInt(mouseX / 50)] = true;
        } else if (state == 1) {
            state = 2;
            second_card = parseInt(mouseX / 50);
            exposed[parseInt(mouseX / 50)] = true;
        } else {
            if (cards[first_card] != cards[second_card]) {
                exposed[first_card] = false;
                exposed[second_card] = false;
            }
            state = 1;
            first_card = parseInt(mouseX / 50);
            exposed[parseInt(mouseX / 50)] = true;
        }
    }
}

addEventListener('click', readPos, false);

setInterval(function() {
    update();
    draw();
}, 16);

Answer 1

I would check your addEventListener method: https://developer.mozilla.org/en-US/docs/Web/API/EventTarget.addEventListener I also recommend you look into using jQuery.

Answer 2

After copy and pasting your code I found a couple of things:

1. You didn't add an event listener to anything, you should add it to something so I added it to document.
2. You initialize the exposed array with values "false" and later check if they are false. These are not the same, the string "false" isn't the Boolean false.
3. You initializes the exposed array as a multi dimensional array [[false,false,false ...]] this should be a single dimension array because later you check exposed[1] (1 depending on the mouse x position.
4. No need to call draw and update every 16 milliseconds, you can call it after someone clicked.
5. Wrapped the whole thing up in a function so there are no global variables created.

Here is the code after changing these obvious errors. There might be room for optimization but for now I've gotten the problems out.

<!DOCTYPE html>
<html>
 <head>
<title>test</title>
</head> 
 <body> 
<div id="game"></div>
 <script type="text/javascript">

     (function(){
         //HTML5 Memory Game implementation

         //main variables
         var cards = [1, 2, 3, 4, 5, 6, 7, 8, 1, 2, 3, 4, 5, 6, 7, 8];
         var exposed = makeArray(false, 16);
         var first_card = 0;
         var second_card = 0;
         var moves = 0;
         var WIDTH = 800;
         var HEIGHT = 100;
         var state = 0;
         var mouseX = 0;
         var mouseY = 0;

         //creating canvas
         var canvas = document.createElement("canvas");
         var ctx = canvas.getContext("2d");
         canvas.width = WIDTH;
         canvas.height = HEIGHT;
         document.getElementById("game").appendChild(canvas);

         //filling empty array with number,character,object
         function makeArray(value, length) {
             var newArray = [];
             var i = 0;
             while (i < length) {
                 newArray.push(value);
                 i++;
             }
             return newArray;
         }

         //shuffling algorithm
         function shuffle(array) {
             var copy = [];
             var n = array.length;
             var i;

             while (n) {
                 i = Math.floor(Math.random() * n--);
                 copy.push(array.splice(i, 1)[0]);
             }

             return copy;
         }

         //where user clicks
         function getClickPosition(event) {
             var X = event.pageX - canvas.offsetLeft;
             var Y = event.pageY - canvas.offsetTop;
             return mouse = [X, Y];
         }

         //read click position
         function readPos(event) {
             mousePos = getClickPosition(event);
             mouseX = mousePos[0];
             mouseY = mousePos[1];
             update();
             draw();
         }

         //initializing
         function init() {
             state = 0;
             moves = 0;
             exposed = makeArray(false, 16);
             cards = shuffle(cards);
         }

         //drawing cards
         function draw() {
             ctx.clearRect(0, 0, WIDTH, HEIGHT);
             for (var i in cards) {
                 if (exposed[i] === true) {
                     ctx.fillStyle = "rgb(150, 150, 150)";
                     ctx.font = "50px Courier New";
                     ctx.fillText(cards[i], (i * 50 + 12), 65);
                 } else {
                     ctx.strokeStyle = "rgb(250, 0, 0)";
                     ctx.fillStyle = "rgb(0, 0, 250)";
                     ctx.fillRect(i * 50, 0, 50, 100);
                     ctx.strokeRect(i * 50, 0, 50, 100);
                 }
             }
         };
         //update cards
         function update() {
             if (exposed[parseInt(mouseX / 50)] === false) {
                 if (state == 0) {
                     state = 1;
                     first_card = parseInt(mouseX / 50);
                     exposed[parseInt(mouseX / 50)] = true;
                 } else if (state == 1) {
                     state = 2;
                     second_card = parseInt(mouseX / 50);
                     exposed[parseInt(mouseX / 50)] = true;
                 } else {
                     if (cards[first_card] != cards[second_card]) {
                         exposed[first_card] = false;
                         exposed[second_card] = false;
                     }
                     state = 1;
                     first_card = parseInt(mouseX / 50);
                     exposed[parseInt(mouseX / 50)] = true;
                 }
             }
         }
         document.body.addEventListener('click', readPos, false);
         init();
         draw();
     })();
 </script>
 </body>
</html>

Answer 3

Your overall logic was good.
The point that was 'bad' was the way you handle the event : the event handler should store some valuable information that the update will later process and clear.
Here you mix your update with event handling, which cannot work especially since the event will not fire on every update.

So i did a little fiddle to show you, the main change is the click event handler, which update the var last_clicked_card :

http://jsfiddle.net/wpymH/

//read click position
function readPos(event) {
    last_clicked_card = -1;
    mousePos = getClickPosition(event);
    mouseX = mousePos[0];
    mouseY = mousePos[1];
    //  on canvas ?
    if ((mouseY>100)||(mouseX<0)||(mouseX>WIDTH)) return;
    // now yes : which card clicked ?
    last_clicked_card = Math.floor(mouseX/50);
 }

and then update is the processing of this information :

//update cards
function update() {
    // return if no new card clicked
    if (last_clicked_card == -1) return;
    // read and clear last card clicked
    var newCard = last_clicked_card;
    last_clicked_card=-1;
    // flip, store it as first card and return 
    // if there was no card flipped
    if (state==0) {  exposed[newCard] = true;
                     first_card = newCard; 
                     state = 1 ;
                     return;        }
    // just unflip card if card was flipped
    if ((state = 1) && exposed[newCard]) {     
                     exposed[newCard]=false ;
                     state=0;
                     return;
    }
    // we have a second card now
    second_card = newCard;
    exposed[second_card] =  true;
    draw();
    // ... i don't know what you want to do ...
    if (cards[first_card] == cards[second_card]) { 
         alert('win'); } 
    else {
         alert('loose'); }

    exposed[first_card]=false;
    exposed[second_card]=false;
    state=0;

}