1.14.4/optifine magma block causes NPE

[wasche]

Not sure if it is optifine related, but I'm guessing so. Was doing some mining and hit a cave, turning on the light level overlay immediately causes an NPE.

crash report: crash-2019-11-18_08.30.23-client.txt

[feldim2425]

I can reproduce it without optifine. The problem seems to be something about getting the spawn information from magma blocks. Just turning on the overlay and placing a magmablock in rage of the overlay is enough to crash immediately.

[feldim2425]

After a quick look into the source code for the magmablock it either seems that it's canEntitySpawn method was programmed without keeping in mind that the EntityType can be null or the method is documented wrong since they marked that parameter as Nullable.

The fix sadly reduces performance a bit since I have to check every singe monster entitytype that might spawn. So I added a config option to make a simple check just for the Zombie entity.

[noobanidus]

That's interesting, I was just coming to report this and was checking through closed issues to see if it had already been discussed. It got brought up on our discord and there was some conversation about it.

I did have a look at the decompiled code from IntelliJ and interestingly enough it doesn't have a @Nullable notation on the EntityType parameter in either the BlockState implementation or the actual Block implementation. I'm not sure if my code has had it stripped out, however.

Of note, though, it seems that BlockMagma is one of the few that actually implement canEntitySpawn. I'm not sure whether that's helpful or not, though, as that can't really be used as a heuristic given modded blocks exist...

I personally would definitely recommend making the Zombie EntityType default, however, and adding an option for the "test every monster entitytype".

[feldim2425]

Here is a link to the source: https://github.com/MinecraftForge/MinecraftForge/blob/6422310453fe4029a409adf19d3bac6bfb3ea589/src/main/java/net/minecraftforge/common/extensions/IForgeBlock.java#L269

IForgeBlock is implemented by Block because Forge uses a .patch file to add it. And I used that method to determine if mobs can spawn.

[wasche]

Thanks for the super fast fix! Any idea what release this'll be in? Just curious, I've no problems building it for myself, but its nice to be lazy and just click the button in the launcher :)

I'd also second making zombie default and the option to check all.

[feldim2425]

Already out in 1.16.1 alpha release on curseforge. Along with some other fixes and features. https://www.curseforge.com/minecraft/mc-mods/more-overlays/files/2829171

EDIT: There it is still alpha since there are some more things planed for 1.14.4.

• The config gui will get it's own list system since the vanilla AbstractList doesn't fit very well from the way it handles gui events, rendering and init-calls
• I want to add some way to open the config ui ingame either by adding a button to the pause screen or via a client side command
• The current system to detect mob spawns should get more options to set it into light only mode and maybe find a better way to detect mobspawns without checking all mobs.
• I want to add IMC messages for the API.

[noobanidus]

That's definitely a bug on Forge's part, then, because neither the function they call in the state nor the function that it calls in the block will accept a null entity type... I might go file a bug.

EDIT: Especially demonstrable by the fact that the Vanilla magma block is accessing the EntityType directly without testing for null first.

EDIT 2: I've filed a Forge issue for this: https://github.com/MinecraftForge/MinecraftForge/issues/6326

[feldim2425]

Also I should add that only checking for the EntityType "Zombie" has the problem, that blocks like the magma block won't show any overlay, since they only allow monsters that resist fire, thats why I currently set the default to check all mobs, because usually users will complain that the overlay doesn't work when they just had to change to the less performant setting. I will try to add more options like a true ligt only mode and maybe I or someone else finds a better way to optimize the check (maybe caching the blocks spawn mode).

[noobanidus]

Perhaps Zombie Pigman could be an alternative to zombie? I believe they should satisfy most conditions.

Additionally, a block(state?) -> boolean conditional map that could be cached might also help to simplify things, or at least speed up the process.

EDIT: Judging by the adjustment of tags to "1.14" and "cleanup", I'm going to presume that the "@Nullable" annotation remaining was an oversight from 1.12, where I presume it was accepting an Entity(Living?) instead of an EntityType.

[feldim2425]

In 1.12 there was no extra parameter at least not in the method I used. It simply required the BlockState and the SpawnPlacementType. I don't know if spawns based on State, Placement and Entity were a thing in 1.12 but I assume "no".