DimensionalDevelopment / JustEnoughIDs

Use the 1.13 chunk format in 1.12 to remove the block, item, and biome ID limits
MIT License
31 stars 33 forks source link

PacketBuffer read/writeItemStack transformation #106

Closed TheBlueCrystal closed 4 years ago

TheBlueCrystal commented 4 years ago

Hello,

first I wanted to thank you very much for that really great mod and all of your effort going into it. It made my giant modpack (444 Mods in it, yeah :) ) possible in the first place, and Block, Item and Biome IDs would have run out long ago without it.

Lately I stumbled about some problems with Project-E and Project-EX, being not able to take some items / blocks out of their transmutation table. First I thought that this is a random behavior, but finally I realized that all blocks and items that could not be spawned had an ID above 32767 assigned. I took a look at their code and found out, that they both use net.minecraft.network.PacketBuffer.readItemStack(ByteBuf) and net.minecraft.network.PacketBuffer.writeItemStack(ByteBuf, Itemstack) to notify the other end of blocks / items that have been taken out of the output slots. Project-E does this implicitly using an InventoryContainer and Project-EX does send explicitly a custom message using ByteBufUtils (which in turn uses PacketBuffer and said methods to serialize the ItemStack). I also looked through your Mixins and transformers and could not find a location where those 2 methods would get patched. The problem is that those two functions originally use a ShortInt for the Item-ID and so for blocks/items having an ID above 32767 the value would go negative and the ItemStack cannot be reconstructed on the other side of the network-connection. Perhaps I overlooked something in your code, and you already transform those two methods, and there is a special case in my modack, why this methods don't get patched. (But I did not see any errors regarding failed transformers). (Or perhaps I'm the first one who hit the 32000 Blocks / Item Limit anyhow :-) )

Could you please have a look at those 2 methods and perhaps add a transformation, if there is not already one in place? (Or tell me that those methods get already patched, and I have to investigate further, why this does not happen in my pack).

Thank you very much in advance! :)

sam-kirby commented 4 years ago

JustEnoughIDs-1.0.3-SNAPSHOT.zip

Can you try this build and let me know if it fixes things for you?

Don't decompress it, just rename it to .jar. Github doesn't let you upload jar files but is fine with zips despite them being the same thing.

TheBlueCrystal commented 4 years ago

Hello @sam-kirby ,

thank you for your quick reply! I have tested the snapshot and when I first started the game, it crashed with an NPE (seemingly unrelated) when Modern-Metals mod tried to register it's ingots in the Industrialcraft2 metal-former. (I can send you the crash-report and/or the log if you think it's somehow important / could be related).

I then started the game again (without any modifications) and it successfully initialized to it's title screen. When I loaded my former map it complained about 6 missing items from Atum, but started up successfully. The patch solves indeed the problems with Project-E! :)

Thank you very, very much for your effort! And please keep up the really good work! :)