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.

Sign up for GitHub

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

Jump to bottom

Fix hexagon geometry #4831

Merged
merged 13 commits into from
Jun 27, 2024
Merged

Fix hexagon geometry #4831

merged 13 commits into from
Jun 27, 2024

Conversation

kwvanderlinde
Copy link
Collaborator

@kwvanderlinde kwvanderlinde commented Jun 11, 2024 •
edited
Loading

Identify the Bug or Feature request

Fixes #2840
Fixes #4830

Description of the Change

A number of underlying problems with hex grid have been fixed:

  • The edge projection was not updated when the cell dimensions were changed, leading to all sorts of discrepancies the more the grid is modified. Now it is always updated when dimensions change.
  • The math in a few places assumed a scaled version of a regular hex while other places assumed an equal edge length. These can't generally be true at the same time, which caused some graphical inaccuracies as well as Hexagonal (Flat Top) Map Grid 2nd Size Input Mutation #2840. These places have now been generalized to work for any shape hex, and we have also committed to not using equal-length edges since it has its own subtleties.
  • The shape cache for hex grids was not being cleared when the grid changed, leading to some odd artifacts depending on when the cache was populated ([Bug]: Hex grid lightting broken if grid is adjusted before any lights are displayed #4830).

With the above, whatever the users sets in the "2nd Size" input is kept and is reflected in the map. Previously, typing in 100 x 400 would change the second size and result in this:
image
Now the 400 is kept and we see this:
image

Some general grid clean up is included as well:

  • Grids, and especially hex grids, used to serialize way too much. Most of the fields are now transient, with only size, offset, and for hex grids also their "aspect ratio" and orientation being serialized. The DTOs now include only these fields as well.
  • Hex grid shapes are now built as a Path2D and filled in as an Area instead of merging together a bunch of individual cells. This avoids precision issues (gaps) without needing to scale up the cell size. It avoids any possibility of holes in the result, and avoids hex grid lights bleeding over the cell edges.
  • The way grid offsets work for hex grids is a little different now. For some reason we were offseting by more than a cell size only for other code to compensate for that. Now we offset by only a fraction of a cell.

Possible Drawbacks

If loading an existing campaign with really stretched out hex grids, snap-to-grid tokens will not end up in the same cell as before. Since the cell dimensions were not accurate before, the conversion between a tokens real position and its cell has changed.

Documentation Notes

N/A

Release Notes

  • Fixed a bug where the 2nd Size for hex grids would not remain as the user set it.
  • Fixed a bug where grid lights on adjusted hex grids could contain gaps.

This change is Reviewable

@kwvanderlinde kwvanderlinde self-assigned this Jun 11, 2024
@kwvanderlinde
Fix edge projection for stretched hex grids
727629d 
The edge projection was not always recalculated when the dimensions change. On top of that, it is not always half the
edge length - that's only for regular hexagons. Now we always recalculate edge projection and edge length together, and
relate them to the diameter propertly.

There was one place where edge length was used where the diameter should have been used.

Also remove a bunch of unused and commented out code, and caches for things that don't need to be cached.
@kwvanderlinde
Mark Grid fields as transient
e90a627 
The movement keys, zone, and cell shape have no business being serialized. The cell shape was also removed from the
DTO to match this.
@kwvanderlinde
Change responsibilities: Grid.createGridArea() is no longer responsib…
5a108b6 
…le for calling setGridShapeCache()
@kwvanderlinde
Clear hex grid cache on changes too by not using the field directly
011304b
@kwvanderlinde
For hex grids, define scaled area in terms of the boundary
0b02e18 
No more unioning up areas or need for hacks to avoid meshing artifacts. We make one path that goes around the entire
boundary, then convert that into a single area.
@kwvanderlinde
Avoid int truncation when making hex shapes
8641d34
@kwvanderlinde
Avoid mild precision issue in hex grid offsets
4f861f1 
Previously we were offseting by 3/8 on the U axis, but this is only correct for regular hexagons. We now express this
more generally in terms of edge length and projection. FIXME

Also some clean up:
- One implementation for both types of hex grids
- Don't offset by more than a grid cell. It's unnecessary and means we need to do so everywhere else too.
@kwvanderlinde kwvanderlinde force-pushed the bugfix/2840+4830-hex-grid-consistency branch from 874b32a to f4832fc Compare June 11, 2024 17:55
@kwvanderlinde
Fix horizontal vs vertical offset coords; they got mixed up when movi…
5e26705 
…ng the implementation into HexGrid
@kwvanderlinde
Remove obsolete note about grid offsets
93218d9
@kwvanderlinde
Grid.createCellShape() is now only for the current size
008973a 
Hex grids already ignored the `size` parameter and for square grids it was only ever called with the current size
anyways. Isometrics grids used it, but they can have their own private overload for that.
@kwvanderlinde
Switch to scaled hexagons instead of equal-length edges
f116809
@kwvanderlinde
Fix loading of existing campaigns to calculate shape after dimensions…
afc988d 
… are set
@kwvanderlinde
Horizontal and vertical hex grids do their own grid offsets
862f67c 
With scaling in play, we can't rely on the width and height being larger or smaller than the other.
@kwvanderlinde kwvanderlinde force-pushed the bugfix/2840+4830-hex-grid-consistency branch from 5132563 to 862f67c Compare June 11, 2024 19:10
@kwvanderlinde kwvanderlinde marked this pull request as ready for review June 11, 2024 20:33
@cwisniew cwisniew added this pull request to the merge queue Jun 27, 2024
Merged via the queue into RPTools:develop with commit 2fae865 Jun 27, 2024
4 checks passed
@kwvanderlinde kwvanderlinde deleted the bugfix/2840+4830-hex-grid-consistency branch June 28, 2024 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cwisniew cwisniew cwisniew approved these changes

Assignees

@kwvanderlinde kwvanderlinde

Labels
bug
Projects
MapTool Current Dev and Test
Status: Merged
Milestone
No milestone
2 participants
@kwvanderlinde @cwisniew