Remove Ponder dependency and implement equivalent classes locally#296
Remove Ponder dependency and implement equivalent classes locally#296dmccoystephenson merged 7 commits intomainfrom
Conversation
- Created local equivalent classes under dansplugins.wildpets.ponder package: - abs/Savable, abs/Lockable, abs/AbstractPluginCommand - services/CommandService - tools/EventHandlerRegistry, UUIDChecker, PermissionChecker - misc/ArgumentParser - Updated WildPets to extend JavaPlugin directly instead of PonderBukkitPlugin - Changed CommandService constructor to accept JavaPlugin directly - Updated all import statements across 22 files - Removed Ponder dependency and JitPack repository from pom.xml Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
|
@copilot move ponder equivalent classes to appropriate packages that do not reference ponder |
- AbstractPluginCommand, CommandService, ArgumentParser, PermissionChecker → commands/ - Savable, Lockable → data/ - EventHandlerRegistry → listeners/ - UUIDChecker → helpers/ - Removed ponder/ directory entirely - Cleaned up redundant same-package imports Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Done in abcc21a. Moved all classes to existing packages:
The |
|
@copilot add CI check to verify compilation |
Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Added in 78b1973. Created Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
This PR removes the external Ponder (JitPack) dependency by inlining equivalent utility classes into the WildPets codebase and updating the plugin to extend JavaPlugin directly.
Changes:
- Replaced Ponder imports/usages with local equivalents under
dansplugins.wildpets.commands,data,listeners, andhelpers. - Updated
WildPetsto extendJavaPluginand wired in the new localCommandService/ event registration utilities. - Removed JitPack + Ponder dependency from
pom.xmland added a GitHub Actions CI workflow.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/dansplugins/wildpets/pet/record/PetRecord.java | Switches Savable import to local interface. |
| src/main/java/dansplugins/wildpets/pet/Pet.java | Switches Lockable/Savable imports to local interfaces. |
| src/main/java/dansplugins/wildpets/listeners/InteractionHandler.java | Updates UUIDChecker import to local helper. |
| src/main/java/dansplugins/wildpets/listeners/EventHandlerRegistry.java | Adds local listener registration utility. |
| src/main/java/dansplugins/wildpets/info/InfoSender.java | Updates UUIDChecker import to local helper. |
| src/main/java/dansplugins/wildpets/helpers/UUIDChecker.java | Adds local UUID/name lookup utility. |
| src/main/java/dansplugins/wildpets/data/Savable.java | Adds local Savable serialization interface. |
| src/main/java/dansplugins/wildpets/data/Lockable.java | Adds local Lockable access-control interface. |
| src/main/java/dansplugins/wildpets/commands/WanderCommand.java | Removes Ponder command base import (uses local base). |
| src/main/java/dansplugins/wildpets/commands/UnlockCommand.java | Removes Ponder command base import (uses local base). |
| src/main/java/dansplugins/wildpets/commands/TameCommand.java | Removes Ponder command base import (uses local base). |
| src/main/java/dansplugins/wildpets/commands/StayCommand.java | Removes Ponder command base import (uses local base). |
| src/main/java/dansplugins/wildpets/commands/StatsCommand.java | Removes Ponder command base import (uses local base). |
| src/main/java/dansplugins/wildpets/commands/SetFreeCommand.java | Removes Ponder command base import (uses local base). |
| src/main/java/dansplugins/wildpets/commands/SelectCommand.java | Removes Ponder command base import (uses local base). |
| src/main/java/dansplugins/wildpets/commands/RenameCommand.java | Removes Ponder command base import (uses local base). |
| src/main/java/dansplugins/wildpets/commands/PermissionChecker.java | Adds local permission-check helper used by command service. |
| src/main/java/dansplugins/wildpets/commands/LockCommand.java | Removes Ponder command base import (uses local base). |
| src/main/java/dansplugins/wildpets/commands/LocateCommand.java | Removes Ponder command base import (uses local base). |
| src/main/java/dansplugins/wildpets/commands/ListCommand.java | Removes Ponder command base import (uses local base). |
| src/main/java/dansplugins/wildpets/commands/InfoCommand.java | Removes Ponder command base import (uses local base). |
| src/main/java/dansplugins/wildpets/commands/HelpCommand.java | Removes Ponder command base import (uses local base). |
| src/main/java/dansplugins/wildpets/commands/GatherCommand.java | Removes Ponder command base import (uses local base). |
| src/main/java/dansplugins/wildpets/commands/FollowCommand.java | Removes Ponder command base import (uses local base). |
| src/main/java/dansplugins/wildpets/commands/DefaultCommand.java | Removes Ponder command base import (uses local base). |
| src/main/java/dansplugins/wildpets/commands/ConfigCommand.java | Removes Ponder command base import (uses local base). |
| src/main/java/dansplugins/wildpets/commands/CommandService.java | Adds local command dispatch/permission enforcement service. |
| src/main/java/dansplugins/wildpets/commands/CheckAccessCommand.java | Removes Ponder command base import (uses local base). |
| src/main/java/dansplugins/wildpets/commands/CallCommand.java | Removes Ponder command base import (uses local base). |
| src/main/java/dansplugins/wildpets/commands/ArgumentParser.java | Adds local argument parsing utility for commands. |
| src/main/java/dansplugins/wildpets/commands/AbstractPluginCommand.java | Adds local command base class replacing Ponder’s base. |
| src/main/java/dansplugins/wildpets/WildPets.java | Moves from PonderBukkitPlugin to JavaPlugin; wires local services. |
| pom.xml | Removes JitPack repo and Ponder dependency. |
| .github/workflows/ci.yml | Adds CI to compile and run tests on PRs/pushes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| private void informSenderTheyDoNotHaveOneOfTheRequiredPermissions(CommandSender sender, ArrayList<String> permissions) { | ||
| sender.sendMessage(ChatColor.RED + "In order to use this command, you need one of the following permissions: '" + permissions + "'"); |
There was a problem hiding this comment.
The "need one of the following permissions" message currently interpolates the raw ArrayList ('" + permissions + "'), which prints with brackets and commas and includes extra quoting. Formatting the permissions as a readable, comma-separated list (and removing the extra quotes) would make this error message clearer for users.
| sender.sendMessage(ChatColor.RED + "In order to use this command, you need one of the following permissions: '" + permissions + "'"); | |
| String joinedPermissions = String.join(", ", permissions); | |
| sender.sendMessage(ChatColor.RED + "In order to use this command, you need one of the following permissions: " + joinedPermissions); |
| public String findPlayerNameBasedOnUUID(UUID playerUUID) { | ||
| if (playerUUID == null) { | ||
| throw new IllegalArgumentException("Player UUID cannot be null!"); | ||
| } | ||
| final Player player = Bukkit.getPlayer(playerUUID); | ||
| if (player != null) { | ||
| return player.getName(); | ||
| } | ||
| final OfflinePlayer offlinePlayer = Bukkit.getOfflinePlayer(playerUUID); | ||
| final String name = offlinePlayer.getName(); | ||
| return name == null || !offlinePlayer.hasPlayedBefore() ? "" : name; | ||
| } |
There was a problem hiding this comment.
findPlayerNameBasedOnUUID returns an empty string when the name can't be resolved, but the method contract doesn't mention this and call sites often check for null rather than empty. Consider returning null for "unknown" (and documenting it), or update the Javadoc to explicitly state that "" is returned when no name is available.
| public void registerEventHandlers(ArrayList<Listener> listeners, Plugin plugin) { | ||
| if (listeners == null || listeners.isEmpty()) { | ||
| throw new IllegalArgumentException("Listeners cannot be null or empty!"); | ||
| } | ||
| if (plugin == null) { | ||
| throw new IllegalArgumentException("Plugin cannot be null!"); | ||
| } | ||
| listeners.forEach(listener -> manager.registerEvents(listener, plugin)); | ||
| } |
There was a problem hiding this comment.
registerEventHandlers takes an ArrayList<Listener> instead of the List interface, which unnecessarily constrains callers and makes the API harder to use. Accepting List<Listener> (or Collection<Listener>) would be more flexible without changing behavior.
| public interface Lockable<T> { | ||
| void setOwner(T toSet); | ||
| T getOwner(); | ||
| void addToAccessList(T toAdd); | ||
| void removeFromAccessList(T toRemove); | ||
| boolean hasAccess(T toCheck); | ||
| ArrayList<T> getAccessList(); | ||
| } |
There was a problem hiding this comment.
Lockable#getAccessList returns an ArrayList<T>, which exposes the concrete collection type and restricts implementations/callers. Returning the List<T> interface would reduce coupling while keeping the same semantics.
| */ | ||
| public boolean sendMessageIfNoArguments(String message, String[] args, CommandSender sender, ChatColor color) { | ||
| if (args.length == 0) { | ||
| sender.sendMessage(ChatColor.RED + message); |
There was a problem hiding this comment.
sendMessageIfNoArguments ignores its color parameter and always prefixes messages with ChatColor.RED, so callers cannot control the color as the signature suggests. Use the provided color when sending the message (or remove the parameter if color should be fixed).
| sender.sendMessage(ChatColor.RED + message); | |
| sender.sendMessage(color + message); |
| if (command.getNames().contains(subCommand)) { | ||
| if (!permissionChecker.checkPermission(sender, command.getPermissions())) { | ||
| return false; | ||
| } | ||
| if (arguments.length == 0) { | ||
| return command.execute(sender); | ||
| } | ||
| else { | ||
| return command.execute(sender, arguments); | ||
| } |
There was a problem hiding this comment.
Subcommand dispatch is currently case-sensitive (command.getNames().contains(subCommand)), so /wp Help (or any mixed-case subcommand) will not match even though Bukkit arguments preserve user casing. Consider normalizing subCommand and command names to a consistent case (e.g., lower-case) or using case-insensitive comparison when matching.
| if (command.getNames().contains(subCommand)) { | |
| if (!permissionChecker.checkPermission(sender, command.getPermissions())) { | |
| return false; | |
| } | |
| if (arguments.length == 0) { | |
| return command.execute(sender); | |
| } | |
| else { | |
| return command.execute(sender, arguments); | |
| } | |
| for (String name : command.getNames()) { | |
| if (name.equalsIgnoreCase(subCommand)) { | |
| if (!permissionChecker.checkPermission(sender, command.getPermissions())) { | |
| return false; | |
| } | |
| if (arguments.length == 0) { | |
| return command.execute(sender); | |
| } | |
| else { | |
| return command.execute(sender, arguments); | |
| } | |
| } |
|
@copilot address comments |
Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
All 6 review comments addressed in 6d657cf:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Merge pull request #296 from Dans-Plugins/copilot/remove-ponder-dependency Remove Ponder dependency and implement equivalent classes locally
.github/workflows/ci.yml) to verify compilation and run tests🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.