Skip to content

Comments

Don't add teleports to POH when already in house#1508

Merged
chsami merged 1 commit intochsami:developmentfrom
Krulvis:development
Sep 11, 2025
Merged

Don't add teleports to POH when already in house#1508
chsami merged 1 commit intochsami:developmentfrom
Krulvis:development

Conversation

@Krulvis
Copy link
Contributor

@Krulvis Krulvis commented Sep 11, 2025

Currently when the players starts webwalker inside PoH the teleport towards PoH is still added and might be executed after the player teleported outside of the house

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 11, 2025

Walkthrough

  • PathfinderConfig: Adds an early return in createMergedList() to skip merging world-to-PoH transports when PohTeleports.isInHouse() is true, returning mergedTransports after Rs2PohCache merges.
  • PohTeleports: Adds @slf4j. Updates usePortalNexus to fetch a GameObject by NexusPortal.PORTAL_IDS, adds a null check, and logs a warning if not found. Minor formatting in getTransportsToPoh.
  • Rs2Walker: Adds multiple debug/info logs in transport handling, path analysis, door detection, and banking routes; formatting/comment tweaks across several methods. Inserts @component annotations near handleGlider. No public API changes.

Possibly related PRs

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 67.39% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title "Don't add teleports to POH when already in house" succinctly and accurately captures the primary change—preventing PoH teleports from being added when the player is inside their house—as described in the PR summary and code diffs; it is specific, concise, and free of noise.
Description Check ✅ Passed The description explains the bug being fixed (teleports to PoH being added when starting inside PoH and potentially executing later) and directly relates to the changes in the PR, satisfying the lenient description check.

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.38.6)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

✨ Finishing touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/poh/PohTeleports.java (1)

147-161: Avoid potential hang when nexus not found or UI fails to open

If the portal isn’t found, the subsequent sleepUntil(getPortalNexusInterface()!=null) can stall. Early-return false and bound the wait for the UI.

     public static boolean usePortalNexus(NexusPortal nexusPortal) {
         //TODO: Add config here to inform the user if the teleport is a wilderness teleport
-        GameObject portal = Rs2GameObject.getGameObject(NexusPortal.PORTAL_IDS);
-        if (getPortalNexusInterface() == null) {
-            if (portal != null) {
-                Rs2GameObject.interact(portal, "Teleport Menu");
-            } else {
-                log.warn("Portal nexus not found");
-            }
-        }
-
-        sleepUntil(() -> getPortalNexusInterface() != null);
+        GameObject portal = Rs2GameObject.getGameObject(NexusPortal.PORTAL_IDS);
+        if (getPortalNexusInterface() == null) {
+            if (portal == null) {
+                log.warn("Portal nexus not found");
+                return false;
+            }
+            Rs2GameObject.interact(portal, "Teleport Menu");
+            if (!sleepUntil(() -> getPortalNexusInterface() != null, 5000)) {
+                log.warn("Portal nexus interface did not open");
+                return false;
+            }
+        }
 
         return interactWithPortalNexusWidget(nexusPortal);
     }
🧹 Nitpick comments (4)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java (4)

319-337: Replace System.out with logger

Use log.debug/log.info for consistency and to respect runtime log levels.

-                System.out.printf("start loop %s, from=%s, to=%s\n", i, currentWorldPoint, nextWorldPoint);
+                log.debug("start loop {}, from={}, to={}", i, currentWorldPoint, nextWorldPoint);
...
-                    System.out.println("marker is null");
+                    log.debug("marker is null");
...
-                    System.out.println("No longer near path");
+                    log.debug("No longer near path");
...
-                        System.out.println("cancel instead of recalculate");
+                        log.debug("cancel instead of recalculate");
...
-                    System.out.println("break out of door");
+                    log.debug("break out of door");
...
-                    System.out.println("break out of rockfall");
+                    log.debug("break out of rockfall");
...
-                    System.out.println("break out of transport");
+                    log.debug("break out of transport");

Also applies to: 340-344, 346-349, 357-360


611-615: Add null-safety to path usage

Guard against pathfinder.getPath() returning null to avoid NPE.

-        List<WorldPoint> path = pathfinder.getPath();
-        if (path.isEmpty() || path.get(path.size() - 1).getPlane() != destination.getPlane()) return Integer.MAX_VALUE;
+        List<WorldPoint> path = pathfinder.getPath();
+        if (path == null || path.isEmpty() || path.get(path.size() - 1).getPlane() != destination.getPlane()) return Integer.MAX_VALUE;

647-648: Null-safety in helper

Mirror the null check here for callers that may pass a null path.

-        if (path.isEmpty() || path.get(path.size() - 1).getPlane() != destination.getPlane()) return Integer.MAX_VALUE;
+        if (path == null || path.isEmpty() || path.get(path.size() - 1).getPlane() != destination.getPlane()) return Integer.MAX_VALUE;

2303-2311: Make annotated component IDs final

Local @component IDs should be final for clarity and consistency with other usages in this class.

-        @Component
-        int VARLAMORE_QUETZAL_MAP = InterfaceID.QuetzalMenu.CONTENTS;
-        @Component
-        int VARLAMORE_QUETZAL_OPTIONS = InterfaceID.QuetzalMenu.ICONS;
+        @Component
+        final int VARLAMORE_QUETZAL_MAP = InterfaceID.QuetzalMenu.CONTENTS;
+        @Component
+        final int VARLAMORE_QUETZAL_OPTIONS = InterfaceID.QuetzalMenu.ICONS;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 70035e7 and 122dc71.

