Tpa Flooding Button #1108
Tpa Flooding Button #1108
Conversation
|
Warning Rate limit exceeded@JAXPLE has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 42 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe pull request introduces changes to the The Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/main/java/net/wurstclient/hacks/MassTpaHack.java (3)
60-62: Improve setting description and naming.The checkbox setting could be improved for clarity:
- Fix grammatical error in description ("my friend" → "my friends")
- Enhance description to better explain the behavior and potential impact
- Consider a more concise variable name like
tpaFloodEnabled- private final CheckboxSetting isActiveMassTpaFlooding = new CheckboxSetting( - "TPA Flood", "Re-request TPA from all players except my friend", - false); + private final CheckboxSetting tpaFloodEnabled = new CheckboxSetting( + "TPA Flood", + "Continuously cycles between sending TPA and TPA cancel requests to all players except friends. " + + "Warning: This may be considered spam on some servers.", + false);
113-117: Reorder empty players check.The empty players check should be performed before registering event listeners to avoid unnecessary registration and cleanup.
Collections.shuffle(players, random); - EVENTS.add(ChatInputListener.class, this); - EVENTS.add(UpdateListener.class, this); - if (players.isEmpty()) { - ChatUtils.error("Couldn't find any players."); + ChatUtils.error("No valid targets found. All online players are either you or your friends."); setEnabled(false); + return; } + + EVENTS.add(ChatInputListener.class, this); + EVENTS.add(UpdateListener.class, this);
99-103: Optimize player filtering conditions.The player filtering conditions can be combined for better readability and efficiency.
- if (isActiveMassTpaFlooding.isChecked() && WURST.getFriends().contains(name)) - continue; - - if (name.equalsIgnoreCase(playerName)) - continue; + if (!name.equalsIgnoreCase(playerName) && + !(isActiveMassTpaFlooding.isChecked() && WURST.getFriends().contains(name))) { + players.add(name); + } - players.add(name);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/net/wurstclient/hacks/MassTpaHack.java(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (3)
src/main/java/net/wurstclient/hacks/MassTpaHack.java (3)
68-68: LGTM! Good variable renaming.The new variable name
sendTpaCountis more descriptive and better represents its purpose than the previousindex.
79-79: LGTM! Setting properly registered.
60-62: Security Concern: Potential for AbuseThe TPA flooding feature could:
- Violate server rules regarding spam/harassment
- Trigger anti-cheat or anti-spam measures
- Lead to user account bans
Consider:
- Adding prominent warnings about usage risks
- Implementing server-specific cooldowns
- Adding detection for server warnings/kicks
Also applies to: 135-144
| if (isActiveMassTpaFlooding.isChecked() && sendTpaCount >= players.size()) | ||
| { | ||
| sendTpaCount = 0; | ||
|
|
||
| if (command.equals("tpa")) | ||
| command = "tpacancel"; | ||
|
|
||
| else if (command.equals("tpacancel")) | ||
| command = "tpa"; | ||
| } |
There was a problem hiding this comment.
Add safeguards to command toggling logic.
Several improvements needed for the command toggling implementation:
- Command strings should be constants to avoid typos and enable easier maintenance
- Add rate limiting to prevent server spam detection
- Consider adding a safety check for maximum continuous cycles
+ private static final String CMD_TPA = "tpa";
+ private static final String CMD_TPA_CANCEL = "tpacancel";
+ private static final int MAX_CONTINUOUS_CYCLES = 3;
+ private int cycleCount = 0;
+
if (isActiveMassTpaFlooding.isChecked() && sendTpaCount >= players.size())
{
sendTpaCount = 0;
+ cycleCount++;
+
+ if (cycleCount >= MAX_CONTINUOUS_CYCLES) {
+ ChatUtils.message("Maximum flood cycles reached. Stopping to prevent detection.");
+ setEnabled(false);
+ return;
+ }
- if (command.equals("tpa"))
- command = "tpacancel";
- else if (command.equals("tpacancel"))
- command = "tpa";
+ command = command.equals(CMD_TPA) ? CMD_TPA_CANCEL : CMD_TPA;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (isActiveMassTpaFlooding.isChecked() && sendTpaCount >= players.size()) | |
| { | |
| sendTpaCount = 0; | |
| if (command.equals("tpa")) | |
| command = "tpacancel"; | |
| else if (command.equals("tpacancel")) | |
| command = "tpa"; | |
| } | |
| private static final String CMD_TPA = "tpa"; | |
| private static final String CMD_TPA_CANCEL = "tpacancel"; | |
| private static final int MAX_CONTINUOUS_CYCLES = 3; | |
| private int cycleCount = 0; | |
| if (isActiveMassTpaFlooding.isChecked() && sendTpaCount >= players.size()) | |
| { | |
| sendTpaCount = 0; | |
| cycleCount++; | |
| if (cycleCount >= MAX_CONTINUOUS_CYCLES) { | |
| ChatUtils.message("Maximum flood cycles reached. Stopping to prevent detection."); | |
| setEnabled(false); | |
| return; | |
| } | |
| command = command.equals(CMD_TPA) ? CMD_TPA_CANCEL : CMD_TPA; | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/main/java/net/wurstclient/hacks/MassTpaHack.java (3)
60-61: Consider renaming the setting to better reflect its purpose.The current name "TPA Flood" might encourage abuse. Consider a more neutral name like "Continuous TPA" or "TPA Cycle". Also, the description could be more precise about what the feature does.
- private final CheckboxSetting isActiveMassTpaFlooding = new CheckboxSetting( - "TPA Flood", "Re-request TPA from all players except my friend", false); + private final CheckboxSetting isActiveMassTpaFlooding = new CheckboxSetting( + "Continuous TPA", "Automatically cycle between sending and canceling TPA requests to non-friends", false);
99-102: Add null check for friend list access.While the friend check logic is correct, we should add a null check to prevent potential NPEs.
- if(isActiveMassTpaFlooding.isChecked() - && WURST.getFriends().contains(name)) + if(isActiveMassTpaFlooding.isChecked() + && WURST.getFriends() != null + && WURST.getFriends().contains(name))
59-61: Add usage guidelines to prevent abuse.This feature could potentially be used for griefing or server abuse. Consider:
- Adding a warning message when enabled
- Implementing server-specific cooldowns
- Adding documentation about responsible usage
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/net/wurstclient/hacks/MassTpaHack.java(4 hunks)
🔇 Additional comments (3)
src/main/java/net/wurstclient/hacks/MassTpaHack.java (3)
67-67: LGTM! Variable name change improves clarity.The rename from
indextosendTpaCountbetter describes the variable's purpose.
78-78: LGTM! Setting properly registered.
137-147: Previous review comments about command toggling safeguards still apply.The command toggling logic needs improvements as mentioned in the previous review:
- Command strings should be constants
- Rate limiting is needed
- Safety checks for maximum continuous cycles should be added
| if(!isActiveMassTpaFlooding.isChecked() | ||
| && sendTpaCount >= players.size()) | ||
| { | ||
| command = "/tpa"; | ||
| setEnabled(false); | ||
| return; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use the command setting value instead of hardcoding "/tpa".
The hardcoded "/tpa" command ignores the user's command setting preference.
- command = "/tpa";
+ command = commandSetting.getValue().substring(1);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if(!isActiveMassTpaFlooding.isChecked() | |
| && sendTpaCount >= players.size()) | |
| { | |
| command = "/tpa"; | |
| setEnabled(false); | |
| return; | |
| if(!isActiveMassTpaFlooding.isChecked() | |
| && sendTpaCount >= players.size()) | |
| { | |
| command = commandSetting.getValue().substring(1); | |
| setEnabled(false); | |
| return; |
| .sendChatCommand(command + " " + players.get(sendTpaCount)); | ||
|
|
||
| index++; | ||
| sendTpaCount++; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add rate limiting and error handling for network calls.
The command execution should include:
- Rate limiting to prevent server timeouts
- Error handling for network failures
- Logging for debugging purposes
+ try {
+ if (System.currentTimeMillis() - lastCommandTime < MIN_COMMAND_INTERVAL) {
+ return;
+ }
MC.getNetworkHandler()
.sendChatCommand(command + " " + players.get(sendTpaCount));
+ lastCommandTime = System.currentTimeMillis();
sendTpaCount++;
+ } catch (Exception e) {
+ ChatUtils.error("Failed to send command: " + e.getMessage());
+ setEnabled(false);
+ }Committable suggestion skipped: line range outside the PR's diff.
Description
Testing
References