swap s2geometry-node (C++) for s2-geometry (pure javascript)

[coolaj86]

All necessary changes for implementing s2-geometry in pure javascript.

Tested in known locations and compared to s2geometry-node results

• https://github.com/coolaj86/s2-geometry-javascript/blob/master/tests/prev-next.js
• https://github.com/coolaj86/s2-geometry-javascript/blob/master/tests/example.startup.txt

Addresses issues #147 #96 #70 #63 #64 #60

@cokeeffekt @d-pollard @breandr @hunterjm @dotomaz @billyriantono @Armax

Try it out now

Pre-packed instructions that even your sister can figure out (probably):

• https://github.com/Daplie/pokemap-webapp
• help wanted someone please test the windows batch script

Simple copy-paste instructions for techies, like you:

• https://github.com/Daplie/node-pokemap

[cokeeffekt]

Legend

[billyriantono]

Really Cool 👍

[Toeler]

That library doesn't produce the correct cell for me. -43.5261282,172.6561085 produces the cell: 8678661352471920640

which is in the middle of the pacific ocean according to s2map.com. It should be around this location.

[brettstack]

I'm using this PR and works for me. Thanks!

On Fri, 29 Jul 2016, 05:18 Toeler, notifications@github.com wrote:

That library doesn't produce the correct cell for me. -43.5261282,172.6561085 produces the cell: 8678661352471920640

which is in the middle of the pacific ocean according to s2map.com. It should be around this location http://s2map.com/#order=latlng&mode=point&s2=false&points=%7B%0Alat:-43.5261282,%0Alng:172.6561085%0A%7D .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Armax/Pokemon-GO-node-api/pull/165#issuecomment-236165934, or mute the thread https://github.com/notifications/unsubscribe-auth/ABy6lwL6cl94tKc0CfHqCWipsT9dHrjhks5qae-XgaJpZM4JW6cR .

[coolaj86]

s2geometry-node produces '7868221231537848320' '3/122120301030023' '-43.525166,172.655096'

s2-geometry v1.2.1 produces '8678661352471920640' '7/00320121210021'

It's either a padding bug or an issue with the GPS conversion (an overflow or wrap perhaps). 7 isn't a valid face.

I've opened an issue here https://github.com/coolaj86/s2-geometry-javascript/issues/6 and I'll probably be able to find and fix it very shortly.

Thanks for the report! :D

[coolaj86]

The good news is that there was a very simple padding issue, fixed here: https://github.com/coolaj86/s2-geometry-javascript/commit/e461dcfd5c2adf543a90b7e5a4b8cf0ecbfe8392

The bad news is that there's also an issue with the Lat/Lng conversion, which is a little deeper, down in code that I didn't write.

I'll see what I can do. :)

[coolaj86]

@Toeler Can you use another library to figure out what the IJ, UV, and ST values should be at that location?

I get 32243,13474 for ij. If that's not correct, that's probably our problem. If it is correct, I'd have to see the UV and ST and compare those.

I was able to see that the Face and XYZ conversion is consistent with s2geometry-node, so those aren't the problem.

Update:

I modified https://github.com/golang/geo to print out the debug values:

f 3
u 0.9576576934048505
v -0.12889961842252406
i 1056555009
j 441532418
lat lng: -43.525166   172.655096
uint64 7868221231537848320
quadkey 3/122120301030023
token 6d318985c

[Toeler]

I don't have any other S2 libraries that work, just your javascript one.

[raz3r]

Using npm install I still see s2geometry-node, is it because of the issue with s2-geometry?

EDIT: Nevermind, using https://github.com/Daplie/Pokemon-GO-node-api.git#master as repository solved the issue.

[coolaj86]

@raz3r There is an issue though https://github.com/coolaj86/s2-geometry-javascript/issues/8 and I would like some help with it.

It's something any code monkey can solve, it just requires the endurance of console.log() and referencing an implementation that works (geo/s2/latlng.go and geo/s2/cellid.go).

