-
Notifications
You must be signed in to change notification settings - Fork 543
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
Fix: solve teleportation time overflow problem. #4163
base: master
Are you sure you want to change the base?
Conversation
Pro Tip!
If your changes do not fall into any of these categories, don't worry. You can just ignore this message in that case! 👀 |
Slimefun preview buildA Slimefun preview build is available for testing! https://preview-builds.walshy.dev/download/Slimefun/4163/afc73256
|
src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/TeleportationManager.java
Outdated
Show resolved
Hide resolved
…ortationManager.java
src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/TeleportationManager.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some code style stuff
src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/TeleportationManager.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/TeleportationManager.java
Outdated
Show resolved
Hide resolved
…ortationManager.java Co-authored-by: JustAHuman-xD <65748158+JustAHuman-xD@users.noreply.github.com>
…ortationManager.java Co-authored-by: JustAHuman-xD <65748158+JustAHuman-xD@users.noreply.github.com>
Why are we moving it to a long? Aren’t we delaying the issue then, this isn’t solving the issue. should limit it to a value instead of moving the problem in my opinion |
Though it may be more robust, I have no idea what will be happening in future overflow bug. If Besides, I have another subject to work at present, so my solution ends here. |
@J3fftw1 As you wish. |
Description
In TP time calculation,$\rm complexity \times complexity$ may result in int overflow when complexity is higher than $4\times 10^{4}$ , not difficult to achieve with several popular addons.
Compared with original solution based on
Math
function, it is better to avoid overflow directly.Proposed changes
Use
long
to storage temporary speed variable, which hardly overflow in TP time calculation.If$\rm speed > distance$ , time cost must be 1 tick, so we can skip calculation.
Otherwise,$\rm \dfrac {distance}{speed} \geq 1$ , while speed must be in range of
int
.Therefore we can simply convert speed to int variable to calculate ultimate time cost.
Related Issues (if applicable)
Checklist
Nonnull
andNullable
annotations to my methods to indicate their behaviour for null values