It would be nice to store some user info along with each cell so that I don't need to store a hash of additional cell info outside of HexGrid since I am already using HexGrid to find the cell I want based on its pixel or cube coordinates.

Something as simple as the ubiquitous public var userInfo: [AnyHashable: Any] would do the trick.

Similar to #2, it would be nice if we could get at the width & height properties in HexSize. (I'd like to be able to create a HexSize+CGSize extension similar to the one in the example for Point+CGPoint.)

The comment for the radius parameter says 0 will produce a single hexagon, but a value of 0 actually throws an InvalidArgumentError.

I also think the doc comments for GridShape cases could use expanding, maybe with some examples. For instance, the .hexagon case just says "radius", but I think it's more common to talk about regular hex grids by "side length" (which is of course "radius + 1"). At least I understand the meaning of radius, but I don't really understand what the .triangle's "side size" parameter means, because it seems to produce a triangle with a side-size of one more than that. (For consistency?) Maybe the triangle parameter is actually a bug, I don't really know.

Library Performance

I just want to get a discussion started here around the topic. For my use (a 2-player abstract strategy game with an AI opponent), I need to call some functions on this library hundreds (maybe thousands) of times per second. In general, most of these calls are re-creating their return values every time, even though they are always the same as long as the set of Cell objects hasn't changed. For those functions, the return value can be cached to gain massive speed-up, which I would like the library to do.

Some cache examples

allCellsCoordinates()

This function was taking up the second most time in my project, and currently looks like this:

    public func allCellsCoordinates() -> Set<CubeCoordinates> {
        return Set(self.cells.map { $0.coordinates })
    }

My initial instinct was to use a standard for in loop to improve the speed, like this:

    public func allCellsCoordinates() -> Set<CubeCoordinates> {
        var  set = Set<CubeCoordinates>()
        for cell in cells {
            set.insert(cell.coordinates)
        }
        return set
    }

I wrote the following tests:

    func testAllCellCoordinatesMap() throws {
        let grid = HexGrid(
            shape: GridShape.hexagon(20))
        measure() {
            for _ in 0...10000 {
                _ = grid.allCellsCoordinatesMap()
            }
        }
    }

    func testAllCellCoordinatesUncached() throws {
        let grid = HexGrid(
            shape: GridShape.hexagon(20))
        measure() {
            for _ in 0...10000 {
                _ = grid.allCellsCoordinatesUncached()
            }
        }
    }

The original (.map version) runs in 2.522 sec, and the second almost twice as fast at 1.803 sec. Oddly enough, when I search for whether .map is slow in swift, the consensus in various articles seems to be that it's actually faster than a standard for in loop, so I'm not sure exactly what's happening here. It occurred to me shortly after the re-write that I should use a cache variable for this value (as I'd already done for the primary bottleneck, which I'll describe next).

That function now looks like this:

    /// A cache variable for all coordinates in all cells.
    private var cacheAllCellsCoordinates = Set<CubeCoordinates>()

    /// Coordinates of all available grid cells
    /// - Returns: `Set<CubeCoordinates>`
    public func allCellsCoordinates() -> Set<CubeCoordinates> {
        if !cacheAllCellsCoordinates.isEmpty {
            return cacheAllCellsCoordinates
        } else {
            var  set = Set<CubeCoordinates>()
            for cell in cells {
                set.insert(cell.coordinates)
            }
            cacheAllCellsCoordinates = set
            return set
        }
    }

With a similar test to the ones above, (running it 10,000 times), it takes 0.003 sec, and is no longer a noticeable function when running in the profiler.

neighborsCoordinates(for coordinates:)

As I said, this function was the primary bottleneck in my AI code (after a bit of refactoring to use coordinates everywhere instead of cells--which I needed to do anyway). Here's how it looked originally:

    public func neighborsCoordinates(for coordinates: CubeCoordinates) throws -> Set<CubeCoordinates> {
        return try Math.neighbors(for: coordinates).filter { isValidCoordinates($0) }
    }

I decided to implement a cache solution right away, and that looks like this:

    /// A cache/LUT variable. The thinking here is, unless our `Cell` set changes, the neighbors are always going to be the same.
    private var cacheNeighborsCoordinatesForCoordinates = [CubeCoordinates: Set<CubeCoordinates>]()

    /// Get all available neighbor coordinates for specified coordinates
    /// - Parameter coordinates: `CubeCoordinates`
    /// - Returns: `Set<CubeCoordinates>`
    /// - Throws: `InvalidArgumentsError` in case underlying cube coordinates initializer propagate the error.
    public func neighborsCoordinates(for coordinates: CubeCoordinates) throws -> Set<CubeCoordinates> {
        if let neighbors = cacheNeighborsCoordinatesForCoordinates[coordinates] {
            return neighbors
        } else {
            let neighbors = try Math.neighbors(for: coordinates).filter { isValidCoordinates($0) }
            cacheNeighborsCoordinatesForCoordinates[coordinates] = neighbors
            return neighbors
        }
    }

