-
Notifications
You must be signed in to change notification settings - Fork 357
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
Change behaviour of Instance getBlock to stop throwing NPE #1940
base: master
Are you sure you want to change the base?
Change behaviour of Instance getBlock to stop throwing NPE #1940
Conversation
returning Block.AIR is very dangerous because you cannot be sure if the chunk is just unloaded or if there is really air. |
I was just about to update this saying why I'm not doing that. The original change did that. The reason why I have decided that this is better is because there is very few circumstances where you actually really want to know that the chunk is unloaded, and this change would force you to do a null check to handle that case every time. I think it is better if you have to be more explicit, checking if the chunk is loaded with |
Returning |
I think that I also prefer the idea of returning null in case you do need to know that it is unloaded, and treating that as air is simple enough. That said, it could also potentially be a new condition to decide this. |
Something like a new |
That's not a bad idea. Though I'm not sure if it would be confusing for people not expecting it to be there. Also, had a discussion about this last night, and @ZakShearman suggested this should be an exception thrown here as it is actually an exceptional condition (what exceptions are meant to be used for), but that it should be a custom one at least, definitely not |
This change means that, if
Instance#getBlock
is called on a block in an unloaded chunk, the method simply returns Block.AIR, instead of throwing aNullPointerException
.This behaviour is in line with
Chunk#getBlock
, and also in line with vanilla behaviour.Motivation
Currently, if you try to get a block in an unloaded chunk using
Instance#getBlock
, it will throw aNullPointerException
. This behaviour is very annoying for several reasons:@Nullable
, when it can actually never return null because of the NPE behaviour.getBlock
, catching aNullPointerException
, every time you get a block from anInstance
. Both of these are undesirable, and the latter is bad practice.This aims to completely eliminate the possibility of encountering a
NullPointerException
when usingInstance#getBlock
, therefore avoiding confusion, saving inevitable debugging time, and moving the API in line with itsChunk
counterpart.