7
\$\begingroup\$

I'm working on a small isometric drawing system in JavaScript. After much help from the Gamedev section, reading old questions, I've got something working. However, it's still a little hacky. I need to fix the culling to be more straightforward and it still isn't quite getting edge tiles down correctly.

I want to improve the code to ensure that:

  1. The "center" tile is actually centered in the drawing area.
  2. The are always enough edge tiles.
  3. The code is straight-forward

Link to live Codepen.

Here is the camera drawing function:

function cameraDraw(position)
{
  var cen = {
        a: position.x + position.y,
        b: position.x - position.y
      },
      max = { //Use testSize instead of canvas size so we can see overdraw
        a: Math.floor(testSize.y/(terrainImg.width/2))+1, //Adding +1 to each for better edging?? Is this normal?
        b: Math.floor(testSize.x/terrainImg.width)+1
      },
      startPos = {
        x: (((cen.a - max.a) + (cen.b - max.b))/2),
        y: (((cen.a - max.a) - (cen.b - max.b))/2)
      },
      offset = { //Subtract 100 to show overdraw
        x: (startPos.x - startPos.y) * (terrainImg.width/2) + terrainImg.width/2 + terrainImg.width/4 - 100, //+terrainImg widths for better placement
        y: (startPos.x + startPos.y) * (terrainImg.width/4) + terrainImg.width/4 - 100
      };

  for(var a = cen.a - max.a; a < cen.a + max.a; a++)
  {
    for(var b = cen.b - max.b; b < cen.b + max.b+1/*I add +1 here to take care of edge issue on the bottom...Should I have to do that, if I'd done it right, above?*/; b++)
    {
      if ((b&1) != (a&1))
        continue; 

      var x = (a+b)/2,
          y = (a-b)/2;

      world[x][y].draw(offset, position);
    }
  }
}

And here is the tile .draw function:

this.draw = function(offset, p)
{
  var drawPos = {
    x: (X-Y)*(terrainImg.width/2) - offset.x,
    y: (X+Y)*(terrainImg.width/4) - offset.y
  };

  if(X == p.x && Y == p.y)
    drawPos.y -= 15;

  ctx.drawImage(terrainImg, drawPos.x, drawPos.y);
};
\$\endgroup\$
5
\$\begingroup\$

This code is pretty well written, although there are a few things that really bug me a little.

First off, inline comments, like the one you've positioned inside that for loop, are eyesores to look at. Preferably, comments should stay on their own separate lines.

Secondly, your naming could be improved. For example, what does the variable cen do? It's name doesn't give any hint as to what it does, nor do it's contained items, a and b. Variable names should be descriptive, and describe the purpose of a variable, all while not being too long, or too short.

Finally, use braces around your if statements. Have you ever heard of the "goto fail" bug? That's what happened in a bug with Apple SSL. The error, in question, looked something like this:

if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
    goto fail;
    goto fail;  /* MISTAKE! THIS LINE SHOULD NOT BE HERE */

Just use braces. They'll prevent all sorts of mistakes from occurring, and they'll make things look clearer overall as well.

\$\endgroup\$
2
\$\begingroup\$

Your code is written well, however parts could be improved:

  1. Instead of having the comma on the same line as the variable it proceeds, put it on the next line:
var cen = {
    a: position.x + position.y,
    b: position.x - position.y
  },

into:

var cen = {
      a: position.x + position.y
    , b: position.x - position.y
  },

That way you can comment out the line easier.

  1. You ought to put whitespace between your operators, for readability purposes.
Math.floor(testSize.y/(terrainImg.width/2))+1
  1. Please don't put comments inside your for statement, the line before or after is the place for those:
for(var b = cen.b - max.b; b < cen.b + max.b+1/*I add +1 here to take care of edge issue on the bottom...Should I have to do that, if I'd done it right, above?*/; b++)
  1. Make sure you include brackets in your if statements, because as @EthanBierlein pointed out, funny business (serious bugs) can happen if you mess them up.
if ((b&1) != (a&1))
    continue; 

Your code could use more explaining as to what everything does, whether line-by-line explanation or per function.
The comma thing and whitespace is probably the thing I can see the most that could be improved.
Other than that, your code looks really nice and tidy, good work!

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.