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

Offset Coordinates produce a reversed grid #112

Closed
altarrok opened this issue Nov 16, 2023 · 5 comments
Closed

Offset Coordinates produce a reversed grid #112

altarrok opened this issue Nov 16, 2023 · 5 comments

Comments

@altarrok
Copy link

altarrok commented Nov 16, 2023

When using the offset coordinates, the grid is mirrored (0 should be leftmost, but it is rightmost column index)

Here is the sandbox that produces it:

https://codesandbox.io/s/three-fiber-hex-grid-forked-ksr9ff?file=/src/App.js

image
The grid rendered, and each hex has the <col, row> numbers on at the top of them

image
This is the grid from redblobgames, with <col, row> numbers on them. Note that the leftmost highlighted hex has col = 0

Wondering if this is an issue on my end, I checked the center positions of the leftmost and the rightmost hex', in the middle row:
<0, 3> (expected to be leftmost, but rightmost in the result): {x: 5, y: 1.5}
<4, 3> (expected to be rightmost, but leftmost in the result): {x: -1.9282032302755086, y: 1.5}

So as one can see, the center positions are incorrect, therefore this is the result of the behavior of the honeycomb library. I checked the API thoroughly, but couldn't find a configuration that would fix this, nor any other previous issues mentioning the problem. This is important for my use case, if you would like to discuss more about the problem please feel free to leave a comment, thank you.

If you run into the same problem and looking for a hack; multiply center.x by -1

@flauwekeul
Copy link
Owner

Hi. I'm quite sure this is a problem on your end 😅 Haven't got the time yet to investigate your code though.

If you implement one of the rendering examples and render or log the offset coordinates, they should be on the expected hexes.

@altarrok
Copy link
Author

@flauwekeul if you missed the part where I specified:

Wondering if this is an issue on my end, I checked the center positions of the leftmost and the rightmost hex', in the middle row: <0, 3> (expected to be leftmost, but rightmost in the result): {x: 5, y: 1.5} <4, 3> (expected to be rightmost, but leftmost in the result): {x: -1.9282032302755086, y: 1.5}

It is not a rendering-related issue, the x positions of the hexes are generated from the honeycomb library, and only after that it is used to render the hexes. Therefore I believe this is an actual issue.

@flauwekeul
Copy link
Owner

This is what I see when I open your code sandbox (Chrome 119):

Screenshot 2023-11-18 at 20 49 21

Maybe I see a different version or you managed to fix it or something? Please let me know when you have code that reproduces the problem.

@altarrok
Copy link
Author

@flauwekeul sorry, I applied my hack fix to it, so it was in the correct way because of the reversed x, if you check now, it should be clear that the center.x out of the library is wrong.

@flauwekeul
Copy link
Owner

I think I figured it out. The problem is hex.center is a rather... confusing property because it always returns the position relative to the top left corner of the hex with coordinates 0, 0. So you set your origin to { x: 5, y: 5 }, but hex.center ignores this and just does this:

  // todo: add to docs that this always returns a point relative to Hex(0, 0)'s top left corner!
  // todo: probably deprecate this, see: https://github.com/flauwekeul/honeycomb/discussions/95#discussioncomment-5158862
  get center(): Point {
    const { width, height, x, y } = this
    return { x: width / 2 - x, y: height / 2 - y }
  }

Copied that from the source code, including the comment.

If you want the actual origin point of each hex (relative to hex with coordinates 0, 0), you can just use hex.x and hex.y. If you change line 133 like so:

-  const { x: originX, y: originY } = hex.center;
+  const { x: originX, y: originY } = hex;

It should all work as you expect.

I'll deprecate hex.center (should've done that earlier...). Sorry for the inconvenience.

flauwekeul added a commit that referenced this issue Nov 20, 2023
It ignores the hex's origin, making it a useless, but especially confusing property. See #112.
github-actions bot pushed a commit that referenced this issue Nov 20, 2023
# [v4.1.5](v4.1.4...v4.1.5) (2023-11-20)

## 🐛 Bug Fixes
- [`cf32269`](cf32269)  Deprecate &#x60;hex.center&#x60; (Issues: [`#112`](#112))

## [4.1.5](v4.1.4...v4.1.5) (2023-11-20)
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

No branches or pull requests

2 participants