romlok / godot-gdhexgrid

A GDScript hexagonal grid implementation for Godot.
MIT License
195 stars 24 forks source link

Fix calculation of cube to offset #5

Open db0 opened 5 years ago

db0 commented 5 years ago

According to the guide, the conversion of cube to odd-q offset uses the z-axis, not the y-axis. I spent a while yesterday trying to debug why godot was not aligning the hex correctly and I noted you're using the wrong axis.

romlok commented 5 years ago

Thanks for the report. It's been a while since I dealt with the implementation details, but are you sure this isn't due to the explicit design decisions (use Y instead of Z, and to flip the Y axis) which diverge the library implementation from the guide?

Then again, I haven't used offset coords in a project, so I could easily have gotten carried away with the z=>y change. Can you show what behaviour you expect (eg. screenshot or demo project), and what the library currently gives instead?

db0 commented 5 years ago

I'll do you one better. Checkout my hexgrid-tilemap implementation demo (based on your code), and see how when clicking it correctly tranforms the tile below the hex. Now change the offset calculation back to the y-axis, and run it. You'll see that as your hex highlight moves down the the screen, the modified tiles happen in the opposite direction.

romlok commented 5 years ago

Sorry about the delay. I've finally had time to look a bit deeper into this, and I still believe that the implementation is working as intended. However, the issue appears to be that my chosen assumptions for the offset grid don't line up well with the implementation or currently available half-step options of TileMap, which is unfortunate.

I did get it working with my original implementation by:

  1. Inverting the Y value when passing the offset coords to TileMap.set_cellv (because TileMap has +y as down, HexCell has +y as up)
  2. Changing from HALF_OFFSET_Y to HALF_OFFSET_NEGATIVE_Y (only introduced in the upcoming Godot 3.2)

I believe right now that the best solution would be to change HexCell's decision about offset coordinates, and have odd columns shifted down half a step, instead of up. This would make it compatible with the existing HALF_OFFSET_Y option of TileMap (providing Y is flipped, as above), at the cost of breaking backward-compatibility.