6
\$\begingroup\$

This is my first game made using canvas, currently it works well but I know my JavaScript could be better as I am still learning. What improvements can I make on the structure of my JavaScript?

document.addEventListener("DOMContentLoaded", function() {

  var c = document.getElementById("board");
  var ctx = c.getContext("2d");
  c.width = 400;
  c.height = 400;

  var map = [
    [4,9,12,8],
    [6,15,3,11],
    [7,0,1,10],
    [13,14,2,5]
  ];

  var winMap = [
    [1,2,3,4],
    [5,6,7,8],
    [9,10,11,12],
    [13,14,15,0]
  ];

  var tileMap = [];

  var tile = {
  width : 100,
  height : 100
  };

  var pos = {
    x : 0,
    y : 0,
    textx : 45,
    texty : 55
  };

  var drawTile = function(){
    ctx.fillStyle = '#EB5E55';
    ctx.shadowColor = '#000';
    ctx.shadowBlur = 4;
    ctx.shadowOffsetX = 0;
    ctx.shadowOffsetY = 2;
    ctx.fillRect(pos.x + 5,pos.y + 5,tile.width-10,tile.height-10);
    ctx.shadowColor = "transparent";
    ctx.fillStyle = '#FFFFFF';
    ctx.font = "20px Arial";
    //adjust center for larger numbers
    if(map[i][j] >= 10){
      ctx.fillText(map[i][j], pos.textx -2 , pos.texty);
    }else{
      ctx.fillText(map[i][j], pos.textx + 2 , pos.texty);
    }
  };

  var buildBoard = function(){
    for(i = 0; i < map.length; i++){

      tileMap[i] = [];

      for(j = 0; j < map[i].length; j++){
        var currentTile = {
            tileName : map[i][j],
            x : pos.x,
            y : pos.y,
            width : 100,
            height : 100,
            tileIndex : j
          };

        if(map[i][j] !== 0){
          //create our numbered box
          drawTile();
          //push box id and cords to tilemap array
          tileMap[i].push(currentTile);

        }else{
          //create our zero box
          tileMap[i].push(currentTile);
        }
        pos.textx += 100;
        pos.x += 100;
      }
      pos.x = 0;
      pos.textx = 43;
      pos.texty += 100; 
      pos.y += 100;
    }
  };

  //get mouse position
  function getPosition(event){

    var x = event.x;
    var y = event.y;
    x -= c.offsetLeft;
    y -= c.offsetTop;

    //Check to see which box we are in
    for(var i = 0; i < tileMap.length; i++){
      for(var j = 0; j < tileMap[i].length; j++){
        if(y > tileMap[i][j].y && y < tileMap[i][j].y + tileMap[i][j].height &&
          x > tileMap[i][j].x && x < tileMap[i][j].x + tileMap[i][j].width){
              checkMove(tileMap[i][j].tileName, tileMap[i][j].tileIndex);
           }
      }
    }
  }


  // detect if move is possible
  var checkMove = function(item, itemIndex){
    //check column for zero and clicked box
    var checkColumn = function(){
      var zeroIndex = null;
      //check for zero
      for(var x = 0; x < map.length; x++){
          zeroIndex = map[x].indexOf(0);
        if(zeroIndex > -1){
          zeroIndex = zeroIndex;
          break;
        }
      }
      if(zeroIndex === itemIndex){
        //create a new array with column values
        var tempArr = [];
        for(var i = 0; i < map.length; i++){
          tempArr.push(map[i][zeroIndex]);
        }
        //keep track of our clicked item and zero
        var zero = tempArr.indexOf(0);
        var selectedItem = tempArr.indexOf(item);

        //sort our tempArray
        if (selectedItem >= tempArr.length) {
          var k = selectedItem - tempArr.length;
          while ((k--) + 1) {
            map[i].push(undefined);
          }
        }
        tempArr.splice(selectedItem, 0, tempArr.splice(zero, 1)[0]);

        //update our map with the correct values for the column
        for(var l = 0; l < map.length; l++){
          map[l][zeroIndex] = tempArr[l];
        }
      }
    };


    //check row for zero and clicked box
    var checkRow = function(){
      for(var i = 0; i< map.length; i++){
        var itemIndex = map[i].indexOf(item);
        var zeroIndex = map[i].indexOf(0);
        //if zero and clicked box are present in same row
        if(itemIndex > -1 && zeroIndex > -1){
          //reorder row
          if (itemIndex >= map[i].length) {
            var k = itemIndex - map[i].length;
            while ((k--) + 1) {
              map[i].push(undefined);
            }
          }
          map[i].splice(itemIndex, 0, map[i].splice(zeroIndex, 1)[0]);
        }
      }
    };

    checkColumn();
    checkRow();

    clear();
  };

  var clear = function(){
    ctx.clearRect(0, 0, 400, 400);
    pos = {
      x : 0,
      y : 0,
      textx : 46,
      texty : 55
    };
    buildBoard();
    checkWin();
  };

  var checkWin = function(){
    var allMatch = true;
    for(var i = 0; i < winMap.length; i++){
      for(var j = 0; j < winMap[i].length; j++){
        if(map[i][j] !== winMap[i][j]){
          allMatch = false;
        }
      }    
    }
    if(allMatch){
      var winMessage = document.querySelector('.win');
      winMessage.classList.remove('hide');
      winMessage.classList.add('fall');
    }
  }

  buildBoard();
  c.addEventListener("mousedown", getPosition, false);
});
<canvas id="board"></canvas>

