search

SleepyTrousers / EnderIO-1.5-1.12

http://enderio.com/
The Unlicense
729 stars 360 forks source link

[1.10.2] FW: Placing conduits before placing Ender Key Chests will not allow access to inventory #3532

Closed gigaherz closed 8 years ago

gigaherz commented 8 years ago

Original issue: https://github.com/gigaherz/Enderthing/issues/13

I'm "forwarding" this issue here because I believe the problem is with EnderIO's caching strategy. Let me explain the situation:

My mod, Enderthing provides ender chests, similar in purpose to the Ender Storage chests. When you place a chest with a color preset in it, or change the colors of an already-placed, it will first place the block itself, and then afterward it will update the color code, and through markDirty(), notify the neighbours of the change.

However, EnderIO is accessing (and, I believe, caching) the IItemHandler instance as soon as I place the block, and based on the symptoms (it behaves as if the placed block was the default white-white-white combination, with value 0, rather than the newly assigned inventory), I suspect it's not properly re-querying the IItemHandler upon receiving an onNeighborChange, and it's reusing the original (stale) reference.

Given that the capability system was designed to have quick lookups (after the testing I did on my Ender-Rift mod, I concluded heavy caching isn't needed, and it's OK to just keep calling getCapability whenever something is needed) I believe not discarding the neighbour's inventory instance when a change is signaled through onNeighbourChange, is a bug in EnderIO.

AnrDaemon commented 8 years ago

"Capability system" was designed to waste server performance, nothing more.

HenryLoenwind commented 8 years ago

Ender IO does not cache item capabilities. Do you change from not presenting a capability to presenting one? It does cache external connections.

Also, markDirty() is not a general notification, it marks your TE for saving and notifies comperators of potential comparator level changes. You probably want to send a block update for everything else.

gigaherz commented 8 years ago

If it does not cache the capabilities, then I have no idea why it would still be reading items from the white-white-white chest, when everything else I have tried, including hoppers and Storage Network, have no issues reading from the right inventory. (EDIT: I will check that again, just in case there's some sort of black magic going on with the TileEntity state, that I didn't notice before.)

I do not go from nothing to something, I go from one something, to a different one, but that shouldn't matter, the issue would be the same regardless.

And I disagree, markDirty/onNeighborChange IS the correct place to refresh any data you have cached about a neighbour, or at least mark that data as dirty, so that it can be discarded later. Forcing a block update notification is overkill, since the block has not been replaced at all.

HenryLoenwind commented 8 years ago

Reacting to markDirty() makes no sense at all. Any RF-powered machine will throw them around at least twice per tick while it's active. Add a third for a fluid tank and one notification every time an item is inserted or removed from its inventory.

What's the problem with getting Ender IO into your dev env? Does it not load from the mods folder or do you need it in you classpath fro the debugger?

gigaherz commented 8 years ago

My bad! I found the real cause of the issue: I had a corner case situation, which only occurred due to the conduit calling getCapability on the block change notification, which happens before ItemBlock#setTileEntityNBT has a chance to run.

gigaherz commented 8 years ago

@HenryLoenwind Ah yes, the issue with loading was a deobf problem -- FG2's deobf feature doesn't properly deobf EnderCore, so I had to use BON2 to deobf it myself, but then it didn't really help much becuase I had no sources to attach so I couldn't step into EnderIO.