java.lang.NullPointerException from TileEntity#func_145838_q

[TheDGOfficial]

This error happens randomly and sometimes repeats itself a lot before stopping happening, while playing on Hypixel (Skyblock). I don't know the actual or root cause of it because the head of the stack trace is a obfuscated vanilla function and the only clue is that EntityCulling mod calls this function.

[01:40:00] [CullThread/ERROR]: catching
java.lang.NullPointerException
	at net.minecraft.tileentity.TileEntity.func_145838_q(TileEntity.java:175) ~[akw.class:?]
	at dev.tr7zw.entityculling.CullTask.run(CullTask.java:71) [CullTask.class:?]
	at java.lang.Thread.run(Thread.java:750) [?:1.8.0_362]

Using OpenJDK 1.8.0_362 on Ubuntu 22.10. Minecraft 1.8.9, EntityCulling 1.5.0-1.8, Forge 11.15.1.2318 1.8.9

[TheDGOfficial]

Seems like the func_145838_q's deobfuscated name is getBlockType:

EntityCulling/EntityCulling-Shared/src/main/java/dev/tr7zw/entityculling/CullTask.java

Line 71 in ce0cf0c

if(unCullable.contains(entry.getBlockType().getUnlocalizedName())) {

On Line 67 a bit just above this, there is this block:

EntityCulling/EntityCulling-Shared/src/main/java/dev/tr7zw/entityculling/CullTask.java

Line 67 in ce0cf0c

}catch(NullPointerException | ConcurrentModificationException ex) {

Maybe this method call can be put inside the catch as well to ignore the exception. This of course won't prevent the exception to be created (creating stack traces is not free in terms of performance, but shouldn't be that high of a cost with JIT), but it can at least prevent the error from being printed to logs, reducing some I/O, because sometimes this error repeats itself a lot (fortunately there's a -XX:+OmitStackTraceInFastThrow setting, but it will still print catching java.lang.NullPointerException, without a stack trace, which is even more confusing, but I guess more performant.)

[tr7zw]

Uh honestly didn't know that anyone actively uses the 1.8 version. This does seems to be an exception inside the getBlockType() method(which is odd, probably some 1.8 jank), I guess first looking into the class to understand the nullpointer would be better, instead of just blanked catching and suppressing it.

[TheDGOfficial]

Uh honestly didn't know that anyone actively uses the 1.8 version. This does seems to be an exception inside the getBlockType() method(which is odd, probably some 1.8 jank), I guess first looking into the class to understand the nullpointer would be better, instead of just blanked catching and suppressing it.

I agree, was just curious if it was intented to be ignored like the other code in the catch block above. I don't have much experience in client development, my experience is mostly in server-side where there's a lot of abstractions to not have to call Minecraft code directly (you even have to use reflection to call direct Minecraft code which prevents developers from calling Minecraft code directly most of the time), so I might be wrong, but after looking at some source code, seems like either worldObj or the return value from getBlockState call in the getBlockType method is null, wouldve known what exactly is null if we could use Java 14+ somehow with -XX:+CodeDetailsInExceptionMessages, but obviously can't use anything above Java 8 in MC 1.8.

Looking at getBlockState it has a check like if (!isValid(pos)) and it returns state of the AIR block, so I think it can't return null. Probably worldObj being null, maybe this method is being called before the variable initializes? Or it is some race condition, because sometimes this error never happens, sometimes it happens 1 time after startup then stops, and sometimes it repeats a lot, then it stops.

After looking a bit more I noticed a method called hasWorldObj in the TileEntity class that checks if worldObj is null or not, so that suggests this variable can be null indeed. Probably have to make a change like this:

 if(entry.hasWorldObj() && unCullable.contains(entry.getBlockType().getUnlocalizedName())) { 

And if instead return of the getBlockState method is null, then probably need a check like this:

 if(entry.hasWorldObj() && entry.getWorld().getBlockState(entry.getPos()) != null && unCullable.contains(entry.getBlockType().getUnlocalizedName())) { 

Of course I don't know the code base of this mod and I didn't really check all the source code, I've only looked at the lines where stack trace pointed me to, so I don't know how adding those conditions would interfere with the mechanics of the mod, but since the current behaviour of getting an exception makes the control flow stop going after the line exception happens, and this changed version would make all the code after the if (which acts a else condition even if there's no else) I think it can introduce other issues if the change is applied by someone that doesn't really know the codebase of the mod nor has much experience in the Forge development, but I might still give this a try later on when I have time, and post the results whether this fixes the error or introduces any other issues (that is assuming I am able to build the project though, I've had bad experience trying to build forge mods from source generally, getting errors from missing dependencies like asbyth/ForgeGradle repo being deleted, and so on).

[tr7zw]

my experience is mostly in server-side where there's a lot of abstractions to not have to call Minecraft code directly (you even have to use reflection to call direct Minecraft code which prevents developers from calling Minecraft code directly most of the time)

Check out my other "small" project from when I was mostly a Spigot dev: https://github.com/tr7zw/Item-NBT-API it's all about breaking that part down, automating reflections into NMS from 1.8-1.19 for NBT.

Looking at getBlockState it has a check like if (!isValid(pos)) and it returns state of the AIR block, so I think it can't return null.

The Stacktrace states, that the exception happens inside the method
at net.minecraft.tileentity.TileEntity.func_145838_q(TileEntity.java:175) ~[akw.class:?], so it's not the issue that entry is null/the return value is.
Since this entire thing happens async, it's possible for it to catch BlockEntities before they are added into the world/after they got removed, so entry.hasWorldObj() sounds like a possible catch to prevent that. Add the condition and give it a shot, making a PR if it works(also tags the improvement to your contribution).

I've had bad experience trying to build forge mods from source generally, getting errors from missing dependencies like asbyth/ForgeGradle repo being deleted, and so on).

At least last time(feb 2022), it just built using ./gradlew build with java 8 on the Github CI. So unless some maven repo died, this should still be the case.

[TheDGOfficial]

Check out my other "small" project from when I was mostly a Spigot dev: https://github.com/tr7zw/Item-NBT-API it's all about breaking that part down, automating reflections into NMS from 1.8-1.19 for NBT.

Yes I know you can make a common interface and implement the methods per each NMS version, then determine which implementation to use from server version in runtime, but this still hacky and repetitive coding, and it promotes adding that functionality to Bukkit/Spigot itself as an abstraction API instead of having to do this, was what I meant.

The Stacktrace states, that the exception happens inside the method
at net.minecraft.tileentity.TileEntity.func_145838_q(TileEntity.java:175) ~[akw.class:?], so it's not the issue that entry is null/the return value is.
Since this entire thing happens async, it's possible for it to catch BlockEntities before they are added into the world/after they got removed, so entry.hasWorldObj() sounds like a possible catch to prevent that. Add the condition and give it a shot, making a PR if it works(also tags the improvement to your contribution).

I was referring to the fact that getBlockType method calls getBlockState on worldObj, then calls getBlock on the return value of getBlockState. So there's 2 things that can NPE: worldObj being null, or getBlockState returning null. Then I said since getBlockState code should return AIR's block state if the position argument is invalid, it shouldn't return null.

At least last time(feb 2022), it just built using ./gradlew build with java 8 on the Github CI. So unless some maven repo died, this should still be the case.

Unfortunately what I predicted was correct. Tried right now and got this error (on a fresh git clone and git checkout 1.8 repo):

FAILURE: Build failed with an exception.

* Where:
Build file '/home/mustafa/EntityCulling/EntityCulling-Forge/build.gradle' line: 4

* What went wrong:
Plugin [id: 'net.minecraftforge.gradle.forge', version: 'ddb1eb0', artifact: 'com.github.asbyth:ForgeGradle:ddb1eb0'] was not found in any of the following sources:

- Gradle Core Plugins (plugin is not in 'org.gradle' namespace)
- Plugin Repositories (could not resolve plugin artifact 'com.github.asbyth:ForgeGradle:ddb1eb0')
  Searched in the following repositories:
    Gradle Central Plugin Repository
    MavenRepo
    MavenLocal(file:/home/mustafa/.m2/repository/)
    Forge(https://maven.minecraftforge.net/)
    Jitpack(https://jitpack.io/)

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 27s

I've found backup GitHub repositories like https://github.com/hax0r31337/ForgeGradle, and I know I can use with JitPack, but what I don't understand is how to replace asbyth's repo with hax0r31337's one, because the build.gradle file contains no mention of asbyth, it just has a listing to depend on a plugin with id "net.minecraftforge.gradle.forge'", so I don't know where asbyth comes from.

Seems like asbyth removed everything from the internet related to them (found a thread about it with a simple search: https://hypixel.net/threads/what-happened-to-asbyth.5099794/), I found the backup repo i mentioned from going to the asbyth's ForgeGradle repo in WebArchive then clicking on the Forks, then going to one of the forks that is not deleted in GitHub.

[tr7zw]

Aaaaa this will suck. 1.8/1.12 are such horrible versions to work with even in 2021. The tooling is so dated, it just doesn't work anymore(be it modern gradle, modern java or just repos containing old but important dependencies vanishing). Last time it took me multiple days and help to get the 1.8 stuff building correctly with mixins at all. Kinda one of the reasons, I stopped backporting/updating any mods for 1.8/1.12 after the initial 3. I'll look at it at some later point when I get to 1.12/1.8 with my https://github.com/tr7zw/ModComposeTemplate project, where the build system gets generated dynamically right before the build time.