Thaumic Augmentation

Thaumic Augmentation

7M Downloads

Impetus Nodes seem to still not be loaded properly and crash

jchung01 opened this issue ยท 2 comments

commented

Making an issue about a crash report another user experienced in the MeatballCraft modpack.
It seems like impetus nodes still aren't loaded properly. This user, on world load, would get this crash https://mclo.gs/QW45Sdk. (This specific report is using Java 22/Cleanroom, but they said they tested entering the world on Java 8 and still crashed) From looking at past issues, it looks like reproducing impetus node issues is difficult, but the user mentioned they were using a cross-dimensional setup using impetus mirrors.

Looking at the code and some related issues, it looks like the world of a TileEntity can still be null during onLoad() (in this case, called from the impetus mirror). This seems to be a Forge bug. ImpetusNode.validateNodeInput() does not seem to check if the world is null, so that would explain the crash. This may have been due to the code changes in #319. I'm not sure if a null check would be sufficient or further reworking of the loading of the impetus node system would be required.

commented

Sorry for the crashing. As you found out, impetus nodes have a lot of history with chunk loading issues. In this case, it looks like onLoad is not suitable for initializing the properties of impetus nodes. I originally used it both because the documentation said to and to avoid having to make every impetus node tickable, but it looks like there's no getting around it.

commented

I took a look at this again and it looks like the only purpose of init (the problematic method that requires a world in onLoad) is as a data consistency check. I might be able to just remove that. Any issues that would have been handled in init before would hopefully get caught by the NodeHelper.validateOutputs calls. The only tradeoff I can think of with this approach is that the nodes would have their input/output slots taken up by invalid nodes until the first tile tick, but since this would only happen if a node was abnormally removed I think that's ok.

I also can't completely delete this code since it's unfortunately API, but I'll mark it for deprecation now and remove it next time I'm able to break API. I can also remove all TA internal uses of it of course.

Edit: init apparently was also responsible for connecting nodes together when one node is loaded after another (such as in different chunks). I added validate calls to all nodes, not just nodes with outputs, and also tell the client to check for newly loaded nodes when it receives node data from the server. Any old code that still uses the init and validateOutputs way will still work (other than maybe crashing in onLoad), but they are all replaced in TA.