SyncBookUnlockStatesMessage breaks with large / multiple books in the same pack

Question

SyncBookUnlockStatesMessage breaks with large / multiple books in the same pack

DaFuqs opened this issue 8 months ago · 3 comments

Describe the bug
When joining a server or singleplayer, Modonomicon syncs the unlocks of the books from Server->Client using SyncBookUnlockStatesMessage.
When multiple medium sized books are installed, or a single book with lots of content, the encode() method throws an exception, because the the buffers size is exceeded.

Stacktrace:

[15:17:20] [Server thread/ERROR] (Minecraft) Error executing task on Server
 io.netty.handler.codec.EncoderException: String too big (was 41335 characters, max 32767)
	at net.minecraft.network.PacketByteBuf.writeString(PacketByteBuf.java:706) ~[minecraft-merged-390f9123b4-1.20.1-net.fabricmc.yarn.1_20_1.1.20.1+build.10-v2.jar:?]
	at net.minecraft.network.PacketByteBuf.writeString(PacketByteBuf.java:701) ~[minecraft-merged-390f9123b4-1.20.1-net.fabricmc.yarn.1_20_1.1.20.1+build.10-v2.jar:?]
	at net.minecraft.network.PacketByteBuf.encodeAsJson(PacketByteBuf.java:159) ~[minecraft-merged-390f9123b4-1.20.1-net.fabricmc.yarn.1_20_1.1.20.1+build.10-v2.jar:?]
	at com.klikli_dev.modonomicon.networking.SyncBookUnlockStatesMessage.encode(SyncBookUnlockStatesMessage.java:38) ~[modonomicon-1.20.1-fabric-1.62.0.jar:?]
	at com.klikli_dev.modonomicon.network.FabricNetworkHelper.sendTo(FabricNetworkHelper.java:20) ~[modonomicon-1.20.1-fabric-1.62.0.jar:?]
	at com.klikli_dev.modonomicon.bookstate.BookUnlockStateManager.syncFor(BookUnlockStateManager.java:55) ~[modonomicon-1.20.1-fabric-1.62.0.jar:?]
	at com.klikli_dev.modonomicon.bookstate.BookUnlockStateManager.updateAndSyncFor(BookUnlockStateManager.java:62) ~[modonomicon-1.20.1-fabric-1.62.0.jar:?]
	at com.klikli_dev.modonomicon.bookstate.BookUnlockStateManager$1.lambda$run$0(BookUnlockStateManager.java:72) ~[modonomicon-1.20.1-fabric-1.62.0.jar:?]
	at net.minecraft.server.ServerTask.run(ServerTask.java:18) ~[minecraft-merged-390f9123b4-1.20.1-net.fabricmc.yarn.1_20_1.1.20.1+build.10-v2.jar:?]
	at net.minecraft.util.thread.ThreadExecutor.executeTask(ThreadExecutor.java:156) ~[minecraft-merged-390f9123b4-1.20.1-net.fabricmc.yarn.1_20_1.1.20.1+build.10-v2.jar:?]
	at net.minecraft.util.thread.ReentrantThreadExecutor.executeTask(ReentrantThreadExecutor.java:23) ~[minecraft-merged-390f9123b4-1.20.1-net.fabricmc.yarn.1_20_1.1.20.1+build.10-v2.jar:?]
	at net.minecraft.server.MinecraftServer.executeTask(MinecraftServer.java:782) ~[minecraft-merged-390f9123b4-1.20.1-net.fabricmc.yarn.1_20_1.1.20.1+build.10-v2.jar:?]
	at net.minecraft.server.MinecraftServer.executeTask(MinecraftServer.java:164) ~[minecraft-merged-390f9123b4-1.20.1-net.fabricmc.yarn.1_20_1.1.20.1+build.10-v2.jar:?]
	at net.minecraft.util.thread.ThreadExecutor.runTask(ThreadExecutor.java:130) ~[minecraft-merged-390f9123b4-1.20.1-net.fabricmc.yarn.1_20_1.1.20.1+build.10-v2.jar:?]
	at net.minecraft.server.MinecraftServer.runOneTask(MinecraftServer.java:764) ~[minecraft-merged-390f9123b4-1.20.1-net.fabricmc.yarn.1_20_1.1.20.1+build.10-v2.jar:?]
	at net.minecraft.server.MinecraftServer.runTask(MinecraftServer.java:758) ~[minecraft-merged-390f9123b4-1.20.1-net.fabricmc.yarn.1_20_1.1.20.1+build.10-v2.jar:?]
	at net.minecraft.util.thread.ThreadExecutor.runTasks(ThreadExecutor.java:115) ~[minecraft-merged-390f9123b4-1.20.1-net.fabricmc.yarn.1_20_1.1.20.1+build.10-v2.jar:?]
	at net.minecraft.server.MinecraftServer.runTasksTillTickEnd(MinecraftServer.java:742) ~[minecraft-merged-390f9123b4-1.20.1-net.fabricmc.yarn.1_20_1.1.20.1+build.10-v2.jar:?]
	at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:675) ~[minecraft-merged-390f9123b4-1.20.1-net.fabricmc.yarn.1_20_1.1.20.1+build.10-v2.jar:?]
	at net.minecraft.server.MinecraftServer.method_29739(MinecraftServer.java:265) ~[minecraft-merged-390f9123b4-1.20.1-net.fabricmc.yarn.1_20_1.1.20.1+build.10-v2.jar:?]
	at java.lang.Thread.run(Thread.java:833) ~[?:?]

To Reproduce

Create a modonomicon book with many, many entries
Join singleplayer

Expected behavior
The sync not breaking. Maybe by splitting the sync up into smaller chunks, or increasing the cap?

Screenshots

A screenshot at the time of sync shortly before the crash. It shows all data that would be synced

System (please complete the following information):

Modonomicon Version: 1.62.0; Fabric
OS: Windows
Minecraft Version: 1.20.1
Modpack Link and Version, or list of mods: Just Modonomicon & Spectrum

klikli-dev · Answer 1 · 2024-03-27T14:34:34.000Z

That's a serious issue, and a bit annoying on MCs side for not handling that (considering that packets one level lower can be split).
I think the best approach is probably to use the deprecated "codec to nbt" encoder instead

klikli-dev · Answer 2 · 2024-03-27T14:41:53.000Z

@DaFuqs please let me know if that fixes it.
It may lead to a different error ( packet too big) because I am not sure if I have packet splitting set up on fabric as I don't think they offer it out of the box

DaFuqs · Answer 3 · 2024-03-28T14:57:51.000Z

I tested various combinations in new and old worlds with read and unread entries. I can confirm that resolved this issue.

The "mark all unlocked entries as read" button still does not seem to show up when first opening the book after join, but I will open a separate issue for that.

Share to