Implemented Kit Cooldown Resetting #3124
Conversation
|
|
||
| @Override | ||
| public void run(final Server server, final User user, final String commandLabel, final String[] args) throws Exception { | ||
| if (user.isAuthorized("essentials.kit.reset")) { |
There was a problem hiding this comment.
Essentials already checks the permissions according to the name passed in the constructor - so this would require users to have both essentials.kitreset and essentials.kit.reset in order to trigger this action, which seems a little strange.
There was a problem hiding this comment.
Plus if they don't have permission... nothing happens. This is bad UX.
|
|
||
| @Override | ||
| public void run(final Server server, final User user, final String commandLabel, final String[] args) throws Exception { | ||
| if (user.isAuthorized("essentials.kit.reset")) { |
There was a problem hiding this comment.
Plus if they don't have permission... nothing happens. This is bad UX.
| final User userTo = getPlayer(server, user, args, 0); | ||
| final Kit kit = new Kit(args[1], ess); | ||
| kit.resetTime(userTo); | ||
| user.sendMessage(tl("kitReset", userTo.getDisplayName(), kit.getName())); |
There was a problem hiding this comment.
But what if user == userTo? They will be sent two messages... does this happen in other parts of EssX?
There was a problem hiding this comment.
Yes it is sadly, but you can just add:
if(userTo != user) {
userTo.sendMessage(tl("kitResetPlayer", kit.getName()));
}
| @Override | ||
| protected List<String> getTabCompleteOptions(final Server server, final User user, final String commandLabel, final String[] args) { | ||
| List<String> options = new ArrayList<>(); | ||
| if (args.length == 1 && user.isAuthorized("essentials.kit.reset")) { |
There was a problem hiding this comment.
See above comment about this perm, but in general doing two perm look ups feels clowny.
| @Override | ||
| public void run(final Server server, final User user, final String commandLabel, final String[] args) throws Exception { | ||
| if (user.isAuthorized("essentials.kit.reset")) { | ||
| final User userTo = getPlayer(server, user, args, 0); |
There was a problem hiding this comment.
EssentialsLoopCommand exists for the sole purpose of commands that act on other targets - you should be extending this class.
There was a problem hiding this comment.
In addition, the player parameter should come before the kit in the command's syntax - this is consistent with other EssentialsX loop commands.
| kitNotFound=\u00a74That kit does not exist. | ||
| kitOnce=\u00a74You can''t use that kit again. | ||
| kitReceive=\u00a76Received kit\u00a7c {0}\u00a76. | ||
| kitReset=\u00a76Resetting\u00a7c {0}''s\u00a76 cooldown for kit\u00a7c {1}\u00a76. |
There was a problem hiding this comment.
Only the username should be highlighted, not the 's.
mdcfe
added
the
status: waiting on author
pop4959
left a comment
pop4959
left a comment
There was a problem hiding this comment.
Needs to address other requested changes
darbyjack
force-pushed
the
reset-kit-cooldowns
branch
from
b89ba35 to
df3e108
Compare
|
Closing due to author inactivity. Can re-open if there are signs of life, especially addressing any concerns that have been mentioned. |
6 participants
This is a remake of my original PR (#2645) that I did a good on when rebasing.
To make things easier, I have created a new PR with everything up-to-date.
Contents: Implements the ability to reset a Kit's Cooldown for a player.
Example:
Closes #163