Skip to content

Comments

feat(agility): Add Agility Pyramid course support#1417

Merged
gmason0 merged 40 commits intochsami:developmentfrom
runsonmypc:agility-pyramid
Aug 29, 2025
Merged

feat(agility): Add Agility Pyramid course support#1417
gmason0 merged 40 commits intochsami:developmentfrom
runsonmypc:agility-pyramid

Conversation

@runsonmypc
Copy link
Contributor

@runsonmypc runsonmypc commented Aug 22, 2025

Description

This PR adds full support for the Agility Pyramid course to the Microbot agility plugin, implementing comprehensive obstacle navigation, state management, and pyramid top turn-in functionality.

Features Added

  • Complete Agility Pyramid course navigation
  • Pyramid top turn-in automation:
    • Hands in pyramids when inventory is full
  • Quality of life improvements:
    • Automatic empty waterskin dropping to free inventory space
    • Direct pyramid stairs interaction after doorway exit
    • Robust plane change detection and handling
    • Recovery from missed obstacles or disconnections

Technical Implementation

  • Created modular course handler following existing plugin architecture
  • Separated obstacle data into PyramidObstacleData for maintainability
  • Implemented PyramidState class for centralized state tracking
  • Added comprehensive obstacle validation and retry logic
  • Integrated with existing AgilityCourse enum and plugin framework

Files Added/Modified

  • PyramidCourse.java - Main course handler implementation
  • PyramidObstacleData.java - Obstacle definitions and areas
  • PyramidState.java - State management system
  • AgilityCourse.java - Added AGILITY_PYRAMID enum entry
  • AgilityScript.java - Minor integration updates

Testing

  • Tested full course completion cycles
  • Verified pyramid top turn-in functionality
  • Confirmed recovery from disconnections
  • Validated obstacle sequence enforcement

Notes

  • Requires waterskins in inventory (will drop empty ones automatically)
  • The plugin will NOT resupply waterskins for you

Summary by CodeRabbit

  • New Features
    • Added Agility Pyramid course option to the Agility plugin.
    • Automatic navigation to the course start and seamless obstacle handling across all floors and planes.
    • Data-driven obstacle area detection for more reliable obstacle identification.
    • Auto turn-in of pyramid tops to Simon Templeton when threshold met.
    • Smart waterskin management (auto-drop empty waterskins).
    • Course-aware progress tracking (XP/position monitoring) for robust completion detection.
    • Supports boosted access from level 30.

Alejandro Legarda and others added 23 commits August 5, 2025 23:40
- Add complete PyramidCourse implementation with position-based navigation
- Define precise tile areas for all obstacles across 5 floors
- Implement special handling for multi-tile obstacles and floor transitions
- Add cooldowns to prevent spam clicking (especially for Cross Gap obstacles)
- Handle stone block interruptions (12 XP drops) with retry logic
- Implement pyramid collection at top with 30-second cooldown
- Add strict obstacle selection to prevent wrong gap/ledge selection
- Ensure bot waits for animations to complete before clicking next obstacle
- Support recovery from any position on the course

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Increase Cross Gap cooldown from 2.5 to 3.5 seconds
- Add longer wait times specifically for Cross Gap obstacles (1.2s)
- Ensure minimum 3 second total wait for Cross Gap completion
- Fix duplicate variable declaration compilation error

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Added inventory checking to PyramidCourse to detect full inventory with pyramid tops
- Implemented Simon Templeton interaction for automatic pyramid top turn-in
- Walker navigates to Simon at (3343, 2827, 0) and handles climbing rocks automatically
- Uses item-on-NPC interaction to turn in pyramid tops
- Shows warning when inventory is full without pyramid tops
- Returns to pyramid start after successful turn-in
- Increased Cross Gap cooldown to 4.5s for better XP detection
- Added double-check for movement/animation in AgilityScript
- Fix stuck XP obstacle flag when stone block interrupts (clear flags on retry)
- Remove flawed distance check for stone block detection (only use 12 XP)
- Extend XP-granting obstacle protection to all obstacles (planks, gaps, ledges)
- Fix floor 5 gap area to include position after stone block interruption
- Clear flags properly in climbing rocks special case
- Reduce walk-to-start distance threshold from 10 to 3 tiles for post-turn-in
… course

- Fixed Cross Gap obstacles completing prematurely due to movement > 3 tiles
- Added proper flag clearing when plane changes between floors
- Expanded floor 3 plank areas to catch gap landing positions
- Cross Gap now waits full 6+ seconds regardless of movement distance

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Instead of using Rs2Walker to path to the pyramid start after exiting
via doorway, directly click on the stairs obstacle when it's visible.
This makes the bot more efficient when restarting the pyramid course.

Falls back to Rs2Walker if stairs aren't found or interaction fails.
- Added handleEmptyWaterskins() method to check and drop Waterskin(0) items
- Integrated check into getCurrentObstacle() to run periodically during course
- Prevents inventory clutter from empty waterskins while training
Major refactoring to improve maintainability and reduce file size:

- Added debug mode for conditional logging (set DEBUG=true to enable verbose logs)
- Extracted obstacle area definitions to PyramidObstacleData class (saved 150 lines)
- Created PyramidState class for centralized state management
- Reduced PyramidCourse.java from 1274 to 1096 lines (14% reduction)
- Improved code organization with better separation of concerns
- Partially integrated state management (can be completed in future iterations)

The functionality remains unchanged while the code is now more maintainable.
… data extraction

