Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add AutoPlaceHack #1051

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open

Conversation

LilyKensa
Copy link

※ I recreated #1048 because I used the wrong branch, sorry.

Description

Added a simple but cool feature, AutoPlace. This hack allows you to free up your right middle finger when bridging, as it will place the blocks for you when you need to.

When it's enabled, blocks will be placed as long as you aim at side faces of blocks.

Testing

It's working, here's the video
Don't complain about my aiming though, I'm using a very bad mouse right now

Notes

😎 I've applied spotless to match the original coding style.

🤔 Not sure if I need to disable some other hacks like ScaffoldWalk when this hack is turned on?

Copy link

coderabbitai bot commented Aug 9, 2024

📝 Walkthrough
📝 Walkthrough

Walkthrough

The changes introduce several new features and modifications across multiple classes in the WurstClient Minecraft mod. A new AutoPlaceHack class has been added for automating block placement, along with instance variables in HackList to integrate this functionality. Additionally, new filtering classes for teammates and NPC-like entities have been introduced, enhancing entity filtering capabilities. The PlayerEspHack now disables teamEspHack when enabled, and various settings have been added or modified in existing hacks to improve user control and gameplay experience.

Changes

File Path Change Summary
src/main/java/net/wurstclient/hack/HackList.java Added variables public final AutoPlaceHack autoPlaceHack and public final TeamEspHack teamEspHack.
src/main/java/net/wurstclient/hacks/AutoPlaceHack.java Introduced class AutoPlaceHack with methods onEnable(), onDisable(), and onUpdate(), and settings for block placement.
src/main/java/net/wurstclient/hacks/AimAssistHack.java Added CheckboxSetting onlyHoldingWeapon to restrict aiming functionality to when a weapon is held.
src/main/java/net/wurstclient/hacks/AutoReconnectHack.java Added CheckboxSetting buttons for toggling button visibility on the reconnect screen, and method shouldShowButtons().
src/main/java/net/wurstclient/hacks/PlayerEspHack.java Added logic to disable teamEspHack when PlayerEspHack is enabled.
src/main/java/net/wurstclient/hacks/SafeWalkHack.java Introduced SliderSetting minDepth and motionPrediction, and modified onClipAtLedge method signature.
src/main/java/net/wurstclient/hacks/TeamEspHack.java Added class TeamEspHack for rendering team indicators with customizable settings.
src/main/java/net/wurstclient/hacks/TriggerBotHack.java Added ItemListSetting blacklistedItems to prevent trigger bot activation with certain items.
src/main/java/net/wurstclient/mixin/ClientPlayerEntityMixin.java Added multiple event injections and modified movement and item use handling.
src/main/java/net/wurstclient/mixin/DisconnectedScreenMixin.java Updated logic for displaying auto-reconnect buttons based on user settings.
src/main/java/net/wurstclient/settings/filterlists/EntityFilterList.java Enhanced genericCombat method with additional filters for teammates and NPCs.
src/main/java/net/wurstclient/settings/filters/FilterNpcLikeSetting.java Introduced class FilterNpcLikeSetting for filtering NPC-like entities.
src/main/java/net/wurstclient/settings/filters/FilterTeammatesSetting.java Introduced class FilterTeammatesSetting for filtering teammates based on display name color.

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between d4b624e and 40778be.

⛔ Files ignored due to path filters (2)
  • src/main/resources/assets/wurst/translations/en_us.json is excluded by !**/*.json
  • src/main/resources/assets/wurst/translations/zh_tw.json is excluded by !**/*.json
📒 Files selected for processing (15)
  • gradle.properties (1 hunks)
  • src/main/java/net/wurstclient/WurstClient.java (1 hunks)
  • src/main/java/net/wurstclient/hack/HackList.java (2 hunks)
  • src/main/java/net/wurstclient/hacks/AimAssistHack.java (5 hunks)
  • src/main/java/net/wurstclient/hacks/AutoPlaceHack.java (1 hunks)
  • src/main/java/net/wurstclient/hacks/AutoReconnectHack.java (2 hunks)
  • src/main/java/net/wurstclient/hacks/PlayerEspHack.java (1 hunks)
  • src/main/java/net/wurstclient/hacks/SafeWalkHack.java (4 hunks)
  • src/main/java/net/wurstclient/hacks/TeamEspHack.java (1 hunks)
  • src/main/java/net/wurstclient/hacks/TriggerBotHack.java (5 hunks)
  • src/main/java/net/wurstclient/mixin/ClientPlayerEntityMixin.java (2 hunks)
  • src/main/java/net/wurstclient/mixin/DisconnectedScreenMixin.java (1 hunks)
  • src/main/java/net/wurstclient/settings/filterlists/EntityFilterList.java (1 hunks)
  • src/main/java/net/wurstclient/settings/filters/FilterNpcLikeSetting.java (1 hunks)
  • src/main/java/net/wurstclient/settings/filters/FilterTeammatesSetting.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/java/net/wurstclient/hack/HackList.java
🧰 Additional context used
🪛 ast-grep
src/main/java/net/wurstclient/hacks/SafeWalkHack.java

[warning] 29-33: Detected a cookie where the Secure flag is either missing or disabled. The Secure cookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set the Secure flag to true so the cookie will only be sent over HTTPS.
Context: (
"Motion prediction",
"Predict your motion to sneak earlier.\n"
+ "If not stopping at edges, increase; If not stopping near a wall, decrease.",
2.0, 1, 5, 0.5, ValueDisplay.DECIMAL.withSuffix("x"))
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration


