Release Microbot 2.6.16 - #1826
Merged
Merged
Conversation
…ispatch fix: restore reliable ground item interaction dispatch
…lker Squash of the full walker/shortest-path effort from PluginTesting, applied cleanly onto Fix-The-Walker (which had no walker changes since the shared ancestor). Walker/shortestpath scope only — no unrelated plugin changes (questhelper, woodcutting, aiohunting, aiofishing, agentserver, etc.) are included. Includes: - P0: version-stamped, self-filling live-collision disk store; regenerated collision-map + blocked-edges/transports/restrictions data. - P1: decomposed the Rs2Walker monolith behind a headless test harness — state/WalkerRouteState, recovery/RouteRecovery, geometry/WalkerPathGeometry (pure, unit-tested route/recovery/geometry decisions). - P2: unified obstacle model (obstacle/: PlannedEdge, ObstacleResolution, ObstacleResolver/Registry, Mineable/Transport resolvers, LiveScene) with the recovery dispatch cutover; rockfall fully migrated end-to-end with legacy handlers deleted. - Door decision/detection layer fully harnessed (classifier, geometry, ahead resolver, probe) — 29 headless tests. - Recovery fixes: take agility shortcuts/transports by stepping onto the origin. - Regenerated client-thread guardrail baseline for the new walker code. 68 files. Full unit suite green on this branch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- PathfinderConfig: include CLIENT_OF_KOUREND in the decomposed quests hash
component with the same absent-from-list fallback as the full verification hash,
so verify-miss detail stays aligned.
- Rs2DoorClassifier.getDoorAction: lowercase dact in the priority-lookup comparator
(matching the case-insensitive filter) so valid actions don't fall back to indexOf("").
- Rs2DoorClassifier.isDoorComposition: use the null-safe name helper instead of
comp.getName().equals("null") (no NPE; handles null/whitespace/case).
- Rs2ObstacleHandler: in instances, skip the MLM-region gates and rely on the
rockfall object-id check (aligns code with its comment); non-instance unchanged.
- RouteRecovery.findForwardRecoveryIndex: drop the unreachable idx==bestIdx tie-break
and unused bestDistFromPlayer (monotonic idx already picks the last qualifying).
- Rs2Walker: return after the quest-locked door reroute so such doors are never
learnBlockedEdge'd (matches tryHandleDoorObject).
- ShortestPathPlugin: invalidate both live-capture base-axis values on enable/disable.
- Tests: real ladder action in Rs2DoorProbeTest; offset player + assertTrue in the
WalkerPathGeometry switchback test; discriminating endpoint-in-range assertion in
Rs2DoorGeometryTest.
Full unit suite green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…s / camping the goal Sync of the walker fixes from the Clock Tower door-route debugging (squash of six PluginTesting commits: 7e9095ed9c, b317648ec8, e890d06579, 2bf709735b, a3bfd07e6e, e8ed93f5d6). Walker/shortest-path scope only. Live-traced across four walks; each layer was verified against the next trace: - Door-recovery suppression no longer strands the player: the suppress branch walks to the furthest REACHABLE route tile before the blocked edge, and the idle nudge is held off while suppression is active (its door-blind forward click was pulling the player around buildings). - Route-blocked scan gate: forward click selection stops at the near side of a closed door/wall ON the route (player-origin BFS, along-route step metric — immune to the switch-back false negatives that historically justified no gate). Lives in a gated live-only variant; the pure selection unit tests exercise stays ungated. - Anchor fold-jump protection: rawPathForwardAnchorIndex prefers walk-connected window tiles, so a route tail folding back beside the start (goal 3 tiles away through a wall) can no longer capture the anchor and pull selection to the goal. - Stale recovery clicks are preempted: the seconds-long recovery pass re-checks door-settling/moving right before clicking, so it cannot cancel an in-flight door-open; walled-target guard widened to the full recovery click radius (unreachable Euclidean-near target => replan, never click through the wall). - Anti-goal-camping: recovery rewinds to the EARLIEST unreachable route tile (the real frontier — where the door is) instead of the Euclidean-near goal, and the interim target is cleared at walk start so script-restarted walks stop yielding to the previous route's objective. +11 headless tests (route-blocked gate, fold anchor, Clock Tower fold shape). Client-thread guardrail baseline regenerated (Rs2Walker lambda renumbering only). Full unit suite green on this branch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Walker & shortest-path: live-collision store, Rs2Walker decomposition, and unified obstacle model
Contributor
WalkthroughThe pull request updates ground-item menu dispatch, bumps the Microbot version, and adds live collision capture, overlay, persistence, learned blocked edges, and route validation. It refactors walker state, path geometry, recovery, door probing, obstacle resolution, transport handling, and pathfinder access through a facade. It also adds Motherlode Mine navigation, Al Kharid toll-gate handling, extensive tests, documentation, roadmap updates, and thread-safety baseline changes. Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
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/walker/lifecycle/Rs2WalkerLifecycleRuntime.java (1)
90-136: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winFuture bookkeeping diverges between the two branches.
The non-cave branch always republishes
setPathfinderFuture(...)for the pathfinder it installs; the cave branch runs synchronously and installs a new pathfinder but leaves the previous (cancelled) future inRs2PathApi. Anything that treats a non-null future as "work in flight" — e.g.setTarget(null)'s!isDone()cancel path — then reasons about a future belonging to a pathfinder that no longer exists.♻️ Clear the stale handle when running synchronously
if (Rs2Player.isInCave()) { + Rs2PathApi.setPathfinderFuture(null); pathfinder = new Pathfinder(Rs2PathApi.getPathfinderConfig(), start, ends); pathfinder.run();🤖 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/lifecycle/Rs2WalkerLifecycleRuntime.java` around lines 90 - 136, Clear the pathfinding future in the synchronous cave branch after cancelling the previous work and before installing or running the new Pathfinder, using the existing Rs2PathApi future accessors. Ensure the cave branch leaves no stale non-null future while preserving the asynchronous future publication in the non-cave branch.
🧹 Nitpick comments (3)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/live/LiveCollisionCapture.java (1)
43-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOrphaned javadoc.
The block at Lines 43-49 documents
DOOR_ACTIONS, but a second javadoc forCAPTURE_VERSIONfollows immediately, so the first one binds to nothing andDOOR_ACTIONS(Line 60) ends up undocumented. Move it down to the field it describes.🤖 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/pathfinder/live/LiveCollisionCapture.java` around lines 43 - 58, Move the door-action Javadoc block so it immediately precedes the DOOR_ACTIONS declaration, ensuring it documents that field rather than CAPTURE_VERSION. Keep the CAPTURE_VERSION Javadoc directly attached to CAPTURE_VERSION and preserve both descriptions unchanged.runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/live/LiveCollisionRegion.java (1)
40-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
{@param}isn't a Javadoc inline tag.
@paramis block-only, so{@paramlocalY}renders literally (and trips doclint). Fold it into prose or givelocalYits own@paramline.📝 Proposed fix
/** - * `@param` localX 0..63 within the region, {`@param` localY} likewise + * `@param` localX 0..63 within the region + * `@param` localY 0..63 within the region * `@return` {`@link` Boolean#TRUE}/{`@link` Boolean#FALSE} for a known edge, else {`@code` null}. */🤖 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/pathfinder/live/LiveCollisionRegion.java` around lines 40 - 43, Update the Javadoc for the relevant LiveCollisionRegion method so localY is documented using valid Javadoc syntax: either describe it as prose alongside localX or add a separate `@param` localY block tag, removing the invalid inline {`@param` localY} usage.runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/PathfinderConfig.java (1)
1918-1943: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe two verdict-hash javadocs are attached to the wrong methods.
The block at Lines 1918-1930 explains the varplayer rationale (
COOLDOWN_MINUTES,LAST_HOME_TELEPORT) but sits onhashVarbitConditionVerdicts; the block at Lines 1935-1941 then calls the varbit overload "Varbit counterpart ofhashVarplayerConditionVerdicts" — i.e. the counterpart of itself. Swapping them (or moving the long rationale down tohashVarplayerConditionVerdictsat Line 1967) restores the intended pairing.🤖 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/pathfinder/PathfinderConfig.java` around lines 1918 - 1943, Correct the Javadocs for hashVarbitConditionVerdicts and hashVarplayerConditionVerdicts by moving the varplayer-specific rationale mentioning COOLDOWN_MINUTES and LAST_HOME_TELEPORT onto hashVarplayerConditionVerdicts, and placing the varbit counterpart description on hashVarbitConditionVerdicts. Preserve the existing method behavior and test-injection parameter documentation.
🤖 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/pathfinder/live/LiveCollisionPersistence.java`:
- Around line 38-60: Use LiveCollisionCapture.CAPTURE_VERSION as the single
source of truth for the store-directory key in LiveCollisionPersistence,
removing its duplicate private CAPTURE_VERSION and updating the constructor path
accordingly. Align the surrounding documentation so the capture-semantics
version is defined and maintained only by LiveCollisionCapture, while preserving
the existing cache-revision directory naming.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/Pathfinder.java`:
- Around line 738-740: Move CollisionMap resolution from the Pathfinder
constructor’s config.getMap() assignment into the search-thread flow in run(),
and call beginSearch() on that same per-run map before pathfinding. Ensure
validateRouteAgainstLiveCollision cannot pin or mutate a separate client-thread
map during an active search; use the pinned map or snapshot associated with the
current run throughout the search.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/WEBWALKER_IMPROVEMENT_PLAN.md`:
- Around line 207-208: The documentation has stale backport statuses that
contradict the recorded re-baseline. In WEBWALKER_IMPROVEMENT_PLAN.md lines
207-208, remove items `#2`, `#3`, and `#11` from the next-backport list, retaining
only genuinely open work; in WEBWALKER_IMPROVEMENT_PLAN.md lines 12-14, update
the “no upstream remote,” POH, and PrimitiveIntList statuses to reflect
completion and the existing upstream remote; in UPSTREAM_COMPARISON.md lines
50-51, replace the completed POH item with the appropriate next open target.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/obstacle/ObstacleResolution.java`:
- Around line 58-60: Update the walkToOrigin factory in ObstacleResolution to
reject a null target before constructing the WALK_TO_ORIGIN resolution. Enforce
the existing factory invariant using the project’s standard null-validation
approach, while preserving valid target behavior.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/obstacle/Rs2ObstacleHandler.java`:
- Around line 69-83: Update the public handleRockfall method to handle a null
path consistently with isMotherlodeRockfallCandidate: return
RockfallResult.NOT_APPLICABLE before accessing path.size() or otherwise
processing the path. Preserve the existing behavior for non-null paths.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/recovery/RouteRecovery.java`:
- Around line 86-89: Update the fallback early-out in the RouteRecovery logic to
return fallbackWp whenever fallbackDistSq is within the target radius, including
values less than or equal to targetEuclidean squared. Preserve the existing
interpolation behavior only for fallbacks outside that radius.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java`:
- Around line 4500-4521: Make reachableTilesCache handling consistent in
resolveRecoveryObstacle by guarding the step (2) call to
RouteRecovery.findReachableTransportOriginAhead so keySet() is never invoked
when the cache is null, while preserving the existing rockfall resolution path
and behavior for non-null caches.
In
`@runelite-client/src/test/java/net/runelite/client/plugins/microbot/util/grounditem/GroundItemInteractionDispatchTest.java`:
- Around line 63-91: Update the bytecode inspection in
GroundItemInteractionDispatchTest so it also examines lambda-generated methods
and same-class helpers, not only the expected click/interact method. Traverse
relevant methods such as lambda$... and follow their calls or scan the class for
Rs2Reflection.invokeMenu, while preserving the existing doInvoke and reflection
count assertions.
In
`@runelite-client/src/test/java/net/runelite/client/plugins/microbot/util/walker/banking/BankedTransportItemPlanningTest.java`:
- Around line 91-102: Add a non-empty assertion for the filtered currency list
in currencyTransportsRemainEligible before iterating, matching the sibling
tests’ assertFalse pattern and preserving the existing eligibility checks.
---
Outside diff comments:
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/lifecycle/Rs2WalkerLifecycleRuntime.java`:
- Around line 90-136: Clear the pathfinding future in the synchronous cave
branch after cancelling the previous work and before installing or running the
new Pathfinder, using the existing Rs2PathApi future accessors. Ensure the cave
branch leaves no stale non-null future while preserving the asynchronous future
publication in the non-cave branch.
---
Nitpick comments:
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/live/LiveCollisionCapture.java`:
- Around line 43-58: Move the door-action Javadoc block so it immediately
precedes the DOOR_ACTIONS declaration, ensuring it documents that field rather
than CAPTURE_VERSION. Keep the CAPTURE_VERSION Javadoc directly attached to
CAPTURE_VERSION and preserve both descriptions unchanged.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/live/LiveCollisionRegion.java`:
- Around line 40-43: Update the Javadoc for the relevant LiveCollisionRegion
method so localY is documented using valid Javadoc syntax: either describe it as
prose alongside localX or add a separate `@param` localY block tag, removing the
invalid inline {`@param` localY} usage.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/PathfinderConfig.java`:
- Around line 1918-1943: Correct the Javadocs for hashVarbitConditionVerdicts
and hashVarplayerConditionVerdicts by moving the varplayer-specific rationale
mentioning COOLDOWN_MINUTES and LAST_HOME_TELEPORT onto
hashVarplayerConditionVerdicts, and placing the varbit counterpart description
on hashVarbitConditionVerdicts. Preserve the existing method behavior and
test-injection parameter documentation.
🪄 Autofix (Beta)
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: 8155a736-a181-45d2-9f21-07306df82f06
⛔ Files ignored due to path filters (6)
runelite-client/src/main/resources/net/runelite/client/plugins/microbot/shortestpath/blocked_edges.tsvis excluded by!**/*.tsvrunelite-client/src/main/resources/net/runelite/client/plugins/microbot/shortestpath/collision-map.zipis excluded by!**/*.ziprunelite-client/src/main/resources/net/runelite/client/plugins/microbot/shortestpath/restrictions.tsvis excluded by!**/*.tsvrunelite-client/src/main/resources/net/runelite/client/plugins/microbot/shortestpath/spirit_trees.tsvis excluded by!**/*.tsvrunelite-client/src/main/resources/net/runelite/client/plugins/microbot/shortestpath/teleportation_items.tsvis excluded by!**/*.tsvrunelite-client/src/main/resources/net/runelite/client/plugins/microbot/shortestpath/transports.tsvis excluded by!**/*.tsv
📒 Files selected for processing (68)
docs/entity-guides/items.mddocs/walker-audit.mddocs/walker-p2-unification.mdgradle.propertiesrunelite-client/src/main/java/net/runelite/client/plugins/microbot/api/tileitem/models/Rs2TileItemModel.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/ShortestPathConfig.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/ShortestPathPanel.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/ShortestPathPlugin.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/UPSTREAM_COMPARISON.mdrunelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/WEBWALKER_IMPROVEMENT_PLAN.mdrunelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/CollisionMap.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/LearnedBlockedEdges.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/Pathfinder.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/PathfinderConfig.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/SplitFlagMap.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/live/LiveCollisionCapture.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/live/LiveCollisionDoorMask.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/live/LiveCollisionOverlay.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/live/LiveCollisionPersistence.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/live/LiveCollisionRegion.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/live/LiveCollisionRegions.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/live/LiveCollisionSnapshot.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/live/LiveCollisionView.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/live/LiveEdgeSource.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/live/LiveRouteValidator.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/grounditem/Rs2GroundItem.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2PathApi.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/WebWalkLog.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/banking/Rs2WalkerBankingPlanner.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/door/DoorProbeContext.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/door/Rs2DoorClassifier.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/door/Rs2DoorDetection.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/door/Rs2DoorGeometry.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/door/Rs2DoorProbe.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/geometry/WalkerPathGeometry.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/lifecycle/Rs2WalkerLifecycleRuntime.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/obstacle/LiveScene.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/obstacle/MineableResolver.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/obstacle/ObstacleRegistry.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/obstacle/ObstacleResolution.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/obstacle/ObstacleResolver.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/obstacle/PlannedEdge.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/obstacle/Rs2LiveScene.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/obstacle/Rs2ObstacleHandler.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/obstacle/TransportResolver.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/obstacle/WalkerActions.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/recovery/RouteRecovery.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/state/WalkerRouteState.javarunelite-client/src/test/java/net/runelite/client/plugins/microbot/shortestpath/LiveCollisionTest.javarunelite-client/src/test/java/net/runelite/client/plugins/microbot/shortestpath/RouteClickTargetRegressionTest.javarunelite-client/src/test/java/net/runelite/client/plugins/microbot/shortestpath/ShortestPathCoreTest.javarunelite-client/src/test/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/LearnedBlockedEdgesTest.javarunelite-client/src/test/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/PathfinderConfigTransportRefreshHashTest.javarunelite-client/src/test/java/net/runelite/client/plugins/microbot/util/grounditem/GroundItemInteractionDispatchTest.javarunelite-client/src/test/java/net/runelite/client/plugins/microbot/util/walker/Rs2WalkerUnitTest.javarunelite-client/src/test/java/net/runelite/client/plugins/microbot/util/walker/TargetWalkabilityPreflightTest.javarunelite-client/src/test/java/net/runelite/client/plugins/microbot/util/walker/banking/BankedTransportItemPlanningTest.javarunelite-client/src/test/java/net/runelite/client/plugins/microbot/util/walker/door/Rs2DoorAheadResolverTest.javarunelite-client/src/test/java/net/runelite/client/plugins/microbot/util/walker/door/Rs2DoorClassifierTest.javarunelite-client/src/test/java/net/runelite/client/plugins/microbot/util/walker/door/Rs2DoorGeometryTest.javarunelite-client/src/test/java/net/runelite/client/plugins/microbot/util/walker/door/Rs2DoorProbeTest.javarunelite-client/src/test/java/net/runelite/client/plugins/microbot/util/walker/geometry/WalkerPathGeometryTest.javarunelite-client/src/test/java/net/runelite/client/plugins/microbot/util/walker/obstacle/MineableResolverTest.javarunelite-client/src/test/java/net/runelite/client/plugins/microbot/util/walker/obstacle/ObstacleRegistryTest.javarunelite-client/src/test/java/net/runelite/client/plugins/microbot/util/walker/obstacle/TransportResolverTest.javarunelite-client/src/test/java/net/runelite/client/plugins/microbot/util/walker/recovery/RouteRecoveryTest.javarunelite-client/src/test/resources/threadsafety/client-thread-guardrail-baseline.txt
Comment on lines
+38
to
+60
| /** | ||
| * Capture/merge semantics version. Distinct from {@link #VERSION} (the byte format): bump this | ||
| * whenever what we *record* changes (e.g. the rockfall-exemption capture rules), even if the file | ||
| * layout is identical. It is part of the on-disk store key, so a bump auto-invalidates every | ||
| * previously-learned region — no manual "Reset learned collision" needed. History: | ||
| * 1 = original; 2 = runtime-handled-obstacle (rockfall) exemption in LiveCollisionCapture. | ||
| */ | ||
| private static final int CAPTURE_VERSION = 2; | ||
|
|
||
| private final File dir; | ||
| /** Root removed by {@link #deleteAllNow()} — the whole {@code live-collision} tree (all revisions). */ | ||
| private final File deleteRoot; | ||
| private final ExecutorService io = Executors.newSingleThreadExecutor(r -> { | ||
| final Thread t = new Thread(r, "live-collision-io"); | ||
| t.setDaemon(true); | ||
| return t; | ||
| }); | ||
|
|
||
| public LiveCollisionPersistence(int cacheRevision) { | ||
| final File liveCollisionBase = new File(new File(RuneLite.RUNELITE_DIR, "microbot"), "live-collision"); | ||
| // Store key = game cache revision + capture-semantics version. A change to either sends the | ||
| // store to a fresh directory; the old ones are inert and pruned by pruneStaleStores(). | ||
| this.dir = new File(liveCollisionBase, cacheRevision + "-c" + CAPTURE_VERSION); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Two CAPTURE_VERSION constants for one concept.
This private CAPTURE_VERSION (Line 45) only names the store directory (Line 60), while the file header written and checked at Lines 181 and 222 uses LiveCollisionCapture.CAPTURE_VERSION. Both javadocs claim to be the capture-semantics version, and both currently read 2, so nothing breaks today — but they must be bumped in lockstep by hand. Bump only the capture side and the directory key stays put: every existing .lcr is read, rejected as stale, and never pruned (pruneStaleStores skips the current dir name), so the files linger and get re-read on every load.
🔧 Proposed fix — single source of truth
- /**
- * Capture/merge semantics version. Distinct from {`@link` `#VERSION`} (the byte format): bump this
- * whenever what we *record* changes (e.g. the rockfall-exemption capture rules), even if the file
- * layout is identical. It is part of the on-disk store key, so a bump auto-invalidates every
- * previously-learned region — no manual "Reset learned collision" needed. History:
- * 1 = original; 2 = runtime-handled-obstacle (rockfall) exemption in LiveCollisionCapture.
- */
- private static final int CAPTURE_VERSION = 2;
+ /** Capture/merge semantics version; owned by {`@link` LiveCollisionCapture} so the store key and the
+ * file header can never drift apart. Distinct from {`@link` `#VERSION`} (the byte format). */
+ private static final int CAPTURE_VERSION = LiveCollisionCapture.CAPTURE_VERSION;📝 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
| /** | |
| * Capture/merge semantics version. Distinct from {@link #VERSION} (the byte format): bump this | |
| * whenever what we *record* changes (e.g. the rockfall-exemption capture rules), even if the file | |
| * layout is identical. It is part of the on-disk store key, so a bump auto-invalidates every | |
| * previously-learned region — no manual "Reset learned collision" needed. History: | |
| * 1 = original; 2 = runtime-handled-obstacle (rockfall) exemption in LiveCollisionCapture. | |
| */ | |
| private static final int CAPTURE_VERSION = 2; | |
| private final File dir; | |
| /** Root removed by {@link #deleteAllNow()} — the whole {@code live-collision} tree (all revisions). */ | |
| private final File deleteRoot; | |
| private final ExecutorService io = Executors.newSingleThreadExecutor(r -> { | |
| final Thread t = new Thread(r, "live-collision-io"); | |
| t.setDaemon(true); | |
| return t; | |
| }); | |
| public LiveCollisionPersistence(int cacheRevision) { | |
| final File liveCollisionBase = new File(new File(RuneLite.RUNELITE_DIR, "microbot"), "live-collision"); | |
| // Store key = game cache revision + capture-semantics version. A change to either sends the | |
| // store to a fresh directory; the old ones are inert and pruned by pruneStaleStores(). | |
| this.dir = new File(liveCollisionBase, cacheRevision + "-c" + CAPTURE_VERSION); | |
| /** | |
| * Capture/merge semantics version; owned by {`@link` LiveCollisionCapture} so the store key and the | |
| * file header can never drift apart. Distinct from {`@link` `#VERSION`} (the byte format). */ | |
| private static final int CAPTURE_VERSION = LiveCollisionCapture.CAPTURE_VERSION; | |
| private final File dir; | |
| /** Root removed by {`@link` `#deleteAllNow`()} — the whole {`@code` live-collision} tree (all revisions). */ | |
| private final File deleteRoot; | |
| private final ExecutorService io = Executors.newSingleThreadExecutor(r -> { | |
| final Thread t = new Thread(r, "live-collision-io"); | |
| t.setDaemon(true); | |
| return t; | |
| }); | |
| public LiveCollisionPersistence(int cacheRevision) { | |
| final File liveCollisionBase = new File(new File(RuneLite.RUNELITE_DIR, "microbot"), "live-collision"); | |
| // Store key = game cache revision + capture-semantics version. A change to either sends the | |
| // store to a fresh directory; the old ones are inert and pruned by pruneStaleStores(). | |
| this.dir = new File(liveCollisionBase, cacheRevision + "-c" + CAPTURE_VERSION); |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 56-56: Prevent path traversal
Context: new File(new File(RuneLite.RUNELITE_DIR, "microbot"), "live-collision")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal'). Security best practice.
(path-traversal-java)
[warning] 56-56: Prevent path traversal
Context: new File(RuneLite.RUNELITE_DIR, "microbot")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal'). Security best practice.
(path-traversal-java)
[warning] 59-59: Prevent path traversal
Context: new File(liveCollisionBase, cacheRevision + "-c" + CAPTURE_VERSION)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal'). Security best practice.
(path-traversal-java)
🤖 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/pathfinder/live/LiveCollisionPersistence.java`
around lines 38 - 60, Use LiveCollisionCapture.CAPTURE_VERSION as the single
source of truth for the store-directory key in LiveCollisionPersistence,
removing its duplicate private CAPTURE_VERSION and updating the constructor path
accordingly. Align the surrounding documentation so the capture-semantics
version is defined and maintained only by LiveCollisionCapture, while preserving
the existing cache-revision directory naming.
Comment on lines
+738
to
+740
| // Pin the live-collision snapshot for this whole search so a mid-search swap on the client | ||
| // thread cannot mix two scenes into one path. No-op when live collision is disabled. | ||
| map.beginSearch(); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C4 'beginSearch\(\)' runelite-client/src/main/java/net/runelite/client/plugins/microbot
rg -nP -C3 'private final ThreadLocal<CollisionMap> map|public CollisionMap getMap' runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/PathfinderConfig.javaRepository: chsami/Microbot
Length of output: 10996
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Pathfinder relevant declarations and run =="
sed -n '1,140p' runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/Pathfinder.java
echo
sed -n '720,770p' runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/Pathfinder.java
echo
echo "== ShortestPathPlugin validator and executors =="
sed -n '1,140p' runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/ShortestPathPlugin.java
echo
sed -n '740,800p' runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/ShortestPathPlugin.java
echo
echo "== Pathfinder config overlay/map declarations =="
sed -n '70,95p' runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/PathfinderConfig.java
echo
sed -n '220,245p' runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/PathfinderConfig.java
echo
echo "== CollisionMap beginSearch/get implementation =="
sed -n '25,80p' runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/CollisionMap.java
echo
echo "== LiveRouteValidator constructor usages of CollisionMap =="
rg -n -C3 'new LiveRouteValidator|firstBlockedStep|validateRouteAgainstLiveCollision|beginSearch\(\)' runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/live runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/ShortestPathPlugin.javaRepository: chsami/Microbot
Length of output: 27976
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== restartPathfinding and ThreadFactory usage =="
rg -n -C5 'new Pathfinder|submit\(|Future|ExecutorService|pathfinderConfig|ThreadFactoryBuilder|shortestPathExecutor' runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/ShortestPathPlugin.java runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/ShortestPathConfig.java runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/Pathfinder.java
echo
echo "== PathfinderConfig ThreadLocal initialization =="
rg -n -C8 'new ThreadLocal|new CollisionMap|PathfinderConfig|Map\.of' runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/ShortestPathConfig.javaRepository: chsami/Microbot
Length of output: 32681
Pinning must happen on the search thread.
this.map is resolved by config.getMap() in the Pathfinder constructor on the client thread, while run() executes later on the shortest-path executor. validateRouteAgainstLiveCollision is on the GameTick thread and re-fetches pathfinderConfig.getMap() before beginSearch(), so during an active search they can use different per-thread CollisionMap instances. A client-thread validation can therefore re-pin a separate map’s pinnedLive during the running search. Resolve the map on the search thread or pass the snapshot through the search so the pin is private to this run.
🤖 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/pathfinder/Pathfinder.java`
around lines 738 - 740, Move CollisionMap resolution from the Pathfinder
constructor’s config.getMap() assignment into the search-thread flow in run(),
and call beginSearch() on that same per-run map before pathfinding. Ensure
validateRouteAgainstLiveCollision cannot pin or mutate a separate client-thread
map during an active search; use the pinned map or snapshot associated with the
current run throughout the search.
Comment on lines
+207
to
+208
| 4. ⬜ **NEXT** — **Backport behind the facade** — resume the Tier 1–5 items (wilderness widths #2, `PrimitiveIntList` #11, POH coverage #3, etc.) changing only internals; consumers untouched. | ||
| 5. ✅ **DONE (re-baseline)** — added the `skretzo` remote, fetched, and re-baselined against upstream HEAD `7e7e5bf94b` (122 commits + major refactor past `07fca57`). Findings in `UPSTREAM_COMPARISON.md` "Re-baseline 2026-07-20": #2 wilderness byte-identical to upstream `WildernessChecker` (DONE); **#3 POH is the one concrete real gap** (upstream ships `teleportation_portals_poh.tsv`, 137 rows; Microbot routes POH programmatically — verify `util/poh` coverage); #10/#12 have no upstream counterpart (Microbot-original). Upstream is now architecturally distinct — only selective feature/data backports are viable, which the facade enables. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Reconcile the completed-backport status.
These sections still schedule or recommend work that the same documents mark completed, and one says no upstream remote exists despite the recorded re-baseline.
runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/WEBWALKER_IMPROVEMENT_PLAN.md#L207-L208: remove#2,#3, and#11from the next-backport list; retain only genuinely open items.runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/WEBWALKER_IMPROVEMENT_PLAN.md#L12-L14: update the stale “no upstream remote,” POH, andPrimitiveIntListstatus.runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/UPSTREAM_COMPARISON.md#L50-L51: replace the completed POH item as the “next target.”
📍 Affects 2 files
runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/WEBWALKER_IMPROVEMENT_PLAN.md#L207-L208(this comment)runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/WEBWALKER_IMPROVEMENT_PLAN.md#L12-L14runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/UPSTREAM_COMPARISON.md#L50-L51
🤖 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/WEBWALKER_IMPROVEMENT_PLAN.md`
around lines 207 - 208, The documentation has stale backport statuses that
contradict the recorded re-baseline. In WEBWALKER_IMPROVEMENT_PLAN.md lines
207-208, remove items `#2`, `#3`, and `#11` from the next-backport list, retaining
only genuinely open work; in WEBWALKER_IMPROVEMENT_PLAN.md lines 12-14, update
the “no upstream remote,” POH, and PrimitiveIntList statuses to reflect
completion and the existing upstream remote; in UPSTREAM_COMPARISON.md lines
50-51, replace the completed POH item with the appropriate next open target.
Comment on lines
+58
to
+60
| public static ObstacleResolution walkToOrigin(WorldPoint target) { | ||
| return new ObstacleResolution(Kind.WALK_TO_ORIGIN, target, null); | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject null walk targets.
Line 58 can create WALK_TO_ORIGIN with no destination, which the walker silently ignores. Enforce the factory invariant.
Proposed fix
+import java.util.Objects;
+
public static ObstacleResolution walkToOrigin(WorldPoint target) {
- return new ObstacleResolution(Kind.WALK_TO_ORIGIN, target, null);
+ return new ObstacleResolution(
+ Kind.WALK_TO_ORIGIN,
+ Objects.requireNonNull(target, "target"),
+ null);
}📝 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
| public static ObstacleResolution walkToOrigin(WorldPoint target) { | |
| return new ObstacleResolution(Kind.WALK_TO_ORIGIN, target, null); | |
| } | |
| public static ObstacleResolution walkToOrigin(WorldPoint target) { | |
| return new ObstacleResolution( | |
| Kind.WALK_TO_ORIGIN, | |
| java.util.Objects.requireNonNull(target, "target"), | |
| null); | |
| } |
🤖 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/obstacle/ObstacleResolution.java`
around lines 58 - 60, Update the walkToOrigin factory in ObstacleResolution to
reject a null target before constructing the WALK_TO_ORIGIN resolution. Enforce
the existing factory invariant using the project’s standard null-validation
approach, while preserving valid target behavior.
Comment on lines
+69
to
+83
| public static RockfallResult handleRockfall(List<WorldPoint> path, int index) { | ||
| if (Rs2PathApi.getPathfinder() == null) return RockfallResult.NOT_APPLICABLE; | ||
|
|
||
| if (index == path.size() - 1) return RockfallResult.NOT_APPLICABLE; | ||
|
|
||
| WorldPoint playerLoc = Rs2Player.getWorldLocation(); | ||
| if (playerLoc == null) return RockfallResult.NOT_APPLICABLE; | ||
|
|
||
| // In an instance the region IDs are the instance template's, not MOTHERLODE_MINE_REGION, so the | ||
| // region gates below would reject every tile. Skip the region checks while instanced and rely on | ||
| // the rockfall object-id check to stay correct; outside instances keep the MLM region restriction. | ||
| final boolean inInstance = Microbot.getClient().getTopLevelWorldView().isInstance(); | ||
|
|
||
| final int lastCandidate = Math.min(index + 1, path.size() - 1); | ||
| if (!inInstance && !isMotherlodeRockfallCandidate(playerLoc, path, index)) return RockfallResult.NOT_APPLICABLE; |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Null-path handling is inconsistent between the two public entry points.
isMotherlodeRockfallCandidate explicitly tolerates path == null (line 52), but handleRockfall dereferences path.size() at line 72 before any guard, so the same input NPEs here. Either both should abstain or the contract should be documented.
🛡️ Proposed guard
- if (index == path.size() - 1) return RockfallResult.NOT_APPLICABLE;
+ if (path == null || path.isEmpty() || index < 0 || index >= path.size()) return RockfallResult.NOT_APPLICABLE;
+ if (index == path.size() - 1) return RockfallResult.NOT_APPLICABLE;📝 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
| public static RockfallResult handleRockfall(List<WorldPoint> path, int index) { | |
| if (Rs2PathApi.getPathfinder() == null) return RockfallResult.NOT_APPLICABLE; | |
| if (index == path.size() - 1) return RockfallResult.NOT_APPLICABLE; | |
| WorldPoint playerLoc = Rs2Player.getWorldLocation(); | |
| if (playerLoc == null) return RockfallResult.NOT_APPLICABLE; | |
| // In an instance the region IDs are the instance template's, not MOTHERLODE_MINE_REGION, so the | |
| // region gates below would reject every tile. Skip the region checks while instanced and rely on | |
| // the rockfall object-id check to stay correct; outside instances keep the MLM region restriction. | |
| final boolean inInstance = Microbot.getClient().getTopLevelWorldView().isInstance(); | |
| final int lastCandidate = Math.min(index + 1, path.size() - 1); | |
| if (!inInstance && !isMotherlodeRockfallCandidate(playerLoc, path, index)) return RockfallResult.NOT_APPLICABLE; | |
| public static RockfallResult handleRockfall(List<WorldPoint> path, int index) { | |
| if (Rs2PathApi.getPathfinder() == null) return RockfallResult.NOT_APPLICABLE; | |
| if (path == null || path.isEmpty() || index < 0 || index >= path.size()) return RockfallResult.NOT_APPLICABLE; | |
| if (index == path.size() - 1) return RockfallResult.NOT_APPLICABLE; | |
| WorldPoint playerLoc = Rs2Player.getWorldLocation(); | |
| if (playerLoc == null) return RockfallResult.NOT_APPLICABLE; | |
| // In an instance the region IDs are the instance template's, not MOTHERLODE_MINE_REGION, so the | |
| // region gates below would reject every tile. Skip the region checks while instanced and rely on | |
| // the rockfall object-id check to stay correct; outside instances keep the MLM region restriction. | |
| final boolean inInstance = Microbot.getClient().getTopLevelWorldView().isInstance(); | |
| final int lastCandidate = Math.min(index + 1, path.size() - 1); | |
| if (!inInstance && !isMotherlodeRockfallCandidate(playerLoc, path, index)) return RockfallResult.NOT_APPLICABLE; |
🤖 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/obstacle/Rs2ObstacleHandler.java`
around lines 69 - 83, Update the public handleRockfall method to handle a null
path consistently with isMotherlodeRockfallCandidate: return
RockfallResult.NOT_APPLICABLE before accessing path.size() or otherwise
processing the path. Preserve the existing behavior for non-null paths.
Comment on lines
+86
to
+89
| int fallbackDistSq = euclideanSq(fallbackWp, playerLoc); | ||
| if (fallbackDistSq == targetEuclidean * targetEuclidean) { | ||
| return fallbackWp; | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Exact-equality early-out reads like an intended <= range check.
fallbackDistSq == targetEuclidean * targetEuclidean only short-circuits on the one radius where the fallback sits exactly on the ring. For a fallback that is already comfortably inside the radius, the method instead interpolates a point at ~targetEuclidean tiles out — i.e. it pushes the click further away than the caller's own fallback, which contradicts the doc ("when the furthest in-range path tile is still beyond the minimap clip").
🐛 Treat anything already in range as done
- int fallbackDistSq = euclideanSq(fallbackWp, playerLoc);
- if (fallbackDistSq == targetEuclidean * targetEuclidean) {
- return fallbackWp;
- }
+ int fallbackDistSq = euclideanSq(fallbackWp, playerLoc);
+ if (fallbackDistSq <= targetEuclidean * targetEuclidean) {
+ return fallbackWp;
+ }📝 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
| int fallbackDistSq = euclideanSq(fallbackWp, playerLoc); | |
| if (fallbackDistSq == targetEuclidean * targetEuclidean) { | |
| return fallbackWp; | |
| } | |
| int fallbackDistSq = euclideanSq(fallbackWp, playerLoc); | |
| if (fallbackDistSq <= targetEuclidean * targetEuclidean) { | |
| return fallbackWp; | |
| } |
🤖 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/recovery/RouteRecovery.java`
around lines 86 - 89, Update the fallback early-out in the RouteRecovery logic
to return fallbackWp whenever fallbackDistSq is within the target radius,
including values less than or equal to targetEuclidean squared. Preserve the
existing interpolation behavior only for fallbacks outside that radius.
Comment on lines
+4500
to
4521
| private static ObstacleResolution resolveRecoveryObstacle(List<WorldPoint> rawPath, int rawEdgeStart, | ||
| int rawEdgeEnd, WorldPoint playerLoc, | ||
| int recoveryMinimapReach, | ||
| Map<WorldPoint, Integer> reachableTilesCache) { | ||
| if (rawPath == null || rawPath.isEmpty() || playerLoc == null) { | ||
| return ObstacleResolution.notApplicable(); | ||
| } | ||
|
|
||
| if (object == null) continue; | ||
| // (1) Rockfall on the blocked frontier: narrow segment scan (shared with the forward walk loop). | ||
| ObstacleResolution rockfall = resolveRockfallOnSegment(rawPath, rawEdgeStart, rawEdgeEnd, reachableTilesCache); | ||
| if (rockfall.kind() != ObstacleResolution.Kind.NOT_APPLICABLE) { | ||
| return rockfall; | ||
| } | ||
|
|
||
| if (object.getId() == ObjectID.MOTHERLODE_ROCKFALL_1 || object.getId() == ObjectID.MOTHERLODE_ROCKFALL_2) { | ||
| Rs2GameObject.interact(object, "mine"); | ||
| return sleepUntil(() -> Rs2GameObject.getGameObject(point) == null); | ||
| } | ||
| // (2) Reachable transport / agility-shortcut origin ahead: wide forward-window scan. | ||
| WorldPoint shortcutOrigin = RouteRecovery.findReachableTransportOriginAhead( | ||
| rawPath, getClosestTileIndex(rawPath, playerLoc), playerLoc, | ||
| reachableTilesCache.keySet(), Rs2PathApi.getTransports(), | ||
| recoveryMinimapReach - 1, ROUTE_PROGRESS_FORWARD_SEARCH_TILES); | ||
| if (shortcutOrigin != null && !shortcutOrigin.equals(playerLoc)) { | ||
| return ObstacleResolution.walkToOrigin(shortcutOrigin); | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Inconsistent null-handling of reachableTilesCache inside the same method.
Step (1) routes through resolveRockfallOnSegment, which explicitly tolerates a null cache, but step (2) dereferences reachableTilesCache.keySet() unguarded. Today's single caller happens to pass a non-null map, so the two treatments disagree for no reason — and the guard block at the top (which checks rawPath/playerLoc) implies the parameters are being validated.
🛡️ Make the contract consistent
- if (rawPath == null || rawPath.isEmpty() || playerLoc == null) {
+ if (rawPath == null || rawPath.isEmpty() || playerLoc == null || reachableTilesCache == null) {
return ObstacleResolution.notApplicable();
}📝 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
| private static ObstacleResolution resolveRecoveryObstacle(List<WorldPoint> rawPath, int rawEdgeStart, | |
| int rawEdgeEnd, WorldPoint playerLoc, | |
| int recoveryMinimapReach, | |
| Map<WorldPoint, Integer> reachableTilesCache) { | |
| if (rawPath == null || rawPath.isEmpty() || playerLoc == null) { | |
| return ObstacleResolution.notApplicable(); | |
| } | |
| if (object == null) continue; | |
| // (1) Rockfall on the blocked frontier: narrow segment scan (shared with the forward walk loop). | |
| ObstacleResolution rockfall = resolveRockfallOnSegment(rawPath, rawEdgeStart, rawEdgeEnd, reachableTilesCache); | |
| if (rockfall.kind() != ObstacleResolution.Kind.NOT_APPLICABLE) { | |
| return rockfall; | |
| } | |
| if (object.getId() == ObjectID.MOTHERLODE_ROCKFALL_1 || object.getId() == ObjectID.MOTHERLODE_ROCKFALL_2) { | |
| Rs2GameObject.interact(object, "mine"); | |
| return sleepUntil(() -> Rs2GameObject.getGameObject(point) == null); | |
| } | |
| // (2) Reachable transport / agility-shortcut origin ahead: wide forward-window scan. | |
| WorldPoint shortcutOrigin = RouteRecovery.findReachableTransportOriginAhead( | |
| rawPath, getClosestTileIndex(rawPath, playerLoc), playerLoc, | |
| reachableTilesCache.keySet(), Rs2PathApi.getTransports(), | |
| recoveryMinimapReach - 1, ROUTE_PROGRESS_FORWARD_SEARCH_TILES); | |
| if (shortcutOrigin != null && !shortcutOrigin.equals(playerLoc)) { | |
| return ObstacleResolution.walkToOrigin(shortcutOrigin); | |
| } | |
| private static ObstacleResolution resolveRecoveryObstacle(List<WorldPoint> rawPath, int rawEdgeStart, | |
| int rawEdgeEnd, WorldPoint playerLoc, | |
| int recoveryMinimapReach, | |
| Map<WorldPoint, Integer> reachableTilesCache) { | |
| if (rawPath == null || rawPath.isEmpty() || playerLoc == null || reachableTilesCache == null) { | |
| return ObstacleResolution.notApplicable(); | |
| } | |
| // (1) Rockfall on the blocked frontier: narrow segment scan (shared with the forward walk loop). | |
| ObstacleResolution rockfall = resolveRockfallOnSegment(rawPath, rawEdgeStart, rawEdgeEnd, reachableTilesCache); | |
| if (rockfall.kind() != ObstacleResolution.Kind.NOT_APPLICABLE) { | |
| return rockfall; | |
| } | |
| // (2) Reachable transport / agility-shortcut origin ahead: wide forward-window scan. | |
| WorldPoint shortcutOrigin = RouteRecovery.findReachableTransportOriginAhead( | |
| rawPath, getClosestTileIndex(rawPath, playerLoc), playerLoc, | |
| reachableTilesCache.keySet(), Rs2PathApi.getTransports(), | |
| recoveryMinimapReach - 1, ROUTE_PROGRESS_FORWARD_SEARCH_TILES); | |
| if (shortcutOrigin != null && !shortcutOrigin.equals(playerLoc)) { | |
| return ObstacleResolution.walkToOrigin(shortcutOrigin); | |
| } |
🤖 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/Rs2Walker.java`
around lines 4500 - 4521, Make reachableTilesCache handling consistent in
resolveRecoveryObstacle by guarding the step (2) call to
RouteRecovery.findReachableTransportOriginAhead so keySet() is never invoked
when the cache is null, while preserving the existing rockfall resolution path
and behavior for non-null caches.
Comment on lines
+63
to
+91
| new ClassReader(input).accept(new ClassVisitor(Opcodes.ASM9) | ||
| { | ||
| @Override | ||
| public MethodVisitor visitMethod(int access, String name, String descriptor, | ||
| String signature, String[] exceptions) | ||
| { | ||
| if (!name.equals(expectedName) || !descriptor.equals(expectedDescriptor)) | ||
| { | ||
| return null; | ||
| } | ||
| calls.matchedMethods++; | ||
| return new MethodVisitor(Opcodes.ASM9) | ||
| { | ||
| @Override | ||
| public void visitMethodInsn(int opcode, String owner, String methodName, | ||
| String methodDescriptor, boolean isInterface) | ||
| { | ||
| if (owner.equals(Type.getInternalName(Microbot.class)) && methodName.equals("doInvoke")) | ||
| { | ||
| calls.doInvoke++; | ||
| } | ||
| if (owner.equals(Type.getInternalName(Rs2Reflection.class)) && methodName.equals("invokeMenu")) | ||
| { | ||
| calls.reflectionInvokeMenu++; | ||
| } | ||
| } | ||
| }; | ||
| } | ||
| }, ClassReader.SKIP_FRAMES); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Cover lambda-wrapped reflection dispatches.
This visitor scans only click/interact itself. A reintroduced runOnClientThreadOptional(() -> Rs2Reflection.invokeMenu(...)) compiles into a separate lambda$... method and would still satisfy the zero-count assertion. Follow same-class lambda/helper calls, or scan the relevant class methods for invokeMenu.
🤖 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/test/java/net/runelite/client/plugins/microbot/util/grounditem/GroundItemInteractionDispatchTest.java`
around lines 63 - 91, Update the bytecode inspection in
GroundItemInteractionDispatchTest so it also examines lambda-generated methods
and same-class helpers, not only the expected click/interact method. Traverse
relevant methods such as lambda$... and follow their calls or scan the class for
Rs2Reflection.invokeMenu, while preserving the existing doInvoke and reflection
count assertions.
Comment on lines
+91
to
+102
| @Test | ||
| public void currencyTransportsRemainEligible() { | ||
| List<Transport> currency = all.stream() | ||
| .filter(t -> t.getType() == TransportType.TRANSPORT) | ||
| .filter(t -> t.getCurrencyAmount() > 0) | ||
| .collect(Collectors.toList()); | ||
|
|
||
| for (Transport t : currency) { | ||
| assertTrue("currency transports must keep qualifying: " + describe(t), | ||
| Rs2WalkerBankingPlanner.planningCoversPlainTransport(t)); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Missing the non-empty precondition every sibling test asserts.
Lines 64, 83 and 111 all guard their filtered list with assertFalse(..., isEmpty()) before looping. This one doesn't, so if currency-bearing TRANSPORT rows ever disappear from the data, the test passes vacuously while claiming currency transports "kept their existing eligibility".
💚 Align with the surrounding pattern
List<Transport> currency = all.stream()
.filter(t -> t.getType() == TransportType.TRANSPORT)
.filter(t -> t.getCurrencyAmount() > 0)
.collect(Collectors.toList());
+ assertFalse("precondition: the data should contain currency-bearing plain transports",
+ currency.isEmpty());
for (Transport t : currency) {📝 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
| @Test | |
| public void currencyTransportsRemainEligible() { | |
| List<Transport> currency = all.stream() | |
| .filter(t -> t.getType() == TransportType.TRANSPORT) | |
| .filter(t -> t.getCurrencyAmount() > 0) | |
| .collect(Collectors.toList()); | |
| for (Transport t : currency) { | |
| assertTrue("currency transports must keep qualifying: " + describe(t), | |
| Rs2WalkerBankingPlanner.planningCoversPlainTransport(t)); | |
| } | |
| } | |
| `@Test` | |
| public void currencyTransportsRemainEligible() { | |
| List<Transport> currency = all.stream() | |
| .filter(t -> t.getType() == TransportType.TRANSPORT) | |
| .filter(t -> t.getCurrencyAmount() > 0) | |
| .collect(Collectors.toList()); | |
| assertFalse("precondition: the data should contain currency-bearing plain transports", | |
| currency.isEmpty()); | |
| for (Transport t : currency) { | |
| assertTrue("currency transports must keep qualifying: " + describe(t), | |
| Rs2WalkerBankingPlanner.planningCoversPlainTransport(t)); | |
| } | |
| } |
🤖 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/test/java/net/runelite/client/plugins/microbot/util/walker/banking/BankedTransportItemPlanningTest.java`
around lines 91 - 102, Add a non-empty assertion for the filtered currency list
in currencyTransportsRemainEligible before iterating, matching the sibling
tests’ assertFalse pattern and preserving the existing eligibility checks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
developmentbranch tomainfor Microbot 2.6.16.walkSteppartial-path termination fix.microbot.versionfrom2.6.15to2.6.16.Impact
Merging this PR triggers the main-branch release workflow, which builds the shaded client JAR, creates the
2.6.16GitHub release/tag, publishes the stable artifact, and updates the production client version.Review follow-up
Validation
./gradlew -q properties --console=plainresolvesmicrobot.version=2.6.16../gradlew :client:runUnitTests --no-daemon --console=plain: passed.