Skip to content

Fix: handle object components in dialog button labels #6502

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
andreiusq wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix: handle object components in dialog button labels #6502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 78 additions & 2 deletions core/src/main/java/org/geysermc/geyser/session/dialog/DialogButton.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
package org.geysermc.geyser.session.dialog;

import org.cloudburstmc.nbt.NbtMap;
import org.geysermc.cumulus.util.FormImage;
import org.geysermc.geyser.session.GeyserSession;
import org.geysermc.geyser.session.dialog.action.DialogAction;
import org.geysermc.geyser.translator.text.MessageTranslator;
Expand All @@ -34,7 +35,11 @@
import java.util.List;
import java.util.Optional;

public record DialogButton(String label, Optional<DialogAction> action) {
public record DialogButton(String label, Optional<FormImage> icon, Optional<DialogAction> action) {

public DialogButton(String label, Optional<DialogAction> action) {
this(label, Optional.empty(), action);
}

public static List<DialogButton> readList(Optional<GeyserSession> session, List<NbtMap> tag, Dialog.IdGetter idGetter) {
if (tag == null) {
Expand All @@ -51,6 +56,77 @@ public static Optional<DialogButton> read(Optional<GeyserSession> session, Objec
if (!(tag instanceof NbtMap map)) {
return Optional.empty();
}
return Optional.of(new DialogButton(MessageTranslator.convertFromNullableNbtTag(session, map.get("label")), DialogAction.read(map.get("action"), idGetter)));
Object label = map.get("label");
return Optional.of(new DialogButton(
MessageTranslator.convertFromNullableNbtTag(session, label),
iconFromLabel(label),
DialogAction.read(map.get("action"), idGetter)));
}

/**
* Java allows a button label to embed an atlas sprite (a {@code type: "object"} component). Bedrock cannot render
* a sprite inside text, but a {@link org.geysermc.cumulus.form.SimpleForm} button can show an image next to its
* text, so we surface the first sprite in the label as the button's icon.
*/
private static Optional<FormImage> iconFromLabel(Object label) {
NbtMap sprite = findSprite(label);
if (sprite == null) {
return Optional.empty();
}
return spriteTexturePath(sprite.getString("atlas", ""), sprite.getString("sprite", ""))
.map(path -> FormImage.of(FormImage.Type.PATH, path));
}

/**
* Recursively searches a label component for the first atlas sprite ({@code type: "object"} with a {@code sprite}).
*/
private static NbtMap findSprite(Object tag) {
if (tag instanceof NbtMap map) {
if ("object".equals(map.getString("type", null)) && map.containsKey("sprite")) {
return map;
}
Object extra = map.get("extra");
if (extra != null) {
return findSprite(extra);
}
} else if (tag instanceof List<?> list) {
for (Object entry : list) {
NbtMap found = findSprite(entry);
if (found != null) {
return found;
}
}
}
return null;
}

/**
* Best-effort translation of a Java atlas sprite to the equivalent vanilla Bedrock texture path. Java lays item
* and block textures out under {@code textures/item} and {@code textures/block}, while Bedrock uses the pluralized
* {@code textures/items} and {@code textures/blocks}. Leaf names do not always match between the two editions, so
* an unrecognized sprite resolves to no icon (Bedrock simply renders no image) rather than a wrong one.
*/
public static Optional<String> spriteTexturePath(String atlas, String sprite) {
if (!atlas.equals("minecraft:items") && !atlas.equals("minecraft:blocks")) {
return Optional.empty();
}
// Sprites may be namespaced (e.g. "minecraft:item/barrier"); we only care about the path.
int colon = sprite.indexOf(':');
if (colon != -1) {
sprite = sprite.substring(colon + 1);
}
int slash = sprite.indexOf('/');
if (slash == -1) {
return Optional.empty();
}
String bedrockCategory = switch (sprite.substring(0, slash)) {
case "item" -> "items";
case "block" -> "blocks";
default -> null;
};
if (bedrockCategory == null) {
return Optional.empty();
}
return Optional.of("textures/" + bedrockCategory + "/" + sprite.substring(slash + 1));
}
}
14 changes: 12 additions & 2 deletions core/src/main/java/org/geysermc/geyser/session/dialog/DialogWithButtons.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.geysermc.cumulus.component.DropdownComponent;
import org.geysermc.cumulus.form.CustomForm;
import org.geysermc.cumulus.form.SimpleForm;
import org.geysermc.cumulus.util.FormImage;
import org.geysermc.geyser.session.GeyserSession;
import org.geysermc.geyser.session.dialog.input.ParsedInputs;
import org.geysermc.geyser.text.GeyserLocale;
Expand Down Expand Up @@ -73,9 +74,9 @@ protected void addCustomComponents(DialogHolder holder, CustomForm.Builder build
protected void addCustomComponents(DialogHolder holder, SimpleForm.Builder builder) {
List<DialogButton> buttons = buttons(holder);
for (DialogButton button : buttons) {
builder.button(button.label());
addButton(builder, button);
}
exitAction.ifPresent(button -> builder.button(button.label()));
exitAction.ifPresent(button -> addButton(builder, button));

builder.validResultHandler(response -> {
if (response.clickedButtonId() == buttons.size()) {
Expand All @@ -86,6 +87,15 @@ protected void addCustomComponents(DialogHolder holder, SimpleForm.Builder build
});
}

private static void addButton(SimpleForm.Builder builder, DialogButton button) {
Optional<FormImage> icon = button.icon();
if (icon.isPresent()) {
builder.button(button.label(), icon.get());
} else {
builder.button(button.label());
}
}

@Override
protected Optional<DialogButton> onCancel() {
return exitAction;
Expand Down
11 changes: 9 additions & 2 deletions core/src/main/java/org/geysermc/geyser/session/dialog/NoticeDialog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.cloudburstmc.nbt.NbtMap;
import org.geysermc.cumulus.form.CustomForm;
import org.geysermc.cumulus.form.SimpleForm;
import org.geysermc.cumulus.util.FormImage;
import org.geysermc.geyser.session.GeyserSession;
import org.geysermc.geyser.session.dialog.input.ParsedInputs;
import org.geysermc.geyser.util.MinecraftKey;
Expand Down Expand Up @@ -58,7 +59,13 @@ protected void addCustomComponents(DialogHolder holder, CustomForm.Builder build

@Override
protected void addCustomComponents(DialogHolder holder, SimpleForm.Builder builder) {
builder.button(button.map(DialogButton::label).orElse("gui.ok"))
.validResultHandler(response -> holder.runButton(button, ParsedInputs.EMPTY));
String label = button.map(DialogButton::label).orElse("gui.ok");
Optional<FormImage> icon = button.flatMap(DialogButton::icon);
if (icon.isPresent()) {
builder.button(label, icon.get());
} else {
builder.button(label);
}
builder.validResultHandler(response -> holder.runButton(button, ParsedInputs.EMPTY));
}
}
30 changes: 19 additions & 11 deletions core/src/main/java/org/geysermc/geyser/translator/text/MessageTranslator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ private static Component componentFromNbtTag(Object nbtTag, Style style) {
} else if (nbtTag instanceof List<?> list) {
return Component.join(JoinConfiguration.noSeparators(), componentsFromNbtList(list, style));
} else if (nbtTag instanceof NbtMap map) {
Component component = null;
Component component;
String text = map.getString("text", map.getString("", null));
if (text != null) {
component = Component.text(text);
Expand All @@ -573,23 +573,31 @@ private static Component componentFromNbtTag(Object nbtTag, Style style) {
args.add(componentFromNbtTag(with, style));
}
component = Component.translatable(translateKey, fallback, args);
} else {
// No renderable text content. This is expected for "object" components (atlas sprites, player
// heads, ...) which Bedrock cannot render inline; anything else is unexpected and still worth
// logging. Either way we fall back to an empty base so styling and any child components in
// "extra" are still rendered, rather than dropping the whole component (and the text around
// it) on the floor. See GeyserMC/Geyser#6499.
if (!"object".equals(map.getString("type", null))) {
GeyserImpl.getInstance().getLogger().error("Expected tag to be a literal string, a list of components, or a component object with a text/translate key: " + nbtTag);
}
component = Component.empty();
}
}

if (component != null) {
Style newStyle = getStyleFromNbtMap(map, style);
component = component.style(newStyle);
Style newStyle = getStyleFromNbtMap(map, style);
component = component.style(newStyle);

Object extra = map.get("extra");
if (extra != null) {
component = component.append(componentFromNbtTag(extra, newStyle));
}

return component;
Object extra = map.get("extra");
if (extra != null) {
component = component.append(componentFromNbtTag(extra, newStyle));
}

return component;
}

GeyserImpl.getInstance().getLogger().error("Expected tag to be a literal string, a list of components, or a component object with a text/translate key: " + nbtTag);
GeyserImpl.getInstance().getLogger().error("Expected tag to be a literal string, a list of components, or a component object: " + nbtTag);
return Component.empty();
}

Expand Down
52 changes: 52 additions & 0 deletions core/src/test/java/org/geysermc/geyser/network/translators/chat/MessageTranslatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@

import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import net.kyori.adventure.text.Component;
import net.kyori.adventure.text.serializer.plain.PlainTextComponentSerializer;
import org.cloudburstmc.nbt.NbtList;
import org.cloudburstmc.nbt.NbtMap;
import org.cloudburstmc.nbt.NbtType;
import org.geysermc.geyser.session.dialog.DialogButton;
import org.geysermc.geyser.translator.text.MessageTranslator;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
Expand Down Expand Up @@ -87,6 +94,51 @@ public void convertMessage() {
}
}

// https://github.com/GeyserMC/Geyser/issues/6499 - an "object" component (atlas sprite / player head) has no
// textual representation on Bedrock, but it must not swallow the text around it.
@Test
public void objectComponentKeepsSurroundingText() {
// Minecraft normalizes the array [<object>, " Hello world"] into the object component with the text in "extra".
NbtMap normalized = NbtMap.builder()
.putString("type", "object")
.putString("atlas", "minecraft:items")
.putString("sprite", "item/barrier")
.putList("extra", NbtType.STRING, " Hello world")
.build();
Component normalizedComponent = MessageTranslator.componentFromNbtTag(normalized);
Assertions.assertEquals(" Hello world", PlainTextComponentSerializer.plainText().serialize(normalizedComponent),
"Object component must not drop the text in its extra");

// The same content expressed as a plain list of components.
NbtMap object = NbtMap.builder()
.putString("type", "object")
.putString("atlas", "minecraft:items")
.putString("sprite", "item/barrier")
.build();
NbtMap text = NbtMap.builder().putString("text", " Hello world").build();
NbtList<NbtMap> list = new NbtList<>(NbtType.COMPOUND, object, text);
Component listComponent = MessageTranslator.componentFromNbtTag(list);
Assertions.assertEquals(" Hello world", PlainTextComponentSerializer.plainText().serialize(listComponent),
"Object component in a list must not drop the sibling text");
}

// https://github.com/GeyserMC/Geyser#6499 - atlas sprites in a button label become the Bedrock button's icon.
@Test
public void spriteMapsToBedrockTexturePath() {
Assertions.assertEquals(Optional.of("textures/items/barrier"),
DialogButton.spriteTexturePath("minecraft:items", "item/barrier"));
Assertions.assertEquals(Optional.of("textures/blocks/stone"),
DialogButton.spriteTexturePath("minecraft:blocks", "block/stone"));
// Namespaced sprite ids are accepted too.
Assertions.assertEquals(Optional.of("textures/items/apple"),
DialogButton.spriteTexturePath("minecraft:items", "minecraft:item/apple"));
// Atlases and sprite categories we can't predict resolve to no icon rather than a wrong one.
Assertions.assertEquals(Optional.empty(),
DialogButton.spriteTexturePath("minecraft:mob_effects", "effect/speed"));
Assertions.assertEquals(Optional.empty(),
DialogButton.spriteTexturePath("minecraft:items", "barrier"));
}

@Test
public void convertMessageLenient() {
Assertions.assertEquals("\n\n\n\n", MessageTranslator.convertMessageLenient("\n\n\n\n"), "All newline message is not handled properly");
Expand Down
Loading