General javascript style notes
- Clean javascript has semicolons in the right places.
// bad
const foo = 1
// good
const foo = 1;
- Clean Javascript always uses delimited blocks
// worst
if(foo === bar) poo = 1
// bad
if(foo === bar) poo = 1;
// good
if (foo === bar) {
poo = 1;
}
// or
if (foo === bar) { poo = 1; }
// or to satisfy some linters semicolons only at line end
if (foo === bar) { poo = 1 }
- Clean Javascript does not repeat itself or have superfluous content
Bad
const endX = cellCoordinates.x + constants.cellWidth
const endY = cellCoordinates.y + constants.cellHeight
if (cellCoordinates.x < x && x < cellCoordinates.x + constants.cellWidth)
if (cellCoordinates.y < y && y < cellCoordinates.y + constants.cellHeight){
Good
const endX = cellCoordinates.x + constants.cellWidth
const endY = cellCoordinates.y + constants.cellHeight
// only one if statment needed
if (cellCoordinates.x < x && x < endX && cellCoordinates.y < y && y < endY){
- General rule is no calculations inside statements
// bad
if((offsetY - y) / (endX - x) >= (offsetY - (constants.cellHeight - constants.offsetHeight)) / (endX - offsetX)) return cellId
// good
const ySlope = (offsetY - y) / (endX - x);
const yOffsetSlope =
(offsetY - (constants.cellHeight - constants.offsetHeight)) /
(endX - offsetX);
if (ySlope >= yOffsetSlope) {
return cellId;
}
More points
It is nice to have descriptive names, it is not nice when descriptive names get in the way of code readability. It is even worse when names are in conflict.
// a map point is a cell coordinate???
const cellCoordinates = getMapPointFromCellId(cellId)
// but the function called gets the cell coordinates
const getMapPointFromCellId = cellId => {
var {x, y} = getCoordinatesFromCellId(cellId)
x = x * constants.cellWidth
if (y % 2) x += constants.offsetWidth
y = y * constants.offsetHeight
return {x, y}
}
Should the variable not be const cellMapPoint = getMapPointFromCellId(cellId)
And constants is meaningless out of context.
And speaking of context you prefixe some variable names with cell and others not.
// prefixed
var cellId = 0
while(cellId < constants.numberCells){
// prefixed
const cellCoordinates = getMapPointFromCellId(cellId)
// not prefixed
const offsetX = cellCoordinates.x + constants.offsetWidth
const offsetY = cellCoordinates.y + constants.offsetHeight
// not prefixed
const endX = cellCoordinates.x + constants.cellWidth
const endY = cellCoordinates.y + constants.cellHeight
It is clear that the function is dealing with cells and that you don't need to prefix some names so why prefix others??
And from the code I can not workout if you are using an Id or an index. An id is different than an index and you would not test an id against a count while(cellId < constants.numberCells){ nor would you increment an id without testing if it exists so I think its an index.
const getCellIndexFromMapPoint = (x, y) => {
var index = 0;
const cells = constants; // ALIAS
while(index < constants.numberCells){
const mapPoint = getMapPointFromCellIndex(index);
const offsetX = mapPoint.x + cells.offsetWidth;
const offsetY = mapPoint.y + cells.offsetHeight;
const endX = mapPoint.x + cells.cellWidth;
const endY = mapPoint.y + cells.cellHeight;
if (mapPoint.x < x && x < endX && mapPoint.y < y && y < endY){
// and so on
}
index += 1;
}
Axonometric projections
With all that said the whole function is a problem as is doing a search when a direct index can be found.
* note that the code below is off the top of my head and may contain typos, it is a suggestion and you should do some extra research on axonometric projections to create code more suited to your needs.*
All axonometric projections (Isometric is a particular type of axonometric projection) can be defined as 4 2d vectors that define the direction and scale of each of the 3D axis X,Y,Z and the location of the origin. If we ignore the Z as we are after a cell coordinate the problem can be simplified even further.
Consider the image above, The x axis and y axis are defined as a 2D vector each with an X and y magnitude that usually defines the size of a pixel. The origin defines where in screen space the coordinate 0,0 will appear.
You can define the axis as a matrix
const projection = [
1, 0.5, // direction and size of x axis
-1, 0.5, // direction and size of y axis
0, 0 // the origin
];
From that matrix you can calculate a screen position for a pixel
function getScreenPos(x, y, screenPos = {}) {
const P = projection; // alias
screenPos.x = x * P[0] + y * P[2] + P[4];
screenPos.y = x * P[1] + y * P[3] + P[5];
return screenPos;
}
Create a projection
In that projection matrix is all you need to convert from screen space to world space (the reverse of the above function). So when you define the projection matrix you also define the inverse matrix
function defineProjection(origin, xAxis, yAxis){
const m = [ // the projection matrix
xAxis.x, xAxis.y,
yAxis.x, yAxis.y,
origin.x, origin.y,
];
const im = []; // the inverse matrix
// get the determinate (I call it cross as it is a cross product of two vectors)
const cross = m[0] * m[3] - m[1] * m[2];
im[0] = m[3] / cross;
im[1] = -m[1] / cross;
im[2] = -m[2] / cross;
im[3] = m[0] / cross;
// define the projection matrix object
return {
matrix : m,
invMatrix : im,
toScreen(worldPos, screenPos = {}) {
const W = worldPos; // alias
screenPos.x = W.x * m[0] + W.y * m[2] + m[4];
screenPos.y = W.x * m[1] + W.y * m[3] + m[5];
return screenPos;
},
toWorld(screenPos, worldPos = {}) {
const x = screenPos.x - m[4];
const y = screenPos.y - m[5];
worldPos.x = x * im[0] + y * im[2];
worldPos.y = x * im[1] + y * im[3];
return worldPos;
},
};
}
Create a map.
Now if you have a regular grid of cells that is defined as an array with a modulo. mapDesc defines the number of cells across and down. cellDesc defines the size of each cell. items are the cells as an array where a cell's position is related to the array index
var worldPos = {
x : (index % cells.mapDesc.width) * cells.cellDesc.width,
y : (index / cells.mapDesc.width | 0) * cells.cellDesc.height
}
// and screen pos is
var screenPos = cells.projection.toScreen(worldPos);
The object below is a bare bones simple version
const cells = {
items : [], // contains cells
projection : defineProjection(
{ x : 1, y : 0.5 }, // x axis
{ x : -0.5, y : 1 }, // y axis
{ x : 0, y : 0 }, // origin
),
mapDesc : {
width : 32, // number cells
height : 32,
},
cellDesc : {
width : 64,
height : 64,
},
getCellAt(screenPos) {
const worldPos = cells.projection.toWorld(screenPos);
worldPos.x /= cellDesc.x;
worldPos.y /= cellDesc.y;
worldPos.x |= 0; // floor
worldPos.y |= 0; // floor
if (worldPos.x >= 0 && worldPos.x < cells.mapDesc.width &&
worldPos.y >= 0 && worldPos.y < cells.mapDesc.height) {
const cellIndex = worldPos.x + worldPos.y * mapDesc.width;
return cells.items[cellIndex];
}
},
}
The above object defines an array of cells (items) and a description of the projection used and details about a cell size and details about the cell layout.
You can call the function getCellAt({x : ?, y : ?}) and it returns a cell if you have given a coordinate over a cell. It does it very fast compared to a search.
Improved search
Even if the cells are randomly placed and you need to do a search, converting to world coordinates means you can work in axis aligned coordinates rather than the complex inversion you do for each cell.
// your search function need only do
const worldPos = projection.toWorld({x,y});
const endX = cell.x + constants.cellWidth
const endY = cell.y + constants.cellHeight
if (cell.x < worldPos.x && worldPos.x < endX &&
cell.y < worldPos.y && worldPos.y < endY){
return index;
}
getMapPointFromCellId? until then, this post is basically off-topic \$\endgroup\$