[warning] 29-33: Detected a cookie where the HttpOnly flag is either missing or disabled. The HttpOnly cookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set the HttpOnly flag to true` in all other cases.
Context: (
"Motion prediction",
"Predict your motion to sneak earlier.\n"
+ "If not stopping at edges, increase; If not stopping near a wall, decrease.",
2.0, 1, 5, 0.5, ValueDisplay.DECIMAL.withSuffix("x"))
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration

🔇 Additional comments (23)
gradle.properties (3)

8-8: Verify compatibility with Minecraft 1.21

The Minecraft version has been updated from 1.21.1 to 1.21. This change aligns with the PR objectives.

Please ensure that all features of the mod are compatible with Minecraft 1.21. You may want to run comprehensive tests to confirm full functionality.


9-9: LGTM: Yarn mappings updated

The Yarn mappings have been updated to 1.21+build.9, which is consistent with the Minecraft version change and likely includes more up-to-date mappings.


16-16: LGTM: Mod version updated

The mod version has been correctly updated to reflect the new Minecraft version (1.21). This change is consistent with the other updates in this file.

src/main/java/net/wurstclient/settings/filters/FilterTeammatesSetting.java (4)

1-9: LGTM: Class declaration and package structure are appropriate.

The FilterTeammatesSetting class is correctly placed in the net.wurstclient.settings.filters package. The class is declared as public final, which is good for immutability. Extending EntityFilterCheckbox suggests it's part of a larger entity filtering system, which aligns with the purpose of the class.


36-40: LGTM: Well-designed static factory method for combat scenarios.

The genericCombat method is a well-implemented static factory method. It provides a convenient way to create a combat-specific FilterTeammatesSetting instance with a clear and accurate description.


42-47: LGTM: Consistent design with genericCombat method.

The genericVision method is well-implemented and follows the same pattern as genericCombat. This consistency in design is commendable. The method provides a convenient way to create a visibility-specific FilterTeammatesSetting instance with a clear and accurate description.


1-47: Summary: Well-implemented teammate filter supporting PR objectives.

The FilterTeammatesSetting class is a solid implementation of a teammate filter based on display name color. It provides convenient factory methods for combat and vision scenarios, which aligns well with the PR objectives of enhancing entity filtering capabilities. This class will support features like the new AutoPlaceHack by providing a way to filter teammates during gameplay.

The implementation is generally sound, with thorough null checks and clear logic. The suggested optimizations, such as caching the player's color and using a constant for the filter name, could further improve its performance and maintainability.

Overall, this new class is a valuable addition to the codebase and contributes positively to the goals of this pull request.

src/main/java/net/wurstclient/hacks/AutoReconnectHack.java (3)

14-14: LGTM: Import statement added correctly.

The new import for CheckboxSetting is necessary and properly placed with other import statements.


34-34: LGTM: New setting correctly added to the hack's configuration.

The buttons setting is properly added to the hack's settings in the constructor, ensuring it's included in the configuration.


Line range hint 1-48: Overall assessment: Well-implemented feature addition.

The changes to AutoReconnectHack successfully introduce a new user preference for controlling the visibility of the reconnect button. This enhancement aligns well with the PR objectives and improves user experience by providing more control over the UI.

Key points:

  1. New CheckboxSetting for button visibility.
  2. Proper integration of the new setting into the hack's configuration.
  3. Clean interface method for accessing the visibility preference.

The implementation is clean, well-documented, and integrates smoothly with the existing code. Great job!

🧰 Tools
🪛 ast-grep

[warning] 26-27: Detected a cookie where the Secure flag is either missing or disabled. The Secure cookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set the Secure flag to true so the cookie will only be sent over HTTPS.
Context: ("Wait time", "Time before reconnecting in seconds.",
5, 0, 60, 0.5, ValueDisplay.DECIMAL.withSuffix("s"))
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration


[warning] 26-27: Detected a cookie where the HttpOnly flag is either missing or disabled. The HttpOnly cookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set the HttpOnly flag to true` in all other cases.
Context: ("Wait time", "Time before reconnecting in seconds.",
5, 0, 60, 0.5, ValueDisplay.DECIMAL.withSuffix("s"))
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration

src/main/java/net/wurstclient/settings/filterlists/EntityFilterList.java (2)

65-66: Clarify the relationship between entity filtering changes and AutoPlace feature.

The additions to the genericCombat() method enhance the entity filtering capabilities of the mod. However, the connection between these changes and the AutoPlace feature mentioned in the PR objectives is not immediately clear.

Could you please clarify:

  1. How do these new filtering options relate to the AutoPlace feature?
  2. Are these changes part of a broader update to the mod's combat system?
  3. Do these filters affect the behavior of AutoPlace when it's active?

Understanding these relationships will help ensure that the changes are consistent with the overall goals of the PR.


65-66: Verify impact and update documentation for new filtering options.

The addition of teammate and NPC-like entity filters enhances the combat-related filtering capabilities. This change might affect other parts of the codebase that use the genericCombat() method.

Please run the following script to check for potential impacts:

Consider updating the class or method documentation to reflect these new filtering options, explaining their purpose and default behavior.

✅ Verification successful

Impact of New Filtering Options Verified

The addition of FilterTeammatesSetting and FilterNpcLikeSetting enhances combat filtering across the following classes:

  • TriggerBotHack.java
  • TpAuraHack.java
  • MultiAuraHack.java
  • KillauraHack.java
  • FightBotHack.java
  • ClickAuraHack.java
  • BowAimbotHack.java
  • TeamEspHack.java
  • AimAssistHack.java

No unintended impacts were found on other parts of the codebase. Please ensure that the class or method documentation is updated to reflect these new filtering options.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for usages of genericCombat() method and potential impacts

# Search for usages of genericCombat() method
echo "Searching for usages of genericCombat() method:"
rg "genericCombat\(\)" --type java

# Search for references to FilterTeammatesSetting and FilterNpcLikeSetting
echo "\nSearching for references to new filter settings:"
rg "FilterTeammatesSetting|FilterNpcLikeSetting" --type java

Length of output: 3350

src/main/java/net/wurstclient/hacks/PlayerEspHack.java (1)

80-80: Approve change, but clarification needed on teamEspHack interaction.

The addition of WURST.getHax().teamEspHack.setEnabled(false); is a good way to prevent potential conflicts between PlayerEspHack and teamEspHack. However, I have a few points to consider:

  1. Could you please clarify the rationale behind disabling teamEspHack when PlayerEspHack is enabled? This interaction isn't mentioned in the PR objectives.

  2. Have you considered a more flexible approach? For example:

    • Adding a setting that allows users to choose whether teamEspHack should be disabled automatically.
    • Implementing logic to make these hacks work together harmoniously instead of disabling one.
  3. It might be helpful to add a comment explaining why teamEspHack is being disabled, to improve code maintainability.

To ensure this change doesn't introduce any unintended side effects, please run the following verification script:

✅ Verification successful

Change Verified

The teamEspHack is only being disabled within PlayerEspHack.java, and no other interactions or dependencies affect it elsewhere in the codebase. This ensures that the change is isolated and does not introduce unintended side effects.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any other interactions between PlayerEspHack and teamEspHack

# Test: Search for other mentions of teamEspHack in PlayerEspHack
echo "Searching for other mentions of teamEspHack in PlayerEspHack:"
rg "teamEspHack" src/main/java/net/wurstclient/hacks/PlayerEspHack.java

# Test: Search for mentions of PlayerEspHack in teamEspHack
echo "Searching for mentions of PlayerEspHack in teamEspHack:"
rg "PlayerEspHack" src/main/java/net/wurstclient/hacks/TeamEspHack.java

# Test: Check if there are any other hacks that disable teamEspHack
echo "Checking for other hacks that disable teamEspHack:"
rg "teamEspHack.setEnabled\(false\)" src/main/java/net/wurstclient/hacks/

Length of output: 788

src/main/java/net/wurstclient/hacks/AimAssistHack.java (3)

15-18: LGTM: New imports added correctly.

The new imports for AxeItem, Item, and Items are necessary for the added functionality and follow the existing import style.


71-72: LGTM: Entity filters updated appropriately.

