Implement correct sign wrapping (#1228)

* Implement correct sign wrapping

This commit ensures that the auto-wrapping nature of Bedrock with signs is corrected. If a Bedrock player sends a sign that is auto-wrapped, it will now be interpreted by Geyser to fit on multiple lines. Additionally, Geyser will crop incoming sign text to prevent auto-wrapping.

* Don't wrap if it's the last line
This commit is contained in:
Camotoy 2020-09-02 00:38:36 -04:00 committed by GitHub
parent 81a48bf96d
commit dcf1731d8a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 163 additions and 21 deletions

View file

@ -240,6 +240,15 @@ public class GeyserSession implements CommandSender {
@Setter @Setter
private boolean thunder = false; private boolean thunder = false;
/**
* Stores the last text inputted into a sign.
*
* Bedrock sends packets every time you update the sign, Java only wants the final packet.
* Until we determine that the user has finished editing, we save the sign's current status.
*/
@Setter
private String lastSignMessage;
private MinecraftProtocol protocol; private MinecraftProtocol protocol;
public GeyserSession(GeyserConnector connector, BedrockServerSession bedrockServerSession) { public GeyserSession(GeyserConnector connector, BedrockServerSession bedrockServerSession) {

View file

@ -32,17 +32,11 @@ import com.nukkitx.protocol.bedrock.packet.BlockEntityDataPacket;
import org.geysermc.connector.network.session.GeyserSession; import org.geysermc.connector.network.session.GeyserSession;
import org.geysermc.connector.network.translators.PacketTranslator; import org.geysermc.connector.network.translators.PacketTranslator;
import org.geysermc.connector.network.translators.Translator; import org.geysermc.connector.network.translators.Translator;
import org.geysermc.connector.utils.SignUtils;
import java.util.HashMap;
import java.util.Map;
@Translator(packet = BlockEntityDataPacket.class) @Translator(packet = BlockEntityDataPacket.class)
public class BedrockBlockEntityDataTranslator extends PacketTranslator<BlockEntityDataPacket> { public class BedrockBlockEntityDataTranslator extends PacketTranslator<BlockEntityDataPacket> {
// In case two people are editing signs at the same time this array holds the temporary messages to be sent
// Position -> Message being held
protected static Map<Position, String> lastMessages = new HashMap<>();
@Override @Override
public void translate(BlockEntityDataPacket packet, GeyserSession session) { public void translate(BlockEntityDataPacket packet, GeyserSession session) {
NbtMap tag = packet.getData(); NbtMap tag = packet.getData();
@ -50,9 +44,8 @@ public class BedrockBlockEntityDataTranslator extends PacketTranslator<BlockEnti
// This is the reason why this all works - Bedrock sends packets every time you update the sign, Java only wants the final packet // This is the reason why this all works - Bedrock sends packets every time you update the sign, Java only wants the final packet
// But Bedrock sends one final packet when you're done editing the sign, which should be equal to the last message since there's no edits // But Bedrock sends one final packet when you're done editing the sign, which should be equal to the last message since there's no edits
// So if the latest update does not match the last cached update then it's still being edited // So if the latest update does not match the last cached update then it's still being edited
Position pos = new Position(tag.getInt("x"), tag.getInt("y"), tag.getInt("z")); if (!tag.getString("Text").equals(session.getLastSignMessage())) {
if (!tag.getString("Text").equals(lastMessages.get(pos))) { session.setLastSignMessage(tag.getString("Text"));
lastMessages.put(pos, tag.getString("Text"));
return; return;
} }
// Otherwise the two messages are identical and we can get to work deconstructing // Otherwise the two messages are identical and we can get to work deconstructing
@ -61,29 +54,61 @@ public class BedrockBlockEntityDataTranslator extends PacketTranslator<BlockEnti
// (Initialized all with empty strings because it complains about null) // (Initialized all with empty strings because it complains about null)
String[] lines = new String[] {"", "", "", ""}; String[] lines = new String[] {"", "", "", ""};
int iterator = 0; int iterator = 0;
// Keep track of the width of each character
// If it goes over the maximum, we need to start a new line to match Java
int widthCount = 0;
// This converts the message into the array'd message Java wants // This converts the message into the array'd message Java wants
for (char character : tag.getString("Text").toCharArray()) { for (char character : tag.getString("Text").toCharArray()) {
// If we get a return in Bedrock, that signals to use the next line. widthCount += SignUtils.getCharacterWidth(character);
if (character == '\n') { // If we get a return in Bedrock, or go over the character width max, that signals to use the next line.
if (character == '\n' || widthCount > SignUtils.JAVA_CHARACTER_WIDTH_MAX) {
// We need to apply some more logic if we went over the character width max
boolean wentOverMax = widthCount > SignUtils.JAVA_CHARACTER_WIDTH_MAX && character != '\n';
widthCount = 0;
// Saves if we're moving a word to the next line
String word = null;
if (wentOverMax && iterator < lines.length - 1) {
// If we went over the max, we want to try to wrap properly like Bedrock does.
// So we look for a space in the Bedrock user's text to imply a word.
int index = newMessage.lastIndexOf(" ");
if (index != -1) {
// There is indeed a space in this line; let's get it
word = newMessage.substring(index + 1);
// 'Delete' that word from the string builder
newMessage.delete(index, newMessage.length());
}
}
lines[iterator] = newMessage.toString(); lines[iterator] = newMessage.toString();
iterator++; iterator++;
// Bedrock, for whatever reason, can hold a message out of bounds // Bedrock, for whatever reason, can hold a message out of the bounds of the four lines
// We don't care about that so we discard that // We don't care about that so we discard that
if (iterator > lines.length - 1) { if (iterator > lines.length - 1) {
break; break;
} }
newMessage = new StringBuilder(); newMessage = new StringBuilder();
if (wentOverMax) {
// Apply the wrapped word to the new line
if (word != null) {
newMessage.append(word);
// And apply the width count
for (char wordCharacter : word.toCharArray()) {
widthCount += SignUtils.getCharacterWidth(wordCharacter);
}
}
// If we went over the max, we want to append the character to the new line.
newMessage.append(character);
widthCount += SignUtils.getCharacterWidth(character);
}
} else newMessage.append(character); } else newMessage.append(character);
} }
// Put the final line on since it isn't done in the for loop // Put the final line on since it isn't done in the for loop
if (iterator < lines.length) lines[iterator] = newMessage.toString(); if (iterator < lines.length) lines[iterator] = newMessage.toString();
Position pos = new Position(tag.getInt("x"), tag.getInt("y"), tag.getInt("z"));
ClientUpdateSignPacket clientUpdateSignPacket = new ClientUpdateSignPacket(pos, lines); ClientUpdateSignPacket clientUpdateSignPacket = new ClientUpdateSignPacket(pos, lines);
session.sendDownstreamPacket(clientUpdateSignPacket); session.sendDownstreamPacket(clientUpdateSignPacket);
//TODO (potentially): originally I was going to update the sign blocks so Bedrock and Java users would match visually
// However Java can still store a lot per-line and visuals are still messed up so that doesn't work
// We remove the sign position from map to indicate there is no work-in-progress sign // We set the sign text cached in the session to null to indicate there is no work-in-progress sign
lastMessages.remove(pos); session.setLastSignMessage(null);
} }
} }

View file

@ -29,6 +29,7 @@ import com.github.steveice10.mc.protocol.data.message.MessageSerializer;
import com.github.steveice10.opennbt.tag.builtin.CompoundTag; import com.github.steveice10.opennbt.tag.builtin.CompoundTag;
import com.nukkitx.nbt.NbtMap; import com.nukkitx.nbt.NbtMap;
import org.geysermc.connector.utils.MessageUtils; import org.geysermc.connector.utils.MessageUtils;
import org.geysermc.connector.utils.SignUtils;
import java.util.HashMap; import java.util.HashMap;
import java.util.Map; import java.util.Map;
@ -46,9 +47,17 @@ public class SignBlockEntityTranslator extends BlockEntityTranslator {
String signLine = getOrDefault(tag.getValue().get("Text" + currentLine), ""); String signLine = getOrDefault(tag.getValue().get("Text" + currentLine), "");
signLine = MessageUtils.getBedrockMessage(MessageSerializer.fromString(signLine)); signLine = MessageUtils.getBedrockMessage(MessageSerializer.fromString(signLine));
//Java allows up to 16+ characters on certain symbols. // Check the character width on the sign to ensure there is no overflow that is usually hidden
if(signLine.length() >= 15 && (signLine.contains("-") || signLine.contains("="))) { // to Java Edition clients but will appear to Bedrock clients
signLine = signLine.substring(0, 14); int signWidth = 0;
StringBuilder finalSignLine = new StringBuilder();
for (char c : signLine.toCharArray()) {
signWidth += SignUtils.getCharacterWidth(c);
if (signWidth <= SignUtils.BEDROCK_CHARACTER_WIDTH_MAX) {
finalSignLine.append(c);
} else {
break;
}
} }
// Java Edition 1.14 added the ability to change the text color of the whole sign using dye // Java Edition 1.14 added the ability to change the text color of the whole sign using dye
@ -56,7 +65,7 @@ public class SignBlockEntityTranslator extends BlockEntityTranslator {
signText.append(getBedrockSignColor(tag.get("Color").getValue().toString())); signText.append(getBedrockSignColor(tag.get("Color").getValue().toString()));
} }
signText.append(signLine); signText.append(finalSignLine.toString());
signText.append("\n"); signText.append("\n");
} }

View file

@ -0,0 +1,99 @@
/*
* Copyright (c) 2019-2020 GeyserMC. http://geysermc.org
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*
* @author GeyserMC
* @link https://github.com/GeyserMC/Geyser
*/
package org.geysermc.connector.utils;
/**
* Provides utilities for interacting with signs. Mainly, it deals with the widths of each character.
* Since Bedrock auto-wraps signs and Java does not, we have to take this into account when translating signs.
*/
public class SignUtils {
// TODO: If we send the Java font via resource pack, does width change?
/**
* The maximum character width that a sign can hold in Bedrock
*/
public static final int BEDROCK_CHARACTER_WIDTH_MAX = 88;
/**
* The maximum character width that a sign can hold in Java
*/
public static final int JAVA_CHARACTER_WIDTH_MAX = 90;
/**
* Gets the Minecraft width of a character
* @param c character to determine
* @return width of the character
*/
public static int getCharacterWidth(char c) {
switch (c) {
case '!':
case ',':
case '.':
case ':':
case ';':
case 'i':
case '|':
case '¡':
return 2;
case '\'':
case 'l':
case 'ì':
case 'í':
return 3;
case ' ':
case 'I':
case '[':
case ']':
case 't':
case '×':
case 'ï':
return 4;
case '"':
case '(':
case ')':
case '*':
case '<':
case '>':
case 'f':
case 'k':
case '{':
case '}':
return 5;
case '@':
case '~':
case '®':
return 7;
default:
return 6;
}
}
}