Applied Energistics 2

Applied Energistics 2

137M Downloads

NPE Crash doing `getBoxes` for Dense Cables

SirStendec opened this issue ยท 1 comments

commented

Describe the bug
The getBoxes method of PartDenseCable calls isDense for all connections. Neither getBoxes nor isDense validate the AEPartLocation. If the location is INTERNAL, it calls BlockPos#offset with null, resulting in a NPE.

My mod, Wireless Utilities, creates grid connections between non-adjacent blocks using createGridConnection() resulting in connections with the location INTERNAL. Thus, if a user tries connecting to a dense cable using a WU machine, AE2 starts throwing NPEs and potentially crashing the user's world.

This can lock a user out of the world if, for example, an entity is colliding with the cable and Minecraft needs to get the cable's collision box, making the world crash during start-up.

To Reproduce

  1. Install Wireless Utilities
  2. Use a Wireless Utilities AE machine (either positional or directional) to connect to a distant dense cable.
  3. It becomes impossible to interact with the dense cable. If an entity collides with it, the world crashes.

Expected behavior
Cables should skip connections with the location INTERNAL within getBoxes.

Additional context
Crash Report: https://gist.github.com/SirStendec/236b42ce087cd73e5ab95aeb78554a68
WU Issue: SirStendec/WirelessUtilities#66

Environment
Running in dev for Wireless Utilities. Tested in single player.

  • Minecraft Version: 1.12.2
  • AE2 Version: stable rv6-stable-7 for Forge 14.23.5.2768
  • Forge Version: 14.23.5.2768
commented

While this might be considered a bug, I most likely won't fix it. Most likely it's even just the most obvious issue, with a couple others still hidden somewhere.

I would considered creating internal connections to random gridnodes as mostly untested and at least unexpected. If maybe not even unsupported, but there isn't a good way to enforce some limtations without probably breaking valid approaches.

E.g. some other wireless block/part can easily destroy old connection either on updates or even periodically to cleanup, which then can easily cause a rebuild loop because some other mod keeps adding new connections each time they get dropped. Currently not even sure, if this already applies to QNBs under some conditions.

There is certainly a good chance to re-evaluate it in the future, but it would require to test the interaction with every single AE block, Which most likely will be a pretty high effort to cover all corner cases. As well as needing at least something similar in AE2 itself, at least as debug block, to allow addons to test their own blocks without having to install a couple random mods or implement their own solution.

For now the best approach would certainly be to only connect gridnodes, which are under the same mod authority e.g. dedicated emitter/receiver pairs or are at least use a whitelist to things which are know to work under any condition and not cause hidden performance issues or similar.