- Extract obstacle area definitions to PyramidObstacleData class (148 lines)
- Create PyramidState class for centralized state management
- Add debug mode with DEBUG flag for conditional logging
- Fix obstacle area ordering to match original exactly
- Reduce PyramidCourse.java from 1274 to 1096 lines (14% reduction)
- Maintain all functionality including empty waterskin dropping
- All 53 obstacle areas verified to have exact same coordinates as original
- Add complete PyramidCourse implementation with position-based navigation
- Define precise tile areas for all obstacles across 5 floors
- Implement special handling for multi-tile obstacles and floor transitions
- Add cooldowns to prevent spam clicking (especially for Cross Gap obstacles)
- Handle stone block interruptions (12 XP drops) with retry logic
- Implement pyramid collection at top with 30-second cooldown
- Add strict obstacle selection to prevent wrong gap/ledge selection
- Ensure bot waits for animations to complete before clicking next obstacle
- Support recovery from any position on the course

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Increase Cross Gap cooldown from 2.5 to 3.5 seconds
- Add longer wait times specifically for Cross Gap obstacles (1.2s)
- Ensure minimum 3 second total wait for Cross Gap completion
- Fix duplicate variable declaration compilation error

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Added inventory checking to PyramidCourse to detect full inventory with pyramid tops
- Implemented Simon Templeton interaction for automatic pyramid top turn-in
- Walker navigates to Simon at (3343, 2827, 0) and handles climbing rocks automatically
- Uses item-on-NPC interaction to turn in pyramid tops
- Shows warning when inventory is full without pyramid tops
- Returns to pyramid start after successful turn-in
- Increased Cross Gap cooldown to 4.5s for better XP detection
- Added double-check for movement/animation in AgilityScript
- Fix stuck XP obstacle flag when stone block interrupts (clear flags on retry)
- Remove flawed distance check for stone block detection (only use 12 XP)
- Extend XP-granting obstacle protection to all obstacles (planks, gaps, ledges)
- Fix floor 5 gap area to include position after stone block interruption
- Clear flags properly in climbing rocks special case
- Reduce walk-to-start distance threshold from 10 to 3 tiles for post-turn-in
… course

- Fixed Cross Gap obstacles completing prematurely due to movement > 3 tiles
- Added proper flag clearing when plane changes between floors
- Expanded floor 3 plank areas to catch gap landing positions
- Cross Gap now waits full 6+ seconds regardless of movement distance

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Instead of using Rs2Walker to path to the pyramid start after exiting
via doorway, directly click on the stairs obstacle when it's visible.
This makes the bot more efficient when restarting the pyramid course.

Falls back to Rs2Walker if stairs aren't found or interaction fails.
- Added handleEmptyWaterskins() method to check and drop Waterskin(0) items
- Integrated check into getCurrentObstacle() to run periodically during course
- Prevents inventory clutter from empty waterskins while training
Major refactoring to improve maintainability and reduce file size:

- Added debug mode for conditional logging (set DEBUG=true to enable verbose logs)
- Extracted obstacle area definitions to PyramidObstacleData class (saved 150 lines)
- Created PyramidState class for centralized state management
- Reduced PyramidCourse.java from 1274 to 1096 lines (14% reduction)
- Improved code organization with better separation of concerns
- Partially integrated state management (can be completed in future iterations)

The functionality remains unchanged while the code is now more maintainable.
… data extraction

- Extract obstacle area definitions to PyramidObstacleData class (148 lines)
- Create PyramidState class for centralized state management
- Add debug mode with DEBUG flag for conditional logging
- Fix obstacle area ordering to match original exactly
- Reduce PyramidCourse.java from 1274 to 1096 lines (14% reduction)
- Maintain all functionality including empty waterskin dropping
- All 53 obstacle areas verified to have exact same coordinates as original
  - Add reachability check before attempting direct stair clicks
  - Use Rs2Walker when stairs are blocked by climbing rocks (e.g., from Simon)
  - Keep direct clicking for efficiency when coming from pyramid exit
  - Fixes navigation issues when returning from Simon Templeton
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 22, 2025

Walkthrough

Adds the Agility Pyramid course: new PyramidCourse implementation, PyramidObstacleData (data-driven obstacle regions), and PyramidState (centralized state). Updates AgilityCourse enum to register AGILITY_PYRAMID and reorders two existing entries. Exposes course APIs for navigation, obstacle resolution, turn-in handling, and completion detection.

Changes

Cohort / File(s) Summary of edits
Agility Pyramid feature
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidCourse.java, .../courses/PyramidObstacleData.java, .../courses/PyramidState.java
Adds PyramidCourse (implements AgilityCourseHandler) with obstacle detection, plane-aware resolution, walk-to-start, turn-in handling, waitForCompletion, and public getters (start point, obstacles, current obstacle, required level, boostability, loot distance). Adds PyramidObstacleData with OBSTACLE_AREAS and ObstacleArea type. Adds PyramidState for cooldowns/flags and turn-in threshold management.
Agility course registry updates
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/enums/AgilityCourse.java
Registers AGILITY_PYRAMID("Agility Pyramid", new PyramidCourse()); reorders CANIFIS_ROOFTOP_COURSE earlier and repositions SHAYZIEN_BASIC_COURSE.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Player
  participant Course as PyramidCourse
  participant State as PyramidState
  participant Data as PyramidObstacleData
  participant World as GameObjects/NPCs
  participant Walker as Rs2Walker

  Player->>Course: select AGILITY_PYRAMID
  Course->>State: reset()
  loop course loop
    Course->>Course: getCurrentObstacle()
    Course->>State: check cooldowns/flags
    alt Need pyramid turn-in (ground & tops >= threshold)
      Course->>State: startPyramidTurnIn()
      Course->>World: interact Simon Templeton (NPC)
      World-->>Course: dialog/interaction
      Course->>State: clearPyramidTurnIn()
    else On-course obstacle flow
      Course->>Data: match OBSTACLE_AREAS (by plane)
      Data-->>Course: area info
      Course->>World: find nearest/strict obstacle (TileObject/GroundObject)
      Course->>State: recordObstacleStart()
      Course-->>World: click/interact obstacle
      Course->>Course: waitForCompletion(agilityExp, plane)
      alt Progress detected (XP/move/plane)
        Course->>State: clear relevant flags
      else Timeout/fallback
        Course->>World: try alternate obstacle or navigate
      end
    end
    alt Not at start
      Course->>World: click stairs if reachable
      else
        Course->>Walker: walk to START_POINT
      end
  end
