CloudburstMC / Cloudburst

Cloudburst Server is a Bedrock first server software. Originally forked from Cloudburst Nukkit.
178 stars 70 forks source link

Mathematical error #100

Closed AlLiberali closed 3 years ago

AlLiberali commented 3 years ago

Return value of org.cloudburstmc.server.player.Player's distance method (line 724 stable, 725 bleeding) should be (int) Math.sqrt(dx dx + dz dz); Also why are you using integer as returning type of a entity distance method use float at least

DaMatrix commented 3 years ago

That method is actually never used, it's probably left over from the legacy Nukkit code.

Also it probably isn't wrong. Using the squared distance is quite common when you only need to compare which distance is longer and don't need to know how long it actually is. sqrt() is expensive. Although you are correct, the method should be returning double, and probably renamed to distanceSq.

AlLiberali commented 3 years ago

Random fact: Java's Math.sqrt() and math.h's sqrt() (glibc) both use IEEE 754 sqrt (glibc first tries kernel's method) method so Java has almost the same speed of C (glibc) in terms of calculating square root. [1] [2]

Also Java uses native code through JNI for its default math implementation (StrictMath.c) so it should be same with glibc in terms of memory usage

Creeperface01 commented 3 years ago

I think the point is that even the fastest sqrt implementation is still considerably slower than the other common operations

AlLiberali commented 3 years ago

I think the point is that even the fastest sqrt implementation is still considerably slower than the other common operations

"Random"