search

klikli-dev / modonomicon

Data-driven minecraft in-game documentation with progress visualization.
22 stars 9 forks source link

SyncBookUnlockStatesMessage breaks with large / multiple books in the same pack #184

Closed DaFuqs closed 3 months ago

DaFuqs commented 3 months ago

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

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 image

System (please complete the following information):

klikli-dev commented 3 months ago

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 commented 3 months ago

@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 commented 3 months ago

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

image

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.