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

About OffsetHexMode #178

Closed
lishaoxia1985 opened this issue Aug 16, 2024 · 5 comments · Fixed by #189
Closed

About OffsetHexMode #178

lishaoxia1985 opened this issue Aug 16, 2024 · 5 comments · Fixed by #189
Labels
documentation Improvements or additions to documentation usability Make the APIs easier to use
Milestone

Comments

@lishaoxia1985
Copy link

In the redblobgames.com, it seems that Pointy hex used EvenRows and OddRows, in the other way Flat hex used EvenColumns and OddColumns.
That means we only need to define an Enum like this:

OffsetHexMode {
Odd,
Even,
}
@ManevilleF
Copy link
Owner

Hi @lishaoxia1985 , thanks for the issue. Good catch, I'll think about a good solution to avoid the confusion, either integrate the orientation in the conversion or have explicit docs

@ManevilleF ManevilleF added documentation Improvements or additions to documentation usability Make the APIs easier to use labels Aug 22, 2024
@ManevilleF ManevilleF added this to the 0.20.0 milestone Dec 20, 2024
@ManevilleF
Copy link
Owner

@lishaoxia1985 could you review #189 ?

@lishaoxia1985
Copy link
Author

lishaoxia1985 commented Dec 20, 2024

  1. Do you think Rows and Columns is necessary?
  2. Even and Odd can be as the parameter of fn to_offset_coordinate(), in fact we only use odd grid or even gird at the same time.

@ManevilleF
Copy link
Owner

Yeah It might not make sense, just changed it in the PR

@lishaoxia1985
Copy link
Author

And we should add the necessary comment to describe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation usability Make the APIs easier to use
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants