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

Coordonate.trueDistanceTo (used by transporter) returns NaN at long distance #295

Closed
perkinslr opened this issue Sep 25, 2015 · 3 comments
Closed
Assignees
Milestone

Comments

@perkinslr
Copy link

Math.sqrt chokes on values above 46340, so the transporter fails to 'get a lock' when pointed at a second transporter far away. The solution is to use Math.hypot instead of Math.sqrt. I've fixed that and comitted it to my fork (release branch), but I don't know how to make a pull request with just that commit.

@Baughn
Copy link
Contributor

Baughn commented Sep 25, 2015

Make a separate branch, from the 1.7.10 branch here, then cherry-pick that commit into it. Then make a PR.

@Baughn
Copy link
Contributor

Baughn commented Oct 29, 2015

Did you end up getting the PR done?

@cm0x4D cm0x4D added the bug label Jul 7, 2016
@cm0x4D cm0x4D self-assigned this Jul 7, 2016
@cm0x4D
Copy link
Member

cm0x4D commented Jul 26, 2016

Math.hypot() is way to slow (about 100x - 200x slower than the actual implementation).

The problem is actually not the Math.sqrt() method itself, it is rather the fact that an 32 bit integer can only hold numbers up to 2'147'483'647 (2^31-1), so given delta values bigger than 46340, the square surpasses the capacity of the data type and finally the method returns a wrong result.

The fix is as simple as to change the int datatype for the distance deltas to long as in the worst case the distance will not surpass 2^31 * 2^31 = 2^62, which still fits in a 64 bit integer (2^63).

cm0x4D pushed a commit that referenced this issue Jul 26, 2016
@cm0x4D cm0x4D closed this as completed Jul 26, 2016
@metc metc added this to the 1.12.0 milestone Oct 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants