Repository navigation
Hotfix/webwalker fixes - #1837
dylanreniers wants to merge 15 commits into
Conversation
Update RuneLite to 1.12.34.1 and Microbot to 2.6.17
Release Microbot 2.6.18
[codex] Update RuneLite to 1.12.35
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 |
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. |
There was a problem hiding this comment.
Re-reviewed head 31b263b6c4771aba44d9af87596c0f8ceb6e93a2 on 2026-09-19. The earlier reached-distance and TSV-whitespace issues are fixed. These material blockers remain:
- GitHub reports merge conflicts. The diff still carries RuneLite API, rs2asm, and version updates outside the walker/input feature. Rebuild the feature commits on current development and reconcile VirtualMouse/Rs2Walker with the current input and recovery implementations.
- Build fails ClientThreadGuardrailTest: Rs2Walker.handleStrongholdOfSecurityAnswer directly calls TileObject.getWorldLocation() off the client thread; the log also reports 14 shifted lambda-baseline entries. Dispatch the new read on the client thread, then verify any baseline regeneration does not hide new violations.
- Rs2Walker sets UNREACHABLE_DOOR_RECOVERY_BACKTRACK_EDGES to 0 and starts raw scans at rawStart. When the nearest raw index is beyond a closed gate, this skips the blocking edge. Preserve the preceding-edge scan or add an automated regression showing off-path closed-gate recovery still works.
Live/manual testing is optional and is not a merge requirement. Validate the reconciled branch with the automated walker/input regressions and green expected CI.
|
Thanks for the work on this, @dylanreniers. I re-checked it against current
If you'd like to continue, please start from current |
Applied fixes: