2.6.5#1794
Merged
Merged
Conversation
This passes the render mode into addRange, which is a few more draw calls, but means adding a new render mode is a lot easier later.
…gout detectUnexpectedLogout() keyed on !Microbot.isLoggedIn(), which is true during transient GameState transitions (LOADING/HOPPING/LOGGING_IN), not only on a real logout. Region or plane transitions such as climbing the Mining Guild ladder to bank therefore fire a false "unexpected logout"; auto-login then "recovers" a session that was never lost, and that recovery reschedules the next break. Because such transitions happen on every bank trip (more often than the break interval), the break timer keeps resetting and real scheduled breaks never fire. Fix: only treat a genuine logged-out GameState (LOGIN_SCREEN or CONNECTION_LOST) as an unexpected logout, and ignore the transient transition states. Strictly narrower than before, so it cannot miss a real logout. Evidence from a 5.5h F2P Mining Guild session: 13 "Unexpected logout detected" warnings, with 0 LOGIN_SCREEN, 0 CONNECTION_LOST, and 0 disconnects in the log, and the break state machine never once entered a real break (every "break cycle" was a 2-3s false recovery). Bump version 2.0.0 to 2.0.1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-logout fix(breakhandlerv2): don't treat LOADING transitions as unexpected logout
Exposed stamina pot usage and auto run settings inside the main microbot setting menu
…gate skip Three fixes for Rs2Walker transport and gate handling: 1. Prevent infinite loops with network transports (spirit trees, fairy rings, quetzals, gliders). The current-tile transport handler's "closer to goal" heuristic allowed taking any network transport whose destination was nearer the goal, creating A→B→A loops. Now network transports require the destination to be on the forward path points. 2. Don't skip segment handlers when the next path tile is unreachable. The 15s post-transport optimization blanket-skipped all handlers (including door/gate resolution) when no nearby transport was planned. Adding a reachability check ensures gates are still opened. 3. Widen blocker scan radius from 2 to 4 tiles. Path smoothing can place path points 3+ tiles from a gate object (e.g. Al Kharid gate), causing the blocker scan to filter it out and leaving the walker stuck in recovery clicks that never reach the gate.
The post-click sleepUntil waited up to 2000ms for the player to reach within 2-4 tiles of the click target, but had no check for the player stopping. When the player hit a gate or obstacle mid-walk, the walker sat idle for the remainder of the timeout before re-evaluating. Add !Rs2Player.isMoving() to the wake condition (after the 600ms game tick floor) so the walker reacts immediately when movement stops.
…breaks PATH_VARIANCE_TOLERANCE_CHEBYSHEV raised from 3 to 6. The variance check is the fallback when the BFS-based isNearPath() fails (e.g. a nearby wall blocks BFS despite the player being 2-3 tiles from the path). At 3, normal path-smoothing drift during walking triggered off-path-but-moving breaks that killed the click chain — the walker would watch the player walk without issuing new clicks, causing visible pauses when the player eventually stopped. At 6 tiles the walker keeps clicking through minor drift while still recalculating when genuinely lost.
isTileReachable() dispatches an unbounded BFS to the client thread for every path index in the inner walk loop. With 10+ indices per iteration, each blocking on the client thread round-trip, this caused multi-second delays before the first minimap click (9s observed on a 23-point path). Compute reachable tiles once via getReachableTilesFromTile(radius=45) before the inner loop and use HashMap lookups instead. One client-thread BFS call replaces N sequential blocking calls.
Hoist the cache variables above the outer tail loop and only recompute when the player's position changes. The outer loop can iterate up to 64 times (door settling, recovery clicks, interim yields) — previously each iteration recomputed the full radius-45 BFS on the client thread even when the player was standing still.
- Rewrite getReachableTilesFromTileInternal to use ArrayDeque BFS (O(n)) instead of O(n*d) HashMap scanning with per-tile HashSet allocation - Use raw tile-by-tile path for door/rockfall/transport detection instead of smoothed path, fixing gates missed between widely-spaced smooth points - Add unbounded BFS fallback for reachability check to eliminate false unreachable hits from bounded cache - Always recompute reachable tiles cache each iteration to pick up collision changes (gates opened by other players) - Skip reachable raw edges in door/rockfall scanning for performance; transport scanning always checks all edges (teleports aren't blockages) - Keep smoothed path for click targeting and spatial proximity checks
…outing Transport nodes (fairy rings, spirit trees, etc.) were placed in a separate pending queue ordered by g-cost, only processed when cheaper than the boundary's g-cost. This caused the pathfinder to walk 988 tiles instead of using fairy ring CIR (~260 tiles) because transport nodes at g=58 were starved behind walking nodes at g<58. Now all nodes share the A* boundary queue with proper heuristic, so a fairy ring destination at f=111 is explored immediately over a walking path at f=2475. Also: unbounded reachable tiles cache, remove isTileReachable fallback.
- refreshTransports: re-read config with = instead of &= so fairy rings / spirit trees / gliders recover after early startup calls when player data isn't loaded yet - stallThresholdMs: replace Rs2Player.isInteracting() (can get stuck returning true) with isMoving()||isAnimating() for the interacting multiplier - close bank before processWalk alongside existing closeWorldMap()
…nreachable The BFS in getReachableTilesFromTileInternal only checked 4 cardinal directions, but OSRS supports diagonal movement. Areas where the only path requires diagonal movement (around corners, through diagonal gaps) were falsely flagged unreachable, causing 4-second pauses while the walker ran expensive door/blocker scans on tiles that were actually walkable. - Add NE/NW/SE/SW diagonal expansion to the BFS, matching the collision checks used by the pathfinder's CollisionMap - Refresh the reachable cache before entering the heavy unreachable handler so transient blocks (stale cache) are caught cheaply - Add Taverley wall blocked edges (gate at x=2936 excluded)
…overy A* with Chebyshev heuristic can't discover multi-hop transport chains where the intermediate leg (walking to a fairy ring) moves AWAY from the target. The heuristic inflates f-cost on those nodes so the A* settles on a worse single-hop teleport before ever exploring the chain. Pre-seed the boundary queue with bridge nodes representing chained routes: item teleport → walk → network transport (fairy ring, spirit tree, gnome glider). Each bridge destination enters the queue with combined g-cost and its own heuristic, so the A* naturally evaluates the chain against direct routes. Also close world map before opening bank in banked-transport flows.
The transport refresh snapshot was captured AFTER filterSimilarTransports ran, baking target-specific filtering into the cached data. This forced targetPacked into the cache key, meaning every pathfind to a new destination triggered a full 450ms refresh of 11,738 transports. Move snapshot capture BEFORE filterSimilarTransports so the cached data is target-independent. filterSimilarTransports still runs after every cache restore (which it already did), so target-specific filtering is always applied fresh. Remove targetPacked from the cache key. Sequential pathfinds (birdhouse runs, farming runs) now hit the cache on the 2nd+ destination instead of paying 450ms each time.
walkWithBankedTransportsAndState ran compareRoutes on every walk, which internally calls getWalkPath 3 times + 2 explicit config.refresh calls — 5 full transport refreshes (450ms each) even for a 20-tile path. For short walks (<=200 Chebyshev), skip the comparison entirely and walk directly. Banking detours only make sense for long-distance routes where a teleport item from the bank could save significant tiles.
…ansport The current-tile transport handler oscillated on stairs: after a planned transport UP (segment handler), the current-tile handler saw the stairs at the new tile, the plane filter allowed it (playerPlane != targetPlane), and it took them back DOWN — creating an infinite up/down loop. Block transports whose destination matches lastTransportHandledAtLocation (where the player was before the last transport). This prevents the handler from undoing a transport that just completed.
…e paths The chain bridge injection could pick a teleport→network chain over nearby stairs for cross-plane walks because the heuristic ignores plane changes. A games necklace chain with f=200 beats a stair path whose true cost is 50 but explores 2M nodes to discover. Gate injection behind MIN_CHAIN_INJECT_DISTANCE (500 Chebyshev). Short walks use the normal A* which finds stairs through local expansion. Chains are only beneficial for long-distance routes where walking to a fairy ring after a teleport saves hundreds of tiles.
The chain injection approach was fundamentally wrong — it created phantom parent chains that don't correspond to real transports, could teleport players out of buildings for in-building walks, and required arbitrary distance guards to contain the damage. Reverted.
When start and target are within 200 Chebyshev tiles, skip refreshTeleports entirely so the A* only considers walking and local transports (stairs, doors). Prevents the pathfinder from choosing absurd teleport routes (games necklace to Corp) for in-building cross-plane walks where the stairs are nearby but the collision map makes them expensive to discover.
Tile (1621, 3822, plane=1) is fully blocked in collision-map.zip (N/E/S/W all false) despite being walkable in-game. This made the target unreachable, causing the A* to explore 2M nodes across the entire map and ultimately choose a teleport-to-Corp route for an in-building walk.
The pathfinder used a static collision-map.zip that can have incorrect data (tiles marked blocked when they're walkable in-game). This caused the A* to explore millions of nodes and choose absurd teleport routes for simple in-building walks. On each refresh, snapshot the game engine's live collision flags for all planes of the loaded scene (104x104). CollisionMap.n/e/s/w now check the live snapshot first for tiles within the scene, falling back to the static map only for tiles outside. The live data is always correct — it reflects actual walls, doors, and terrain as the game engine sees them. Removes the ignoreCollisionPacked bandaid for the Arceuus Library tile since the live data handles it properly.
CollisionMap is stored in a ThreadLocal — each thread gets its own instance. The snapshot was captured on the refresh thread's instance but the pathfinder runs on a different thread with a different CollisionMap that never received the snapshot. Making the snapshot fields and setter static ensures all threads see the same live data.
Two bugs found by review: 1. isBlocked() didn't check BLOCK_MOVEMENT_FULL from live data. A tile occupied by an object (rock, tree) with no directional flags set would appear walkable — the pathfinder could route through walls. Now checks BLOCK_MOVEMENT_FULL first. 2. Three separate volatile fields (liveFlags, liveBaseX, liveBaseY) could be read inconsistently during a scene change. Bundle into a single immutable LiveSnapshot object with one volatile reference. Also consolidate the repeated live-flag lookup into liveFlag().
The initial refreshTeleports guard was bypassed by the wilderness level initialization. wildernessLevel starts at 31, so on the FIRST node expansion the A* detected 'not in wilderness' and called refreshTeleports(node, 0) — adding all teleports including games necklace to Corp. This caused the pathfinder to teleport out of buildings for in-building cross-plane walks. For short-distance paths: set wildernessLevel=0 upfront (skipping the initial refresh), and guard the in-loop wilderness refresh with the shortDistance flag so teleports are never injected mid-search.
The live snapshot was REPLACING static data for scene tiles, which introduced a regression: closed doors in live data blocked paths that the static map showed as open, preventing the pathfinder from finding stair routes and forcing teleport detours. Now: if the static map says walkable, trust it. Only consult live data when the static map says blocked — the live data can reveal the tile is actually walkable (fixing stale static data) but can never make a walkable tile appear blocked.
…ort-zone Dijkstra - Revert live collision snapshot (CollisionMap + PathfinderConfig): the "unblock only" overlay had structural bugs (missing BLOCK_MOVEMENT_FULL, s()/w() not checking destination tile) causing paths through furniture. Walker already handles dynamic obstacles via door/transport handlers. - Restore upstream Pathfinder with pending queue (transports sorted by g-cost, walking nodes by f-cost). Our prior merge into boundary with heuristic made transport chain discovery worse, not better. - Transport-zone Dijkstra: walking nodes after a transport landing get heuristic=0 (Dijkstra) so A* freely explores to find the next transport in a chain. Pure walking retains full Chebyshev heuristic for performance. This fixes multi-hop routing (e.g. Ardougne cloak -> fairy ring). - Fix transportsPacked overwrite in refreshTeleports: the packed map was unconditionally overwritten with teleports-only, losing existing transports (stairs/doors) at the player's position. Now uses the merged set.
… short-path clicks - Skip path tiles on a different plane than the player in processWalk loop. These tiles trigger unreachable handlers, BFS refreshes, and door probes that can never succeed from the wrong plane. Eliminates 8-second stalls on cross-plane walks. - Bound getReachableTilesFromTile to HANDLER_RANGE*3 (39 tiles) instead of Integer.MAX_VALUE. Reduces BFS from ~10,816 tiles to ~6,000 max while covering routes around walls. - Add diagonal movement to isTileReachableInternal to match getReachableTilesFromTileInternal. Fixes disagreement between the two methods that caused oscillation on diagonally-reachable tiles. - For paths <= 5 tiles, set nextWalkingDistance to 0 so the path loop can click nearby tiles instead of spinning without issuing movement. - Lower banking skip threshold from 200 to 100 Chebyshev. - Recalculate path instead of giving up on pathfinder-still-null when target is still set.
…re off isSpiritTreeDestinationEnabled only checked the destination, so routes FROM a disabled tree (e.g. Farming Guild) to permanent trees (GE, Gnome Stronghold) still passed. Renamed to isSpiritTreeRouteEnabled and now rejects any route where either endpoint touches a toggled-off tree. Also removes dead pre-quest-gate assignments in refresh() that were immediately overwritten by refreshTransports(), and adds a defensive useSpiritTrees check in the walker's spirit tree dispatch.
Pre-existing Rs2Walker/Rs2Tile violations detected by the scanner after rebase onto upstream/development. No new violations introduced.
…heuristic cascade Three fixes: - Rs2Walker: track transport origin separately from landing tile so the reverse-transport filter correctly blocks A→B→A loops (cave entrance ping-pong). New lastTransportOriginLocation field, cleared on walk session start. - Rs2Tile: getReachableTilesFromTileInternal now fully honours ignoreCollision — cardinal direction flags and diagonal corridor flags are short-circuited, leaving only bounds checks. - Pathfinder: remove `|| node.heuristic == 0` from afterTransport checks in all three addNeighbors variants. The zero-heuristic no longer cascades past the first ring of walking nodes after a transport, restoring A* guidance for post-transport paths.
… scripted-walk pathfinder guard - Pathfinder: network-transport-aware admissible heuristic so teleport-> fairy/spirit-tree/glider/quetzal chains are discovered. Each enabled network hub becomes an A* landmark: h = min(directWalk, dist(node->origin) + min(dest->goal)). Stays admissible AND consistent (landmark set fixed per pathfind) so A* optimality holds, while the search is pulled toward useful hubs instead of ignoring them. Fixes the cloak->fairy->CIR route to the Farming Guild being passed over for a worse single teleport. Adds no graph edges (cannot teleport out of buildings) and never zeroes the heuristic (cannot collapse to whole-map Dijkstra), unlike the prior reverted bridge-injection and cascade attempts. - Rs2Walker: current-tile transport handler no longer fires off-path region-crossing transports (boat/ship/charter/canoe) by straight-line distance. A boat destination can be straight-line "closer" to the goal while sitting on another landmass, which made the walker click the Fossil Island rowboat (~17s of landing-timeout retries) before falling through to the planned teleport. These transports are now only used when explicitly on the planned path. - ShortestPathPlugin: skip onGameTick arrival-detection (which nulls the pathfinder) while a scripted Rs2Walker target is active, fixing intermittent pathfinder-still-null stalls.
The recovery-click rewrite in Rs2Walker added two lambda expressions, shifting every subsequent compiler-assigned lambda ordinal by +2. The guardrail baseline keys on those synthetic lambda names, so 26 entries renamed (same callers, same inferred-client-thread API calls) — no new real violations. Regenerated to match.
The opportunistic current-tile transport handler admitted off-path transports whose destination was straight-line "closer" to the goal, using WorldPoint#distanceTo which ignores the OSRS underground +6400 Y-offset. Across coordinate bands an inner underground region reads numerically closer to a surface goal than the dungeon exit beside it, so the walker re-took transports the pathfinder never chose — looping forever on the Mor Ul Rek cave entrance/exit and bouncing Varrock<-> Kourend teleports. Trust the pathfinder: the current-tile handler now admits only transports whose destination lies on the planned forward route, ordered by route position. Unify the one remaining distance check (madeProgressToward) and the pathfinder heuristic on a single band-aware helper (WorldPointUtil.undergroundAwareDistance) so the coordinate convention lives in exactly one place. - WorldPointUtil: add undergroundAwareDistance (min of direct and Y-band-folded Chebyshev) - Pathfinder: baseHeuristicToNearestTarget/baseHeuristicFromStart call the shared helper (byte-identical formula; admissibility unchanged) - Rs2Walker: forward-route-only admission; drop off-path distance branch and now-unused isRegionCrossingTransport/isNetworkTransport - Rs2WalkerProgress: band-aware madeProgressToward - add Rs2WalkerProgressTest locking the regression - regenerate client-thread guardrail baseline (lambda-ordinal renames)
Two walker fixes in handleCurrentTileTransportTowardPath and the path loop: 1. Nearby transport handler: source candidates from transports whose origin is reachable within 5 tiles of the player, not just the exact player tile, and dispatch via the transport's own origin so NPC/'Follow' transports auto-walk the short hop. NPC transports (e.g. Elkoy in the Tree Gnome Village maze) roam and sit a tile off the planned path, so exact-tile matching never took them and the walker thrashed through the maze instead. Keeps the destination-on-forward-route gate (prevents the Mor Ul Rek cave loop) and relies on getTransports() already being the usable/filtered set. 2. Door-settling yield: gate the main forward minimap click on isDoorInteractionSettling(). When a door is interacted with but traversal isn't yet confirmed, handleDoorsInRawSegment returns false and the loop fell through to the forward click in the same pass, clicking the tile behind the door and cancelling the open. Yield that pass instead; the next settled pass walks through. The door-edge-resolution branch is left untouched.
[ci skip]
…ite-master-1-12-29 # Conflicts: # gradle.properties
[codex] Merge latest RuneLite master into development
fix(walker): spirit tree filtering, transport-zone Dijkstra, cross-plane fixes
Contributor
WalkthroughThis PR delivers a comprehensive League 6 content update alongside significant infrastructure refactoring. The primary changes include: (1) extensive game constant additions across ItemID, NpcID, ObjectID, AnimationID, and related APIs to support new items, NPCs, and UI elements; (2) a GPU rendering architecture refactor moving entity-projection state from global caching to per-SceneContext storage with VAO range-based batching; (3) pathfinding enhancements introducing underground-aware distance calculations, transport landmark heuristics, and tile reachability via BFS; (4) Rs2Walker navigation overhaul with reachable-tile caching, nearby-transport probing, and raw-edge-aware segment handling; (5) break handler logout detection switched to GameState-based checking; and (6) microbot configuration additions for auto-run and stamina pot movement features. Version dependencies are bumped and world hopper extended with Japan, Singapore, and South Africa regions. Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
|
![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: 7
🧹 Nitpick comments (2)
runelite-api/src/main/java/net/runelite/api/gameval/NpcID.java (1)
68306-68314: ⚡ Quick winDuplicate Javadoc for semantically distinct constants.
Both
POH_SERVANT_DEMON_BASEandPOH_SERVANT_DEMON_LEAGUES_6share identical documentation ("Demon butler"), which doesn't explain the distinction between the base and Leagues 6 variants. Since this file is auto-generated, the fix would need to be applied to the generator to produce variant-specific documentation.🤖 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-api/src/main/java/net/runelite/api/gameval/NpcID.java` around lines 68306 - 68314, The Javadoc for POH_SERVANT_DEMON_BASE and POH_SERVANT_DEMON_LEAGUES_6 is identical and doesn't describe the variant; update the generator/template that emits NpcID constants so it produces variant-specific Javadoc (e.g., include the variant tag or suffix in the description like "Demon butler (base)" and "Demon butler (Leagues 6)") by using the NPC metadata/variant field when building the comment for constants POH_SERVANT_DEMON_BASE and POH_SERVANT_DEMON_LEAGUES_6 (and other variant pairs) so each generated constant gets a distinct, informative Javadoc.runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/Pathfinder.java (1)
296-312: 💤 Low valueDistance metric inconsistency between landmark and base heuristics.
applyLandmarkscomputes distance-to-landmark using plain Chebyshev (Math.max(Math.abs(...), Math.abs(...))), whilebaseHeuristicToNearestTargetandbaseHeuristicFromStartuseWorldPointUtil.undergroundAwareDistancewhich accounts for the underground Y-offset wrap.For routes involving underground tiles, the plain Chebyshev distance to landmarks will be significantly larger than underground-aware distance, reducing the landmark benefit for those paths. The heuristic remains admissible, but it's less tight than it could be.
♻️ Suggested fix for consistency
private int applyLandmarks(int packedPos, int base, int[] landmarks, int[] residuals) { if (landmarks == null || landmarks.length == 0) { return base; } int px = WorldPointUtil.unpackWorldX(packedPos); int py = WorldPointUtil.unpackWorldY(packedPos); int best = base; for (int i = 0; i < landmarks.length; i++) { int lx = WorldPointUtil.unpackWorldX(landmarks[i]); int ly = WorldPointUtil.unpackWorldY(landmarks[i]); - int viaHub = Math.max(Math.abs(px - lx), Math.abs(py - ly)) + residuals[i]; + int viaHub = WorldPointUtil.undergroundAwareDistance(px, py, lx, ly) + residuals[i]; if (viaHub < best) { best = viaHub; } } return best; }🤖 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 296 - 312, applyLandmarks uses a plain Chebyshev metric which is inconsistent with the underground-aware heuristics; change the distance computation to call WorldPointUtil.undergroundAwareDistance between the packed positions (packedPos and landmarks[i]) instead of unpacking and using Math.max; keep adding residuals[i] and comparing to base as before so best is updated using the same underground-aware metric used by baseHeuristicToNearestTarget/baseHeuristicFromStart.
🤖 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-api/src/main/java/net/runelite/api/gameval/ItemID.java`:
- Line 87486: Add a Javadoc comment for the constant POH_DUMMY_THRONE_8 to match
the existing pattern used for other item constants (a short descriptive line
above the declaration). Update the generator template that emits item constants
so it always outputs a Javadoc block for each entry (ensure the template uses
the item name/description source to populate the comment), and regenerate the
file so POH_DUMMY_THRONE_8 has the same descriptive comment style as the
surrounding constants.
In `@runelite-api/src/main/java/net/runelite/api/NpcID.java`:
- Line 6561: The deprecated NpcID shim replaced the historical DEMON_BUTLER
constant (old id 229) with the new id 7331 and removed DEMON_BUTLER_7331;
restore stable backwards compatibility by keeping DEMON_BUTLER with its original
value (229) and add a new constant DEMON_BUTLER_7331 = 7331 (or rename the
existing 7331 constant to DEMON_BUTLER_7331) inside the NpcID class so callers
using the unsuffixed alias continue to work while the new game id is exposed
with the _7331 suffix.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/ShortestPathPlugin.java`:
- Around line 570-572: The early return when Rs2Walker.getCurrentTarget() !=
null prevents the destination-completion logic from running; remove that
unconditional return in ShortestPathPlugin so the completion branch that clears
the target and scheduled future can execute even during active walks, or
refactor so only the scheduling/sending branch is skipped while still running
the completion check (invoke the existing completion/cleanup code paths that
clear target and scheduledFuture when destination reached).
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java`:
- Around line 8392-8396: The early-return fast path using chebyshevToTarget and
walkWithStateInternal skips banked-transport logic; change the branch so that
before returning you run the bank/route check (e.g., call compareRoutes or the
existing walkWithBankedTransports* path) to determine if banked
coins/keys/teleports are required—only then allow the direct walk. Specifically,
in the block that checks chebyshevToTarget, inspect forceBanking and invoke the
same route/missing-item analysis used by
compareRoutes/walkWithBankedTransports*; if that analysis says no banked
transport is needed, call walkWithStateInternal(target, distance), otherwise
proceed with the banked-transport handling instead of returning immediately.
- Around line 1033-1035: After calling Rs2Bank.closeBank() in Rs2Walker, wait
for the bank UI to actually close before invoking processWalk(): replace the
immediate transition to processWalk() with a sleepUntil-style wait (e.g.
sleepUntil(() -> !Rs2Bank.isOpen(), timeoutMs)) so the code polls
Rs2Bank.isOpen() and only calls processWalk() after it returns false; do not use
a fixed sleep, keep the timeout reasonable and log/handle failure if the bank
fails to close within the timeout.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/worldhopper/WorldHopperPlugin.java`:
- Around line 599-603: The region filter currently allows worlds with a null
region to bypass filtering; change the check in WorldHopperPlugin so that when
regionFilter is not empty, a world with world.getRegion() == null is treated as
non-matching and skipped. Replace the condition that now only checks
!regionFilter.contains(RegionFilterMode.of(world.getRegion())) with logic that
first verifies world.getRegion() is non-null and then checks
RegionFilterMode.of(world.getRegion()) against regionFilter (i.e., if
regionFilter is not empty and (world.getRegion() == null ||
!regionFilter.contains(RegionFilterMode.of(world.getRegion()))), continue). This
ensures regionFilter, regionFilter.contains(...),
RegionFilterMode.of(world.getRegion()), and world.getRegion() are the referenced
symbols to update.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/worldhopper/WorldSwitcherPanel.java`:
- Line 274: Region filter allows worlds with null region to bypass
include-filters; change the logic in WorldSwitcherPanel (the line using
regionFilterMode, world.getRegion(), and RegionFilterMode.of(...)) so that when
regionFilterMode is non-empty a null region is treated as a non-match and is
filtered out — i.e., replace the current condition with one that checks
(world.getRegion() == null ||
!regionFilterMode.contains(RegionFilterMode.of(world.getRegion()))) so
unknown-region worlds do not pass an active include filter.
---
Nitpick comments:
In `@runelite-api/src/main/java/net/runelite/api/gameval/NpcID.java`:
- Around line 68306-68314: The Javadoc for POH_SERVANT_DEMON_BASE and
POH_SERVANT_DEMON_LEAGUES_6 is identical and doesn't describe the variant;
update the generator/template that emits NpcID constants so it produces
variant-specific Javadoc (e.g., include the variant tag or suffix in the
description like "Demon butler (base)" and "Demon butler (Leagues 6)") by using
the NPC metadata/variant field when building the comment for constants
POH_SERVANT_DEMON_BASE and POH_SERVANT_DEMON_LEAGUES_6 (and other variant pairs)
so each generated constant gets a distinct, informative Javadoc.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/Pathfinder.java`:
- Around line 296-312: applyLandmarks uses a plain Chebyshev metric which is
inconsistent with the underground-aware heuristics; change the distance
computation to call WorldPointUtil.undergroundAwareDistance between the packed
positions (packedPos and landmarks[i]) instead of unpacking and using Math.max;
keep adding residuals[i] and comparing to base as before so best is updated
using the same underground-aware metric used by
baseHeuristicToNearestTarget/baseHeuristicFromStart.
🪄 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
Run ID: d827f71d-4060-40ca-ae19-bd8a4524e3b4
⛔ Files ignored due to path filters (4)
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/worldhopper/flag_jp.pngis excluded by!**/*.pngrunelite-client/src/main/resources/net/runelite/client/plugins/worldhopper/flag_sg.pngis excluded by!**/*.pngrunelite-client/src/main/resources/net/runelite/client/plugins/worldhopper/flag_za.pngis excluded by!**/*.png
📒 Files selected for processing (47)
gradle.propertieslibs.versions.tomlrunelite-api/src/main/interfaces/interfaces.tomlrunelite-api/src/main/java/net/runelite/api/ItemID.javarunelite-api/src/main/java/net/runelite/api/NpcID.javarunelite-api/src/main/java/net/runelite/api/NullItemID.javarunelite-api/src/main/java/net/runelite/api/NullNpcID.javarunelite-api/src/main/java/net/runelite/api/ObjectID.javarunelite-api/src/main/java/net/runelite/api/Renderable.javarunelite-api/src/main/java/net/runelite/api/Scene.javarunelite-api/src/main/java/net/runelite/api/ScriptID.javarunelite-api/src/main/java/net/runelite/api/gameval/AnimationID.javarunelite-api/src/main/java/net/runelite/api/gameval/DBTableID.javarunelite-api/src/main/java/net/runelite/api/gameval/InterfaceID.javarunelite-api/src/main/java/net/runelite/api/gameval/ItemID.javarunelite-api/src/main/java/net/runelite/api/gameval/NpcID.javarunelite-api/src/main/java/net/runelite/api/gameval/ObjectID.javarunelite-api/src/main/java/net/runelite/api/gameval/ObjectID1.javarunelite-api/src/main/java/net/runelite/api/gameval/SpotanimID.javarunelite-api/src/main/java/net/runelite/api/gameval/SpriteID.javarunelite-api/src/main/java/net/runelite/api/gameval/VarPlayerID.javarunelite-api/src/main/java/net/runelite/api/gameval/VarbitID.javarunelite-api/src/main/java/net/runelite/api/hooks/DrawCallbacks.javarunelite-client/src/main/java/net/runelite/client/plugins/cluescrolls/clues/hotcold/HotColdLocation.javarunelite-client/src/main/java/net/runelite/client/plugins/devtools/DevToolsPlugin.javarunelite-client/src/main/java/net/runelite/client/plugins/gpu/GpuPlugin.javarunelite-client/src/main/java/net/runelite/client/plugins/gpu/VAO.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotConfig.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotPlugin.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/Script.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/breakhandlerv2/BreakHandlerV2Plugin.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/breakhandlerv2/BreakHandlerV2Script.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/ShortestPathPlugin.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/WorldPointUtil.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/util/tile/Rs2Tile.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/shared/Rs2WalkerProgress.javarunelite-client/src/main/java/net/runelite/client/plugins/worldhopper/RegionFilterMode.javarunelite-client/src/main/java/net/runelite/client/plugins/worldhopper/WorldHopperPlugin.javarunelite-client/src/main/java/net/runelite/client/plugins/worldhopper/WorldSwitcherPanel.javarunelite-client/src/main/java/net/runelite/client/plugins/worldhopper/WorldTableRow.javarunelite-client/src/main/java/net/runelite/client/plugins/xpupdater/XpUpdaterPlugin.javarunelite-client/src/main/resources/item_variations.jsonrunelite-client/src/test/java/net/runelite/client/plugins/microbot/util/walker/shared/Rs2WalkerProgressTest.javarunelite-client/src/test/resources/threadsafety/client-thread-guardrail-baseline.txt
| * Uncharged trident (o) | ||
| */ | ||
| public static final int TOTS_UNCHARGED_ORN = 33434; | ||
| public static final int POH_DUMMY_THRONE_8 = 33436; |
Contributor
There was a problem hiding this comment.
Missing Javadoc comment for POH_DUMMY_THRONE_8.
This constant lacks a Javadoc comment while all other constants in the file follow the pattern of having descriptive documentation. Since this file is auto-generated, the issue likely lies in the generation script.
🤖 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-api/src/main/java/net/runelite/api/gameval/ItemID.java` at line
87486, Add a Javadoc comment for the constant POH_DUMMY_THRONE_8 to match the
existing pattern used for other item constants (a short descriptive line above
the declaration). Update the generator template that emits item constants so it
always outputs a Javadoc block for each entry (ensure the template uses the item
name/description source to populate the comment), and regenerate the file so
POH_DUMMY_THRONE_8 has the same descriptive comment style as the surrounding
constants.
| public static final int COOK_7329 = 7329; | ||
| public static final int BUTLER_7330 = 7330; | ||
| public static final int DEMON_BUTLER_7331 = 7331; | ||
| public static final int DEMON_BUTLER = 7331; |
Contributor
There was a problem hiding this comment.
Keep the historical DEMON_BUTLER alias stable in this deprecated shim.
Line 6561 repurposes DEMON_BUTLER from the old 229 id to 7331 and, per the diff summary, drops DEMON_BUTLER_7331. That changes the meaning of existing source code and also breaks callers that referenced the suffixed alias. Since net.runelite.api.NpcID is already a deprecated compatibility layer, it should preserve the old unsuffixed name and keep the new form suffixed here instead of mirroring the gameval rename.
🤖 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-api/src/main/java/net/runelite/api/NpcID.java` at line 6561, The
deprecated NpcID shim replaced the historical DEMON_BUTLER constant (old id 229)
with the new id 7331 and removed DEMON_BUTLER_7331; restore stable backwards
compatibility by keeping DEMON_BUTLER with its original value (229) and add a
new constant DEMON_BUTLER_7331 = 7331 (or rename the existing 7331 constant to
DEMON_BUTLER_7331) inside the NpcID class so callers using the unsuffixed alias
continue to work while the new game id is exposed with the _7331 suffix.
Comment on lines
+570
to
+572
| if (Rs2Walker.getCurrentTarget() != null) { | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
Early return bypasses destination-completion cleanup.
At Line 570, returning when Rs2Walker.getCurrentTarget() != null prevents the completion branch (Line 583-590) from ever clearing target/scheduled future during active walks, which is exactly when completion should be evaluated.
💡 Proposed fix
- if (Rs2Walker.getCurrentTarget() != null) {
- return;
- }
+ // Keep completion checks active even while walker has an active target.
+ // Active target indicates a walk in progress, not "skip completion logic".🤖 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/ShortestPathPlugin.java`
around lines 570 - 572, The early return when Rs2Walker.getCurrentTarget() !=
null prevents the destination-completion logic from running; remove that
unconditional return in ShortestPathPlugin so the completion branch that clears
the target and scheduled future can execute even during active walks, or
refactor so only the scheduling/sending branch is skipped while still running
the completion check (invoke the existing completion/cleanup code paths that
clear target and scheduledFuture when destination reached).
Comment on lines
+1033
to
+1035
| if (Rs2Bank.isOpen()) { | ||
| Rs2Bank.closeBank(); | ||
| } |
Contributor
There was a problem hiding this comment.
Wait for the bank UI to close before starting the walk.
This now issues Rs2Bank.closeBank() and immediately enters processWalk(), but elsewhere in this file bank close is treated as asynchronous. If the bank is still open on the next tick, the first click/interaction can be eaten by the bank interface and the walk starts in a bad state. As per coding guidelines, "Use sleepUntil(condition, timeoutMs) for waits; do not use fixed sleep(ms) to wait on game state."
🤖 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 1033 - 1035, After calling Rs2Bank.closeBank() in Rs2Walker, wait
for the bank UI to actually close before invoking processWalk(): replace the
immediate transition to processWalk() with a sleepUntil-style wait (e.g.
sleepUntil(() -> !Rs2Bank.isOpen(), timeoutMs)) so the code polls
Rs2Bank.isOpen() and only calls processWalk() after it returns false; do not use
a fixed sleep, keep the timeout reasonable and log/handle failure if the bank
fails to close within the timeout.
Comment on lines
+8392
to
+8396
| int chebyshevToTarget = pl.distanceTo(target); | ||
| if (!forceBanking && chebyshevToTarget <= 100) { | ||
| WebWalkLog.bankWalkDebug("skip_compare_short_distance dist={} goal={}", chebyshevToTarget, target); | ||
| return walkWithStateInternal(target, distance); | ||
| } |
Contributor
There was a problem hiding this comment.
This short-distance fast path bypasses banked-transport logic entirely.
Returning walkWithStateInternal(...) here means nearby targets never reach compareRoutes(...) or missing-item analysis. So walkWithBankedTransports* behaves like plain walking for any sub-100-tile destination that still needs banked coins/keys/teleports to be routed correctly.
🤖 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 8392 - 8396, The early-return fast path using chebyshevToTarget and
walkWithStateInternal skips banked-transport logic; change the branch so that
before returning you run the bank/route check (e.g., call compareRoutes or the
existing walkWithBankedTransports* path) to determine if banked
coins/keys/teleports are required—only then allow the direct walk. Specifically,
in the block that checks chebyshevToTarget, inspect forceBanking and invoke the
same route/missing-item analysis used by
compareRoutes/walkWithBankedTransports*; if that analysis says no banked
transport is needed, call walkWithStateInternal(target, distance), otherwise
proceed with the banked-transport handling instead of returning immediately.
Comment on lines
599
to
603
| // Check world region if filter is enabled | ||
| if (!regionFilter.isEmpty() && !regionFilter.contains(RegionFilterMode.of(world.getRegion()))) | ||
| if (!regionFilter.isEmpty() && world.getRegion() != null && !regionFilter.contains(RegionFilterMode.of(world.getRegion()))) | ||
| { | ||
| continue; | ||
| } |
Contributor
There was a problem hiding this comment.
Region filter is bypassed for unknown-region worlds.
At Line 600, a null region currently skips the filter check and lets that world through even when region filtering is enabled. That makes quick-hop behavior inconsistent with the selected region constraint.
Proposed fix
- if (!regionFilter.isEmpty() && world.getRegion() != null && !regionFilter.contains(RegionFilterMode.of(world.getRegion())))
+ if (!regionFilter.isEmpty() &&
+ (world.getRegion() == null || !regionFilter.contains(RegionFilterMode.of(world.getRegion()))))
{
continue;
}📝 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
| // Check world region if filter is enabled | |
| if (!regionFilter.isEmpty() && !regionFilter.contains(RegionFilterMode.of(world.getRegion()))) | |
| if (!regionFilter.isEmpty() && world.getRegion() != null && !regionFilter.contains(RegionFilterMode.of(world.getRegion()))) | |
| { | |
| continue; | |
| } | |
| // Check world region if filter is enabled | |
| if (!regionFilter.isEmpty() && | |
| (world.getRegion() == null || !regionFilter.contains(RegionFilterMode.of(world.getRegion())))) | |
| { | |
| continue; | |
| } |
🤖 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/worldhopper/WorldHopperPlugin.java`
around lines 599 - 603, The region filter currently allows worlds with a null
region to bypass filtering; change the check in WorldHopperPlugin so that when
regionFilter is not empty, a world with world.getRegion() == null is treated as
non-matching and skipped. Replace the condition that now only checks
!regionFilter.contains(RegionFilterMode.of(world.getRegion())) with logic that
first verifies world.getRegion() is non-null and then checks
RegionFilterMode.of(world.getRegion()) against regionFilter (i.e., if
regionFilter is not empty and (world.getRegion() == null ||
!regionFilter.contains(RegionFilterMode.of(world.getRegion()))), continue). This
ensures regionFilter, regionFilter.contains(...),
RegionFilterMode.of(world.getRegion()), and world.getRegion() are the referenced
symbols to update.
| } | ||
|
|
||
| if (!regionFilterMode.isEmpty() && !regionFilterMode.contains(RegionFilterMode.of(world.getRegion()))) | ||
| if (!regionFilterMode.isEmpty() && world.getRegion() != null && !regionFilterMode.contains(RegionFilterMode.of(world.getRegion()))) |
Contributor
There was a problem hiding this comment.
Region filter currently lets unknown-region worlds pass through
On Line 274, when regionFilterMode is active, world.getRegion() == null bypasses filtering and still gets displayed, which is inconsistent with include-filter behavior.
Suggested fix
- if (!regionFilterMode.isEmpty() && world.getRegion() != null && !regionFilterMode.contains(RegionFilterMode.of(world.getRegion())))
+ if (!regionFilterMode.isEmpty() && (world.getRegion() == null || !regionFilterMode.contains(RegionFilterMode.of(world.getRegion()))))
{
continue;
}📝 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
| if (!regionFilterMode.isEmpty() && world.getRegion() != null && !regionFilterMode.contains(RegionFilterMode.of(world.getRegion()))) | |
| if (!regionFilterMode.isEmpty() && (world.getRegion() == null || !regionFilterMode.contains(RegionFilterMode.of(world.getRegion())))) | |
| { | |
| continue; | |
| } |
🤖 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/worldhopper/WorldSwitcherPanel.java`
at line 274, Region filter allows worlds with null region to bypass
include-filters; change the logic in WorldSwitcherPanel (the line using
regionFilterMode, world.getRegion(), and RegionFilterMode.of(...)) so that when
regionFilterMode is non-empty a null region is treated as a non-match and is
filtered out — i.e., replace the current condition with one that checks
(world.getRegion() == null ||
!regionFilterMode.contains(RegionFilterMode.of(world.getRegion()))) so
unknown-region worlds do not pass an active include filter.
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
8 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.
No description provided.