You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(walker): do not recast a teleport that already landed within its scatter - #2253
In-client bug report MB-UNDGVJ, reported while running MahoganyHomesPlugin 0.0.21 on Microbot 2.6.57.
A walk from Falador to a house in the middle of Varrock did two things the player did not expect:
The walker cast Civitas illa Fortis Teleport, landed a few tiles from the catalog destination, and then cast the same teleport a second time, spending a second set of runes.
The walker went via Civitas illa Fortis and the quetzal instead of casting Varrock Teleport, which the player appeared able to cast.
This PR fixes only the double cast (1). The route choice (2) is not fixed. The walker may still travel via Civitas illa Fortis and the quetzal instead of casting Varrock Teleport. This PR only adds a log line that makes the cause of (2) diagnosable in the next report, so the report must stay open for that half.
Root cause
Double cast: a teleport is accepted as landed when the player is within 10 tiles of its catalog destination, but the shared skip gate that decides whether to dispatch the same transport again only skipped when the player was strictly under 3 tiles away. In the report the player landed exactly 3 tiles from the destination, so the cast counted as landed and the next dispatch still fired the same teleport again. Any teleport that scatters the player 3 to 9 tiles from its destination was affected.
Route choice: not proven. Level, spellbook and runes are ruled out by the other casts in the same session. That leaves the varbit condition on the plain Varrock Teleport row in teleportation_spells.tsv (it requires the Varrock teleport destination not to be set to the Grand Exchange, varbit 4585 = 0) or plain route cost. The report carries no value for that varbit, so the cause cannot be confirmed from it.
Change
Rs2WalkerTransports: the skip gate now treats a teleport as already taken when the player is within the same landing radius used to accept the cast and the destination equals the last handled transport destination. The wider radius only applies to the destination just handled and is cleared with the walk target.
PathfinderConfig.refreshTransports: logs one INFO line, [WebWalk] cfg | spell_teleport_rejected ..., when every spell teleport landing within 64 tiles of the walk target was rejected. The line names each spell, its destination, the distance to the goal, the rejection reason and the live value of every varbit the catalog row checks. It is suppressed when a spell teleport near the target is usable, and is not repeated while its content is unchanged. Route selection itself is unchanged.
New PathfinderConfigSpellRejectionLogTest (5 tests): the near-target check with the report's goal and both Varrock rows, and the exact log line text.
Rs2WalkerTransportsTeleportLandedTest (6 tests): the landed-teleport helper, including the coordinates from the report.
ClientThreadGuardrailTest and QueryableTerminalGuardrailTest: no regressions.
Reviewed by the code reviewer and the player-experience reviewer; both approved with non-blocking nits.
Known gaps
Route choice is unchanged. Players may still be routed via Civitas illa Fortis and the quetzal instead of Varrock Teleport. If the next report shows the Grand Exchange destination setting is the cause, the likely follow-up is to keep the plain Varrock row usable through the spell's right-click option, plus a player-facing message explaining why Varrock Teleport was not used.
The new log line is diagnostic wording, not player wording. It is silent when Varrock Teleport is usable but loses on route cost, so a report with a transport-cache miss and no spell_teleport_rejected line has to be read as "usable, lost on cost".
The line only fires on a transport-cache miss. A repeat walk served from the snapshot cache does not log again.
The duplicate check compares the whole line including the goal distance, so a script alternating between two targets near a teleport the player cannot use (free world, no runes, low Magic, teleports switched off) can log the line again on each cache miss. At most one line per route calculation.
Minigame teleports are accepted within 20 tiles while the skip covers 10; not changed, no evidence of a problem.
The wiring inside refreshTransports and the guard wiring in handleSelectedTransport are not unit tested; both need a live client. The full unit suite was not run locally.
…scatter
The originless-teleport guard only skipped a dispatch when the player stood
less than 3 tiles from the catalog destination, while the wait after a cast
accepts anything within 10 tiles as landed. A cast that scattered the player
3 tiles from the destination was reported as handled and then dispatched
again on the next pass of the same route, spending a second set of runes.
The guard now also skips when the destination is the one the walker just
handled and the player is still inside the landing radius. A teleport that
has not been handled in this walk keeps the previous 3 tile radius.
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
The report that led to the double cast fix also asked why the walker took
Civitas illa Fortis plus the quetzal instead of Varrock Teleport. The
catalog only offers the plain Varrock destination when varbit 4585 is 0,
but the report carries no value for it, so the cause cannot be proven.
When the transport pass rejects every spell teleport that lands within 64
tiles of the walk target, the walker now logs one line naming each spell,
the rejection reason and the current value of every varbit the row checks.
The line is skipped when a spell teleport near the target is usable and is
not repeated while its content is unchanged.
This does not change which route is chosen.
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
No actionable comments were generated in the recent review.
Walkthrough
PathfinderConfig now logs sorted details for qualifying rejected spell teleports near a goal, subject to nearby usability and duplicate-summary checks. Rs2WalkerTransports now checks the player’s plane, distance, and last handled destination before dispatching a teleport. New tests cover spell rejection selection and log formatting, and the teleport landing conditions.
Merge Risk: Minimal · up to 1368b
The change skips a teleport that has already landed near its destination and adds a diagnostic log. The diagnostic may occasionally be omitted after a goal change. This does not affect walking behavior, and no merge-blocking risk remains.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title names the main change: a teleport that already landed within its scatter radius is no longer recast. It matches the walker double-cast fix in Rs2WalkerTransports and the teleport landing che…
Description check
The description states the problem, root cause, change, validation, and known gaps. It matches the changeset: the Rs2WalkerTransports skip gate, the PathfinderConfig diagnostic log, and the new unit t…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Title check
Explanation
The title names the main change: a teleport that already landed within its scatter radius is no longer recast. It matches the walker double-cast fix in Rs2WalkerTransports and the teleport landing check. The title is specific and does not cover the logging addition, which is acceptable since it is secondary.
Full details: Description check
Explanation
The description states the problem, root cause, change, validation, and known gaps. It matches the changeset: the Rs2WalkerTransports skip gate, the PathfinderConfig diagnostic log, and the new unit tests. It also explains that route choice is not fixed.
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.
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
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.
Problem
In-client bug report MB-UNDGVJ, reported while running MahoganyHomesPlugin 0.0.21 on Microbot 2.6.57.
A walk from Falador to a house in the middle of Varrock did two things the player did not expect:
This PR fixes only the double cast (1). The route choice (2) is not fixed. The walker may still travel via Civitas illa Fortis and the quetzal instead of casting Varrock Teleport. This PR only adds a log line that makes the cause of (2) diagnosable in the next report, so the report must stay open for that half.
Root cause
Double cast: a teleport is accepted as landed when the player is within 10 tiles of its catalog destination, but the shared skip gate that decides whether to dispatch the same transport again only skipped when the player was strictly under 3 tiles away. In the report the player landed exactly 3 tiles from the destination, so the cast counted as landed and the next dispatch still fired the same teleport again. Any teleport that scatters the player 3 to 9 tiles from its destination was affected.
Route choice: not proven. Level, spellbook and runes are ruled out by the other casts in the same session. That leaves the varbit condition on the plain Varrock Teleport row in
teleportation_spells.tsv(it requires the Varrock teleport destination not to be set to the Grand Exchange, varbit 4585 = 0) or plain route cost. The report carries no value for that varbit, so the cause cannot be confirmed from it.Change
Rs2WalkerTransports: the skip gate now treats a teleport as already taken when the player is within the same landing radius used to accept the cast and the destination equals the last handled transport destination. The wider radius only applies to the destination just handled and is cleared with the walk target.PathfinderConfig.refreshTransports: logs one INFO line,[WebWalk] cfg | spell_teleport_rejected ..., when every spell teleport landing within 64 tiles of the walk target was rejected. The line names each spell, its destination, the distance to the goal, the rejection reason and the live value of every varbit the catalog row checks. It is suppressed when a spell teleport near the target is usable, and is not repeated while its content is unchanged. Route selection itself is unchanged.Validation
Focused unit run, BUILD SUCCESSFUL, 37 tests, 0 failures, 0 errors, 0 skipped:
PathfinderConfigSpellRejectionLogTest(5 tests): the near-target check with the report's goal and both Varrock rows, and the exact log line text.Rs2WalkerTransportsTeleportLandedTest(6 tests): the landed-teleport helper, including the coordinates from the report.ClientThreadGuardrailTestandQueryableTerminalGuardrailTest: no regressions.Reviewed by the code reviewer and the player-experience reviewer; both approved with non-blocking nits.
Known gaps
spell_teleport_rejectedline has to be read as "usable, lost on cost".refreshTransportsand the guard wiring inhandleSelectedTransportare not unit tested; both need a live client. The full unit suite was not run locally.🤖 Generated with Claude Code