📒 Files selected for processing (3)
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/PathfinderConfig.java (1 hunks)
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/poh/PohTeleports.java (4 hunks)
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java (55 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: Krulvis
PR: chsami/Microbot#1506
File: runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java:1412-1423
Timestamp: 2025-09-11T09:51:12.393Z
Learning: In the microbot pathfinding system, global (null-keyed) transports like teleportation spells are automatically added to the player's current worldpoint by PathfinderConfig.refreshTeleports method during pathfinder refresh, which filters usableTeleports by wilderness level and adds them to the transport map at the packedLocation, eliminating the need to manually union null-keyed transports when iterating through available transports.
Learnt from: Krulvis
PR: chsami/Microbot#1506
File: runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java:1412-1423
Timestamp: 2025-09-11T09:51:12.393Z
Learning: In the microbot pathfinding system, global (null-keyed) transports like teleportation spells are automatically added to the player's start worldpoint by PathfinderConfig.refreshTeleports method during pathfinder refresh, eliminating the need to manually union null-keyed transports when iterating through available transports.
Learnt from: Krulvis
PR: chsami/Microbot#1506
File: runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java:1412-1423
Timestamp: 2025-09-11T09:51:12.393Z
Learning: In PathfinderConfig.refreshTeleports, global teleports (originally stored with null origins in usableTeleports) are filtered by wilderness level and then added to the transport map at the player's current WorldPoint (unpacked from packedLocation). This means handleTransports automatically finds these teleports when processing the player's start position without needing special null-key handling. The method also carefully preserves existing transports at the same location to avoid overwriting.
📚 Learning: 2025-09-11T09:51:12.393Z
Learnt from: Krulvis
PR: chsami/Microbot#1506
File: runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java:1412-1423
Timestamp: 2025-09-11T09:51:12.393Z
Learning: In the microbot pathfinding system, global (null-keyed) transports like teleportation spells are automatically added to the player's current worldpoint by PathfinderConfig.refreshTeleports method during pathfinder refresh, which filters usableTeleports by wilderness level and adds them to the transport map at the packedLocation, eliminating the need to manually union null-keyed transports when iterating through available transports.

Applied to files:

  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/PathfinderConfig.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/poh/PohTeleports.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java
📚 Learning: 2025-09-11T09:51:12.393Z
Learnt from: Krulvis
PR: chsami/Microbot#1506
File: runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java:1412-1423
Timestamp: 2025-09-11T09:51:12.393Z
Learning: In the microbot pathfinding system, global (null-keyed) transports like teleportation spells are automatically added to the player's start worldpoint by PathfinderConfig.refreshTeleports method during pathfinder refresh, eliminating the need to manually union null-keyed transports when iterating through available transports.

Applied to files:

  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/PathfinderConfig.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/poh/PohTeleports.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java
📚 Learning: 2025-09-11T09:51:12.393Z
Learnt from: Krulvis
PR: chsami/Microbot#1506
File: runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java:1412-1423
Timestamp: 2025-09-11T09:51:12.393Z
Learning: In PathfinderConfig.refreshTeleports, global teleports (originally stored with null origins in usableTeleports) are filtered by wilderness level and then added to the transport map at the player's current WorldPoint (unpacked from packedLocation). This means handleTransports automatically finds these teleports when processing the player's start position without needing special null-key handling. The method also carefully preserves existing transports at the same location to avoid overwriting.

Applied to files:

  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/PathfinderConfig.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/poh/PohTeleports.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java
🧬 Code graph analysis (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/poh/PohTeleports.java (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/gameobject/Rs2GameObject.java (1)
  • Rs2GameObject (36-2010)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java (5)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/player/Rs2Player.java (1)
  • Rs2Player (67-2076)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/gameobject/Rs2GameObject.java (1)
  • Rs2GameObject (36-2010)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2Inventory.java (1)
  • Rs2Inventory (46-2324)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/equipment/Rs2Equipment.java (1)
  • Rs2Equipment (27-485)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/magic/Rs2Magic.java (1)
  • Rs2Magic (44-608)
🔇 Additional comments (4)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java (2)

351-356: Verify transport handling in instances (non-PoH)

Condition PohTeleports.isInHouse() || !inInstance skips transports in any non-PoH instance. Please confirm we don’t need transports in other instanced areas. This aligns with prior learning that global (null-keyed) teleports are injected at the player’s start via PathfinderConfig.refreshTeleports.


1497-1503: POH transport hook looks good

The dedicated POH branch delegating to PohTransport.execute() is a clean addition.

runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/PathfinderConfig.java (1)

311-315: Early return correctly prevents world→PoH merge when already in house

This matches the PR goal and avoids enqueuing PoH teleports from inside the PoH.

runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/poh/PohTeleports.java (1)

3-3: Logging enablement is fine

Adding @slf4j is appropriate for new warnings and diagnostics.

Also applies to: 38-39

@chsami chsami merged commit 2b5a01b into chsami:development Sep 11, 2025
2 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Sep 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants