Botania

Botania

133M Downloads

crash when rightclicking EIO cpacitor cell while wearing rings

mindforger opened this issue ยท 31 comments

commented

http://pastebin.com/VV11cN4R

botania 248

only happens to me but not to other guys on server with a copy of my config and mods folder

okay wait a secoond ... ONE thing that i may have not tested

okay it still is crashing on forge 1614

i don't get it, it looks like the server is assuming i am equipping that ring when i open the window ... happens with all type of rings in every slot

commented

yeah well looks like its fastcraft

commented

@TheWhiteWolves please reopen, did just happen again without fastcraft ... same error again

commented

i did some research, why the fudge is this methode even called when i open a capacitor bank!? shouldn't this be called only when equipping an bauble to play the sound ? (by the way it normally doesn't play when opening the cap)

commented

I think that is a side effect of the baubles inventory code being, um, "talky". We have inventory slots in the cap bank GUI, and they are bound to the server baubles inv on the server and to the client baubles inv on the client. Now those two inventories talk to each other and sync their contents. But the slots are also connected to each other by vanilla's slot code, which also syncs the content between server and client. So it is double-synced.

Same happens for (most) TEs. But also, most TEs don't care what happens to their slots on the client, so getting an update from the server directly (description packet) and one from the slot doesn't matter. But Baubles call those callbacks, like onEquipped(), also on the client. And it triggers when the slot is synced, too.

I have special code in ender io for the integrated server, because there the Baubles inventory is in a shared field for server and client. I need to look into it further to see if I need to have (and can have) a fake/proxy Baubles inventory for multiplayer, too. Need to be careful, don't want to skip those client-side callbacks completely.

commented

i don't want to rush anybody, but this bug starts to render my gameplay a nightmare, can't there be added some more defensive code in this area ?! or am i allowed to build my own branch to get rid of this artifact somehow? Or maybe hand me a version with a "try, catch and vomit into my logfile" to give you more details as i can reproduce this 100% on my server

commented

You are more than welcome to create a branch and fix the problem yourself, if that's what you're asking. If you choose to do that however, and you wish to have your fix appear in botania, you will need to keep the contributing guidelines in mind.

For now I can also recommend disabling enderIO till this problem is fixed (or botania, but since this is the botania repo, I am not recommending that)

commented

JRoffel yeah well "disabling" will render the whole world lost and it's a server with more people than just me ;P but i'll make myself a version with said structure, not sure what information you may need, i''ll try dumping all inside this routine i can get

commented

ok, please try this version: http://loenwind.info/EnderIO-1.7.10-2.3.0.422t1_beta.jar

This is a client-only change and I left the version number at 422, so there should be no need to touch the server.

If it doesn't help, there may be a log message with a stack trace above the crash. That'd be a sign I messed something up (which isn't unlikely), and I need that message to fix it. Sry, I'm not in the mood to set up a server with eio and botania for some extensive testing today. Blame my boss at work. ;)

PS: More null checks in Botania won't hurt, but this problem lies in the design of Baubles and in how I'm using it.

commented

I have an idea what to do. I think I can build you an "try if this fixes the crashed" version tonight.

commented

meanwhile i try to setup a workspace ... haven't done this for quiet a while :D
yeah .. this is getting ugly ... you guys sure do love linux :\

commented

I had a look at the Botania code and the calling Baubles code.

https://github.com/Vazkii/Botania/blob/master/src/main/java/vazkii/botania/common/item/equipment/bauble/ItemBauble.java#L132

https://github.com/Azanor/Baubles/blob/master/src/main/java/baubles/common/container/InventoryBaubles.java#L152

At first glance it seems safe to not null-check player, but it comes from a weak reference. So it could become null any time. In my book that warrants a null check, even though it doesn't make any sense to have a Baubles inventory without the player it belongs to.

I'm not even sure how it can be null in this crash. Ok, it is rubbish that isEquipped() is called here, but where did the player object go? On the client! That must be the reason this crash only happens for one person.

My fix will only remove the bogus isEquipped() call. But the weak reference to the player has gone here, so any other (legit) action with the Baubles inventory will run into the same problem.

Update: I see something. mindforger, did you enter another local world or server before going onto the server where it crashed? I think Baubles only throws away Baubles inventories when loading a local world. So when entering a server, the old inventory object may linger on the client---without a player object but linked to the player's name and updated with the correct content.

commented

thrown a null check in there, it can't hurt to make sure, but it won't be out in a full build until Vaz comes round and makes one.

commented

So, if I understand this correctly, this is not a botania issue?

commented

No code should run into NPEs, that blame goes to Botania. But the root cause lies somewhere else, yes.

I can take my answering to the linked Ender IO ticket, if you want. I just answered to where I got the notifications from... ;)

commented

