Skip to content

feat(Rs2Cannon): Added pickup and start for cannon fix(Rs2Equipment): doInvoke with option and target #1579

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Sorries wants to merge 2 commits into from
Closed

feat(Rs2Cannon): Added pickup and start for cannon fix(Rs2Equipment): doInvoke with option and target #1579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ef604ef
feat(Rs2Cannon): Added pickup and start for cannon
Oct 20, 2025
aa49ca3
fix(Rs2Cannon): return true when cannon does not exist for pickup()
Oct 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...lient/src/main/java/net/runelite/client/plugins/microbot/util/equipment/Rs2Equipment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ public static boolean isWearing(String[] names, EquipmentInventorySlot[] searchS
}

private static boolean unEquip(Rs2ItemModel item) {
return interact(item, "remove");
return interact(item, "Remove");
}

public static boolean unEquip(Predicate<Rs2ItemModel> predicate) {
Expand Down Expand Up @@ -376,7 +376,7 @@ public static void invokeMenu(Rs2ItemModel rs2Item, String action) {
rectangle = getSafeBounds(InterfaceID.WORNITEMS,24);
}

Microbot.doInvoke(new NewMenuEntry(param0, param1, menuAction.getId(), identifier, -1, rs2Item.getName()), rectangle);
Microbot.doInvoke(new NewMenuEntry(action, param0, param1, menuAction.getId(), identifier, -1, target), rectangle);
//Rs2Reflection.invokeMenu(param0, param1, menuAction.getId(), identifier, rs2Item.id, action, target, -1, -1);
}

Expand Down
63 changes: 63 additions & 0 deletions ...-client/src/main/java/net/runelite/client/plugins/microbot/util/gameobject/Rs2Cannon.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import net.runelite.api.TileObject;
import net.runelite.api.VarPlayer;
import net.runelite.api.coords.WorldArea;
import net.runelite.api.gameval.VarPlayerID;
import net.runelite.client.plugins.cannon.CannonPlugin;
import net.runelite.client.plugins.microbot.Microbot;
import net.runelite.client.plugins.microbot.util.inventory.Rs2Inventory;
Expand Down Expand Up @@ -32,6 +33,7 @@ public static boolean repair() {
Microbot.status = "Repairing Cannon";

Rs2GameObject.interact(brokenCannon, "Repair");
sleepUntil(() -> !Rs2GameObject.exists(brokenCannon.getId()),5000);
return true;
}

Expand Down Expand Up @@ -72,4 +74,65 @@ public static boolean refill(int cannonRefillAmount) {
return true;
}

public static boolean pickup() {

Microbot.status = "Picking up Cannon";

TileObject cannon = Rs2GameObject.findObject(new Integer[]{ObjectID.DWARF_MULTICANNON, ObjectID.DWARF_MULTICANNON_43027});
if (cannon == null) return false;

// Create centered WorldArea (3x3 area with cannon at 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;
Microbot.pauseAllScripts.compareAndSet(false, true);
int attempts = 0;
while (Rs2GameObject.exists(cannon.getId()) && Rs2Inventory.emptySlotCount() >= 4 && attempts < 3){
Rs2GameObject.interact(cannon, "Pick-up");
sleepUntil(() -> !Rs2GameObject.exists(cannon.getId()),5000);
if(!Rs2GameObject.exists(cannon.getId())){
return true;
}
attempts++;
}
Microbot.pauseAllScripts.compareAndSet(true, false);
return false;
}
Comment on lines 102 to 104
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 20, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

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.

Suggested change
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.


public static boolean start() {
if (!Rs2Inventory.hasItemAmount("cannonball", 30, true)) {
//System.out.println("Not enough cannonballs!");
return false;
}

Microbot.status = "Starting up Cannon";

TileObject cannon = Rs2GameObject.findObject(new Integer[]{ObjectID.DWARF_MULTICANNON, ObjectID.DWARF_MULTICANNON_43027});
if (cannon == null) return false;

// Create centered WorldArea (3x3 area with cannon at 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;
int attempts = 0;
Comment on lines +118 to +125
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 20, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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.

Suggested change
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;
}
Comment on lines +126 to +137
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 20, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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.

}