I also added a new function to invalidate the cache variables that gets called after updatePixelDimensions in the cells didSet:

    /// This gets called whenever the `Cell` set is updated.
    private func invalidateCacheVariables() {
        cacheAllCellsCoordinates = Set<CubeCoordinates>()
        cacheNeighborsCoordinatesForCoordinates = [CubeCoordinates: Set<CubeCoordinates>]()
    }

I wrote some tests for this as well, you can see them on my fork. I've commented out the original test, because even running the loop (calling it on all the cell coordinates in a hex20 grid 10 times), takes longer than I want to wait. (More than a few minutes.)

alternatives / discussion

I can imagine some legitimate reasons you might object to these sorts of changes in the library, so I wanted to outline the changes I've already made before I tackle any others. (For my purposes, it may already be "fast enough", but some additional testing will be needed.) I do feel like, if some function return values are cached, maybe as many as can be should be. That's the main reason I'm creating this issue. Should I go ahead and write cache variables for the others where it makes sense?

If you aren't interested in this type of improvement, then I will definitely not bother until I need it. (And in that case, I'd probably just maintain my own fork of the library for my own use.

I was just looking at #12 from last year, and realized I never addressed your request to add some details to the README. This issue is just to state and/or discuss my intention to retroactively do that.

I will write some details about that function, as well as how drawing is impacted by some of the the other properties and initializers, mostly likely putting it all under the "Drawing related functions" section. (I might also drop "function" from that header.)

Firstly, thanks for the great work!

I came across this library recently and tried to use it to render a hex grid, but the result was not correct, the hexagons cells were overlapping.

Going back and forth between the doc from redblobgames.com and your source I noticed there is maybe a mix up between the layout size and the hexSize used for calculating hexCornerOffset.

The original doc says : "Note that size.x and size.y are not the width and height of the hexagons."
https://www.redblobgames.com/grids/hexagons/implementation.html#hex-to-pixel

Unfortunately, that's exactly what happens in hexCornerOffset, the given hexSize is used as is, so somehow each hexagon gets it's width doubled.

I fixed the rendering of my hexGridView class by updating hexCornerOffset to :

fileprivate static func hexCornerOffset(at index: Int, hexSize: HexSize, orientation: Orientation) -> Point {
let angle = 2.0 * Double.pi * (
OrientationMatrix(orientation: orientation).startAngle + Double(index)) / 6
return Point(x: hexSize.width/2 * cos(angle), y: hexSize.height/2 * sin(angle))
}

Where hexSize.width and hexSize.height are simply divided by two.

Thanks again.
Take care.

Problem
When drawing a grid, it would be nice to "fit" the grid inside a specific CGSize.

Ideal solution
Something like:

let hexGrid = HexGrid(shape: GridShape.hexagon(2), gridSize: CGSize(width: 200, height: 200))

Describe alternatives you've considered
Currently, if I want to "fit" the grid to a size, I need to do the math to figure out how many across and divide the total width by that number, do the same for height, and then I take the smaller number and use that. The new initializer could do something similar, but I'm not even sure how to do that for all the possible grid types, so it might also be nice to have functions that return the max number of hexes horizontally, or vertically, and possibly get the hex "farthest" in a given direction, maybe?

Description
Double check coordinate conversions works as expected. Especially focus on Offset, Axial and Cube coordinate systems.

• Fix any potential issue.
• In case of any change in converter, grid generators will probably need update as well.
• Update unit tests accordingly.

Issue relates to #13

Describe the bug
As mentioned in #12, when generating cells using the .rectangle GridShape, the resulting grid appears rotated from what is expected in 3 out of 4 possible combinations of Orientation and OffsetLayout.

To Reproduce
It is probably easiest to reproduce this behavior by drawing/visualizing the resulting grid. Here are some screenshots generated using the SKHexGrid project. The coordinates shown are Axial.

Orientation: .pointyOnTop, OffsetLayout: .even

Orientation: .pointyOnTop, OffsetLayout: .odd

Orientation: .flatOnTop, OffsetLayout: .even

Orientation: .flatOnTop, OffsetLayout: .odd

Expected behavior
You can see from the screenshots that only the .flatOnTop, .even layout appears to be correct.