mekanism / Mekanism

A mod for Minecraft
MIT License
1.39k stars 527 forks source link

Universal Cable Crash On Removal #1123

Closed XileForce closed 10 years ago

XileForce commented 10 years ago

http://pastebin.com/W2M5yAMM

Crash occurs when breaking a universal cable connecting an ic2 power generation item to anything else such as an mfsu or macerator.

Easiest way to recreate this is to place a solar panel, connect a universal cable to it and connect that to an mfsu then break the universal cable. crashes on servers and single player immediately. confirmed happening on build 6.02.17 and 6.02.20. Additionally I tested a variety of ic2 builds ranging from build 386 to 397 as well as updating my multipart. Hope this helps!

CoryManson commented 10 years ago

See here: https://github.com/aidancbrady/Mekanism/issues/837 and here: http://bt.industrial-craft.net/view.php?id=869

Aidan believes this is an IC2 bug (Even though it only occurs in Mekanism v6 and not 5.7) and the IC2 folks have completely ignored it. So now we are left with crashing servers and nobody willing to fix it. :)

Have a great day!

XileForce commented 10 years ago

Yes I can confirm this ony happens with mekanism 6.0 as I just updated. Ic2 isn't going to fix as they arent updating 1.6.4 anymore so mekanism is my only hope :(

Algester commented 10 years ago

sadly if the IC2 power network API code isnt fixed neither can aidan actually help you so you have to convince the IC2-exp devs... ahem... to actually get the code to work...

CoryManson commented 10 years ago

The confusing part is that there was no issue in Mekanism 5.7. So something new in v6 has caused the issue.

Would Aidan not be using the exact same IC2 API as was in 5.7?

XileForce commented 10 years ago

That's all I can think of. ATM my only option is banning universal cables from my server :(.I just recently updated to mek 6.maybe its fixed in a Dev build?

CoryManson commented 10 years ago

Does not appear to be fixed in latest Mekanism builds. Problem has existed since last year.

I am currently figuring out a way to remove and ban UniCable from my TPPI server

aidancbrady commented 10 years ago

First off, this is completely not my fault and is a result of issues on both FMP and IC2's ends.

IC2 could technically avoid this by adding in a null check to its energy net code, and I'm trying to get in touch with Player in order for him to do so. However, this issue is invalid now, as there is literally nothing I can do without dropping IC2 or FMP.

jakimfett commented 10 years ago

@aidancbrady, what's the possibility of a fix for this, not in the conventional "everything works as planned" way, but in the "the machine/cable causing this gets deleted" sort of a way?

Reason I ask...this is the bug that is currently taking my server offline 2-3 times per day, and given IC2's recent jump to 1.7, I doubt this will get fixed on IC2's end any time soon, if ever, especially with Greg at the helm.

unpairedbracket commented 10 years ago

In case anyone wants to know why this is happening, and how it can be not IC2's fault, nor ours or Multipart's, this is why (or at least why I think) it's happening.

When a TileEntity is removed, it must also be removed from the IC2 EnergyNet, so that the EnergyNet doesn't keep trying to use it. This is done by posting an event to the Forge Event Bus telling IC2 that it's gone, which we do when the universal cable part is broken. The IC2 EnergyNet doesn't appear to "notice" this immediately, so tries to use the TileEntity one last time at the end of that tick. By this point the universal cable part is long gone, so the method that IC2 calls on the TileEntity crashes with a NullPointerException. This happens because of the clever stuff ChickenBones has Multipart doing behind the scenes to get Tiles to appear to implement APIs that are actually on the parts themselves. He creates a "trait" which gets glued on to the TileEntity and passes through API calls to the parts. By the time IC2 calls it's API method, the part the trait's trying to use has gone to null so everything breaks.

It might be possible to fix this by hand-writing a trait instead of auto-generating one, but I've never managed to get that to work.

jakimfett commented 10 years ago

@unpairedbracket per my conversation with Player (see http://pastebin.com/VRygBA1R for the conversation text) indicates that #837 is a Mekanism problem, but #1123 is an IC2 problem.

These two issues are NOT the same (despite my earlier confusion about this). #837 deals with a ConcurrentModificationException, #1123 is a NullPointerException.

unpairedbracket commented 10 years ago

I've proposed a partial solution to this in #1280, sound OK to everyone?

jakimfett commented 10 years ago

At this point, any sort of a fix is better than no fix at all.

I'd much rather have to explain to players that they'll need to put generators next to an energy cube than trying to explain that using IC2 gens with the UniCables will crash the server.