Web walker: fix off-route minimap clicks and transport-cache churn; sync shortest-path + Quest Helper to upstream#1822
Open
infuse21 wants to merge 71 commits into
Open
Conversation
Merge development into main after successful CI.
Promote development to main.
Promote development with RuneLite startup and ground-item looting fixes.
Merge development into main for RuneLite 1.12.31
feat: improve plugin hub UI
Development
Release 2.6.12
Merge development into main for RuneLite 1.12.32
Release Microbot 2.6.14 on RuneLite 1.12.32
- Defers off-path recalculation while the player is still moving, animating, interacting, settling after doors/transports, or making recent route/interim progress. - Reworks route minimap clicks and unreachable-tile recovery to stay anchored to the planned raw path, use Euclidean minimap reach, and avoid backtracking to earlier nearby path branches. - Adds sticky interim checkpoint tracking so recovery/continuation clicks are not spammed while valid movement is already in flight. - Tightens door handling by deferring door probes while moving and only blacklisting wrong-traversal doors when the player started near the attempted door edge. - Improves charter ship destination widget lookup/clicking and optional confirmation handling.
cold start - hopefuly fixed Mid-route Stalling rs2 walker now accepts adjacent same-plane transport/objct landing
- Adds a per-loop `WalkLoopSnapshot` so route progress decisions reuse a stable player location and reachable-tile snapshot during each walker loop. - Reworks closest-path-index and route-progress tracking to prefer reachable distance when available, fall back safely when reachability is empty, and preserve forward progress on routes that double back near earlier branches. - Keeps primary minimap clicks, direct short-walk fallback, and unreachable recovery anchored to the raw shortest-path route before using smoothed waypoint nudges or generic fallbacks. - Adds raw-route target selection helpers with candidate predicates so recovery and continuation clicks stay on known walkable/unloaded route tiles. - Defers off-path recalculation while movement, interaction, animation, door/transport settling, recent clicks, route progress, or interim checkpoint progress indicate the player is still advancing. - Adds sticky interim checkpoint tracking and early `interim-in-flight` yields before broad scans, current-tile transport checks, direct short-walk attempts, and path segment scans. - Hints route progress after clicked-ahead checkpoints so later route anchoring does not snap back to nearby already-travelled branches. - Improves startup behavior by skipping speculative timeout-backed obstacle scans before the first movement click, while still allowing immediate planned transport steps to dispatch. - Suppresses active route idle nudges and stall recovery clicks when the current route segment is an explicit transport edge. - Tightens door/gate handling so wrong-traversal blacklisting only happens when the player started near the attempted door edge. - Accepts adjacent same-plane transport/object landings when the player settles near the expected destination instead of requiring the exact destination tile. - Improves charter ship handling by recursively resolving destination widgets, invoking the clickable widget/action, and confirming travel prompts when needed. - Updates `ShortestPathPlugin` route cleanup, pathfinder inventory quantity checks, WebWalk logging volume, movement docs, unit coverage, and the client-thread guardrail baseline. • Rs2Walker.java: immediate planned transports now suppress idle/stall recovery, so first-step teleports like Home Teleport should fire before pointless movement clicks. • Rs2Walker.java: active interim route clicks now yield before broad scans, reducing stop-start spam. • Rs2Walker.java: normal route clicks prefer raw shortest-path tiles instead of side/fallback tiles. • Rs2Walker.java: public walker entrypoints now reject client-thread calls before lock waits or player/tile reads. • Rs2WalkerUnitTest.java: added focused tests for route anchoring, raw-route click selection, interim yielding, and immediate-transport idle nudge suppression. • movement.md: documented the new walker gotchas Implemented the Al Kharid toll gate fix. What changed: • Added Pay-toll(10gp) transports for all four gate tiles in transports.tsv. • Added quest-free Open gate transports for Prince Ali Rescue completion. • Replaced the old quest-only tile restriction with blocked edge rules, so non-quest accounts can route through if they have 10 coins. • Walker now treats paid TRANSPORT rows as currency transports for item/banking checks. • Added special handling for Al Kharid toll gate confirmation: it uses Pay-toll(10gp) and clicks Yes, okay / Yes if prompted. I did not add the “talk to guard” fallback yet; this handles the cleaner object interaction path first.
…48b9742c1eaa667b18052785eb6ed058 (4.16.1) and marked the sync as successful in .quest-helper-sync. Kept the Microbot-specific integration intact: hidden always-on plugin wiring, QuestScript, config toggles, custom quest logic, resource path fixes, and the agent start hook. I also restored compatibility accessors needed by the automation runner and preserved precise farming patch coordinates where they matched older local data.
removed toll-gate helper pattern to avoid guardrail violation
Quest helper
Development web walker
Verified against the live tree on So-I-dont-Fuck-The-Walker-KEK: - #3 (POH TSV) reclassified OPEN -> NEEDS-VERIFY: teleportation_poh.tsv is absent by design; POH is routed programmatically via util/poh/, not a TSV. teleportation_portals.tsv (101 rows) is a different TELEPORTATION_PORTAL type (Castle Wars/god-wars), not POH. - chsami#27 (mushtrees) corrected 8 -> 12 of 16 pairs. - Confirmed boat world-view DONE (WorldPointUtil.fromLocalInstance), closing UPSTREAM_COMPARISON.md #6; PrimitiveIntList (chsami#11) still OPEN. - Added 2026-07-20 reconciliation note and a Facade migration section documenting the frozen 23-member ShortestPathPlugin coupling contract and the staged decouple-before-backport plan. Docs only; no walker/engine code changed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Read-only sweep of all 26 consumer files outside the shortestpath package. Documents the complete member surface Rs2PathApi must reproduce: - Pathfinder external surface: getPath, isDone (only 2). - PathfinderConfig: refresh, setUseBankItems, isUseBankItems, getTransports, getAllTransports, setIgnoreTeleportAndItems, isTransportUsableWithLeaguesContext, isUseSpiritTrees. - Transport: 15 getters/setters (getDestination/getDisplayInfo/getType dominate); TransportType: all 20 constants + isTeleport + valueOf. - WorldPointUtil: packWorldPoint/unpackWorldPoint/undergroundAwareDistance. - CollisionMap: only the static ignoreCollisionPacked field (Rs2Tile). Key finding: the facade only needs to wrap mutable plugin state (Pathfinder/PathfinderConfig/marker/future/mutex); Transport, TransportType and WorldPointUtil are pure data / pure functions and can stay as-is behind it. Docs only; no walker/engine code changed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Introduce a Microbot-owned facade (util/walker/Rs2PathApi) that delegates 1:1 to ShortestPathPlugin's mutable static surface catalogued in Stage 1: pathfinder lifecycle (get/set pathfinder, future, executor, mutex), config (getPathfinderConfig, setReachedDistance, override x3), target/walker state (exit, start-point flag, lastLocation), marker get/set, and getTransports; plus the CONFIG_GROUP and MARKER_IMAGE constants. Thin delegation only: no behaviour change, no state of its own. Value types (Pathfinder, PathfinderConfig, Transport) are intentionally not re-wrapped - they are the stable Microbot-facing path API. No consumers migrated yet (that is Stage 3, walker last); ShortestPathPlugin members stay public and binary-compatible until then. Compiles clean (:client:compileJava). Walker untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Route the low-risk, single-package consumers through the facade instead of ShortestPathPlugin directly: - Script.exit path -> Rs2PathApi.exit - questhelper/QuestScript -> Rs2PathApi.getMarker/exit/getPathfinder - util/bank/Rs2Bank, util/depositbox/Rs2DepositBox, util/npc/Rs2NpcManager, util/skills/slayer/Rs2Slayer -> Rs2PathApi.getPathfinderConfig - util/leaguetransport/Rs2LeaguesTransport: field access ShortestPathPlugin.pathfinderConfig -> Rs2PathApi.getPathfinderConfig() Import-only swap where all refs were method calls; value-type imports (Pathfinder, Transport, TransportType, PathfinderConfig) kept as-is. MicrobotPluginChoice keeps ShortestPathPlugin.class (config registration, not state). No residual ShortestPathPlugin refs in migrated files. Rs2Walker deferred to last. Compiles clean (:client:compileJava, exit 0). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Route the walker subpackage helpers through the facade: - lifecycle/Rs2WalkerLifecycleRuntime (26 refs): marker, pathfinder, future, executor, config, start-point, lastLocation, MARKER_IMAGE - banking/Rs2WalkerBankingPlanner (13 refs): getPathfinderConfig, override MARKER_IMAGE resolves to the facade's re-exported constant. Value-type imports (Pathfinder, Transport, TransportType) kept. awaits/ and shared/ subpackages import only value types (Pathfinder/WorldPointUtil) and need no migration. No residual ShortestPathPlugin refs; Rs2Walker.java (77 refs) remains for the final Stage 3c pass. Compiles clean (:client:compileJava, exit 0). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Route the walker itself (77 refs) through the facade. Rs2PathApi is in
the same package so no import is needed; the shortestpath.* wildcard
import stays for value types (Transport/TransportType/WorldPointUtil/etc).
- 5x ShortestPathPlugin.pathfinderConfig field reads -> getPathfinderConfig()
- all method calls -> Rs2PathApi.*
- method reference ShortestPathPlugin::getPathfinder -> Rs2PathApi::getPathfinder
- javadoc {@link ShortestPathPlugin#...} -> {@link Rs2PathApi#...}
No field assignments existed, so no missing-setter issues. Zero residual
ShortestPathPlugin refs in Rs2Walker. All consumers now go through the
facade; ShortestPathPlugin's public static surface is fully wrapped.
Verified: :client:compileJava exit 0; shortestpath unit+regression suite
green (73 tests: ShortestPathCoreTest 43, ShortestPathTier1RegressionTest
18, PathSmootherTest 6, TransportTypeTest 4, benchmark/refresh 1+1;
0 failures, 0 errors).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Record the staged-plan status: Stages 1 (catalogue), 2 (Rs2PathApi), and 3a/3b/3c (consumer migration incl. Rs2Walker) done with commit refs; Stage 4 (backport behind facade) is now the next step. Note that 100% of plugin-state access routes through Rs2PathApi and only plugin identity refs (.class / getSimpleName) remain. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Live-code verification against the KEK branch found most flagged "backport" items already implemented since the 2026-04-18 audit: - #2 wilderness boundaries DONE (PathfinderConfig.java:41-46 match the upstream values; isInLevel20/30Wilderness wired; Chebyshev landed) - #8 bidirectional BFS DONE (meet-in-the-middle, BIDIRECTIONAL_MIN_CHEBYSHEV) - chsami#31 currency-aware transport filter DONE (PathfinderConfig.java:879-892) - chsami#33/chsami#34 transport-index perf DONE (buildPathFirstIndex map) - chsami#11 stale premise: Node already packed-int, not List<Integer>; the only List<WorldPoint> left is the getPath() facade boundary (not a behind-the-facade change) Genuinely-open internal items remain: chsami#10 (reachability prune), chsami#12 (SplitFlagMap LRU eviction), chsami#30 (one-way flag), #3 (POH coverage) - correctness/data-sensitive, want real upstream source. No upstream remote is configured yet (still pinned to 07fca57). Docs only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Added the skretzo remote and fetched Skretzo/shortest-path. Upstream HEAD is 122 commits + a major refactor past the pinned 07fca57 (transport/ + transport/parser packages, Destination/ItemVariations/ JewelleryBoxTier model, NodeGraph/IntDeque/IntMinHeap/PrimitiveIntList pathfinder internals, standalone WildernessChecker, leagues/, PendingTask). Re-checked the open items against real current source: - #2 wilderness: byte-identical to upstream WildernessChecker -> DONE - #3 POH: real gap - upstream ships teleportation_portals_poh.tsv (137 rows); Microbot routes POH programmatically. Next target: diff that TSV against util/poh coverage and backfill. - chsami#10/chsami#12: no upstream counterpart (Microbot-original perf ideas) - chsami#30 one-way: not confirmed upstream; deprioritized Documented in UPSTREAM_COMPARISON.md "Re-baseline 2026-07-20" and updated the facade-migration staged plan (step 5 done). The divergence now only supports selective feature/data backports, which the facade makes safe. Docs only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Diffed Microbot's programmatic POH model against upstream's teleportation_portals_poh.tsv (137 rows / 50 dests) and the game's 42 ObjectID.POH_PORTAL_MAG_* portal-chamber destinations. PohPortal covered 28; added the 12 missing ones: Barbarian Outpost, Fishing Guild, Port Khazard, Marim, Civitas illa Fortis, Ourania, Cemetery, Dareeyak, Trollheim, Lassar, Ice Plateau, Paddewwa (PohPortal 28 -> 40). Each uses the authoritative arrival tile from the upstream data set and the MAG/MARBLE/TEAK object-id families (compiler-verified to exist; League variants omitted). They flow into the transport graph automatically: PohPortal.findPortalsInPoh() (BY_OBJECT_ID built from values()) -> PohPanel.portalPanel -> new PohTransport(...), so the walker now detects and routes through these portals when built in a POH. Verified: :client:compileJava clean; no duplicate enum constants; shortestpath regression suite green. Tracker #3 marked DONE in WEBWALKER_IMPROVEMENT_PLAN.md and UPSTREAM_COMPARISON.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Recorded in UPSTREAM_COMPARISON.md the file-level and content-level diff of transport data against current upstream: - File-level: the 4 upstream-only TSVs are handled (POH portals/boxes programmatically, quetzal whistle inline, home teleports partial); Microbot adds blocked_edges/dangerous_tiles/npcs/restrictions. - transports.tsv content: present in both (4949 vs 5168 rows). By origin->dest identity, 778 upstream-only pairs vs 553 Microbot-only. Only 14 named; 764 anonymous route objects (mostly Climb/Ladder/ Stairs). 713 of 737 upstream-only origins are brand-new tiles -> a real coverage gap, not coordinate drift. - Flagged bulk import as risky (action-column schema mismatch + collision-map coverage), recommended scoped per-region sync starting with the 14 named routes. Docs only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace bundled collision-map.zip with the current upstream build: 2724 regions (was 2663; +61 new regions covering Varlamore/Kebos and other recent content). Format verified compatible before swap: identical x_y region-entry naming, identical packPosition, and identical per-tile bit layout (FLAG_COUNT=2, same z/y/x index arithmetic) - upstream only refactored BitSet -> long[] internally, the on-disk byte encoding is unchanged, so Microbot's BitSet-based FlagMap decodes it correctly. Validated: shortestpath suite green (73 tests, incl. ShortestPathCoreTest 43 + Tier1 regression 18 doing real cross-region pathfinding; 0 fail). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The visible cold start is not the teleport - it is the gap between starting a walk and the walker doing anything at all: walk_start cache_miss reason=verify ... changed=varbits, slow refresh transports=2888ms pf_ready elapsed=2418ms transport_handoff_enter TELEPORTATION_SPELL <- only now does it act That first miss is attributed to varbits, a different cause from the varplayer cooldown fixed in the previous commit. TransportVarbit is structurally identical to TransportVarPlayer - same operators, same matches() - so it gets the same treatment: hash the condition verdict plus the condition identity rather than the raw varbit value. A varbit whose value moves without flipping any verdict cannot change a transport's usability, so it must not force a full re-evaluation the walker blocks on before its first action. Whether this removes the walk-start miss depends on whether that varbit's verdict genuinely flipped. If a miss still appears with changed=varbits it is a real usability change, and the remaining lever is the cost of a miss (~2.3s of useTransport over 5714 transports) rather than its frequency. The value provider is injectable, so the test asserts the real behaviour: 7 -> 9 against "> 5" does not invalidate, 7 -> 3 does. Extends movement.md chsami#23 to cover varbits. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A minimap click is resolved by the game's own pathing, so line of sight is irrelevant to walking - a player clicks past a corner, through a doorway, or around a building and the server routes them there. Clamping click targets to a straight walkable line made the walker advance corner-to-corner, stopping at each corner to re-aim before issuing the next click. That is a visible behavioural tell and no human walks that way. It also bought no correctness. The off-route click (3176,3428) that motivated the clamp was not caused by a lack of line of sight: selection returned null on a stale anchor, so the caller fell through to clamping a far smoothed waypoint to a Euclidean radius, which landed off the planned route. The game then pathed to that off-route tile its own way. Once the anchor self-heal landed, selection returns a raw-route point by construction and the LOS tier was doing nothing but shortening clicks. The invariant that actually matters is that the target sits ON the raw route: then however the server walks there, we arrive on the planned path. - Primary selection is again the furthest forward raw-route point within minimap reach, with no LOS and no bounded-reachability requirement (the latter could also shorten clicks without protecting anything the route membership does not). - The outside-clip route fallback drops its LOS-preferring tier too. - Removes hasWalkableLineOfSight and isOffSceneOrUnloaded, now unused. - Retained: the anchor self-heal (the real fix), raw-route-only selection, the reachability guard on the wall-distance fallback branch, and the door/gate pre-click guards, which are what stop a click skipping a closed door. Replaces LineOfSightClickRegressionTest with RouteClickTargetRegressionTest, which pins the correct invariant - the historic click is not on the raw route, and the route offers long forward reach within minimap range so clicks are not shortened to the nearest corner. Rewrites movement.md chsami#20 to record that LOS clamping was tried and reverted, so it is not reintroduced. Guardrail baseline regenerated: all 24 entries are lambda-index renumbering in untouched methods (26 removed, 24 added), no new non-lambda violations. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ary selection chsami#19 instructed selecting the primary click from the player's bounded reachable set "not walkability". The primary selector now uses isKnownWalkableOrUnloaded, so the entry contradicted the code it documents. Scope it to what is still true and still implemented: reachability gating belongs on click targets that are invented rather than taken from the route - the getPointWithWallDistance nudge and the scaled-radius directional fallback - plus the bidirectional rejoin. Records that gating primary selection was tried and reverted, since a BFS bounded by click radius can exclude legitimate around-a-corner points and shorten clicks for no benefit: a point taken from the raw route is safe by construction (chsami#20). Documentation only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Full audit of all 23 entries against the current source. Symbol check: every symbol named in a "Where this applies" line resolves. Five symbols (atTransportDestination, DIRECT_CLICK_MAX_DISTANCE, clickOptimisticRecoveryTarget, isRouteActive, playerIsIdleForShortWindow) appear only inside "Pattern to follow" blocks as illustrative pseudo-code, which is easy to mistake for a real API when grepping. Numeric claims are consistent with the code: chsami#21 correctly describes 2500ms as the previous idle-nudge window and quotes the current 1200ms constant, and the walker lock #1 refers to exists. Two fixes: - #5 says to suppress the inverse adjacent transport after a *successful* crossing. That is incomplete in isolation: a reader following only #5 reproduces the bug chsami#22 fixes, because adjacent transports require an exact-tile landing and a crossing can succeed while the landing check fails. Cross-referenced. - Added a reading guide noting that pattern blocks may be pseudo-code, that "Where this applies" is the authoritative symbol list, and that chsami#19/chsami#20 deliberately record reverted approaches. Documentation only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Route selection always returned the furthest candidate inside a fixed radius (MINIMAP_REACH_EUCLIDEAN - 1), so every click covered the same tile span. The existing randomisation nearby does not address this: nextWalkingDistance (9-12) gates whether a waypoint is far enough to be worth clicking, and proximityWake (2-4) gates the post-click wait, but neither varies the reach used to pick the tile. Jitter the reach per click via routeClickReach, applied inside selectRouteClickTarget so every click path picks it up rather than only the main route loop. Varying reach is the safe axis: it changes only how far along the route the target sits, never sideways, so the target stays on the planned route. Lateral tile offsets are the wrong axis - they leave the route, which is what produced clicks on the wrong side of a fence or inside a building, and route membership is what keeps the server's own pathing on our route. Lateral randomness belongs inside the tile, in click-point jitter. Bounds are load-bearing. The floor (7) stays clear of INTERIM_CLOSE_TILES (5) so a short click cannot land inside the interim-close threshold, clear the checkpoint immediately and cause click thrash. The ceiling is the caller's reach, already tuned to the minimap clip. A shortened reach also retries at full reach before giving up, so it can never be the reason selection returns null and drops the click onto the off-route wall-nudge clamp. Adds movement.md chsami#24 and tests for the bounds, actual variation, and degenerate reach values. Not yet validated live. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The player-anchored retry ran at full reach, bypassing the jitter. That retry fires on most first clicks of a route (sel=route@player), so in practice the variation was skipped exactly where it is most visible: measured first-click distances clustered at 9.0-10.0 rather than spreading across the 7-10 band. Use the jittered reach there as well, keeping the full-reach fallback so a shortened reach still cannot be the reason selection returns null. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
Important Review skippedToo many files! This PR contains 328 files, which is 228 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (336)
You can disable this status message by setting the 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 |
The class javadoc still said no consumers had been switched over yet and that this was still pending as Stage 3, but Stage 3 completed three commits later in this same branch (eb0e274 leaf consumers, 6ee086b walker-package helpers, e6e7f0c Rs2Walker). A reviewer opening the facade was told the migration had not happened when it had. Records the end state instead, plus the grep that verifies it: the only remaining ShortestPathPlugin static references under microbot/util/ are the delegations in this class, which is the invariant that confines an upstream backport to this one file. Doc only; no behaviour change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A walk to an underground goal reported UNREACHABLE while it was still advancing. Two defects combined. partialPath is recomputed every iteration as "path end is further than distance from the goal". The pathfinder stopped 31 tiles short of (3087,9720), so it was true from the first snapshot onward and the retry budget was armed for the whole walk rather than for a stuck stretch. The path loop also exits for good reasons. Opening a door ends the iteration and lands in the same branch as a failure to move, so it spent a retry. Observed: retry 1 at the end of the first segment, ~100 tiles and 46 seconds of successful walking, then retries 2 and 3 within one second of a single door open - the second without the player moving at all. It gave up on the surface having never reached the ladder. Refill the budget when route progress advanced since the last charge, and exempt exit reasons that denote work done (door/transport/blocker handled, movement in flight). The exemption is deliberately narrow: not-near-path, click-failed-off-minimap and the waiting/retry states still consume it, so a genuinely unreachable goal still terminates. Also stop passing placeholders to recordUnreachable. pathEndpoint was the player's location and pathSize a literal 0, so the log read as "pathfinder returned an empty path at your feet" and endpointToTarget became a surface-to-underground y delta (6346) instead of the real 31-tile shortfall. Underground is the same plane at y+6400, so WorldPoint.distanceTo does not bail out across that boundary - it just returns a meaningless number. movement.md chsami#25 and chsami#26 record both. Tests: :client:runUnitTests green, 1113 tests, 0 failures. Guardrail reports 961 known violations, no regressions - baseline unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The refill condition used routeProgressAdvancedAtMs alone. That field is also bumped by stabilizeRouteProgressIndex whenever the route is merely REPLACED (path size or endpoints differ), and every partial retry calls recalculatePath() - so a retry could refill the budget it had just spent. The path stabilises once the walker is truly stuck, so it still terminated, but only after extra iterations, and a route that flapped between two shapes on successive recalcs could have kept refilling until the 64-iteration tail cap. Also require the player to be standing somewhere new since the last retry. When a target is genuinely unreachable the player stops moving, so movement is the signal that separates "advancing" from "stuck" - which is what lets the budget drain and report UNREACHABLE. Found while probing a walk whose goal (3087,9720) is blocked in the collision map, with 0 walkable tiles within 6. The pathfinder correctly stopped at (3056,9720), the last walkable tile before the rock face. Tests: :client:runUnitTests green, 1113 tests, 0 failures. Guardrail 961 known violations, no regressions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A walk to (3087,9720) - inside the rock east of the Dwarven Mine, with zero walkable tiles within 6 - spent ~80s covering 100+ tiles from Draynor, went underground, stopped at (3056,9720) which is the last walkable tile before the rock face, and then reported partial-retries-exhausted. The pathfinder was right; a path request from (3056,9720) to the goal returns size 1, meaning you cannot leave that tile toward the target. Only the diagnosis was wrong: the failure read as a walker fault rather than a bad destination coordinate. The intended target was the Motherlode cave mouth at (3060,9766). processWalk now checks the collision map for any walkable tile within the arrival distance and returns UNREACHABLE immediately with cause target-not-walkable, naming the nearest walkable tile so the coordinate can be corrected. The guard is deliberately permissive. An unmapped region reads as fully blocked, because SplitFlagMap.get returns false for one and CollisionMap.isBlocked negates all four directions - so a naive check would reject every instance and every region absent from the collision map. Adds SplitFlagMap.hasRegion and a CollisionMap delegate, and every ambiguous case (no map, unmapped target, unmapped tile in the search box) returns walkable and lets the pathfinder decide. Only mapped, wholly blocked terrain is rejected. Tests: 5 new cases against the real collision map, covering the buried coordinate, the cave mouth and last-walkable tile still being accepted, unmapped regions never being rejected, and a generous arrival distance reaching real ground. Suite green, 1118 tests, 0 failures. Guardrail 961 known violations, no regressions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
handleRockfall required BOTH the player and currentTarget to be in region 14936, so any walk leaving the Motherlode Mine bailed before inspecting anything. Observed live at=(3751,5672) goal=(3040,9810): the goal is region 12185, so no outbound route could mine through a rockfall standing directly in its path. Approaching the mine from outside was equally dead, since the player is not in 14936 yet. Rockfalls only exist in the Motherlode Mine, so a region gate is right - it just has to test where the rockfall could BE (the player's tile, or the path tiles about to be inspected), never where the walk ends. Extracted as isMotherlodeRockfallCandidate so both directions are testable without a client. Also defer the pickaxe check until a rockfall is actually found. It ran before any rockfall was located and called setTarget(null), so a route merely passing through the mine without a pickaxe was killed outright. Now only a rockfall genuinely in the way abandons the walk. Note this handler still only fires when a rockfall lies ON the path, and transports.tsv contains no rockfall entries (grep -ic rockfall = 0), while the collision map marks those tiles blocked. So the pathfinder never routes through one and the handler stays dormant. Cataloguing MLM rockfalls as Mine;Rockfall;26679/26680 transports - mirroring the existing Mine;Rock slide;2634 entries - is what will actually exercise it. That needs per-rockfall coordinates from a live session and is tracked separately. Tests: 3 new gate cases (leaving the mine, approaching from outside, closed elsewhere). Suite green, 1121 tests, 0 failures. Guardrail 961 known violations, no regressions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
An obstacle that stands BETWEEN two walkable tiles - Motherlode rockfalls, rubble, rock slides - can only be modelled as a transport if you know both of its open ends, and the object's own position does not give you those. /objects returns position only, so collecting them meant reading coordinates off a screenshot. Reports each match's surrounding tiles with live-scene walkability, plus a "sides" object pairing opposing neighbours on each axis: the axis whose bothWalkable is true is the transport's origin/destination pair. Walkability is read from the live scene collision flags rather than the bundled collision map, which is the point - the bundled map has these obstacles baked in as permanent walls, so it cannot tell you they are passable. Also reports selfWalkable so a cleared obstacle is distinguishable from a standing one. The flag array is read once per request inside a single client-thread call. Rs2Tile.isWalkable hops the client thread per invocation, so the obvious implementation would have cost 8 hops per object. Tests: suite green, 1121 tests, 0 failures. Client-thread guardrail 961 known violations, no regressions. Documented in AGENT_SERVER.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The bundled collision map bakes rockfalls in as permanent walls. That is correct for upstream, which has no automation to clear them, but it left the mine carved into disconnected pockets: a route from the cave arrival tile (3728,5692) to the WEST_LOWER mining area died 10 tiles short, and walking anywhere inside the mine mostly failed. Rs2Walker.handleRockfall already knew how to mine 26679/26680, but it only fires when a rockfall lies ON the path - and since nothing contradicted the collision map, the pathfinder never routed through one. The handler was effectively dead code. Each rockfall is now a Mine;Rockfall;<id> transport between its two open sides, mirroring the existing Mine;Rock slide;2634 entries. 32 spawn points, 90 rows: 19 are chokepoints where exactly one axis is open, 13 are bypassable and only add an alternative edge. Duration 10 keeps an open corridor preferred over mining when both exist. Positions came from the live scene via GET /objects/neighbours, unioned across 9 samples taken minutes apart - rockfalls respawn, and a single snapshot was already one short. All 32 appeared in every sample, so the set looks complete, though a spot that was cleared for the whole session would not have been seen. A missing rockfall is safe: it just stays a wall, which is today's behaviour. Item requirements use ';' (alternatives) rather than the ' ' separator the existing rock slide row uses. Note this is presentational only: Transport parses the column into Set<Set<Integer>>, but PathfinderConfig.hasRequiredItems flattens it and calls anyMatch, so either separator behaves as "any one pickaxe". ';' matches the data model's intent and reads correctly. Tests: 4 new offline cases against the real collision map and transport data, incl. the original failure (arrival -> WEST_LOWER, previously 10 tiles short) and a full Draynor -> mining area route. Suite green, 1125 tests, 0 failures. Guardrail 961 known violations, no regressions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
hasRequiredTransportItems only admitted plain TRANSPORT entries carrying a currency cost, so an item-gated obstacle - a machete for a jungle bush, a pickaxe for a Motherlode rockfall, a Shantay pass - fell through every branch to the catch-all "return true", was never reported missing by getMissingTransports, and was never withdrawn. That contradicted the pathfinder. PathfinderConfig.hasRequiredItems ORs in Rs2Bank.hasItem when useBankItems is set, and refreshAvailableItemIds includes bank contents on the same flag, so a route was planned through the obstacle on the strength of a banked item the planner then declined to fetch - stranding the walk at the obstacle it had just routed through. Extracted the decision as planningCoversPlainTransport so both directions are testable without a client. This only widens which transports are ELIGIBLE, not how many bank trips happen. Collection is path-scoped: getTransportsForDestination pathfinds first and inspects only transports on the resulting route, so an item is fetched solely when the chosen route actually needs it. The downstream machinery already handled generic items - getMissingTransportItemIdsWith- Quantities walks getItemIdRequirements for any transport, picks whichever alternative has the most in bank, and uses quantity 1 for non-currency transports - so nothing else had to change. 235 rows in transports.tsv become eligible: 90 Motherlode rockfalls (new in this branch), 102 machete-gated jungle bush/tree/vines, 10 White Wolf Mountain rock slides, 6 Shantay passes, and ~27 keyed doors, ladders and slopes. Only the Motherlode case is verifiable offline; the Karamja and desert routes are untested live. Tests: 5 new cases over the real transport data - every item-gated plain transport eligible, every requirement-free one still ignored, currency transports unchanged, rockfalls covered, and null/non-plain types rejected. Full suite green, guardrail 961 known violations, no regressions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ting" This reverts commit d16e9d1.
This reverts commit f9aef42.
Region 58_88 (Motherlode Mine, x3712-3775 y5632-5695) regressed in 631884f "update collision map" (23 Jan 2026). Connectivity from the cave arrival tile (3728,5692) collapsed from 1473 reachable tiles to 192 - 87% of the mine cut off - and every lower-floor mining spot became unreachable: a585dc3 (19 Nov 2025) walkable=1623 reachable=1473 spots OK 631884f (23 Jan 2026) walkable=1580 reachable=192 spots dead a19a334 (12 May 2026) walkable=1606 reachable=192 spots dead f8b1138 (20 Jul 2026) walkable=1606 reachable=192 spots dead This is why the world map could once be used to walk into Motherlode and then could not: the pathfinder had no route through the mine, so any target inside it was unreachable regardless of how it was set. Restores just that one 952-byte region from a585dc3. Every other region is left on the current upstream sync - verified by comparing decompressed entries, only 58_88 differs. Safe despite the age: Motherlode Mine last changed in the game on 8 May 2024 (Project Rebalance, minor follow-up 15 May 2024), so the 19 Nov 2025 capture postdates the last map change by 18 months. The January regression was a bad regeneration, not a map update. After: walkable=1623 reachable=1473, with WEST_LOWER, WEST_MID, SOUTH_EAST and SOUTH_WEST all reachable from the arrival tile. The upper floor (WEST_UPPER, EAST_UPPER) stays unreachable in every version including the good one - that needs the ladder catalogued, which is gated behind the 100 golden nugget unlock and is tracked separately. With the mine genuinely connected, Rs2Walker.handleRockfall works as designed: routes now pass through the mine and it mines a rockfall that lies on the path. That is the correct model, unlike the transport rows reverted in 289090d - a transport relocates the player, and mining a rockfall does not. The archive is byte-larger-compressed differently (1197109 -> 1058342) because it was rewritten by a different deflate implementation; entry names, count (2724) and decompressed contents are unchanged apart from 58_88. Verified offline by decoding the FlagMap bitset directly. NOT yet verified in-game. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…uting" This reverts 5c06b86, restoring the pre-flight guard. It was reverted because it appeared to block legitimate walks into the Motherlode Mine. That diagnosis was wrong. The walks it rejected were targets in genuine solid rock in the Dwarven Mine (region 48_151): (3079,9710), (3086,9719), (3087,9720), (3087,9723), (3087,9724) - all produced by clicking the Motherlode inset on the world map, which resolves against the host region's coordinate space and so never yields a Motherlode tile. The guard was correct to refuse them, and naming the nearest walkable tile was the most useful diagnostic in the whole session. What was actually broken was collision region 58_88, fixed in cf8cf03. The revert reasoning also claimed the guard turns "degraded but progressing" into hard failure. That does not hold up: walking to the nearest tile to a rock face achieves nothing, and it costs ~30s before failing with a less specific error. The remaining objection - that collision data can be wrong, so "blocked" is not proof of "impossible" - is real, and the guard already answers it by abstaining wherever data is absent: no map, unmapped target, or any unmapped tile in the search box all return walkable and let the pathfinder decide. Only mapped, wholly blocked terrain is rejected. Not yet verified: this has not been compiled or tested since the revert, and TargetWalkabilityPreflightTest asserts against Dwarven Mine coordinates that cf8cf03 does not touch, so it should be unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds an immutable live-collision overlay on top of the static SplitFlagMap so pathfinding and smoothing can reflect the loaded scene, gated by a config flag that defaults OFF. Out-of-scene routing is unchanged — the static map still owns global routes. New pathfinder/live package: - LiveCollisionSnapshot: immutable, world-addressed, known/value per edge so the scene rim reads as unknown and falls back to the static map. - LiveCollisionOverlay: shared, atomically-swapped volatile holder read lock-free by every CollisionMap. - LiveCollisionCapture: client-thread builder mirroring Rs2Tile's CollisionDataFlag translation; instances return null (static fallback) pending dedicated chunk mapping. CollisionMap.get() consults a per-search pinned snapshot then the static map, so both neighbour expansion and PathSmoother pick it up from one place. PathfinderConfig owns the shared overlay behind its ThreadLocal<CollisionMap>; Pathfinder.run() pins the snapshot per search so a mid-search swap cannot mix two scenes. ShortestPathPlugin rebuilds the snapshot on scene-base change from the onGameTick client thread (no per-tick allocation). With the flag off a CollisionMap reads byte-for-byte as the static-only map, so the existing shortestpath suite is unchanged. New LiveCollisionTest covers the flag translation (walls, symmetry, full-block closing all four edges), rim/out-of-scene fallback, overlay-wins-in-scene, and no-overlay determinism. Guardrail stays green. Stage 2 (door/openable-edge preservation) and Stage 3 (debounced scene-change recalc) follow. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ge 2) A closed door blocks its doorway in RuneLite's live collision, so with the overlay on the pathfinder would route around an openable door instead of walking up and opening it — the static map treats the doorway as passable and the runtime door subsystem (Rs2Walker.handleDoors) opens the physical door as the path crosses it. LiveCollisionCapture now scans the loaded scene on the client thread for wall objects with a door action (open / go-through / walk-through / pass / pick-lock / pay-toll, incl. multiloc impostors) and leaves every edge touching such a tile UNKNOWN in the snapshot. Unknown edges already fall back to the static map in CollisionMap.get, so the existing door pipeline keeps owning doors unchanged — no CollisionMap change needed. Transports need no handling: getNeighbors adds transport edges independently of the collision traversability check, so a live-blocked walking edge never removes a transport edge. That, plus the door exemption, covers the false-"unreachable" cases without a blanket fallback that would defeat live blocking of genuine new obstacles. Tests: door-tile edges stay unknown even when live flags block all four sides; null door mask matches the no-door path. Full suite and guardrail green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
GET /live-collision[?x=&y=&plane=] reports whether the live-collision overlay is enabled and captured, the snapshot's scene base, and per-tile the overlay's own edge readings (overlayRaw: true/false/null=defer-to-static), the resolved edges the pathfinder uses, and the static-only edges. Where resolved differs from static, live collision is changing routing. The overlay and pathfinder are otherwise not observable over HTTP; this exists to verify the hybrid live-collision work against a running client. Logic lives in PathfinderConfig.liveCollisionDiagnostics (immutable reads, any thread); the handler is thin. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
RuneLite's collision for the outer ring of the loaded scene is incomplete while adjacent chunks are only partially loaded. Verified against a running client: the border produced a uniform band of ~1260 false walls out to about five tiles deep, while the deep interior matched reality. Stage 1's rim handling only exempted the outermost neighbour-out-of-scene edges, so those false walls leaked into the overlay and would fabricate obstacles near every scene boundary. LiveCollisionCapture now treats any tile within SCENE_BORDER_MARGIN (8) of the scene edge as unknown, deferring to the static map there. The scene always loads centred on the player (~tile 52 of 104), so the trusted ~88x88 interior still covers everything near the player, and the border recentres before the player reaches it. Eight gives headroom over the ~5 tiles of artifact observed (real live-vs-static differences began at ~24 tiles in, a clean gap). Re-verified live after the fix: border diffs 1260 -> 0, while the genuine interior corrections (incl. 3 stale-static unblocks) are preserved. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…stage 3) Stage 1 refreshed the overlay only on scene-base change, so a door closing or a temporary object appearing mid-scene was not picked up until the next reload, and the walker would walk into it and stall. Now object spawn/despawn (game + wall) mark the snapshot dirty, and refreshLiveCollision rebuilds it at most once per tick. After a mid-scene rebuild it validates the walker's remaining route against the fresh overlay via LiveRouteValidator and, if a walking step within a 15-tile look-ahead is now blocked, calls the walker's public recalculatePath() so it routes around before stalling. Cooldown-gated (3s) so churn cannot spam recalcs. Driven entirely from the plugin's onGameTick (client thread) using public walker API — the walker's internal loop is untouched. Openable doors and transports are skipped by LiveRouteValidator (door edges are unknown in the overlay, transport jumps are non-adjacent), so this never fights the runtime door handler. Scene reloads do not trigger a proactive recalc (they usually just recenter); the walker's own machinery picks up the fresh snapshot there. LiveRouteValidator is a pure helper with unit tests: nearest-index, detects a live-blocked walking step, skips transport/cross-plane jumps, respects the look-ahead bound. The four object subscribers only set a volatile flag (no client reads), so the guardrail is unaffected. Full suite green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ision Global collision/hybrid live collision
Contributor
Author
|
this also adds a new config option in the web walker under development. use live collision, Off by default. |
…nto So-I-dont-Fuck-The-Walker-KEK # Conflicts: # docs/entity-guides/movement.md
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
2 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.
Web walker: upstream sync, path-API facade, and route-click fixes
39 commits, 326 files. Four independent workstreams — they can be reviewed (or taken) separately, in the order below. 91% of the file count is a mechanical upstream sync.
been validated live no gamebreaking issues found. walks so much better now: reach jitter (committed after the last test run).
Partially addressed: the cold start is much less frequent but the worst case is unchanged — if a cache miss is already in flight when a walk starts, that walk still waits (~3.1s observed).
Known open items (deliberately not in this PR)
accepted, and a successful crossing reportedfailedand cost a 5s timeout. The right test is route progress past the transport edge, not distance to a tile. It touches every transport type, so it deserves its own change and its own testing.useTransport()cost — ~2.3s across 5,714 plain transports dominates any genuine cache miss. Only worth attacking now that misses are rare.Testing
:client:runUnitTestsgreen — 1111 tests, 0 failures, 4 skipped (pre-existing), 47s.New/updated coverage:
RouteClickTargetRegressionTest— on-route invariant, forward reach past cornersPathfinderConfigTransportRefreshHashTest— skill / varbit / varplayer verdict behaviour, including that a ticking cooldown does not invalidate but an expiring one doesShortestPathCoreTestRs2WalkerUnitTest— bidirectional rejoin, stale-anchor scan, reach-jitter bounds, adjacent-shortcut suppressionThe client-thread guardrail baseline was regenerated; each time the diff was verified to be lambda-index renumbering in untouched methods plus relocated calls, with no new non-lambda violations.
movement.mdgotchas were audited against source: #19 and #20 contradicted the shipped code and were corrected, #5 was cross-referenced to #22, and a reading guide was added noting that "Pattern to follow" blocks may contain pseudo-code.