Change behaviour of Instance getBlock to stop throwing NPE

[BomBardyGamer]

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 a NullPointerException.

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 a NullPointerException. This behaviour is very annoying for several reasons:

• It is not documented anywhere. It is not at all obvious from any documentation anywhere that this is the case.
• This is not something you should expect this method to do, especially consdering the overridden signature is @Nullable, when it can actually never return null because of the NPE behaviour.
• It means that you either have to check if the chunk is loaded, or use a try catch around getBlock, catching a NullPointerException, every time you get a block from an Instance. Both of these are undesirable, and the latter is bad practice.

This aims to completely eliminate the possibility of encountering a NullPointerException when using Instance#getBlock, therefore avoiding confusion, saving inevitable debugging time, and moving the API in line with its Chunk counterpart.

[ElAndree]

returning Block.AIR is very dangerous because you cannot be sure if the chunk is just unloaded or if there is really air.
why arent you just returning null instead of throwing a npe?

[BomBardyGamer]

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 isChunkLoaded, for the few cases where you actually want to know if the chunk is loaded or not.

[UserUNP]

Returning null would probably be better so the code wouldn't check if a chunk is loaded twice.
It's also more convenient if the goal is simply not only checking if a chunk is loaded, but also getting the block as well.

[mworzala]

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.

[TheMode]

Something like a new getVisibleBlockcould be more fitting, as unloaded chunks are invisible

[BomBardyGamer]

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 NullPointerException. That's also an option, like a ChunkNotLoadedException.