Skip to main content
replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

Before making this more efficient I'd suggest making it more readable:

in your switch statement you have break in both the true and else parts of your if statement.

if(checkmove(pos.x, pos.y-2)) {
    pos.y -= 2;
    break;
} else {
    break;
} 

It would be much easier to read if you just move the break out of the if statement.

if(checkmove(pos.x, pos.y-2)) {
    pos.y -= 2;
}
break;

In another questionanother question I had suggested a bit shaving optimisation that might help here. Math.floor(number) is equivalent to number >> 0 this bit shifts the number by 0 bits and converts it to an integer.

Math.floor(x/20)

becomes

(x / 20) >> 0

similarly adding almost 1 (+ 1 - Number.EPSILON) and then flooring will give you the ceiling. (number + 1 - Number.EPSILON) >> 0

The checkmove() function calculates this multiple times on the one line. I'd rewrite it to calculate once.

function checkmove(x, y) {
  var floorX = (x/20) >> 0;
  var floorY = (y/20) >> 0;
  var ceilX = ((x/20) + 1 - Number.EPSILON) >> 0;
  var ceilY = ((y/20) + 1 - Number.EPSILON) >> 0;

  return level[floorX][floorY] == 1 ||
      level[ceilX][floorY] == 1 ||
      level[floorX][ceilY] == 1 ||
      level[ceilX][ceilY] == 1;
}

Before making this more efficient I'd suggest making it more readable:

in your switch statement you have break in both the true and else parts of your if statement.

if(checkmove(pos.x, pos.y-2)) {
    pos.y -= 2;
    break;
} else {
    break;
} 

It would be much easier to read if you just move the break out of the if statement.

if(checkmove(pos.x, pos.y-2)) {
    pos.y -= 2;
}
break;

In another question I had suggested a bit shaving optimisation that might help here. Math.floor(number) is equivalent to number >> 0 this bit shifts the number by 0 bits and converts it to an integer.

Math.floor(x/20)

becomes

(x / 20) >> 0

similarly adding almost 1 (+ 1 - Number.EPSILON) and then flooring will give you the ceiling. (number + 1 - Number.EPSILON) >> 0

The checkmove() function calculates this multiple times on the one line. I'd rewrite it to calculate once.

function checkmove(x, y) {
  var floorX = (x/20) >> 0;
  var floorY = (y/20) >> 0;
  var ceilX = ((x/20) + 1 - Number.EPSILON) >> 0;
  var ceilY = ((y/20) + 1 - Number.EPSILON) >> 0;

  return level[floorX][floorY] == 1 ||
      level[ceilX][floorY] == 1 ||
      level[floorX][ceilY] == 1 ||
      level[ceilX][ceilY] == 1;
}

Before making this more efficient I'd suggest making it more readable:

in your switch statement you have break in both the true and else parts of your if statement.

if(checkmove(pos.x, pos.y-2)) {
    pos.y -= 2;
    break;
} else {
    break;
} 

It would be much easier to read if you just move the break out of the if statement.

if(checkmove(pos.x, pos.y-2)) {
    pos.y -= 2;
}
break;

In another question I had suggested a bit shaving optimisation that might help here. Math.floor(number) is equivalent to number >> 0 this bit shifts the number by 0 bits and converts it to an integer.

Math.floor(x/20)

becomes

(x / 20) >> 0

similarly adding almost 1 (+ 1 - Number.EPSILON) and then flooring will give you the ceiling. (number + 1 - Number.EPSILON) >> 0

The checkmove() function calculates this multiple times on the one line. I'd rewrite it to calculate once.

function checkmove(x, y) {
  var floorX = (x/20) >> 0;
  var floorY = (y/20) >> 0;
  var ceilX = ((x/20) + 1 - Number.EPSILON) >> 0;
  var ceilY = ((y/20) + 1 - Number.EPSILON) >> 0;

  return level[floorX][floorY] == 1 ||
      level[ceilX][floorY] == 1 ||
      level[floorX][ceilY] == 1 ||
      level[ceilX][ceilY] == 1;
}
fixed ceiling using rounding instead of ceiling trick
Source Link
James Khoury
  • 3.2k
  • 1
  • 25
  • 52