The addition of FilterTeammatesSetting and FilterNpcLikeSetting to the entityFilters initialization enhances the entity filtering capabilities as intended. The default values and placement in the list are appropriate.


Line range hint 1-238: Overall assessment: Changes are well-implemented and align with PR objectives.

The modifications to the AimAssistHack class successfully introduce the new onlyHoldingWeapon feature and improve entity filtering capabilities. The code is well-structured, and the new functionality is integrated seamlessly with the existing implementation. The minor suggestions provided earlier will further enhance code readability and maintainability.

src/main/java/net/wurstclient/WurstClient.java (1)

54-54: LGTM: Minecraft version update.

The change from "1.21.1" to "1.21" for the MC_VERSION constant is consistent with the update mentioned in the PR objectives for the gradle.properties file. This update appears to be part of a broader effort to align the project with a specific Minecraft version.

To ensure consistency across the codebase, please run the following script:

✅ Verification successful

Verified: Minecraft version update is consistent across the codebase.

All references to version "1.21.1" have been successfully updated to "1.21". The changes in gradle.properties and WurstClient.java are consistent with the project’s objectives. Comments referencing "1.21" in other files pertain to bug tracking and historical context, which are appropriate and intentional.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify Minecraft version consistency across the codebase.

# Test 1: Check for any remaining references to version 1.21.1
echo "Checking for remaining references to version 1.21.1:"
rg "1\.21\.1" --type java --type gradle --type properties

# Test 2: Verify that all relevant files now reference version 1.21
echo "Verifying references to version 1.21:"
rg "1\.21(?!\.)" --type java --type gradle --type properties

# Note: Review the output to ensure all occurrences are intentional and consistent.

Length of output: 577


Script:

#!/bin/bash
# Description: Verify Minecraft version consistency across the codebase.

# Define a custom type for .properties files
rg --type-add 'properties:*.properties'

# Test 1: Check for any remaining references to version 1.21.1
echo "Checking for remaining references to version 1.21.1:"
rg --pcre2 "1\.21\.1" --type java --type gradle --type properties

# Test 2: Verify that all relevant files now reference version 1.21
echo "Verifying references to version 1.21:"
rg --pcre2 "1\.21(?!\.)" --type java --type gradle --type properties

# Note: Review the output to ensure all occurrences are intentional and consistent.

Length of output: 847


Script:

#!/bin/bash
# Description: Verify Minecraft version consistency across the codebase.

# Test 1: Check for any remaining references to version 1.21.1
echo "Checking for remaining references to version 1.21.1:"
rg --type-add 'properties:*.properties' --pcre2 "1\.21\.1" --type java --type gradle --type properties

# Test 2: Verify that all relevant files now reference version 1.21
echo "Verifying references to version 1.21:"
rg --type-add 'properties:*.properties' --pcre2 "1\.21(?!\.)" --type java --type gradle --type properties

# Note: Review the output to ensure all occurrences are intentional and consistent.

Length of output: 911

src/main/java/net/wurstclient/settings/filters/FilterNpcLikeSetting.java (1)

16-22: ⚠️ Potential issue

Clarify the filtering logic for player names not matching criteria

The current implementation filters out entities based on specific name characteristics, such as name length and character composition. However, returning true in these cases may not align with the intended filtering behavior. Please confirm that returning true is intended to include the entity when it doesn't match the NPC-like criteria.

To verify the behavior, check how the test method is used in the context of EntityFilterCheckbox and ensure that the return values correctly reflect whether an entity should be filtered out or not.

src/main/java/net/wurstclient/hacks/TriggerBotHack.java (1)

158-159: Verify that "minecraft:air" correctly represents an empty hand

When the player's main hand is empty, the item is represented as Items.AIR. Ensure that "minecraft:air" is the correct identifier returned by Registries.ITEM.getId(Items.AIR).toString(). If not, the blacklist check for an empty hand may not work as intended.

Run the following script to confirm that "minecraft:air" is the correct item name for an empty stack:

✅ Verification successful

"minecraft:air" correctly represents an empty hand

The verification process confirms that "minecraft:air" is indeed the correct identifier used to represent an empty hand in the codebase. This is evidenced by the following code snippet from src/main/java/net/wurstclient/hacks/TriggerBotHack.java:

if(stack.isEmpty())
{
    if(blacklistedItems.getItemNames().contains("minecraft:air"))
        return;
}

This code checks if the player's hand is empty (stack.isEmpty()), and then verifies if "minecraft:air" is in the blacklisted items. This implementation correctly uses "minecraft:air" to represent an empty hand, validating the original implementation.

The use of Registries.ITEM.getId(Items.AIR).toString() is not explicitly found in the codebase, but the consistent use of "minecraft:air" across different parts of the code (including nuker settings and base finder configurations) further supports its correctness.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the item ID for an empty hand (Items.AIR)

# Test: Get the registry ID of Items.AIR
# Expected output: minecraft:air
echo "Item ID for Items.AIR:"
ast-grep --pattern $'Registries.ITEM.getId(Items.AIR)' *.java --json | jq '.captures[0].name'

Length of output: 273


Script:

#!/bin/bash
# Description: Locate all Java files and verify the item ID for an empty hand (Items.AIR)

# Find all .java files in the repository
java_files=$(fd --extension java)

if [ -z "$java_files" ]; then
  echo "No Java files found in the repository."
  exit 1
fi

echo "Verifying the item ID for Items.AIR in the following files:"
echo "$java_files"

# Search for the pattern in all found Java files and extract the item ID
ast-grep --pattern 'Registries.ITEM.getId(Items.AIR)' $java_files --json | jq '.captures[0].name'

Length of output: 180446


Script:

#!/bin/bash
# Description: Locate and verify the item ID for an empty hand (Items.AIR)

# Find all .java files in the repository
java_files=$(fd --extension java)

if [ -z "$java_files" ]; then
  echo "No Java files found in the repository."
  exit 1
fi

echo "Searching for 'Registries.ITEM.getId(Items.AIR)' in the following files:"
echo "$java_files"

# Search for the pattern with flexible whitespace and display matching lines with 2 lines of context
rg -C 2 'Registries\.ITEM\.getId\s*\(\s*Items\.AIR\s*\)\.toString\s*\(\s*\)' $java_files

Length of output: 180401


Script:

#!/bin/bash
# Description: Search for references to Items.AIR and related empty hand checks

echo "Searching for references to Items.AIR:"
rg -C 2 'Items\.AIR' src/main/java

echo -e "\nSearching for potential empty hand checks:"
rg -C 2 'getStack\(\)\.isEmpty\(\)' src/main/java

echo -e "\nSearching for 'minecraft:air' string:"
rg -C 2 '"minecraft:air"' src/main/java

Length of output: 3499

src/main/java/net/wurstclient/hacks/TeamEspHack.java (2)

78-78: Verify the disabling of playerEspHack when TeamEspHack is enabled

In the onEnable() method, playerEspHack is automatically disabled:

WURST.getHax().playerEspHack.setEnabled(false);

Please confirm if this behavior is intentional. If both hacks should not be active simultaneously, consider informing the user when one hack disables another, or adjusting the UI to prevent them from being enabled at the same time.


103-103: Question the necessity of the Y-coordinate filtering condition

At line 103, there's a filter that limits players based on the Y-coordinate difference:

.filter(e -> Math.abs(e.getY() - MC.player.getY()) <= 1e6);

Given that the vertical build limit in Minecraft is much less than 1,000,000 blocks, this condition might be unnecessary or could be replaced with a more meaningful threshold. If there isn't a specific reason for this large value, consider removing it to simplify the code.

src/main/java/net/wurstclient/mixin/ClientPlayerEntityMixin.java (2)

273-273: Integration of SafeWalk functionality into clipAtLedge method looks good

The addition of || WurstClient.INSTANCE.getHax().safeWalkHack.shouldClip(); correctly ensures that the clipAtLedge() method accounts for the status of the SafeWalk hack.


286-286: Correct invocation of SafeWalk's onClipAtLedge within adjustMovementForSneaking

The call to safeWalkHack.onClipAtLedge() inside adjustMovementForSneaking properly allows the SafeWalk hack to adjust player movement when movement is not null.


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

This pull request has been open for a while with no recent activity. If you're still working on this or waiting for a review, please add a comment or commit within the next 7 days to keep it open. Otherwise, the pull request will be automatically closed to free up time for other tasks.

Pull requests should be closed if:

  • They have been superseded by another pull request
  • They are out of scope or don't align with the project
  • They have become obsolete due to other changes
  • They have bugs or conflicts that won't be resolved

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🧹 Outside diff range and nitpick comments (3)
src/main/java/net/wurstclient/hacks/AutoPlaceHack.java (3)

98-101: Ensure consistent handling of ActionResult.

Currently, you check if actionResult.isAccepted() before swinging the hand. For complete accuracy, you should also handle the case where the result is a SUCCESS, which may not always equate to isAccepted().

Consider updating the condition:

-    if(actionResult.isAccepted())
+    if(actionResult.isAccepted() && actionResult.shouldSwingHand())

29-35: Clarify the description for fastPlace setting.

The description "Builds as if FastPlace was enabled, even if it's not." might be misleading. It could be rephrased for better clarity.

Update the setting description:

-    "Builds as if FastPlace was enabled, even if it's not.", true);
+    "Allows block placement without the usual cooldown, even if FastPlace hack is disabled.", true);

70-72: Optimize inventory slot retrieval.

Accessing player.getInventory().selectedSlot directly can be replaced with a more direct method for readability and potential performance benefits.

Simplify the code:

