Add player trade command (/wp trade <playerName>)#298
Conversation
Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
… consistency Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
/wp trade <playerName>)
There was a problem hiding this comment.
Pull request overview
Adds a new /wp trade <playerName> subcommand to transfer ownership of the caller’s currently selected pet to another online player, updating both in-memory pet lists and persisted pet records.
Changes:
- Introduces
TradeCommandto validate and perform pet ownership transfer, notify both players, and clear the sender’s selection. - Adds
PetListRepository.transferPet()to move a pet between owners’ pet lists while preserving entity properties. - Wires the command into plugin initialization, help output, and permissions.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/resources/plugin.yml | Adds wp.trade permission node. |
| src/main/java/dansplugins/wildpets/pet/list/PetListRepository.java | Adds transferPet() for moving pets between owners. |
| src/main/java/dansplugins/wildpets/commands/TradeCommand.java | New /wp trade command implementation. |
| src/main/java/dansplugins/wildpets/commands/HelpCommand.java | Adds trade command line to help output. |
| src/main/java/dansplugins/wildpets/WildPets.java | Registers TradeCommand in command initialization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| PetList newList = getPetList(newOwnerUUID); | ||
| if (newList == null) { | ||
| createPetListForPlayer(newOwnerUUID); | ||
| newList = getPetList(newOwnerUUID); | ||
| } | ||
|
|
||
| pet.setOwnerUUID(newOwnerUUID); | ||
| pet.removeFromAccessList(oldList.getOwnerUUID()); | ||
| pet.addToAccessList(newOwnerUUID); | ||
| newList.addPet(pet); | ||
| return true; |
There was a problem hiding this comment.
transferPet allows bypassing the configured petLimit (currently only enforced during taming). A player can trade pets to someone who is already at the limit, exceeding the cap. Add a limit check against configService.getInt("petLimit") before adding to the recipient list (and return false with a meaningful failure path).
| String targetName = args[0]; | ||
| Player targetPlayer = Bukkit.getPlayer(targetName); | ||
|
|
||
| if (targetPlayer == null) { | ||
| player.sendMessage(ChatColor.RED + "That player is not online."); | ||
| return false; |
There was a problem hiding this comment.
Bukkit.getPlayer(String) performs partial name matching and can return the wrong player if multiple online names share a prefix. For a trade/ownership transfer, use Bukkit.getPlayerExact(targetName) (or otherwise disambiguate) to avoid transferring a pet to an unintended player.
| Pet pet = ephemeralData.getPetSelectionForPlayer(player.getUniqueId()); | ||
| if (pet == null) { | ||
| player.sendMessage(ChatColor.RED + "No pet selected. Use /wp select first."); | ||
| return false; | ||
| } | ||
|
|
||
| if (args.length < 1) { | ||
| return execute(sender); | ||
| } | ||
|
|
There was a problem hiding this comment.
The missing-argument check happens after the selected-pet check, so /wp trade with no args can report "No pet selected" instead of showing usage. For consistency with commands like RenameCommand, validate args.length first (show usage), then validate selection/transfer prerequisites.
| Pet pet = ephemeralData.getPetSelectionForPlayer(player.getUniqueId()); | |
| if (pet == null) { | |
| player.sendMessage(ChatColor.RED + "No pet selected. Use /wp select first."); | |
| return false; | |
| } | |
| if (args.length < 1) { | |
| return execute(sender); | |
| } | |
| if (args.length < 1) { | |
| return execute(sender); | |
| } | |
| Pet pet = ephemeralData.getPetSelectionForPlayer(player.getUniqueId()); | |
| if (pet == null) { | |
| player.sendMessage(ChatColor.RED + "No pet selected. Use /wp select first."); | |
| return false; | |
| } |
|
@copilot address comments |
… check Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
All three review comments addressed in 86b1d14:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public boolean transferPet(Pet pet, UUID newOwnerUUID) { | ||
| PetList oldList = getPetList(pet.getOwnerUUID()); | ||
| if (oldList == null || !oldList.getPets().remove(pet)) { |
There was a problem hiding this comment.
Pet exposes getOwnerUUID(), but this new code calls pet.getOwnerUUID() (missing UUID). This won't compile and will prevent the plugin from building. Update the call to use the correct method name.
TradeCommand.java- new command to transfer selected pet to another online player (/wp trade <playerName>)transferPetmethod toPetListRepositoryfor clean pet ownership transferTradeCommandinWildPets.javacommand initializationwp.tradepermission toplugin.ymlHelpCommand.javahelp textargs.lengthbefore pet selection for correct error messagesBukkit.getPlayerExact()instead ofBukkit.getPlayer()to avoid partial name matchingpetLimitcheck intransferPet()to prevent exceeding the configured pet cap (with rollback on failure)Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.