Conversation
TransportNode's constructor was pre-adding previous.cost to travelTime,
then passing that sum as `wait` to Node(WorldPoint, Node, int) — which
re-adds previous.cost. The upshot: any TransportNode chained off another
TransportNode had its cost double-counted. This only surfaced once a
real transport->transport chain became the only viable path (e.g. Map
of Alacrity -> agility shortcut), which is why it stayed latent.
Concrete impact: "go to nearest bank" from outside Nemus retreat picked
Shilo (cost 55) over Nemus (observed 74, should be 40 = 34 MoA + 3
climb + 3 walk). The shortcut's TransportNode was being stamped at g=71
instead of g=37, tripling the cost of every agility-shortcut edge
downstream of a teleport.
Passing the final absolute cost through Node(int, Node, int cost)
instead of the wait-based constructor also sidesteps a second bug: for
plane-crossing transports with travelTime=0 (e.g. Slayer Tower chains),
the wait<=0 branch of Node.cost falls back to
WorldPointUtil.distanceBetween, which returns Integer.MAX_VALUE across
planes — overflowing previousCost + distance into Integer.MIN_VALUE
territory and making those transports look "free". The absolute-cost
path never consults distanceBetween.
Also in this pass:
- CollisionMap [MoA] debug log: report actual per-transport costs
instead of the hardcoded `+ 4` (all MoA durations happened to be
4, but the log was incidental to the value, not tracking it).
- Rs2Walker.findMoaWidget: token-set membership instead of
substring match — avoids false positives like token "log"
matching a hypothetical "logstrum" widget label.
MoaAuditPlugin and the runMoaAudit()/closeMoaWidgetIfOpen() helpers in Rs2Walker were intended as a temporary offline landing-coord audit tool (self-labelled [TEMP] "Delete when done") and were supposed to be held on a separate debug branch, not merged. They slipped into upstream with PR #1750 because the amend that removed them locally was never pushed before the PR was merged. Scope: delete-only. No functional changes to MoA handling — handleSeasonalTransport, lockedMoaRegions, and blacklistedMoaDestinations are untouched.
…d guardrail - collectMoaChildren called getDynamicChildren/getNestedChildren/getStaticChildren directly; the static guardrail scanner can't follow through to the callers' runOnClientThreadOptional wrappers, so it flagged a new violation. - Inlined the three child-array fetches into findMoaWidget and computeMoaHotkeyByIndex — they now happen inside the client-thread lambda where the scanner can see the wrapper. - Regenerated client-thread-guardrail-baseline.txt: combined effect of the MoA edits and the audit-plugin removal shifted pre-existing Rs2Walker lambda indices, plus a few entries the old baseline was missing.
Symptom: with Asgarnia MoA region locked, walker runs in a tight loop at
the teleport tile. Each attempt blacklists one destination, re-path picks
a different destination *from the same locked region*, walker tries it,
fails, repeat. Log shows SEASONAL_TRANSPORT usable count steps down per
attempt (122 -> 121 -> 120 -> 119 ...) while src barely moves.
Cause: handleSeasonalTransport already adds the region name to
Rs2Walker.lockedMoaRegions when it detects <str>…</str> markup on the
region row, but PathfinderConfig's MoA filter only checks
blacklistedMoaDestinations (per-destination). Region lock wasn't propagating
to the pathfinder.
Fix: in PathfinderConfig.useTransport's MoA branch, extract the region
segment from the displayInfo ("Map of Alacrity: <Region> - <Name>") and
reject the transport if that region is in lockedMoaRegions.
…meout handleSeasonalTransport was blacklisting a destination whenever the post-region-click destination widget didn't appear in 3s. That trigger is ambiguous: combat, lag, or any other handler closing the widget all manifest as "never appeared", so e.g. a player getting attacked during a teleport attempt permanently poisoned that destination for the whole session. Walker then avoids a legitimate bank forever. Change: on timeout, just return false and let the pathfinder/walker retry. Keep positive-evidence blacklisting (<str> markup on the region or destination row) — those cases actually observe the lock.
… steal Canvas.setFocusable(true) + Canvas.dispatchEvent triggers the window manager to grant focus to the game window, stealing it from whatever app the user was actually typing in. Invoke the canvas's registered KeyListeners directly instead; keep withFocusCanvas as a no-op wrapper so existing call sites still compile.
… keypress After the destination hotkey is pressed, the widget needs a moment to dismiss and kick off the teleport animation. Without this wait, the caller's !Rs2Player.isAnimating() check in the walker transport loop passes instantly, letting the walker race into the next transport step — e.g. clicking Royal seed pod mid-MoA-teleport, which teleports the player back to Grand Tree and starts a MoA ↔ seed-pod loop.
Lambda indices in Rs2Walker shifted by 1 after the MoA widget-close wait added a sleepUntil lambda inside handleSeasonalTransport.
tradablePrimaryId() returned the first tradable ID from ItemCollections, which could be a league cosmetic (e.g., Trailblazer axe id=20011) priced at Integer.MAX_VALUE on the Wiki real-time API. This caused "need 2,147,483,647 gp" errors and blocked quest item acquisition. Changes: - tradablePrimaryId(): fetch prices for all tradable variants and pick the cheapest, avoiding league/cosmetic items - fetchInstabuyReferencePrice(): reject prices >= Integer.MAX_VALUE as invalid, falling back to -1 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…obot/questhelper/QuestScript.java Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
fix(questscript): avoid league/cosmetic items with MAX_VALUE prices
fix(shortestpath): TransportNode cost double-count + overflow; remove MoA audit temp plugin
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (29)
WalkthroughThis PR introduces an F2P webwalker regression testing harness alongside substantial improvements to walker robustness and pathfinding. The core addition is a complete testing infrastructure (F2PWebWalkerHarnessPlugin, 16 pre-configured routes, runner script, and documentation) for validating webwalker behavior on fresh F2P accounts. To support this, Rs2Walker gains cancellation-aware path management, improved minimap click targeting with interpolation and fallback strategies, better door/transport interaction suppression to avoid repeated failures, and path reuse optimization. The agent server gains distance-based walk arrival detection via an optional parameter, and a new quest-helper HTTP API for remote quest control. Supporting changes include version bump to 2.5.7, improvements to quest-item price selection and Romeo & Juliet quest routing, and simplification of keyboard event dispatch. Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.42.1)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.javaThanks 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
webwalker improvements