Fixed incorrect math for isometric screen-to-map tile projection.

[Sam-Izdat]

The isometric projection math for screenToChart was a little screwy, causing some undesired behavior:

I changed this math to correspond to the expected behavior, same as in the Tiled Map Editor:

[Sam-Izdat]

There also seems to be an incorrect check for out-of-bounds in the method getIndexFromCoords:

Note that when the camera is moved, everything is fine. However, when the tilemap is moved, the method now returns -1 for the width that it was moved by, starting at the leftmost bounds of tilemap.

The problem seems to be here:

if (x > this.x + halfWidth || x < this.x - halfWidth) return -1;
if (y > this.y + this.heightInPixels || y < this.y) return -1;

If I'm not overlooking something, the pixel coordinates must always be relative to the tilemap layer itself, so considering this.x or this.y will inevitably make part of the map non-addressable. Removing those constraints resolved this issue and also correctly reported when the pointer was out bounds:

if (x > halfWidth || x < -halfWidth) return -1;
if (y > this.heightInPixels || y < 0) return -1;

[Sam-Izdat]

On second look, the method above to check for out-of-bounds is still insufficient. The tilemap is basically treated as orthogonal, filling its rectangular area completely:

The correct way to check appears to be this:

if (Math.abs(x) > halfWidth - halfWidth * Math.abs(y - halfHeight) / halfHeight) return -1;

The map is now treated as isometric and the empty corners return as out-of-bounds:

However, possibly due to floating point rounding errors, there's a few stray pixels at the bottom-right that pass the check when they probably shouldn't, causing the screenToChart method (which doesn't check for bad values) to "wrap" them back around to the opposite side of the map:

I'm really not sure about the best way to fix that.

[BenjaminHarding]

@Sam-Izdat Great work! Definitely looking forward to looking at this and merging it into the next commit.

[Sam-Izdat]

Thanks - happy to be of use.

I think grunt is misbehaving on windows, though, so I may have to fix that last commit when I have access to my linux box. The reasoning for the last edit was: multiplying is somewhat faster than dividing and caching should shave off a few cycles. It's probably bordering on micro-optimization, but I figured it'd be best, since this will run in the update loop more often than not.

[BenjaminDRichards]

I've just discovered that we haven't addressed this PR, since @BenjaminHarding moved on to other projects. It looks good and I intend to fully review and merge it at some point in the future. Full disclosure: we're short-handed and super busy with other priorities, so KiwiJS hasn't had the love it needs in a little while. KiwiJS is a key part of those other projects, so we'll get this done sooner or later. It's just taking longer than I like.

I'll probably use git trickery to merge the branch onto v1.5.0, the current working branch, for review. I think that will mean closing this request, so don't be surprised when that happens. I'm keeping it open for now as a reminder, and will of course credit everything in the release.