Skip to content

refactor: adjacent hexes, remove clockwise argument, closes #2470#2471

Merged
DreadKnight merged 7 commits intoFreezingMoon:masterfrom
andretchen0:adjacent-hexes
Aug 8, 2023
Merged

refactor: adjacent hexes, remove clockwise argument, closes #2470#2471
DreadKnight merged 7 commits intoFreezingMoon:masterfrom
andretchen0:adjacent-hexes

Conversation

@andretchen0
Copy link
Contributor

@andretchen0 andretchen0 commented Aug 4, 2023

As discussed here, this closes an 8-year-old TODO. As mentioned, the single caller using the clockwise argument doesn't make much sense in the context of a game with players facing opposing direction. So the clockwise argument was removed.

That's my take, admittedly. Feel free to comment/disagree, as always.

Otherwise, the function eliminates reliance on the stateful Hex class. It still requires a bit of state to determine what's in- and out-of-bounds. HexGrid holds that information, but it previously required a Demeter violation to access – reading into length of some arrays. That's been added instead as a method as HexGrid.isInBounds.

Meta

This uses elements included in this PR. As it's a separate issue, I've open this as a separate PR.

@vercel
Copy link

vercel bot commented Aug 4, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
ancientbeast ✅ Ready (Inspect) Visit Preview Aug 4, 2023 10:55pm

@ghost
Copy link

ghost commented Aug 4, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@andretchen0 andretchen0 marked this pull request as ready for review August 4, 2023 22:43
@andretchen0 andretchen0 changed the title refactor: adjacent hexes, remove clockwise argument refactor: adjacent hexes, remove clockwise argument, closes #2470 Aug 5, 2023
@DreadKnight DreadKnight merged commit b040676 into FreezingMoon:master Aug 8, 2023
@andretchen0 andretchen0 deleted the adjacent-hexes branch August 8, 2023 14:42
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

Successfully merging this pull request may close these issues.

2 participants