[syrys]

@coolaj86 hey there. Im curious, if the "implementation that works" that you linked to before (https://github.com/golang/geo/blob/master/s2/cellid.go), if this code works perfectly fine, can we not just import that as a library and execute that code directly from node?

By using something like this (theres probably other ways of doing it too): https://www.npmjs.com/package/gonode

[coolaj86]

@cokeeffekt @Toeler @Armax @billyriantono @breandr I fixed the bug in my library. This is ready to merge as of s2-geometry v1.2.6.

I've exhaustively tested every location on earth at level 15 and all generated values exactly match with s2geometry-node: https://github.com/coolaj86/s2-geometry-javascript/blob/master/tests/exhaustive.js

[coolaj86]

This was the fix: https://github.com/coolaj86/s2-geometry-javascript/commit/68274eb677be0338984b0c7d59290ec323c47e67

Aside from the original padding error, it can essentially be boiled down to a very simple change:

// Faces 0, 2, and 4 start at 'a'
// Faces 1, 3, and 5 start at 'd'
var currentSquare = (face % 2) ? 'd' : 'a';

No big deal though... only half of the earth was getting wrong positions when it was hard-coded to 'a'... hahaha

[segura2010]

@coolaj86 i just tried your s2-geometry fork and it seems to work! Is it ready to use??

Good job!! :D

[coolaj86]

@segura2010 Yes, this is ready to merge.

I've tested over 2 million random locations which all have passed.

The only bug reports I've gotten since the update were people using old versions.

[coolaj86]

@syrys the point is to get away from C++ complied code and use pure JavaScript to reduce complexity of the install. Otherwise using s2geometry-node is fine.

[d-pollard]

@coolaj86 just for clarification - this will completely remove the need for the C++ dependency for pure javascript (tested and working)? Would like to know before I swap out for my repo

[st0ffern]

@d-pollard it does, we already use it here https://github.com/stoffern/pokemongo-api and it works completely fine. The only problem left is the calculation of neighbour cells, it gets wrong cells. as we have the right ones. it can be tested here: s2map.com

[cpainchaud]

I totally vote for it! please merge and let's get rid of non native one

[coolaj86]

@stoffern check the new README and see if that usage of S2.latLngToNeighborKeys(lat, lng, level); works for you now. If not, please open an issue at https://github.com/Daplie/s2-geometry.js.

[st0ffern]

We have our own calculation for getting the neighbour cells

[coolaj86]

What's the holdup on getting this merged in?

All necessary functionality has been implemented. (the neighbor cell calculation is an additional feature which does not affect this project - and has also been updated, so it should work now anyway)

[billyriantono]

your commit is conflict need to resolve first 😄

[brettstack]

Would be easier to just send new PR

[brettstack]

Big +1 on getting this merged. I'm working to get this 100% client-side compatible. It requires users to install a small browser extension to bypass CORS and set Origin header to 'niantic', but seeing as we've already seen Niantic block the major cloud providers, it's likely they'll soon continue with blocking the smaller ones. I have it working to the point of the second request in the PTC auth flow, but this is returning a 200 with no location header as opposed to the 302. If anyone has any ideas on that issue, I'd love to hear it. :)

[coolaj86]

Oh, it looks like the branches got crossed. It's showing commits from my other PR.

Here it is: https://github.com/Armax/Pokemon-GO-node-api/pull/222

[coolaj86]

I also just did a force push to this original PR which also fixed the merge conflict.

[coolaj86]

@breandr do you have a repo for the browser-only fork? Or is it commercial?

[brettstack]

Can't remember if this includes all the latest? I was just hacking, waiting for this PR to get merged as it's obviously a big part of it.

https://github.com/breandr/pokemon-go-extension https://github.com/breandr/pokemon-go-client https://github.com/breandr/pokemon-go

Once this is merged, I will clean up and send PR to this repo (need to get auth working as well). I'll need to find an alternative to fetch which I don't think works with Node (and request doesn't work with client).