Skip to content

Conversation

@douglasg14b
Copy link

Not sure how to do two different pull request for two different things, but this request is dependent on #39


Adds two new methods:

  • getNeighbors()

    • Gets the 8 tiles surrounding the provided tile
  • tilesToFeatureCollection()

    • Produces a simple GeoJSON FeatureCollection for an array of tiles

    Let me know if these are additions you are willing to accept, hopefully these are not gratuitous.

@mrnix
Copy link
Contributor

mrnix commented Oct 21, 2021

👍

const z = tile[2];

return [
[x - 1, y - 1, z], // NW
Copy link
Contributor

@mrnix mrnix Oct 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it doesn't check for valid tiles.
what about [-1, -1, 3]

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's been quite some time since I touched this and I've lost all familiarity. Do you already have a function that checks for valid tiles? If not, can you define what such a check might look like?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remember correctly many other functions don't perform these checks (I assume for performance reasons), Is there a reason this function is special in that regard?

Copy link
Contributor

@mrnix mrnix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, you should add a checks for valid tiles

@douglasg14b
Copy link
Author

@mrnix Update on this PR & review?

@douglasg14b
Copy link
Author

@mrnix ?

@douglasg14b
Copy link
Author

Apparently I have to submit a review completely to comment on a review on github? Hopefully my comments are visible now...

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