Loading
sequenceDiagram
  autonumber
  participant Course as PyramidCourse
  participant State as PyramidState

  Note over Course,State: Special-phase handling
  Course->>State: startCrossGap() / startXpObstacle()
  Note right of State: flags + cooldown timers
  State-->>Course: flag active
  Course->>State: clearCrossGap() / clearXpObstacle() on completion
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 4a67258 and aa5cd29.

📒 Files selected for processing (1)
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidCourse.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidCourse.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build
✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Pert added 5 commits August 22, 2025 19:31
…rve run energy

  Modified Agility Pyramid course to hand in pyramid tops when carrying at least 4 (randomized) OR when inventory is full, whichever comes first. This helps conserve run energy since pyramid tops are heavy items that drain energy faster.
… retry after stone block

- Remove 6-second Cross Gap cooldown check that prevented retries
- Flag-based protection is sufficient to prevent clicking during animation
- Fixes 4-second unnecessary wait after stone block interruptions
…State

- Remove lastCrossGapTime variable and CROSS_GAP_COOLDOWN constant
- Remove isCrossGapCooldownActive() method no longer needed
- Simplify startCrossGap() to only set flag without timestamp
- Clean up reset() method
- Convert all infoLog() calls to debugLog() for cleaner output
- Remove unused infoLog() method and comment
- Debug logs only print when DEBUG flag is enabled
…st practices

- Add @slf4j annotation for proper SLF4J logging
- Remove custom debugLog method and DEBUG flag
- Replace all debugLog calls with log.debug
- Replace Microbot.log calls with structured logging (error/warn/info)
- Use parameterized logging for better performance
- Remove printStackTrace in favor of proper exception logging
}

// Small delay after stopping to ensure animations are fully complete
sleep(300, 400);
Copy link
Owner

Choose a reason for hiding this comment

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

Not sure if this is specific for pyramid? I'm not a fan of changing the logic for all courses

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a mistake, removed!

…lity script

- Remove double-check and delay that was added for pyramid gaps
- These changes affected all agility courses unnecessarily
- Pyramid-specific timing should be handled in PyramidCourse only
- Addresses chsami's concern about changing logic for all courses
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: 2

🧹 Nitpick comments (6)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidState.java (2)

12-27: Consider memory visibility for timing fields (use volatile)
lastObstacleStartTime and lastClimbingRocksTime may be read/written across threads (scheduled executor vs. other helpers). Make them volatile to guarantee visibility, similar to the AtomicBoolean flags.

-    private long lastObstacleStartTime = 0;
-    private long lastClimbingRocksTime = 0;
+    private volatile long lastObstacleStartTime = 0;
+    private volatile long lastClimbingRocksTime = 0;

25-27: Prefer monotonic clock for elapsed time comparisons
For cooldown checks, System.nanoTime() avoids wall‑clock adjustments. Not critical here, but it’s a safer pattern for elapsed timing.

-    private static final long OBSTACLE_COOLDOWN = 1500; // 1.5 seconds between obstacles
-    private static final long CLIMBING_ROCKS_COOLDOWN = 30000; // 30 seconds - pyramid respawn time
+    private static final long OBSTACLE_COOLDOWN_MS = 1500; // 1.5 seconds between obstacles
+    private static final long CLIMBING_ROCKS_COOLDOWN_MS = 30000; // 30 seconds - pyramid respawn time
+    private volatile long lastObstacleStartNano = 0L;
+    private volatile long lastClimbingRocksNano = 0L;

And update the reads/writes:

-        lastObstacleStartTime = System.currentTimeMillis();
+        lastObstacleStartTime = System.currentTimeMillis();
+        lastObstacleStartNano = System.nanoTime();
-        return System.currentTimeMillis() - lastObstacleStartTime < OBSTACLE_COOLDOWN;
+        return System.nanoTime() - lastObstacleStartNano < java.util.concurrent.TimeUnit.MILLISECONDS.toNanos(OBSTACLE_COOLDOWN_MS);
-        lastClimbingRocksTime = System.currentTimeMillis();
+        lastClimbingRocksTime = System.currentTimeMillis();
+        lastClimbingRocksNano = System.nanoTime();
-        return System.currentTimeMillis() - lastClimbingRocksTime < CLIMBING_ROCKS_COOLDOWN;
+        return System.nanoTime() - lastClimbingRocksNano < java.util.concurrent.TimeUnit.MILLISECONDS.toNanos(CLIMBING_ROCKS_COOLDOWN_MS);
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidCourse.java (4)

36-38: Avoid static global state for course runtime flags
PyramidState as static leaks across script instances/world hops and persists between plugin lifecycles unless explicitly reset. Prefer instance state.

-    private static final PyramidState state = new PyramidState();
+    private final PyramidState state = new PyramidState();

If you rely on static for cross-instance behavior, clarify and ensure reset() is called at plugin start/stop.


140-151: Replace blocking Thread.sleep with Global.sleep to keep behavior consistent
Direct Thread.sleep in the scheduler can unnecessarily block. Use existing sleep utility to align with project conventions.

-        try {
-            Thread.sleep(50); // Very brief check
-        } catch (InterruptedException e) {
-            // Ignore
-        }
+        Global.sleep(35, 65); // brief jittered delay

162-192: Full-plane area scan with verbose logging may be noisy at 10 Hz
At 100 ms ticks, per-area debug lines will flood logs and add overhead. Consider guarding with if (log.isDebugEnabled()) or reducing frequency.

-        log.debug("Checking areas for plane " + playerPos.getPlane() + " player position " + playerPos + ":");
+        if (log.isDebugEnabled()) {
+            log.debug("Checking areas for plane {} player position {}", playerPos.getPlane(), playerPos);
+        }
...
-                log.debug("  - Area: " + area.name + " at (" + area.minX + "," + area.minY + ") to (" + area.maxX + "," + area.maxY + ") - contains player: " + contains);
+                if (log.isDebugEnabled()) {
+                    log.debug("  - Area: {} at ({},{}) to ({},{}) - contains player: {}", area.name, area.minX, area.minY, area.maxX, area.maxY, contains);
+                }

1013-1083: Dialogue handling with Simon: add a broader fallback for option text
The exact string can vary due to punctuation/capitalization. You already use partial matches under the hood for clickOption, but guarding with a more generic contains (e.g., “pyramid tops”) would reduce brittleness if the line changes slightly.

-                    if (Rs2Dialogue.hasDialogueOption("I've got some pyramid tops for you.")) {
-                        Rs2Dialogue.clickOption("I've got some pyramid tops for you.");
+                    if (Rs2Dialogue.hasDialogueOption("pyramid tops")) {
+                        Rs2Dialogue.clickOption("pyramid tops");
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d7eaf86 and 5c9288d.

📒 Files selected for processing (5)
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/AgilityScript.java (1 hunks)
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidCourse.java (1 hunks)
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidObstacleData.java (1 hunks)
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidState.java (1 hunks)
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/enums/AgilityCourse.java (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidCourse.java (7)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/models/AgilityObstacleModel.java (1)
  • AgilityObstacleModel (6-33)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/Global.java (1)
  • Global (10-171)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/dialogues/Rs2Dialogue.java (1)
  • Rs2Dialogue (18-725)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/gameobject/Rs2GameObject.java (1)
  • Rs2GameObject (36-2002)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2Inventory.java (1)
  • Rs2Inventory (60-2244)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/player/Rs2Player.java (1)
  • Rs2Player (67-2062)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidObstacleData.java (2)
  • PyramidObstacleData (11-119)
  • ObstacleArea (16-38)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/AgilityScript.java (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/player/Rs2Player.java (1)
  • Rs2Player (67-2062)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build
🔇 Additional comments (12)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidState.java (1)

139-146: State reset path looks solid
Reset clears timers, flags, and regenerates the threshold. Good defensive baseline for restarts.

runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/AgilityScript.java (1)

115-155: Course-specific branches remain contained; no global logic changes needed for Pyramid
The integration leverages existing handleWalkToStart hook for non-special courses, so Pyramid doesn’t add extra global complexity. This looks appropriate.

runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/enums/AgilityCourse.java (2)

16-28: Enum integration for Agility Pyramid looks correct
Importing PyramidCourse and adding AGILITY_PYRAMID("Agility Pyramid", new PyramidCourse()) registers the course cleanly.


27-44: No ordinal-based persistence found — reordering AgilityCourse is safe

Short summary: I scanned the repo for ordinal/values()-based persistence of AgilityCourse and inspected the plugin config that references it. I found no usages that persist AgilityCourse by ordinal; RuneLite's ConfigManager deserializes enums by name (Enum.valueOf), so reordering the enum constants will not remap stored config values. (github.com, pastebin.com)

Files / locations inspected (no changes required):

  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/enums/AgilityCourse.java (enum — snippet you posted).
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/MicroAgilityConfig.java (plugin config returning AgilityCourse; default = CANIFIS_ROOFTOP_COURSE).
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/questhelper/helpers/skills/agility/* (Agility.java, Varrock.java, DraynorVillage.java, Ardougne.java, SeersVillage.java, Pollnivneach.java, Falador.java, Canifis.java, AlKharid.java, Rellekka.java) — these reference the enum but do not persist its ordinal.

What I searched for / verified:

  • Repo scan for patterns using AgilityCourse with .ordinal(), values()[…], or array-indexing — no matches referencing AgilityCourse found.
  • Confirmed config usage is via a Config interface (MicroAgilityConfig) — persisted via ConfigManager. ConfigManager converts stored strings to enums with Enum.valueOf (i.e., by name). (github.com, pastebin.com)

Recommendation: No change required to block this PR. As a general best practice, avoid persisting enums by ordinal anywhere (use names or explicit stable ids) — but in this case the codebase already persists config enums by name.

runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidObstacleData.java (2)

33-38: Area bounds check is clear and inclusive
The rectangle containment and plane check are straightforward and correct.


94-111: Floor 4 plane mapping and top-floor areas: request validation against in-game coordinates
Floor 4 uses plane=2 and top uses plane=3. That matches common mapping for the pyramid instanced region, but these coordinates are finicky. If you have a small capture confirming these bounds via developer console/overlay, please attach it for posterity.

runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidCourse.java (6)

85-115: Turn-in gating logic is solid; threshold usage is coherent
Checks for full inventory or threshold and limits turn‑in to ground level. Properly clears the flag when leaving turn‑in flow without forcibly regenerating threshold. Nicely done.


244-257: Strict obstacle selection is appropriate for preventing skips
The strict finder for gaps/ledges prevents skipping and handles floor-specific cases. Good balance between robustness and flexibility.


290-299: Fallback to stairs on top region is a nice recovery path
Good UX-oriented fallback if object resolution fails on the top.


740-756: Verify existence of Rs2GameObject.findObjectByIdAndDistance or replace with nearest search
I don’t see findObjectByIdAndDistance in the shared utilities. If it doesn’t exist, this will not compile. Replace with a safe nearest search.

Proposed replacement:

-                TileObject pyramidStairs = Rs2GameObject.findObjectByIdAndDistance(10857, 10);
+                TileObject pyramidStairs = Rs2GameObject.getAll(obj ->
+                        obj.getId() == 10857 &&
+                        obj.getWorldLocation().distanceTo(playerLocation) <= 10 &&
+                        obj.getPlane() == playerLocation.getPlane())
+                    .stream()
+                    .min((a,b) -> Integer.compare(
+                        a.getWorldLocation().distanceTo(playerLocation),
+                        b.getWorldLocation().distanceTo(playerLocation)))
+                    .orElse(null);

762-996: Completion loop handles XP, plane changes, failures, and long animations well

  • Stone block 12 XP detection is a smart retry signal.
  • Cross-gap requires XP or timeout; movement alone doesn’t complete it—good.
  • Proper clearing of flags on success/failure/timeout prevents stuck states.

1089-1097: QoL waterskin drop looks good
Drops only WATERSKIN0 and yields control for a short period. Nice touch to keep inventory free.

Pert added 5 commits August 25, 2025 11:23
- Make lastObstacleStartTime and lastClimbingRocksTime volatile
- Make pyramidTurnInThreshold volatile
- Ensures memory visibility across scheduled executor threads
- Addresses CodeRabbit review comments about thread safety
- Switch from currentTimeMillis to nanoTime for cooldown tracking
- Avoids issues with wall-clock adjustments (DST, NTP sync)
- Convert cooldown constants to nanoseconds for comparison
- Provides monotonic timing immune to system clock changes
- Addresses CodeRabbit review about timing precision
- Remove static from PyramidState to prevent state leaks across instances
- Each course instance now has its own state properly cleaned on restart
- Replace Thread.sleep with Global.sleep for consistency
- Use jittered delay (35-65ms) for better anti-pattern behavior
- Addresses CodeRabbit review about state isolation
…ogging

- Change scheduler interval from 100ms to 600ms (10Hz to 1.67Hz)
- Wrap verbose debug statements with log.isDebugEnabled() checks
- Convert string concatenation to parameterized logging
- Significantly reduces debug log spam and improves performance
- Addresses CodeRabbit review about logging overhead
@runsonmypc
Copy link
Contributor Author

Addressed coderabbit comments. Dialogue fallbacks are unnecessary as using the pyramid on Simon converts to cash immediately.

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: 2

🧹 Nitpick comments (7)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidState.java (2)

23-27: Minor: deterministic testing hook for pyramidTurnInThreshold

If you plan to unit-test turn-in logic, consider a package-private setter or constructor injection for the threshold (guarded for tests) to avoid flakiness from Rs2Random.

-    private volatile int pyramidTurnInThreshold = generateNewThreshold();
+    private volatile int pyramidTurnInThreshold = generateNewThreshold();
+    // VisibleForTesting
+    void setPyramidTurnInThresholdForTesting(int value) {
+        this.pyramidTurnInThreshold = value;
+    }

111-115: Clarify comment around threshold regeneration

The comment says the threshold is generated when grabbing the pyramid top (via climbing rocks), which is implemented in recordClimbingRocks(). Consider making that explicit to avoid confusion for future readers.

-        // Threshold is now generated when grabbing pyramid top, not after turn-in
+        // Threshold is regenerated when grabbing the pyramid top (recordClimbingRocks), not after turn-in
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidCourse.java (5)

39-48: Remove or wire up FLOOR_2_SEQUENCE (currently unused)

The static sequence is never referenced; it can confuse future maintainers. Either use it to enforce ordering or drop it.

-    // Define the strict obstacle sequence to prevent skipping ahead
-    private static final List<Integer> FLOOR_2_SEQUENCE = Arrays.asList(
-        10884, // Gap Cross 1
-        10859, // Gap Jump  
-        10861, // Gap Cross 2
-        10860, // Ledge
-        10865, // Low wall
-        10859, // Gap jump (end)
-        10857  // Stairs up
-    );
+    // NOTE: If strict floor-2 sequencing is needed, consider deriving it from PyramidObstacleData metadata.

826-835: Avoid deprecated findObjectByIdAndDistance; select stairs via getAll + reachability

This avoids deprecated API and keeps behavior explicit and testable.

-                // Try to directly click on the pyramid stairs if visible AND reachable
-                TileObject pyramidStairs = Rs2GameObject.findObjectByIdAndDistance(10857, 10);
-                if (pyramidStairs != null && 
-                    pyramidStairs.getWorldLocation().distanceTo(START_POINT) <= 2 &&
-                    Rs2GameObject.canReach(pyramidStairs.getWorldLocation())) {
-                    // We're close and can reach it (e.g., coming from pyramid exit) - click directly
-                    log.debug("Clicking directly on pyramid stairs (reachable from current position)");
-                    if (Rs2GameObject.interact(pyramidStairs)) {
-                        Global.sleep(600, 800); // Small delay after clicking
-                        return true;
-                    }
-                }
+                // Try to directly click on the pyramid stairs if visible AND reachable
+                List<TileObject> stairsCandidates = Rs2GameObject.getAll(obj ->
+                    obj.getId() == 10857 &&
+                    obj.getPlane() == playerLocation.getPlane() &&
+                    obj.getWorldLocation().distanceTo(playerLocation) <= 10 &&
+                    obj.getWorldLocation().distanceTo(START_POINT) <= 2 &&
+                    Rs2GameObject.canReach(obj.getWorldLocation())
+                );
+                if (!stairsCandidates.isEmpty()) {
+                    TileObject pyramidStairs = stairsCandidates.stream()
+                        .min((a,b) -> Integer.compare(
+                            a.getWorldLocation().distanceTo(playerLocation),
+                            b.getWorldLocation().distanceTo(playerLocation)))
+                        .orElse(null);
+                    if (pyramidStairs != null) {
+                        log.debug("Clicking directly on pyramid stairs (reachable from current position)");
+                        if (Rs2GameObject.interact(pyramidStairs)) {
+                            Global.sleep(600, 800); // Small delay after clicking
+                            return true;
+                        }
+                    }
+                }

1196-1201: Reduce log noise: downgrade waterskin drop to debug

Dropping empties every tick at info can spam logs.

-        if (Rs2Inventory.contains(ItemID.WATERSKIN0)) {
-            log.info("Found empty waterskin(s), dropping them");
+        if (Rs2Inventory.contains(ItemID.WATERSKIN0)) {
+            log.debug("Found empty waterskin(s), dropping them");

182-187: Avoid string-matching on area names; use typed metadata from PyramidObstacleData

Multiple branches rely on name.contains(...) to drive logic (climbing rocks “grab pyramid”, gap vs ledge variants, floor-2 sub-steps). This is brittle. Prefer metadata in PyramidObstacleData (e.g., type enum, flags: xpGranting, longAnimation, isClimbingRocks, subStep) and branch on that instead.

Example (supporting code outside this file):

  • In PyramidObstacleData.ObstacleArea add:
public enum Kind { STAIRS, LOW_WALL, LEDGE, GAP, PLANK_END, CLIMBING_ROCKS, DOORWAY }
public final Kind kind;
public final boolean xpGranting;
public final boolean longAnimation;
public final int subStep; // for strict ordering within a floor if needed
  • Populate these fields in OBSTACLE_AREAS.
  • Then replace name.contains(...) checks with:
if (currentArea.kind == Kind.GAP && currentArea.longAnimation) { ... }
if (currentArea.kind == Kind.CLIMBING_ROCKS) { ... }
if (currentArea.kind == Kind.GAP && currentArea.subStep == 1) { ... }

I can push a follow-up PR to wire this across both files.

Also applies to: 290-312, 443-466


883-885: Defensive null-check on TopLevelWorldView

In some edge cases the world view can be null and cause NPEs. Guard before use.

-            int currentPlane = Microbot.getClient().getTopLevelWorldView().getPlane();
+            int currentPlane = Microbot.getClient().getTopLevelWorldView() != null
+                ? Microbot.getClient().getTopLevelWorldView().getPlane()
+                : Rs2Player.getWorldLocation().getPlane();
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 5c9288d and fdad534.

📒 Files selected for processing (3)
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/AgilityScript.java (1 hunks)
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidCourse.java (1 hunks)
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidState.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/AgilityScript.java
🧰 Additional context used
🧬 Code graph analysis (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidCourse.java (7)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/models/AgilityObstacleModel.java (1)
  • AgilityObstacleModel (6-33)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/Global.java (1)
  • Global (10-171)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/dialogues/Rs2Dialogue.java (1)
  • Rs2Dialogue (18-725)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/gameobject/Rs2GameObject.java (1)
  • Rs2GameObject (36-2002)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2Inventory.java (1)
  • Rs2Inventory (60-2244)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/player/Rs2Player.java (1)
  • Rs2Player (67-2062)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidObstacleData.java (2)
  • PyramidObstacleData (11-119)
  • ObstacleArea (16-38)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build
🔇 Additional comments (8)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidState.java (1)

11-28: Solid, centralized state with appropriate concurrency primitives

Use of volatile longs for monotonic nanoTime deltas and AtomicBoolean flags is correct and thread-safe for this plugin’s needs. Cooldown constants in nanos are appropriate.

runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidCourse.java (7)

300-314: XP-obstacle gating looks good

Flagging all XP-granting obstacles and deferring new selections until XP drops materially reduces misclicks and state flapping. Nice.


248-251: Good fallback to nearest obstacle when no area matched

The staged fallback (special-case, then nearest) is pragmatic and will help recover from slight desyncs or missed clicks.


713-783: Nearest “any pyramid obstacle” search is careful (plane-aware + reachability)

Using reachability and longer radius on plane 3 is sensible for the top area’s sparsity.


155-159: Click-throttling via obstacle cooldown is a good guardrail

This prevents spam interacts during transitional animations.


798-848: Turn-in flow integration from ground-level start is robust

Conditionally diverting to Simon when full or above threshold, with state gating, matches the PR’s objectives.


1118-1189: Turn-in dialogue handling is resilient

Correctly handles both “already in dialogue” and “use item on NPC” flows, and clears the state when out of dialogue and inventory is empty.


851-1101: Action Required: Upstream ignores waitForCompletion return value

The call in AgilityScript.java does not check the boolean returned by waitForCompletion, so returning false in your new 12 XP branch will never trigger a retry. You’ll need to update the script to capture and act on that result.

• In runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/AgilityScript.java (around line 173), change:

// current (ignored return):
plugin.getCourseHandler().waitForCompletion(agilityExp, client.getLocalPlayer().getWorldLocation().getPlane());

to something like:

boolean attemptComplete = plugin.getCourseHandler()
    .waitForCompletion(agilityExp, client.getLocalPlayer().getWorldLocation().getPlane());
if (!attemptComplete)
{
    // retry obstacle: e.g., re-run interaction or loop back
    continue; // or your retry logic here
}

This ensures that when your PyramidCourse returns false (stone-block interruption), the script actually retries the obstacle.

Likely an incorrect or invalid review comment.

Comment on lines +911 to +928
// Check if this is a stone block (12 XP)
if (xpGained == 12) {
log.debug("Hit by stone block (12 XP) - clearing flags to allow immediate retry");
hitByStoneBlock = true;
lastKnownXp = currentXp;

// Clear flags to allow immediate retry of the obstacle
if (state.isDoingCrossGap()) {
state.clearCrossGap();
}
if (state.isDoingXpObstacle()) {
state.clearXpObstacle();
}

// Return immediately to retry the obstacle
Global.sleep(300, 400); // Small delay before retry
return true;
}
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 25, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix: stone-block XP branch should return false to trigger an immediate retry (not true)

Currently, on 12 XP (stone block), you clear flags and return true. Elsewhere (Lines 1055–1068), the retry path returns false. Returning true here can incorrectly mark the step as “complete,” skipping the intended retry.

-                    // Return immediately to retry the obstacle
-                    Global.sleep(300, 400); // Small delay before retry
-                    return true;
+                    // Return immediately to retry the obstacle
+                    Global.sleep(300, 400); // Small delay before retry
+                    return false;
📝 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 if this is a stone block (12 XP)
if (xpGained == 12) {
log.debug("Hit by stone block (12 XP) - clearing flags to allow immediate retry");
hitByStoneBlock = true;
lastKnownXp = currentXp;
// Clear flags to allow immediate retry of the obstacle
if (state.isDoingCrossGap()) {
state.clearCrossGap();
}
if (state.isDoingXpObstacle()) {
state.clearXpObstacle();
}
// Return immediately to retry the obstacle
Global.sleep(300, 400); // Small delay before retry
return true;
}
// Check if this is a stone block (12 XP)
if (xpGained == 12) {
log.debug("Hit by stone block (12 XP) - clearing flags to allow immediate retry");
hitByStoneBlock = true;
lastKnownXp = currentXp;
// Clear flags to allow immediate retry of the obstacle
if (state.isDoingCrossGap()) {
state.clearCrossGap();
}
if (state.isDoingXpObstacle()) {
state.clearXpObstacle();
}
// Return immediately to retry the obstacle
Global.sleep(300, 400); // Small delay before retry
return false;
}
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidCourse.java
around lines 911 to 928, the branch handling the 12 XP "stone block" currently
clears retry flags and then returns true, which incorrectly marks the obstacle
step as complete; change the final return value to false so the method signals
an immediate retry (preserve the clearing of state flags and the small sleep
delay before returning false).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While returning false would be more semantically correct, it would have no actual impact on the plugin's behavior since:

  1. The return value is never used
  2. The position-based obstacle selection already ensures correct retry behavior
  3. The observed behavior (clicking the same obstacle again after stone block) works correctly as-is

Copy link
Contributor

Choose a reason for hiding this comment

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

@runsonmypc Thank you for the clarification! You're absolutely right - if the return value isn't actually used by the calling code and the position-based obstacle selection already handles the retry logic correctly, then the change would indeed be purely semantic with no functional impact.

I appreciate you explaining how the agility plugin's retry mechanism works in practice. It's helpful to understand that the obstacle selection is driven by player position rather than the return value from waitForCompletion().

🐰
◕ ◕
( ◡ )
Learning something new! Thanks for the insight into how the retry mechanism actually works.


✏️ Learnings added
Learnt from: runsonmypc
PR: chsami/Microbot#1417
File: runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidCourse.java:912-929
Timestamp: 2025-08-25T18:35:52.311Z
Learning: In the Microbot agility plugin, the return value from waitForCompletion() method is not used by calling code, and retry behavior is handled by position-based obstacle selection rather than return values.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Pert added 3 commits August 25, 2025 13:53
- Remove dead code that was never referenced
- Sequence enforcement is handled through area-based system
- Addresses CodeRabbit review about unused code
…n comment

- Add package-private setter for pyramidTurnInThreshold for unit testing
- Clarify comment about when threshold is regenerated (in recordClimbingRocks)
- Improves testability and code documentation
- Addresses CodeRabbit review suggestions
- Replace deprecated findObjectByIdAndDistance with Rs2GameObject.getAll
- Use modern stream filtering and selection for finding stairs
- Change waterskin drop message from info to debug level
- Addresses CodeRabbit review about deprecated API and log spam
Copy link
Contributor

@gmason0 gmason0 left a comment

Choose a reason for hiding this comment

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

Again, please revert linting changes. I have no clue what you changed in the AgilityScript.

Pert added 2 commits August 25, 2025 14:13
- Check if TopLevelWorldView is null before accessing getPlane()
- Fallback to Rs2Player.getWorldLocation().getPlane() if null
- Prevents NPE during client loading or disconnection
- Addresses CodeRabbit review about defensive programming
- Revert line ending/formatting changes that obscured the diff
- Keep only the functional change: 100ms to 600ms scheduler interval
- Makes PR review much cleaner and easier to understand
@runsonmypc
Copy link
Contributor Author

Again, please revert linting changes. I have no clue what you changed in the AgilityScript.

Fixed, was caused by difference in invisible line endings. Unsure how or why it happened. I do use WSL so likely related.

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: 1

♻️ Duplicate comments (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidCourse.java (2)

488-521: Prefer reachable selection to avoid unclickable targets (carry-over from prior review)

Nearest-by-distance can select obstacles behind blocking geometry; filter by reachability first, then pick the nearest. Keeps clicks reliable and reduces retries.

Apply this diff:

     private TileObject findNearestObstacleWithinDistance(WorldPoint playerPos, int obstacleId, int maxDistance) {
         if (log.isDebugEnabled()) {
             log.debug("Looking for obstacle {} within {} tiles", obstacleId, maxDistance);
         }
 
         List<TileObject> obstacles = Rs2GameObject.getAll(obj -> 
             obj.getId() == obstacleId && 
             obj.getPlane() == playerPos.getPlane() &&
             obj.getWorldLocation().distanceTo(playerPos) <= maxDistance
         );
 
-        if (obstacles.isEmpty()) {
+        if (obstacles.isEmpty()) {
             if (log.isDebugEnabled()) {
                 log.debug("No obstacles found within {} tiles", maxDistance);
             }
             return null;
         }
 
-        // Log all found obstacles for debugging
-        if (log.isDebugEnabled()) {
-            log.debug("Found {} obstacles within {} tiles:", obstacles.size(), maxDistance);
-            for (TileObject obj : obstacles) {
-                log.debug("  - {} at {} (distance: {})", 
-                    obstacleId, obj.getWorldLocation(), obj.getWorldLocation().distanceTo(playerPos));
-            }
-        }
+        // Consider only reachable obstacles
+        List<TileObject> reachable = obstacles.stream()
+            .filter(this::isObstacleReachable)
+            .collect(Collectors.toList());
+        if (reachable.isEmpty()) {
+            if (log.isDebugEnabled()) {
+                log.debug("Found {} obstacles within {}, but none are reachable", obstacles.size(), maxDistance);
+            }
+            return null;
+        }
+
+        // Log reachable obstacles for debugging
+        if (log.isDebugEnabled()) {
+            log.debug("Found {} reachable obstacles within {} tiles:", reachable.size(), maxDistance);
+            for (TileObject obj : reachable) {
+                log.debug("  - {} at {} (distance: {})",
+                    obstacleId, obj.getWorldLocation(), obj.getWorldLocation().distanceTo(playerPos));
+            }
+        }
 
-        return obstacles.stream()
+        return reachable.stream()
             .min((a, b) -> Integer.compare(
                 a.getWorldLocation().distanceTo(playerPos),
                 b.getWorldLocation().distanceTo(playerPos)
             ))
             .orElse(null);
     }

909-929: Stone-block branch should signal an immediate retry (not completion) — fix return value

Comment says “retry the obstacle,” but the code returns true, marking it complete and skipping the intended retry logic. Prior review flagged this; please change to return false.

Apply this diff:

                 if (xpGained == 12) {
                     log.debug("Hit by stone block (12 XP) - clearing flags to allow immediate retry");
                     hitByStoneBlock = true;
                     lastKnownXp = currentXp;
                     
                     // Clear flags to allow immediate retry of the obstacle
                     if (state.isDoingCrossGap()) {
                         state.clearCrossGap();
                     }
                     if (state.isDoingXpObstacle()) {
                         state.clearXpObstacle();
                     }
                     
                     // Return immediately to retry the obstacle
                     Global.sleep(300, 400); // Small delay before retry
-                    return true;
+                    return false;
                 }
🧹 Nitpick comments (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidState.java (1)

39-41: Guard “unset” timestamps to avoid a rare startup cooldown stall

If the plugin initializes within 1.5s of JVM start, lastObstacleStartTime == 0 could cause a transient cooldown. Make “0 means no cooldown” explicit.

Apply this diff:

 public boolean isObstacleCooldownActive() {
-    return System.nanoTime() - lastObstacleStartTime < OBSTACLE_COOLDOWN;
+    // Treat 0 as "no cooldown recorded yet"
+    long last = lastObstacleStartTime;
+    if (last == 0) return false;
+    return System.nanoTime() - last < OBSTACLE_COOLDOWN;
 }
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidCourse.java (1)

720-731: Choose the nearest plank-end ground object (don’t return first found)

findNearestPyramidObstacle currently returns the first 10868 ground object found within 15 tiles, which can be non-deterministic. Pick the nearest to the player, mirroring your other helpers.

Apply this diff:

-        // First check for ground objects (plank ends)
-        List<GroundObject> groundObjects = Rs2GameObject.getGroundObjects();
-        for (GroundObject go : groundObjects) {
-            if (go.getId() == 10868 && 
-                go.getPlane() == playerPos.getPlane() &&
-                go.getWorldLocation().distanceTo(playerPos) <= 15) {
-                if (log.isDebugEnabled()) {
-                    log.debug("Found nearby plank end (ground object) at {}", go.getWorldLocation());
-                }
-                return go;
-            }
-        }
+        // First check for ground objects (plank ends)
+        List<GroundObject> groundObjects = Rs2GameObject.getGroundObjects();
+        List<GroundObject> nearbyPlanks = groundObjects.stream()
+            .filter(go -> go.getId() == 10868
+                && go.getPlane() == playerPos.getPlane()
+                && go.getWorldLocation().distanceTo(playerPos) <= 15)
+            .collect(Collectors.toList());
+        if (!nearbyPlanks.isEmpty()) {
+            GroundObject nearestPlank = nearbyPlanks.stream()
+                .min((a, b) -> Integer.compare(
+                    a.getWorldLocation().distanceTo(playerPos),
+                    b.getWorldLocation().distanceTo(playerPos)))
+                .orElse(null);
+            if (nearestPlank != null && log.isDebugEnabled()) {
+                log.debug("Found nearby plank end (ground object) at {}", nearestPlank.getWorldLocation());
+            }
+            return nearestPlank;
+        }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between fdad534 and 4a67258.

📒 Files selected for processing (2)
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidCourse.java (1 hunks)
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidState.java (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidCourse.java (7)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/models/AgilityObstacleModel.java (1)
  • AgilityObstacleModel (6-33)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/Global.java (1)
  • Global (10-171)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/dialogues/Rs2Dialogue.java (1)
  • Rs2Dialogue (18-725)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/gameobject/Rs2GameObject.java (1)
  • Rs2GameObject (36-2002)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2Inventory.java (1)
  • Rs2Inventory (60-2244)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/player/Rs2Player.java (1)
  • Rs2Player (67-2062)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidObstacleData.java (2)
  • PyramidObstacleData (11-119)
  • ObstacleArea (16-38)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build
🔇 Additional comments (4)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidState.java (2)

17-24: Thread-safe state design looks solid

Use of AtomicBoolean for flags and volatile for timestamps/threshold is appropriate for cross-thread reads/writes in the plugin loop. Randomized turn-in threshold with a test override is a nice touch for determinism in tests.


147-154: Reset sequence is complete

Reset clears timestamps, flags, and regenerates the threshold — exactly what we want when restarting the course or recovering from a bad state.

runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/PyramidCourse.java (2)

814-847: Nice QoL: direct stairs interaction with reachability check

The direct click on stairs when returning from Simon (with canReach gating) is a good shortcut before falling back to pathing. Cleanly implemented with sensible bounds.


1197-1205: Waterskin auto-drop behavior aligns with PR notes

Dropping WATERSKIN(0) proactively prevents inventory stalls and matches the PR’s QoL description. Keeping a short sleep is fine to let the inventory update.

Prevent NPE when Rs2Player.getWorldLocation() returns null during logout/disconnect
by adding null check and early return before accessing playerPos methods
@gmason0 gmason0 merged commit a9b3fd6 into chsami:development Aug 29, 2025
2 checks passed
@chsami chsami mentioned this pull request Sep 3, 2025
@coderabbitai coderabbitai bot mentioned this pull request Sep 20, 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.

3 participants