Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Impetus Nodes seem to still not be loaded properly and crash #379

Closed
jchung01 opened this issue Jun 9, 2024 · 2 comments
Closed

Impetus Nodes seem to still not be loaded properly and crash #379

jchung01 opened this issue Jun 9, 2024 · 2 comments
Labels
Bug Something isn't working Fixed in Dev This is fixed in the source, but might take a little bit to get released in an update

Comments

@jchung01
Copy link

jchung01 commented Jun 9, 2024

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.

@TheCodex6824
Copy link
Owner

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.

@TheCodex6824 TheCodex6824 added the Bug Something isn't working label Jun 19, 2024
@TheCodex6824
Copy link
Owner

TheCodex6824 commented Dec 8, 2024

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.

TheCodex6824 added a commit that referenced this issue Dec 9, 2024
@TheCodex6824 TheCodex6824 added the Fixed in Dev This is fixed in the source, but might take a little bit to get released in an update label Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Fixed in Dev This is fixed in the source, but might take a little bit to get released in an update
Projects
None yet
Development

No branches or pull requests

2 participants