feat(agility): Add efficient alching mode with improved obstacle completion detection#1337
Conversation
runsonmypc
changed the title
| if (Rs2Player.isMoving() || Rs2Player.isAnimating()) | ||
| { | ||
| // Disable XP checks for Colossal Wyrm courses (they have multi-XP drop obstacles) | ||
| boolean isColossalWyrm = config.agilityCourse().name().contains("COLOSSAL_WYRM"); |
There was a problem hiding this comment.
I think the real issue here is the waitForCompletion method from the CourseHandler interface.
This method should be holding this future until the obstacle is completed.
We can create exceptions for specfic courses by overriding this method in the course's class, or globally change it by changing it in the interface.
| { | ||
| plugin.getCourseHandler().waitForCompletion(agilityExp, Microbot.getClient().getLocalPlayer().getWorldLocation().getPlane()); | ||
| // Handle alchemy if enabled | ||
| if (config.alchemy()) { |
There was a problem hiding this comment.
Can this be refactored to not be so, nested? I think we could create a method on the interface & override it if there is going to be exceptions for specfic courses.
runsonmypc
force-pushed
the
agility-efficient-alching
branch
from
b53f764 to
1ece95d
Compare
|
@g-mason0 Thanks for the review! I've addressed both of your concerns: Changes Made:
|
| if (plugin.getCourseHandler().getCurrentObstacleIndex() > 0) | ||
| { | ||
| if (Rs2Player.isMoving() || Rs2Player.isAnimating()) | ||
| boolean obstacleComplete = plugin.getCourseHandler().isObstacleComplete( |
There was a problem hiding this comment.
Shouldn't the functionality be at the end of the loop? seems like you wanted to replace waitForCompletion.
There was a problem hiding this comment.
@g-mason0 Thanks for the feedback! You're right - the logic was scattered. I've refactored to centralize everything in the course handler:
- Added shouldClickObstacle() method that handles the "don't click while animating unless XP gained" logic
- Kept waitForCompletion() for post-click blocking
- Removed duplicate XP tracking and animation checks from the main script
Now the handler controls all timing decisions. The script just asks "should I click?" and the handler decides based on animation/XP state. Much cleaner than having these checks spread across multiple places.
The XP drop detection for early action still works - if we get XP while animating, shouldClickObstacle() returns true and we can click the next obstacle immediately.
- Add new config option for efficient alching - When enabled and obstacle is 5+ tiles away, clicks obstacle first, then alchs during movement, then clicks again - Add timestamp-based waiting to prevent mid-obstacle interactions during animation pauses - Track agility XP to detect obstacle completion - Fix double alching bug on first obstacle after lap completion
- Change hardcoded 1000ms delay to random 700-1100ms range - Makes bot behavior less predictable and more human-like - Applies to both Colossal Wyrm XP check disable and general pause detection
- Add 100-300ms random delay after detecting XP gain - Makes reactions more human-like while still faster than timeout
- Add config option to randomly skip alching with 0-100% chance - Default set to 5% skip chance for more human-like behavior - Applies to all obstacles including first obstacle (index 0) - Works with both normal and efficient alching modes
…ode nesting - Add isObstacleComplete() method to AgilityCourseHandler for course-specific completion logic - Override completion detection in Colossal Wyrm courses to handle multi-XP drops - Extract alching logic into separate helper methods (shouldPerformAlch, performEfficientAlch, performNormalAlch) - Consolidate course-specific actions into handleCourseSpecificActions() - Reduce nesting and improve code readability per PR feedback
…start - Add Rs2Player.isWalking() check at obstacle 0 to skip alching while walking - Ensures obstacle is clicked first after returning to course start
- Add config to skip alching when obstacle is <5 tiles away - Move skip inefficient config above alch skip chance - Fix return handling for efficient alch in skip inefficient mode
- Keep waitForCompletion() as blocking post-click check (not replaced) - Add pre-click animation guard to prevent clicks while animating without XP - Move animation check before alching code to guard all click paths - Remove redundant obstacle completion check that was causing conflicts Addresses g-mason0's comment: The XP detection is intentionally split - pre-click check prevents premature clicks, post-click waitForCompletion blocks until XP/timeout. This ensures we only act while animating when XP drop signals completion.
- Move animation/XP checking to shouldClickObstacle() method in handler - Remove duplicate XP tracking update after waitForCompletion() - Simplify flow: only update lastAgilityXp when XP changes (line 137) Addresses g-mason0's comment: XP detection logic now lives in the course handler methods rather than scattered in the script. The handler determines when to click (shouldClickObstacle) and when complete (waitForCompletion).
- Override shouldClickObstacle() in both Wyrm courses to ignore XP drops - Some Wyrm obstacles have multiple XP drops per animation, making early action unreliable - Maintains standard animation-based timing for these specific courses
runsonmypc
force-pushed
the
agility-efficient-alching
branch
from
1ece95d to
0ef7b74
Compare
WalkthroughAdds XP-aware gating and polling-based completion checks to the agility loop, modularizes course-specific actions, and integrates configurable alching (efficient/skip/skip-chance). Extends course handlers (Colossal Wyrm variants) with movement/animation/time-based completion logic and adds new config options. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant AS as AgilityScript
participant CFG as Config
participant CH as CourseHandler
participant CRS as Course (e.g., Wyrm)
participant G as Game
AS->>G: read current agility XP
AS->>CFG: read alch config (efficient, skipInefficient, alchSkipChance)
AS->>AS: shouldPerformAlch()?
alt Alch path
AS->>G: perform efficient or normal alch
AS->>AS: update lastAgilityXp if XP gained
AS-->>AS: continue loop
else No alch
AS->>AS: handleCourseSpecificActions(location)
alt Handled
AS-->>AS: continue loop
else Not handled
AS->>CH: shouldClickObstacle(currentXp,lastXp)?
alt Ready
AS->>G: interact with obstacle
AS->>CRS: waitForCompletion(previousXp, plane) [polling]
CRS-->>AS: completed or timeout
AS->>AS: update lastAgilityXp if XP gained
else Not ready
AS-->>AS: continue loop
end
end
end
sequenceDiagram
autonumber
participant AS as AgilityScript
participant G as Game
note right of AS: Efficient alch flow (if enabled & obstacle distant)
AS->>G: Click obstacle (focus)
AS->>G: Cast High Alch on item
AS->>G: Click obstacle again
AS->>AS: poll for completion / XP change
note right of AS: Normal alch: just cast High Alch and poll
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/AgilityCourseHandler.java (1)
93-129: waitForCompletion ignores the new isObstacleComplete hook; integrate it to honor course-specific rules.Right now, this loop returns true on any AGILITY XP change—even if still animating. That breaks the stated intent to let courses like Colossal Wyrm disable XP-based completion and rely on movement/animation + timing. Integrate isObstacleComplete() into the polling loop and track “last movement stop” time so course overrides can enforce their custom wait. This also resolves a past reviewer concern about making the CourseHandler hold the future until completion.
Apply this diff to delegate completion checks to the hook and support movement-stop delay:
@@ - double initialHealth = Rs2Player.getHealthPercentage(); - int timeoutMs = 15000; - long startTime = System.currentTimeMillis(); - - // Check every 100ms for completion - while (System.currentTimeMillis() - startTime < timeoutMs) - { - // Check if we gained XP - if (Microbot.getClient().getSkillExperience(Skill.AGILITY) != agilityExp) - { - // XP gained! Check if we're still animating - if (Rs2Player.isAnimating() || Rs2Player.isMoving()) - { - // We got XP while still animating - this is the early detection! - // Return true but the caller can check animation state - return true; - } - // Got XP and not animating - return true; - } - - // Check other completion conditions - if (Rs2Player.getHealthPercentage() < initialHealth || - Microbot.getClient().getTopLevelWorldView().getPlane() != plane) - { - return true; - } - - // Sleep before next check - Global.sleep(100); - } + double initialHealth = Rs2Player.getHealthPercentage(); + final int timeoutMs = 15_000; + final int pollMs = 100; + final int waitDelayMs = 250; // time to wait after movement stops + long startTime = System.currentTimeMillis(); + + // Track movement-stop time to support course-specific completion logic + boolean wasMoving = Rs2Player.isMoving() || Rs2Player.isAnimating(); + long lastMovementStop = wasMoving ? 0L : System.currentTimeMillis(); + + while (System.currentTimeMillis() - startTime < timeoutMs) + { + // Update movement-stop timestamp on transition + boolean nowMoving = Rs2Player.isMoving() || Rs2Player.isAnimating(); + if (!nowMoving && wasMoving) { + lastMovementStop = System.currentTimeMillis(); + } + wasMoving = nowMoving; + + // Delegate completion decision to course-specific hook + int currentXp = Microbot.getClient().getSkillExperience(Skill.AGILITY); + if (isObstacleComplete(currentXp, agilityExp, lastMovementStop, waitDelayMs)) { + return true; + } + + // Fallback completion conditions + if (Rs2Player.getHealthPercentage() < initialHealth || + Microbot.getClient().getTopLevelWorldView().getPlane() != plane) + { + return true; + } + + Global.sleep(pollMs); + } @@ // Timeout reached return false;
🧹 Nitpick comments (6)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/AgilityCourseHandler.java (1)
210-223: Consider documenting parameters and semantics of isObstacleComplete.Please add brief Javadoc to clarify:
- previousXp is the XP at interaction start.
- lastMovingTime is the timestamp when movement last transitioned to “stopped.”
- waitDelay is the minimal idle duration to treat as complete.
This prevents misuse and makes the contract clear for course authors.
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/MicroAgilityConfig.java (1)
81-115: Add input validation for alchSkipChance and clarify “5+ tiles away” semantics.
- alchSkipChance should be clamped/validated to [0,100] to avoid misconfiguration resulting in always/never alching.
- If feasible, add a short note that “5+ tiles away” is computed by WorldPoint.distanceTo to align expectations.
Suggested change (adds @range; adjust imports accordingly):
@@ -import net.runelite.client.config.ConfigItem; +import net.runelite.client.config.ConfigItem; +import net.runelite.client.config.Range; @@ @ConfigItem( keyName = "alchSkipChance", name = "Alch Skip Chance", description = "Percentage chance to skip alching on any obstacle (0-100)", position = 8, section = generalSection ) + @Range(min = 0, max = 100) default int alchSkipChance()If @range isn’t available in your target Runelite version, fall back to clamping at the call site.
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/AgilityScript.java (4)
141-181: Reduce log spam and duplicate inventory scans in alch flow.
- getAlchItem() logs “No items specified…” and “No valid items…” every tick when alching is enabled but misconfigured. That will spam logs at 10 Hz. Prefer a single warning (e.g., once per minute or on state change), or remove logs from the hot path.
- You call getAlchItem() inside shouldPerformAlch() and again in the block below. Avoid scanning twice per tick.
Minimal change: make shouldPerformAlch() only check config flags and skip chance, and defer item lookup to the call site (see diff below).
@@ - // Check if we should skip alching based on configured chance - if (Math.random() * 100 < config.alchSkipChance()) - { - return false; - } - - return getAlchItem().isPresent(); + // Check if we should skip alching based on configured chance + if (Math.random() * 100 < config.alchSkipChance()) + { + return false; + } + // Item availability is checked at the call site to avoid repeated inventory scans each tick + return true;
183-198: Consider using the wait result and surface timeouts.You’re ignoring the boolean returned by waitForCompletion(). After integrating course-specific completion, it’d be useful to:
- Bail early on false (timeout) to avoid acting on stale state.
- Optionally log a throttled warning to help diagnose stalls.
This is optional but improves observability.
354-359: Remove unused parameter from performNormalAlch and update call sites.currentAgilityXp isn’t used. Simplify the signature and the two call sites.
@@ - private void performNormalAlch(String alchItem, int currentAgilityXp) + private void performNormalAlch(String alchItem) { // Simple alch - waitForCompletion handles all timing Rs2Magic.alch(alchItem, 50, 75); } @@ - performNormalAlch(alchItem.get(), currentAgilityXp); + performNormalAlch(alchItem.get()); @@ - performNormalAlch(alchItem.get(), currentAgilityXp); + performNormalAlch(alchItem.get());Also applies to: 161-163, 178-179
331-352: Efficient alching path is coherent; minor safety nit.Consider a small guard to ensure the second interact() isn’t attempted if the first click failed to start movement (rare, but avoids a no-op alch while stationary). You can check Rs2Player.isMoving() or distance delta before the second click.
📜 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.
📒 Files selected for processing (5)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/AgilityScript.java(6 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/MicroAgilityConfig.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/AgilityCourseHandler.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/ColossalWyrmAdvancedCourse.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/ColossalWyrmBasicCourse.java(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/ColossalWyrmBasicCourse.java (1)
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/ColossalWyrmAdvancedCourse.java (1)
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/AgilityCourseHandler.java (2)
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/util/Global.java (1)
Global(10-171)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/AgilityScript.java (3)
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/player/Rs2Player.java (1)
Rs2Player(67-2062)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/magic/Rs2Magic.java (1)
Rs2Magic(43-600)
⏰ 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/ColossalWyrmBasicCourse.java (1)
38-42: Gating click until fully idle is correct for multi-XP obstacles.Returning true only when not moving and not animating prevents early re-clicks on Colossal Wyrm. This aligns with the PR goal to avoid mid-obstacle interactions.
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/AgilityCourseHandler.java (1)
80-91: Reasonable default: allow re-click only after XP while moving/animating.This balances responsiveness with safety. If we’re still moving/animating, require XP > lastXp; otherwise allow clicking. Good baseline behavior for most courses.
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/ColossalWyrmAdvancedCourse.java (1)
38-42: Same as Basic: correct to require fully idle before clicking.Avoids early interactions on multi-XP obstacles; consistent with the Basic course override.
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/AgilityScript.java (5)
41-41: XP tracking initialization looks good.Capturing lastAgilityXp at startup enables early XP-based completion and gating.
Also applies to: 62-62
108-111: Good consolidation of course-specific handling.Early-returning here keeps the main loop simpler and respects per-course behaviors.
128-139: Gating re-clicks via shouldClickObstacle is appropriate; ensure lastAgilityXp stays accurate.The pattern “check gate → update lastAgilityXp when it increases” is correct. After integrating isObstacleComplete in the handler, this gating will remain consistent across courses (with Wyrm overrides).
315-329: Skip-chance logic is fine; seed/cycle not required.The simple Math.random()-based skip is acceptable here. After removing the item lookup from this method (previous comment), this becomes a cheap check in the loop.
360-380: Nice extraction of course-specific actions.This trims nesting and keeps the main loop focused.
...in/java/net/runelite/client/plugins/microbot/agility/courses/ColossalWyrmAdvancedCourse.java
Show resolved
Hide resolved
.../main/java/net/runelite/client/plugins/microbot/agility/courses/ColossalWyrmBasicCourse.java
Show resolved
Hide resolved
- Add @range(min = 0, max = 100) annotation to validate percentage bounds - Prevents misconfiguration with values outside 0-100 range
- Remove duplicate getAlchItem() call from shouldPerformAlch() - Comment out log statements that spam at 10 Hz when misconfigured - Inventory now scanned once per tick instead of twice
|
Addressed coderabbit comments. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/ColossalWyrmBasicCourse.java (1)
48-87: Nice: you’ve wired the course-specific isObstacleComplete into waitForCompletion
This addresses the earlier concern that the override wouldn’t be consulted for Wyrm.runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/ColossalWyrmAdvancedCourse.java (1)
48-87: Nice: you’ve wired course-specific completion into the wait loop
This resolves the prior concern that the override wasn’t being consulted.
🧹 Nitpick comments (6)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/ColossalWyrmBasicCourse.java (3)
74-79: Use player plane via Rs2Player.getWorldLocation() (handles instanced scenes)
The TopLevelWorldView plane can be less robust than the player’s actual world location, especially in instances. Prefer the player’s plane.- if (Rs2Player.getHealthPercentage() < initialHealth || - Microbot.getClient().getTopLevelWorldView().getPlane() != plane) + if (Rs2Player.getHealthPercentage() < initialHealth || + (Rs2Player.getWorldLocation() != null && Rs2Player.getWorldLocation().getPlane() != plane)) { return true; }
51-55: Extract magic numbers and/or reuse Global.sleepUntilTrue to simplify the loop
TIMEOUT (15000 ms), WAIT_DELAY (2000 ms), and the poll interval (100 ms) should be named constants for readability/tuning. Alternatively, reuse Global.sleepUntilTrue to centralize timing.Apply within this class:
- int timeoutMs = 15000; + int timeoutMs = TIMEOUT_MS; ... - int waitDelay = 2000; // Colossal Wyrm needs longer wait after movement stops + int waitDelay = WAIT_DELAY_MS; // Colossal Wyrm needs longer wait after movement stops ... - Global.sleep(100); + Global.sleep(POLL_INTERVAL_MS);Add near the top of the class (outside the shown ranges):
// Proposed constants private static final int TIMEOUT_MS = 15_000; private static final int WAIT_DELAY_MS = 2_000; private static final int POLL_INTERVAL_MS = 100;Or replace the custom while-loop with:
boolean completed = Global.sleepUntilTrue( () -> { if (Rs2Player.isMoving() || Rs2Player.isAnimating()) { lastMovingTimeRef.set(System.currentTimeMillis()); hasStartedRef.set(true); return false; } return hasStartedRef.get() && System.currentTimeMillis() - lastMovingTimeRef.get() >= waitDelay; }, 100, /* poll */ 15000 /* timeout */ );Also applies to: 56-56, 81-83
47-87: Client-State Access Is Consistent—Optional Thread Safety Refactor
Other agility courses inrunelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/also accessMicrobot.getClient()getters (XP, plane, player location) directly on the calling thread. No existing course wraps these inrunOnClientThreadOptional, so your implementation is consistent with the codebase.If you’ve experienced intermittent NPEs or race conditions when reading client state, you may optionally batch or wrap these reads on the client thread. For example, in
ColossalWyrmBasicCourse.javawithinwaitForCompletion(lines 47–87):• Wrap calls such as
Microbot.getClient().getSkillExperience(Skill.AGILITY) Microbot.getClient().getTopLevelWorldView().getPlane() Rs2Player.getHealthPercentage()inside
Microbot.getClientThread().runOnClientThreadOptional(() -> /* getter call */)or collect multiple values in one
runOnClientThreadOptionalinvocation.No critical or mandatory changes are required–this is purely an optional thread‐safety enhancement.
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/ColossalWyrmAdvancedCourse.java (3)
74-79: Prefer player plane via Rs2Player.getWorldLocation()
Match the Basic course suggestion for robustness in instances.- if (Rs2Player.getHealthPercentage() < initialHealth || - Microbot.getClient().getTopLevelWorldView().getPlane() != plane) + if (Rs2Player.getHealthPercentage() < initialHealth || + (Rs2Player.getWorldLocation() != null && Rs2Player.getWorldLocation().getPlane() != plane)) { return true; }
51-55: Extract constants / reuse Global helpers
Duplicate note as Basic: replace magic numbers with named constants or use Global.sleep helpers to centralize timing.- int timeoutMs = 15000; + int timeoutMs = TIMEOUT_MS; ... - int waitDelay = 2000; + int waitDelay = WAIT_DELAY_MS; ... - Global.sleep(100); + Global.sleep(POLL_INTERVAL_MS);Add near top of class:
private static final int TIMEOUT_MS = 15_000; private static final int WAIT_DELAY_MS = 2_000; private static final int POLL_INTERVAL_MS = 100;Also applies to: 56-56, 81-83
48-99: Reduce duplication between Basic and Advanced (extract shared logic)
The three methods (shouldClickObstacle, waitForCompletion, isObstacleComplete) are identical across Basic/Advanced except obstacle definitions and possibly waitDelay. Factor them into:
- a shared abstract base (e.g., AbstractColossalWyrmCourse with getPostMoveDelayMs()), or
- default methods on AgilityCourseHandler with a single course-specific knob for waitDelay.
This lowers maintenance cost and keeps behaviors in sync.
If helpful, I can provide a concrete refactor sketch.
📜 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.
📒 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/MicroAgilityConfig.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/AgilityCourseHandler.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/ColossalWyrmAdvancedCourse.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/ColossalWyrmBasicCourse.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/AgilityScript.java
- runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/AgilityCourseHandler.java
- runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/MicroAgilityConfig.java
🧰 Additional context used
🧬 Code graph analysis (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/ColossalWyrmAdvancedCourse.java (2)
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/player/Rs2Player.java (1)
Rs2Player(67-2062)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/ColossalWyrmBasicCourse.java (3)
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/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 (6)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/ColossalWyrmBasicCourse.java (3)
41-45: Good: disables XP-gated early clicks for multi-XP obstacles
Blocking clicks while moving/animating directly addresses mid-obstacle misclicks on Wyrm.
89-99: LGTM: XP-agnostic completion check tailored for Wyrm
This properly ignores XP and waits for a quiet window after movement/animation.
3-12: Import net.runelite.api.gameval.ObjectID is valid and preferred
The RuneLite API provides ObjectID.java in both net.runelite.api and net.runelite.api.gameval, and the gameval package contains the newer, preferred IDs. Your import ofimport net.runelite.api.gameval.ObjectID;is correct, compiles successfully, and aligns with existing microbot plugins that use the gameval IDs. No changes are necessary.
Likely an incorrect or invalid review comment.
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/ColossalWyrmAdvancedCourse.java (3)
41-45: Good: avoids early clicks; consistent with Basic
This mirrors the Basic course’s safety.
89-99: LGTM: XP-agnostic completion heuristic for multi-XP obstacles
Same validation as Basic; appropriate for Wyrm.
3-12: Importnet.runelite.api.gameval.ObjectIDis correct
TheObjectIDclass in thenet.runelite.api.gamevalpackage exists (as confirmed in your API dependency), so the current import on line 6 of ColossalWyrmAdvancedCourse.java is valid and needs no change.
| public boolean waitForCompletion(final int agilityExp, final int plane) | ||
| { | ||
| double initialHealth = Rs2Player.getHealthPercentage(); | ||
| int timeoutMs = 15000; | ||
| long startTime = System.currentTimeMillis(); | ||
| long lastMovingTime = System.currentTimeMillis(); | ||
| int waitDelay = 2000; // Colossal Wyrm needs longer wait after movement stops | ||
|
|
||
| // Check every 100ms for completion | ||
| while (System.currentTimeMillis() - startTime < timeoutMs) | ||
| { | ||
| // Update last moving time if player is still moving/animating | ||
| if (Rs2Player.isMoving() || Rs2Player.isAnimating()) | ||
| { | ||
| lastMovingTime = System.currentTimeMillis(); | ||
| } | ||
|
|
||
| // Get current XP | ||
| int currentXp = Microbot.getClient().getSkillExperience(Skill.AGILITY); | ||
|
|
||
| // Use our custom completion logic that ignores XP | ||
| if (isObstacleComplete(currentXp, agilityExp, lastMovingTime, waitDelay)) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| // Check other completion conditions (health loss, plane change) | ||
| if (Rs2Player.getHealthPercentage() < initialHealth || | ||
| Microbot.getClient().getTopLevelWorldView().getPlane() != plane) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| // Sleep before next check | ||
| Global.sleep(100); | ||
| } | ||
|
|
||
| // Timeout reached | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Same potential false-positive completion as Basic when no movement occurs
Mirror the hasStarted guard to ensure we observed movement/animation before declaring completion.
long lastMovingTime = System.currentTimeMillis();
int waitDelay = 2000; // Colossal Wyrm needs longer wait after movement stops
+ boolean hasStarted = false;
...
- if (Rs2Player.isMoving() || Rs2Player.isAnimating())
+ if (Rs2Player.isMoving() || Rs2Player.isAnimating())
{
lastMovingTime = System.currentTimeMillis();
+ hasStarted = true;
}
...
- if (isObstacleComplete(currentXp, agilityExp, lastMovingTime, waitDelay))
+ if (hasStarted && isObstacleComplete(currentXp, agilityExp, lastMovingTime, waitDelay))
{
return true;
}📝 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.
| public boolean waitForCompletion(final int agilityExp, final int plane) | |
| { | |
| double initialHealth = Rs2Player.getHealthPercentage(); | |
| int timeoutMs = 15000; | |
| long startTime = System.currentTimeMillis(); | |
| long lastMovingTime = System.currentTimeMillis(); | |
| int waitDelay = 2000; // Colossal Wyrm needs longer wait after movement stops | |
| // Check every 100ms for completion | |
| while (System.currentTimeMillis() - startTime < timeoutMs) | |
| { | |
| // Update last moving time if player is still moving/animating | |
| if (Rs2Player.isMoving() || Rs2Player.isAnimating()) | |
| { | |
| lastMovingTime = System.currentTimeMillis(); | |
| } | |
| // Get current XP | |
| int currentXp = Microbot.getClient().getSkillExperience(Skill.AGILITY); | |
| // Use our custom completion logic that ignores XP | |
| if (isObstacleComplete(currentXp, agilityExp, lastMovingTime, waitDelay)) | |
| { | |
| return true; | |
| } | |
| // Check other completion conditions (health loss, plane change) | |
| if (Rs2Player.getHealthPercentage() < initialHealth || | |
| Microbot.getClient().getTopLevelWorldView().getPlane() != plane) | |
| { | |
| return true; | |
| } | |
| // Sleep before next check | |
| Global.sleep(100); | |
| } | |
| // Timeout reached | |
| return false; | |
| } | |
| public boolean waitForCompletion(final int agilityExp, final int plane) | |
| { | |
| double initialHealth = Rs2Player.getHealthPercentage(); | |
| int timeoutMs = 15000; | |
| long startTime = System.currentTimeMillis(); | |
| long lastMovingTime = System.currentTimeMillis(); | |
| int waitDelay = 2000; // Colossal Wyrm needs longer wait after movement stops | |
| boolean hasStarted = false; | |
| // Check every 100ms for completion | |
| while (System.currentTimeMillis() - startTime < timeoutMs) | |
| { | |
| // Update last moving time if player is still moving/animating | |
| if (Rs2Player.isMoving() || Rs2Player.isAnimating()) | |
| { | |
| lastMovingTime = System.currentTimeMillis(); | |
| hasStarted = true; | |
| } | |
| // Get current XP | |
| int currentXp = Microbot.getClient().getSkillExperience(Skill.AGILITY); | |
| // Use our custom completion logic that ignores XP, but only after movement starts | |
| if (hasStarted && isObstacleComplete(currentXp, agilityExp, lastMovingTime, waitDelay)) | |
| { | |
| return true; | |
| } | |
| // Check other completion conditions (health loss, plane change) | |
| if (Rs2Player.getHealthPercentage() < initialHealth || | |
| Microbot.getClient().getTopLevelWorldView().getPlane() != plane) | |
| { | |
| return true; | |
| } | |
| // Sleep before next check | |
| Global.sleep(100); | |
| } | |
| // Timeout reached | |
| return false; | |
| } |
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/ColossalWyrmAdvancedCourse.java
around lines 48-87, add a hasStarted boolean guard to avoid false-positive
completion when there was never any movement: initialize hasStarted = false
before the loop, set hasStarted = true whenever Rs2Player.isMoving() ||
Rs2Player.isAnimating() is observed, and require hasStarted to be true before
treating the obstacle as complete (i.e., only return true from the
isObstacleComplete branch if hasStarted is true); keep the existing health-loss
and plane-change early exits unchanged.
.../main/java/net/runelite/client/plugins/microbot/agility/courses/ColossalWyrmBasicCourse.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/AgilityScript.java (1)
382-402: Move course-specific branching into the handler interfaceThe instanceof chain is a code smell that will grow as courses expand. Prefer a default method on AgilityCourseHandler, e.g., handleCourseSpecificActions(WorldPoint), and override in courses that need it. This aligns with earlier review feedback to push exceptions into course handlers.
If you’d like, I can sketch the interface default method and the minimal overrides.
🧹 Nitpick comments (6)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/AgilityScript.java (6)
92-95: Avoid duplicate XP reads; reuse currentAgilityXpYou read AGILITY XP twice (currentAgilityXp and agilityExp). Keeping one source-of-truth simplifies reasoning and reduces TOCTOU risk. Pass currentAgilityXp to performEfficientAlch(...) and waitForCompletion(...).
Apply this diff:
- final int agilityExp = Microbot.getClient().getSkillExperience(Skill.AGILITY); ... - if (performEfficientAlch(gameObject, alchItem.get(), agilityExp)) + if (performEfficientAlch(gameObject, alchItem.get(), currentAgilityXp)) ... - boolean completed = plugin.getCourseHandler().waitForCompletion(agilityExp, + boolean completed = plugin.getCourseHandler().waitForCompletion(currentAgilityXp, Microbot.getClient().getLocalPlayer().getWorldLocation().getPlane());Also applies to: 114-116, 155-159, 187-189
141-182: De-duplicate distance checks and avoid magic number 5Hard-coding 5 in multiple places and re-computing distance increases drift risk. Introduce a constant and compute once.
Apply this diff within the method:
if (shouldPerformAlch()) { Optional<String> alchItem = getAlchItem(); if (alchItem.isPresent()) { + final int distanceToObstacle = gameObject.getWorldLocation().distanceTo(playerWorldLocation); // Check if we should skip inefficient alchs if (config.skipInefficient()) { // Only alch if obstacle is far enough for efficient alching - if (gameObject.getWorldLocation().distanceTo(playerWorldLocation) >= 5) + if (distanceToObstacle >= EFFICIENT_ALCH_DISTANCE) {And add this constant near the top of the class (outside this hunk):
// Place near other fields private static final int EFFICIENT_ALCH_DISTANCE = 5;
185-199: Completion wait + antiban timing — LGTM; slight resilience nitThe wait/timeout path looks good and the throttled warning is helpful. Consider caching LocalPlayer once per tick to avoid repeated getLocalPlayer() lookups and a narrow TOCTOU window if logout occurs mid-tick, but this is minor.
I can provide a small patch to cache the local player reference at the top of the loop if you want it.
Also applies to: 203-208
326-340: Use integer RNG for skip chanceUsing doubles for percent comparisons is fine, but an integer RNG is simpler and avoids float semantics.
Apply this diff:
- if (Math.random() * 100 < config.alchSkipChance()) + if (java.util.concurrent.ThreadLocalRandom.current().nextInt(100) < config.alchSkipChance()) { return false; }
219-239: Null-safe itemsToAlch and avoid forced lowercaseIf Rs2Inventory.hasItem is case-sensitive, downcasing user input can break matches. Also guard against a null config value.
Apply this diff:
- String itemsInput = config.itemsToAlch().trim(); + final String itemsInput = Optional.ofNullable(config.itemsToAlch()).orElse("").trim(); ... - List<String> itemsToAlch = Arrays.stream(itemsInput.split(",")) + List<String> itemsToAlch = Arrays.stream(itemsInput.split(",")) .map(String::trim) - .map(String::toLowerCase) .filter(s -> !s.isEmpty()) .collect(Collectors.toList());Follow-up: If available, prefer resolving to Rs2ItemModel (via Rs2Inventory.get) and calling Rs2Magic.alch(Rs2ItemModel, …) to avoid any case sensitivity issues end-to-end.
342-374: Add a guard for widget (spell) selection before the second interactIt looks like there’s no existing helper in Microbot for clearing a selected spell/widget (we only have Rs2Inventory.deselect() for items). To avoid sending a WIDGET_TARGET_ON_GAME_OBJECT action on the obstacle when the alchemy spell remains selected, bail out (and retry on the next tick) if any widget is still selected:
• File:
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/AgilityScript.java
Lines: around 350–360Rs2Magic.alch(alchItem, 50, 75); - Rs2GameObject.interact(gameObject); + // Avoid using the alch spell on the obstacle – retry next tick if still selected + if (Microbot.getClient().isWidgetSelected()) + { + return false; + } + Rs2GameObject.interact(gameObject);Optional refactor: if you’d rather clear the selection here instead of bailing, add a helper (e.g.
Rs2Magic.deselect()orRs2Util.clearSelectedWidget()) that clicks the “Cancel” widget before the second interact.
📜 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.
📒 Files selected for processing (5)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/AgilityScript.java(8 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/MicroAgilityConfig.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/AgilityCourseHandler.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/ColossalWyrmAdvancedCourse.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/ColossalWyrmBasicCourse.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/AgilityCourseHandler.java
- runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/ColossalWyrmAdvancedCourse.java
- runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/MicroAgilityConfig.java
- runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/courses/ColossalWyrmBasicCourse.java
🧰 Additional context used
🧬 Code graph analysis (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/AgilityScript.java (3)
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/player/Rs2Player.java (1)
Rs2Player(67-2062)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/magic/Rs2Magic.java (1)
Rs2Magic(43-600)
⏰ 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/AgilityScript.java (4)
41-43: XP state + throttled timeout tracking — LGTMGood addition. lastAgilityXp enables early-action gating, and lastTimeoutWarning prevents log spam. No concerns.
63-64: Initialize lastAgilityXp on start — LGTMCorrect place to seed the XP baseline before the loop begins.
109-113: Early return for course-specific actions — LGTMCentralizing these actions before obstacle logic keeps the main loop clean and predictable.
129-139: XP-gated click decision — LGTMDelegating “can I click now?” to the handler (with current/last XP) is exactly the right abstraction to avoid mid-obstacle spam.
gmason0
left a comment
gmason0
left a comment
There was a problem hiding this comment.
there is still some heavy nesting here, but otherwise this seems to be alot better than before.
We can move forward with this - thank for your being on top of this despite the long review period.
2 participants
This PR introduces an efficient alching mode for the agility plugin along with improved obstacle completion detection to prevent mid-obstacle interactions, and introduces XP Drops as an early signal for obstacle completion.
Features Added:
Bug Fixes:
Technical Improvements:
isObstacleComplete()methodTesting Notes:
Please let me know if you know of any other courses that have obstacles with XP drops before they end
Summary by CodeRabbit
New Features
Refactor