Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for irregular grids #3

Merged
merged 1 commit into from
Dec 29, 2019
Merged

Conversation

jaylorch
Copy link

This adds support for irregular grids, as found in the examples shape and statue_park_loop.

It changes the interface to use Point and Vector instead of generic tuples. This should also help with a planned addition of support for hexagonal grids, and perhaps even later support for three-dimensional grids. All examples have been updated to use the new interface, including in the tutorial. (However, I haven't tested the tutorial, as I'm not sure the right way to do that.)

Note: There are a few "too-many" pylint warnings, which I'm not sure what to do about. Perhaps we want to disable these types of warnings globally?

@obijywk
Copy link
Owner

obijywk commented Dec 29, 2019

I'll disable R0902 (too-many-instance-attributes)... that one seems annoying to work around.

For R0914 (too-many-locals) I usually think of that as an indication that we're doing too much inside of a single function, and should introduce some helper functions. It's usually pretty easy to fix by doing that. So I think I'd prefer to leave that on, and we can refactor the examples that are running into this limit to not do everything inside of main(). I think let's merge this for now, though, and fix that in a follow-up (#6 ).

In general this looks great! Thanks for fixing up all the code to support this. I made a few personal-style-preference-ish cleanups (and fixed a few typing comment mistakes) that I'll push as I merge this.

I created a few more issues (#4 #5 ) for cleanups I think we should make as a follow-up to this. The mostly-mechanical changes you made to the examples to support this are the right way to start, but I think we can now go back and simplify the code for many of the examples now that they have a locations list to work with.

@obijywk obijywk merged commit 3125ccb into obijywk:master Dec 29, 2019
@jaylorch jaylorch deleted the irregular-grid branch December 29, 2019 20:12
@jaylorch
Copy link
Author

Thanks! My next plan is to replace the location set (which is currently passed in as just a list of points) with an abstract Grid class. This way, using subclasses RectangularGrid, FlatToppedHexGrid, and PointyToppedHexGrid, it would represent a not-necessarily-rectangular grid. The idea is that the Grid would have its own methods for adjacency, rotation, etc. So that seems like a good place to put the print routine to fix issue #4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants