Paper icon indicating copy to clipboard operation
Paper copied to clipboard

Escaping edit of sign w/ legacy colors removes legacy colors

Open aurorasmiles opened this issue 2 years ago • 4 comments

Expected behavior

Signs using legacy colors should be handled the same way as sign with components: When editing the sign and pressing ESC, the sign should not change

Observed/Actual behavior

When editing a sign with legacy styling and escaping without doing any changes, the text looses all legacy styling

Steps/models to reproduce

  1. Place a sign with legacy colors (I used setblock via console: setblock -4 64 -6 minecraft:oak_sign[rotation=8,waterlogged=false]{back_text:{color:"black",has_glowing_text:0b,messages:['{"text":""}','{"text":""}','{"text":""}','{"text":""}']},front_text:{color:"black",has_glowing_text:0b,messages:['{"text":"§1test"}','{"text":""}','{"text":""}','{"text":""}']},is_waxed:0b} replace
  2. Right click on the sign
  3. Press escape

Plugin and Datapack List

none

Paper version

git-Paper-49 (MC: 1.20.1) (Implementing API version 1.20.1-R0.1-SNAPSHOT) (Git: e8bec64)

Other

Upstream seems to have fixed this bug for components in https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/commits/66c5ce4c7ba6a32b250c4630eb4c189fc5483542#nms-patches/net/minecraft/world/level/block/entity/TileEntitySign.patch but probably forgot about legacy stuff

aurorasmiles avatar Jun 26 '23 16:06 aurorasmiles

Placing legacy text in components is pretty much 100% unsupported and is considered malformed

electronicboy avatar Jun 26 '23 16:06 electronicboy

I'm aware of that, but old legacy signs still exist. Unless there is a nice way to just convert all old legacy signs?

aurorasmiles avatar Jun 26 '23 16:06 aurorasmiles

The issue here is likely going to be the fact that the signs are parsed back and forth between legacy and JSON, id imagine that if you look at the contents it's comparing it's different

There was logic for converting components, but generally maintaining that stuff was a headache and not ideal for state people shouldn't be inducing

electronicboy avatar Jun 26 '23 16:06 electronicboy

Ok, so there are a couple things to consider here. We cannot allow clients to send sign update packets with legacy codes in the strings. If we allow it, modified clients can just create colored signs whenever they want. So the alternative is to convert the legacy text to proper components when the sign tile entity is loaded. We already do similar things for itemstack names and lore.

This does mean that editing the signs won't show the colored text, but the root style is applied on the edited text, so the style is preserved.

Machine-Maker avatar Aug 16 '23 17:08 Machine-Maker

Closing as wontfix as it's getting less and less feasible to inject logic to fix this into the codecs required. The proper fix is for people not to use legacy color codes, and use proper components.

Machine-Maker avatar Sep 24 '24 00:09 Machine-Maker