Hotfix/webwalker fixes - #1837
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe walking task now loops while logged in and processes target state changes and terminal walker states. Nearby reachable targets can use canvas clicks before minimap fallback. Mouse clicks include randomized press-release timing, and shape movement uses helper-selected points. Run-energy toggling uses randomized activation thresholds. Transport preparation, camera delays, direct-walk state handling, and door-await timings were updated. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/player/Rs2Player.java (1)
413-442: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
toggleRunEnergyjavadoc no longer matches its return contract.The javadoc states
falseis returned only "if the run energy toggle widget was not found." The new threshold check at line 433 also returnsfalsewhengetRunEnergy() < nextRunEnergyThreshold, a case unrelated to the widget. A caller that treatsfalseas "widget missing" per the documented contract cannot distinguish that from "deliberately not enabling run yet."Update the javadoc to document the new early-return case.
📝 Proposed javadoc fix
/** * Toggles the player's run energy on or off. * * `@param` toggle {`@code` true} to enable running, {`@code` false} to disable it. - * `@return` {`@code` true} if the toggle action was performed successfully or was already in the desired state, - * {`@code` false} if the run energy toggle widget was not found. + * `@return` {`@code` true} if the toggle action was performed successfully or was already in the desired state, + * {`@code` false} if the run energy toggle widget was not found, or (when {`@code` toggle} is + * {`@code` true}) if run energy is below the currently randomized activation threshold. */🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/player/Rs2Player.java` around lines 413 - 442, Update the javadoc for toggleRunEnergy(boolean) to document that it also returns false when enabling is requested but getRunEnergy() is below nextRunEnergyThreshold, in addition to the missing-widget case. Keep the existing behavior and return conditions unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/ShortestPathScript.java`:
- Around line 98-103: Update the banked-transport branch in the
ShortestPathScript walk-state flow to use the configured reached-distance source
instead of the hardcoded 10. Ensure Rs2Walker.walkWithBankedTransportsAndState
receives the same reachedDistanceOrDefault()-based value honored by
Rs2Walker.walkWithState(target), preserving consistent arrival thresholds across
both branches.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/door/Rs2WalkerAwaits.java`:
- Line 18: Update the boundary assertions in Rs2WalkerAwaitsTest for
shouldAcceptIdleDoorAwait to match DOOR_IDLE_ACCEPT_MIN_MS at 600 ms: treat 600
ms as rejected and 601 ms as accepted, preserving the strict elapsedMs >
threshold behavior.
---
Outside diff comments:
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/player/Rs2Player.java`:
- Around line 413-442: Update the javadoc for toggleRunEnergy(boolean) to
document that it also returns false when enabling is requested but
getRunEnergy() is below nextRunEnergyThreshold, in addition to the
missing-widget case. Keep the existing behavior and return conditions unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b0d14e91-55b0-448e-9a65-d0c1c4378e5e
📒 Files selected for processing (5)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/ShortestPathScript.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/mouse/VirtualMouse.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/player/Rs2Player.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/door/Rs2WalkerAwaits.java
| WalkerState state; | ||
| if (config.walkWithBankedTransports()) { | ||
| state = Rs2Walker.walkWithBankedTransportsAndState(target, 10, false); | ||
| } else { | ||
| state = Rs2Walker.walkWithState(target); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Banked-transport branch ignores the configured reached distance.
The banked-transport call hardcodes 10 for the distance argument. The non-banked branch instead calls Rs2Walker.walkWithState(target), which resolves to reachedDistanceOrDefault() and honors config.reachedDistance(). As a result, a walk driven through this loop uses a different arrival threshold depending on config.walkWithBankedTransports(), and a user-configured reached distance is silently dropped for every banked-transport walk.
Use the same distance source for both branches.
🔧 Proposed fix to use a consistent distance source
WalkerState state;
if (config.walkWithBankedTransports()) {
- state = Rs2Walker.walkWithBankedTransportsAndState(target, 10, false);
+ state = Rs2Walker.walkWithBankedTransportsAndState(target, config.reachedDistance(), false);
} else {
state = Rs2Walker.walkWithState(target);
}📝 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.
| WalkerState state; | |
| if (config.walkWithBankedTransports()) { | |
| state = Rs2Walker.walkWithBankedTransportsAndState(target, 10, false); | |
| } else { | |
| state = Rs2Walker.walkWithState(target); | |
| } | |
| WalkerState state; | |
| if (config.walkWithBankedTransports()) { | |
| state = Rs2Walker.walkWithBankedTransportsAndState(target, config.reachedDistance(), false); | |
| } else { | |
| state = Rs2Walker.walkWithState(target); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/ShortestPathScript.java`
around lines 98 - 103, Update the banked-transport branch in the
ShortestPathScript walk-state flow to use the configured reached-distance source
instead of the hardcoded 10. Ensure Rs2Walker.walkWithBankedTransportsAndState
receives the same reachedDistanceOrDefault()-based value honored by
Rs2Walker.walkWithState(target), preserving consistent arrival thresholds across
both branches.
| private static final int DOOR_TRAVERSAL_PROGRESS_WAIT_MS = 1200; | ||
| /** Stationary and not animating for longer than this, with the edge unresolved, means the click didn't land. */ | ||
| private static final long DOOR_IDLE_ACCEPT_MIN_MS = 1_200L; | ||
| private static final long DOOR_IDLE_ACCEPT_MIN_MS = 600L; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Align the idle-await test boundary with the new threshold.
Line 18 sets DOOR_IDLE_ACCEPT_MIN_MS to 600 ms, but Rs2WalkerAwaitsTest.java still expects 1200 ms and 800 ms to be rejected. Because shouldAcceptIdleDoorAwait uses elapsedMs > DOOR_IDLE_ACCEPT_MIN_MS, those assertions now fail. Update the test to use 600/601 ms, or restore the constant to 1200 ms if that contract is required.
Proposed test boundary update
-assertFalse(Rs2WalkerAwaits.shouldAcceptIdleDoorAwait(false, false, 1200L, false));
-assertTrue(Rs2WalkerAwaits.shouldAcceptIdleDoorAwait(false, false, 1201L, true));
+assertFalse(Rs2WalkerAwaits.shouldAcceptIdleDoorAwait(false, false, 600L, false));
+assertTrue(Rs2WalkerAwaits.shouldAcceptIdleDoorAwait(false, false, 601L, true));
-assertFalse(Rs2WalkerAwaits.shouldAcceptIdleDoorAwait(false, false, 1200L, true));
-assertFalse(Rs2WalkerAwaits.shouldAcceptIdleDoorAwait(false, false, 800L, true));
+assertFalse(Rs2WalkerAwaits.shouldAcceptIdleDoorAwait(false, false, 600L, true));
+assertFalse(Rs2WalkerAwaits.shouldAcceptIdleDoorAwait(false, false, 500L, true));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/door/Rs2WalkerAwaits.java`
at line 18, Update the boundary assertions in Rs2WalkerAwaitsTest for
shouldAcceptIdleDoorAwait to match DOOR_IDLE_ACCEPT_MIN_MS at 600 ms: treat 600
ms as rejected and 601 ms as accepted, preserving the strict elapsedMs >
threshold behavior.
…d via another interaction on the map
|
I’m holding this hotfix from merge. The changes are broad enough (walker task lifecycle, pathfinder fallback, door timing, canvas/mouse input, run-energy behavior, Stronghold answers, and transport data) that they need the normal integration path and runtime evidence. Please address the following:
These are substantial walker/input changes, so I’m leaving them with the author rather than editing and merging them without runtime validation. |
3 participants
Applied fixes: