Replace `@Nullable` with `@UnknownNullability` on `MinecraftClient.player`

[Earthcomputer]

(Reminder: Loom now has the ability to apply annotation changes from mapping sets such as yarn.)

I want to start a discussion on replacing Mojang's use of the @Nullable annotation with @UnknownNullability when the nullness of something frequently does not need to be checked. Here's what the @Nullable docs have to say about this situation:

By convention, this annotation applied only when the value should always be checked against null because the developer could do nothing to prevent null from happening. Otherwise, too eager Nullable usage could lead to too many false positives from static analysis tools.

For example, Map.get(Object key) should not be annotated Nullable because someone may have put not-null value in the map by this key and is expecting to find this value there ever since. It could be annotated as UnknownNullability or left unannotated.

On the other hand, the Reference.get() should be annotated Nullable because it returns null if object got collected which can happen at any time completely unexpectedly.

In our case, @UnknownNullability makes sense not only from a documentation perspective, but also to override the @FieldsAreNonnullByDefault annotation.

This issue applies to fields such as MinecraftClient.player, MinecraftClient.world, and others.

[modmuss50]

Im not sure these 2 specific fields are the best example of where this needs to change, as these fields are null when you arent ingame, and easy to get from the MinecraftClient singleton and something a modder does need to think about.

A better example would be: BlockEntity.getWorld() imo, as in the vast majoriy of cases when you have a BlockEntity its world isnt going to be null.

[Earthcomputer]

A better example would be: BlockEntity.getWorld() imo, as in the vast majoriy of cases when you have a BlockEntity its world isnt going to be null.

That is true, and it's very annoying to check in the same way as MinecraftClient.player is. But even here, modders need to be aware of the possibility that in some cases the world can be null, which can happen in for example BlockEntity.onBlockReplaced (see the null check CampfireBlockEntity). But I think that sometimes the annoyance of having way too many false positives outweighs the benefits in some cases, including for MinecraftClient.player. If you're using that field chances are you conceptually have a world open.

[Akhil-kotipali]

Hello! I want to work on this problem. I intend to begin by going over areas such as MinecraftClient.player as well as MinecraftClient.world to find out where swapping out @nullable with @UnknownNullability It makes sense for nullability, and then Ask for feedback on a small initial PR. Would you kindly let me know if I may move forward?