Pvp utils improvement #69
Conversation
CraftedMods
added
improvement
|
I cannot really proceed until someone approved it. Unless we see the current situation as an exception and don't require a PR review. |
| @@ -28,13 +28,14 @@ public boolean canCommandSenderUseCommand (ICommandSender sender) | |||
| public void processCommand (ICommandSender sender, String[] args) | |||
| { | |||
| EntityPlayerMP player = getCommandSenderAsPlayer (sender); | |||
| EnumPvPMode currentMode = PvPUtils.getPvPMode (player); | |||
There was a problem hiding this comment.
Why add this here and not check the mode in cancelPvPTimer?
There was a problem hiding this comment.
Because firstly I used it in the other function too but then I left the function as it was. It could also be moved to the function you've mentioned, but at least I like it to have "metadata" like this in a higher level because they could be used in future "child functions". But it's not that important, I think.
|
Also, I'm aware of the formatting issues. But I think it's the best to apply our formatting (and hopefully cleanup, see #64) guidelines once when we prepare a new release (or pre-release), because otherwise every commit is polluted with formattings, which makes the PRs hard to review. |
|
The "this calls" were actually a leftover from my old cleanup template. |
|
But this is why I'm strongly for reviews of the PR's. Two or three pairs of eyes see more than one. |
Closes #41. I didn't add utilities for the logging messages because they're not reused through the code.