setBlockLight should define setBlockLight and resolve a few other minor inconsistencies #3365
Conversation
…or inconsistencies
SirYwell
left a comment
SirYwell
left a comment
There was a problem hiding this comment.
Mostly good, but please take another look at the usages of getBlockIS() regarding nullness if you're already at it.
| if (region != null && !region.contains(ox, oz)) { | ||
| return summary; | ||
| } | ||
| try (FaweInputStream fis = getBlockIS()) { |
There was a problem hiding this comment.
if getBlockIS() can indeed return null, then this needs to be covered here as well
There was a problem hiding this comment.
if
getBlockIS()can indeed return null, then this needs to be covered here as well
Assuming you just wanted a simple NPE check, done so.
There was a problem hiding this comment.
Should be possible to just do the null check at the top inside the try-with-resources statement, no?
There was a problem hiding this comment.
Should be possible to just do the null check at the top inside the try-with-resources statement, no?
I guess that would take a cleaner approach. I'm dumb.
3 participants
I addressed these in my larger Folia pull request, and because of rejection, I would like to forward these upstream separately. I am confident a lot of these issues aren't intentional. If any of these changes are deliberate, I'd be more than happy to revise!