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

refactor: hashing, offsetNeighbors #2468

Merged
merged 2 commits into from
Aug 8, 2023
Merged

refactor: hashing, offsetNeighbors #2468

merged 2 commits into from
Aug 8, 2023

Conversation

andretchen0
Copy link
Contributor

@andretchen0 andretchen0 commented Aug 3, 2023

This adds a few useful functions to const.ts: hashOffsetCoords and offsetNeighbors. It also puts the hashing function to use in pointFacade.

Hash details

I had previously implemented a private hashing function in pointFacade. But I thought it could be useful elsewhere, so I moved it to const.ts.

An example useful case: sets. Sets are fast and practical. But they don't work with our Hex representation out of the box:

s.add({x:1, y:1})
s.has({x:1, y:1}) // false 

That's because objects in sets are compared by reference. However, if we hash the Hex to a value, it can be compared to other hashed values in the set:

s.add(hash({x:1, y:1}))
s.has(hash({x:1, y:1})) // true 

I also refactored the hash function. Previously, it was {x:number, y:number} => string. Now it's {x:number, y:number} => number.

Note

The hashing function uses 16 bits for x, 16 bits for y. (Bitwise operations in JS are limited to 32 bits.) That gives us a max hex x of 65535, and a max hex y of 65535. That should be more than enough – the current maps have a max x of 15 and max y of 8 – but I thought it was worth pointing out, as it's a hard limit.

Fwiw, this happens to be the same hashing scheme that Wesnoth uses for locations.

@vercel
Copy link

vercel bot commented Aug 3, 2023

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

Name Status Preview Updated (UTC)
ancientbeast ✅ Ready (Inspect) Visit Preview Aug 3, 2023 8:53pm

@ghost
Copy link

ghost commented Aug 3, 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 3, 2023 21:03
@DreadKnight DreadKnight merged commit 683b5f4 into FreezingMoon:master Aug 8, 2023
3 checks passed
@DreadKnight
Copy link
Member

Good to know the info in the note 👍🏻

@andretchen0 andretchen0 deleted the hashing 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