-
Notifications
You must be signed in to change notification settings - Fork 192
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
Update redstone wire optimizations and fix #581 #583
Update redstone wire optimizations and fix #581 #583
Conversation
private static boolean ignoreNonWires = false; | ||
|
||
public static int getNeighborBlockSignal(Block wireBlock, RedstoneWireEvaluator evaluator, Level level, BlockPos pos) { | ||
ignoreWires = true; |
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 am concerned about these static fields, mod compatibility with threading mods will definitely suffer
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 think converting them to a local variable should be possible if I am not misunderstanding the code
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.
yeah that'd work
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.
Since you are the original author of this optimization and also the tests are passing, I mostly trust that the code is correct. When merging I will change the static fields to local variables, as I do not see a good reason to keep them static.
private static boolean ignoreNonWires = false; | ||
|
||
public static int getNeighborBlockSignal(Block wireBlock, RedstoneWireEvaluator evaluator, Level level, BlockPos pos) { | ||
ignoreWires = true; |
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 think converting them to a local variable should be possible if I am not misunderstanding the code
Rebased onto develop and replaced the static fields with local variables. Thanks for the PR |
No description provided.