From 678c7aa3a42aaf64b8b1b7df75e787cbec9fe2ad Mon Sep 17 00:00:00 2001 From: Kezz Date: Wed, 21 May 2025 18:06:26 +0100 Subject: [PATCH] Modern-ify Adventure uses and fix bug in TranslatableMapper (#1575) * fix: Don't ignore the player's locale in message translation * feature: Use PointersSupplier to save constructing a Pointers instance for every player * fix: Don't use a custom implementation of Identity for players We don't need to carry about this object for every player. * chore: Stop using deprecated TranslationRegistry * fix: Simplify TranslatableMapper and fix bugs - The fallback string is not intended to be translated, so don't do that. - Check if the string can be translated in the default locale before using the closest mapper as devs may have their own strings. - Remove the hardcoded check for TranslationRegistry instance as devs (and us now) can use non-TranslationRegistry translator instances. --- .../velocitypowered/proxy/VelocityServer.java | 7 +-- .../connection/client/ConnectedPlayer.java | 44 +++++++++---------- .../proxy/util/TranslatableMapper.java | 25 ++--------- 3 files changed, 29 insertions(+), 47 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java b/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java index e616fb4d..c3e8d472 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java @@ -81,6 +81,7 @@ import java.net.http.HttpClient; import java.nio.file.Files; import java.nio.file.Path; import java.security.KeyPair; +import java.text.MessageFormat; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -104,7 +105,7 @@ import net.kyori.adventure.audience.ForwardingAudience; import net.kyori.adventure.key.Key; import net.kyori.adventure.text.Component; import net.kyori.adventure.translation.GlobalTranslator; -import net.kyori.adventure.translation.TranslationRegistry; +import net.kyori.adventure.translation.TranslationStore; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.bstats.MetricsBase; @@ -337,8 +338,8 @@ public class VelocityServer implements ProxyServer, ForwardingAudience { } private void registerTranslations() { - final TranslationRegistry translationRegistry = TranslationRegistry - .create(Key.key("velocity", "translations")); + final TranslationStore.StringBased translationRegistry = + TranslationStore.messageFormat(Key.key("velocity", "translations")); translationRegistry.defaultLocale(Locale.US); try { ResourceUtils.visitResources(VelocityServer.class, path -> { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java index 22e1dc9c..e1f9df8e 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java @@ -123,6 +123,7 @@ import net.kyori.adventure.permission.PermissionChecker; import net.kyori.adventure.platform.facet.FacetPointers; import net.kyori.adventure.platform.facet.FacetPointers.Type; import net.kyori.adventure.pointer.Pointers; +import net.kyori.adventure.pointer.PointersSupplier; import net.kyori.adventure.resource.ResourcePackInfoLike; import net.kyori.adventure.resource.ResourcePackRequest; import net.kyori.adventure.resource.ResourcePackRequestLike; @@ -152,7 +153,16 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player, private static final ComponentLogger logger = ComponentLogger.logger(ConnectedPlayer.class); - private final Identity identity = new IdentityImpl(); + private static final @NotNull PointersSupplier POINTERS_SUPPLIER = + PointersSupplier.builder() + .resolving(Identity.UUID, Player::getUniqueId) + .resolving(Identity.NAME, Player::getUsername) + .resolving(Identity.DISPLAY_NAME, player -> Component.text(player.getUsername())) + .resolving(Identity.LOCALE, Player::getEffectiveLocale) + .resolving(PermissionChecker.POINTER, Player::getPermissionChecker) + .resolving(FacetPointers.TYPE, player -> Type.PLAYER) + .build(); + /** * The actual Minecraft connection. This is actually a wrapper object around the Netty channel. */ @@ -181,14 +191,6 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player, private final ResourcePackHandler resourcePackHandler; private final BundleDelimiterHandler bundleHandler = new BundleDelimiterHandler(this); - private final @NotNull Pointers pointers = - Player.super.pointers().toBuilder() - .withDynamic(Identity.UUID, this::getUniqueId) - .withDynamic(Identity.NAME, this::getUsername) - .withDynamic(Identity.DISPLAY_NAME, () -> Component.text(this.getUsername())) - .withDynamic(Identity.LOCALE, this::getEffectiveLocale) - .withStatic(PermissionChecker.POINTER, getPermissionChecker()) - .withStatic(FacetPointers.TYPE, Type.PLAYER).build(); private @Nullable String clientBrand; private @Nullable Locale effectiveLocale; private final @Nullable IdentifiedKey playerKey; @@ -257,7 +259,7 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player, @Override public @NonNull Identity identity() { - return this.identity; + return Identity.identity(this.getUniqueId()); } @Override @@ -363,7 +365,7 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player, @Override public @NotNull Pointers pointers() { - return this.pointers; + return POINTERS_SUPPLIER.view(this); } @Override @@ -396,14 +398,20 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player, } /** - * Translates the message in the user's locale. + * Translates the message in the user's locale, falling back to the default locale if not set. * * @param message the message to translate * @return the translated message */ public Component translateMessage(Component message) { - Locale locale = ClosestLocaleMatcher.INSTANCE - .lookupClosest(getEffectiveLocale() == null ? Locale.getDefault() : getEffectiveLocale()); + Locale locale = this.getEffectiveLocale(); + if (locale == null && settings != null) { + locale = settings.getLocale(); + } + if (locale == null) { + locale = Locale.getDefault(); + } + locale = ClosestLocaleMatcher.INSTANCE.lookupClosest(locale); return GlobalTranslator.render(message, locale); } @@ -1361,14 +1369,6 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player, return playerKey; } - private class IdentityImpl implements Identity { - - @Override - public @NonNull UUID uuid() { - return ConnectedPlayer.this.getUniqueId(); - } - } - @Override public ProtocolState getProtocolState() { return connection.getState().toProtocolState(); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/util/TranslatableMapper.java b/proxy/src/main/java/com/velocitypowered/proxy/util/TranslatableMapper.java index edc06761..6f30d3d1 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/util/TranslatableMapper.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/util/TranslatableMapper.java @@ -24,9 +24,6 @@ import net.kyori.adventure.text.Component; import net.kyori.adventure.text.TranslatableComponent; import net.kyori.adventure.text.flattener.ComponentFlattener; import net.kyori.adventure.translation.GlobalTranslator; -import net.kyori.adventure.translation.TranslationRegistry; -import net.kyori.adventure.translation.Translator; -import org.jetbrains.annotations.Nullable; /** * Velocity Translation Mapper. @@ -43,25 +40,9 @@ public enum TranslatableMapper implements BiConsumer componentConsumer ) { - for (final Translator source : GlobalTranslator.translator().sources()) { - if (source instanceof TranslationRegistry registry - && registry.contains(translatableComponent.key())) { - componentConsumer.accept(GlobalTranslator.render(translatableComponent, - ClosestLocaleMatcher.INSTANCE.lookupClosest(Locale.getDefault()))); - return; - } - } - final @Nullable String fallback = translatableComponent.fallback(); - if (fallback == null) { - return; - } - for (final Translator source : GlobalTranslator.translator().sources()) { - if (source instanceof TranslationRegistry registry && registry.contains(fallback)) { - componentConsumer.accept( - GlobalTranslator.render(Component.translatable(fallback), - ClosestLocaleMatcher.INSTANCE.lookupClosest(Locale.getDefault()))); - return; - } + final Locale locale = ClosestLocaleMatcher.INSTANCE.lookupClosest(Locale.getDefault()); + if (GlobalTranslator.translator().canTranslate(translatableComponent.key(), locale)) { + componentConsumer.accept(GlobalTranslator.render(translatableComponent, locale)); } } }