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.

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
+    final TranslationStore.StringBased<MessageFormat> translationRegistry =
-        .create(Key.key("velocity", "translations"));
+            TranslationStore.messageFormat(Key.key("velocity", "translations"));
     translationRegistry.defaultLocale(Locale.US);
     try {
       ResourceUtils.visitResources(VelocityServer.class, path -> {

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<ConnectedPlayer> POINTERS_SUPPLIER =
+          PointersSupplier.<ConnectedPlayer>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
+    Locale locale = this.getEffectiveLocale();
-        .lookupClosest(getEffectiveLocale() == null ? Locale.getDefault() : 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();

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<TranslatableComponent, Cons
           final TranslatableComponent translatableComponent,
           final Consumer<Component> componentConsumer
   ) {
-    for (final Translator source : GlobalTranslator.translator().sources()) {
+    final Locale locale = ClosestLocaleMatcher.INSTANCE.lookupClosest(Locale.getDefault());
-      if (source instanceof TranslationRegistry registry
+    if (GlobalTranslator.translator().canTranslate(translatableComponent.key(), locale)) {
-              && registry.contains(translatableComponent.key())) {
+      componentConsumer.accept(GlobalTranslator.render(translatableComponent, locale));
-        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;
-      }
     }
   }
 }