feat(Rs2Cannon): Added pickup and start for cannon fix(Rs2Equipment): doInvoke with option and target#1579
feat(Rs2Cannon): Added pickup and start for cannon fix(Rs2Equipment): doInvoke with option and target#1579Sorries wants to merge 2 commits intochsami:mainfrom
Conversation
Sorries
commented
Oct 20, 2025
Sorries
commented
- Added pickup and start for cannon ( the start should be incorporated within qolCannonScript before refill() )
- Fix the NewMenuEntry when doInvoke as old one would set target as ""
fix(Rs2Equipment): doInvoke with option and target
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR modifies equipment and cannon interaction handling. In Rs2Equipment.java, the NewMenuEntry constructor calls are updated to pass explicit action and target string parameters instead of using positional parameters with item names, and the unEquip logic now uses a capitalized "Remove" string. In Rs2Cannon.java, two new public static methods—pickup() and start()—are added to handle cannon pickup and firing operations with attempt limits and position validation, plus an import for VarPlayerID is added and the repair() method receives an additional sleepUntil call. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/equipment/Rs2Equipment.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/gameobject/Rs2Cannon.java(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/**/*.java: Place all gameplay automation code under runelite-client/src/main/java/net/runelite/client/plugins/microbot
Prefix configuration interfaces with the plugin name (e.g., ExampleConfig) within microbot plugins
Files:
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/gameobject/Rs2Cannon.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/equipment/Rs2Equipment.java
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
Put shared helpers under .../microbot/util
Files:
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/gameobject/Rs2Cannon.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/equipment/Rs2Equipment.java
**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
**/*.java: Use tabs for indentation, follow the brace placement style exemplified in MicrobotPlugin.java, and keep lines under 120 characters
Use UpperCamelCase for types and lowerCamelCase for members
Files:
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/gameobject/Rs2Cannon.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/equipment/Rs2Equipment.java
🧠 Learnings (1)
📚 Learning: 2025-08-25T15:51:39.272Z
Learnt from: runsonmypc
PR: chsami/Microbot#1417
File: runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/AgilityScript.java:13-13
Timestamp: 2025-08-25T15:51:39.272Z
Learning: Both net.runelite.api.ItemID and net.runelite.api.gameval.ItemID are valid import paths in the RuneLite codebase. The microbot plugins consistently use net.runelite.api.ItemID as their established pattern.
Applied to files:
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/gameobject/Rs2Cannon.java
...ite-client/src/main/java/net/runelite/client/plugins/microbot/util/gameobject/Rs2Cannon.java
Show resolved
Hide resolved
| Microbot.pauseAllScripts.compareAndSet(true, false); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
pickup() always returns true even if pickup failed.
If attempts exhaust or not enough slots, the method still returns true. Return success based on the cannon actually disappearing.
- Microbot.pauseAllScripts.compareAndSet(true, false);
- return true;
+ Microbot.pauseAllScripts.compareAndSet(true, false);
+ return !Rs2GameObject.exists(cannon.getId());📝 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.
| Microbot.pauseAllScripts.compareAndSet(true, false); | |
| return true; | |
| } | |
| Microbot.pauseAllScripts.compareAndSet(true, false); | |
| return !Rs2GameObject.exists(cannon.getId()); | |
| } |
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/gameobject/Rs2Cannon.java
around lines 100-102, the pickup() method always returns true even when pickup
fails; change the logic to return true only when the cannon is actually removed
from the world (e.g., wait/poll until the cannon game object is no longer
present or inventory/cannon-slot state confirms pickup) and return false when
attempts are exhausted or there aren’t enough inventory slots; keep the existing
pause/resume calls but use a success flag based on the disappearance check and
ensure timeouts/attempt limits cause a false return.
| WorldArea cannonLocation = new WorldArea( | ||
| cannon.getWorldLocation().getX() - 1, | ||
| cannon.getWorldLocation().getY() - 1, | ||
| 3, 3, | ||
| cannon.getWorldLocation().getPlane() | ||
| ); | ||
| if (!cannonLocation.toWorldPoint().equals(CannonPlugin.getCannonPosition().toWorldPoint())) return false; | ||
| int attempts = 0; |
There was a problem hiding this comment.
Same center-vs-corner mismatch in start().
The position check compares the area’s SW corner, not the center.
WorldArea cannonLocation = new WorldArea(
cannon.getWorldLocation().getX() - 1,
cannon.getWorldLocation().getY() - 1,
3, 3,
cannon.getWorldLocation().getPlane()
);
- if (!cannonLocation.toWorldPoint().equals(CannonPlugin.getCannonPosition().toWorldPoint())) return false;
+ var sw = cannonLocation.toWorldPoint();
+ var center = new net.runelite.api.coords.WorldPoint(sw.getX() + 1, sw.getY() + 1, sw.getPlane());
+ if (!center.equals(CannonPlugin.getCannonPosition().toWorldPoint())) 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.
| WorldArea cannonLocation = new WorldArea( | |
| cannon.getWorldLocation().getX() - 1, | |
| cannon.getWorldLocation().getY() - 1, | |
| 3, 3, | |
| cannon.getWorldLocation().getPlane() | |
| ); | |
| if (!cannonLocation.toWorldPoint().equals(CannonPlugin.getCannonPosition().toWorldPoint())) return false; | |
| int attempts = 0; | |
| WorldArea cannonLocation = new WorldArea( | |
| cannon.getWorldLocation().getX() - 1, | |
| cannon.getWorldLocation().getY() - 1, | |
| 3, 3, | |
| cannon.getWorldLocation().getPlane() | |
| ); | |
| var sw = cannonLocation.toWorldPoint(); | |
| var center = new net.runelite.api.coords.WorldPoint(sw.getX() + 1, sw.getY() + 1, sw.getPlane()); | |
| if (!center.equals(CannonPlugin.getCannonPosition().toWorldPoint())) return false; | |
| int attempts = 0; |
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/gameobject/Rs2Cannon.java
around lines 116 to 123, the area-to-position comparison uses
cannonLocation.toWorldPoint(), which yields the SW corner, so the equality check
is comparing corners instead of centers; change the comparison to use the area's
center (e.g., getCentralWorldPoint() or compute x+1,y+1 for a 3x3 area) and
compare that center WorldPoint to
CannonPlugin.getCannonPosition().toWorldPoint(); update the conditional
accordingly so it returns false only when the centers differ.
| while (Microbot.getClientThread().runOnClientThreadOptional(() -> Microbot.getClient().getVarpValue(VarPlayerID.MCANNONMULTI)).orElse(0) == 0 && attempts < 3){ | ||
| Microbot.log("Starting Cannon"); | ||
| Rs2GameObject.interact(cannon, "Fire"); | ||
| sleepUntil(()-> Microbot.getClientThread().runOnClientThreadOptional(() -> Microbot.getClient().getVarpValue(VarPlayerID.MCANNONMULTI)).orElse(0) == 1048576); | ||
| sleep(250); | ||
| if (Microbot.getClientThread().runOnClientThreadOptional(() -> Microbot.getClient().getVarpValue(VarPlayerID.MCANNONMULTI)).orElse(0) == 1048576){ | ||
| return true; | ||
| } | ||
| attempts++; | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
start() lacks pause/unpause symmetry and early return inside loop complicates it.
Other cannon ops pauseAllScripts; start() should too, without leaking the paused state.
- int attempts = 0;
- while (Microbot.getClientThread().runOnClientThreadOptional(() -> Microbot.getClient().getVarpValue(VarPlayerID.MCANNONMULTI)).orElse(0) == 0 && attempts < 3){
- Microbot.log("Starting Cannon");
- Rs2GameObject.interact(cannon, "Fire");
- sleepUntil(()-> Microbot.getClientThread().runOnClientThreadOptional(() -> Microbot.getClient().getVarpValue(VarPlayerID.MCANNONMULTI)).orElse(0) == 1048576);
- sleep(250);
- if (Microbot.getClientThread().runOnClientThreadOptional(() -> Microbot.getClient().getVarpValue(VarPlayerID.MCANNONMULTI)).orElse(0) == 1048576){
- return true;
- }
- attempts++;
- }
- return false;
+ Microbot.pauseAllScripts.compareAndSet(false, true);
+ boolean started = false;
+ int attempts = 0;
+ while (Microbot.getClientThread().runOnClientThreadOptional(() ->
+ Microbot.getClient().getVarpValue(VarPlayerID.MCANNONMULTI)).orElse(0) == 0 && attempts < 3) {
+ Microbot.log("Starting Cannon");
+ Rs2GameObject.interact(cannon, "Fire");
+ sleepUntil(() -> Microbot.getClientThread().runOnClientThreadOptional(() ->
+ Microbot.getClient().getVarpValue(VarPlayerID.MCANNONMULTI)).orElse(0) == 1048576);
+ sleep(250);
+ started = Microbot.getClientThread().runOnClientThreadOptional(() ->
+ Microbot.getClient().getVarpValue(VarPlayerID.MCANNONMULTI)).orElse(0) == 1048576;
+ if (started) break;
+ attempts++;
+ }
+ Microbot.pauseAllScripts.compareAndSet(true, false);
+ return started;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/gameobject/Rs2Cannon.java
around lines 124 to 135, start() currently pauses scripts asymmetrically and
returns early from inside the loop which can leak paused state; wrap the whole
start logic with a pause/unpause pair that records whether scripts were already
paused, call Microbot.pauseAllScripts() before the loop, then use try { ... }
finally { if we paused them here then Microbot.unpauseAllScripts(); } and remove
the early return inside the loop (use a success flag or break) so unpause always
runs before returning.
|
Does not meet coding standard |
2 participants
