velocity-feat-mod #24
Conversation
WalkthroughThis pull request updates Maven configuration and project module declarations across multiple projects. In the root Changes
Sequence Diagram(s)sequenceDiagram
participant Proxy
participant ZVMain as ZanderVelocityMain
participant CmdMgr as CommandManager
participant Sched as Scheduler
participant Logger
Proxy->>ZVMain: Emit ProxyInitializeEvent
ZVMain->>CmdMgr: Register commands (discord, ping, etc.)
ZVMain->>Sched: Start Heartbeat & Tip tasks
ZVMain->>Logger: Log initialization details
sequenceDiagram
participant Player
participant DiscordCmd as discord Command
participant API
Player->>DiscordCmd: Execute "discord" command
DiscordCmd->>API: Perform HTTP GET for configuration data
API-->>DiscordCmd: Return JSON response
DiscordCmd->>Player: Send clickable Discord invitation message
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
🔭 Outside diff range comments (9)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/util/announcement/TipChatter.java (1)
18-55: 🛠️ Refactor suggestionHandle resources properly and improve error handling in the scheduled task.
While the implementation of the announcement tip task works functionally, there are several areas that could be improved:
- The ScheduledExecutorService is created but never shut down, which could lead to resource leaks.
- Error handling is basic - it prints a stack trace but doesn't implement any retry logic.
- There's no validation of the JSON response before attempting to extract values.
Consider adding a method to shutdown the scheduler and implementing more robust error handling:
+ private static ScheduledExecutorService scheduler; + public static void startAnnouncementTipTask() { // Create a ScheduledExecutorService with a single thread - ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1); + scheduler = Executors.newScheduledThreadPool(1); // Schedule the task to run every 10 seconds scheduler.scheduleAtFixedRate(() -> { try { // GET request to fetch Announcement Tip. Request req = Request.builder() .setURL(ConfigurationManager.getConfig().get("BaseAPIURL") + "/announcement/get?announcementType=tip") .setMethod(Request.Method.GET) .addHeader("x-access-token", String.valueOf(ConfigurationManager.getConfig().get("APIKey"))) .build(); Response res = req.execute(); String json = res.getBody(); + + // Validate response status code + if (res.getStatusCode() != 200) { + System.out.println("Failed to fetch announcements: " + res.getStatusCode() + " - " + json); + return; + } String colourMessageFormat = JsonPath.read(json, "$.data[0].colourMessageFormat"); String link = JsonPath.read(json, "$.data[0].link"); // Broadcast the message to all online players for (ProxiedPlayer player : ProxyServer.getInstance().getPlayers()) { // Send the message to each player TextComponent message = new TextComponent(ChatColor.translateAlternateColorCodes('&', String.valueOf(ConfigurationManager.getConfig().get("announcementTipPrefix"))) + ChatColor.translateAlternateColorCodes('&', colourMessageFormat)); if (link != null && !link.isEmpty()) { // Set the click event only if link is true message.setClickEvent(new ClickEvent(ClickEvent.Action.OPEN_URL, link)); } player.sendMessage(message); } } catch (Exception e) { // Handle exceptions here e.printStackTrace(); System.out.println("Announcement Tip Failed, will try again in " + ConfigurationManager.getConfig().getInt("announcementTipInterval") + " minutes."); } }, 0, ConfigurationManager.getConfig().getInt("announcementTipInterval"), TimeUnit.MINUTES); } + + public static void stopAnnouncementTipTask() { + if (scheduler != null && !scheduler.isShutdown()) { + scheduler.shutdown(); + try { + if (!scheduler.awaitTermination(5, TimeUnit.SECONDS)) { + scheduler.shutdownNow(); + } + } catch (InterruptedException e) { + scheduler.shutdownNow(); + Thread.currentThread().interrupt(); + } + } + }Don't forget to call
stopAnnouncementTipTask()in the plugin'sonDisable()method.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserCommandSpyEvent.java (1)
17-53: 🛠️ Refactor suggestionImprove error handling for command spy functionality.
The current implementation disconnects players when an API-related exception occurs, which seems excessive for what appears to be a non-critical feature.
Consider a more graceful error handling approach:
try { DiscordCommandSpy commandSpy = DiscordCommandSpy.builder() .username(player.getDisplayName()) .command(command) .server(player.getServer().getInfo().getName()) .build(); Request destroySessionReq = Request.builder() .setURL(ConfigurationManager.getConfig().get("BaseAPIURL") + "/discord/spy/command") .setMethod(Request.Method.POST) .addHeader("x-access-token", String.valueOf(ConfigurationManager.getConfig().get("APIKey"))) .setRequestBody(commandSpy.toString()) .build(); Response commandSpyRes = destroySessionReq.execute(); - plugin.getProxy().getConsole().sendMessage(new TextComponent("Response (" + commandSpyRes.getStatusCode() + "): " + commandSpyRes.getBody())); + // Log only if there's an issue with the response + if (commandSpyRes.getStatusCode() != 200) { + plugin.getLogger().warning("Command spy API error - Response (" + commandSpyRes.getStatusCode() + "): " + commandSpyRes.getBody()); + } } catch (Exception e) { - player.disconnect(new TextComponent("An error has occurred. Is the API down?")); - System.out.println(e); - return; + plugin.getLogger().warning("Failed to send command to spy API: " + e.getMessage()); + e.printStackTrace(); }This approach:
- Only logs API responses when there's an error (status code != 200)
- Uses the plugin logger instead of raw console messages
- Logs the error without disconnecting the player
- Uses proper logger methods (warning) instead of System.out
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserSocialSpyEvent.java (1)
24-27: 🛠️ Refactor suggestion
⚠️ Potential issuePotential Risk: Array Index Out-of-Bounds in Command Parsing
The code extractsmessageParts[1]without verifying that the split result contains enough elements. In cases where a user might input an incomplete command (e.g., without specifying a target player), this could throw an exception. Adding a length check before accessingmessageParts[1]is highly recommended.zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/rules.java (1)
1-44: 🛠️ Refactor suggestionClass name needs PascalCase and code requires refactoring
The class name
rulesdoesn't follow Java naming conventions. Class names should use PascalCase (start with an uppercase letter).Additionally, there are several code improvements needed:
- Replace direct exception printing with proper logging
- Extract common API request logic to reduce duplication across command classes
- Move hardcoded endpoints to constants or configuration
package org.modularsoft.zander.velocity.commands; import com.jayway.jsonpath.JsonPath; import com.velocitypowered.api.command.CommandSource; import com.velocitypowered.api.command.SimpleCommand; import dev.dejvokep.boostedyaml.route.Route; import io.github.ModularEnigma.Request; import io.github.ModularEnigma.Response; import net.kyori.adventure.text.Component; import net.kyori.adventure.text.event.ClickEvent; import net.kyori.adventure.text.format.NamedTextColor; import org.modularsoft.zander.velocity.ZanderVelocityMain; +import java.util.logging.Level; +import java.util.logging.Logger; -public class rules implements SimpleCommand { +public class Rules implements SimpleCommand { + private static final Logger LOGGER = Logger.getLogger(Rules.class.getName()); + private static final String WEB_CONFIG_ENDPOINT = "/web/configuration"; + @Override public void execute(final Invocation invocation) { CommandSource source = invocation.source(); String BaseAPIURL = ZanderVelocityMain.getConfig().getString(Route.from("BaseAPIURL")); String APIKey = ZanderVelocityMain.getConfig().getString(Route.from("APIKey")); try { - // GET request to link to discord. + // GET request to fetch web configuration Request req = Request.builder() - .setURL(BaseAPIURL + "/web/configuration") + .setURL(BaseAPIURL + WEB_CONFIG_ENDPOINT) .setMethod(Request.Method.GET) .addHeader("x-access-token", APIKey) .build(); Response res = req.execute(); String json = res.getBody(); String siteAddress = JsonPath.read(json, "$.data.siteAddress"); Component message = Component.text("Please read and abide by the rules which you can find on our website here: " + siteAddress + "/rules") .color(NamedTextColor.RED); message = message.clickEvent(ClickEvent.clickEvent(ClickEvent.Action.OPEN_URL,siteAddress + "/rules")); source.sendMessage(message); } catch (Exception e) { Component builder = Component.text("An error has occurred. Is the API down?").color(NamedTextColor.RED); source.sendMessage(builder); - System.out.println(e); + LOGGER.log(Level.SEVERE, "Error fetching rules information", e); } } }zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/discord.java (1)
1-45: 🛠️ Refactor suggestionClass name needs PascalCase and code requires refactoring
Similar to the rules command, the class name
discorddoesn't follow Java naming conventions. Class names should use PascalCase.Additional issues:
- The comment on line 22 is incorrect - this is getting web configuration, not linking directly to Discord
- The
returnstatement on line 37 is unnecessary as it's at the end of the try block- Raw exception printing should be replaced with proper logging
- There's significant code duplication across command classes
package org.modularsoft.zander.velocity.commands; import com.jayway.jsonpath.JsonPath; import com.velocitypowered.api.command.CommandSource; import com.velocitypowered.api.command.SimpleCommand; import dev.dejvokep.boostedyaml.route.Route; import io.github.ModularEnigma.Request; import io.github.ModularEnigma.Response; import net.kyori.adventure.text.Component; import net.kyori.adventure.text.event.ClickEvent; import net.kyori.adventure.text.format.NamedTextColor; import org.modularsoft.zander.velocity.ZanderVelocityMain; +import java.util.logging.Level; +import java.util.logging.Logger; -public class discord implements SimpleCommand { +public class Discord implements SimpleCommand { + private static final Logger LOGGER = Logger.getLogger(Discord.class.getName()); + private static final String WEB_CONFIG_ENDPOINT = "/web/configuration"; + @Override public void execute(final Invocation invocation) { CommandSource source = invocation.source(); String BaseAPIURL = ZanderVelocityMain.getConfig().getString(Route.from("BaseAPIURL")); String APIKey = ZanderVelocityMain.getConfig().getString(Route.from("APIKey")); try { - // GET request to link to discord. + // GET request to fetch web configuration Request req = Request.builder() - .setURL(BaseAPIURL + "/web/configuration") + .setURL(BaseAPIURL + WEB_CONFIG_ENDPOINT) .setMethod(Request.Method.GET) .addHeader("x-access-token", APIKey) .build(); Response res = req.execute(); String json = res.getBody(); String siteAddress = JsonPath.read(json, "$.data.siteAddress"); Component message = Component.text("Get to know the community and join our Discord here: " + siteAddress + "/discord") .color(NamedTextColor.BLUE); message = message.clickEvent(ClickEvent.clickEvent(ClickEvent.Action.OPEN_URL,siteAddress + "/discord")); source.sendMessage(message); - return; } catch (Exception e) { Component builder = Component.text("An error has occurred. Is the API down?").color(NamedTextColor.RED); source.sendMessage(builder); - System.out.println(e); + LOGGER.log(Level.SEVERE, "Error fetching Discord information", e); } } }zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/website.java (1)
1-44: 🛠️ Refactor suggestionClass name needs PascalCase and significant code duplication
The class name
websitedoesn't follow Java naming conventions for class names. Additionally, there is a high degree of code duplication between this class and the other command classes (Rules and Discord).Issues to address:
- Rename class to follow PascalCase convention
- The comment on line 22 incorrectly refers to Discord
- Extract common API request logic to a utility class to avoid duplication
- Use proper logging instead of printing raw exceptions
package org.modularsoft.zander.velocity.commands; import com.jayway.jsonpath.JsonPath; import com.velocitypowered.api.command.CommandSource; import com.velocitypowered.api.command.SimpleCommand; import dev.dejvokep.boostedyaml.route.Route; import io.github.ModularEnigma.Request; import io.github.ModularEnigma.Response; import net.kyori.adventure.text.Component; import net.kyori.adventure.text.event.ClickEvent; import net.kyori.adventure.text.format.NamedTextColor; import org.modularsoft.zander.velocity.ZanderVelocityMain; +import java.util.logging.Level; +import java.util.logging.Logger; -public class website implements SimpleCommand { +public class Website implements SimpleCommand { + private static final Logger LOGGER = Logger.getLogger(Website.class.getName()); + private static final String WEB_CONFIG_ENDPOINT = "/web/configuration"; + @Override public void execute(final Invocation invocation) { CommandSource source = invocation.source(); String BaseAPIURL = ZanderVelocityMain.getConfig().getString(Route.from("BaseAPIURL")); String APIKey = ZanderVelocityMain.getConfig().getString(Route.from("APIKey")); try { - // GET request to link to discord. + // GET request to fetch web configuration Request req = Request.builder() - .setURL(BaseAPIURL + "/web/configuration") + .setURL(BaseAPIURL + WEB_CONFIG_ENDPOINT) .setMethod(Request.Method.GET) .addHeader("x-access-token", APIKey) .build(); Response res = req.execute(); String json = res.getBody(); String siteAddress = JsonPath.read(json, "$.data.siteAddress"); Component message = Component.text("For all information regarding the Network, visit our website: " + siteAddress) .color(NamedTextColor.YELLOW); message = message.clickEvent(ClickEvent.clickEvent(ClickEvent.Action.OPEN_URL,siteAddress)); source.sendMessage(message); } catch (Exception e) { Component builder = Component.text("An error has occurred. Is the API down?").color(NamedTextColor.RED); source.sendMessage(builder); - System.out.println(e); + LOGGER.log(Level.SEVERE, "Error fetching website information", e); } } }zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/rules.java (1)
15-15: 🛠️ Refactor suggestionClass name should follow Java naming conventions
The class name
rulesshould be PascalCase according to Java conventions.-public class rules extends Command { +public class Rules extends Command {zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/website.java (2)
15-15: 🛠️ Refactor suggestionClass name should follow Java naming conventions
The class name
websiteshould be PascalCase according to Java conventions.-public class website extends Command { +public class Website extends Command {
29-34: 💡 Verification agent🧩 Analysis chain
Consider refactoring API key handling
The API key is exposed in request headers. Consider storing sensitive information more securely or implementing a token refresh mechanism.
🏁 Script executed:
#!/bin/bash # Find all occurrences of API key usage in headers to assess the pattern across the codebase rg -A 1 "addHeader.*x-access-token" --glob "*.java"Length of output: 11751
Action Required: Centralize and Secure API Key Management
The API key is embedded directly into HTTP headers in multiple parts of the codebase (including in
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/website.javaat lines 29-34). Rather than spreading this pattern, please consider the following improvements:
- Centralized Token Management: Create a shared utility or API client abstraction that retrieves and injects the API key across requests.
- Secure Storage: Store sensitive credentials using secure storage mechanisms (e.g., environment variables or a secrets manager) rather than directly accessing them from configuration.
- Token Refresh: If applicable, implement a token refresh workflow to automatically replace expired tokens.
This refactor will improve security and maintainability across the codebase.
🧹 Nitpick comments (84)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserChatEvent.java (2)
54-58: Consider adding null checks before accessing nested properties.When building the DiscordChat object, there's a potential for NullPointerException if the player's server or server info is null. This could happen if a player's connection state is in transition.
DiscordChat chat = DiscordChat.builder() .username(((ProxiedPlayer) event.getSender()).getDisplayName()) - .server(((ProxiedPlayer) event.getSender()).getServer().getInfo().getName()) + .server(player.getServer() != null && player.getServer().getInfo() != null ? + player.getServer().getInfo().getName() : "unknown") .content(event.getMessage().toString()) .build();Also, since you already have a
playervariable, consider using it instead of castingevent.getSender()multiple times.
71-72: Improve error logging.The current implementation prints the exception to standard output but doesn't provide context for troubleshooting.
player.sendMessage(new TextComponent(ChatColor.YELLOW + "The chat filter could not be reached at this time, there maybe an issue with the API.")); -System.out.println(e); +plugin.getProxy().getLogger().warning("Error processing chat filter: " + e.getMessage()); +e.printStackTrace();zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/user/UserCreation.java (3)
9-14: Consider making fields private and final.While Lombok's
@Getterannotation is used correctly, consider making the fields explicitly private and final for better encapsulation and immutability:- @Getter UUID uuid; - @Getter String username; + @Getter private final UUID uuid; + @Getter private final String username;
15-18: Consider caching the Gson instance for better performance.Creating a new Gson instance on every toString() call may be inefficient. Consider using a static instance:
@Builder public class UserCreation { + private static final Gson GSON = new Gson(); @Getter UUID uuid; @Getter String username; @Override public String toString() { - return new Gson().toJson(this); + return GSON.toJson(this); } }
1-20: Note code duplication between modules.This class appears to be identical to
UserCreation.javain thezander-velocitymodule. Consider extracting this model to a shared module to avoid duplication if these classes need to remain in sync.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordLeave.java (1)
1-17: Clean implementation of the Discord leave event model.The implementation follows the project's established pattern for model classes and is consistent with other Discord event models in both the waterfall and velocity modules.
Consider these optional improvements:
- Create a static Gson instance to avoid creating a new one on each
toString()call:+ private static final Gson GSON = new Gson(); @Override public String toString() { - return new Gson().toJson(this); + return GSON.toJson(this); }
- Consider making the class immutable by using
finalfor the field or using Lombok's@Valueinstead of@Builderand@Getter:-@Builder -public class DiscordLeave { - - @Getter String username; +@Value +@Builder +public class DiscordLeave { + + String username;
- Add validation to ensure the username isn't null, if appropriate for your use case.
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordJoin.java (1)
1-6: Package Declaration & Imports Validation
The package declaration has been updated toorg.modularsoft.zander.waterfall.model.discord, which aligns well with the restructuring of the project. The necessary imports (Gson, Lombok’s Builder, and Getter) are appropriately included, matching patterns used in related classes.Consider whether instantiating a new
Gsonobject each time intoString()is acceptable for your performance profile—if these methods are called frequently, you might consider defining a static final instance to avoid unnecessary object creation.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordSwitch.java (2)
7-12: Class Declaration and Field AccessibilityThe class effectively leverages Lombok’s
@Builderand@Getterannotations to reduce boilerplate code. However, consider declaring the fields explicitly asprivate(e.g.,private String username;) to better encapsulate the class internals and adhere to common Java best practices.
13-17:toStringMethod EnhancementThe overridden
toStringmethod provides a neat JSON representation using Gson. To optimize performance, consider refactoring by using a static final Gson instance (e.g.,private static final Gson GSON = new Gson();) rather than creating a new instance with every method call. This change can reduce object instantiation overhead.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/spy/DiscordSocialSpy.java (2)
10-13: Enhance Field Encapsulation
All fields are annotated with@Getter, but they are currently using default (package-private) access. It is a good practice to declare them asprivateto enforce encapsulation and protect the object's state.Example change:
- @Getter String usernameFrom; + @Getter private String usernameFrom;Repeat similarly for
usernameTo,directMessage, andserver.
15-18: Optimize toString() Method for Performance
The overriddentoString()method creates a newGsoninstance on each call. This might not be a problem unless this method is invoked very frequently. However, for better performance, consider caching a static final instance ofGson.Example change:
- @Override - public String toString() { - return new Gson().toJson(this); - } + private static final Gson gson = new Gson(); + + @Override + public String toString() { + return gson.toJson(this); + }zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordLeave.java (2)
10-10: Enhance Field EncapsulationThe
usernamefield is declared without an explicit access modifier. For better encapsulation and in line with common best practices when using Lombok, consider declaring it asprivate(and possiblyfinalif immutability is desired). For example:- @Getter String username; + @Getter + private final String username;
12-15: OptimizetoStringImplementationCurrently, a new
Gsoninstance is created every timetoString()is called. If this method is invoked frequently, consider caching a sharedGsoninstance to improve performance. For example:+ private static final Gson GSON = new Gson(); @Override public String toString() { - return new Gson().toJson(this); + return GSON.toJson(this); }This change reduces object creation overhead in scenarios where
toString()is called repeatedly.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/session/SessionSwitch.java (2)
16-18: Consider using a static Gson instance for better performance.Creating a new Gson instance on each
toString()call can be inefficient if called frequently. Consider using a static instance instead.@Builder public class SessionSwitch { @Getter UUID uuid; @Getter String server; + private static final Gson GSON = new Gson(); @Override public String toString() { - return new Gson().toJson(this); + return GSON.toJson(this); } }
12-13: Consider making fields final for immutability.Since this appears to be a data transfer object, making fields final would ensure immutability and thread safety.
@Builder public class SessionSwitch { - @Getter UUID uuid; - @Getter String server; + @Getter final UUID uuid; + @Getter final String server; @Override public String toString() { return new Gson().toJson(this); } }zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/session/UserOnSwitch.java (2)
27-28: Cache configuration values for better performance.Consider caching the API URL and key as class fields rather than retrieving them on each event, especially if they don't change during runtime.
public class UserOnSwitch { private static final Logger logger = ZanderVelocityMain.getLogger(); + private static final String BASE_API_URL = ZanderVelocityMain.getConfig().getString(Route.from("BaseAPIURL")); + private static final String API_KEY = ZanderVelocityMain.getConfig().getString(Route.from("APIKey")); @Subscribe public void onServerConnect(ServerConnectedEvent event) { Player player = event.getPlayer(); String username = player.getUsername(); UUID playerUUID = player.getUniqueId(); String server = event.getServer().getServerInfo().getName(); - String BaseAPIURL = ZanderVelocityMain.getConfig().getString(Route.from("BaseAPIURL")); - String APIKey = ZanderVelocityMain.getConfig().getString(Route.from("APIKey"));
27-28: Follow Java naming conventions for variables.Variables
BaseAPIURLandAPIKeyshould follow camelCase convention rather than PascalCase.- String BaseAPIURL = ZanderVelocityMain.getConfig().getString(Route.from("BaseAPIURL")); - String APIKey = ZanderVelocityMain.getConfig().getString(Route.from("APIKey")); + String baseApiUrl = ZanderVelocityMain.getConfig().getString(Route.from("BaseAPIURL")); + String apiKey = ZanderVelocityMain.getConfig().getString(Route.from("APIKey"));Don't forget to update these variables throughout the code where they're used.
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserOnProxyPing.java (2)
19-21: Method Naming Convention Suggestion
Although not part of the annotated changes, note that the event handler methodPlayerOnProxyPingstarts with an uppercase letter. For improved readability and to adhere to Java’s conventional lowerCamelCase naming for methods, consider renaming it to something likeonProxyPing.
38-41: Exception Handling Improvement
In the catch block, the exception is logged usingSystem.out.print(e). For production quality code and better logging practices, consider using a logging framework (or at leaste.printStackTrace()) to capture error details.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/session/SessionCreate.java (2)
12-13: Consider making fields private for better encapsulationWhile Lombok's @Getter provides public access methods, it's a best practice to explicitly declare the fields as private for better encapsulation.
- @Getter UUID uuid; - @Getter String ipAddress; + @Getter private UUID uuid; + @Getter private String ipAddress;
13-13: Consider adding validation for ipAddressThe ipAddress field lacks validation. Depending on how this class is used, it might be beneficial to add validation to ensure the IP address is in a correct format.
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/session/SessionDestroy.java (1)
1-19: LGTM! Simple and effective session destroy model.This class effectively serves as a data transfer object for session destruction events. The implementation is clean and follows good practices with the use of Lombok annotations for reducing boilerplate code.
Consider reusing a single Gson instance rather than creating a new one each time toString() is called, which could be inefficient if called frequently:
public class SessionDestroy { + private static final Gson GSON = new Gson(); @Getter UUID uuid; @Override public String toString() { - return new Gson().toJson(this); + return GSON.toJson(this); } }zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/session/UserOnDisconnect.java (3)
16-19: Method naming convention doesn't follow Java standards.The event handler method
UserDisconnectEventstarts with an uppercase letter, which doesn't follow Java's standard camelCase naming convention for methods.@Subscribe - public void UserDisconnectEvent (DisconnectEvent event) { + public void userDisconnectEvent(DisconnectEvent event) { Player player = event.getPlayer();
23-24: Logic for checking if player is connected needs clarification.You're checking if the player is still connected and returning early if true, but this is during a disconnect event. The comment would help explain why this check is necessary (e.g., handling quick reconnects).
boolean isConnected = isConnected(ZanderVelocityMain.getProxy(), player.getUsername()); - if (isConnected) return; + // If player is still connected (might happen with quick reconnects), skip processing + if (isConnected) return;
26-28: Fix typo in comment.There's a typo in the comment ("Destory" instead of "Destroy").
// - // Destory Session API POST + // Destroy Session API POST //zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/util/api/Heartbeat.java (4)
13-17: Heartbeat Task Initialization
The implementation for starting the heartbeat task via a scheduled executor is straightforward. However, consider whether the executor should eventually be shut down gracefully during application shutdown to avoid potential thread leaks.
23-27: Repeated Configuration Lookups
The code callsConfigurationManager.getConfig()twice in quick succession (lines 24 and 26). Consider retrieving the configuration once into a local variable which can improve readability and ensure consistency if the configuration is subject to change.
Example diff:- .setURL(ConfigurationManager.getConfig().get("BaseAPIURL") + "/heartbeat") - .addHeader("x-access-token", String.valueOf(ConfigurationManager.getConfig().get("APIKey"))) + Configuration config = ConfigurationManager.getConfig(); + .setURL(config.get("BaseAPIURL") + "/heartbeat") + .addHeader("x-access-token", String.valueOf(config.get("APIKey")))
37-39: Refactor Repeated Player Disconnection Logic
The logic for disconnecting all players is duplicated in both the unsuccessful heartbeat check (lines 37–39) and within the exception handler (lines 48–50). Extracting this logic into a separate helper method (e.g.,disconnectAllPlayers(String reason)) would adhere to DRY principles and improve maintainability.
Example diff:+ private static void disconnectAllPlayers(String reason) { + ProxyServer.getInstance().getPlayers().forEach(player -> player.disconnect(reason)); + } ... - ProxyServer.getInstance().getPlayers().forEach(player -> { - player.disconnect("API Heartbeat Failed, the server is temporarily offline."); - }); + disconnectAllPlayers("API Heartbeat Failed, the server is temporarily offline.");Also applies to: 48-50
32-33: Logging Considerations
UsingSystem.out.printlnande.printStackTrace()is fine during initial development; however, for production code it is recommended to use a dedicated logging framework. This would allow for configurable log levels and better centralized log management.Also applies to: 45-46
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/spy/DiscordCommandSpy.java (3)
16-16: Consider using a static Gson instance for better performance.Creating a new Gson instance on every
toString()call is inefficient. Consider using a static instance instead.@Builder public class DiscordCommandSpy { @Getter String username; @Getter String command; @Getter String server; + + private static final Gson GSON = new Gson(); @Override public String toString() { - return new Gson().toJson(this); + return GSON.toJson(this); } }
10-12: Consider adding field validation and documentation.Consider adding field validation and Javadoc comments to improve code maintainability.
@Builder public class DiscordCommandSpy { + /** + * The username of the player who executed the command + */ @Getter String username; + + /** + * The command that was executed + */ @Getter String command; + + /** + * The server on which the command was executed + */ @Getter String server;You might also want to consider making the fields
finalif they're not meant to be changed after object creation.
1-19:❓ Verification inconclusive
Consider centralizing duplicate model classes.
This class appears to be nearly identical to
DiscordCommandSpyin the velocity module. Consider creating a shared model library to avoid code duplication between modules.
🏁 Script executed:
#!/bin/bash # Check similarities between the Velocity and Waterfall model classes # Let's check what similar model classes exist in both modules echo "Similar Discord model classes in both modules:" fd -e java "Discord.*\.java" zander-velocity/src/main zander-waterfall/src/main | grep -v "Test" | sort # Compare the contents of the DiscordCommandSpy classes echo -e "\nComparison of DiscordCommandSpy classes:" diff -u <(cat zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/spy/DiscordCommandSpy.java) \ <(cat zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/spy/DiscordCommandSpy.java) || trueLength of output: 1824
🏁 Script executed:
#!/bin/bash # Compute SHA256 hashes for both DiscordCommandSpy classes to verify if they are identical echo "SHA256 for zander-velocity DiscordCommandSpy:" sha256sum zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/spy/DiscordCommandSpy.java echo "SHA256 for zander-waterfall DiscordCommandSpy:" sha256sum zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/spy/DiscordCommandSpy.javaLength of output: 635
Centralize Duplicate Model Classes
It appears that a nearly identical implementation of
DiscordCommandSpyexists in both the Velocity and Waterfall modules. To reduce duplication and simplify maintenance, please consider consolidating these into a shared model library. Before proceeding, verify that both versions indeed serve the same purpose and that no module‐specific behavior is expected. Also, review any related models (e.g.,DiscordSocialSpy) for possible centralization.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserOnLogin.java (3)
20-21: Consider Renaming the Event Handler Method
While the functionality remains unaffected, the method nameUserLoginEvent(line 21) does not follow standard Java naming conventions. For better readability and consistency, consider renaming it to something likeonUserLoginorhandleUserLogin.
Suggested diff:- public void UserLoginEvent (PostLoginEvent event) { + public void onUserLogin(PostLoginEvent event) {
80-89: Improve Exception Logging Strategy
Currently, the catch blocks log exceptions usingSystem.out.println(e)and disconnect the player with a generic error message. For improved traceability and consistency with the plugin’s logging practices, consider using the plugin’s logging mechanism (e.g.,plugin.getLogger()). This also allows for better log level management.
Suggested diff example:- System.out.println(e); + plugin.getLogger().severe("Error during API call", e);
24-89: Refactor Nested Try-Catch Blocks to Reduce Redundancy
Both the outer and inner try-catch blocks share similar error handling logic (disconnecting the player and issuing an error message). Consider refactoring the duplicated error-handling code into a helper method. This will improve maintainability and clarity.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ConfigurationManager.java (2)
18-21: Folder Creation Logic Clarification
The nested one-line if-statement on line 20 may reduce readability. Consider refactoring it as:if (!dataFolder.exists()) { if (!dataFolder.mkdirs()) { plugin.getLogger().info("Failed to create configuration folder."); } }This improves clarity.
24-34: Resource Management Improvement
The file-copy logic (lines 28–33) would benefit from using try-with-resources for both theInputStreamandFileOutputStreamto ensure resources are automatically closed even if an exception occurs.zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/session/SessionSwitch.java (1)
1-21: Well-structured session switch bean.
The updated package structure is consistent with the overall reorganization. The use of Lombok to auto-generate builder methods and getters, along with Gson for JSON conversion, offers a clean implementation.
Suggestion: Consider explicitly marking the fields (e.g.,uuidandserver) asprivateto improve encapsulation and clarity of intent.zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/session/SessionCreate.java (1)
1-21: Clear and consistent session create bean.
The reorganization of the package and design using Lombok and Gson is clear and in line with other session-related classes.
Suggestion: As with similar classes, explicitly declare the fields asprivateto enhance encapsulation.zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/spy/DiscordCommandSpy.java (1)
1-20: Clean command spy class aligned with new module structure.
The package update is correctly applied, and the class maintains consistency with the overall design using Lombok for builders and getters.
Suggestion: For improved encapsulation, consider explicitly declaring the fields asprivate.zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/Report.java (1)
1-20: Good model implementation with room for improvements.The
Reportclass looks good overall with the builder pattern and immutable design. A few suggestions to enhance it:
- Consider adding the
finalmodifier to the fields to enforce immutability:- @Getter String reportPlatform; + @Getter private final String reportPlatform;
- The current
toString()implementation creates a new Gson instance on every call, which is inefficient if called frequently.+ private static final Gson GSON = new Gson(); + @Override public String toString() { - return new Gson().toJson(this); + return GSON.toJson(this); }
- Consider adding class and field-level Javadoc to document the purpose and expected values for each field.
zander-velocity/dependency-reduced-pom.xml (1)
1-82: Overall POM Structure and Dependency Management Looks Good
The new POM for thezander-velocitymodule is well organized. The resource filtering, Maven Compiler Plugin configuration (using Java 17), and the Maven Shade Plugin (with proper package relocation) are clearly defined.
• Note: Thevelocity-apidependency is currently specified as a snapshot (3.4.0-SNAPSHOT); ensure that this is acceptable for your release process or plan to upgrade to a stable version when available.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserOnSwitch.java (2)
27-48: Session Switch API Integration and Error Handling
The creation and submission of theSessionSwitchAPI request are implemented correctly. However, consider using the plugin’s logger (e.g.,plugin.getLogger()) instead ofSystem.out.printlnwhen printing exceptions. This would improve consistency with other parts of the codebase and provide better control over logging.
50-70: Discord Switch API Integration is Consistent
The second try block for the Discord switch API POST request mirrors the Session switch block effectively. The consistent structure enhances readability. Again, replacingSystem.out.printlnwith proper logger statements is recommended for production-quality error logging.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserSocialSpyEvent.java (1)
29-53: Discord Social Spy API Call is Well-Structured
The builder pattern forDiscordSocialSpyand the subsequent API request handling are clear and consistent with the rest of the event logic. As in previous files, consider using the plugin’s logging framework for error reporting rather thanSystem.out.printlnand potentiallyplayer.disconnect(..)to provide a more robust error handling mechanism.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserOnDisconnect.java (2)
27-45: Session Destroy API Call Handling
The API call for destroying the session is correctly built and executed. One observation: the early return with the conditionif (player.isConnected()) return;might need further verification to ensure it aligns with the intended logic upon disconnect events. Additionally, replacingSystem.out.println(e)with the plugin’s logger is advisable for improved error tracking.
48-69: Discord Leave API Integration is Consistent and Clear
The second try block, handling the Discord leave request, follows a clear and consistent pattern. Consistent with other API calls, consider utilizing the project’s logging mechanism rather than directly printing exceptions.zander-waterfall/pom.xml (2)
12-24: POM Module and Artifact Updates Need Attention
The artifact ID has been updated fromzander-proxytozander-waterfall, and module declarations have been adjusted (with the removal of the old module and the addition ofzander-velocity). This is consistent with the overall project reorganization.
• Important: The Maven Compiler Plugin is configured to use source and target version 14 here, which contrasts with the Java 17 configuration in thezander-velocitymodule. Review whether this discrepancy is intentional or if all modules should be aligned to the same Java version.
25-41: Repository and Dependency Declarations are Clear
The repository definitions and dependency declarations are clearly stated. Verify that the dependencies (e.g.,waterfall-api,json-simple,Requests) are the intended versions for this module. Also, the use of a system-scoped dependency fornuvotifier.jarshould be reviewed to ensure it remains compatible with your deployment environment.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/ping.java (2)
9-13: Consider using PascalCase for class namesIn Java, the convention is to use PascalCase (starting with uppercase) for class names. Consider renaming
pingtoPingto follow standard Java naming conventions.-public class ping extends Command { +public class Ping extends Command { - public ping() { + public Ping() {
15-24: Add command output for non-player sendersYour implementation doesn't handle cases where the command is executed by a non-player sender (such as the console). Consider adding a response for non-player senders.
@Override public void execute(CommandSender commandSender, String[] strings) { if (commandSender instanceof ProxiedPlayer) { ProxiedPlayer player = (ProxiedPlayer) commandSender; int ping = player.getPing(); player.sendMessage(new TextComponent("Your ping is: " + ChatColor.GREEN + ping)); return; } + commandSender.sendMessage(new TextComponent(ChatColor.RED + "This command can only be executed by a player!")); }zander-waterfall/dependency-reduced-pom.xml (1)
36-50: Consider versioning the system dependencyThe
nuvotifier-universaldependency is using a system scope with a hard-coded path. This approach has a few drawbacks:
- It makes the build less portable
- The dependency isn't properly versioned in your repository
Consider using a local Maven repository instead, or if possible, use a published version of the library from a public repository.
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/Filter.java (2)
7-11: Use proper field declaration with LombokWhen using Lombok's
@Getter, it's better to explicitly declare the field's access modifier and use proper formatting.@Builder public class Filter { - @Getter String content; + @Getter + private String content;
12-15: Consider caching the Gson instanceCreating a new Gson instance for each toString() call is inefficient. Consider using a static Gson instance instead.
+ private static final Gson GSON = new Gson(); + @Override public String toString() { - return new Gson().toJson(this); + return GSON.toJson(this); }zander-velocity/src/main/java/org/modularsoft/zander/velocity/util/ChatFreezeManager.java (2)
6-12: Add thread safety to chat freeze stateThe
chatFrozenstate is accessed and modified from multiple places and could potentially be accessed concurrently. Consider using thread-safe mechanisms.- private static boolean chatFrozen = false; + private static volatile boolean chatFrozen = false; public static boolean isChatFrozen() { return chatFrozen; } public static void setChatFrozen(boolean frozen) { + synchronized (ChatFreezeManager.class) { chatFrozen = frozen; + } }
1-13: Consider adding persistence for the chat freeze stateCurrently, the chat freeze state is stored in memory and will be reset if the server restarts. Consider implementing persistence for this state, especially if it's important to maintain it across server restarts.
zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/ping.java (1)
10-10: Rename class to follow Java naming conventionsThe class name
pingdoesn't follow Java's standard naming conventions, which recommend using PascalCase for class names. Consider renaming the class toPingCommandto better align with Java conventions.-public class ping implements SimpleCommand { +public class PingCommand implements SimpleCommand {zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/moderation/FreezeChatListener.java (1)
13-21: Implementation looks good with minor improvement suggestionsThe chat freeze listener is well-implemented with proper event subscription and permission checking. A few suggestions for enhancement:
- Consider adding null safety checks for the player object (though Velocity's API likely guarantees it's non-null in this context)
- Consider moving the frozen chat message to a configuration file for easier customization by server administrators
Otherwise, the integration with ChatFreezeManager is clean and the functionality works as expected.
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/discord.java (1)
15-15: Consider renaming class to follow Java conventionsThis is a pre-existing issue (not introduced by this change), but the class name
discorddoesn't follow Java's standard naming conventions. Consider renaming it toDiscordCommandin a future update to better align with Java conventions.zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/website.java (1)
1-44: Consider creating a utility class to eliminate code duplicationAll three command classes (Rules, Discord, Website) share almost identical code for making API requests and handling responses. To follow the DRY principle, consider extracting this common functionality into a utility class.
// Example utility class that could be created: package org.modularsoft.zander.velocity.util; import com.jayway.jsonpath.JsonPath; import com.velocitypowered.api.command.CommandSource; import io.github.ModularEnigma.Request; import io.github.ModularEnigma.Response; import net.kyori.adventure.text.Component; import net.kyori.adventure.text.event.ClickEvent; import net.kyori.adventure.text.format.NamedTextColor; import org.modularsoft.zander.velocity.ZanderVelocityMain; import dev.dejvokep.boostedyaml.route.Route; import java.util.logging.Level; import java.util.logging.Logger; public class WebConfigUtil { private static final Logger LOGGER = Logger.getLogger(WebConfigUtil.class.getName()); private static final String WEB_CONFIG_ENDPOINT = "/web/configuration"; public static String getSiteAddress() throws Exception { String baseAPIURL = ZanderVelocityMain.getConfig().getString(Route.from("BaseAPIURL")); String apiKey = ZanderVelocityMain.getConfig().getString(Route.from("APIKey")); Request req = Request.builder() .setURL(baseAPIURL + WEB_CONFIG_ENDPOINT) .setMethod(Request.Method.GET) .addHeader("x-access-token", apiKey) .build(); Response res = req.execute(); String json = res.getBody(); return JsonPath.read(json, "$.data.siteAddress"); } public static void sendClickableMessage(CommandSource source, String message, String url, NamedTextColor color) { try { String siteAddress = getSiteAddress(); Component component = Component.text(message + siteAddress + url) .color(color); component = component.clickEvent(ClickEvent.clickEvent(ClickEvent.Action.OPEN_URL, siteAddress + url)); source.sendMessage(component); } catch (Exception e) { Component builder = Component.text("An error has occurred. Is the API down?").color(NamedTextColor.RED); source.sendMessage(builder); LOGGER.log(Level.SEVERE, "Error fetching site information", e); } } }Then in your command classes, you could simplify to:
public class Website implements SimpleCommand { @Override public void execute(final Invocation invocation) { WebConfigUtil.sendClickableMessage( invocation.source(), "For all information regarding the Network, visit our website: ", "", NamedTextColor.YELLOW ); } }zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/moderation/freezechat.java (3)
33-36: Use text components more effectively for better message compositionString concatenation in text components can be replaced with component joining for better maintainability and text formatting control.
- Component message = isNowFrozen - ? Component.text("Chat has been frozen by " + senderName + ".").color(NamedTextColor.RED) - : Component.text("Chat has been unfrozen by " + senderName + ".").color(NamedTextColor.GREEN); + Component message = isNowFrozen + ? Component.text("Chat has been frozen by ").color(NamedTextColor.RED) + .append(Component.text(senderName).color(NamedTextColor.GOLD)) + .append(Component.text(".").color(NamedTextColor.RED)) + : Component.text("Chat has been unfrozen by ").color(NamedTextColor.GREEN) + .append(Component.text(senderName).color(NamedTextColor.GOLD)) + .append(Component.text(".").color(NamedTextColor.GREEN));
37-37: Validate proxy reference before accessingEnsure the
ZanderVelocityMain.proxyis not null before accessing its methods to preventNullPointerException.- ZanderVelocityMain.proxy.getAllPlayers().forEach(player -> player.sendMessage(message)); + if (ZanderVelocityMain.proxy != null) { + ZanderVelocityMain.proxy.getAllPlayers().forEach(player -> player.sendMessage(message)); + } else { + source.sendMessage(Component.text("Error: Unable to broadcast message, proxy reference is null.") + .color(NamedTextColor.RED)); + }
39-41: Improve confirmation message formattingSimilar to the broadcast message, use component joining for better maintainability.
- source.sendMessage(Component.text("You have " + (isNowFrozen ? "frozen" : "unfrozen") + " the chat.") - .color(NamedTextColor.YELLOW)); + source.sendMessage(Component.text("You have ").color(NamedTextColor.YELLOW) + .append(Component.text(isNowFrozen ? "frozen" : "unfrozen").color(NamedTextColor.GOLD)) + .append(Component.text(" the chat.").color(NamedTextColor.YELLOW)));zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/rules.java (1)
44-47: Improve error loggingConsider using a more targeted error message for logging instead of printing the entire exception stack. This will make logs more concise and readable.
} catch (Exception e) { player.sendMessage(new TextComponent("An error has occurred. Is the API down?")); - System.out.println(e); + plugin.getLogger().warning("Error fetching rules URL: " + e.getMessage()); + if (plugin.getLogger().isLoggable(java.util.logging.Level.FINE)) { + e.printStackTrace(); + } }zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/website.java (1)
44-47: Improve error loggingConsider using a more targeted error message for logging instead of printing the entire exception stack. This will make logs more concise and readable.
} catch (Exception e) { player.sendMessage(new TextComponent("An error has occurred. Is the API down?")); - System.out.println(e); + plugin.getLogger().warning("Error fetching website URL: " + e.getMessage()); + if (plugin.getLogger().isLoggable(java.util.logging.Level.FINE)) { + e.printStackTrace(); + } }zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/moderation/clearchat.java (1)
48-49: Simplify confirmation messageThe confirmation message is unnecessarily verbose for the command executor. They already know they initiated the command, and the third-person perspective is unusual.
- source.sendMessage(Component.text("Chat cleared for this server by " + senderName + ".") + source.sendMessage(Component.text("You have cleared the chat for this server.") .color(NamedTextColor.GREEN));zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ZanderProxyMain.java (1)
1-1: Consider renaming the class to match the new package name.
You’ve updated the package to “waterfall” but the class name is stillZanderProxyMain. Renaming it to something likeZanderWaterfallMainmay help maintain naming consistency and avoid confusion down the line.zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/UserCommandSpyEvent.java (1)
34-55: Avoid forcibly disconnecting users on API failures.
Currently, any failure triggersplayer.disconnect(). Consider a gentler fallback, e.g., notifying them that the feature is temporarily unavailable while letting them remain connected. This will reduce frustration when your external API is down.try { ... } catch (Exception e) { - Component builder = Component.text("An error has occurred. Is the API down?") - .color(NamedTextColor.RED); - player.disconnect(builder); + player.sendMessage(Component.text("Command spy unavailable. You can continue playing.") + .color(NamedTextColor.RED)); System.out.println(e); }zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/UserSocialSpyEvent.java (1)
31-39: Validate command structure more thoroughly.
When the user types a direct message command but omits parts of it, you log a warning and return early. That’s good. For further robustness, consider providing them with usage hints if the command format is invalid.zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/UserOnProxyPing.java (1)
45-60: Extract duplicate code to a helper method.The code for creating a server ping description is duplicated in both the try block and catch block. Consider extracting this to a helper method to improve maintainability.
} catch (Exception e) { - System.out.print(e); + ZanderVelocityMain.getLogger().error("Error fetching MOTD from API", e); - // ServerPing serverPing = event.getPing(); - - String motdTopLine = ZanderVelocityMain.getConfig().getString(Route.from("announcementMOTDTopLine")); - Component serverPingDescription = LegacyComponentSerializer.builder() - .character('&') - .build() - .deserialize(motdTopLine + "\n" + "&3&lPowered by Zander"); - - ServerPing newServerPing = ServerPing.builder() - .description(serverPingDescription) - .build(); - - event.setPing(newServerPing); + createDefaultServerPing(event); +} + +private void createDefaultServerPing(ProxyPingEvent event) { + String motdTopLine = ZanderVelocityMain.getConfig().getString(Route.from("announcementMOTDTopLine")); + Component serverPingDescription = LegacyComponentSerializer.builder() + .character('&') + .build() + .deserialize(motdTopLine + "\n" + "&3&lPowered by Zander"); + + ServerPing newServerPing = ServerPing.builder() + .description(serverPingDescription) + .build(); + + event.setPing(newServerPing); }zander-velocity/pom.xml (1)
42-48: Consider adding more comprehensive excludes in the Maven Shade Plugin.Your current configuration only excludes META-INF/*.MF files. Consider adding more comprehensive excludes to avoid potential conflicts with other libraries.
<filter> <artifact>*:*</artifact> <excludes> <exclude>META-INF/*.MF</exclude> + <exclude>META-INF/*.SF</exclude> + <exclude>META-INF/*.DSA</exclude> + <exclude>META-INF/*.RSA</exclude> + <exclude>META-INF/LICENSE*</exclude> + <exclude>META-INF/NOTICE*</exclude> </excludes> </filter>zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/UserChatEvent.java (1)
45-47: Inconsistent logging format.Line 46 logs with a "[FILTER]" prefix, but line 67 doesn't use any prefix. Consider standardizing your logging format across the codebase.
Boolean success = JsonPath.parse(phraseJson).read("$.success"); String phraseCaughtMessage = JsonPath.read(phraseJson, "$.message"); ZanderVelocityMain.getLogger().info("[FILTER] Response (" + phraseRes.getStatusCode() + "): " + phraseRes.getBody());And in line 67:
Response discordChatReqRes = discordChatReq.execute(); -ZanderVelocityMain.getLogger().info("Response (" + discordChatReqRes.getStatusCode() + "): " + discordChatReqRes.getBody()); +ZanderVelocityMain.getLogger().info("[DISCORD] Response (" + discordChatReqRes.getStatusCode() + "): " + discordChatReqRes.getBody());zander-velocity/src/main/java/org/modularsoft/zander/velocity/util/api/Heartbeat.java (1)
37-38: Remove or convert to proper logging.Debug print statements should be converted to proper logging using the appropriate log level.
Boolean heartbeat = JsonPath.read(json, "$.success"); -System.out.println("API Heartbeat Success"); +ZanderVelocityMain.getLogger().debug("API Heartbeat Success");zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/report.java (1)
43-71: Add more robust error handling for API interactions.The current implementation catches all exceptions and provides a generic error message. This makes it difficult to diagnose specific issues (network errors, API errors, authentication failures, etc.).
try { // Build the report object Report report = Report.builder() .reportPlatform("INGAME") .reportedUser(reportedUsername) .reporterUser(player.getUsername()) .reportReason(reason) .build(); // Create and send the request Request reportReq = Request.builder() .setURL(BaseAPIURL + "/report/create") .setMethod(Request.Method.POST) .addHeader("x-access-token", APIKey) .setRequestBody(report.toString()) .build(); // Get the response from the API Response reportRes = reportReq.execute(); String json = reportRes.getBody(); Boolean success = JsonPath.read(json, "$.success"); String message = JsonPath.read(json, "$.message"); if (success) { player.sendMessage(Component.text(message).color(NamedTextColor.GREEN)); } else { player.sendMessage(Component.text(message).color(NamedTextColor.RED)); } -} catch (Exception e) { - player.sendMessage(Component.text("An error has occurred. Is the API down?").color(NamedTextColor.RED)); - System.out.println(e); +} catch (IOException e) { + player.sendMessage(Component.text("Failed to connect to the report API. Please try again later.").color(NamedTextColor.RED)); + ZanderVelocityMain.getLogger().error("Failed to connect to report API", e); +} catch (Exception e) { + player.sendMessage(Component.text("An error occurred while processing your report.").color(NamedTextColor.RED)); + ZanderVelocityMain.getLogger().error("Error processing report command", e); }zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/session/UserOnLogin.java (4)
18-18: Method naming should follow Java conventions.Method names in Java should follow camelCase format.
-public void UserLoginEvent (PostLoginEvent event) { +public void userLoginEvent(PostLoginEvent event) {
32-40: Check API response status code before proceeding.The code logs the API response but doesn't check if the request was successful based on the status code before proceeding.
Response createUserRes = createUserReq.execute(); ZanderVelocityMain.getLogger().info("Response (" + createUserRes.getStatusCode() + "): " + createUserRes.getBody()); +// Verify successful status code (2xx range) +if (createUserRes.getStatusCode() < 200 || createUserRes.getStatusCode() >= 300) { + ZanderVelocityMain.getLogger().warn("User creation API returned non-success status code: " + createUserRes.getStatusCode()); + // Consider how to handle non-successful responses +}
48-48: Improve handling of IP address formatting.The IP address is retrieved using
toString()which includes the/prefix. This might cause issues with API processing.- .ipAddress(player.getRemoteAddress().getAddress().toString()) + .ipAddress(player.getRemoteAddress().getAddress().getHostAddress())
75-78: Provide more specific error messages.The current implementation disconnects the player with a generic error message. Consider logging the specific exception details and providing more context in the error message.
} catch (Exception e) { - Component builder = Component.text("An error has occurred. Is the API down?").color(NamedTextColor.RED); + ZanderVelocityMain.getLogger().error("Error during session creation or Discord notification", e); + Component builder = Component.text("An error occurred while setting up your session. Please try again later.").color(NamedTextColor.RED); player.disconnect(builder); }zander-velocity/src/main/java/org/modularsoft/zander/velocity/util/announcement/TipChatter.java (3)
32-32: Comment doesn't match actual interval configuration.The comment mentions "every 10 seconds" but the code uses a configurable interval from the config in minutes.
- // Schedule the task to run every 10 seconds + // Schedule the task to run at configurable intervals (defined in config)
64-68: Standardize logging approach.The code mixes SLF4J logging with direct System.out.println calls. Standardize on the SLF4J logger for consistency.
} catch (Exception e) { // Handle exceptions here logger.error("Announcement Tip Failed, will try again later.", e); - System.out.println("Announcement Tip Failed, will try again in " + ZanderVelocityMain.getConfig().getString(Route.from("announcementTipInterval")) + " minutes."); + logger.info("Announcement Tip Failed, will try again in {} minutes.", ZanderVelocityMain.getConfig().getString(Route.from("announcementTipInterval"))); }
48-51: Remove or reduce excessive logging.The current implementation logs the entire JSON response, which could be large and pollute logs. Consider logging only essential information.
// Log the color message format and link logger.info("Announcement Tip: {}", colourMessageFormat); -logger.info("Link: {}", link); -logger.info("JSON: {}", json); +if (link != null && !link.isEmpty()) { + logger.info("Link: {}", link); +} +logger.debug("JSON response received: {}", json); // Move to debug levelzander-velocity/src/main/java/org/modularsoft/zander/velocity/ZanderVelocityMain.java (3)
48-55: Consider avoiding static fieldsStatic fields can make testing difficult and could cause issues if the plugin is loaded multiple times. Additionally, the
proxyfield is both static and public which violates encapsulation. Consider making these instance fields instead.- @Getter - private static Logger logger; - @Getter - public static ProxyServer proxy; - @Getter - private static YamlDocument config; + @Getter + private final Logger logger; + @Getter + private final ProxyServer proxy; + @Getter + private static YamlDocument config; // Can remain static if needed for global access
69-71: Unnecessary CommandManager instantiationYou already have a
commandManagerfield that's injected in the constructor. There's no need to get it again from the proxy.- // Commands - CommandManager commandManager = proxy.getCommandManager(); + // CommandsThen update the subsequent command registrations to use
this.commandManagerinstead.
116-116: Consider moving startup message to initialization methodThe startup message is currently in the constructor, but at this point the plugin hasn't been fully initialized (event listeners and commands haven't been registered yet). Consider moving this message to the end of the
onProxyInitializationmethod.- logger.info("Zander Proxy has started.");And add to the end of
onProxyInitialization:logger.info("Zander Proxy has been fully initialized.");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
zander-waterfall/src/main/resources/nuvotifier.jaris excluded by!**/*.jar
📒 Files selected for processing (65)
pom.xml(2 hunks)zander-auth/pom.xml(2 hunks)zander-hub/pom.xml(1 hunks)zander-velocity/dependency-reduced-pom.xml(1 hunks)zander-velocity/pom.xml(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/ZanderVelocityMain.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/discord.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/moderation/clearchat.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/moderation/freezechat.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/ping.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/report.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/rules.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/website.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/UserChatEvent.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/UserCommandSpyEvent.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/UserOnProxyPing.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/UserSocialSpyEvent.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/moderation/FreezeChatListener.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/session/UserOnDisconnect.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/session/UserOnLogin.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/session/UserOnSwitch.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/Filter.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/Report.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordChat.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordJoin.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordLeave.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordSwitch.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/spy/DiscordCommandSpy.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/spy/DiscordSocialSpy.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/session/SessionCreate.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/session/SessionDestroy.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/session/SessionSwitch.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/user/UserCreation.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/util/ChatFreezeManager.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/util/announcement/TipChatter.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/util/api/Heartbeat.java(1 hunks)zander-velocity/src/main/resources/config.yml(1 hunks)zander-waterfall/dependency-reduced-pom.xml(1 hunks)zander-waterfall/pom.xml(1 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ConfigurationManager.java(1 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ZanderProxyMain.java(1 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/discord.java(1 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/ping.java(1 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/rules.java(1 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/website.java(1 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserChatEvent.java(2 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserCommandSpyEvent.java(2 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserOnDisconnect.java(1 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserOnLogin.java(1 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserOnProxyPing.java(1 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserOnSwitch.java(2 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserSocialSpyEvent.java(2 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/Filter.java(1 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordChat.java(1 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordJoin.java(1 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordLeave.java(1 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordSwitch.java(1 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/spy/DiscordCommandSpy.java(1 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/spy/DiscordSocialSpy.java(1 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/session/SessionCreate.java(1 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/session/SessionDestroy.java(1 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/session/SessionSwitch.java(1 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/user/UserCreation.java(1 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/util/announcement/TipChatter.java(2 hunks)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/util/api/Heartbeat.java(1 hunks)
🧰 Additional context used
🧬 Code Definitions (29)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordLeave.java (1)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/discord.java (1)
discord(15-51)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordChat.java (4)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordChat.java (1)
Builder(7-19)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordJoin.java (1)
Builder(7-17)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordLeave.java (1)
Builder(7-17)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordSwitch.java (1)
Builder(7-18)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordJoin.java (1)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/discord.java (1)
discord(15-51)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordLeave.java (6)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordJoin.java (1)
Builder(7-17)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordLeave.java (1)
Builder(7-17)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/Filter.java (1)
Builder(7-17)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordChat.java (1)
Builder(7-19)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordSwitch.java (1)
Builder(7-18)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordJoin.java (1)
Builder(7-17)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordChat.java (1)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/discord.java (1)
discord(15-51)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/session/SessionSwitch.java (4)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/session/SessionCreate.java (1)
Builder(9-20)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/session/SessionDestroy.java (1)
Builder(9-19)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/session/SessionSwitch.java (1)
Builder(9-20)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/user/UserCreation.java (1)
Builder(9-20)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordSwitch.java (1)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/discord.java (1)
discord(15-51)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordJoin.java (5)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordJoin.java (1)
Builder(7-17)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/Filter.java (1)
Builder(7-17)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordChat.java (1)
Builder(7-19)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordSwitch.java (1)
Builder(7-18)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordLeave.java (1)
Builder(7-17)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/util/api/Heartbeat.java (1)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ConfigurationManager.java (1)
ConfigurationManager(13-54)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/spy/DiscordSocialSpy.java (11)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/Filter.java (1)
Builder(7-17)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordChat.java (1)
Builder(7-19)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordJoin.java (1)
Builder(7-17)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordLeave.java (1)
Builder(7-17)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/spy/DiscordSocialSpy.java (1)
Builder(7-20)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/session/SessionCreate.java (1)
Builder(9-20)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordSwitch.java (1)
Builder(7-18)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/session/SessionDestroy.java (1)
Builder(9-19)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/session/SessionSwitch.java (1)
Builder(9-20)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/user/UserCreation.java (1)
Builder(9-20)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/Filter.java (1)
Builder(7-17)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/user/UserCreation.java (1)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/user/UserCreation.java (1)
Builder(9-20)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordSwitch.java (6)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordJoin.java (1)
Builder(7-17)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordSwitch.java (1)
Builder(7-18)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordChat.java (1)
Builder(7-19)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/spy/DiscordCommandSpy.java (1)
Builder(7-19)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordLeave.java (1)
Builder(7-17)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordJoin.java (1)
Builder(7-17)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/discord.java (2)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ConfigurationManager.java (1)
ConfigurationManager(13-54)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ZanderProxyMain.java (1)
ZanderProxyMain(12-58)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/rules.java (2)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ConfigurationManager.java (1)
ConfigurationManager(13-54)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ZanderProxyMain.java (1)
ZanderProxyMain(12-58)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/session/SessionDestroy.java (1)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/session/SessionDestroy.java (1)
Builder(9-19)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/moderation/freezechat.java (1)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/util/ChatFreezeManager.java (1)
ChatFreezeManager(3-13)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/util/announcement/TipChatter.java (1)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ConfigurationManager.java (1)
ConfigurationManager(13-54)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/spy/DiscordCommandSpy.java (5)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/spy/DiscordCommandSpy.java (1)
Builder(7-19)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordChat.java (1)
Builder(7-19)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordSwitch.java (1)
Builder(7-18)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordLeave.java (1)
Builder(7-17)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordJoin.java (1)
Builder(7-17)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/session/SessionCreate.java (3)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/session/SessionCreate.java (1)
Builder(9-20)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/session/SessionDestroy.java (1)
Builder(9-19)zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/user/UserCreation.java (1)
Builder(9-20)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ZanderProxyMain.java (2)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/util/announcement/TipChatter.java (1)
TipChatter(17-56)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/util/api/Heartbeat.java (1)
Heartbeat(13-54)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/website.java (2)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ConfigurationManager.java (1)
ConfigurationManager(13-54)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ZanderProxyMain.java (1)
ZanderProxyMain(12-58)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserCommandSpyEvent.java (3)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ConfigurationManager.java (1)
ConfigurationManager(13-54)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ZanderProxyMain.java (1)
ZanderProxyMain(12-58)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/discord.java (1)
discord(15-51)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserOnProxyPing.java (2)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ConfigurationManager.java (1)
ConfigurationManager(13-54)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ZanderProxyMain.java (1)
ZanderProxyMain(12-58)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserSocialSpyEvent.java (3)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ConfigurationManager.java (1)
ConfigurationManager(13-54)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ZanderProxyMain.java (1)
ZanderProxyMain(12-58)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/discord.java (1)
discord(15-51)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/ZanderVelocityMain.java (5)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/moderation/clearchat.java (1)
clearchat(12-51)zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/moderation/freezechat.java (1)
freezechat(10-43)zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/moderation/FreezeChatListener.java (1)
FreezeChatListener(11-23)zander-velocity/src/main/java/org/modularsoft/zander/velocity/util/announcement/TipChatter.java (1)
TipChatter(18-71)zander-velocity/src/main/java/org/modularsoft/zander/velocity/util/api/Heartbeat.java (1)
Heartbeat(15-62)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserOnLogin.java (2)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ConfigurationManager.java (1)
ConfigurationManager(13-54)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ZanderProxyMain.java (1)
ZanderProxyMain(12-58)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserOnSwitch.java (2)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ConfigurationManager.java (1)
ConfigurationManager(13-54)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ZanderProxyMain.java (1)
ZanderProxyMain(12-58)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserOnDisconnect.java (3)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ConfigurationManager.java (1)
ConfigurationManager(13-54)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ZanderProxyMain.java (1)
ZanderProxyMain(12-58)zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/discord.java (1)
discord(15-51)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/moderation/FreezeChatListener.java (1)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/util/ChatFreezeManager.java (1)
ChatFreezeManager(3-13)
🪛 YAMLlint (1.35.1)
zander-velocity/src/main/resources/config.yml
[error] 8-8: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (69)
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordChat.java (1)
1-19: Code looks good with proper model implementation.The class is well-structured using Lombok annotations for generating boilerplate code. The
@Builderpattern provides a convenient way to create instances, and@Getterappropriately exposes the fields. ThetoString()method using Gson ensures proper JSON serialization for API communication.This implementation matches the parallel structure in the velocity module with identical field names and serialization approach, maintaining consistency across modules.
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserChatEvent.java (1)
1-15: Package and import restructuring looks good.The renaming from
proxytowaterfallin package declarations and imports is consistent with the overall project reorganization mentioned in the PR summary. All import statements have been properly updated to reflect the new package structure.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/user/UserCreation.java (1)
1-8: Package reorganization looks good.The move from
org.modularsoft.zander.proxy.model.usertoorg.modularsoft.zander.waterfall.model.useraligns with the module reorganization mentioned in the PR summary.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordJoin.java (1)
7-17: Class Implementation Review: DiscordJoin
The class uses Lombok’s@Builderand@Getterannotations effectively to reduce boilerplate. Overriding thetoString()method to return a JSON representation using Gson matches the implementations seen in related classes (e.g., in the zander-velocity module), ensuring consistency across the codebase.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/DiscordSwitch.java (1)
1-6: Package and Import Statements CheckThe package declaration and imported libraries (Gson and Lombok annotations) are properly defined and well-organized within the context of the project.
zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/session/SessionDestroy.java (1)
1-1: Updated Package Declaration.The package declaration has been updated from the previous module (
org.modularsoft.zander.proxy.model.session) to the new structure (org.modularsoft.zander.velocity.model.session). This change reflects the overall reorganization in the project and aligns the class with the appropriate module. Please verify that all references and import statements across the codebase (especially in classes such asUserOnDisconnect.java) have been updated accordingly.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/spy/DiscordSocialSpy.java (3)
1-6: Organize Imports and Package Declaration
The package declaration and import statements are clear and conventional. If your project enforces an import ordering or grouping standard (e.g., static imports separate from non-static), consider aligning with that guideline.
7-8: Appropriate Use of Lombok's @builder Annotation
The use of@Builderat the class level is consistent with similar implementations elsewhere in your project. This enables the builder pattern cleanly.
20-21: File Structure and Closure
The closing bracket and overall file structure are correct. No issues here.zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordLeave.java (1)
1-1: Confirm Package Reorganization in Accordance with Project StructureThe package declaration has been updated to reflect the new module structure (
org.modularsoft.zander.velocity.model.discord). This is consistent with the overall reorganization efforts across the project.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/session/SessionSwitch.java (1)
9-20: Clean and well-structured DTO class.The
SessionSwitchclass is well-designed with appropriate use of Lombok annotations for implementing the builder pattern and generating getters.zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/session/UserOnSwitch.java (4)
18-74: Well-structured event handler with appropriate error handling.The class effectively manages the server connection event with separate handling for session and Discord APIs.
33-52: Critical disconnect on session switch failure is appropriate.The code correctly disconnects the player when the session switch fails, since this is likely essential for proper player tracking.
55-73: Non-critical handling of Discord switch failure is appropriate.Good decision to allow the player to continue playing even if the Discord switch fails, as this is likely a secondary feature.
43-44: Ensure response body format matches API expectations.The comment "Ensure this method works properly" suggests uncertainty about the format. Verify that the JSON serialization from
toString()matches what the API expects.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserOnProxyPing.java (2)
1-1: Package Declaration Updated
The package declaration now correctly reflects the module reorganization by moving toorg.modularsoft.zander.waterfall.events. This aligns with the broader restructuring goals.
4-5: Updated Import Statements
The imports forConfigurationManagerandZanderProxyMainhave been updated to reference the neworg.modularsoft.zander.waterfallpackage, which is consistent with the overall package reorganization.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/session/SessionCreate.java (1)
1-20: Good implementation of the session creation modelThe SessionCreate class is well structured with appropriate use of Lombok annotations for reducing boilerplate code. The implementation matches the design pattern used in other related models across the codebase.
zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/session/UserOnDisconnect.java (1)
71-74: LGTM! Utility method is concise and effective.The
isConnectedutility method is well-implemented and follows Java's Optional pattern correctly.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/util/api/Heartbeat.java (2)
1-1: Package Declaration Updated
The package declaration now correctly reflects the module reorganization from the proxy to the waterfall package. This helps maintain consistency across modules.
7-7: Updated Import Statement
The import forConfigurationManageris updated to the new package structure. Confirm that all usages ofConfigurationManagerin this file are now consistent with the new location.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/discord/spy/DiscordCommandSpy.java (1)
1-19: LGTM - Consistent model class implementation.The implementation follows established patterns used across other Discord-related model classes in the codebase.
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserOnLogin.java (2)
1-1: Package Declaration Updated Correctly
The package declaration has been updated to reflect the reorganization from the proxy module to the waterfall module. This aligns with the new project structure.
3-7: Import Statements Refactored for New Module Structure
The imports now reference thezander.waterfallpackage instead of the old proxy package. This ensures that all dependencies (ConfigurationManager, ZanderProxyMain, DiscordJoin, SessionCreate, and UserCreation) are correctly aligned with the new package structure.zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordJoin.java (2)
1-3: Package Declaration Updated
The package declaration has been correctly updated toorg.modularsoft.zander.velocity.model.discordto reflect the new module structure.
7-8: Lombok and Gson Usage is Appropriate
The use of Lombok’s@Builderannotation and the generated getter for theusernamefield is a clean approach. The overriddentoStringmethod using Gson for JSON conversion is consistent with the project’s design pattern.zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/Filter.java (2)
1-2: Package Declaration Updated
The package declaration is now correctly set toorg.modularsoft.zander.velocity.modelin line with the module restructuring.
7-8: Consistent Model Class Design
The use of Lombok’s@Builderand@Getterannotations along with the JSON conversion in the overriddentoStringmethod keeps the implementation clean and consistent with similar model classes.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ConfigurationManager.java (2)
1-2: Updated Package Declaration
The package declaration is correctly updated toorg.modularsoft.zander.waterfall, aligning this file with the new organizational structure.
39-45: Configuration Loading and Saving Handling
The block that loads and then immediately saves the configuration (lines 39–43) is straightforward; however, verify that automatically saving right after loading is the intended behavior to prevent any accidental overwrites.zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/user/UserCreation.java (2)
1-2: Package Declaration Updated
The package declaration is updated from the original proxy location toorg.modularsoft.zander.velocity.model.user, which is correct given the reorganization.
9-10: Consistent Use of Lombok and JSON Conversion
The use of Lombok’s@Builderand@Getterannotations is consistent with other model classes. The overriddentoStringmethod using Gson is clear; however, please verify that having aUserCreationclass under the velocity module does not conflict with any similarly named class possibly intended for the waterfall module.zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordChat.java (2)
1-2: Updated Package Declaration
The package declaration is now correctly set toorg.modularsoft.zander.velocity.model.discord.
7-8: Lombok and Gson Implementation
The class makes effective use of Lombok’s@Builderand@Getterannotations, and thetoStringmethod cleanly returns a JSON representation of the object. This consistency enhances code maintainability.zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/DiscordSwitch.java (1)
1-19: Solid reorganization with clean implementation.
The updated package declaration correctly reflects the move from the proxy module to the velocity module. The use of Lombok’s@Builderand@Getteralong with Gson for JSON serialization is concise and clear.zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/discord/spy/DiscordSocialSpy.java (1)
1-21: Consistent package refactor and effective bean design.
The package update from the proxy to the velocity namespace is properly applied. The application of Lombok’s annotations and Gson in thetoString()method keeps the class lightweight and readable.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/util/announcement/TipChatter.java (2)
1-1: Package and import updates look consistent with the project reorganization.The package has been updated from proxy to waterfall, which aligns with the broader restructuring effort in the project.
11-11: Import path updated correctly.The import for ConfigurationManager has been properly updated to reflect the new package structure.
zander-auth/pom.xml (3)
18-18: Repository URL updated correctly.The PaperMC repository URL has been updated to the current official URL.
38-38:✅ Verification successful
Lombok version update looks good.
The lombok dependency has been updated from 1.18.22 to 1.18.36.
🌐 Web query:
What is the latest version of Project Lombok?💡 Result:
The latest version of Project Lombok is 1.18.36, released on November 15th, 2024[1][3][4]. This version adds JDK23 support and includes several bug fixes, such as resolving issues with Eclipse projects using
com.pro-crafting-tools:jasperreports-maven-plugin[1]. The release also introduced improved documentation generation for constructors and addressed edge cases with@SuperBuilderon types containing generic array parameters[1][6].Key details about Lombok 1.18.36:
- Maven Central dependency format:
<dependency> <groupId>org.projectlombok</groupId> <artifactId>lombok</artifactId> <version>1.18.36</version> </dependency>- MIT licensed with 27,832 recorded usages in Maven projects[2][6]
- Available for download directly from projectlombok.org/download[5]
This release follows version 1.18.34 from June 2024, which added Eclipse 2024-06 support and modified
@lombok.Generatedannotation behavior[3][5].Citations:
- 1: https://projectlombok.org/changelog
- 2: https://mvnrepository.com/artifact/org.projectlombok
- 3: https://projectlombok.org/all-versions
- 4: https://central.sonatype.com/artifact/org.projectlombok/lombok
- 5: https://projectlombok.org/download
- 6: https://mvnrepository.com/artifact/org.projectlombok/lombok
- 7: https://projectlombok.org
- 8: https://groups.google.com/g/project-lombok/c/mjqmzR7Laa4
- 9: https://github.com/projectlombok/lombok
Review Approval – Lombok Version Verified
The Lombok dependency update in
zander-auth/pom.xmlto version 1.18.36 is confirmed as it matches the latest release (published November 15th, 2024). No further changes are needed.
- Location:
zander-auth/pom.xml(line 38)- Updated snippet:
<version>1.18.36</version>
26-26:✅ Verification successful
Paper API version update looks good.
The paper-api dependency has been updated from 1.20.2-R0.1-SNAPSHOT to 1.21.4-R0.1-SNAPSHOT.
🌐 Web query:
What is the latest version of Paper API?💡 Result:
The latest version of the Paper API is 1.21.4-R0.1-SNAPSHOT[3][7], which aligns with Minecraft version 1.21.4. This information comes directly from PaperMC's official GitHub repository and its associated developer documentation[3][7].
Key details about Paper API:
- Compatibility: Maintains backward compatibility with Spigot and Bukkit plugins while introducing performance optimizations and additional features[1].
- Usage: Developers can integrate it using Maven or Gradle with the dependency
io.papermc.paper:paper-api:1.21.4-R0.1-SNAPSHOT[3].- Features: Includes enhanced APIs for events, schedulers, persistent data storage, and modern Minecraft mechanics like data components[2][7].
Paper API versions typically correspond to their associated Minecraft server versions[5], and the project actively maintains both current and archived versions for backward compatibility[3]. For production use, developers should always verify version compatibility with their target server implementation.
Citations:
- 1: https://www.g-portal.com/wiki/en/minecraft-paper/
- 2: https://docs.papermc.io/paper/dev/api
- 3: https://github.com/PaperMC/Paper
- 4: https://commonpaper.com/blog/introducing-the-common-paper-api/
- 5: https://gist.github.com/osipxd/6119732e30059241c2192c4a8d2218d9
- 6: https://www.spigotmc.org/threads/api-version.633462/
- 7: https://jd.papermc.io/paper/1.21
- 8: https://www.spigotmc.org/threads/using-newer-versions-of-spigot-paper-api-for-older-server-versions.520919/
Paper API version update verified and approved.
- The dependency version in
zander-auth/pom.xml(line 26) is correctly updated from1.20.2-R0.1-SNAPSHOTto1.21.4-R0.1-SNAPSHOT.- Web query confirms that
1.21.4-R0.1-SNAPSHOTis the latest release as per PaperMC's official sources.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserCommandSpyEvent.java (1)
1-13: Package and import updates look consistent with the project reorganization.The package declarations and imports have been appropriately updated from the 'proxy' to 'waterfall' namespace, which aligns with the broader restructuring of the codebase.
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserOnSwitch.java (1)
1-13: Updated Package and Import Statements are Correct
The package declaration and the updated import paths (switching fromproxytowaterfall) are correctly applied. This ensures that the class now references the appropriate modules (e.g.,ConfigurationManager,ZanderProxyMain,DiscordSwitch, andSessionSwitch) in the new package structure.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserSocialSpyEvent.java (1)
1-15: Package and Import Updates are in Order
The package declaration and update of import statements (including the new location forDiscordSocialSpy) correctly reflect the new module organization.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/events/UserOnDisconnect.java (1)
1-23: Package Updates and Import Adjustments are Correct
The package declaration and updated import statements (including forSessionDestroyand the newDiscordLeave) are correct. The change aligns with the overall reorganization fromzander-proxytozander-waterfall.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/ping.java (1)
1-1: Package rename from proxy to waterfall looks goodThe package has been updated from
org.modularsoft.zander.proxy.commandstoorg.modularsoft.zander.waterfall.commands, which aligns with the broader project reorganization mentioned in the summary.zander-waterfall/dependency-reduced-pom.xml (1)
9-9: ArtifactId update from zander-proxy to zander-waterfall looks correctThe artifact ID has been successfully updated from
zander-proxytozander-waterfall, which aligns with the project restructuring from the PR summary.zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/Filter.java (1)
1-6: LGTM: Package structure and imports look goodThe package structure and imports are well-organized and appropriately chosen.
zander-velocity/src/main/java/org/modularsoft/zander/velocity/util/ChatFreezeManager.java (1)
1-5: LGTM: Class setup and static field initialization look goodThe class structure and static field initialization are well-implemented.
zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/ping.java (1)
12-29: Command implementation looks goodThe ping command is well implemented with proper player instance checking, error handling for non-player sources, and good use of the Adventure API for text formatting.
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/discord.java (1)
1-5: Package restructuring looks goodThe package and import changes from
proxytowaterfallare consistent with the project restructuring mentioned in the PR summary.pom.xml (2)
13-15: Module restructuring looks goodThe changes to the modules section reflect the project restructuring mentioned in the PR summary:
- Commenting out the
zander-waterfallmodule with a note about decommissioning- Adding the new
zander-velocitymodule
27-54:❓ Verification inconclusive
Verify the impact of commenting out the plugins section
The entire plugins section has been commented out, including the maven-shade-plugin and maven-compiler-plugin. This could affect the build process for the project.
Please verify:
- Is this intentional?
- How will dependencies be packaged without the shade plugin?
- What Java version will be used for compilation without the compiler plugin configuration?
If the build is now managed by the child modules individually, this change may be valid, but it should be verified.
🏁 Script executed:
#!/bin/bash # Check if child POMs have their own plugin configurations echo "Checking for plugin configurations in child modules..." grep -r "<plugins>" --include="pom.xml" .Length of output: 284
Let's further verify the plugin usage in the child modules. To do that, please run the following shell script to search for any usage of the
maven-compiler-pluginandmaven-shade-pluginin allpom.xmlfiles:
🏁 Script executed:
#!/bin/bash echo "Searching for maven-compiler-plugin usage..." grep -R "maven-compiler-plugin" --include="pom.xml" . echo "Searching for maven-shade-plugin usage..." grep -R "maven-shade-plugin" --include="pom.xml" .Length of output: 714
Clarify Intentional Modular Plugin Configurations
It appears the parent POM’s plugin section has been deliberately commented out, shifting responsibility to the child modules. Notably:
- zander-velocity: Configures both the
maven-compiler-pluginand themaven-shade-plugin, ensuring that the proper Java version is used for compilation and dependencies are being packaged.- zander-waterfall: Only the
maven-compiler-pluginis configured. Please confirm whether the absence of themaven-shade-pluginhere is intentional—if this module does not require dependency shading or if it’s handled by another mechanism.Please verify that this modular approach is intentional and that any changes to dependency packaging or compilation settings are documented accordingly.
zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/moderation/freezechat.java (1)
23-25: LGTM - Good implementation of toggle functionalityThe chat freeze toggle is implemented correctly by checking the current state and inverting it.
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/rules.java (1)
1-5: Package path update looks goodThe change from "proxy" to "waterfall" in the package and import paths is consistent with the broader refactoring in this pull request.
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/commands/website.java (1)
1-5: Package path update looks goodThe change from "proxy" to "waterfall" in the package and import paths is consistent with the broader refactoring in this pull request.
zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/moderation/clearchat.java (1)
25-27: Good use of pattern matching with instanceofThe modern Java pattern matching syntax with instanceof is clean and readable.
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/ZanderProxyMain.java (1)
7-10: Package import changes look correct.
The shifted import statements appear to be in sync with the new package structure. Just validate that no references to the old proxy package remain elsewhere.zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/UserCommandSpyEvent.java (1)
16-29: Edge case check for empty or single-slash commands.
Whenevent.getMessage()is just"/"or missing additional parameters,substring(1)returns an empty string, which you continue to log and forward to the API if it’s not in the excluded list. Confirm if that’s the expected behavior or if you want to ignore empty commands.zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/UserSocialSpyEvent.java (1)
77-80: Inconsistent error handling compared toUserCommandSpyEvent.
Here, you let the user continue playing on errors; inUserCommandSpyEvent, the user is forcibly disconnected. Confirm you want these contrasting behaviors, or make them consistent for a better user experience.zander-hub/pom.xml (2)
17-22: Repository updates look good.The update to the PaperMC repository URL and the addition of the Sonatype repository will ensure access to the latest dependencies. These changes align with modern Maven configuration practices.
29-30: Consider the implications of changing scope from 'provided' to 'compile'.Changing the paper-api dependency scope from 'provided' to 'compile' means this dependency will be packaged with your final artifact rather than being expected at runtime. This could:
- Increase your artifact size
- Potentially cause conflicts if the dependency is already provided by the runtime environment
Confirm this change is intentional and necessary for your deployment environment.
zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/report.java (1)
73-98: The tab completion implementation looks good.The suggest method is well implemented, providing username completion based on online players.
zander-velocity/src/main/java/org/modularsoft/zander/velocity/ZanderVelocityMain.java (6)
37-46: Plugin annotation implementation looks goodThe plugin metadata is well-defined with clear ID, version, author information, and dependencies. The dependency on
signedvelocityis properly declared.
87-97: Field assignment in constructor looks goodThe constructor with dependency injection is properly implemented. Fields are assigned correctly.
99-110: Configuration loading implementation is solidGood use of YamlDocument to handle configuration with appropriate settings. The code includes auto-update capability and proper error handling.
110-114: Good error handling for configuration loadingThe error handling when configuration loading fails is appropriate - logging the error and shutting down the plugin gracefully.
57-68: Event registration looks well-organizedThe event listeners are registered systematically. Good separation of concerns with different listeners for different events.
72-78: Command registration implementation is cleanCommands are registered with appropriate meta builders. The organization makes it easy to see what commands are available.
| .setRequestBody(discordSwitch.toString()) // Ensure this method works properly | ||
| .build(); | ||
|
|
||
| Response discordSwitchRes = discordSwitchReq.execute(); | ||
| logger.info("Discord Switch Response ({}): {}", discordSwitchRes.getStatusCode(), discordSwitchRes.getBody()); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider checking Discord API response status code.
Similar to the session switch, verify that the Discord API response indicates success.
Response discordSwitchRes = discordSwitchReq.execute();
logger.info("Discord Switch Response ({}): {}", discordSwitchRes.getStatusCode(), discordSwitchRes.getBody());
+
+ if (discordSwitchRes.getStatusCode() < 200 || discordSwitchRes.getStatusCode() >= 300) {
+ logger.error("Discord Switch API returned error status: {}", discordSwitchRes.getStatusCode());
+ player.sendMessage(Component.text("Discord integration error, but you can continue playing.").color(NamedTextColor.RED));
+ }📝 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.
| .setRequestBody(discordSwitch.toString()) // Ensure this method works properly | |
| .build(); | |
| Response discordSwitchRes = discordSwitchReq.execute(); | |
| logger.info("Discord Switch Response ({}): {}", discordSwitchRes.getStatusCode(), discordSwitchRes.getBody()); | |
| .setRequestBody(discordSwitch.toString()) // Ensure this method works properly | |
| .build(); | |
| Response discordSwitchRes = discordSwitchReq.execute(); | |
| logger.info("Discord Switch Response ({}): {}", discordSwitchRes.getStatusCode(), discordSwitchRes.getBody()); | |
| if (discordSwitchRes.getStatusCode() < 200 || discordSwitchRes.getStatusCode() >= 300) { | |
| logger.error("Discord Switch API returned error status: {}", discordSwitchRes.getStatusCode()); | |
| player.sendMessage(Component.text("Discord integration error, but you can continue playing.").color(NamedTextColor.RED)); | |
| } |
| Response switchSessionRes = switchSessionReq.execute(); | ||
| logger.info("Session Switch Response ({}): {}", switchSessionRes.getStatusCode(), switchSessionRes.getBody()); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider checking the response status code before proceeding.
Currently, you log the status code but don't verify if it indicates success. Consider checking if the status code is in the 2xx range before proceeding.
Response switchSessionRes = switchSessionReq.execute();
logger.info("Session Switch Response ({}): {}", switchSessionRes.getStatusCode(), switchSessionRes.getBody());
+
+ if (switchSessionRes.getStatusCode() < 200 || switchSessionRes.getStatusCode() >= 300) {
+ logger.error("Session Switch API returned error status: {}", switchSessionRes.getStatusCode());
+ player.disconnect(Component.text("Server error. Please try again later.").color(NamedTextColor.RED));
+ return;
+ }📝 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.
| Response switchSessionRes = switchSessionReq.execute(); | |
| logger.info("Session Switch Response ({}): {}", switchSessionRes.getStatusCode(), switchSessionRes.getBody()); | |
| Response switchSessionRes = switchSessionReq.execute(); | |
| logger.info("Session Switch Response ({}): {}", switchSessionRes.getStatusCode(), switchSessionRes.getBody()); | |
| if (switchSessionRes.getStatusCode() < 200 || switchSessionRes.getStatusCode() >= 300) { | |
| logger.error("Session Switch API returned error status: {}", switchSessionRes.getStatusCode()); | |
| player.disconnect(Component.text("Server error. Please try again later.").color(NamedTextColor.RED)); | |
| return; | |
| } |
| @Builder | ||
| public class SessionCreate { | ||
|
|
||
| @Getter UUID uuid; | ||
| @Getter String ipAddress; | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return new Gson().toJson(this); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Note potential code duplication across modules
There's an identical SessionCreate class in the zander-velocity module. Consider if this could be moved to a common/shared module to avoid duplication, or if they need to remain separate implementations for specific reasons.
🏁 Script executed:
#!/bin/bash
# Check if there are common model classes between velocity and waterfall modules
echo "Checking for potential shared model classes between modules..."
echo "SessionCreate class structure in both modules:"
diff -y <(cat zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/session/SessionCreate.java) <(cat zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/session/SessionCreate.java) || echo "Differences found"
echo -e "\nSearching for other potentially duplicated model classes:"
find zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model -type f -name "*.java" | while read file; do
basename=$(basename "$file")
velocity_file="zander-velocity/src/main/java/org/modularsoft/zander/velocity/model/$(echo "$file" | grep -o "session/.*" || echo "$file" | grep -o "user/.*" || true)"
if [ -f "$velocity_file" ]; then
echo "Duplicate found: $basename"
fi
doneLength of output: 7328
Action Required: Review and Refactor Duplicate Session Classes
The SessionCreate class exists in both the zander-waterfall and zander-velocity modules, as confirmed by the duplicate file check. In addition, similar duplications (e.g., SessionDestroy and SessionSwitch) have been detected. Please evaluate if these shared session models can be consolidated into a common/shared module to reduce maintenance overhead and ensure consistency, or if they must remain independent due to domain-specific requirements.
- Affected Files:
zander-waterfall/src/main/java/org/modularsoft/zander/waterfall/model/session/SessionCreate.javazander-velocity/src/main/java/org/modularsoft/zander/velocity/model/session/SessionCreate.java- (Additional duplicates noted for
SessionDestroy.java,SessionSwitch.java, andUserCreation.java)
| try { | ||
| SessionDestroy destroySession = SessionDestroy.builder() | ||
| .uuid(player.getUniqueId()) | ||
| .build(); | ||
|
|
||
| Request destroySessionReq = Request.builder() | ||
| .setURL(BaseAPIURL + "/session/destroy") | ||
| .setMethod(Request.Method.POST) | ||
| .addHeader("x-access-token", APIKey) | ||
| .setRequestBody(destroySession.toString()) | ||
| .build(); | ||
|
|
||
| Response destroySessionRes = destroySessionReq.execute(); | ||
| ZanderVelocityMain.getLogger().info("Response (" + destroySessionRes.getStatusCode() + "): " + destroySessionRes.getBody()); | ||
| } catch (Exception e) { | ||
| Component builder = Component.text("An error has occurred. Is the API down?").color(NamedTextColor.RED); | ||
| player.disconnect(builder); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve exception handling for session destroy request.
The current error handling disconnects a player who is already in the process of disconnecting, which may not be the best approach. Additionally, the exception details aren't being logged, which makes debugging harder.
try {
SessionDestroy destroySession = SessionDestroy.builder()
.uuid(player.getUniqueId())
.build();
Request destroySessionReq = Request.builder()
.setURL(BaseAPIURL + "/session/destroy")
.setMethod(Request.Method.POST)
.addHeader("x-access-token", APIKey)
.setRequestBody(destroySession.toString())
.build();
Response destroySessionRes = destroySessionReq.execute();
ZanderVelocityMain.getLogger().info("Response (" + destroySessionRes.getStatusCode() + "): " + destroySessionRes.getBody());
} catch (Exception e) {
- Component builder = Component.text("An error has occurred. Is the API down?").color(NamedTextColor.RED);
- player.disconnect(builder);
+ ZanderVelocityMain.getLogger().severe("Failed to destroy session for " + player.getUsername() + ": " + e.getMessage());
+ e.printStackTrace();
}📝 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.
| try { | |
| SessionDestroy destroySession = SessionDestroy.builder() | |
| .uuid(player.getUniqueId()) | |
| .build(); | |
| Request destroySessionReq = Request.builder() | |
| .setURL(BaseAPIURL + "/session/destroy") | |
| .setMethod(Request.Method.POST) | |
| .addHeader("x-access-token", APIKey) | |
| .setRequestBody(destroySession.toString()) | |
| .build(); | |
| Response destroySessionRes = destroySessionReq.execute(); | |
| ZanderVelocityMain.getLogger().info("Response (" + destroySessionRes.getStatusCode() + "): " + destroySessionRes.getBody()); | |
| } catch (Exception e) { | |
| Component builder = Component.text("An error has occurred. Is the API down?").color(NamedTextColor.RED); | |
| player.disconnect(builder); | |
| } | |
| try { | |
| SessionDestroy destroySession = SessionDestroy.builder() | |
| .uuid(player.getUniqueId()) | |
| .build(); | |
| Request destroySessionReq = Request.builder() | |
| .setURL(BaseAPIURL + "/session/destroy") | |
| .setMethod(Request.Method.POST) | |
| .addHeader("x-access-token", APIKey) | |
| .setRequestBody(destroySession.toString()) | |
| .build(); | |
| Response destroySessionRes = destroySessionReq.execute(); | |
| ZanderVelocityMain.getLogger().info("Response (" + destroySessionRes.getStatusCode() + "): " + destroySessionRes.getBody()); | |
| } catch (Exception e) { | |
| ZanderVelocityMain.getLogger().severe("Failed to destroy session for " + player.getUsername() + ": " + e.getMessage()); | |
| e.printStackTrace(); | |
| } |
| // | ||
| // Send Discord API POST for disconnect message | ||
| // | ||
| try { | ||
| DiscordLeave leave = DiscordLeave.builder() | ||
| .username(player.getUsername()) | ||
| .build(); | ||
|
|
||
| Request discordLeaveReq = Request.builder() | ||
| .setURL(BaseAPIURL + "/discord/leave") | ||
| .setMethod(Request.Method.POST) | ||
| .addHeader("x-access-token", APIKey) | ||
| .setRequestBody(leave.toString()) | ||
| .build(); | ||
|
|
||
| Response discordLeaveRes = discordLeaveReq.execute(); | ||
| ZanderVelocityMain.getLogger().info("Response (" + discordLeaveRes.getStatusCode() + "): " + discordLeaveRes.getBody()); | ||
| } catch (Exception e) { | ||
| Component builder = Component.text("An error has occurred. Is the API down?").color(NamedTextColor.RED); | ||
| player.disconnect(builder); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve exception handling for Discord notification request.
Similar to the previous issue, error handling disconnects a player who is already disconnecting. Also, there's duplicated error handling code that could be refactored.
try {
DiscordLeave leave = DiscordLeave.builder()
.username(player.getUsername())
.build();
Request discordLeaveReq = Request.builder()
.setURL(BaseAPIURL + "/discord/leave")
.setMethod(Request.Method.POST)
.addHeader("x-access-token", APIKey)
.setRequestBody(leave.toString())
.build();
Response discordLeaveRes = discordLeaveReq.execute();
ZanderVelocityMain.getLogger().info("Response (" + discordLeaveRes.getStatusCode() + "): " + discordLeaveRes.getBody());
} catch (Exception e) {
- Component builder = Component.text("An error has occurred. Is the API down?").color(NamedTextColor.RED);
- player.disconnect(builder);
+ ZanderVelocityMain.getLogger().severe("Failed to send Discord leave notification for " + player.getUsername() + ": " + e.getMessage());
+ e.printStackTrace();
}📝 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.
| // | |
| // Send Discord API POST for disconnect message | |
| // | |
| try { | |
| DiscordLeave leave = DiscordLeave.builder() | |
| .username(player.getUsername()) | |
| .build(); | |
| Request discordLeaveReq = Request.builder() | |
| .setURL(BaseAPIURL + "/discord/leave") | |
| .setMethod(Request.Method.POST) | |
| .addHeader("x-access-token", APIKey) | |
| .setRequestBody(leave.toString()) | |
| .build(); | |
| Response discordLeaveRes = discordLeaveReq.execute(); | |
| ZanderVelocityMain.getLogger().info("Response (" + discordLeaveRes.getStatusCode() + "): " + discordLeaveRes.getBody()); | |
| } catch (Exception e) { | |
| Component builder = Component.text("An error has occurred. Is the API down?").color(NamedTextColor.RED); | |
| player.disconnect(builder); | |
| } | |
| // | |
| // Send Discord API POST for disconnect message | |
| // | |
| try { | |
| DiscordLeave leave = DiscordLeave.builder() | |
| .username(player.getUsername()) | |
| .build(); | |
| Request discordLeaveReq = Request.builder() | |
| .setURL(BaseAPIURL + "/discord/leave") | |
| .setMethod(Request.Method.POST) | |
| .addHeader("x-access-token", APIKey) | |
| .setRequestBody(leave.toString()) | |
| .build(); | |
| Response discordLeaveRes = discordLeaveReq.execute(); | |
| ZanderVelocityMain.getLogger().info("Response (" + discordLeaveRes.getStatusCode() + "): " + discordLeaveRes.getBody()); | |
| } catch (Exception e) { | |
| ZanderVelocityMain.getLogger().severe("Failed to send Discord leave notification for " + player.getUsername() + ": " + e.getMessage()); | |
| e.printStackTrace(); | |
| } |
| if (!heartbeat) { | ||
| // Kick all players | ||
| ZanderVelocityMain.getProxy().getAllPlayers().forEach(player -> { | ||
| Component message = Component.text("API Heartbeat Failed, the server is temporarily offline.") | ||
| .color(NamedTextColor.RED); | ||
| player.disconnect(message); | ||
| }); | ||
| } | ||
| } catch (Exception e) { | ||
| // Handle exceptions here | ||
| e.printStackTrace(); | ||
| ZanderVelocityMain.getLogger().error("API Heartbeat Failed, kicking all players until back online."); | ||
|
|
||
| // Kick all players | ||
| ZanderVelocityMain.getProxy().getAllPlayers().forEach(player -> { | ||
| Component message = Component.text("API Heartbeat Failed, the server is temporarily offline.") | ||
| .color(NamedTextColor.RED); | ||
| player.disconnect(message); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract duplicate player disconnection logic.
The code for disconnecting players is duplicated in both the normal flow and exception handling. Extract this to a helper method.
// Check if the heartbeat is not successful
if (!heartbeat) {
- // Kick all players
- ZanderVelocityMain.getProxy().getAllPlayers().forEach(player -> {
- Component message = Component.text("API Heartbeat Failed, the server is temporarily offline.")
- .color(NamedTextColor.RED);
- player.disconnect(message);
- });
+ disconnectAllPlayers();
}
} catch (Exception e) {
// Handle exceptions here
e.printStackTrace();
ZanderVelocityMain.getLogger().error("API Heartbeat Failed, kicking all players until back online.");
-// Kick all players
-ZanderVelocityMain.getProxy().getAllPlayers().forEach(player -> {
- Component message = Component.text("API Heartbeat Failed, the server is temporarily offline.")
- .color(NamedTextColor.RED);
- player.disconnect(message);
-});
+disconnectAllPlayers();
}
}, 0, 60, TimeUnit.SECONDS);
}
+/**
+ * Disconnects all players with a server offline message
+ */
+private static void disconnectAllPlayers() {
+ Component message = Component.text("API Heartbeat Failed, the server is temporarily offline.")
+ .color(NamedTextColor.RED);
+ ZanderVelocityMain.getProxy().getAllPlayers().forEach(player -> {
+ player.disconnect(message);
+ });
}📝 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.
| if (!heartbeat) { | |
| // Kick all players | |
| ZanderVelocityMain.getProxy().getAllPlayers().forEach(player -> { | |
| Component message = Component.text("API Heartbeat Failed, the server is temporarily offline.") | |
| .color(NamedTextColor.RED); | |
| player.disconnect(message); | |
| }); | |
| } | |
| } catch (Exception e) { | |
| // Handle exceptions here | |
| e.printStackTrace(); | |
| ZanderVelocityMain.getLogger().error("API Heartbeat Failed, kicking all players until back online."); | |
| // Kick all players | |
| ZanderVelocityMain.getProxy().getAllPlayers().forEach(player -> { | |
| Component message = Component.text("API Heartbeat Failed, the server is temporarily offline.") | |
| .color(NamedTextColor.RED); | |
| player.disconnect(message); | |
| }); | |
| } | |
| if (!heartbeat) { | |
| disconnectAllPlayers(); | |
| } | |
| } catch (Exception e) { | |
| // Handle exceptions here | |
| e.printStackTrace(); | |
| ZanderVelocityMain.getLogger().error("API Heartbeat Failed, kicking all players until back online."); | |
| disconnectAllPlayers(); | |
| } | |
| }, 0, 60, TimeUnit.SECONDS); | |
| } | |
| /** | |
| * Disconnects all players with a server offline message | |
| */ | |
| private static void disconnectAllPlayers() { | |
| Component message = Component.text("API Heartbeat Failed, the server is temporarily offline.") | |
| .color(NamedTextColor.RED); | |
| ZanderVelocityMain.getProxy().getAllPlayers().forEach(player -> { | |
| player.disconnect(message); | |
| }); | |
| } |
|
|
||
| import static org.modularsoft.zander.velocity.ZanderVelocityMain.proxy; | ||
|
|
||
| public class report implements SimpleCommand { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Class naming should follow Java conventions.
The class name 'report' does not follow Java naming conventions, which recommend PascalCase for class names.
-public class report implements SimpleCommand {
+public class Report implements SimpleCommand {📝 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.
| public class report implements SimpleCommand { | |
| public class Report implements SimpleCommand { |
| try { | ||
| // | ||
| // Start Session API POST | ||
| // | ||
| SessionCreate createSession = SessionCreate.builder() | ||
| .uuid(player.getUniqueId()) | ||
| .ipAddress(player.getRemoteAddress().getAddress().toString()) | ||
| .build(); | ||
|
|
||
| Request createSessionReq = Request.builder() | ||
| .setURL(BaseAPIURL + "/session/create") | ||
| .setMethod(Request.Method.POST) | ||
| .addHeader("x-access-token", APIKey) | ||
| .setRequestBody(createSession.toString()) | ||
| .build(); | ||
|
|
||
| Response createSessionRes = createSessionReq.execute(); | ||
| ZanderVelocityMain.getLogger().info("Response (" + createSessionRes.getStatusCode() + "): " + createSessionRes.getBody()); | ||
|
|
||
| // Send Discord API POST for join message | ||
| DiscordJoin join = DiscordJoin.builder() | ||
| .username(player.getUsername()) | ||
| .build(); | ||
|
|
||
| Request discordJoinReq = Request.builder() | ||
| .setURL(BaseAPIURL + "/discord/join") | ||
| .setMethod(Request.Method.POST) | ||
| .addHeader("x-access-token", APIKey) | ||
| .setRequestBody(join.toString()) | ||
| .build(); | ||
|
|
||
| Response discordJoinRes = discordJoinReq.execute(); | ||
| ZanderVelocityMain.getLogger().info("Response (" + discordJoinRes.getStatusCode() + "): " + discordJoinRes.getBody()); | ||
| } catch (Exception e) { | ||
| Component builder = Component.text("An error has occurred. Is the API down?").color(NamedTextColor.RED); | ||
| player.disconnect(builder); | ||
| } | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Restructure nested try-catch blocks for better error handling.
The nested try-catch structure makes error handling confusing and potentially difficult to maintain. Consider refactoring to use separate methods for each API call.
The current nested try-catch blocks should be refactored into separate methods for each API call (createUser, createSession, sendDiscordJoin). This would improve code readability and maintainability while still preserving the error handling logic.
| // Create a ScheduledExecutorService with a single thread | ||
| ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add a way to shut down the scheduler.
The class creates a ScheduledExecutorService but doesn't provide a way to shut it down cleanly, which might lead to resource leaks when the plugin is disabled.
Add a shutdown method and store the scheduler as a class field:
public class TipChatter {
// Initialize the logger
private static final Logger logger = LoggerFactory.getLogger(TipChatter.class);
+
+ // Store the scheduler as a class field
+ private static ScheduledExecutorService scheduler;
public static void startAnnouncementTipTask() {
String BaseAPIURL = ZanderVelocityMain.getConfig().getString(Route.from("BaseAPIURL"));
String APIKey = ZanderVelocityMain.getConfig().getString(Route.from("APIKey"));
String announcementTipPrefix = ZanderVelocityMain.getConfig().getString(Route.from("announcementTipPrefix"));
// Create a ScheduledExecutorService with a single thread
- ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1);
+ scheduler = Executors.newScheduledThreadPool(1);And add a shutdown method:
public static void stopAnnouncementTipTask() {
if (scheduler != null && !scheduler.isShutdown()) {
logger.info("Shutting down announcement tip scheduler");
scheduler.shutdown();
try {
if (!scheduler.awaitTermination(5, TimeUnit.SECONDS)) {
scheduler.shutdownNow();
}
} catch (InterruptedException e) {
scheduler.shutdownNow();
Thread.currentThread().interrupt();
}
}
}| // Start the Heartbeat task | ||
| Heartbeat.startHeartbeatTask(); | ||
|
|
||
| // Start the Announcement Tip task | ||
| TipChatter.startAnnouncementTipTask(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add plugin shutdown handling for background tasks
The Heartbeat and TipChatter tasks are started but there's no mechanism to stop them when the plugin is disabled. This could lead to resource leaks.
Consider implementing a shutdown method annotated with @Subscribe for the ProxyShutdownEvent:
@Subscribe
public void onProxyShutdown(ProxyShutdownEvent event) {
// Stop the Heartbeat task
Heartbeat.stopHeartbeatTask();
// Stop the Announcement Tip task
TipChatter.stopAnnouncementTipTask();
logger.info("Zander Proxy has been shut down.");
}You'll also need to modify the Heartbeat and TipChatter classes to expose methods to properly shut down their schedulers.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/report.java (3)
31-34: Add more descriptive error message.The error message could be more helpful by explaining what the parameters are for.
- player.sendMessage(Component.text("Usage: /report <username> <reason>").color(NamedTextColor.RED)); + player.sendMessage(Component.text("Usage: /report <username> <reason> - Report a player for breaking rules").color(NamedTextColor.RED));
44-49: Add validation for report reason length.There's no validation for the report reason length, which could allow for excessively long reports or empty reasons.
StringBuilder reasonBuilder = new StringBuilder(); for (int i = 1; i < args.length; i++) { reasonBuilder.append(args[i]).append(" "); } String reason = reasonBuilder.toString().trim(); + + if (reason.isEmpty()) { + player.sendMessage(Component.text("You must provide a reason for the report.").color(NamedTextColor.RED)); + return; + } + + if (reason.length() > 200) { + player.sendMessage(Component.text("Report reason is too long. Please keep it under 200 characters.").color(NamedTextColor.RED)); + return; + }
21-86: Refactor execute method for improved maintainability.The execute method is quite long. Consider refactoring it into smaller, focused methods to improve readability and maintainability.
@Override public void execute(Invocation invocation) { CommandSource source = invocation.source(); String[] args = invocation.arguments(); String BaseAPIURL = ZanderVelocityMain.getConfig().getString(Route.from("BaseAPIURL")); String APIKey = ZanderVelocityMain.getConfig().getString(Route.from("APIKey")); if (source instanceof Player) { Player player = (Player) source; - - if (args.length < 2) { - player.sendMessage(Component.text("Usage: /report <username> <reason>").color(NamedTextColor.RED)); - return; - } - - String reportedUsername = args[0]; - - // Check if the player is trying to report themselves - if (reportedUsername.equalsIgnoreCase(player.getUsername())) { - player.sendMessage(Component.text("You cannot report yourself.").color(NamedTextColor.RED)); - return; - } - - StringBuilder reasonBuilder = new StringBuilder(); - for (int i = 1; i < args.length; i++) { - reasonBuilder.append(args[i]).append(" "); - } - String reason = reasonBuilder.toString().trim(); - - try { - // Build the report object - Report report = Report.builder() - .reportPlatform("INGAME") - .reportedUser(reportedUsername) - .reporterUser(player.getUsername()) - .reportReason(reason) - .build(); - - // Create and send the request - Request reportReq = Request.builder() - .setURL(BaseAPIURL + "/report/create") - .setMethod(Request.Method.POST) - .addHeader("x-access-token", APIKey) - .setRequestBody(report.toString()) - .build(); - - // Get the response from the API - Response reportRes = reportReq.execute(); - String json = reportRes.getBody(); - Boolean success = JsonPath.read(json, "$.success"); - String message = JsonPath.read(json, "$.message"); - - if (success) { - player.sendMessage(Component.text(message).color(NamedTextColor.GREEN)); - } else { - player.sendMessage(Component.text(message).color(NamedTextColor.RED)); - } - - } catch (Exception e) { - player.sendMessage(Component.text("An error has occurred. Is the API down?").color(NamedTextColor.RED)); - System.out.println(e); - } + handlePlayerReport(player, args, BaseAPIURL, APIKey); } else { source.sendMessage(Component.text("Only players can use this command.").color(NamedTextColor.RED)); } } + private void handlePlayerReport(Player player, String[] args, String baseAPIURL, String apiKey) { + if (args.length < 2) { + player.sendMessage(Component.text("Usage: /report <username> <reason> - Report a player for breaking rules").color(NamedTextColor.RED)); + return; + } + + String reportedUsername = args[0]; + if (!validateReportedPlayer(player, reportedUsername)) { + return; + } + + String reason = buildReasonString(args); + if (!validateReportReason(player, reason)) { + return; + } + + submitReport(player, reportedUsername, reason, baseAPIURL, apiKey); + } + + private boolean validateReportedPlayer(Player reporter, String reportedUsername) { + // Check if the player is trying to report themselves + if (reportedUsername.equalsIgnoreCase(reporter.getUsername())) { + reporter.sendMessage(Component.text("You cannot report yourself.").color(NamedTextColor.RED)); + return false; + } + + // Check if the reported player exists on the server + boolean playerExists = proxy.getAllPlayers().stream() + .anyMatch(p -> p.getUsername().equalsIgnoreCase(reportedUsername)); + if (!playerExists) { + reporter.sendMessage(Component.text("Player " + reportedUsername + " is not online.").color(NamedTextColor.RED)); + return false; + } + + return true; + } + + private String buildReasonString(String[] args) { + StringBuilder reasonBuilder = new StringBuilder(); + for (int i = 1; i < args.length; i++) { + reasonBuilder.append(args[i]).append(" "); + } + return reasonBuilder.toString().trim(); + } + + private boolean validateReportReason(Player player, String reason) { + if (reason.isEmpty()) { + player.sendMessage(Component.text("You must provide a reason for the report.").color(NamedTextColor.RED)); + return false; + } + + if (reason.length() > 200) { + player.sendMessage(Component.text("Report reason is too long. Please keep it under 200 characters.").color(NamedTextColor.RED)); + return false; + } + + return true; + } + + private void submitReport(Player player, String reportedUsername, String reason, String baseAPIURL, String apiKey) { + try { + // Build the report object + Report report = Report.builder() + .reportPlatform("INGAME") + .reportedUser(reportedUsername) + .reporterUser(player.getUsername()) + .reportReason(reason) + .build(); + + // Create and send the request + Request reportReq = Request.builder() + .setURL(baseAPIURL + "/report/create") + .setMethod(Request.Method.POST) + .addHeader("x-access-token", apiKey) + .setRequestBody(report.toString()) + .build(); + + // Get the response from the API + Response reportRes = reportReq.execute(); + String json = reportRes.getBody(); + Boolean success = JsonPath.read(json, "$.success"); + String message = JsonPath.read(json, "$.message"); + + if (success) { + player.sendMessage(Component.text(message).color(NamedTextColor.GREEN)); + } else { + player.sendMessage(Component.text(message).color(NamedTextColor.RED)); + } + + } catch (Exception e) { + player.sendMessage(Component.text("An error has occurred. Is the API down?").color(NamedTextColor.RED)); + ZanderVelocityMain.getLogger().error("Error occurred when processing report command", e); + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/report.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (1)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/report.java (1)
20-20: Class naming should follow Java conventions.The class name 'report' does not follow Java naming conventions, which recommend PascalCase for class names.
-public class report implements SimpleCommand { +public class Report implements SimpleCommand {
| // Check if the player is trying to report themselves | ||
| if (reportedUsername.equalsIgnoreCase(player.getUsername())) { | ||
| player.sendMessage(Component.text("You cannot report yourself.").color(NamedTextColor.RED)); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for reported player existence.
The code checks if a player is reporting themselves but doesn't verify if the reported player exists on the server. This could lead to reports being submitted for non-existent players.
// Check if the player is trying to report themselves
if (reportedUsername.equalsIgnoreCase(player.getUsername())) {
player.sendMessage(Component.text("You cannot report yourself.").color(NamedTextColor.RED));
return;
}
+
+ // Check if the reported player exists on the server
+ boolean playerExists = proxy.getAllPlayers().stream()
+ .anyMatch(p -> p.getUsername().equalsIgnoreCase(reportedUsername));
+ if (!playerExists) {
+ player.sendMessage(Component.text("Player " + reportedUsername + " is not online.").color(NamedTextColor.RED));
+ return;
+ }📝 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.
| // Check if the player is trying to report themselves | |
| if (reportedUsername.equalsIgnoreCase(player.getUsername())) { | |
| player.sendMessage(Component.text("You cannot report yourself.").color(NamedTextColor.RED)); | |
| return; | |
| } | |
| // Check if the player is trying to report themselves | |
| if (reportedUsername.equalsIgnoreCase(player.getUsername())) { | |
| player.sendMessage(Component.text("You cannot report yourself.").color(NamedTextColor.RED)); | |
| return; | |
| } | |
| // Check if the reported player exists on the server | |
| boolean playerExists = proxy.getAllPlayers().stream() | |
| .anyMatch(p -> p.getUsername().equalsIgnoreCase(reportedUsername)); | |
| if (!playerExists) { | |
| player.sendMessage(Component.text("Player " + reportedUsername + " is not online.").color(NamedTextColor.RED)); | |
| return; | |
| } |
|
|
||
| } catch (Exception e) { | ||
| player.sendMessage(Component.text("An error has occurred. Is the API down?").color(NamedTextColor.RED)); | ||
| System.out.println(e); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace System.out.println with proper logging.
Using System.out.println for error logging is not ideal in production environments. Consider using a proper logging framework.
- System.out.println(e);
+ ZanderVelocityMain.getLogger().error("Error occurred when processing report command", 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.
| System.out.println(e); | |
| ZanderVelocityMain.getLogger().error("Error occurred when processing report command", e); |
| .addHeader("x-access-token", APIKey) | ||
| .setRequestBody(report.toString()) | ||
| .build(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Content-Type header is missing in the API request.
When sending JSON in the request body, you should include a Content-Type header to specify the format of the data.
.setURL(BaseAPIURL + "/report/create")
.setMethod(Request.Method.POST)
.addHeader("x-access-token", APIKey)
+ .addHeader("Content-Type", "application/json")
.setRequestBody(report.toString())
.build();📝 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.
| .addHeader("x-access-token", APIKey) | |
| .setRequestBody(report.toString()) | |
| .build(); | |
| .setURL(BaseAPIURL + "/report/create") | |
| .setMethod(Request.Method.POST) | |
| .addHeader("x-access-token", APIKey) | |
| .addHeader("Content-Type", "application/json") | |
| .setRequestBody(report.toString()) | |
| .build(); |
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/ZanderVelocityMain.java (1)
80-84:⚠️ Potential issueAdd plugin shutdown handling for background tasks
The Heartbeat and TipChatter tasks are started but there's no mechanism to stop them when the plugin is disabled. This could lead to resource leaks.
Implement a shutdown method annotated with
@Subscribefor theProxyShutdownEvent:@Subscribe public void onProxyShutdown(ProxyShutdownEvent event) { // Stop the Heartbeat task Heartbeat.stopHeartbeatTask(); // Stop the Announcement Tip task TipChatter.stopAnnouncementTipTask(); logger.info("Zander Velocity has been shut down."); }You'll also need to modify the Heartbeat and TipChatter classes to expose methods to properly shut down their schedulers.
🧹 Nitpick comments (16)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/ZanderVelocityMain.java (6)
47-53: Consider using instance variables rather than static variablesThe class uses static variables for Logger, ProxyServer, and config which can lead to tight coupling and make testing more difficult. While this pattern is common in Minecraft plugins, it's generally better to use instance variables when possible.
- @Getter - private static Logger logger; - @Getter - public static ProxyServer proxy; - @Getter - private static YamlDocument config; + @Getter + private final Logger logger; + @Getter + private final ProxyServer proxy; + @Getter + private static YamlDocument config; // This could remain static if needed for global accessYou would then need to update other classes to access these through an instance rather than statically.
94-96: Initialize instance variables correctlyWhen using static variables with instance assignment, you should use
ZanderVelocityMain.proxyrather thanthis.proxyto make it clear you're assigning to static fields.- this.proxy = proxy; - this.logger = logger; + ZanderVelocityMain.proxy = proxy; + ZanderVelocityMain.logger = logger; this.commandManager = commandManager;
99-109: Improve configuration loading with validationThe configuration loading is missing validation of critical settings that would be needed for the plugin to function correctly. Consider adding validation for important configuration values like API keys and endpoints.
Add validation after loading the configuration:
try { config = YamlDocument.create(new File(dataDirectory.toFile(), "config.yml"), Objects.requireNonNull(getClass().getResourceAsStream("/config.yml")), GeneralSettings.DEFAULT, LoaderSettings.builder().setAutoUpdate(true).build(), DumperSettings.DEFAULT, UpdaterSettings.builder().setVersioning(new BasicVersioning("config-version")) .setOptionSorting(UpdaterSettings.OptionSorting.SORT_BY_DEFAULTS).build()); config.update(); config.save(); + + // Validate critical configuration values + if (config.getString("APIKey", "").isEmpty()) { + logger.warn("APIKey is not set in the configuration. API-dependent features will not work correctly."); + } + if (config.getString("BaseAPIURL", "").isEmpty()) { + logger.warn("BaseAPIURL is not set in the configuration. API-dependent features will not work correctly."); + }
110-114: Improve exception handling for configuration loadingThe current exception handling catches IOException but doesn't provide detailed information about what went wrong. Consider logging the exception details to help with troubleshooting.
} catch (IOException e) { - logger.error("Could not create or load plugin configuration, plugin will now be disabled."); + logger.error("Could not create or load plugin configuration, plugin will now be disabled.", e); Optional<PluginContainer> container = proxy.getPluginManager().getPlugin("zander-velocity"); container.ifPresent(pluginContainer -> pluginContainer.getExecutorService().shutdown()); }
69-78: Consider adding command aliases and tab completionsThe current command registration is straightforward but doesn't take advantage of Velocity's command suggestion system. Add command aliases and tab completions to improve user experience.
For example, you could enhance the "discord" command registration:
- commandManager.register(commandManager.metaBuilder("discord").build(), new discord()); + commandManager.register( + commandManager.metaBuilder("discord") + .aliases("dc") + .plugin(this) + .build(), + new discord() + );Apply similar improvements to other commands as appropriate.
64-64: Passing 'this' reference unnecessarilyYou're passing the main class instance to UserOnProxyPing when registering the event listener, but not to other event listeners. This seems inconsistent and may not be necessary.
Either ensure UserOnProxyPing actually needs this reference, or remove it:
- proxy.getEventManager().register(this, new UserOnProxyPing(this)); + proxy.getEventManager().register(this, new UserOnProxyPing());zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/report.java (3)
44-49: Consider adding validation for the report reason length.The report reason is built by concatenating all arguments after the username, but there's no validation for minimum or maximum length.
StringBuilder reasonBuilder = new StringBuilder(); for (int i = 1; i < args.length; i++) { reasonBuilder.append(args[i]).append(" "); } String reason = reasonBuilder.toString().trim(); + + // Validate reason length + if (reason.length() < 3) { + player.sendMessage(Component.text("Report reason must be at least 3 characters long.").color(NamedTextColor.RED)); + return; + } + if (reason.length() > 200) { + player.sendMessage(Component.text("Report reason must be less than 200 characters.").color(NamedTextColor.RED)); + return; + }
76-82: Consider adding rate limiting for report submissions.There's no mechanism to prevent spam reports. Consider implementing rate limiting to prevent abuse of the report system.
You could store the last report time for each player in a Map and check it before processing a new report:
// Add this as a class field private final Map<UUID, Long> lastReportTime = new HashMap<>(); private static final long REPORT_COOLDOWN_MS = 60000; // 1 minute cooldown // Add this check before processing the report UUID playerUUID = player.getUniqueId(); long currentTime = System.currentTimeMillis(); if (lastReportTime.containsKey(playerUUID)) { long lastTime = lastReportTime.get(playerUUID); if (currentTime - lastTime < REPORT_COOLDOWN_MS) { long secondsLeft = (REPORT_COOLDOWN_MS - (currentTime - lastTime)) / 1000; player.sendMessage(Component.text("Please wait " + secondsLeft + " seconds before submitting another report.").color(NamedTextColor.RED)); return; } } lastReportTime.put(playerUUID, currentTime);
106-110: Consider adding a case-insensitive check for username autocompletion.The current implementation checks if the username starts with the given argument, but it might be more user-friendly to use a case-insensitive check and also match usernames that contain the given string, not just at the beginning.
// Autocomplete the username (first argument) for (Player player : proxy.getAllPlayers()) { - if (player.getUsername().toLowerCase().startsWith(args[0].toLowerCase())) { + if (player.getUsername().toLowerCase().contains(args[0].toLowerCase())) { completions.add(player.getUsername()); } }zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/UserSocialSpyEvent.java (5)
36-37: Improve command detection logic.The current implementation checks if the command "contains" various keywords, which could lead to false positives. For example, a custom command like "msgboard" would trigger this handler. Consider using a more precise check.
- if (command.contains("msg") || command.contains("tell") || command.contains("w") - || command.contains("message") || command.contains("r")) { + if (command.startsWith("msg ") || command.startsWith("tell ") || command.startsWith("w ") + || command.startsWith("message ") || command.startsWith("r ")) {
68-70: Follow consistent naming conventions.Variable names
BaseAPIURLandAPIKeyuse PascalCase while Java conventions typically use camelCase for variables. Consider renaming for consistency.- String BaseAPIURL = ZanderVelocityMain.getConfig().getString(Route.from("BaseAPIURL")); - String APIKey = ZanderVelocityMain.getConfig().getString(Route.from("APIKey")); + String baseApiUrl = ZanderVelocityMain.getConfig().getString(Route.from("BaseAPIURL")); + String apiKey = ZanderVelocityMain.getConfig().getString(Route.from("APIKey"));
79-80: Implement more robust response handling.The code logs the response status code and body but doesn't handle non-success status codes. Consider checking the status code and handling error responses appropriately.
Response socialSpyRes = socialSpyReq.execute(); - logger.info("Social Spy Response ({}): {}", socialSpyRes.getStatusCode(), socialSpyRes.getBody()); + // Check if response indicates success + if (socialSpyRes.getStatusCode() >= 200 && socialSpyRes.getStatusCode() < 300) { + logger.debug("Social Spy message sent successfully: {}", socialSpyRes.getStatusCode()); + } else { + logger.warn("Social Spy API returned error ({}): {}", socialSpyRes.getStatusCode(), socialSpyRes.getBody()); + }
82-85: Improve error handling specificity.The current implementation catches all exceptions generically, which makes debugging difficult. Consider catching more specific exceptions and providing more detailed error handling.
- } catch (Exception e) { - logger.error("Error occurred while handling social spy request", e); - player.sendMessage(Component.text("An error occurred, but you can continue playing.").color(NamedTextColor.RED)); + } catch (IOException e) { + logger.error("Network error while sending social spy request", e); + player.sendMessage(Component.text("Unable to connect to the messaging service. Your message was still sent to the player.").color(NamedTextColor.RED)); + } catch (Exception e) { + logger.error("Unexpected error while handling social spy request", e); + player.sendMessage(Component.text("An error occurred with the messaging system, but your message was still delivered.").color(NamedTextColor.RED));
19-88: Consider privacy implications of monitoring private messages.This class monitors and forwards private messages between players to an external system. Make sure this functionality is:
- Documented in your server rules/privacy policy
- Compliant with relevant privacy laws
- Only accessible to authorized staff members
Additionally, consider adding a notification to players that their messages may be monitored.
zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/UserCommandSpyEvent.java (2)
16-34: Command filtering approach needs refinement.The command filtering approach for direct messages is fragile. Using string prefix checks could miss variations or commands from plugins. Consider using a more robust approach such as a configuration-based exclusion list or a regex pattern.
- // Check if the command is one we need to log (ignore direct message commands) - if (command.startsWith("msg") || command.startsWith("tell") || command.startsWith("w") - || command.startsWith("message") || command.startsWith("r")) { - return; - } + // Check if the command is one we need to log (ignore direct message commands) + List<String> ignoredCommandPrefixes = ZanderVelocityMain.getConfig() + .getStringList(Route.from("IgnoredCommandPrefixes"), Arrays.asList("msg", "tell", "w", "message", "r")); + + for (String prefix : ignoredCommandPrefixes) { + if (command.startsWith(prefix)) { + return; + } + }
36-38: Improve code documentation.The current comment is minimal. Adding more comprehensive documentation would help future developers understand this class's purpose and functionality.
-// -// Command Spy API POST -// +/** + * Send command information to the Discord command spy API + * This allows staff to monitor player commands from Discord for moderation purposes + * The API endpoint sends this data to a Discord webhook configured in the backend + */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/ZanderVelocityMain.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/report.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/UserCommandSpyEvent.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/UserOnProxyPing.java(1 hunks)zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/UserSocialSpyEvent.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/UserOnProxyPing.java
🧰 Additional context used
🧬 Code Definitions (1)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/ZanderVelocityMain.java (5)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/moderation/clearchat.java (1)
clearchat(12-51)zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/moderation/freezechat.java (1)
freezechat(10-43)zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/moderation/FreezeChatListener.java (1)
FreezeChatListener(11-23)zander-velocity/src/main/java/org/modularsoft/zander/velocity/util/announcement/TipChatter.java (1)
TipChatter(18-71)zander-velocity/src/main/java/org/modularsoft/zander/velocity/util/api/Heartbeat.java (1)
Heartbeat(15-62)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (5)
zander-velocity/src/main/java/org/modularsoft/zander/velocity/commands/report.java (4)
20-20: Class naming should follow Java conventions.The class name 'report' does not follow Java naming conventions, which recommend PascalCase for class names.
-public class report implements SimpleCommand { +public class Report implements SimpleCommand {
38-42: Add validation for reported player existence.The code checks if a player is reporting themselves but doesn't verify if the reported player exists on the server. This could lead to reports being submitted for non-existent players.
// Check if the player is trying to report themselves if (reportedUsername.equalsIgnoreCase(player.getUsername())) { player.sendMessage(Component.text("You cannot report yourself.").color(NamedTextColor.RED)); return; } + + // Check if the reported player exists on the server + boolean playerExists = proxy.getAllPlayers().stream() + .anyMatch(p -> p.getUsername().equalsIgnoreCase(reportedUsername)); + if (!playerExists) { + player.sendMessage(Component.text("Player " + reportedUsername + " is not online.").color(NamedTextColor.RED)); + return; + }
63-65: Content-Type header is missing in the API request.When sending JSON in the request body, you should include a Content-Type header to specify the format of the data.
.setURL(BaseAPIURL + "/report/create") .setMethod(Request.Method.POST) .addHeader("x-access-token", APIKey) + .addHeader("Content-Type", "application/json") .setRequestBody(report.toString()) .build();
90-90: Replace System.out.println with proper logging.Using System.out.println for error logging is not ideal in production environments. Consider using a proper logging framework.
- System.out.println(e); + ZanderVelocityMain.getLogger().error("Error occurred when processing report command", e);zander-velocity/src/main/java/org/modularsoft/zander/velocity/events/UserSocialSpyEvent.java (1)
76-77:✅ Verification successful
Ensure proper JSON serialization.
The comment mentions "Ensure proper serialization to JSON" but calls
toString()on thesocialSpyobject. This might not produce valid JSON unlesstoString()is overridden properly in theDiscordSocialSpyclass.Please verify that the
DiscordSocialSpy.toString()method returns properly formatted JSON. Consider using a dedicated JSON library like Jackson or Gson for reliable serialization:
🏁 Script executed:
#!/bin/bash # Check if DiscordSocialSpy has a proper toString() method or JSON serialization rg -A 15 "class DiscordSocialSpy"Length of output: 3222
JSON Serialization Verified:
- The
DiscordSocialSpy.toString()method correctly returns JSON usingnew Gson().toJson(this).- The usage of
toString()in.setRequestBody(socialSpy.toString())is appropriate and produces valid JSON.
| Response reportRes = reportReq.execute(); | ||
| String json = reportRes.getBody(); | ||
| Boolean success = JsonPath.read(json, "$.success"); | ||
| String message = JsonPath.read(json, "$.message"); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for API responses.
The code assumes that the API response will always contain the expected JSON structure. Add error handling to gracefully handle cases where the response is malformed or missing expected fields.
// Get the response from the API
Response reportRes = reportReq.execute();
String json = reportRes.getBody();
- Boolean success = JsonPath.read(json, "$.success");
- String message = JsonPath.read(json, "$.message");
+ Boolean success = false;
+ String message = "Unknown error occurred";
+
+ try {
+ success = JsonPath.read(json, "$.success");
+ message = JsonPath.read(json, "$.message");
+ } catch (Exception e) {
+ ZanderVelocityMain.getLogger().error("Error parsing API response", e);
+ player.sendMessage(Component.text("Error processing server response. Please try again.").color(NamedTextColor.RED));
+ return;
+ }📝 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.
| Response reportRes = reportReq.execute(); | |
| String json = reportRes.getBody(); | |
| Boolean success = JsonPath.read(json, "$.success"); | |
| String message = JsonPath.read(json, "$.message"); | |
| // Get the response from the API | |
| Response reportRes = reportReq.execute(); | |
| String json = reportRes.getBody(); | |
| Boolean success = false; | |
| String message = "Unknown error occurred"; | |
| try { | |
| success = JsonPath.read(json, "$.success"); | |
| message = JsonPath.read(json, "$.message"); | |
| } catch (Exception e) { | |
| ZanderVelocityMain.getLogger().error("Error parsing API response", e); | |
| player.sendMessage(Component.text("Error processing server response. Please try again.").color(NamedTextColor.RED)); | |
| return; | |
| } |
| Player player = (Player) event.getCommandSource(); | ||
| String command = event.getCommand(); | ||
|
|
||
| logger.info("Command: {}", command); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove or change log level for debug information.
This appears to be a debug statement that logs every command executed. In a production environment, this would generate excessive log entries and potentially expose sensitive information. Consider removing this line or changing it to debug level.
- logger.info("Command: {}", command);
+ logger.debug("Command: {}", command);📝 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.
| logger.info("Command: {}", command); | |
| logger.debug("Command: {}", command); |
| String BaseAPIURL = ZanderVelocityMain.getConfig().getString(Route.from("BaseAPIURL")); | ||
| String APIKey = ZanderVelocityMain.getConfig().getString(Route.from("APIKey")); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate configuration values before use.
The code doesn't check if the API URL or key are null, empty, or valid before using them. This could lead to NullPointerExceptions or malformed requests.
// Fetch API URL and key
String BaseAPIURL = ZanderVelocityMain.getConfig().getString(Route.from("BaseAPIURL"));
String APIKey = ZanderVelocityMain.getConfig().getString(Route.from("APIKey"));
+ // Validate configuration values
+ if (BaseAPIURL == null || BaseAPIURL.isEmpty()) {
+ logger.error("BaseAPIURL is not configured properly");
+ return;
+ }
+
+ if (APIKey == null || APIKey.isEmpty()) {
+ logger.error("APIKey is not configured properly");
+ return;
+ }📝 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.
| String BaseAPIURL = ZanderVelocityMain.getConfig().getString(Route.from("BaseAPIURL")); | |
| String APIKey = ZanderVelocityMain.getConfig().getString(Route.from("APIKey")); | |
| String BaseAPIURL = ZanderVelocityMain.getConfig().getString(Route.from("BaseAPIURL")); | |
| String APIKey = ZanderVelocityMain.getConfig().getString(Route.from("APIKey")); | |
| // Validate configuration values | |
| if (BaseAPIURL == null || BaseAPIURL.isEmpty()) { | |
| logger.error("BaseAPIURL is not configured properly"); | |
| return; | |
| } | |
| if (APIKey == null || APIKey.isEmpty()) { | |
| logger.error("APIKey is not configured properly"); | |
| return; | |
| } |
| if (command.contains("msg") || command.contains("tell") || command.contains("w") | ||
| || command.contains("message") || command.contains("r")) { | ||
|
|
||
| // Split the command into parts and check for minimum required arguments | ||
| String[] commandParts = command.split(" "); | ||
| if (commandParts.length < 3) { | ||
| logger.warn("Invalid direct message format from player: {}", player.getUsername()); | ||
| return; // Invalid command format, return without processing | ||
| } |
There was a problem hiding this comment.
Add special handling for the "r" (reply) command.
The "r" (reply) command typically doesn't require specifying a target player as it replies to the last person who messaged you. The current implementation will incorrectly parse this command format.
if (command.contains("msg") || command.contains("tell") || command.contains("w")
|| command.contains("message") || command.contains("r")) {
// Split the command into parts and check for minimum required arguments
String[] commandParts = command.split(" ");
+ // Special handling for reply command which has a different format
+ if (command.startsWith("r ")) {
+ if (commandParts.length < 2) {
+ logger.warn("Invalid reply format from player: {}", player.getUsername());
+ return;
+ }
+ // Here you would need to retrieve the last person who messaged this player
+ // This requires maintaining a conversation state somewhere
+ String directMessage = String.join(" ", Arrays.copyOfRange(commandParts, 1, commandParts.length));
+ // Rest of your code with the retrieved target player
+ } else {
if (commandParts.length < 3) {
logger.warn("Invalid direct message format from player: {}", player.getUsername());
return; // Invalid command format, return without processing
}
+ }📝 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.
| if (command.contains("msg") || command.contains("tell") || command.contains("w") | |
| || command.contains("message") || command.contains("r")) { | |
| // Split the command into parts and check for minimum required arguments | |
| String[] commandParts = command.split(" "); | |
| if (commandParts.length < 3) { | |
| logger.warn("Invalid direct message format from player: {}", player.getUsername()); | |
| return; // Invalid command format, return without processing | |
| } | |
| if (command.contains("msg") || command.contains("tell") || command.contains("w") | |
| || command.contains("message") || command.contains("r")) { | |
| // Split the command into parts and check for minimum required arguments | |
| String[] commandParts = command.split(" "); | |
| // Special handling for reply command which has a different format | |
| if (command.startsWith("r ")) { | |
| if (commandParts.length < 2) { | |
| logger.warn("Invalid reply format from player: {}", player.getUsername()); | |
| return; | |
| } | |
| // Here you would need to retrieve the last person who messaged this player | |
| // This requires maintaining a conversation state somewhere | |
| String directMessage = String.join(" ", Arrays.copyOfRange(commandParts, 1, commandParts.length)); | |
| // Rest of your code with the retrieved target player | |
| } else { | |
| if (commandParts.length < 3) { | |
| logger.warn("Invalid direct message format from player: {}", player.getUsername()); | |
| return; // Invalid command format, return without processing | |
| } | |
| } |
| String BaseAPIURL = ZanderVelocityMain.getConfig().getString(Route.from("BaseAPIURL")); | ||
| String APIKey = ZanderVelocityMain.getConfig().getString(Route.from("APIKey")); | ||
| String command = event.getCommand(); // Get the full command |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for configuration values.
The code retrieves configuration values but doesn't validate if they're present or valid. Missing values could cause runtime errors.
String BaseAPIURL = ZanderVelocityMain.getConfig().getString(Route.from("BaseAPIURL"));
String APIKey = ZanderVelocityMain.getConfig().getString(Route.from("APIKey"));
+
+ // Validate required configuration
+ if (BaseAPIURL == null || BaseAPIURL.isEmpty()) {
+ ZanderVelocityMain.getLogger().error("BaseAPIURL is not configured. Command spy feature disabled.");
+ return;
+ }
+
+ if (APIKey == null || APIKey.isEmpty()) {
+ ZanderVelocityMain.getLogger().error("APIKey is not configured. Command spy feature disabled.");
+ return;
+ }📝 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.
| String BaseAPIURL = ZanderVelocityMain.getConfig().getString(Route.from("BaseAPIURL")); | |
| String APIKey = ZanderVelocityMain.getConfig().getString(Route.from("APIKey")); | |
| String command = event.getCommand(); // Get the full command | |
| String BaseAPIURL = ZanderVelocityMain.getConfig().getString(Route.from("BaseAPIURL")); | |
| String APIKey = ZanderVelocityMain.getConfig().getString(Route.from("APIKey")); | |
| // Validate required configuration | |
| if (BaseAPIURL == null || BaseAPIURL.isEmpty()) { | |
| ZanderVelocityMain.getLogger().error("BaseAPIURL is not configured. Command spy feature disabled."); | |
| return; | |
| } | |
| if (APIKey == null || APIKey.isEmpty()) { | |
| ZanderVelocityMain.getLogger().error("APIKey is not configured. Command spy feature disabled."); | |
| return; | |
| } | |
| String command = event.getCommand(); // Get the full command |
| } catch (Exception e) { | ||
| Component builder = Component.text("An error has occurred. Is the API down?").color(NamedTextColor.RED); | ||
| player.disconnect(builder); | ||
| System.out.println(e); | ||
| } |
There was a problem hiding this comment.
Critical: Improve error handling and don't disconnect players.
Disconnecting players when the API fails is extremely disruptive to user experience. Instead, log the error and continue execution.
} catch (Exception e) {
- Component builder = Component.text("An error has occurred. Is the API down?").color(NamedTextColor.RED);
- player.disconnect(builder);
- System.out.println(e);
+ ZanderVelocityMain.getLogger().error("Failed to process command spy for {}: {}",
+ player.getUsername(), e.getMessage(), e);
+ // Optionally notify admins about API issues but don't disconnect the player
+ ZanderVelocityMain.getServer().getAllPlayers().stream()
+ .filter(p -> p.hasPermission("zander.admin"))
+ .forEach(p -> p.sendMessage(
+ Component.text("Command spy API error: " + e.getMessage())
+ .color(NamedTextColor.RED)
+ ));
}📝 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.
| } catch (Exception e) { | |
| Component builder = Component.text("An error has occurred. Is the API down?").color(NamedTextColor.RED); | |
| player.disconnect(builder); | |
| System.out.println(e); | |
| } | |
| } catch (Exception e) { | |
| ZanderVelocityMain.getLogger().error("Failed to process command spy for {}: {}", | |
| player.getUsername(), e.getMessage(), e); | |
| // Optionally notify admins about API issues but don't disconnect the player | |
| ZanderVelocityMain.getServer().getAllPlayers().stream() | |
| .filter(p -> p.hasPermission("zander.admin")) | |
| .forEach(p -> p.sendMessage( | |
| Component.text("Command spy API error: " + e.getMessage()) | |
| .color(NamedTextColor.RED) | |
| )); | |
| } |
| try { | ||
| DiscordCommandSpy commandSpy = DiscordCommandSpy.builder() | ||
| .username(player.getUsername()) | ||
| .command(command) | ||
| .server(player.getCurrentServer().get().getServer().getServerInfo().getName()) | ||
| .build(); | ||
|
|
||
| Request commandSpyReq = Request.builder() | ||
| .setURL(BaseAPIURL + "/discord/spy/command") | ||
| .setMethod(Request.Method.POST) | ||
| .addHeader("x-access-token", String.valueOf(APIKey)) | ||
| .setRequestBody(commandSpy.toString()) | ||
| .build(); | ||
|
|
||
| Response commandSpyRes = commandSpyReq.execute(); | ||
| ZanderVelocityMain.getLogger().info("Response (" + commandSpyRes.getStatusCode() + "): " + commandSpyRes.getBody()); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add proper exception handling and make API calls asynchronous.
The current implementation makes synchronous API calls in an event handler, which could block the main thread. Also, the code doesn't handle specific exceptions or include timeout handling.
try {
DiscordCommandSpy commandSpy = DiscordCommandSpy.builder()
.username(player.getUsername())
.command(command)
.server(player.getCurrentServer().get().getServer().getServerInfo().getName())
.build();
- Request commandSpyReq = Request.builder()
- .setURL(BaseAPIURL + "/discord/spy/command")
- .setMethod(Request.Method.POST)
- .addHeader("x-access-token", String.valueOf(APIKey))
- .setRequestBody(commandSpy.toString())
- .build();
-
- Response commandSpyRes = commandSpyReq.execute();
- ZanderVelocityMain.getLogger().info("Response (" + commandSpyRes.getStatusCode() + "): " + commandSpyRes.getBody());
+ // Execute API call asynchronously to not block the main thread
+ ZanderVelocityMain.getServer().getScheduler().buildTask(ZanderVelocityMain.getInstance(), () -> {
+ try {
+ Request commandSpyReq = Request.builder()
+ .setURL(BaseAPIURL + "/discord/spy/command")
+ .setMethod(Request.Method.POST)
+ .addHeader("x-access-token", String.valueOf(APIKey))
+ .setRequestBody(commandSpy.toString())
+ .setTimeout(5000) // 5 second timeout
+ .build();
+
+ Response commandSpyRes = commandSpyReq.execute();
+ ZanderVelocityMain.getLogger().info("Command spy response: Status " +
+ commandSpyRes.getStatusCode() + " - " + commandSpyRes.getBody());
+ } catch (Exception ex) {
+ ZanderVelocityMain.getLogger().error("Failed to send command spy data: ", ex);
+ }
+ }).schedule();📝 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.
| try { | |
| DiscordCommandSpy commandSpy = DiscordCommandSpy.builder() | |
| .username(player.getUsername()) | |
| .command(command) | |
| .server(player.getCurrentServer().get().getServer().getServerInfo().getName()) | |
| .build(); | |
| Request commandSpyReq = Request.builder() | |
| .setURL(BaseAPIURL + "/discord/spy/command") | |
| .setMethod(Request.Method.POST) | |
| .addHeader("x-access-token", String.valueOf(APIKey)) | |
| .setRequestBody(commandSpy.toString()) | |
| .build(); | |
| Response commandSpyRes = commandSpyReq.execute(); | |
| ZanderVelocityMain.getLogger().info("Response (" + commandSpyRes.getStatusCode() + "): " + commandSpyRes.getBody()); | |
| try { | |
| DiscordCommandSpy commandSpy = DiscordCommandSpy.builder() | |
| .username(player.getUsername()) | |
| .command(command) | |
| .server(player.getCurrentServer().get().getServer().getServerInfo().getName()) | |
| .build(); | |
| // Execute API call asynchronously to not block the main thread | |
| ZanderVelocityMain.getServer().getScheduler().buildTask(ZanderVelocityMain.getInstance(), () -> { | |
| try { | |
| Request commandSpyReq = Request.builder() | |
| .setURL(BaseAPIURL + "/discord/spy/command") | |
| .setMethod(Request.Method.POST) | |
| .addHeader("x-access-token", String.valueOf(APIKey)) | |
| .setRequestBody(commandSpy.toString()) | |
| .setTimeout(5000) // 5 second timeout | |
| .build(); | |
| Response commandSpyRes = commandSpyReq.execute(); | |
| ZanderVelocityMain.getLogger().info("Command spy response: Status " + | |
| commandSpyRes.getStatusCode() + " - " + commandSpyRes.getBody()); | |
| } catch (Exception ex) { | |
| ZanderVelocityMain.getLogger().error("Failed to send command spy data: ", ex); | |
| } | |
| }).schedule(); |
| DiscordCommandSpy commandSpy = DiscordCommandSpy.builder() | ||
| .username(player.getUsername()) | ||
| .command(command) | ||
| .server(player.getCurrentServer().get().getServer().getServerInfo().getName()) | ||
| .build(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle potential NullPointerException in player server information retrieval.
The code assumes that player.getCurrentServer().get() will always be available, but this might not be the case if the player is in the process of switching servers.
try {
+ // Check if player is on a server
+ if (!player.getCurrentServer().isPresent()) {
+ ZanderVelocityMain.getLogger().debug("Player {} not connected to a server, skipping command spy", player.getUsername());
+ return;
+ }
+
DiscordCommandSpy commandSpy = DiscordCommandSpy.builder()
.username(player.getUsername())
.command(command)
.server(player.getCurrentServer().get().getServer().getServerInfo().getName())
.build();📝 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.
| DiscordCommandSpy commandSpy = DiscordCommandSpy.builder() | |
| .username(player.getUsername()) | |
| .command(command) | |
| .server(player.getCurrentServer().get().getServer().getServerInfo().getName()) | |
| .build(); | |
| try { | |
| // Check if player is on a server | |
| if (!player.getCurrentServer().isPresent()) { | |
| ZanderVelocityMain.getLogger().debug("Player {} not connected to a server, skipping command spy", player.getUsername()); | |
| return; | |
| } | |
| DiscordCommandSpy commandSpy = DiscordCommandSpy.builder() | |
| .username(player.getUsername()) | |
| .command(command) | |
| .server(player.getCurrentServer().get().getServer().getServerInfo().getName()) | |
| .build(); | |
| } |
Summary by CodeRabbit