View The CodePen

\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

Your puzzle looks nice! Here are a few recommendations regarding your code:

Standards

Your getPosition function accesses non-standard attributes and doesn't work e.g. in Firefox. Follow this advice and write

let rect = canvas.getBoundingClientRect();
let x = event.clientX - rect.left;
let y = event.clientY - rect.top;

Scope

Remove references to globals such as c or map from within your functions and pass them via parameters.

Move all code that doesn't directly rely on the DOMContentLoaded event out of that one huge event listener.

State

Separate the game state from your UI properties. E.g. the tile's width and height are attributes of their visual representation on the screen and should not be part of your board model.

Also separate the logic that handles game state updates from the logic that handles UI updates. E.g. introduce a board model and functions operating on that model such as push, isSolved etc. and listen to state changes for UI updates by using the observer design pattern.

Your board representation is redundant and more complicated than necessary. Simply storing numbers at specific columns and rows in a two-dimensional array - similar to your map and winMap arrays - should be sufficient.

Consider flattening your two-dimensional array into a large one-dimensional array by concatenating rows or columns. This simplifies array creation, copying and comparing and improves performance. E.g. your checkWin logic could then be rewritten as

map.length == winMap.length && map.every((tile, i) => tile == winMap[i])

Also consider explicitly tracking and storing the coordinates of the empty tile. This simplifies and generalizes your code somewhat.

An exemplary board state would then consist of a board with width, height, tiles and the x and y coordinates of the empty tile, e.g.:

const board = {width: 2, height: 2, tiles: [3, 1, 0, 2], x: 0, y: 1}

It is advantageous to make your board state immutable. State changes by pushing tiles then require overriding the old array of tiles with the updated one instead of directly mutating it. For moving or rather pushing a tile at position x, y, you could then use the following efficient function:

function pushTile(board, x, y) {
  if ((x == board.x) == (y == board.y)) return board;

  let tiles = board.tiles.slice();
  let start = board.x + board.y * board.width;
  let stop  = x + y * board.width;
  let step  = y == board.y ? 1 : board.width;

  if (start > stop) step = -step;

  for (let i = start; i != stop; i += step) {
    [tiles[i + step], tiles[i]] = [tiles[i], tiles[i + step]];
  }
  return Object.assign({}, board, {tiles: tiles, x: x, y: y});
}

Naming

Choose descriptive names. E.g. change c into canvas. Separate and clearly name concerns. E.g. your function checkWin should just check whether the puzzle is solved, not perform additional tasks such as displaying a message. The same holds for checkMove.

Optional

The useCapture argument in addEventListener("mousedown", click, false) is false per default, you can leave it out.

User Interface

Your user interface can be simplified by replacing the <canvas> with styled DOM elements for each individual tile. Your browser is pretty good at event handling and mapping mouse coordinates, so take advantage of that.

Using react.js - an established library which handles and minimizes DOM updates when (UI) state changes - we get the following code:

// Compute new board with tiles at x, y pushed towards the empty space:
function pushTile(board, x, y) {
  if ((x == board.x) == (y == board.y)) return board;
  
  let tiles = board.tiles.slice();
  let start = board.x + board.y * board.width;
  let stop  = x + y * board.width;
  let step  = y == board.y ? 1 : board.width;

  if (start > stop) step = -step;

  for (let i = start; i != stop; i += step) {
    [tiles[i + step], tiles[i]] = [tiles[i], tiles[i + step]];
  }
  return Object.assign({}, board, {tiles: tiles, x: x, y: y});
}

// Compare board tiles with a given solution:
function isSolved(board, solution) {
  return board.tiles.every((tile, i) => tile == solution[i]);
}

// Retrieve value of tile at x, y:
function tileAt(board, x, y) {
  return board.tiles[x + y * board.width];
}

// React component displaying a single tile:
function Tile(props) {
  const className = props.isSpace ? "tile tile-space" : "tile";
  return (
    <button className={className} onMouseDown={props.onMouseDown}>
      {props.value}
    </button>
  );
}

// React component displaying a grid of tiles: 
function Tiles(props) {
  let rows = [];
  for (let y = 0; y < props.board.height; y++) {
    let row = [];
    for (let x = 0; x < props.board.width; x++) {
      row.push(
        <Tile
          value={tileAt(props.board, x, y)}
          isSpace={props.board.x == x && props.board.y == y} 
          onMouseDown={() => props.onMouseDown(x, y)}
        />
      );
    }
    rows.push(<div className="tiles-row">{row}</div>);
  }
  return (<div className="tiles">{rows}</div>);
}

// React component displaying a complete puzzle:
class Puzzle extends React.Component {
  constructor(props) {
    super(props);
    
    const board = {
      tiles: props.tiles,
      width: props.width,
      height: props.height,
      x: props.x,
      y: props.y
    };
    this.solution = props.solution;
    this.state = {board: board, solved: isSolved(board, this.solution)}
  }
  handleMouseDown(x, y) {
    let board = pushTile(this.state.board, x, y);
    this.setState({board: board, solved: isSolved(board, this.solution)});
  }
  render() {
    return (
      <div className="puzzle">
        <Tiles
          board={this.state.board}
          onMouseDown={(x, y) => this.handleMouseDown(x, y)}
        />
        {this.state.solved && <span className="puzzle-solved">You won!</span>}
      </div>
    );
  }
}

// Initialise and render the puzzle:
ReactDOM.render(
  <Puzzle
    tiles={[
      4,  9,  12, 8,
      6,  15, 3,  11,
      7,  0,  1,  10,
      13, 14, 2,  5
    ]}
    solution={[
      1,  2,  3,  4,
      5,  6,  7,  8,
      9,  10, 11, 12,
      13, 14, 15, 0
    ]}
    width={4} height={4}
    x={1} y={2}
  />,
  document.getElementById("root")
);
.puzzle {
  position: relative;
  display: inline-block;
  text-align: center;
  background-color: #3A3335;
  border: 2px solid #3A3335;
  box-shadow: 0 10px 30px -10px #000;
}

.puzzle-solved {
  position: absolute;
  width: 100%;
  left: 50%;
  top: 50%;
  transform: translate(-50%,-50%) rotate(-15deg);
  font-size: 50px;
  color: #FDF0D5;
  text-shadow: 3px 3px 0 #000, -1px -1px 0 #000, 1px -1px 0 #000, -1px 1px 0 #000, 1px 1px 0 #000;
}

.tile {
  margin: 5px;
  width: 90px;
  height: 90px;
  font-size: 20px;
  color: #FDF0D5;
  background-color: #EB5E55;
  box-shadow: 0px 2px 4px 0px #000;
  border: none;
}

.tile-space {
  visibility: hidden;
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/15.1.0/react.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/15.1.0/react-dom.min.js"></script>

<div id="root"></div>

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

I do not have enough reputation to comment so here is an answer..

Given only row 0, column 3 is empty..

When I click on 0,0 all the block on row 0 will move to the right and the board will end up with 0,0 being empty.

..however..

When I click on 3,3 only one block will move up and the board will end up with 1,3 being empty..

You may want to check this, seems like a minor bug to me!

\$\endgroup\$
2

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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