Closed mgrider closed 2 years ago
This is a draft because it includes #11, so that'll need to get merged before this can be reviewed on its own.
So far it looks fine. Would you mind to add also tests for newly introduced fitGrid()
function? Maybe a few words about this functionality in readme file would be nice as well.
Thank you.
Yes! I will add some tests in the next few days. 💯
@fananek This is ready for review again.
I added a test with a few values.
I also fixed a bug in updatePixelDimensions()
where a minimum X or Y value might be greater than zero, so zero was reported as the minimum incorrectly. This was happening with triangle grids where the orientation was .flatOnTop
. I didn't write a test for it, but I could imagine that as a nice additional enhancement. There are probably a lot of edge-case values we could (should?) add to the test suite.
As an aside, (and this deserves it's own issue, which I'll be adding later), I don't think the rectangle style grid creation functions are correct. I haven't looked into it yet, but you can see the problem more easily now in my SKHexGrid project.
Note that the head of SKHexGrid
is broken unless you use this branch for its package dependency (which you can do by dragging a checkout of this branch into the project).
[Edited to remove commit message from already merged commit.]