Minecraftforge: `SRecipeBookPacket` needs patch to allow huge number of recipes

[diesieben07]

Minecraft Version: 1.15.2 (1.14.4 probably, too, but not tested there)

Forge Version: 31.2.0 and earlier

Logs: n/a, this issue is not visible in the logs.

When there are too many recipes, SRecipeBookPacket gets too large for the network protocol and fails with the above error. Some kind of workaround patch is needed.

[diesieben07]

After some discussion on Discord, this is a vanilla bug. You can probably reproduce this with just datapacks on vanilla.
Still might want to have a Forge patch here?

[diesieben07]

The fix could be as simple as literally just sending the INIT packet multiple times (splitting the data), as the packet handler appends. This would even work in vanilla.

[LexManos]

Someone needs to make a datapack for vanilla that triggers this.
So that we can submit it to Mojang and have them fix it.
As this is something I told them about from the very beginning and I don't think it should be on us to fix.

[Cyborgmas]

https://bugs.mojang.com/browse/MC-185901

[noncom]

My colleague had created such datapacks and we have verified that they reproduce the problem on Vanilla Minecraft client-server setup.

DataPack_001.zip - this datapack contains 32 994 recipes and exhibits the issue.
DataPack_002.zip - this datapack contains 20 392 tags and also reproduces the problem when being used alone.

The evidence of the second datapack means that it's not only recipes that can happen to be an issue. We think that any datapack content can cause this.

@diesieben07 @LexManos what is the best way to proceed with this, are you going to create a bug to Mojang yourself or should I create it or should I add all this information to the bug created by Cyborgmas?

Just in case: the datapacks are built automatically by data-generators, the project that my colleague used to generate the datapacks is here: https://github.com/metanchik/MinecraftRandomDataPackGenerator it's based on Forge MDK, but it's not a mod and the datapack generation routines have their own entry points.

PS: Ha, @Cyborgmas we had a very close timing :)

[LexManos]

Yes, any colossal packet that is data driven will cause this issue. It is a issue in Mojang's syncing design at the root. It is up to them to change how these things are synced, as it's an obvious issue. So file it with them.
If/When they do fix it. We can see if it's possible to backport.
But again, this is a network protocol issue. That means that we can not do it as we must maintain network compatibility.

[noncom]

Ok, I've added a comment to the issue by Cyborgmas in Mojang JIRA.

[diesieben07]

But again, this is a network protocol issue. That means that we can not do it as we must maintain network compatibility.

The SRecipeBookPacket receving code in vanilla allows sending the packet multiple times, splitting the recipe list.

[diesieben07]

Is there any interest in me spending the effort to implement this splitting workaround?

[diesieben07]

Welp.
More digging and terrible debugging later...
It's not actually SRecipeBookPacket. It's SUpdateRecipes. Which means this cannot be fixed without changing the network protocol, as far as I can tell.

[Cyborgmas]

ah which would explain why tags also caused the issue with the STagsListPacket

[Shadows-of-Fire]

It might be possible to cancel the original packet if it would be too large, and resend split packets if the connection was forge-forge, but I'm not sure if that's worth doing before vanilla implements a fix.