Before making this more efficient I'd suggest making it more readable:

in your switch statement you have break in both the true and else parts of your if statement.

if(checkmove(pos.x, pos.y-2)) {
    pos.y -= 2;
    break;
} else {
    break;
} 

It would be much easier to read if you just move the break out of the if statement.

if(checkmove(pos.x, pos.y-2)) {
    pos.y -= 2;
}
break;

In another question I had suggested a bit shaving optimisation that might help here. Math.floor(number) is equivalent to number >> 0 this bit shifts the number by 0 bits and converts it to an integer.

Math.floor(x/20)

becomes

(x / 20) >> 0

similarly adding 0.5almost 1 (+ 1 - Number.EPSILON) and then flooring will give you the ceiling. (number + 01 - Number.5EPSILON) >> 0

The checkmove() function calculates this multiple times on the one line. I'd rewrite it to calculate once.

function checkmove(x, y) {
  var floorX = (x/20) >> 0;
  var floorY = (y/20) >> 0;
  var ceilX = ((x/20) + 01 - Number.5EPSILON) >> 0;
  var ceilY = ((y/20) + 01 - Number.5EPSILON) >> 0;

  return level[floorX][floorY] == 1 ||
      level[ceilX][floorY] == 1 ||
      level[floorX][ceilY] == 1 ||
      level[ceilX][ceilY] == 1;
}

Before making this more efficient I'd suggest making it more readable:

in your switch statement you have break in both the true and else parts of your if statement.

if(checkmove(pos.x, pos.y-2)) {
    pos.y -= 2;
    break;
} else {
    break;
} 

It would be much easier to read if you just move the break out of the if statement.

if(checkmove(pos.x, pos.y-2)) {
    pos.y -= 2;
}
break;

In another question I had suggested a bit shaving optimisation that might help here. Math.floor(number) is equivalent to number >> 0 this bit shifts the number by 0 bits and converts it to an integer.

Math.floor(x/20)

becomes

(x / 20) >> 0

similarly adding 0.5 and then flooring will give you the ceiling. (number + 0.5) >> 0

The checkmove() function calculates this multiple times on the one line. I'd rewrite it to calculate once.

function checkmove(x, y) {
  var floorX = (x/20) >> 0;
  var floorY = (y/20) >> 0;
  var ceilX = ((x/20) + 0.5) >> 0;
  var ceilY = ((y/20) + 0.5) >> 0;

  return level[floorX][floorY] == 1 ||
      level[ceilX][floorY] == 1 ||
      level[floorX][ceilY] == 1 ||
      level[ceilX][ceilY] == 1;
}

Before making this more efficient I'd suggest making it more readable:

in your switch statement you have break in both the true and else parts of your if statement.

if(checkmove(pos.x, pos.y-2)) {
    pos.y -= 2;
    break;
} else {
    break;
} 

It would be much easier to read if you just move the break out of the if statement.

if(checkmove(pos.x, pos.y-2)) {
    pos.y -= 2;
}
break;

In another question I had suggested a bit shaving optimisation that might help here. Math.floor(number) is equivalent to number >> 0 this bit shifts the number by 0 bits and converts it to an integer.

Math.floor(x/20)

becomes

(x / 20) >> 0

similarly adding almost 1 (+ 1 - Number.EPSILON) and then flooring will give you the ceiling. (number + 1 - Number.EPSILON) >> 0

The checkmove() function calculates this multiple times on the one line. I'd rewrite it to calculate once.

function checkmove(x, y) {
  var floorX = (x/20) >> 0;
  var floorY = (y/20) >> 0;
  var ceilX = ((x/20) + 1 - Number.EPSILON) >> 0;
  var ceilY = ((y/20) + 1 - Number.EPSILON) >> 0;

  return level[floorX][floorY] == 1 ||
      level[ceilX][floorY] == 1 ||
      level[floorX][ceilY] == 1 ||
      level[ceilX][ceilY] == 1;
}
fixed spelling mistake ciel => ceil
Source Link
James Khoury
  • 3.2k
  • 1
  • 25
  • 52

