-
Notifications
You must be signed in to change notification settings - Fork 545
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 Arithmetic Error #3541
Fix Arithmetic Error #3541
Conversation
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.
It's likely Biscuit will want the update progress to be in an else block but I personally don't mind it. I will just approve and let him or Walshy decide if they want to suggest the change before merging
yeah I mean I don't mind changing it if that's what's preferred :) |
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.
I approve of this, I even checked the logic and made a truth table.
But as pointed out, style-wise an inclusive-if would be preferred and we generally like to keep a reference to the fixed issue so we can better track these things.
Otherwise, it looks good! Thanks!
src/main/java/io/github/thebusybiscuit/slimefun4/core/machines/MachineProcessor.java
Outdated
Show resolved
Hide resolved
…/MachineProcessor.java Co-authored-by: TheBusyBiscuit <TheBusyBiscuit@users.noreply.github.com>
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.
LGTM
Description
We found this issue by accident, but it seems that if Slimefun's ticks are faster than expected, Tier 3 items such as Electric Ore Grinders will produce an ArithmeticException.
https://pastebin.com/ZGWDJyWn - the error from our fork, this small patch fixed it.
Proposed changes
Don't update the progress bar if the operation is already finished
Related Issues (if applicable)
Resolves #3538
Checklist
Nonnull
andNullable
annotations to my methods to indicate their behaviour for null values