search

maruohon / autoverse

A Minecraft mod about simple automation
GNU General Public License v3.0
6 stars 4 forks source link

When Trash buffer is full of Forestry bees, opening the GUI will crash client. #7

Open ghost opened 6 years ago

ghost commented 6 years ago

Whenever a player opens a trash buffer that's full of bees on our Exoria server, the client will disconnect due to payload too high, I would assume this is due to each bee carrying more NBT than expected. So far, this only happens when piping bees into the buffers, we have 2 different buffers causing this issue, both of them in Forestry bees setups.

Minecraft 1.12.2 Forge 14.23.4.2705 Exoria pack version 1.1.0 Autoverse 1.1.0 Forestry 5.8.0.295

Expected behaviour: Trash buffer splits the packets into less than 32768 bytes payloads

Observed behaviour: Trash buffer forces the client to disconnect on opening the GUI with the following error: 2018-07-02_12 05 55

maruohon commented 6 years ago

Can you post the exception from the log? I'm not sure in which part of the code that 32k limit is... All the machines in Autoverse should be using the same Container sync code, and as far as I can see, the size limit on the NBT data should be 2 MB per slot. Unless Netty's packet stuff has that hard 32k limit, but I don't think it would be that low? But even so, I don't see how an individual bee stack would hit that limit... 🤔

Even better would be if you could send the level.dat file and that one region file where you have the setup/Trash Buffers causing crashes.

ghost commented 6 years ago

Sure thing, attached is a zip containing the exception from the logs, level.dat and the region file. One of the buffers affected is at X:-737 Z:398 Y:129 Ex-Crash.zip

maruohon commented 6 years ago

Are those coordinates correct? This is what is there: 2018-07-02_20 30 19

maruohon commented 6 years ago

Oh wait, the block ID map is all messed up... I guess I might have to get the pack, my minimal test instance corrupted the level.dat on load, for some reason...

ghost commented 6 years ago

Yeah, that region file is in the Exoria dimension, opening it without the pack won't load the proper worldgen. You should only have ash, basalt, lava and acid in that world.

For further reference, IronBackpack dealt with this issue a while back: https://github.com/gr8pefish/IronBackpacks/issues/117 According to the commit, they fixed it by only sending the slotindex on ItemStackMessage. https://github.com/gr8pefish/IronBackpacks/commit/f36563fb6b131167a220af9d215e024201be96c8

ghost commented 6 years ago

D'oh, that's also my fault.

I had not given you the right dimension. Exoria dimension is DIM1100, attached zip contains the correct region and folder structure.

Ex-Crash.zip

maruohon commented 6 years ago

Okay I got it to load and tried some things. It seems that putting a drone (the ones that were laying on top of the pipes) into 31 slots is still fine, but the 32nd slot crashes, or if I fill ~5 more slots with the pixie dust.

The strange part is that it even happens when I do it one slot at a time, and the Container is only supposed to sync changed slots, ie. one slot at a time if I fill them like that... So atm this doesn't make sense to me. Also a Trash Buffer full of Drones was fine in my simple test world. Unless those Drones had slightly less NBT data on them or something...

I'll need to investigate this in a dev environment to check how the sync code works, and if it's broken somehow so that it syncs more than it should...

As to the Iron Backbacks fix, that fix isn't applicable to this case. They only needed to send an item usage message to the server, the ItemStack wasn't necessary for that. But here I need to sync the ItemStack in the slots from the server to the client, so that the client knows what's in the slots and can render the items and interact with them.