Before making this more efficient I'd suggest making it more readable:

in your switch statement you have break in both the true and else parts of your if statement.

if(checkmove(pos.x, pos.y-2)) {
    pos.y -= 2;
    break;
} else {
    break;
} 

It would be much easier to read if you just move the break out of the if statement.

if(checkmove(pos.x, pos.y-2)) {
    pos.y -= 2;
}
break;

In another question I had suggested a bit shaving optimisation that might help here. Math.floor(number) is equivalent to number >> 0 this bit shifts the number by 0 bits and converts it to an integer.

Math.floor(x/20)

becomes

(x / 20) >> 0

similarly adding 0.5 and then flooring will give you the ceiling. (number + 0.5) >> 0

The checkmove() function calculates this multiple times on the one line. I'd rewrite it to calculate once.

function checkmove(x, y) {
  var floorX = (x/20) >> 0;
  var floorY = (y/20) >> 0;
  var cielXceilX = ((x/20) + 0.5) >> 0;
  var cielYceilY = ((y/20) + 0.5) >> 0;

  return level[floorX][floorY] == 1 ||
      level[cielX][floorY]level[ceilX][floorY] == 1 ||
      level[floorX][cielY]level[floorX][ceilY] == 1 ||
      level[cielX][cielY]level[ceilX][ceilY] == 1;
}

Before making this more efficient I'd suggest making it more readable:

in your switch statement you have break in both the true and else parts of your if statement.

if(checkmove(pos.x, pos.y-2)) {
    pos.y -= 2;
    break;
} else {
    break;
} 

It would be much easier to read if you just move the break out of the if statement.

if(checkmove(pos.x, pos.y-2)) {
    pos.y -= 2;
}
break;

In another question I had suggested a bit shaving optimisation that might help here. Math.floor(number) is equivalent to number >> 0 this bit shifts the number by 0 bits and converts it to an integer.

Math.floor(x/20)

becomes

(x / 20) >> 0

similarly adding 0.5 and then flooring will give you the ceiling. (number + 0.5) >> 0

The checkmove() function calculates this multiple times on the one line. I'd rewrite it to calculate once.

function checkmove(x, y) {
  var floorX = (x/20) >> 0;
  var floorY = (y/20) >> 0;
  var cielX = ((x/20) + 0.5) >> 0;
  var cielY = ((y/20) + 0.5) >> 0;

  return level[floorX][floorY] == 1 ||
      level[cielX][floorY] == 1 ||
      level[floorX][cielY] == 1 ||
      level[cielX][cielY] == 1;
}

Before making this more efficient I'd suggest making it more readable:

in your switch statement you have break in both the true and else parts of your if statement.

if(checkmove(pos.x, pos.y-2)) {
    pos.y -= 2;
    break;
} else {
    break;
} 

It would be much easier to read if you just move the break out of the if statement.

if(checkmove(pos.x, pos.y-2)) {
    pos.y -= 2;
}
break;

In another question I had suggested a bit shaving optimisation that might help here. Math.floor(number) is equivalent to number >> 0 this bit shifts the number by 0 bits and converts it to an integer.

Math.floor(x/20)

becomes

(x / 20) >> 0

similarly adding 0.5 and then flooring will give you the ceiling. (number + 0.5) >> 0

The checkmove() function calculates this multiple times on the one line. I'd rewrite it to calculate once.

function checkmove(x, y) {
  var floorX = (x/20) >> 0;
  var floorY = (y/20) >> 0;
  var ceilX = ((x/20) + 0.5) >> 0;
  var ceilY = ((y/20) + 0.5) >> 0;

  return level[floorX][floorY] == 1 ||
      level[ceilX][floorY] == 1 ||
      level[floorX][ceilY] == 1 ||
      level[ceilX][ceilY] == 1;
}
Source Link
James Khoury
  • 3.2k
  • 1
  • 25
  • 52
Loading