PMspam: /msg spam support #4230
Conversation
| * Declared globally to save its value<br><br> | ||
| * *Will be reset if (it > playerNames.length) | ||
| */ | ||
| int it = 0; |
There was a problem hiding this comment.
Instead of having a comment here, use a more descriptive variable name, like playerIndex. Also, the variable should be declared above the constructor, and with a visibility modifier.
| String text = messages.get().get(i); | ||
|
|
||
| // PMspam REALIZATION /////////////////////////////////////////////////////////////////////////////////////// |
There was a problem hiding this comment.
There's no point in having this comment, the if (PMspam.get()) block is already enough to know what this part does.
| // PMspam REALIZATION /////////////////////////////////////////////////////////////////////////////////////// | ||
| // If our button is toggled // TODO: List caching | ||
| if (PMspam.get()) { | ||
| if(mc.player != null) { |
There was a problem hiding this comment.
Generally, you'd want to append this condition to the previous if statement to improve readability, but in this context mc.player will never be null, so you can remove it entirely.
| } | ||
| // to avoid out of bound | ||
| if (playerNames.length > 0) { | ||
| if (it >= playerNames.length) { it = 0; } |
There was a problem hiding this comment.
In single-line if statements, you can omit the braces.
| } | ||
| // if MULTIPLAYER | ||
| else if (mc.player.getEntityWorld().getPlayers() != null) { | ||
| playerNames = mc.player.networkHandler.getCommandSource().getPlayerNames().toArray(new String[0]); |
There was a problem hiding this comment.
Instead of going through player to get the network handler, you can directly call mc.getNetworkHandler()
|
|
||
| // if LOCAL SERVER | ||
| if (mc.getServer() != null && mc.isConnectedToLocalServer()) { | ||
| playerNames = mc.getServer().getPlayerNames(); |
There was a problem hiding this comment.
Even if you're on a local server, you have access to the network handler. That means you can entirely remove this forking and always use the multiplayer branch, which ends up simplifying the code.
| if (PMspam.get()) { | ||
| if(mc.player != null) { | ||
| // Var to store String[] of names | ||
| String[] playerNames = new String[0]; |
There was a problem hiding this comment.
You don't need to define the variable since you're always gonna redefine it later, you should just write this as String[] playerNames;. This change also has the side effect of removing an unused object allocation.
| } | ||
| // if MULTIPLAYER | ||
| else if (mc.player.getEntityWorld().getPlayers() != null) { | ||
| playerNames = mc.player.networkHandler.getCommandSource().getPlayerNames().toArray(new String[0]); |
There was a problem hiding this comment.
Instead of calling networkHandler.getCommandSource().getPlayerNames(), you should be using networkHandler.getPlayerList(). getPlayerNames() allocates a new List every method call and internally uses getPlayerList(), so you can cut out the middle man and just use it directly. As a bonus, this returns an already cached list so you dont need to worry about it yourself.
| // PMspam button | ||
| private final Setting<Boolean> PMspam = sgGeneral.add(new BoolSetting.Builder() | ||
| .name("PMspam") | ||
| .description("Parses playersList and /msg them") |
There was a problem hiding this comment.
Avoid using developer syntax in descriptions, most of Meteor Client's userbase aren't used to it. You should just say "the player list" instead of "playersList".
| @@ -76,6 +76,15 @@ public class Spam extends Module { | |||
| .build() | |||
| ); | |||
|
|
|||
| // PMspam button | |||
There was a problem hiding this comment.
Unnecessary comment, the setting is self-explanatory.
| // Exclude our name // TODO: Friends name | ||
| if (playerNames.get(playerIndex).getProfile().getName().equals(mc.getSession().getUsername()) && playerNames.size() != 1) playerIndex++; | ||
|
|
||
| if (playerIndex < playerNames.size()) { |
There was a problem hiding this comment.
As I understand It's better to add here if (playerIndex >= playerNames.size()) playerIndex = 0; as well because in situation:
playerNames.size() is 2
playerIndex of our name is 1
We increment due to it's our name. And we're getting out of range so can't pass this
if (playerIndex < playerNames.size())
So we skip 1 message 😢
It's not a big problem but {delay} between messages is x2. One time in a cycle
There was a problem hiding this comment.
It's not a big problem, but it wouldn't be working as intended
I think you should implement that fix
Added "color formatting" symbols cleaner.
Added "color formatting" symbols cleaner.
# Conflicts: # src/main/java/meteordevelopment/meteorclient/systems/modules/misc/Spam.java
| @@ -115,6 +124,23 @@ private void onTick(TickEvent.Post event) { | |||
| } | |||
|
|
|||
| String text = messages.get().get(i); | |||
|
|
|||
| if (PMspam.get() && mc.getNetworkHandler() != null) { | |||
There was a problem hiding this comment.
mc.getNetworkHandler() is never null during TickEvent.Post
…'NullPointerException'". But - Done
|
the warning is due to the fact that it can return null, but only in the main menu |
Type of change
Description
Added a checkbox that will parse nicknames of people on server
And spam them in private messages
Related issues
No issues, but i have an idea to add "Ignore Friends"
How Has This Been Tested?
It was tested by many ways and now it works perfectly.
No problems, whether it's single-player, local, or multiplayer
Checklist: