LambdaSix / InfiniMap

InfiniMap - (Nearly) Infinite Maps
MIT License
27 stars 1 forks source link

_chunkHeight and _chunkWidth are used inconsistently across X and Y dimensions. #11

Closed james0x0A closed 6 years ago

james0x0A commented 7 years ago

Common outside usage suggests that X corresponds to width, and Y or Z to height but constructor order and component order in the coordinate spaces suggest that this project maps "Height" to the X dimension, "Width" to the Y dimension, and "Depth" to the Z dimension.

With that in mind, ChunksWithin seems to be incorrect,. Width and Height should be swapped. https://github.com/LambdaSix/InfiniMap/blob/master/InfiniMap/ChunkMap.cs#L238-L289

Since the problem only shows up when chunk width and height are different, it wasn't caught by the current tests.

LambdaSix commented 7 years ago

For the item/world/chunk space coordinates, they're all Points so don't really concern themselves with the notion of width/height/depth, though notionally they follow normal desktop screen coordinates; X being 0..n across the screen, Y being 0..n down the screen and Z being 0..n out of the screen, ergo, right hand coordinate system.

I tend to use named parameters so I didn't really notice this as such, but looking over the code I agree that it probably makes more sense that given the various space constructors using X/Y/Z, ChunkMap should follow that more globally and use Width/Height/Depth.