Actually Additions

Actually Additions

66M Downloads

Don't send Crate NBT to the client

Eufranio opened this issue ยท 18 comments

commented

Title. People can put crates inside crates and may cause a playerdata corruption. Pipes should also be forbidden to insert crates into crates.

commented

how should this cause a playerdata corruption. As far as I know the crates doesn't store any data inside the playerdata and nothing else in ActAdd too

commented

You can store Storage Crates that have items in them after they've been picked up with a Storage Crate Keeper in them inside other Storage Crates.
This doesn't cause any data corruption though, so please explain.

commented

sameer/ExtraUtilities#5
This happens with many items that store other items.

commented

But no ActAdd block or items stores playerdata, everything is nbt and is only saved inside the world itself, like every modder should do it. If AE2 makes such bad things it's not our problem

commented

AE2 is just an example. What if players fill crates with thousands and thousands of items, heavy items maybe (with a lot of NBT) and store crates inside crates. The NBT would be too long, and probably kick the player when receiving the packets. It would not corrupt, but would turn the game unplayable since you wont be able to join anymore, unless you delete your stuff.

commented

If a player break a crate with too much data inside, he will probably be kicked, just like the issue I linked. See the problem?

commented

sigh.
I guess I just won't make it send the NBT to the client.

commented

So, how would you handle this? Load the NBT only on place?

commented

if the NBT kick only happens while the crate is in item form, you could just not sync the data while it is an item. once placed, syncing has to be done like usual.
If kicks also happen while placed, you'd had to split up the NBT data into multiple packets and reassemble them on the client side.

commented

The data of the block by default isn't sent to the client until opened, so just dont sync the NBT of the item is fine.

commented

This is exactly what I said.
Should the client also get kicked during usual sync in block form (aka when a crate gets opened/items get changed by the player/...), you'd have to replace the normal Minecraft inventory syncing mechanism to split up the NBT into multiple packets.

commented

@Eufranio Why you don't test how many crates you can fill with items and put inside the next one up to the crash point

commented

@canitzp that doesn't really say much if you don't also log the nbt data length when syncing

commented

@XDjackieXD I don't want to get a crash report, I want to know how many crates you can put in each other or other methods to overload the nbt and if it is normal for a player to do such things

commented

this depends of what is inside this crates as nbt gets longer with the amount of stuff stored in it. this of course also gets longer when placing items inside a crate that use nbt themself (like for an opencomputers tablet).
This is why "trying it" isn't really useful without logging of the size of the data it attempts to send

commented

Also I don't know what causes the crash. Minecraft allows up to 2GB of payload data per packet (the integer encoding the payload length can go up to 2147483647) and TCP should fragment a packet once it gets bigger than the MTU (which is usually around 1.5kB).
@Eufranio mind sending us the crash log so I can try to figure out what in MC is crashing when receiving such a long packet?

commented

Please stop commenting on this.
I know the solution already and I will implement it once I get the time.

commented

Closed by 3c07f45