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

First version of zone support #12

Merged
merged 7 commits into from
May 7, 2022
Merged

First version of zone support #12

merged 7 commits into from
May 7, 2022

Conversation

pchesneau
Copy link
Collaborator

@pchesneau pchesneau commented May 5, 2022

I Recreate a MR to simplify Rebases.
This is a draft (yet effective) implementation for #3 .

What is done :

  • Fix rounding issues ( at least for external points of hexagons)
  • Detects clusters
  • Draws boundaries around clusters.

However there is still some issues :

  • the drawing algorithm is implemented in hexamap.py yet it would be better in HexagonGrid; but it cannot be placed there as it would add an impossible circle dependency between Hexagon and HexagonGrid.

Is it really a good idea to have Hexagon know it's grid? I would have expected the opposite. (In fact I half expected to see HexagonGrid implement the factory pattern to produce hexagon. (so that every Hexagon is registered in it's grid. )
@Ezian WDYT ?

@pchesneau
Copy link
Collaborator Author

Hello @Ezian ,
I've rebased + done some refactor. I still have to add some documentation.

Also, it seems there is an offset issue with the current version, are you aware ?

@Ezian
Copy link
Owner

Ezian commented May 6, 2022

What do you mean ?

Coordinate are correctly displayed in the main branch. Which offset do you are talking about ?

@pchesneau
Copy link
Collaborator Author

Copy link
Owner

@Ezian Ezian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really enjoy to read this PR, thanks !

I have few feedback, mostly improvement.

Moreover, before merging, it would be great to add:

  • Example in documentation (and documentation BTW)
  • Regenerate the hexgrid-example.svg which is shown in the Readme.md.

I have in mind to write a Contribute.md file in order to explicit this rules.

classes/hexagon.py Show resolved Hide resolved
classes/hexagon.py Show resolved Hide resolved
classes/hexagon.py Show resolved Hide resolved
classes/hexagon.py Outdated Show resolved Hide resolved
classes/hexagon.py Outdated Show resolved Hide resolved
classes/hexagon.py Outdated Show resolved Hide resolved
classes/hexagon.py Outdated Show resolved Hide resolved
classes/hexagon.py Outdated Show resolved Hide resolved
hexamap.py Outdated Show resolved Hide resolved
@Ezian
Copy link
Owner

Ezian commented May 6, 2022

Have a look to https://github.com/Ezian/hex-chronicle/blob/main/hexgrid-example.svg ;)

Huho shit. I had introduce it when I take Pylint stuff in account...

@Ezian
Copy link
Owner

Ezian commented May 6, 2022

I've botched the compute of the grid.origin point...

@Ezian
Copy link
Owner

Ezian commented May 6, 2022

Fixed.

@pchesneau pchesneau marked this pull request as ready for review May 6, 2022 22:24
@pchesneau pchesneau linked an issue May 6, 2022 that may be closed by this pull request
Closed
@Ezian Ezian merged commit 3af0eb0 into Ezian:main May 7, 2022
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.

Zones
2 participants