7
\$\begingroup\$

I have a 2D array (or rather, an array of arrays) representing tile maps in game. The 'world' map array holds smaller arrays representing individual map parts which I am calling 'rooms':

var worldMap = [
    [room1, room2],
    [room3, room4]
];

I have some code which takes note of the 'current' room (which will be visible on screen to the player) and calculates what map should be loaded as the 'next' room. This will be triggered when the player walks off the edge of the screen. The key variable is currentMap - this variable will be passed to the function responsible for rendering the tile map that the user can see.

My code works and suits my purposes, but I'm wondering if there is a more efficient or elegant way to write it.

JSFiddle Code

function indexOf2d(arrayParam, valueParam) {
    var index = [-1, -1];
    if (!Array.isArray(arrayParam)) {
        return index;
    }
    arrayParam.some(function (sub, posX) {
        if (!Array.isArray(sub)) {
            return false;
        }
        var posY = sub.indexOf(valueParam);
        if (posY !== -1) {
            index[0] = posX;
            index[1] = posY;
            outputA = index[0];
            outputB = index[1];
            return true;
        }
        return false;
    });
    return index;
}

function goToNextRoomDown(map, valueA, valueB) {
    var newValue = valueA + 1;
    var result = map[newValue][valueB];
    return result;
}

Please note that although I am only showing 'goToNextRoomDown', I intend for the user to be able to access maps that are above, below, left and right of their current location. I'm intending to write a function that handles all of these possibilities, rather than having four separate functions, but before I do that I'm more interested in improving what I have presented so far. I'm not asking anyone to write the code for me, so I hope this question still qualifies as on-topic.

// get the index of the currentMap within the worldMap
console.log(indexOf2d(worldMap, currentMap));
// use the index value outputs to calculate the next room 'down' the worldMap array
console.log('next room down: ' + goToNextRoomDown(worldMap, outputA, outputB));
// assign the new room as the currentMap
currentMap = goToNextRoomDown(worldMap, outputA, outputB);
\$\endgroup\$
4
\$\begingroup\$

I like your question, games are one the best ways to get to know a language better.

From a once over:

  • Unless you have other 2d searching needs elsewhere, I would have called indexOf2d as findRoomLocation
  • I would have made findRoomLocation a part of worldMap
  • You are both returning the found location with index and setting the globals outputA and outputB. Setting those globals is considered very bad practice.
  • If you can exit early from loops (you could exit once you found a room), then I would advise against use some and go for for instead
  • Also, indexOf is slower than rolling your own search so I would use 2 old school loops

    worldMap.findRoom2 = function( room ){
      var notFound = [-1, -1],
      row;
    
      if (!Array.isArray(this)) {
        return notFound;
      }
    
      for( var x = 0 , xLength = this.length ; x < xLength ; x++ ) {
        row = this[x];
        if (!Array.isArray(row)) {
          continue;
        }    
        for( var y = 0 , yLength = row.length ; y < yLength ; y++ ) {
          if( row[y] == room ){
            return [x,y];
          }
        } 
      }
      return notFound;
    };
    

    If this was for a personal non production project I would settle for something like this

    worldMap.findRoom = function( room )
    {
      var x = -1, y;
      while( this[y=-1,++x] )
        while( this[x][++y] )
          if( this[x][y] == room )
            return [x,y];
      return [-1,-1]; //Not found
    };
    

All in all, I would counter propose something like this:

// noprotect
var room1 = [
    [1, 1, 1, 1, 1, 1, 1],
    [1, 1, 1, 1, 1, 1, 1],
    [1, 1, 1, 1, 1, 1, 1]
];

var room2 = [
    [2, 2, 2, 2, 2, 2, 2],
    [2, 2, 2, 2, 2, 2, 2],
    [2, 2, 2, 2, 2, 2, 2]
];

var room3 = [
    [3, 3, 3, 3, 3, 3, 3],
    [3, 3, 3, 3, 3, 3, 3],
    [3, 3, 3, 3, 3, 3, 3]
];

var room4 = [
    [4, 4, 4, 4, 4, 4, 4],
    [4, 4, 4, 4, 4, 4, 4],
    [4, 4, 4, 4, 4, 4, 4]
];

var worldMap = [
    [room1, room2],
    [room3, room4]
];

var outputA;
var outputB;
var currentMap = room2;

worldMap.findRoom = function( room ){
  var notFound = [-1, -1],
      row;
  
  if (!Array.isArray(this)) {
        return notFound;
  }
  
  for( var x = 0 , xLength = this.length ; x < xLength ; x++ ) {
    row = this[x];
    if (!Array.isArray(row)) {
        continue;
    }    
    for( var y = 0 , yLength = row.length ; y < yLength ; y++ ) {
      if( row[y] == room ){
        return [x,y];
      }
    } 
  }
  return notFound;
};

worldMap.findRoomAlt = function( room )
{
  var x = -1, y = 0;
  while( this[y=-1,++x] )
    while( this[x][++y] )
      if( this[x][y] == room )
        return [x,y];
  return [-1,-1]; //Not found
};

worldMap.findNextRoom = function( directionName ){
  //dx & dy -> Delta on x and y axis
  var directions = {
    down : {  dx : 1 , dy : 0 }  
  };
  var direction = directions[directionName];
  //Paranoia
  if(!direction){
    //Do something, I would actually throw an error
  }
  
  var location = this.findRoom(currentMap);
  location[0] += direction.dx;
  location[1] += direction.dy;
  return this[location[0]][location[1]];
};

// show the existing currentMap
console.log('existing currentMap = ' + currentMap);

// get the index of the currentMap within the worldMap
console.log(worldMap.findRoom(currentMap));

// use the index value outputs to calculate the next room 'down' the worldMap array

console.log('next room down: ' + worldMap.findNextRoom('down'));

currentMap = worldMap.findNextRoom('down');
console.log('new currentMap = ' + currentMap);

\$\endgroup\$
  • \$\begingroup\$ Just a nitpick about indexOf: it doesn't seem that it's called all that often, so unless the user is running an old browser on a very old machine, I don't think the performance 'hit' is worth rolling out two ugly loops. +1 anyway, good suggestions. \$\endgroup\$ – Chris Cirefice Sep 23 '14 at 1:26

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.