search

PaperMC / Paper

The most widely used, high performance Minecraft server that aims to fix gameplay and mechanics inconsistencies
https://papermc.io/
Other
9.44k stars 2.21k forks source link

Escaping edit of sign w/ legacy colors removes legacy colors #9399

Open aurorasmiles opened 1 year ago

aurorasmiles commented 1 year ago

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

electronicboy commented 1 year ago

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

aurorasmiles commented 1 year ago

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

electronicboy commented 1 year ago

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

Machine-Maker commented 11 months ago

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.