views:

169

answers:

2

I'm creating a building game in JavaScript and PHP that involves a grid. Each square in the grid is a div, with an own onmouseover and onmousedown function:

for(x=0; x < width; x++)
{
    for(y=0; y < height; y++)
    {
        var div = document.createElement("div");
        //...
        div.onmouseclick = function() {blockClick(x, y)}
        div.onmouseover = function() {blockMouseover(x, y)}
        game.appendChild(div);
    }
}

But, all of the squares seem to have the x and y of the last square that was added. I can sort of see why this is happening - it is making a pointer to x and y instead of cloning the variables - but how could I fix it? I even tried

for(x=0; x < width; x++)
{
    for(y=0; y < height; y++)
    {
        var div = document.createElement("div");
        var myX = x;
        var myY = y;
        div.onmouseclick = function() {blockClick(myX, myY)}
        div.onmouseover = function() {blockMouseover(myX, myY)}
        game.appendChild(div);
    }
}

with the same result.

I was using div.setAttribute("onmouseover", ...) which worked in Firefox, but not IE. Thanks!

A: 

That is because you are actually sharing the same variables across each handler because of the surrounding closure. So you need to create local closures like this:

(function(x,y){
    div.onmouseclick = function() {blockClick(x, y)};
    div.onmouseover = function() {blockMouseover(x, y)};
})(x, y);
Sean Kinsey
+1  A: 

You need to create a closure to retain the value of x and y. This should do the trick :

for(var x=0; x < width; x++) {
  for(var y=0; y < height; y++) {
    var div = document.createElement("div");
    (function(x,y){
       div.onmouseclick = function() {blockClick(x, y)}
       div.onmouseover = function() {blockMouseover(x, y)}
    })(x,y);
    game.appendChild(div);
  }
}

A cleaner way of doing this is to define a function to create the div and then call it at each iteration :

var createDiv = function(x,y){
  var div = document.createElement("div");
  div.onmouseclick = function() {blockClick(x, y)};
  div.onmouseover = function() {blockMouseover(x, y)};
  return div;
}

for(var x=0; x < width; x++) {
  for(var y=0; y < height; y++) {
    game.appendChild(createDiv(x,y));
  }
}
Adrien Friggeri
Second one works great, thanks!
phpscriptcoder