Quark

Quark

143M Downloads

Edited signs are not saved if edited in a different session from the placement

yezhiyi9670 opened this issue · 2 comments

commented

Edited signs are not saved if edited in a different session from the placement.

Reproduce (singleplayer):

  1. Place two signs. Write something on them.
  2. Edit one sign.
  3. Exit the world, and reopen it.
    ✅ The edited sign still shows the edited text.
  4. Edit the other sign. Exit the world, and reopen it.
    ❎ The sign that is edited later gets reverted to the pre-edit status.
commented

Okay, after some further investigation, I've figured out what causes this bug:

When you place a sign in the world, it initializes an empty SignTileEntity and brings up the editing GUI. You write some text and hit "Done", which sends a CUpdateSignPacket to the server, which saves the text you just wrote to the server-side tile entity, marking it for saving. All good and normal.

What the packet handler does NOT do is ever call setEditable(false) on the sign tile entity - and in the sign tile entity's constructorisEditable defaults to true. This means that when Quark comes along and sends a packet to open up the sign GUI again, isEditable is still true, so the CUpdateSignPacket sent when the GUI is again closed is accepted. This is all working so far.

However, the problem comes when you unload the chunk and reload it. When you unload the chunk, the tile entity gets unloaded from memory, being serialized to disk instead; and when you load the chunk again, it needs to fetch that sign's data from the disk, so it creates a new SignTileEntity and calls SignTileEntity#read. The very first thing that function does is set isEditable to false, and suddenly, there's our issue - the sign is no longer editable. You see, isEditable isn't committed to disk - it's just blindly set to false in the read method. That boolean is checked by the CUpdateSignPacket handler, and lo and behold, trying to edit the reloaded sign, we see a console message:

[20:13:41] [Server thread/WARN] [minecraft/MinecraftServer]: Player offbeatwitch just tried to change non-editable sign

The fix for this is actually quite simple Quark-side: just call setEditable(true) on the server-side SignTileEntity; we already setPlayer to the editing player so this is a one-line change. I'm going to put in a PR for affected versions (1.14+, as far as I know)

commented

Also seeing this behavior occurring when changing dimensions; if you place a sign and edit it right then, it works ok, but if you:

  1. Place a sign in the Nether
  2. Go through a portal and back
  3. Edit the sign
  4. Go through a portal and back

you'll find that the edit has reverted.

Smells sort of like a packet sync issue to me (since changes show client-side until you reload the dimension)