Split NoSlowdown with checkboxes #800
Conversation
|
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:
|
|
Still relevant. |
|
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:
|
WalkthroughWalkthroughEnhancements to the Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- src/main/java/net/wurstclient/hacks/NoSlowdownHack.java (1 hunks)
- src/main/java/net/wurstclient/mixin/BlockMixin.java (1 hunks)
- src/main/java/net/wurstclient/mixin/ClientPlayerEntityMixin.java (1 hunks)
Additional comments: 11
src/main/java/net/wurstclient/hacks/NoSlowdownHack.java (5)
19-22: The addition of
blocksanditemssettings is consistent with the PR's objective to allow users to selectively enable or disable slowdown prevention. Ensure that the naming and default values align with the project's standards and user expectations.32-34: The
noBlockSlownessmethod correctly checks both if the hack is enabled and if theblockssetting is checked. This is a logical implementation for the intended functionality.37-39: The
noItemSlownessmethod's logic is sound, checking for theALLvalue in theitemssetting to determine if item slowness should be prevented.42-44: The
noNonBlockingItemSlownessmethod is implemented correctly, checking for theEXCEPT_BLOCKINGvalue in theitemssetting. This allows for the specific use case where blocking item slowness is not prevented.47-64: The
ItemSlownessenum is well-defined with clear naming for each value, and thetoStringmethod is overridden to return the human-readable name. This is a good practice for readability and maintainability.src/main/java/net/wurstclient/mixin/BlockMixin.java (1)
- 47-57: The change to use
noBlockSlownessinstead ofisEnabledin theonGetVelocityMultipliermethod aligns with the new settings introduced in theNoSlowdownHackclass. This ensures that the velocity multiplier is only adjusted when block slowness prevention is enabled.src/main/java/net/wurstclient/mixin/ClientPlayerEntityMixin.java (5)
73-81: The additional conditional checks in the
onTickMovementItemUsemethod correctly implement the new item slowness settings. It ensures that the slowdown is prevented based on the user's configuration.103-109: The
afterIsUsingItemmethod correctly resets thehideNextItemUseflag. This is important to ensure that the flag is only active for the duration it's needed and does not affect subsequent logic.144-154: The handling of the current screen in
beforeUpdateNauseato prevent it from closing when PortalGUI is enabled seems correct. However, ensure that this does not cause any issues with other parts of the game that might expect the current screen to be non-null.178-182: The
onCanSprintmethod modification to allow sprinting when the player is too hungry aligns with the functionality of an AutoSprint hack. Ensure that this behavior is clearly documented and communicated to users, as it deviates from the game's default behavior.274-289: The
hasStatusEffectmethod's additional conditional checks for specific status effects are implemented correctly. They allow the associated hacks to override the default status effect behavior, which is the intended functionality.
|
Hi @ThisTestUser, thanks for the pull request! It seems there are some format violations that need to be fixed. If you're using Eclipse, please make sure to use the cleanup and format settings located in the codestyle folder. This will help ensure consistency with the existing codebase. Alternatively, if you're not using Eclipse, you can also fix these violations by running the following command from the command line: Once you've done this, please commit the changes and push them to your branch. This should cause the check to pass and allow your pull request to be merged. (The github-actions bot was supposed to post this comment automatically, but something is wrong with its permissions.) |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- src/main/java/net/wurstclient/hacks/NoSlowdownHack.java (1 hunks)
- src/main/java/net/wurstclient/mixin/BlockMixin.java (1 hunks)
- src/main/java/net/wurstclient/mixin/ClientPlayerEntityMixin.java (2 hunks)
Additional comments: 9
src/main/java/net/wurstclient/hacks/NoSlowdownHack.java (6)
19-21: The addition of the
blocksCheckboxSetting is clear and well-documented, providing a description for the user interface.23-24: The
itemsEnumSetting is introduced correctly with a clear description and default value. It's important to ensure that the enum values are used consistently throughout the codebase.Verification successful
I will generate a new script to search for references to the
ItemSlownessenum throughout the codebase to ensure that it is being used consistently.
The search for references to the
ItemSlownessenum in the codebase has produced several results, indicating that the enum is indeed used in various parts of the code. Here are the findings:
- The
ItemSlownessenum is declared inNoSlowdownHack.javaand is used to initialize theitemsEnumSetting with a default value ofItemSlowness.ALL.- Methods
noItemSlowness()andnoItemSlownessExceptShields()inNoSlowdownHack.javause theItemSlownessenum to check the selected item slowness setting.ClientPlayerEntityMixin.javareferences methods fromNoSlowdownHackthat utilize theItemSlownessenum.Based on the output, the
ItemSlownessenum values are being used consistently within theNoSlowdownHackclass and are referenced in theClientPlayerEntityMixinclass. There are no indications of inconsistent usage or undefined references to the enum values.* 34-36: The `noBlockSlowness` method correctly checks both if the hack is enabled and if the blocks setting is checked. This is a simple and effective way to handle the conditional logic.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the enum values are used consistently in the codebase. ast-grep --lang java --pattern $'ItemSlowness.$_'Length of output: 50
Script:
#!/bin/bash # Search for references to the ItemSlowness enum throughout the codebase. rg --type java 'ItemSlowness'Length of output: 1117
- 39-41: The
noItemSlownessmethod is implemented correctly, checking for theALLenum value. It's important to ensure that the method is used correctly wherever item slowness should be considered.Verification successful
To verify the usage of the
noItemSlowness()method more thoroughly, I will generate a new shell script usingrg(ripgrep), which is a line-oriented search tool that recursively searches the current directory for a regex pattern. This will help us to find any instance of the method being used, even if it's part of a more complex expression.
The
rgcommand has successfully identified two occurrences of thenoItemSlowness()method:
- Within its own declaration in the
NoSlowdownHack.javafile.- In the
ClientPlayerEntityMixin.javafile, where it is being called.This confirms that the method is indeed being used in the codebase, specifically within the
ClientPlayerEntityMixin.javafile. The usage inClientPlayerEntityMixin.javasuggests that the method is being used to determine behavior based on thenoItemSlownesssetting.Given this information, the method's implementation and usage have been verified as correct based on the provided context and the search results.
* 44-47: The `noItemSlownessExceptShields` method is implemented correctly, checking for the `EXCEPT_SHIELDS` enum value. This method should be used in contexts where shields are an exception to the no slowness rule.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the noItemSlowness method is used correctly in the codebase. ast-grep --lang java --pattern $'noItemSlowness()'Length of output: 51
Script:
#!/bin/bash # Search for the usage of noItemSlowness method in the codebase. rg "noItemSlowness\(\)"Length of output: 246
Verification successful
The
rgcommand has found two occurrences of thenoItemSlownessExceptShieldsmethod:
- Within the
NoSlowdownHack.javafile, which is the declaration of the method itself.- Within the
ClientPlayerEntityMixin.javafile, which indicates that the method is indeed being used in the codebase.Based on this information, we can confirm that the
noItemSlownessExceptShieldsmethod is implemented and used within the codebase, specifically in theClientPlayerEntityMixin.javafile. This aligns with the review comment that the method should be used in contexts where shields are an exception to the no slowness rule.* 50-67: The `ItemSlowness` enum is well-defined with clear names and a proper `toString` method. It's important to ensure that the enum is not used in a way that could cause unexpected behavior.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the noItemSlownessExceptShields method is used correctly in the codebase. ast-grep --lang java --pattern $'noItemSlownessExceptShields()'Length of output: 64
Script:
#!/bin/bash # Search for the usage of noItemSlownessExceptShields method in the codebase. rg "noItemSlownessExceptShields"Length of output: 234
Verification successful
The search for occurrences of the
ItemSlownessenum in the codebase has produced several results:
- Usage in
ClientPlayerEntityMixin.javafor methodsnoItemSlowness()andnoItemSlownessExceptShields().- Declaration and usage within
NoSlowdownHack.javafor managing item slowness settings and conditional logic.Based on the output, the
ItemSlownessenum is used in the expected manner to control the behavior of the NoSlowdown feature, with methods checking the selected enum value to determine the appropriate behavior. There are no indications from this output that the enum is used in a way that could cause unexpected behavior.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the ItemSlowness enum is not used in a way that could cause unexpected behavior. ast-grep --lang java --pattern $'ItemSlowness.$_'Length of output: 50
Script:
#!/bin/bash # Search for usages of the ItemSlowness enum throughout the codebase. ast-grep --lang java --pattern $'enum $_ { $$$ }' --exec rg 'ItemSlowness'Length of output: 267
Script:
#!/bin/bash # Search for occurrences of the ItemSlowness enum in the codebase. rg 'ItemSlowness' -tjavaLength of output: 1112
src/main/java/net/wurstclient/mixin/BlockMixin.java (1)
- 53-54: The change in the
onGetVelocityMultipliermethod to checknoBlockSlownessinstead ofisEnabledis consistent with the new settings introduced in theNoSlowdownHackclass. This ensures that the velocity multiplier is only adjusted when block slowness cancellation is specifically enabled.src/main/java/net/wurstclient/mixin/ClientPlayerEntityMixin.java (2)
29-29: The import of
UseActionis necessary for the new logic added to theonTickMovementItemUsemethod. It's important to ensure that this import is used correctly in the context of the changes.79-84: The addition of boolean variables
noSlowdownandnoSlowdownShieldin theonTickMovementItemUsemethod is correct. They are used to determine whether to apply item use slowdown based on the new settings. The logic appears sound, but it's important to verify that the conditions are correctly applied in the game's context.Verification successful
The new context from the shell scripts provides the necessary information to verify the implementation of the
noSlowdownandnoSlowdownShieldconditions within theonTickMovementItemUsemethod:
- The
noSlowdownboolean is set based on thenoItemSlowness()method from thenoSlowdownHack.- The
noSlowdownShieldboolean is set based on thenoItemSlownessExceptShields()method from thenoSlowdownHackand checks if the current active item's use action is notBLOCK.- The conditional
if(noSlowdown || noSlowdownShield)then setshideNextItemUsetotrueif either condition is met.This logic aligns with the PR's objective to allow separate toggling of slowdown prevention for items and shields. The conditions are applied correctly in the context of the game, as they check the specific settings before applying the slowdown effect.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the conditions for noSlowdown and noSlowdownShield are correctly applied in the game's context. ast-grep --lang java --pattern $'onTickMovementItemUse($_)'Length of output: 60
Script:
#!/bin/bash # Search for the onTickMovementItemUse method within the ClientPlayerEntityMixin class to verify the conditions. ast-grep --lang java --pattern $'class ClientPlayerEntityMixin { $$$ void onTickMovementItemUse($_) { $$$ } $$$ }'Length of output: 127
Script:
#!/bin/bash # Use ripgrep to search for the onTickMovementItemUse method in the codebase. rg 'void onTickMovementItemUse' -C 3Length of output: 802
Script:
#!/bin/bash # Display more lines of the onTickMovementItemUse method to verify the conditions for noSlowdown and noSlowdownShield. rg 'void onTickMovementItemUse' -C 10 -A 10 src/main/java/net/wurstclient/mixin/ClientPlayerEntityMixin.javaLength of output: 537
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/main/java/net/wurstclient/mixin/ClientPlayerEntityMixin.java (2 hunks)
Additional comments: 2
src/main/java/net/wurstclient/mixin/ClientPlayerEntityMixin.java (2)
29-29: The addition of the
UseActionimport is necessary for the new condition in theonTickMovementItemUsemethod.79-83: The new condition in the
onTickMovementItemUsemethod correctly implements the feature to prevent slowdown when using items, except for shields. This aligns with the PR objectives to allow more granular control over the NoSlowdown feature.
|
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:
|
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/main/java/net/wurstclient/hacks/NoSlowdownHack.java (1 hunks)
Additional comments: 3
src/main/java/net/wurstclient/hacks/NoSlowdownHack.java (3)
- 19-31: The addition of
CheckboxSettingfor blocks andEnumSettingfor items introduces more granular control over the NoSlowdown feature, allowing users to customize their experience based on the type of slowness they wish to cancel. This is a positive change that enhances the hack's functionality and user control. However, ensure that the UI elements for these settings are properly integrated and tested within the client's settings menu.- 34-48: The methods
noBlockSlowness,noItemSlowness, andnoItemSlownessExceptShieldsprovide a clear and concise way to check the conditions for cancelling slowness effects. It's good to see that these methods are leveraging the settings introduced earlier. However, it's important to ensure that these methods are called in the appropriate places within the codebase to effectively apply the NoSlowdown settings. Additionally, consider adding a method comment explaining the purpose and usage of each method for better maintainability.- 50-67: The
ItemSlownessenum is well-defined, with clear options for different item slowness behaviors. Using an enum for this purpose is a good choice as it makes the code more readable and maintainable. However, ensure that all relevant items are considered when implementing the logic forEXCEPT_SHIELDS. It might be beneficial to document which items are affected by eachItemSlownessoption, either in the code comments or in the user documentation, to provide clarity to the users.
|
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:
|
There was a problem hiding this comment.
Actionable comments posted: 1
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/main/java/net/wurstclient/mixin/ClientPlayerEntityMixin.java (2 hunks)
Additional Context Used
Learnings (1)
src/main/java/net/wurstclient/mixin/ClientPlayerEntityMixin.java (1)
User: Alexander01998 PR: Wurst-Imperium/Wurst7#772 File: src/main/java/net/wurstclient/mixin/PlayerEntityMixin.java:22-22 Timestamp: 2023-12-15T11:22:20.950Z Learning: Preference for using `@Inject` over `@Redirect` mixins in Minecraft modding to avoid compatibility issues has been noted. This will influence future code reviews where Mixin libraries are used.
Additional comments not posted (1)
src/main/java/net/wurstclient/mixin/ClientPlayerEntityMixin.java (1)
33-33: Import ofnet.minecraft.util.UseActionadded.This import is necessary for the new logic in
onTickMovementItemUseto function correctly, as it checks theUseActionof the item being used.
| if(WurstClient.INSTANCE.getHax().noSlowdownHack.noItemSlowness() | ||
| || (WurstClient.INSTANCE.getHax().noSlowdownHack | ||
| .noItemSlownessExceptShields() | ||
| && ((ClientPlayerEntity)(Object)this).getActiveItem() | ||
| .getUseAction() != UseAction.BLOCK)) |
There was a problem hiding this comment.
Logic to selectively prevent item slowness based on new settings.
Consider breaking this complex conditional into multiple lines or extracting it into a method for better readability and maintainability.
- if(WurstClient.INSTANCE.getHax().noSlowdownHack.noItemSlowness()
- || (WurstClient.INSTANCE.getHax().noSlowdownHack
- .noItemSlownessExceptShields()
- && ((ClientPlayerEntity)(Object)this).getActiveItem()
- .getUseAction() != UseAction.BLOCK))
- hideNextItemUse = true;
+ boolean shouldHideItemUse = WurstClient.INSTANCE.getHax().noSlowdownHack.noItemSlowness()
+ || (WurstClient.INSTANCE.getHax().noSlowdownHack.noItemSlownessExceptShields()
+ && getActiveItemUseAction() != UseAction.BLOCK);
+ hideNextItemUse = shouldHideItemUse;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.
| if(WurstClient.INSTANCE.getHax().noSlowdownHack.noItemSlowness() | |
| || (WurstClient.INSTANCE.getHax().noSlowdownHack | |
| .noItemSlownessExceptShields() | |
| && ((ClientPlayerEntity)(Object)this).getActiveItem() | |
| .getUseAction() != UseAction.BLOCK)) | |
| boolean shouldHideItemUse = WurstClient.INSTANCE.getHax().noSlowdownHack.noItemSlowness() | |
| || (WurstClient.INSTANCE.getHax().noSlowdownHack.noItemSlownessExceptShields() | |
| && getActiveItemUseAction() != UseAction.BLOCK); | |
| hideNextItemUse = shouldHideItemUse; |
There was a problem hiding this comment.
@coderabbitai Good idea, bad example. Creating a local variable for WurstClient.INSTANCE.getHax().noSlowdownHack would improve readability in this case.
But also, it seems odd that there are two separate methods noItemSlowness() and noItemSlownessExceptShields() here that both need to be called together like that. A better solution might be to have a single noItemSlowness(boolean shield) method, which would handle the special case for shields internally.
There was a problem hiding this comment.
@Alexander01998, you're absolutely right on both counts.
Creating a local variable for WurstClient.INSTANCE.getHax().noSlowdownHack does indeed make the code cleaner and more readable by reducing redundancy. Additionally, your suggestion to combine noItemSlowness() and noItemSlownessExceptShields() into a single method that internally handles the shield case is an excellent idea. It not only simplifies the calling code but also encapsulates the logic related to item slowness in a more maintainable way.
Here's how the revised code might look:
NoSlowdownHack noSlowdownHack = WurstClient.INSTANCE.getHax().noSlowdownHack;
boolean shouldHideItemUse = noSlowdownHack.noItemSlowness(getActiveItemUseAction() != UseAction.BLOCK);
hideNextItemUse = shouldHideItemUse;In this version, noItemSlowness(boolean shield) would internally decide the logic based on whether the active item's use action is BLOCK, simplifying the external checks.
Thank you for your insightful suggestions!
New Learnings Added
User: Alexander01998
PR: Wurst-Imperium/Wurst7#800
File: src/main/java/net/wurstclient/mixin/ClientPlayerEntityMixin.java:100-104
Timestamp: 2024-05-07T00:00:25.901Z
Learning: Alexander01998 suggested improving readability by creating a local variable for `WurstClient.INSTANCE.getHax().noSlowdownHack` and combining `noItemSlowness()` and `noItemSlownessExceptShields()` into a single method `noItemSlowness(boolean shield)`.
Note: Learnings are effective only in the context of specific code segments. To apply general coding guidelines, please configure review instructions.
There was a problem hiding this comment.
I was thinking more like this:
// The cast to Object and back is necessary because this code is inside of a mixin.
boolean shield = ((ClientPlayerEntity)(Object)this).getActiveItem().getUseAction() == UseAction.BLOCK;
if(WurstClient.INSTANCE.getHax().noSlowdownHack.noItemSlowness(shield))
hideNextItemUse = true;Or, if keeping the two methods:
NoSlowdownHack noSlowdown = WurstClient.INSTANCE.getHax().noSlowdownHack;
boolean shield = ((ClientPlayerEntity)(Object)this).getActiveItem().getUseAction() == UseAction.BLOCK;
if(noSlowdown.noItemSlowness() || (noSlowdown.noItemSlownessExceptShields() && shield)
hideNextItemUse = true;But the first solution is much cleaner.
|
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:
|
Splits the various features of NoSlowdown (honey and soul sand slowdown, item slowdown) with settings. This is useful because some anticheats only block NoSlowdown when using shields.
Summary by CodeRabbit