10
\$\begingroup\$

I'm learning JS and Canvas. A friend of mine gave me the task to create a Battle City replica. I've already managed to make the map and the player to move. Nothing fancy just some squares with a color, I tried to replicate the first map:

enter image description here

My code currently creates the map using 26 x 26 little squares (I chosen that amount because there are 13 "cols" in the image above, but each brick col is destroyed partially by a certain amount per shot (if IRC that was about 2-4 shots in the original game), in my case that's 2 bullets, so 13x2 = 26), my player uses 2 x 2 which is not ideal, because it requires to check 2 blocks for every direction, if I wanted to use a bigger grid, the size of the player grid might increase as well, making the code unmaintainable, how could I improve this code in order to have my player a single entity instead of a 4-block entity?

I think my intersection logic is kind of rudimentary, is there a way to improve this as well?

And I also struggled a little bit with the map drawing, as it's drawn vertically so I had to change the i and j variables so that the map wouldn't be rotated 90 degrees, I'm also interested in other options to do this and not having to paint the map and player every time I move the player in one direction.

const mapGrid = [
    [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
    [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
    [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
    [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
    [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
    [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
    [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 2, 2, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
    [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 2, 2, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
    [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
    [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
    [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
    [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
    [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
    [1, 1, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 1, 1],
    [2, 2, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 2, 2],
    [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
    [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
    [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
    [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
    [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
    [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
    [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
    [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
    [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 0, 1, 1, 1, 1, 0, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
    [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 3, 3, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
    [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 3, 3, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
  ];
  
const canvas = document.getElementById('map');
const ctx = canvas.getContext('2d');

const width = 24;
const height = 24;

const Directions = {
  up: 1,
  left: 2,
  right: 3,
  down: 4,
};

Object.freeze(Directions);

const playerCoords = [
  mapGrid.length - 2, 8,
];

const goalCoords = [6, 12];

const toRelativeCoord = (fromCoord) => fromCoord * width;

const drawMap = () => {
  ctx.beginPath();

  for (let i = 0; i < mapGrid.length; i += 1) {
    for (let j = 0; j < mapGrid[i].length; j += 1) {
      switch (mapGrid[i][j]) {
        case 1: //Bricks
          ctx.fillStyle = '#993333';
          break;
        case 2: //Iron-Bricks
          ctx.fillStyle = '#C0C0C0';
          break;
        case 3: //Base
          ctx.fillStyle = '#CCCC99';
          break;
        case 4: //Player
          ctx.fillStyle = '#FFFF00';
          break;
        default: //Road
          ctx.fillStyle = '#000000';
          break;
      }
      ctx.fillRect(j * width, i * height, width, height);
    }
  }
};

const drawPlayer = () => {
  ctx.beginPath();
  ctx.fillStyle = '#FFFF00';
  ctx.fillRect(toRelativeCoord(playerCoords[1]),
    toRelativeCoord(playerCoords[0]), width * 2, height * 2);
};

const repaint = () => {
  drawMap();
  drawPlayer();
  if (hasReachedGoal()) {
    alert('Game Over')
  }
};

const isMapEdge = (x, y, direction) => {
  switch (direction) {
    case Directions.up:
      return x - 1 < 0;
    case Directions.left:
      return y - 1 < 0;
    case Directions.right:
      return y + 2 === mapGrid[0].length;
    default: // back
      return x + 2 === mapGrid.length;
  }
};

const upIsClear = (x, y) => {
  if (isMapEdge(x, y, Directions.up)) {
    return false;
  }
  return mapGrid[x - 1][y] === 0 && mapGrid[x - 1][y + 1] === 0;
};

const leftIsClear = (x, y) => {
  if (isMapEdge(x, y, Directions.left)) {
    return false;
  }
  return mapGrid[x][y - 1] === 0 && mapGrid[x + 1][y - 1] === 0;
};

const rightIsClear = (x, y) => {
  if (isMapEdge(x, y, Directions.right)) {
    return false;
  }
  return mapGrid[x][y + 2] === 0 && mapGrid[x + 1][y + 2] === 0;
};

const downIsClear = (x, y) => {
  if (isMapEdge(x, y, Directions.down)) {
    return false;
  }
  return mapGrid[x + 2][y] === 0 && mapGrid[x + 2][y + 1] === 0;
};

const moveUp = () => {
  if (upIsClear(playerCoords[0], playerCoords[1])) {
    playerCoords[0] -= 1;
    repaint();
  }
};

const moveLeft = () => {
  if (leftIsClear(playerCoords[0], playerCoords[1])) {
    playerCoords[1] -= 1;
    repaint();
  }
};

const moveRight = () => {
  if (rightIsClear(playerCoords[0], playerCoords[1])) {
    playerCoords[1] += 1;
    repaint();
  }
};

const moveDown = () => {
  if (downIsClear(playerCoords[0], playerCoords[1])) {
    playerCoords[0] += 1;
    repaint();
  }
};

const listenToEvents = () => {
  document.addEventListener('keypress', (event) => {
    if (event.key === 'W' || event.key === 'w') {
      moveUp();
    } else if (event.key === 'A' || event.key === 'a') {
      moveLeft();
    } else if (event.key === 'S' || event.key === 's') {
      moveDown();
    } else if (event.key === 'D' || event.key === 'd') {
      moveRight();
    }
  });
};

const intersects = (coord1, coord2) => {
  return coord1 == coord2 || coord1 + 1 == coord2 || coord1 - 1 == coord2;
}

const hasReachedGoal = () => {
  if ((intersects(playerCoords[0], goalCoords[0])) && intersects(playerCoords[1], goalCoords[1]) ||
      (intersects(playerCoords[0], goalCoords[0])) && intersects(playerCoords[1] + 1, goalCoords[1]) ||
      (intersects(playerCoords[0] + 1, goalCoords[0])) && intersects(playerCoords[1], goalCoords[1]) ||
      (intersects(playerCoords[0] + 1, goalCoords[0])) && intersects(playerCoords[1] + 1, goalCoords[1])) {
        alert('Hey!')
  }
  return false;
}

/**
 * DEVELOPER NOTE
 * x = rows
 * y = columns
 *
 * 0, 0 = top left corner
*/

const initialize = () => {
  drawMap();
  drawPlayer();
  listenToEvents();
};

initialize();
<html>
    <head>
        <title>Tank</title>
        <link rel="stylesheet" type="text/css" href="style.css">
    </head>
    <body>
        <canvas id="map" width="624" height="624"></canvas>
    </body>

    <script type="text/javascript" src="main.js"></script>
</html>


Edit

I did some research on how to use classes, and I think the code is now more readable and more structured. I believe this is an improvement, but anyway I'd appreciate if someone could judge it with an expert and critic eye so that I can improve this code.

I'm still interested in how to paint the map horizontally instead of vertically, as currently I have to switch X and Y coords for some calculations.

index.html

<html>
    <head>
        <title>Tank</title>
    </head>
    <body>
        <canvas id="map" width="624" height="624"></canvas>
    </body>

    <script type="text/javascript" src="cell.js"></script>
    <script type="text/javascript" src="goal.js"></script>
    <script type="text/javascript" src="tank.js"></script>
    <script type="text/javascript" src="game.js"></script>
</html>

cell.js

const CellTypes = {
    road: 0,
    bricks: 1,
    ironBricks: 2,
    base: 3,
    player: 4,
    goal: 5
}

class Cell {
    static cellWidth = 24;
    static cellHeight = 24;

    constructor(x, y, color, type) {
        this.color = color;
        this.type = type;
        this.width = Cell.cellWidth;
        this.height = Cell.cellHeight;
        this.x = x * this.width;
        this.y = y * this.height;
    }
}

goal.js

class Goal extends Cell {
    constructor(x, y, color) {
        super(x, y, color, CellTypes.goal);
        this.width = this.width * 2;
        this.height = this.height * 2;
    }
}

tank.js

const Directions = {
    up: 1,
    left: 2,
    right: 3,
    down: 4,
};

class Tank extends Cell {
    constructor(x, y, color) {
        super(x, y, color, CellTypes.player)
        this.direction = Directions.up;
        this.speed = 12;
        this.width = this.width * 2;
        this.height = this.height * 2;
    }

    moveUp() {
        this.y -= this.speed;
    }

    moveDown() {
        this.y += this.speed;
    }

    moveLeft() {
        this.x -= this.speed;
    }

    moveRight() {
        this.x += this.speed;
    }
}

game.js

let maze = {
    map: [
        [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
        [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
        [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
        [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
        [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
        [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
        [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 2, 2, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
        [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 2, 2, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
        [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
        [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
        [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
        [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
        [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
        [1, 1, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 1, 1],
        [2, 2, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 2, 2],
        [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
        [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
        [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
        [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
        [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
        [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
        [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
        [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
        [0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 0, 1, 1, 1, 1, 0, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0],
        [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 3, 3, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
        [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 3, 3, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
    ],
    goals: {
        goalColor: '#34EB9E',
        coords: [
            [12, 4], [0, 0]
        ]
    }
}

let cells = new Array(maze.map.length);
let goals = new Array(maze.goals.coords.length);
//Player coords
let player = {
    x: 8,
    y: 24
}

const canvas = document.getElementById('map');
const ctx = canvas.getContext('2d');
let tank = new Tank(player.x, player.y, '#FFFF00');;

const initialize = () => {
    configureMaze();
    repaint();
    listenToEvents();
}

//Sets the data as cells objects
const configureMaze = () => {
    for(let i = 0; i < maze.map.length; i++) {
        cells[i] = new Array(maze.map[i].length);
        for(let j = 0; j < maze.map[i].length; j++) {
            switch(maze.map[i][j]) {
                case 1:
                    cells[i][j] = new Cell(j, i, '#993333', CellTypes.bricks);
                    break;
                case 2:
                    cells[i][j] = new Cell(j, i, '#C0C0C0', CellTypes.ironBricks);
                    break;
                case 3:
                    cells[i][j] = new Cell(j, i, '#CCCC99', CellTypes.base);
                    break;
                default:
                    cells[i][j] = new Cell(j, i, '#000000', CellTypes.road);
                    break;
            }
        }
    }
}

//Draws the maze based on the configuration
const drawMaze = () => {
    ctx.beginPath();

    cells.forEach(cellsArr => {
        cellsArr.forEach(cell => {
            ctx.fillStyle = cell.color;
            ctx.fillRect(cell.x, cell.y, cell.width, cell.height)
        })
    })
}

//Goals are where some powerups will be
const drawGoals = () => {
    let i = 0;
    maze.goals.coords.forEach(coord => {
        goals[i] = new Goal(coord[0], coord[1], '#34EB9E');
        ctx.beginPath();
        ctx.fillStyle = '#34EB9E';
        ctx.fillRect(goals[i].x, goals[i].y, goals[i].width, goals[i].height);
        i++;
    })
}

//Draws the player's tank
const drawPlayerTank = () => {
    ctx.beginPath();
    ctx.fillStyle = tank.color;
    ctx.fillRect(tank.x, tank.y, tank.width, tank.height);
}

//Repaints the UI
const repaint = () => {
    ctx.clearRect(0, 0, canvas.width, canvas.height);
    drawMaze();
    drawGoals();
    drawPlayerTank();
}

//Checks if the tank is on the canvas limit
const isMapLimit = (direction) => {
    switch (direction) {
        case Directions.up:
            return tank.y - 1 < 0;
        case Directions.down:
            return tank.y + 1 >= toCanvasCoord(maze.map.length - 2, Cell.cellWidth);
        case Directions.left:
            return tank.x - 1 < 0;
        case Directions.right:
            return tank.x + 1 >= toCanvasCoord(maze.map[0].length - 2, Cell.cellHeight);
    }
}

//Transforms map coords to canvas coords
const toCanvasCoord = (coord, toValue) => {
    return coord * toValue;
}

//Transforms canvas coords to map coords
const toMapCoord = (coord, toValue) => {
    return Math.floor(coord / toValue);
}

//Checks for intersection of coords
const intersects = (x1, y1, x2, y2, width, height) => {
    return x1 + width > x2 && y1 + height > y2 && x1 < x2 + width && y1 < y2 + height;
}

//Checks if we're standing in any of the goals zones
const isGoal = () => {
    for (let i = 0; i < goals.length; i++) {
        if (intersects(tank.x, tank.y, goals[i].x, goals[i].y, goals[i].width, goals[i].height)) {
            return true;
        }
    }
    return false;
}

//Checks if the cell that we're trying to move is a road cell
const isRoadCell = (direction) => {
    let xCoord1; //xCoord for the top left corner
    let yCoord1; //yCoord for the top left corner
    let xCoord2; //xCoord for the tank's width
    let yCoord2; //xCoord for the tank's height
    switch (direction) {
        case Directions.up:
            xCoord1 = toMapCoord(tank.x, Cell.cellWidth);
            yCoord1 = toMapCoord(tank.y - tank.speed, Cell.cellHeight);
            xCoord2 = toMapCoord(tank.x + tank.width - 1, Cell.cellWidth);
            yCoord2 = toMapCoord(tank.y - tank.speed, Cell.cellHeight);
            break;
        case Directions.down:
            xCoord1 = toMapCoord(tank.x, Cell.cellWidth);
            yCoord1 = toMapCoord(tank.y + tank.height, Cell.cellHeight);
            xCoord2 = toMapCoord(tank.x + tank.width - 1, Cell.cellWidth);
            yCoord2 = toMapCoord(tank.y + tank.height, Cell.cellHeight);
            break;
        case Directions.left:
            xCoord1 = toMapCoord(tank.x - tank.speed, Cell.cellWidth);
            yCoord1 = toMapCoord(tank.y, Cell.cellHeight);
            xCoord2 = toMapCoord(tank.x - tank.speed, Cell.cellWidth);
            yCoord2 = toMapCoord(tank.y + tank.height - 1, Cell.cellHeight);
            break;
        case Directions.right:
            xCoord1 = toMapCoord(tank.x + tank.width, Cell.cellWidth);
            yCoord1 = toMapCoord(tank.y, Cell.cellHeight);
            xCoord2 = toMapCoord(tank.x + tank.width, Cell.cellWidth);
            yCoord2 = toMapCoord(tank.y + tank.height - 1, Cell.cellHeight);
            break;
    }
    if (maze.map[yCoord1][xCoord1] === CellTypes.road && maze.map[yCoord2][xCoord2] === CellTypes.road) {
        return true;
    }
    return false;
}

//Listens to WASD key presses
const listenToEvents = () => {
    document.addEventListener('keypress', (event) => {
        if (event.key === 'W' || event.key === 'w') {
            tank.direction = Directions.up;
            if (!isMapLimit(tank.direction) && isRoadCell(tank.direction)) {
                tank.moveUp();
                repaint();
            }
        } else if (event.key === 'A' || event.key === 'a') {
            tank.direction = Directions.left;
            if (!isMapLimit(tank.direction) && isRoadCell(tank.direction)) {
                tank.moveLeft();
                repaint();
            }
        } else if (event.key === 'S' || event.key === 's') {
            tank.direction = Directions.down;
            if (!isMapLimit(tank.direction) && isRoadCell(tank.direction)) {
                tank.moveDown();
                repaint();
            }
        } else if (event.key === 'D' || event.key === 'd') {
            tank.direction = Directions.right;
            if (!isMapLimit(tank.direction) && isRoadCell(tank.direction)) {
                tank.moveRight();
                repaint();
            }
        }
        if (isGoal()) {
            alert('GOAL!')
        }
    });
}

initialize();
\$\endgroup\$
1
+50
\$\begingroup\$

Intersection logic

For the intersection logic, maybe it would be appropriate to make a method on the cell class that would determine if another cell (e.g. tank) overlaps with it. Refer to this answer for inspiration.

Review

Just as a POJO is created for the definition of Directions I'd recommend setting up a POJO for the colors used in drawMap. That way instead of using the switch statement, the code can check to see if the value exists as a key in the mapping and then set the value of ctx.fillStyle accordingly.

The keypress handler can be simplified using event.key.toLowerString(), as well as a mapping of keys to directions.

hasReachedGoal calls alert. Some users may have disabled alerts in a browser setting. It is better to use HTML5 <dialog> element - it allows more control over the style and doesn't block the browser. Bear in mind that it isn't supported by IE and Safari but there is a polyfill

In game.js the function grawGoals() uses foreach with a callback function that accepts coord as a parameter. Instead of manually setting and updating a counter variable i.e. i accept it as the second parameter because forEach passes three arguments1.

In the original code mapGrid is declared with const, yet in game.js the maze is declared with let. It is better to use const as a default and then when you determine re-assignment is needed switch to let. This helps avoid accidental re-assignment.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ The color thing is a really good suggestion, I didn't think of it. Would you say it's better to change Directions map, to something like up: 'W', ...? Those alerts were just for testing purposes actually, but I didn't know the <dialog> element existed. Thank you for this really helpful answer. I'll leave the question open until the bounty expires just in case someone else can provide some extra points. \$\endgroup\$ – Frakcool Jun 19 '20 at 18:32

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.