I'm just trying to keep a low amount of issues open, so trying to get them closed when something points to a fault away from botania, but if part of the problem described in this issue is indeed within botania, then I won't request to close ot

commented

Update: I see something. mindforger, did you enter another local world or server before going onto the server where it crashed? I think Baubles only throws away Baubles inventories when loading a local world. So when entering a server, the old inventory object may linger on the client---without a player object but linked to the player's name and updated with the correct content.
no i didn't enter any world, but i am having slight issues with performance since this issue came up, i'll try to build a whole new mod folder, maybe something is roaming in my configs or mods folder causing a pseudo world to load ... (i can only remember such issues with mystcraft, but we dropped that mod .. maybe RFTools)

thanks for investigation anyways, i'll DL and test this afternoon, await an answer around 19.00 CET

commented

this one crashed a bit more clearly

http://pastebin.com/9EfGRn1i

and the full report http://pastebin.com/AP4NgGPn

this turns out to be an access error to baubles on EIO side. i am checking my mod versions here

commented

head -> table

Class<?> inventoryBaubles = Class.forName("baubles.common.container.InventoryBaubles ");

I feel so stupid right now. Trying to load a class with the name "InventoryBaubles[SPACE]" won't ever work. Copy&Paste at its finest...

New version incoming.

commented
commented

yeah well .. nope still
http://pastebin.com/fNhEZ96g

but still funny XD

i'll wait for the next botania iteration then .. i did not manage to get gradle setting up ... a little bit spoiled by gradlew wrapper for windows :(

i also dropped it onto the server just to be sure i did not forget anything

commented

Ah, ok, I see. That is a new crash. The old one (on S30PacketWindowItems) was indeed fixed, this happens on S2FPacketSetSlot.

Standby while I see if I can slap the same fix onto that one.

commented

Ok, I had a look at it. No I cannot get rid of that call to onEquipped(). It happens exactly the same way in the player inventory:

[21:57:39] [Client thread/INFO] [STDERR]: [java.lang.Throwable$WrappedPrintStream:println:-1]:  at crazypants.enderio.item.ItemMagnet.onEquipped(ItemMagnet.java:210)
[21:57:39] [Client thread/INFO] [STDERR]: [java.lang.Throwable$WrappedPrintStream:println:-1]:  at baubles.common.container.InventoryBaubles.setInventorySlotContents(InventoryBaubles.java:145)
[21:57:39] [Client thread/INFO] [STDERR]: [java.lang.Throwable$WrappedPrintStream:println:-1]:  at net.minecraft.inventory.Slot.putStack(Slot.java:104)
[21:57:39] [Client thread/INFO] [STDERR]: [java.lang.Throwable$WrappedPrintStream:println:-1]:  at net.minecraft.inventory.Container.putStackInSlot(Container.java:547)
[21:57:39] [Client thread/INFO] [STDERR]: [java.lang.Throwable$WrappedPrintStream:println:-1]:  at net.minecraft.client.network.NetHandlerPlayClient.handleSetSlot(NetHandlerPlayClient.java:1155)
[21:57:39] [Client thread/INFO] [STDERR]: [java.lang.Throwable$WrappedPrintStream:println:-1]:  at net.minecraft.network.play.server.S2FPacketSetSlot.processPacket(S2FPacketSetSlot.java:33)
[21:57:39] [Client thread/INFO] [STDERR]: [java.lang.Throwable$WrappedPrintStream:println:-1]:  at net.minecraft.network.play.server.S2FPacketSetSlot.processPacket(S2FPacketSetSlot.java:79)
[21:57:39] [Client thread/INFO] [STDERR]: [java.lang.Throwable$WrappedPrintStream:println:-1]:  at net.minecraft.network.NetworkManager.processReceivedPackets(NetworkManager.java:241)
...

So now it comes down to: Why is player null for you? @Azanor

commented

ok, anything in the log about my code doesn't do its thing?

commented

hours of log reading later ... nothing at all ...

commented

@TheWhiteWolves has your edit of 275729d been merged into some downloadable version already? i am working on some stuff currently but i plan to return to my server soon

commented

you should probably consider setting up milestones or additionally using the release system of github to make it a bit more traceable which botania version refers to wich part of the

wow do my posts always sound so demanding .. just a suggestion for better understanding :)

commented

@mindforger Milestones would be a good idea IF vaz wouldn't forget to actually do it :P. Plus, I don't think we have set release moments for the builds, but don't quote me on that

commented

Not yet sorry, vaz hasnt been round and built 249 yet

commented

she said she would, but she forgot since she's been so busy with psi

commented

did not crash yet so far :)
but i won't bet on it this time :( just give me a few more days of intensive building and right cklicking :D

commented

I'm going to close this for now, if it pops up again for u ill look at re-opening it