Conversation
jackah2
left a comment
jackah2
left a comment
There was a problem hiding this comment.
Looks pretty good, just want a few things added. Thanks for working on this!
| super(plugin, baseCommand, subCommand); | ||
| super.description("Toggles Citizens NPCs' ability to travel through the portal."); | ||
| super.arguments("portal"); | ||
| super.requiresPlayer(); |
There was a problem hiding this comment.
It doesn't look this command requires the sender to be a player
|
|
||
| @Override | ||
| protected boolean onCommand(CommandSender sender, String[] args) { | ||
| Player player = (Player) sender; |
| * | ||
| * @param event The Citizens NavigationCompleteEvent | ||
| */ | ||
| @EventHandler(priority = EventPriority.NORMAL) |
There was a problem hiding this comment.
Nitpick but you don't need to specify the priority here; it'll default to NORMAL. Unless you were having issues without the priority then it's ok
| @Override | ||
| protected boolean onCommand(CommandSender sender, String[] args) { | ||
| Player player = (Player) sender; | ||
|
|
There was a problem hiding this comment.
If Citizens isn't loaded you should send a message to the user and return early.
| Messenger.msg(sender, Messenger.ReplaceMessage.SUGGEST_DELETE, "/portal remove " + portal.getName()); | ||
| return true; | ||
| } | ||
|
|
There was a problem hiding this comment.
If the portal's filler is water or lava I'd send a message to the user telling them about the consequences as noted in the README. I'd do the same thing in the setfiller sub-command if the portal allows citizens and the new fill type is water/lava.
| pm.registerEvents(new PortalDamageListener(), this); | ||
|
|
||
| // check if Citizens is enabled | ||
| if (pm.isPluginEnabled("Citizens")) { |
There was a problem hiding this comment.
I have a few comments later about conditionally doing stuff in commands if Citizens is enabled so I'd store pm.isPluginEnabled("Citizens") as an instance variable in here for use across the plugin.
Co-authored-by: Jack Henhapl <jackah2@illinois.edu>
4 participants
Portals now supports Citizens!
New Features:
/portal allowcitizens <portal_name>allows Citizens NPCs to use the specified portalallowcitizensstatus saves in configNotes:
WATERfillerLAVAfillerresolves #15