Skip to content

Add support for GlobalTranslator #40

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

Merged
Tofaa2 merged 2 commits into from
Jul 21, 2025
Merged

Add support for GlobalTranslator #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9ea8e9c
feat: Add user locale handling and metadata translation support
huanmeng-qwq Jul 21, 2025
acbb438
feat: Introduce UserLocaleProvider implementation for Spigot, Velocit…
huanmeng-qwq Jul 21, 2025
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
2 changes: 2 additions & 0 deletions api/src/main/java/me/tofaa/entitylib/EntityLibAPI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,6 @@ public interface EntityLibAPI<T> {
@NotNull
EntityContainer getDefaultContainer();

@NotNull
UserLocaleProvider getUserLocaleProvider();
}
13 changes: 13 additions & 0 deletions api/src/main/java/me/tofaa/entitylib/Platform.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ public interface Platform<P> {
*/
@NotNull EntityUuidProvider getEntityUuidProvider();

/**
* Gets the provider responsible for retrieving the locale of a user.
*
* @return a non-null {@link UserLocaleProvider} instance.
*/
@NotNull UserLocaleProvider getUserLocaleProvider();

/**
* Sets the entityId integer provider. This can be provided by a platform if needed.
* @param provider the entityId integer provider.
Expand All @@ -38,6 +45,12 @@ public interface Platform<P> {
*/
void setEntityUuidProvider(@NotNull EntityUuidProvider provider);

/**
* Sets the provider responsible for retrieving the locale of a user.
*
* @param provider the {@link UserLocaleProvider} instance to be set. Must not be null.
*/
void setUserLocaleProvider(@NotNull UserLocaleProvider provider);

/**
* @return the logger EntityLib uses internally.
Expand Down
8 changes: 8 additions & 0 deletions api/src/main/java/me/tofaa/entitylib/UserLocaleProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package me.tofaa.entitylib;

import java.util.Locale;
import java.util.UUID;

public interface UserLocaleProvider {
Locale locale(UUID user);
}
Comment on lines +6 to +8
Copy link

@coderabbitai coderabbitai bot Jul 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add null annotations and documentation for better API clarity.

The interface lacks proper documentation and null safety annotations. Consider the following improvements:

+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Provides user locale information based on user UUID.
+ * Implementations should handle cases where users are not found gracefully.
+ */
 public interface UserLocaleProvider {
-    Locale locale(UUID user);
+    /**
+     * Retrieves the locale for the specified user.
+     *
+     * @param user the UUID of the user, must not be null
+     * @return the user's locale, or a default locale if the user is not found
+     */
+    @NotNull Locale getUserLocale(@NotNull UUID user);
 }

The method name locale should also be more descriptive - getUserLocale clearly indicates what the method does.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public interface UserLocaleProvider {
Locale locale(UUID user);
}
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
/**
* Provides user locale information based on user UUID.
* Implementations should handle cases where users are not found gracefully.
*/
public interface UserLocaleProvider {
/**
* Retrieves the locale for the specified user.
*
* @param user the UUID of the user, must not be null
* @return the user's locale, or a default locale if the user is not found
*/
@NotNull
Locale getUserLocale(@NotNull UUID user);
}
🤖 Prompt for AI Agents 
In api/src/main/java/me/tofaa/entitylib/UserLocaleProvider.java around lines 6
to 8, add JavaDoc comments to the interface and its method to clarify their
purpose. Rename the method from locale to getUserLocale for better
descriptiveness. Add appropriate nullability annotations such as @Nullable or
@NonNull to the method parameter and return type to improve null safety and API
clarity.

34 changes: 34 additions & 0 deletions api/src/main/java/me/tofaa/entitylib/utils/PacketUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package me.tofaa.entitylib.utils;

import com.github.retrooper.packetevents.protocol.entity.data.EntityData;
import com.github.retrooper.packetevents.protocol.entity.data.EntityDataTypes;
import com.github.retrooper.packetevents.wrapper.play.server.WrapperPlayServerEntityMetadata;
import me.tofaa.entitylib.EntityLib;
import java.util.Locale;
import java.util.Optional;
import java.util.UUID;
import net.kyori.adventure.text.Component;
import net.kyori.adventure.translation.GlobalTranslator;

public class PacketUtil {
private PacketUtil() {
}

public static void renderPacket(UUID user, WrapperPlayServerEntityMetadata metadata) {
Locale locale = EntityLib.getApi().getUserLocaleProvider().locale(user);
for (final EntityData<?> entityData : metadata.getEntityMetadata()) {
if (entityData.getType() == EntityDataTypes.ADV_COMPONENT) {
Component component = (Component) entityData.getValue();
final Component rendered = GlobalTranslator.render(component, locale);
((EntityData<Component>) entityData).setValue(rendered);
} else if (entityData.getType() == EntityDataTypes.OPTIONAL_ADV_COMPONENT) {
final Optional<Component> optional = (Optional<Component>) entityData.getValue();
if (optional.isPresent()) {
final Component component = optional.get();
final Component rendered = GlobalTranslator.render(component, locale);
((EntityData<Optional<Component>>) entityData).setValue(Optional.of(rendered));
}
}
}
}
Comment on lines +17 to +33
Copy link

@coderabbitai coderabbitai bot Jul 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Core logic is sound but needs safety improvements.

The method correctly handles both ADV_COMPONENT and OPTIONAL_ADV_COMPONENT types for locale-aware rendering. However, there are safety concerns:

  1. Unsafe casting: The casts on lines 21, 23, 25, and 29 could throw ClassCastException if the entity data types don't match expectations.
  2. Missing null checks: No validation that locale, component, or optional are non-null.
  3. Error handling: No handling if getUserLocaleProvider().locale(user) fails.

Consider adding safety checks:

 public static void renderPacket(UUID user, WrapperPlayServerEntityMetadata metadata) {
+    UserLocaleProvider provider = EntityLib.getApi().getUserLocaleProvider();
+    if (provider == null) return;
+    
-    Locale locale = EntityLib.getApi().getUserLocaleProvider().locale(user);
+    Locale locale = provider.locale(user);
+    if (locale == null) return;
+    
     for (final EntityData<?> entityData : metadata.getEntityMetadata()) {
         if (entityData.getType() == EntityDataTypes.ADV_COMPONENT) {
-            Component component = (Component) entityData.getValue();
+            Object value = entityData.getValue();
+            if (!(value instanceof Component)) continue;
+            Component component = (Component) value;
+            if (component == null) continue;
             final Component rendered = GlobalTranslator.render(component, locale);
             ((EntityData<Component>) entityData).setValue(rendered);
         } else if (entityData.getType() == EntityDataTypes.OPTIONAL_ADV_COMPONENT) {
-            final Optional<Component> optional = (Optional<Component>) entityData.getValue();
+            Object value = entityData.getValue();
+            if (!(value instanceof Optional)) continue;
+            final Optional<Component> optional = (Optional<Component>) value;
             if (optional.isPresent()) {
                 final Component component = optional.get();
+                if (component == null) continue;
                 final Component rendered = GlobalTranslator.render(component, locale);
                 ((EntityData<Optional<Component>>) entityData).setValue(Optional.of(rendered));
             }
         }
     }
 }
🤖 Prompt for AI Agents 
In api/src/main/java/me/tofaa/entitylib/utils/PacketUtil.java between lines 17
and 33, the method renderPacket performs unsafe casts on entityData values and
lacks null checks and error handling. To fix this, add checks to ensure locale
is not null before proceeding, verify that the values retrieved from entityData
are non-null before casting and rendering, and use instanceof checks or
try-catch blocks to safely cast entityData values to avoid ClassCastException.
Also, handle potential exceptions or null returns from
getUserLocaleProvider().locale(user) gracefully to prevent runtime errors.

}
31 changes: 25 additions & 6 deletions api/src/main/java/me/tofaa/entitylib/wrapper/WrapperEntity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,39 @@
import com.github.retrooper.packetevents.protocol.world.Location;
import com.github.retrooper.packetevents.util.Vector3d;
import com.github.retrooper.packetevents.wrapper.PacketWrapper;
import com.github.retrooper.packetevents.wrapper.play.server.*;
import com.github.retrooper.packetevents.wrapper.play.server.WrapperPlayServerBundle;
import com.github.retrooper.packetevents.wrapper.play.server.WrapperPlayServerDestroyEntities;
import com.github.retrooper.packetevents.wrapper.play.server.WrapperPlayServerEntityHeadLook;
import com.github.retrooper.packetevents.wrapper.play.server.WrapperPlayServerEntityMetadata;
import com.github.retrooper.packetevents.wrapper.play.server.WrapperPlayServerEntityRotation;
import com.github.retrooper.packetevents.wrapper.play.server.WrapperPlayServerEntityTeleport;
import com.github.retrooper.packetevents.wrapper.play.server.WrapperPlayServerEntityVelocity;
import com.github.retrooper.packetevents.wrapper.play.server.WrapperPlayServerSetPassengers;
import com.github.retrooper.packetevents.wrapper.play.server.WrapperPlayServerSpawnEntity;
import com.github.retrooper.packetevents.wrapper.play.server.WrapperPlayServerSystemChatMessage;
import me.tofaa.entitylib.EntityLib;
import me.tofaa.entitylib.container.EntityContainer;
import me.tofaa.entitylib.meta.EntityMeta;
import me.tofaa.entitylib.meta.types.ObjectData;
import me.tofaa.entitylib.tick.Tickable;
import me.tofaa.entitylib.utils.PacketUtil;
import me.tofaa.entitylib.ve.ViewerRule;
import me.tofaa.entitylib.wrapper.spawning.SpawnPacketProvider;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.function.Consumer;
import net.kyori.adventure.text.Component;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.jetbrains.annotations.UnmodifiableView;

import java.util.*;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.function.Consumer;

public class WrapperEntity implements Tickable {
private final UUID uuid;
private final int entityId;
Expand Down Expand Up @@ -510,6 +524,11 @@ private static void sendPacket(UUID user, PacketWrapper<?> wrapper) {
return;
}

// Special handling for entity metadata packets to support `GlobalTranslator` functionality and component rendering
if (wrapper instanceof WrapperPlayServerEntityMetadata) {
PacketUtil.renderPacket(user, (WrapperPlayServerEntityMetadata) wrapper);
}
Comment on lines +527 to +530
Copy link

@coderabbitai coderabbitai bot Jul 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Good integration of locale-aware component rendering.

The special handling for entity metadata packets is well-implemented and properly placed before packet transmission. The instanceof check is efficient and the cast is safe.

Consider adding error handling around PacketUtil.renderPacket() to prevent packet sending failures if locale processing encounters issues.

🏁 Script executed:

#!/bin/bash
# Verify PacketUtil.renderPacket error handling
rg -A 10 -B 5 "renderPacket.*throws|catch.*renderPacket" --type java

Length of output: 70

🏁 Script executed:

#!/bin/bash
# Locate PacketUtil class and inspect renderPacket method declaration for exception behavior
rg -n "class PacketUtil" --type java
rg -A5 -B2 "renderPacket(" --type java

Length of output: 244

🏁 Script executed:

#!/bin/bash
# Find all occurrences of `renderPacket` to locate its definition
rg -n "renderPacket" --type java

# Once the file is identified, show the method declaration with surrounding context
rg -C5 "renderPacket" api/src/main/java/me/tofaa/entitylib/utils/PacketUtil.java

Length of output: 964

Add runtime error handling around PacketUtil.renderPacket

The renderPacket method doesn’t declare checked exceptions, so any failures (for example, NPEs during locale rendering) would be unchecked and could propagate, potentially interrupting packet transmission. Wrap the call in a try/catch for RuntimeException and log the error to ensure the packet still sends.

• File: api/src/main/java/me/tofaa/entitylib/wrapper/WrapperEntity.java
Around lines 527–530

Suggested diff:

-        if (wrapper instanceof WrapperPlayServerEntityMetadata) {
-            PacketUtil.renderPacket(user, (WrapperPlayServerEntityMetadata) wrapper);
-        }
+        if (wrapper instanceof WrapperPlayServerEntityMetadata) {
+            try {
+                PacketUtil.renderPacket(user, (WrapperPlayServerEntityMetadata) wrapper);
+            } catch (RuntimeException e) {
+                EntityLib.getLogger()
+                         .error("Failed to render entity metadata packet for user {}", user, e);
+            }
+        }
🤖 Prompt for AI Agents 
In api/src/main/java/me/tofaa/entitylib/wrapper/WrapperEntity.java around lines
527 to 530, the call to PacketUtil.renderPacket can throw unchecked runtime
exceptions that may disrupt packet transmission. Wrap this call in a try/catch
block catching RuntimeException, and log the exception to prevent interruption
while still sending the packet.


EntityLib.getApi().getPacketEvents().getProtocolManager().sendPacket(channel, wrapper);
}

Expand Down
6 changes: 6 additions & 0 deletions common/src/main/java/me/tofaa/entitylib/common/AbstractEntityLibAPI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import me.tofaa.entitylib.APIConfig;
import me.tofaa.entitylib.EntityLibAPI;
import me.tofaa.entitylib.Platform;
import me.tofaa.entitylib.UserLocaleProvider;
import me.tofaa.entitylib.container.EntityContainer;
import me.tofaa.entitylib.tick.TickContainer;
import me.tofaa.entitylib.wrapper.WrapperEntity;
Expand Down Expand Up @@ -63,4 +64,9 @@ public PacketEventsAPI<?> getPacketEvents() {
public @NotNull Collection<TickContainer<?, T>> getTickContainers() {
return tickContainers;
}

@Override
public @NotNull UserLocaleProvider getUserLocaleProvider() {
return platform.getUserLocaleProvider();
}
}
24 changes: 14 additions & 10 deletions platforms/spigot/src/main/java/me/tofaa/entitylib/spigot/SpigotEntityLibPlatform.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,17 @@
import io.github.retrooper.packetevents.bstats.charts.SimplePie;
import me.tofaa.entitylib.APIConfig;
import me.tofaa.entitylib.EntityLib;
import me.tofaa.entitylib.UserLocaleProvider;
import me.tofaa.entitylib.common.AbstractPlatform;
import me.tofaa.entitylib.utils.ConcurrentWeakIdentityHashMap;
import org.bukkit.Bukkit;
import org.bukkit.World;
import org.bukkit.entity.Entity;
import java.util.logging.Logger;
import org.bukkit.plugin.Plugin;
import org.bukkit.plugin.java.JavaPlugin;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import java.util.Map;
import java.util.logging.Logger;
import java.util.stream.Stream;

public class SpigotEntityLibPlatform extends AbstractPlatform<JavaPlugin> {

private SpigotEntityLibAPI api;
private UserLocaleProvider userLocaleProvider = new SpigotPlayerLocaleProvider();

public SpigotEntityLibPlatform(@NotNull JavaPlugin plugin) {
super(plugin);
Expand All @@ -36,7 +30,7 @@ public void setupApi(@NotNull APIConfig settings) {
this.api.onLoad();
this.api.onEnable();
if (settings.shouldUseBstats()) {
PacketEventsAPI<Plugin> pe = (PacketEventsAPI<Plugin>)api.getPacketEvents();
PacketEventsAPI<Plugin> pe = (PacketEventsAPI<Plugin>) api.getPacketEvents();
Metrics metrics = new Metrics(pe.getPlugin(), 21916);
metrics.addCustomChart(new SimplePie("entitylib-version", () -> EntityLib.getVersion().toString()));
}
Expand All @@ -52,4 +46,14 @@ public SpigotEntityLibAPI getAPI() {
public String getName() {
return "Spigot";
}

@Override
public @NotNull UserLocaleProvider getUserLocaleProvider() {
return userLocaleProvider;
}

@Override
public void setUserLocaleProvider(@NotNull final UserLocaleProvider provider) {
this.userLocaleProvider = provider;
}
}
73 changes: 73 additions & 0 deletions platforms/spigot/src/main/java/me/tofaa/entitylib/spigot/SpigotPlayerLocaleProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package me.tofaa.entitylib.spigot;

import me.tofaa.entitylib.UserLocaleProvider;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.util.Locale;
import java.util.UUID;
import java.util.function.Function;
import net.kyori.adventure.translation.Translator;
import org.bukkit.Bukkit;
import org.bukkit.entity.Player;
import org.jetbrains.annotations.NotNull;


/**
* This implementation is based on code from the scoreboard-library project:
* <a href="https://github.com/MegavexNetwork/scoreboard-library/blob/bc8e3c2d2ecf9973ec0d6bb8ae4af94ed008b360/commons/src/main/java/net/megavex/scoreboardlibrary/implementation/commons/LocaleProvider.java">LocaleProvider</a>
* Modified and adapted for use in EntityLib.
*/
public class SpigotPlayerLocaleProvider implements UserLocaleProvider {
private static final Locale DEFAULT_LOCALE = Locale.US;
private static final Function<Player, Locale> provider = get();

@Override
public Locale locale(final UUID user) {
final Player player = Bukkit.getPlayer(user);
return player == null ? DEFAULT_LOCALE : provider.apply(player);
}
Comment on lines +26 to +29
Copy link

@coderabbitai coderabbitai bot Jul 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Consider handling potential race conditions

There's a potential race condition where a player could disconnect between the null check and provider.apply(player). Consider wrapping the entire operation in a try-catch block to handle any exceptions gracefully.

 @Override
 public Locale locale(final UUID user) {
     final Player player = Bukkit.getPlayer(user);
-    return player == null ? DEFAULT_LOCALE : provider.apply(player);
+    if (player == null) {
+        return DEFAULT_LOCALE;
+    }
+    try {
+        return provider.apply(player);
+    } catch (RuntimeException e) {
+        // Player might have disconnected or other issues
+        return DEFAULT_LOCALE;
+    }
 }

🏁 Script executed:

#!/bin/bash
# Locate and display the SpigotPlayerLocaleProvider implementation for review
fd SpigotPlayerLocaleProvider.java -type f -x sed -n '1,200p' {}

Length of output: 237

🏁 Script executed:

#!/bin/bash
# Corrected script to locate and display the SpigotPlayerLocaleProvider implementation
file=$(find . -type f -name SpigotPlayerLocaleProvider.java)
if [ -z "$file" ]; then
  echo "SpigotPlayerLocaleProvider.java not found"
  exit 1
fi
echo "Found file at: $file"
sed -n '1,200p' "$file"

Length of output: 3649

Handle reflection exceptions in locale to prevent runtime errors

Wrap the call to provider.apply(player) in a try/catch so that any RuntimeException thrown by the reflection-based lookup falls back to DEFAULT_LOCALE rather than bubbling up.

Locations to update:

  • platforms/spigot/src/main/java/me/tofaa/entitylib/spigot/SpigotPlayerLocaleProvider.java – the locale(UUID) method

Suggested diff:

 @Override
 public Locale locale(final UUID user) {
     final Player player = Bukkit.getPlayer(user);
-    return player == null ? DEFAULT_LOCALE : provider.apply(player);
+    if (player == null) {
+        return DEFAULT_LOCALE;
+    }
+    try {
+        return provider.apply(player);
+    } catch (RuntimeException e) {
+        // Fallback to default locale if reflective lookup fails
+        return DEFAULT_LOCALE;
+    }
 }
🤖 Prompt for AI Agents 
In
platforms/spigot/src/main/java/me/tofaa/entitylib/spigot/SpigotPlayerLocaleProvider.java
around lines 26 to 29, the locale(UUID) method calls provider.apply(player)
which may throw a RuntimeException due to reflection issues. Wrap the call to
provider.apply(player) in a try/catch block catching RuntimeException, and
return DEFAULT_LOCALE in the catch block to prevent runtime errors from
propagating.


private static @NotNull Function<Player, Locale> get() {
MethodHandles.Lookup lookup = MethodHandles.publicLookup();
try {
MethodHandle adventureMethod = lookup.findVirtual(Player.class, "locale", MethodType.methodType(Locale.class));
return player -> {
try {
return (Locale) adventureMethod.invokeExact(player);
} catch (Throwable e) {
throw new RuntimeException(e);
}
};
Comment on lines +35 to +41
Copy link

@coderabbitai coderabbitai bot Jul 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Preserve stack traces when rethrowing exceptions

When catching and rethrowing exceptions, preserve the original stack trace for better debugging.

 return player -> {
     try {
         return (Locale) adventureMethod.invokeExact(player);
     } catch (Throwable e) {
-        throw new RuntimeException(e);
+        throw new RuntimeException("Failed to get locale via Adventure API", e);
     }
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return player -> {
try {
return (Locale) adventureMethod.invokeExact(player);
} catch (Throwable e) {
throw new RuntimeException(e);
}
};
return player -> {
try {
return (Locale) adventureMethod.invokeExact(player);
} catch (Throwable e) {
throw new RuntimeException("Failed to get locale via Adventure API", e);
}
};
🤖 Prompt for AI Agents 
In
platforms/spigot/src/main/java/me/tofaa/entitylib/spigot/SpigotPlayerLocaleProvider.java
around lines 35 to 41, the caught Throwable is rethrown by wrapping it in a new
RuntimeException without preserving the original stack trace. To fix this,
ensure the original exception is passed as the cause to the RuntimeException
constructor so the stack trace is preserved for better debugging.

} catch (IllegalAccessException | NoSuchMethodException ignored) {
}

MethodType methodType = MethodType.methodType(String.class);
try {
MethodHandle legacySpigotMethod = lookup.findVirtual(Player.Spigot.class, "getLocale", methodType);
return player -> {
try {
Locale locale = Translator.parseLocale((String) legacySpigotMethod.invokeExact(player.spigot()));
return locale == null ? DEFAULT_LOCALE : locale;
} catch (Throwable e) {
throw new RuntimeException(e);
}
};
Comment on lines +48 to +55
Copy link

@coderabbitai coderabbitai bot Jul 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Apply consistent exception handling improvements

Similar to the previous comment, improve exception messages here too.

 return player -> {
     try {
         Locale locale = Translator.parseLocale((String) legacySpigotMethod.invokeExact(player.spigot()));
         return locale == null ? DEFAULT_LOCALE : locale;
     } catch (Throwable e) {
-        throw new RuntimeException(e);
+        throw new RuntimeException("Failed to get locale via legacy Spigot API", e);
     }
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return player -> {
try {
Locale locale = Translator.parseLocale((String) legacySpigotMethod.invokeExact(player.spigot()));
return locale == null ? DEFAULT_LOCALE : locale;
} catch (Throwable e) {
throw new RuntimeException(e);
}
};
return player -> {
try {
Locale locale = Translator.parseLocale((String) legacySpigotMethod.invokeExact(player.spigot()));
return locale == null ? DEFAULT_LOCALE : locale;
} catch (Throwable e) {
throw new RuntimeException("Failed to get locale via legacy Spigot API", e);
}
};
🤖 Prompt for AI Agents 
In
platforms/spigot/src/main/java/me/tofaa/entitylib/spigot/SpigotPlayerLocaleProvider.java
around lines 48 to 55, the exception handling wraps any Throwable in a
RuntimeException without a descriptive message. Update the catch block to
include a clear, informative message when throwing the RuntimeException,
incorporating the original exception to improve debugging and maintain
consistency with previous exception handling improvements.

} catch (IllegalAccessException | NoSuchMethodException ignored) {
}

try {
MethodHandle legacyMethod = lookup.findVirtual(Player.class, "getLocale", methodType);
return player -> {
try {
Locale locale = Translator.parseLocale((String) legacyMethod.invokeExact(player));
return locale == null ? DEFAULT_LOCALE : locale;
} catch (Throwable e) {
throw new RuntimeException(e);
}
};
Comment on lines +61 to +68
Copy link

@coderabbitai coderabbitai bot Jul 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Apply consistent exception handling improvements

Apply the same exception handling improvement here.

 return player -> {
     try {
         Locale locale = Translator.parseLocale((String) legacyMethod.invokeExact(player));
         return locale == null ? DEFAULT_LOCALE : locale;
     } catch (Throwable e) {
-        throw new RuntimeException(e);
+        throw new RuntimeException("Failed to get locale via legacy API", e);
     }
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return player -> {
try {
Locale locale = Translator.parseLocale((String) legacyMethod.invokeExact(player));
return locale == null ? DEFAULT_LOCALE : locale;
} catch (Throwable e) {
throw new RuntimeException(e);
}
};
return player -> {
try {
Locale locale = Translator.parseLocale((String) legacyMethod.invokeExact(player));
return locale == null ? DEFAULT_LOCALE : locale;
} catch (Throwable e) {
throw new RuntimeException("Failed to get locale via legacy API", e);
}
};
🤖 Prompt for AI Agents 
In
platforms/spigot/src/main/java/me/tofaa/entitylib/spigot/SpigotPlayerLocaleProvider.java
around lines 61 to 68, the exception handling wraps all exceptions in a
RuntimeException without preserving the original exception type or message
clearly. Refactor the catch block to handle exceptions more consistently by
logging the error or rethrowing a more specific exception with the original
exception as the cause, ensuring better clarity and maintainability in error
handling.

} catch (IllegalAccessException | NoSuchMethodException ignored) {
throw new RuntimeException("No way to get players locale found");
}
}
}
13 changes: 13 additions & 0 deletions ...s/standalone/src/main/java/me/tofaa/entitylib/standalone/StandaloneEntityLibPlatform.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@

import me.tofaa.entitylib.APIConfig;
import me.tofaa.entitylib.EntityLibAPI;
import me.tofaa.entitylib.UserLocaleProvider;
import me.tofaa.entitylib.common.AbstractPlatform;
import java.util.Locale;
import org.jetbrains.annotations.NotNull;

public class StandaloneEntityLibPlatform extends AbstractPlatform<Object> {


private StandaloneEntityLibApi api;
private UserLocaleProvider userLocaleProvider = (user) -> Locale.US;

public StandaloneEntityLibPlatform() {
super(null);
Expand All @@ -34,4 +37,14 @@ public EntityLibAPI<?> getAPI() {
public String getName() {
return "Standalone";
}

@Override
public @NotNull UserLocaleProvider getUserLocaleProvider() {
return userLocaleProvider;
}

@Override
public void setUserLocaleProvider(@NotNull final UserLocaleProvider provider) {
this.userLocaleProvider = provider;
}
}
12 changes: 12 additions & 0 deletions platforms/velocity/src/main/java/me/tofaa/entitylib/velocity/VelocityEntityLibPlatform.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@
import io.github.retrooper.packetevents.velocity.factory.VelocityPacketEventsBuilder;
import me.tofaa.entitylib.APIConfig;
import me.tofaa.entitylib.EntityLibAPI;
import me.tofaa.entitylib.UserLocaleProvider;
import me.tofaa.entitylib.common.AbstractPlatform;
import org.jetbrains.annotations.NotNull;

import java.util.logging.Logger;

public class VelocityEntityLibPlatform extends AbstractPlatform<ProxyServer> {
private VelocityEntityLibAPI api;
private UserLocaleProvider userLocaleProvider;
Copy link

@coderabbitai coderabbitai bot Jul 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Missing field initialization could cause NPE.

The userLocaleProvider field is not initialized, which could lead to NullPointerException if getUserLocaleProvider() is called before setUserLocaleProvider(). Consider providing a default implementation like other platforms.

-private UserLocaleProvider userLocaleProvider;
+private UserLocaleProvider userLocaleProvider = new VelocityPlayerLocaleProvider(handle);

Or provide a safe fallback:

-private UserLocaleProvider userLocaleProvider;
+private UserLocaleProvider userLocaleProvider = (user) -> Locale.US;

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In
platforms/velocity/src/main/java/me/tofaa/entitylib/velocity/VelocityEntityLibPlatform.java
at line 20, the userLocaleProvider field is declared but not initialized,
risking a NullPointerException if accessed before being set. To fix this,
initialize userLocaleProvider with a default implementation similar to other
platforms or provide a safe fallback instance during declaration or in the
constructor to ensure it is never null when accessed.

private Object plugin;

public VelocityEntityLibPlatform(Object plugin, ProxyServer handle) {
Expand Down Expand Up @@ -54,4 +56,14 @@ public EntityLibAPI<?> getAPI() {
public String getName() {
return "Velocity";
}

@Override
public @NotNull UserLocaleProvider getUserLocaleProvider() {
return userLocaleProvider;
}

@Override
public void setUserLocaleProvider(final UserLocaleProvider userLocaleProvider) {
this.userLocaleProvider = userLocaleProvider;
}
Comment on lines +60 to +68
Copy link

@coderabbitai coderabbitai bot Jul 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

@NotNull annotation inconsistent with nullable field.

The getUserLocaleProvider() method is annotated with @NotNull but the field can be null since it's not initialized. This violates the annotation contract and could mislead callers.

Either initialize the field with a default value or remove the @NotNull annotation and add null checking where the provider is used.

🤖 Prompt for AI Agents 
In
platforms/velocity/src/main/java/me/tofaa/entitylib/velocity/VelocityEntityLibPlatform.java
around lines 60 to 68, the getUserLocaleProvider() method is annotated with
@NotNull but the userLocaleProvider field is not initialized and can be null,
violating the annotation contract. To fix this, either initialize
userLocaleProvider with a default non-null value or remove the @NotNull
annotation from the getter and add null checks wherever userLocaleProvider is
used to handle potential null values safely.

}
20 changes: 20 additions & 0 deletions ...orms/velocity/src/main/java/me/tofaa/entitylib/velocity/VelocityPlayerLocaleProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package me.tofaa.entitylib.velocity;

import com.velocitypowered.api.proxy.Player;
import com.velocitypowered.api.proxy.ProxyServer;
import me.tofaa.entitylib.UserLocaleProvider;
import java.util.Locale;
import java.util.UUID;

public class VelocityPlayerLocaleProvider implements UserLocaleProvider {
private final ProxyServer proxyServer;

public VelocityPlayerLocaleProvider(final ProxyServer proxyServer) {
this.proxyServer = proxyServer;
}
Comment on lines +9 to +14
Copy link

@coderabbitai coderabbitai bot Jul 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add null validation and documentation.

The constructor lacks null validation and the class needs documentation to explain its purpose.

+import java.util.Objects;
+
+/**
+ * Velocity platform implementation of {@link UserLocaleProvider}.
+ * Retrieves player locales from the Velocity proxy server.
+ */
 public class VelocityPlayerLocaleProvider implements UserLocaleProvider {
     private final ProxyServer proxyServer;

     public VelocityPlayerLocaleProvider(final ProxyServer proxyServer) {
-        this.proxyServer = proxyServer;
+        this.proxyServer = Objects.requireNonNull(proxyServer, "proxyServer cannot be null");
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public class VelocityPlayerLocaleProvider implements UserLocaleProvider {
private final ProxyServer proxyServer;
public VelocityPlayerLocaleProvider(final ProxyServer proxyServer) {
this.proxyServer = proxyServer;
}
import java.util.Objects;
/**
* Velocity platform implementation of {@link UserLocaleProvider}.
* Retrieves player locales from the Velocity proxy server.
*/
public class VelocityPlayerLocaleProvider implements UserLocaleProvider {
private final ProxyServer proxyServer;
public VelocityPlayerLocaleProvider(final ProxyServer proxyServer) {
this.proxyServer = Objects.requireNonNull(proxyServer, "proxyServer cannot be null");
}
// existing methods...
}
🤖 Prompt for AI Agents 
In
platforms/velocity/src/main/java/me/tofaa/entitylib/velocity/VelocityPlayerLocaleProvider.java
around lines 9 to 14, add JavaDoc comments above the class to describe its
purpose clearly. Also, add a null check in the constructor for the proxyServer
parameter, throwing an appropriate exception if it is null to prevent potential
NullPointerExceptions later.


@Override
public Locale locale(final UUID user) {
return proxyServer.getPlayer(user).map(Player::getEffectiveLocale).orElse(Locale.US);
}
Comment on lines +16 to +19
Copy link

@coderabbitai coderabbitai bot Jul 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider method naming consistency and add null annotations.

The method should align with interface improvements and include proper null safety.

+    import org.jetbrains.annotations.NotNull;
+
     @Override
-    public Locale locale(final UUID user) {
+    public @NotNull Locale getUserLocale(@NotNull final UUID user) {
+        Objects.requireNonNull(user, "user cannot be null");
         return proxyServer.getPlayer(user).map(Player::getEffectiveLocale).orElse(Locale.US);
     }

Note: This assumes the interface method name is updated as suggested in the earlier review.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In
platforms/velocity/src/main/java/me/tofaa/entitylib/velocity/VelocityPlayerLocaleProvider.java
around lines 16 to 19, rename the method from locale to getLocale to match the
updated interface naming convention. Additionally, add appropriate nullability
annotations such as @Nullable or @NonNull to the method signature and parameters
to ensure null safety and clarity about the method's contract.

}
Loading