-
Notifications
You must be signed in to change notification settings - Fork 58
Fix for cant disable sprint scaffold issue #847 #850
Conversation
Added logo_small so that you can have the logo in the Minecraft Launcher.
This was annoying me really bad, so I decided to go ahead and fix it. Added: (sprint module): Disable While Scaffold Added: (scaffold module): Cancel Sprint At Start NOTE: Double-tapping w to sprint still works for some reason. If anyone knows why this happens and how to stop it please feel free to edit the pr.
Caution Review failedThe pull request is closed. 📝 Walkthrough📝 WalkthroughWalkthroughThe changes involve modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant Player
participant Sprint
participant Scaffold
Player->>Scaffold: Enable Scaffold Module
alt cancelSprintAtStart is toggled
Scaffold->>Player: Set sprinting state to false
end
Player->>Sprint: Check if sprinting is allowed
alt disableWhileScaffold is toggled
Sprint->>Player: Set sprint key state to false
else
Sprint->>Player: Set sprint key state to true
end
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
Documentation and Community
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
⛔ Files ignored due to path filters (1)
logo/logo_small.png
is excluded by!**/*.png
📒 Files selected for processing (2)
src/main/java/keystrokesmod/module/impl/movement/Sprint.java
(3 hunks)src/main/java/keystrokesmod/module/impl/world/Scaffold.java
(3 hunks)
🔇 Additional comments (7)
src/main/java/keystrokesmod/module/impl/movement/Sprint.java (4)
7-7
: LGTM!
The ButtonSetting import is correctly added to support the new setting.
20-20
: LGTM!
The field is properly declared with appropriate access modifiers and a descriptive name.
49-56
:
Add null check and consider other sprint conditions.
There are several potential issues with this implementation:
- Missing null check for
ModuleManager.scaffold
- The else branch unconditionally enables sprinting, which might override other sprint conditions
- Inconsistent indentation
Consider this safer implementation:
- if(ModuleManager.scaffold.isEnabled() && disableWhileScaffold.isToggled()) {
- KeyBinding.setKeyBindState(mc.gameSettings.keyBindSprint.getKeyCode(), false);
- }
- else {
- KeyBinding.setKeyBindState(mc.gameSettings.keyBindSprint.getKeyCode(), true);
- }
+ boolean shouldDisableSprint = ModuleManager.scaffold != null
+ && ModuleManager.scaffold.isEnabled()
+ && disableWhileScaffold.isToggled();
+
+ if (shouldDisableSprint) {
+ KeyBinding.setKeyBindState(mc.gameSettings.keyBindSprint.getKeyCode(), false);
+ } else {
+ // Preserve existing sprint state if not explicitly disabled
+ boolean canSprint = MoveUtil.isMoving() && !stopSprint;
+ KeyBinding.setKeyBindState(mc.gameSettings.keyBindSprint.getKeyCode(), canSprint);
+ }
Let's verify the scaffold module initialization:
Line range hint 63-77
: Verify interaction with omni mode.
The new scaffold-related sprint controls might interfere with the omni mode functionality. Please verify that the changes don't affect sprinting behavior when in omni mode.
✅ Verification successful
Based on the search results, I can see that the Sprint module's omni mode implementation is properly handled in the switch statement and doesn't interfere with the scaffold-related sprint controls. The omni mode functionality is contained within its own switch block and only modifies the yaw when enabled, while the sprint control is handled separately after the switch block. Here's my final response:
The omni mode implementation is safe and independent of scaffold controls
The omni mode functionality is properly encapsulated in its own switch block (lines 64-70) and only affects yaw rotation when enabled. The sprint control that follows (lines 72-73) is independent of this logic and won't interfere with omni mode's operation. The changes maintain clean separation of concerns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential interactions between omni mode and scaffold
ast-grep --pattern 'case $_:
$$$
mc.thePlayer.setSprinting($_);
$$$'
Length of output: 76
Script:
#!/bin/bash
# Let's try a different approach to analyze the omni mode implementation
# First, let's find all references to omni mode
rg -i "omnimode" -B 2 -A 2
# Also check for sprint-related code
rg "setSprinting" -B 2 -A 2
# And check for any switch statements related to sprint modes
ast-grep --pattern 'switch ($$$) {
$$$
case $_:
$$$
}'
Length of output: 123819
src/main/java/keystrokesmod/module/impl/world/Scaffold.java (3)
96-96
: LGTM! Field declaration follows class conventions.
The new cancelSprintAtStart
setting is properly declared as private final and follows the existing naming pattern.
246-252
: LGTM! Implementation is clean and effective.
The code properly implements the sprint cancellation functionality:
- Checks if the setting is enabled
- Disables sprinting if needed
- Uses proper bracing and follows the code style
250-251
: Verify interaction with Sprint module's disableWhileScaffold setting.
Let's ensure there are no conflicts between this new setting and the Sprint module's disableWhileScaffold
setting.
//Only needs to work in legit mode to // This setting is only applicable in legit mode as omni mode handles sprinting differently Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range comments (1)
src/main/java/keystrokesmod/module/impl/movement/Sprint.java (1)
Line range hint
1-85
: Document module interactions.Consider adding class-level documentation explaining the interaction between Sprint and Scaffold modules, particularly how the
disableWhileScaffold
setting works in conjunction with Scaffold'scancelSprintAtStart
setting. This would help future maintainers understand the relationship between these modules.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
📒 Files selected for processing (1)
src/main/java/keystrokesmod/module/impl/movement/Sprint.java
(3 hunks)
🔇 Additional comments (2)
src/main/java/keystrokesmod/module/impl/movement/Sprint.java (2)
7-7
: LGTM!
The ButtonSetting import is correctly added and necessary for the new functionality.
20-20
: LGTM!
The field declaration follows good practices with appropriate access modifiers and naming.
Description
When the "sprint" module is enabled (particularly the "legit" mode), it effectively overrides whatever is set in the sprint options in "scaffold". While this is fine for the sprint scaffolds, if you don't want a sprint scaffold then you would be out of luck, as enabling scaffold also normally preserves your sprint state. So I added 2 options, one to stop sprint from overriding scaffold's options and another to remove the sprint state when scaffold is first aenabled.
Testing
References
Summary by CodeRabbit
New Features
Bug Fixes