0
\$\begingroup\$

I would like to simplify and optimize this function which finds me the neighbors of a cell thanks to cartesian coordinates of a cell Id

const getNeighborsCellId = cellId => {
    const neighbors = []
    const {x, y} = getCoordinatesFromCellId(cellId)



    // LOL ->

    const isStaggered = y % 2 
    if (isStaggered){


      neighbors.push(getCellIdFromCoordinates(x, y - 1)) 
      neighbors.push(getCellIdFromCoordinates(x, y - 2)) 
      neighbors.push(getCellIdFromCoordinates(x + 1, y - 1)) 
      neighbors.push(getCellIdFromCoordinates(x - 1, y)) 
      neighbors.push(getCellIdFromCoordinates(x + 1, y)) 
      neighbors.push(getCellIdFromCoordinates(x, y + 1)) 
      neighbors.push(getCellIdFromCoordinates(x, y + 2))
      neighbors.push(getCellIdFromCoordinates(x + 1, y + 1))

    }
    else {
      neighbors.push(getCellIdFromCoordinates(x - 1, y - 1))
      neighbors.push(getCellIdFromCoordinates(x, y - 2))
      neighbors.push(getCellIdFromCoordinates(x, y - 1))
      neighbors.push(getCellIdFromCoordinates(x - 1, y))
      neighbors.push(getCellIdFromCoordinates(x + 1, y))
      neighbors.push(getCellIdFromCoordinates(x - 1, y + 1))
      neighbors.push(getCellIdFromCoordinates(x, y + 2))
      neighbors.push(getCellIdFromCoordinates(x, y + 1))
    }

    return neighbors
}
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

First off, I'm not a game developer, and this is a fundamental point, but wouldn't it be easier to use "regular" coordinates (for example: https://i.sstatic.net/XpFSQ.png) instead of "staggered" ones (https://i.sstatic.net/Mt5Mw.png)?


Anyway, aside from that, in my opinion there are two major problems about your your function: A) Is doing too much and B) repeating itself.

The main function of calculating the neighbor coordinates should be separate and take an coords object and return an array of coordinates objects. It should use a list of (either pre-calculated or hardcoded) relative coordinates of the neighbors. If you have, for example, functions to calculete the explicit north, north-east, east, etc. neighbors, then they should be sharing this data.

const CELL_NEIGHBORS = [
   // Even:
   [{dx: 0, dy: -1}, {dx: 0, dy: -2}, {dx: +1, dy: -1} /* etc. */ ],
   // Odd:
   [ /* TODO */ ]
];

function getAllNeighborCoordinates({x, y}) {
   const evenOdd = y % 2;

   return CELL_NEIGHBORS[evenOdd].map({dx, dy} => {x: x + dx, y: y + dy});
}

Now if you change getCellIdFromCoordinates to also take an coordinates object, the final function would be:

 function getNeighborsCellId(cellId) {
   return getAllNeighborCoordinates(getCoordinatesFromCellId(cellId))
     .map(getCellIdFromCoordinates);
 }

(DISCLAMER: I'm not that into ES2015 yet, so there may be errors in my code.)

\$\endgroup\$

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.