-    if(!(player.getInventory().getStack(player.getInventory().selectedSlot)
+    if(!(player.getMainHandStack()
         .getItem() instanceof BlockItem))
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 37febd5 and d4b624e.

⛔ Files ignored due to path filters (2)
  • src/main/resources/assets/wurst/translations/en_us.json is excluded by !**/*.json
  • src/main/resources/assets/wurst/translations/zh_tw.json is excluded by !**/*.json
📒 Files selected for processing (2)
  • src/main/java/net/wurstclient/hack/HackList.java (1 hunks)
  • src/main/java/net/wurstclient/hacks/AutoPlaceHack.java (1 hunks)
🧰 Additional context used
🔇 Additional comments (1)
src/main/java/net/wurstclient/hack/HackList.java (1)

52-52: LGTM: AutoPlaceHack added correctly

The addition of AutoPlaceHack is consistent with the existing pattern and placed in the correct alphabetical order. This aligns with the PR objective of introducing the new AutoPlace feature.

Consider potential interactions with other hacks, especially ScaffoldWalk as mentioned in the PR notes. It might be beneficial to review the AutoPlaceHack implementation to ensure it doesn't conflict with existing functionalities. You can use the following script to check for any direct references between AutoPlaceHack and ScaffoldWalk:

✅ Verification successful

Verified: No interactions found between AutoPlaceHack and ScaffoldWalk

The search results indicate that there are no interactions between AutoPlaceHack and ScaffoldWalk. The addition of AutoPlaceHack does not conflict with existing functionalities.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any interactions between AutoPlaceHack and ScaffoldWalk
rg -i "autoplacehack.*scaffoldwalk|scaffoldwalk.*autoplacehack" --type java

Length of output: 77

Comment on lines +78 to +81
BlockHitResult blockHitResult = (BlockHitResult)hitResult;
if(blockHitResult.getSide() == Direction.UP
|| blockHitResult.getSide() == Direction.DOWN)
return;
Copy link

@coderabbitai coderabbitai bot Oct 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider allowing placement on the top and bottom faces.

Currently, the hack does not allow placing blocks when targeting the top or bottom face of a block. This might limit the usefulness of the hack in certain building scenarios.

Consider removing or adjusting this condition to allow more flexible block placement.

-    if(blockHitResult.getSide() == Direction.UP
-        || blockHitResult.getSide() == Direction.DOWN)
-        return;
📝 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.

Suggested change
BlockHitResult blockHitResult = (BlockHitResult)hitResult;
if(blockHitResult.getSide() == Direction.UP
|| blockHitResult.getSide() == Direction.DOWN)
return;
BlockHitResult blockHitResult = (BlockHitResult)hitResult;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be made configurable in the settings. The PR author said they only want it to place blocks horizontally, but I agree this might limit its usefulness e.g. if you're trying to tower up.

Some different ideas for how the settings might work:

  • a single checkbox "Allow vertical placement"
  • an enum "Direction" ["Horizontal", "Vertical", "Both"]
  • multiple checkboxes "Place up", "Place down", "Place horizontal"
  • a long enum "Place directions" ["Horizontal", "Vertical", "Up only", "Down only", "Up/horizontal", "Down/horizontal", "All"]

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Alexander01998

Great suggestion! Making the placement directions configurable would enhance the flexibility of the AutoPlace hack.

You could implement this by adding an EnumSetting to allow users to select their preferred placement directions. Here's how you might do it:

private final EnumSetting<PlacementDirection> placementDirection =
    new EnumSetting<>("Placement Direction", PlacementDirection.values(), PlacementDirection.HORIZONTAL);

private enum PlacementDirection
{
    HORIZONTAL("Horizontal"),
    VERTICAL("Vertical"),
    BOTH("Both");

    private final String name;

    private PlacementDirection(String name)
    {
        this.name = name;
    }

    @Override
    public String toString()
    {
        return name;
    }
}

Then, adjust the onUpdate method to handle the selected option:

BlockHitResult blockHitResult = (BlockHitResult) hitResult;
Direction side = blockHitResult.getSide();

switch (placementDirection.getSelected())
{
    case HORIZONTAL:
        if (side == Direction.UP || side == Direction.DOWN)
            return;
        break;
    case VERTICAL:
        if (side.getAxis().isHorizontal())
            return;
        break;
    case BOTH:
        // Allow all directions
        break;
}

Alternatively, if you prefer multiple checkboxes for more granular control, you could add:

private final CheckboxSetting placeUp =
    new CheckboxSetting("Place Up", true);
private final CheckboxSetting placeDown =
    new CheckboxSetting("Place Down", true);
private final CheckboxSetting placeHorizontal =
    new CheckboxSetting("Place Horizontal", true);

And modify the onUpdate method accordingly:

BlockHitResult blockHitResult = (BlockHitResult) hitResult;
Direction side = blockHitResult.getSide();

if ((side == Direction.UP && !placeUp.isChecked()) ||
    (side == Direction.DOWN && !placeDown.isChecked()) ||
    (side.getAxis().isHorizontal() && !placeHorizontal.isChecked()))
    return;

This gives users the flexibility to enable or disable placement in specific directions based on their needs.

Let me know your thoughts on these options!

Comment on lines +96 to +97
assert MC.interactionManager != null;
MC.interactionManager.interactItem(player, Hand.MAIN_HAND);
Copy link

@coderabbitai coderabbitai bot Oct 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Redundant call to interactItem.

The call to MC.interactionManager.interactItem(player, Hand.MAIN_HAND); is unnecessary in this context because you are already interacting with the block using interactBlock. This might lead to unintended item interactions.

Remove the redundant method call:

-    MC.interactionManager.interactItem(player, Hand.MAIN_HAND);
📝 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.

Suggested change
assert MC.interactionManager != null;
MC.interactionManager.interactItem(player, Hand.MAIN_HAND);
assert MC.interactionManager != null;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interactBlock and interactItem work in complicated and confusing ways. Sometimes both need to be called and sometimes even twice to match vanilla Minecraft behavior. I'd recommend instead using Wurst's utility net.wurstclient.util.InteractionSimulator whenever possible.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Alexander01998, thank you for the clarification. Using InteractionSimulator makes sense to ensure consistent behavior with vanilla Minecraft. I've updated the code to use InteractionSimulator:

@Override
public void onUpdate()
{
    if(MC.currentScreen instanceof HandledScreen)
        return;
    
    if(MC.itemUseCooldown != 0 && !fastPlace.isChecked())
        return;
    
    ClientPlayerEntity player = MC.player;
    assert player != null;
    
    // Item in hand is not a block
    if(!(player.getInventory().getStack(player.getInventory().selectedSlot)
        .getItem() instanceof BlockItem))
        return;
    
    HitResult hitResult = MC.crosshairTarget;
    if(hitResult == null || hitResult.getType() != HitResult.Type.BLOCK)
        return;
    
    BlockHitResult blockHitResult = (BlockHitResult)hitResult;
    if(blockHitResult.getSide() == Direction.UP
        || blockHitResult.getSide() == Direction.DOWN)
        return;
    
    if(!BlockUtils.canBeClicked(blockHitResult.getBlockPos()))
        return;
    
    BlockPos blockToPlacePos =
        blockHitResult.getBlockPos().offset(blockHitResult.getSide());
    if(!BlockUtils.getState(blockToPlacePos).isReplaceable())
        return;
    
    // Option: only below feet
    if(blockToPlacePos.getY() != BlockPos.ofFloored(MC.player.getPos())
        .down().getY() && onlyBelowFeet.isChecked())
        return;
    
    // Use InteractionSimulator to simulate block placement
    InteractionSimulator.rightClickBlock(blockHitResult, Hand.MAIN_HAND);
}

(•ᴗ•) A curious rabbit tilts its head in understanding.

Comment on lines +66 to +67
ClientPlayerEntity player = MC.player;
assert player != null;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid using assertions for null checks in production code.

Using assert statements for null checks is not recommended in production code because assertions may be disabled at runtime. Instead, consider implementing explicit null checks to prevent NullPointerException.

Apply this change:

-    assert player != null;
+    if (player == null)
+        return;
📝 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.

Suggested change
ClientPlayerEntity player = MC.player;
assert player != null;
ClientPlayerEntity player = MC.player;
if (player == null)
return;

Comment on lines +91 to +94
// Option: only below feet
if(blockToPlacePos.getY() != BlockPos.ofFloored(MC.player.getPos())
.down().getY() && onlyBelowFeet.isChecked())
return;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Simplify the condition for better readability.

The conditional statement can be simplified for clarity by checking onlyBelowFeet first before comparing positions.

Refactor the condition:

-    if(blockToPlacePos.getY() != BlockPos.ofFloored(MC.player.getPos())
-        .down().getY() && onlyBelowFeet.isChecked())
+    if (onlyBelowFeet.isChecked() && blockToPlacePos.getY() != MC.player.getBlockPos().down().getY())
📝 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.

Suggested change
// Option: only below feet
if(blockToPlacePos.getY() != BlockPos.ofFloored(MC.player.getPos())
.down().getY() && onlyBelowFeet.isChecked())
return;
// Option: only below feet
if (onlyBelowFeet.isChecked() && blockToPlacePos.getY() != MC.player.getBlockPos().down().getY())
return;

Comment on lines +66 to +103
ClientPlayerEntity player = MC.player;
assert player != null;

// Item in hand is not a block
if(!(player.getInventory().getStack(player.getInventory().selectedSlot)
.getItem() instanceof BlockItem))
return;

HitResult hitResult = MC.crosshairTarget;
if(hitResult == null || hitResult.getType() != HitResult.Type.BLOCK)
return;

BlockHitResult blockHitResult = (BlockHitResult)hitResult;
if(blockHitResult.getSide() == Direction.UP
|| blockHitResult.getSide() == Direction.DOWN)
return;

if(!BlockUtils.canBeClicked(blockHitResult.getBlockPos()))
return;

BlockPos blockToPlacePos =
blockHitResult.getBlockPos().offset(blockHitResult.getSide());
if(!BlockUtils.getState(blockToPlacePos).isReplaceable())
return;

// Option: only below feet
if(blockToPlacePos.getY() != BlockPos.ofFloored(MC.player.getPos())
.down().getY() && onlyBelowFeet.isChecked())
return;

assert MC.interactionManager != null;
MC.interactionManager.interactItem(player, Hand.MAIN_HAND);
ActionResult actionResult = MC.interactionManager.interactBlock(player,
Hand.MAIN_HAND, blockHitResult);
if(actionResult.isAccepted())
MC.player.swingHand(Hand.MAIN_HAND);

MC.itemUseCooldown = 4;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Handle potential NullPointerException for MC.interactionManager.

Similar to the player entity, MC.interactionManager could be null. Relying on assertions might not prevent exceptions in production.

Implement a null check:

-    assert MC.interactionManager != null;
+    if (MC.interactionManager == null)
+        return;
📝 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.

Suggested change
ClientPlayerEntity player = MC.player;
assert player != null;
// Item in hand is not a block
if(!(player.getInventory().getStack(player.getInventory().selectedSlot)
.getItem() instanceof BlockItem))
return;
HitResult hitResult = MC.crosshairTarget;
if(hitResult == null || hitResult.getType() != HitResult.Type.BLOCK)
return;
BlockHitResult blockHitResult = (BlockHitResult)hitResult;
if(blockHitResult.getSide() == Direction.UP
|| blockHitResult.getSide() == Direction.DOWN)
return;
if(!BlockUtils.canBeClicked(blockHitResult.getBlockPos()))
return;
BlockPos blockToPlacePos =
blockHitResult.getBlockPos().offset(blockHitResult.getSide());
if(!BlockUtils.getState(blockToPlacePos).isReplaceable())
return;
// Option: only below feet
if(blockToPlacePos.getY() != BlockPos.ofFloored(MC.player.getPos())
.down().getY() && onlyBelowFeet.isChecked())
return;
assert MC.interactionManager != null;
MC.interactionManager.interactItem(player, Hand.MAIN_HAND);
ActionResult actionResult = MC.interactionManager.interactBlock(player,
Hand.MAIN_HAND, blockHitResult);
if(actionResult.isAccepted())
MC.player.swingHand(Hand.MAIN_HAND);
MC.itemUseCooldown = 4;
ClientPlayerEntity player = MC.player;
assert player != null;
// Item in hand is not a block
if(!(player.getInventory().getStack(player.getInventory().selectedSlot)
.getItem() instanceof BlockItem))
return;
HitResult hitResult = MC.crosshairTarget;
if(hitResult == null || hitResult.getType() != HitResult.Type.BLOCK)
return;
BlockHitResult blockHitResult = (BlockHitResult)hitResult;
if(blockHitResult.getSide() == Direction.UP
|| blockHitResult.getSide() == Direction.DOWN)
return;
if(!BlockUtils.canBeClicked(blockHitResult.getBlockPos()))
return;
BlockPos blockToPlacePos =
blockHitResult.getBlockPos().offset(blockHitResult.getSide());
if(!BlockUtils.getState(blockToPlacePos).isReplaceable())
return;
// Option: only below feet
if(blockToPlacePos.getY() != BlockPos.ofFloored(MC.player.getPos())
.down().getY() && onlyBelowFeet.isChecked())
return;
if (MC.interactionManager == null)
return;
MC.interactionManager.interactItem(player, Hand.MAIN_HAND);
ActionResult actionResult = MC.interactionManager.interactBlock(player,
Hand.MAIN_HAND, blockHitResult);
if(actionResult.isAccepted())
MC.player.swingHand(Hand.MAIN_HAND);
MC.itemUseCooldown = 4;

@LilyKensa
Copy link
Author

wait a min did i just done something very stupid again

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.

Actionable comments posted: 11

🧹 Outside diff range and nitpick comments (14)
gradle.properties (1)

8-16: Overall assessment of gradle.properties changes

The changes in this file are mostly consistent with the PR objectives to update the mod for Minecraft 1.21. However, the Fabric API version downgrade needs further investigation to ensure compatibility and explain the reason for the change.

Consider implementing a version compatibility check in your build process to ensure that all dependencies are at their latest compatible versions. This could help prevent potential issues with version mismatches in the future.

src/main/java/net/wurstclient/settings/filters/FilterTeammatesSetting.java (2)

10-13: Consider using a constant for the filter name.

The constructor looks good overall. However, the hardcoded string "Filter teammates" might be better as a constant or configurable value. This would make maintenance easier and potentially support localization in the future.

Consider refactoring like this:

+ private static final String FILTER_NAME = "Filter teammates";

  public FilterTeammatesSetting(String description, boolean checked)
  {
-   super("Filter teammates", description, checked);
+   super(FILTER_NAME, description, checked);
  }

15-34: LGTM: Thorough null checks and correct teammate identification logic.

The test method is well-implemented with thorough null checks to prevent NullPointerExceptions. The logic for determining if an entity is a teammate based on name color is correct.

Consider caching the player's display name color to potentially improve performance, especially if this method is called frequently:

public final class FilterTeammatesSetting extends EntityFilterCheckbox
{
+   private TextColor cachedPlayerColor;

    // ... existing code ...

    @Override
    public boolean test(Entity e)
    {
        Text theirName = e.getDisplayName();
        if(theirName == null)
            return false;
        TextColor theirColor = theirName.getStyle().getColor();
        if(theirColor == null)
            return false;
        
        assert WurstClient.MC.player != null;
+       if (cachedPlayerColor == null) {
            Text myName = WurstClient.MC.player.getDisplayName();
            if(myName == null)
                return false;
-           TextColor myColor = myName.getStyle().getColor();
-           if(myColor == null)
+           cachedPlayerColor = myName.getStyle().getColor();
+           if(cachedPlayerColor == null)
                return false;
+       }
        
-       return !theirColor.equals(myColor);
+       return !theirColor.equals(cachedPlayerColor);
    }

    // ... existing code ...
}

This change would cache the player's color, potentially reducing the number of object creations and method calls in scenarios where this method is called frequently.

src/main/java/net/wurstclient/hacks/AutoReconnectHack.java (2)

22-25: LGTM: New CheckboxSetting added with clear description.

The new buttons setting aligns well with the PR objectives, allowing users to control the visibility of the reconnect button. The description is informative, and the default value of true ensures a good out-of-the-box experience.

Consider adding "when disconnected" to the end of the description for extra clarity:

 private final CheckboxSetting buttons = new CheckboxSetting(
 	"Reconnect screen button", "Shows a button on the reconnect"
-		+ " screen that lets you quickly enable AutoReconnect.",
+		+ " screen that lets you quickly enable AutoReconnect when disconnected.",
 	true);

38-41: LGTM: New method added to check button visibility preference.

The shouldShowButtons() method provides a clean interface for accessing the button visibility preference. The implementation is correct and the method name is descriptive.

For a minor optimization, you could make this method a one-liner:

 public boolean shouldShowButtons()
 {
-	return buttons.isChecked();
+	return buttons.isChecked();
 }

This change doesn't affect functionality but makes the code slightly more concise.

src/main/java/net/wurstclient/settings/filterlists/EntityFilterList.java (1)

65-66: LGTM! Consider grouping related filters for better readability.

The addition of FilterTeammatesSetting and FilterNpcLikeSetting enhances the entity filtering capabilities, which aligns well with the PR objectives. The placement and default values (false) are consistent with other filters.

To improve code organization and readability, consider grouping related filters together. For example, you could move these new filters next to other player-related filters like FilterSleepingSetting or FilterInvisibleSetting.

src/main/java/net/wurstclient/mixin/DisconnectedScreenMixin.java (1)

73-74: Approved: Improved control over reconnect button visibility

The addition of the conditional check before calling addReconnectButtons() is a good improvement. It ensures that the reconnect buttons are only displayed when appropriate, based on the state of the autoReconnectHack.

Consider extracting the condition to a descriptive method name for improved readability:

-if(WurstClient.INSTANCE.getHax().autoReconnectHack.shouldShowButtons())
+if(shouldShowReconnectButtons())
 	addReconnectButtons();

+private boolean shouldShowReconnectButtons() {
+	return WurstClient.INSTANCE.getHax().autoReconnectHack.shouldShowButtons();
+}

This change would make the code more self-documenting and easier to maintain.

src/main/java/net/wurstclient/hacks/AimAssistHack.java (2)

57-60: LGTM: New setting added correctly.

The onlyHoldingWeapon setting is well-implemented and aligns with the PR objectives. The description is clear and the default value is appropriate.

Consider adding a comment explaining why sticks are considered weapons, as it might not be immediately obvious to other developers.


156-162: LGTM: onUpdate method updated correctly.

The new check for the onlyHoldingWeapon setting is well-implemented and correctly enforces the new functionality. The early return optimizes performance when the condition is not met.

Consider extracting the weapon check into a separate method for better readability and potential reuse, e.g.:

private boolean isHoldingWeapon() {
    Item itemInHand = MC.player.getInventory().getMainHandStack().getItem();
    return itemInHand instanceof SwordItem
        || itemInHand instanceof AxeItem
        || itemInHand == Items.STICK;
}

Then use it in the onUpdate method:

if (onlyHoldingWeapon.isChecked() && !isHoldingWeapon()) {
    target = null;
    return;
}
src/main/java/net/wurstclient/settings/filters/FilterNpcLikeSetting.java (2)

23-43: Optimize the loop by avoiding unnecessary array creation

Creating an array with .toArray() can be inefficient. Iterate over the stream directly to improve performance.

Apply this diff to optimize the loop:

-int letters = 0, digits = 0, maxLetters = 0, maxDigits = 0;
-for(int c : name.chars().toArray())
+int letters = 0, digits = 0, maxLetters = 0, maxDigits = 0;
+IntStream stream = name.codePoints();
+PrimitiveIterator.OfInt iterator = stream.iterator();
+while (iterator.hasNext())
 {
     int c = iterator.nextInt();

Alternatively, you can use a for-each loop over the stream:

-for(int c : name.chars().toArray())
+for(int c : name.codePoints().toArray())

55-67: Consistent descriptions in factory methods

The descriptions in genericCombat and genericVision methods are identical. If there are differences in how these settings are used, consider updating the descriptions to reflect their specific purposes.

Apply this diff to clarify descriptions:

-return new FilterNpcLikeSetting(
-    "Won't attack NPC-like players (Sort by Hypixel NPC names.)",
+    "Won't attack NPC-like players (identifies NPCs by their names).",

And

-return new FilterNpcLikeSetting(
-    "Won't show NPC-like players (Sort by Hypixel NPC names.)",
+    "Won't show NPC-like players (identifies NPCs by their names).",
src/main/java/net/wurstclient/hacks/AutoPlaceHack.java (1)

38-42: Add a description to the hack for better clarity.

Including a description helps users understand the purpose and functionality of the AutoPlace hack within the client UI.

You can set the description in the constructor:

	super("AutoPlace");
	setDescription("Automatically places blocks when aiming at block sides, assisting in bridging.");
src/main/java/net/wurstclient/hacks/SafeWalkHack.java (1)

26-26: Typo in description for 'minDepth' setting

The description contains a typo: "Increase to stop SafeWalk from stucking on stairs." It should be "from getting stuck on stairs."

Apply this diff to correct the typo:

-"Increase to stop SafeWalk from stucking on stairs.\n"
+"Increase to stop SafeWalk from getting stuck on stairs.\n"
src/main/java/net/wurstclient/hacks/TriggerBotHack.java (1)

72-76: Consider renaming the setting for consistency

The setting is named "Black-listed Items" with a hyphen. For consistency with other settings and standard terminology, consider renaming it to "Blacklisted Items" without the hyphen.

Apply this diff to rename the setting:

 private final ItemListSetting blacklistedItems = new ItemListSetting(
-    "Black-listed Items",
+    "Blacklisted Items",
     "Will not trigger when these items are in your hand, so you will never mis-click when you are in the lobby.",
     "minecraft:compass", "minecraft:clock");
🛑 Comments failed to post (11)
gradle.properties (1)

13-13: 💡 Codebase verification

⚠️ Potential issue

Fabric API version downgraded below the latest version

The Fabric API version has been downgraded to 0.102.0+1.21, which is below both the previous version 0.105.0+1.21.1 and the latest version 0.106.0+1.21.2. This downgrade may introduce compatibility issues or omit important features and fixes.

🔗 Analysis chain

Verify Fabric API version downgrade

The Fabric API version has been changed from 0.105.0+1.21.1 to 0.102.0+1.21. This is a version downgrade, which is unusual for an update.

Please verify that this older version of Fabric API is compatible with all features of the mod and Minecraft 1.21. Consider checking the Fabric API changelog to ensure no critical features or fixes are being missed. If possible, explain the reason for downgrading.

To verify the latest compatible version, you can run:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check latest Fabric API version for Minecraft 1.21
curl -s https://api.modrinth.com/v2/project/fabric-api/version | jq -r '.[] | select(.game_versions | contains(["1.21"])) | .version_number' | sort -V | tail -n 1

Length of output: 179

src/main/java/net/wurstclient/settings/filters/FilterNpcLikeSetting.java (3)

45-51: ⚠️ Potential issue

Reevaluate the filtering conditions for better accuracy

The conditions at the end of the test method may not accurately classify NPC-like names. For example, names with certain patterns might bypass the filter unintentionally. Consider reviewing and refining these conditions to ensure they match the intended criteria for NPC-like player names.

If possible, provide examples of player names that should be filtered and adjust the conditions accordingly.


24-43: 🛠️ Refactor suggestion

Use codePoints() instead of chars() to handle Unicode characters correctly

Using name.chars() processes characters as UTF-16 code units, which may not correctly handle characters outside the Basic Multilingual Plane (BMP). Replace name.chars() with name.codePoints() to operate on Unicode code points and ensure accurate character processing.

Apply this diff to use codePoints():

-for(int c : name.chars().toArray())
+for(int c : name.codePoints().toArray())
📝 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.

		for(int c : name.codePoints().toArray())
		{
			if(Character.isDigit(c))
			{
				letters = 0;
				digits += 1;
			}else if(Character.isLetter(c) && Character.isLowerCase(c))
			{
				digits = 0;
				letters += 1;
			}else
			{
				return true;
			}
			
			if(letters > maxLetters)
				maxLetters = letters;
			if(digits > maxDigits)
				maxDigits = digits;
		}

24-43: ⚠️ Potential issue

Handle uppercase letters and special characters in player names

The test method only considers lowercase letters due to the condition Character.isLowerCase(c). Player names with uppercase letters or special characters will result in return true, possibly excluding legitimate players from the filter. Consider modifying the condition to include uppercase letters and handle special characters appropriately.

Apply this diff to include uppercase letters:

-}else if(Character.isLetter(c) && Character.isLowerCase(c))
+}else if(Character.isLetter(c))

Additionally, if special characters are allowed in player names, adjust the logic to account for them, or explicitly handle them based on your requirements.

Committable suggestion was skipped due to low confidence.

src/main/java/net/wurstclient/hacks/AutoPlaceHack.java (1)

70-72: 🛠️ Refactor suggestion

Simplify item in hand check for better readability.

You can use player.getMainHandStack() to retrieve the item in the player's main hand, making the code more concise.

Refactor the code as follows:

-if(!(player.getInventory().getStack(player.getInventory().selectedSlot)
-    .getItem() instanceof BlockItem))
+if (!(player.getMainHandStack().getItem() instanceof BlockItem))
    return;
📝 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 (!(player.getMainHandStack().getItem() instanceof BlockItem))
			return;
src/main/java/net/wurstclient/hacks/SafeWalkHack.java (1)

71-82: 🛠️ Refactor suggestion

Refactor to eliminate code duplication between 'shouldClip()' and 'onClipAtLedge()'

Both methods shouldClip() and onClipAtLedge() perform similar calculations for adjustedBox and check MC.world.isSpaceEmpty(player, adjustedBox). Consider refactoring to avoid code duplication by reusing shouldClip() within onClipAtLedge().

Apply this diff to refactor the code:

 public boolean shouldClip()
 {
     ClientPlayerEntity player = MC.player;
     
     Box box = player.getBoundingBox();
     Box adjustedBox = box
         .offset(player.getVelocity().multiply(motionPrediction.getValue()))
         .stretch(0, -minDepth.getValue(), 0)
         .expand(-edgeDistance.getValue(), 0, -edgeDistance.getValue());
     
     return MC.world.isSpaceEmpty(player, adjustedBox);
 }

 public void onClipAtLedge()
 {
     ClientPlayerEntity player = MC.player;
     
     if(!isEnabled() || !sneak.isChecked() || !player.isOnGround())
     {
         if(sneaking)
             setSneaking(false);
         
         return;
     }
     
     setSneaking(shouldClip());
 }

This refactoring removes the duplicated code by utilizing shouldClip() within onClipAtLedge(), enhancing maintainability.

Also applies to: 84-102

src/main/java/net/wurstclient/hacks/TriggerBotHack.java (3)

166-167: 🛠️ Refactor suggestion

Improve performance by using a Set for blacklist items

If blacklistedItems.getItemNames() returns a List, the contains method has a linear time complexity. Converting the item names to a Set will improve the performance of the contains checks, especially if the blacklist grows.

Consider updating the ItemListSetting to use a Set internally or cache the Set representation:

// In ItemListSetting class
public Set<String> getItemNameSet() {
    return new HashSet<>(itemNames);
}

Then, use getItemNameSet() in your check:

-if(blacklistedItems.getItemNames().contains(itemName))
+if(blacklistedItems.getItemNameSet().contains(itemName))
     return;

72-76: ⚠️ Potential issue

Fix grammatical errors in the setting description

There are grammatical mistakes in the description of the blacklistedItems setting:

  • "these item" should be "these items"
  • "in lobby" should be "in the lobby"

Apply this diff to correct the description:

 private final ItemListSetting blacklistedItems = new ItemListSetting(
-    "Black-listed Items",
-    "Will not trigger when these item are in your hand, so you will never mis-click when you are in lobby.",
+    "Blacklisted Items",
+    "Will not trigger when these items are in your hand, so you will never mis-click when you are in the lobby.",
     "minecraft:compass", "minecraft:clock");
📝 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.

	private final ItemListSetting blacklistedItems = new ItemListSetting(
		"Blacklisted Items",
		"Will not trigger when these items are in your hand, so you will never mis-click when you are in the lobby.",
		"minecraft:compass", "minecraft:clock");
	

156-168: 🛠️ Refactor suggestion

Simplify the item blacklist checking logic

The current logic for checking the blacklisted items can be simplified to improve readability and maintainability. By assigning the item name in both cases (when the stack is empty or not), you can perform a single check against the blacklist.

Apply this diff to streamline the code:

 ItemStack stack = MC.player.getInventory().getMainHandStack();

-if(stack.isEmpty())
-{
-    if(blacklistedItems.getItemNames().contains("minecraft:air"))
-        return;
-}
-else
-{
-    Item item = stack.getItem();
-    String itemName = Registries.ITEM.getId(item).toString();
-
-    if(blacklistedItems.getItemNames().contains(itemName))
-        return;
-}
+
+String itemName = stack.isEmpty()
+    ? "minecraft:air"
+    : Registries.ITEM.getId(stack.getItem()).toString();
+
+if(blacklistedItems.getItemNames().contains(itemName))
+    return;
📝 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.

		ItemStack stack = MC.player.getInventory().getMainHandStack();

		String itemName = stack.isEmpty()
			? "minecraft:air"
			: Registries.ITEM.getId(stack.getItem()).toString();

		if(blacklistedItems.getItemNames().contains(itemName))
			return;
		
src/main/java/net/wurstclient/hacks/TeamEspHack.java (2)

46-47: 🛠️ Refactor suggestion

Consider refactoring to eliminate code duplication

The comment on line 46 indicates that this class is "pretty much just cloned from PlayerEspHack.java". To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, consider abstracting shared functionality into a base class or utility methods. This would reduce code duplication and make it easier to implement future changes or fixes across both hacks.


168-177: ⚠️ Potential issue

Correct RGB color extraction for accurate team colors

The current method of extracting RGB components from teamColor may not produce the correct colors due to integer division and modulus operations. This can lead to incorrect coloring of the ESP boxes and tracers.

Apply the following diff to correctly extract the RGB values using bitwise operations:

@@ -168,10 +168,7 @@
    int teamColor = colorComponent.getRgb();

-   b = (float)(teamColor % 256);
-   g = (float)(teamColor % 65536 / 256);
-   r = (float)(teamColor / 65536);
-
-   b /= 256;
-   g /= 256;
-   r /= 256;
+   r = ((teamColor >> 16) & 0xFF) / 255.0f;
+   g = ((teamColor >> 8) & 0xFF) / 255.0f;
+   b = (teamColor & 0xFF) / 255.0f;

Repeat the same correction in the renderTracers method at lines 218-227 to ensure consistency.

Also applies to: 218-227

@LilyKensa
Copy link
Author

wait what i didn't do that merge

@LilyKensa
Copy link
Author

bro no way, every time I think "oh I finally learned how to use git" it just slaps me in the face and says "no you don't"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants