Conversation
…e manual lookup of identifier
…leanup and efficiency improvements
Clicking tag on a tagged npc in instances wouldn't work because they aren't added to npcTags
…bridge, and slayer tower shortcuts
chore(Rs2Equipment): Added equipment slot bounds when doInvoking
Allow interacting with Rs2ItemModel's submenu
[ci skip]
# Conflicts: # cache/pom.xml # pom.xml # runelite-api/pom.xml # runelite-api/src/main/java/net/runelite/api/Perspective.java # runelite-client/pom.xml # runelite-client/src/main/java/net/runelite/client/plugins/grounditems/GroundItem.java # runelite-jshell/pom.xml # runelite-maven-plugin/pom.xml
WalkthroughThis PR introduces WorldView- and camera-focus-aware APIs to runelite-api (new interfaces, methods, and events), extends Perspective with WorldView projections, and updates Actor/WorldEntity contracts. Client and numerous plugins are refactored to operate per-WorldView (ground items/markers, object and NPC indicators, player indicators, interact highlight, tile indicators), and ModelOutlineRenderer/GameEventManager propagate WorldView context. Microbot changes add a settings search UI, dialogue option helpers, camera focus on ActorModel, equipment safe-bounds interaction, inventory submenu action resolution, Rs2ItemModel subops, and walker refactors. Adds agility shortcuts and Surge potion. Removes RuneTracker config/path. Parent and microbot versions are bumped. Tests stub new WorldView lookup. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
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: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
runelite-client/pom.xml (1)
450-476: Blocker: malformed XML and duplicate in shade plugin
- Extraneous characters "§y" on Line 472 break XML.
- Two sibling blocks; schema expects a single containing all entries.
Merge the filters and remove the stray characters:
- <filters> - <filter> - <artifact>*:*</artifact> - <excludes> - <exclude>META-INF/versions/**/module-info.class</exclude> - </excludes> - </filter> - </filters> @@ - <filters> + <filters> <filter> <artifact>*:*</artifact> <excludes> + <exclude>META-INF/versions/**/module-info.class</exclude> <exclude>META-INF/*.SF</exclude> - <exclude>META-INF/*.DSA</exclude>§y + <exclude>META-INF/*.DSA</exclude> <exclude>META-INF/*.RSA</exclude> </excludes> </filter> </filters>runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java (1)
1650-1669: Trapdoor open/closed ID mapping used in reverseOPEN_TO_CLOSED_MAPPINGS is open→closed, but variables are assigned as closed=open and open=closed, inverting the logic and comparisons.
- for (Map.Entry<Integer, Integer> entry : OPEN_TO_CLOSED_MAPPINGS.entrySet()) { - final int closedTrapdoorId = entry.getKey(); - final int openTrapdoorId = entry.getValue(); - - if (transport.getObjectId() == openTrapdoorId) { - if (tileObject.getId() == closedTrapdoorId) { + for (Map.Entry<Integer, Integer> entry : OPEN_TO_CLOSED_MAPPINGS.entrySet()) { + final int openTrapdoorId = entry.getKey(); + final int closedTrapdoorId = entry.getValue(); + + if (transport.getObjectId() == openTrapdoorId) { + if (tileObject.getId() == closedTrapdoorId) { Rs2GameObject.interact(tileObject, "Open"); sleepUntil(() -> Rs2GameObject.exists(openTrapdoorId)); TileObject openTrapdoor = Rs2GameObject.getAll(o -> o.getId() == openTrapdoorId, tileObject.getWorldLocation(), 10).stream().findFirst().orElse(null); if (openTrapdoor != null) { Rs2GameObject.interact(openTrapdoor, transport.getAction()); } - } else if (tileObject.getId() == openTrapdoorId) { + } else if (tileObject.getId() == openTrapdoorId) { Rs2GameObject.interact(tileObject, transport.getAction()); } sleepUntil(() -> !Rs2Player.isAnimating()); sleepUntilTrue(() -> Rs2Player.getWorldLocation().distanceTo(transport.getDestination()) < OFFSET); return true; } }runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/dialogues/Rs2Dialogue.java (1)
516-545: Doc/behavior mismatch: getDialogueText promises null, returns empty stringJavadoc says “returns null if no dialogue is active,” but the implementation returns "". Align one of them.
Apply one of:
- * or the relevant widget is not visible, the method returns {@code null}. + * or the relevant widget is not visible, the method returns an empty string. @@ - public static String getDialogueText() { - if (!isInDialogue()) return ""; + public static String getDialogueText() { + if (!isInDialogue()) return "";Or change code to return null and update downstream checks accordingly.
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2ItemModel.java (2)
500-519: Unnoted-ID logic is inverted for stackable unnoted itemsIf an item is already unnoted, you should return its own ID regardless of stackability. Current condition can return the noted variant for stackable unnoted items.
private static int getUnNotedId(ItemComposition composition) { try { if (composition == null) { log.warn("Could not get item composition for item ID, returning original ID"); return -1; } int itemId = composition.getId(); boolean isNoted = composition.getNote() == 799; int linkedNoteId = composition.getLinkedNoteId(); - // if already stackable, return original ID - if ((!isNoted && !composition.isStackable()) || linkedNoteId == -1) { - return itemId; - } - return linkedNoteId; + // If already unnoted, return original ID + if (!isNoted || linkedNoteId == -1) { + return itemId; + } + // If noted, return unnoted counterpart + return linkedNoteId; } catch (Exception e) { log.error("Error getting unnoted item ID: {}", e.getMessage()); return -1; // fall back to original on error } }
472-491: Fallback for noted-ID helper should not be -1For parity with Javadoc and callers, return original ID when composition is null or link is missing.
private static int getNotedItemId(ItemComposition composition) { try { if (composition == null) { - return -1; + return -1; // keep sentinel here } int itemId = composition.getId(); boolean isNoted = composition.getNote() == 799; int linkedNoteId = composition.getLinkedNoteId(); - // if already stackable, return original ID + // if already noted & stackable, or no noted variant, return original ID if ((isNoted && composition.isStackable()) || linkedNoteId == -1) { return itemId; } return linkedNoteId; } catch (Exception e) { - return -1; // fall back to original on error + return -1; // let callers translate -1 to original id } }And ensure the public static wrappers translate -1 to original ID (see previous comment).
runelite-client/src/main/java/net/runelite/client/plugins/objectindicators/ObjectIndicatorsPlugin.java (1)
578-618: Tile lookup inconsistent with extended scene (likely miss on TOPLEVEL).This uses Scene.getTiles()[level][x][y] with no TOPLEVEL offset. InteractHighlightPlugin switched to getExtendedTiles() and adds the extended-scene offset for TOPLEVEL menu coords. Without matching here, SHIFT‑examine lookups can fail on TOPLEVEL.
Align with extended scene:
private TileObject findTileObject(WorldView wv, int x, int y, int id) { - int level = wv.getPlane(); - Scene scene = wv.getScene(); - Tile[][][] tiles = scene.getTiles(); - final Tile tile = tiles[level][x][y]; + int level = wv.getPlane(); + Scene scene = wv.getScene(); + final int offset = wv.isTopLevel() + ? (net.runelite.api.Constants.EXTENDED_SCENE_SIZE - net.runelite.api.Constants.SCENE_SIZE) / 2 + : 0; + Tile[][][] tiles = scene.getExtendedTiles(); + final Tile tile = tiles[level][x + offset][y + offset];This matches the extended-scene handling used in InteractHighlightPlugin.findTileObject(worldId, x, y, id). Based on Perspective/Constants usage shown in the PR.
runelite-api/src/main/java/net/runelite/api/Perspective.java (1)
1060-1064: Viewport Y offset bug (uses X offset for Y).vpY1 should use getViewportYOffset(); current code culls triangles incorrectly.
Apply this one-line fix:
- int vpY1 = client.getViewportXOffset(); + int vpY1 = client.getViewportYOffset();runelite-client/src/main/java/net/runelite/client/plugins/npchighlight/NpcIndicatorsPlugin.java (1)
592-600: Unify instanced-region check
Replaceif (!client.isInInstancedRegion())at NpcIndicatorsPlugin.java:595 with:WorldView wv = client.findWorldViewFromWorldPoint(npc.getWorldLocation()); if (wv != null && !wv.isInstance())
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (50)
cache/pom.xml(1 hunks)pom.xml(1 hunks)runelite-api/pom.xml(1 hunks)runelite-api/src/main/java/net/runelite/api/Actor.java(1 hunks)runelite-api/src/main/java/net/runelite/api/CameraFocusableEntity.java(1 hunks)runelite-api/src/main/java/net/runelite/api/Client.java(1 hunks)runelite-api/src/main/java/net/runelite/api/GraphicsObject.java(1 hunks)runelite-api/src/main/java/net/runelite/api/Perspective.java(14 hunks)runelite-api/src/main/java/net/runelite/api/WorldEntity.java(1 hunks)runelite-api/src/main/java/net/runelite/api/WorldView.java(3 hunks)runelite-api/src/main/java/net/runelite/api/coords/LocalPoint.java(2 hunks)runelite-api/src/main/java/net/runelite/api/coords/WorldArea.java(3 hunks)runelite-api/src/main/java/net/runelite/api/coords/WorldPoint.java(1 hunks)runelite-api/src/main/java/net/runelite/api/events/WorldEntityDespawned.java(1 hunks)runelite-api/src/main/java/net/runelite/api/events/WorldEntitySpawned.java(1 hunks)runelite-api/src/main/java/net/runelite/api/events/WorldViewLoaded.java(1 hunks)runelite-api/src/main/java/net/runelite/api/events/WorldViewUnloaded.java(1 hunks)runelite-client/pom.xml(2 hunks)runelite-client/src/main/java/net/runelite/client/game/AgilityShortcut.java(6 hunks)runelite-client/src/main/java/net/runelite/client/game/npcoverlay/NpcOverlayService.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/grounditems/GroundItem.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/grounditems/GroundItemsOverlay.java(6 hunks)runelite-client/src/main/java/net/runelite/client/plugins/grounditems/GroundItemsPlugin.java(6 hunks)runelite-client/src/main/java/net/runelite/client/plugins/grounditems/Lootbeam.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/groundmarkers/GroundMarkerMinimapOverlay.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/groundmarkers/GroundMarkerOverlay.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/groundmarkers/GroundMarkerPlugin.java(12 hunks)runelite-client/src/main/java/net/runelite/client/plugins/interacthighlight/InteractHighlightOverlay.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/interacthighlight/InteractHighlightPlugin.java(3 hunks)runelite-client/src/main/java/net/runelite/client/plugins/interfacestyles/InterfaceStylesPlugin.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/itemidentification/ItemIdentification.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/ui/MicrobotConfigPanel.java(6 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/dialogues/Rs2Dialogue.java(8 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/equipment/Rs2Equipment.java(3 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2Inventory.java(3 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2ItemModel.java(21 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java(5 hunks)runelite-client/src/main/java/net/runelite/client/plugins/npchighlight/NpcIndicatorsPlugin.java(10 hunks)runelite-client/src/main/java/net/runelite/client/plugins/objectindicators/ObjectIndicatorsOverlay.java(3 hunks)runelite-client/src/main/java/net/runelite/client/plugins/objectindicators/ObjectIndicatorsPlugin.java(12 hunks)runelite-client/src/main/java/net/runelite/client/plugins/playerindicators/PlayerIndicatorsService.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/tileindicators/TileIndicatorsOverlay.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/xpupdater/XpUpdaterConfig.java(0 hunks)runelite-client/src/main/java/net/runelite/client/plugins/xpupdater/XpUpdaterPlugin.java(3 hunks)runelite-client/src/main/java/net/runelite/client/ui/overlay/outline/ModelOutlineRenderer.java(17 hunks)runelite-client/src/main/java/net/runelite/client/util/GameEventManager.java(3 hunks)runelite-client/src/test/java/net/runelite/client/plugins/loottracker/LootTrackerPluginTest.java(1 hunks)runelite-client/src/test/java/net/runelite/client/plugins/timetracking/farming/CompostTrackerTest.java(1 hunks)runelite-jshell/pom.xml(1 hunks)runelite-maven-plugin/pom.xml(1 hunks)
💤 Files with no reviewable changes (1)
- runelite-client/src/main/java/net/runelite/client/plugins/xpupdater/XpUpdaterConfig.java
🧰 Additional context used
🧠 Learnings (2)
📚 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/inventory/Rs2ItemModel.java
📚 Learning: 2025-08-26T13:22:16.518Z
Learnt from: Voxsylvae
PR: chsami/Microbot#1415
File: runelite-client/src/main/java/net/runelite/client/plugins/microbot/pluginscheduler/tasks/requirements/PrePostScheduleRequirements.java:1215-1221
Timestamp: 2025-08-26T13:22:16.518Z
Learning: The Rs2Equipment.get(slot) method in the Microbot framework returns Rs2ItemModel instances with slot indices that correspond to valid EquipmentInventorySlot values, as items are retrieved by iterating through the enum values themselves. When processing equipment items that were originally obtained via Rs2Equipment.get(slot), the slot indices are guaranteed to be valid for EquipmentInventorySlot.values()[index] access.
Applied to files:
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2ItemModel.java
🧬 Code graph analysis (13)
runelite-api/src/main/java/net/runelite/api/events/WorldViewLoaded.java (3)
runelite-api/src/main/java/net/runelite/api/events/WorldEntityDespawned.java (1)
Value(33-37)runelite-api/src/main/java/net/runelite/api/events/WorldEntitySpawned.java (1)
Value(36-40)runelite-api/src/main/java/net/runelite/api/events/WorldViewUnloaded.java (1)
Value(33-37)
runelite-api/src/main/java/net/runelite/api/events/WorldViewUnloaded.java (3)
runelite-api/src/main/java/net/runelite/api/events/WorldEntityDespawned.java (1)
Value(33-37)runelite-api/src/main/java/net/runelite/api/events/WorldEntitySpawned.java (1)
Value(36-40)runelite-api/src/main/java/net/runelite/api/events/WorldViewLoaded.java (1)
Value(33-37)
runelite-api/src/main/java/net/runelite/api/events/WorldEntityDespawned.java (1)
runelite-api/src/main/java/net/runelite/api/events/WorldEntitySpawned.java (1)
Value(36-40)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/equipment/Rs2Equipment.java (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/menu/NewMenuEntry.java (1)
NewMenuEntry(11-317)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2Inventory.java (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/menu/NewMenuEntry.java (1)
NewMenuEntry(11-317)
runelite-client/src/main/java/net/runelite/client/plugins/groundmarkers/GroundMarkerMinimapOverlay.java (1)
runelite-api/src/main/java/net/runelite/api/Perspective.java (1)
Perspective(62-1159)
runelite-client/src/main/java/net/runelite/client/game/AgilityShortcut.java (1)
runelite-api/src/main/java/net/runelite/api/gameval/DBTableID.java (1)
Quest(8-404)
runelite-client/src/main/java/net/runelite/client/plugins/npchighlight/NpcIndicatorsPlugin.java (1)
runelite-client/src/main/java/net/runelite/client/ui/overlay/outline/ModelOutlineRenderer.java (2)
AllArgsConstructor(60-65)AllArgsConstructor(67-74)
runelite-client/src/main/java/net/runelite/client/plugins/groundmarkers/GroundMarkerOverlay.java (1)
runelite-api/src/main/java/net/runelite/api/Perspective.java (1)
Perspective(62-1159)
runelite-api/src/main/java/net/runelite/api/events/WorldEntitySpawned.java (3)
runelite-api/src/main/java/net/runelite/api/events/WorldEntityDespawned.java (1)
Value(33-37)runelite-api/src/main/java/net/runelite/api/events/WorldViewLoaded.java (1)
Value(33-37)runelite-api/src/main/java/net/runelite/api/events/WorldViewUnloaded.java (1)
Value(33-37)
runelite-client/src/main/java/net/runelite/client/ui/overlay/outline/ModelOutlineRenderer.java (1)
runelite-api/src/main/java/net/runelite/api/Perspective.java (1)
Perspective(62-1159)
runelite-client/src/main/java/net/runelite/client/plugins/grounditems/GroundItemsOverlay.java (1)
runelite-api/src/main/java/net/runelite/api/Perspective.java (1)
Perspective(62-1159)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java (4)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2Inventory.java (1)
Rs2Inventory(46-2313)runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/WorldPointUtil.java (1)
WorldPointUtil(15-274)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/dialogues/Rs2Dialogue.java (1)
Rs2Dialogue(19-756)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/equipment/Rs2Equipment.java (1)
Rs2Equipment(25-392)
⏰ 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). (2)
- GitHub Check: Build
- GitHub Check: Build
🔇 Additional comments (3)
runelite-client/src/main/java/net/runelite/client/plugins/grounditems/Lootbeam.java (1)
105-108: LGTM!The getter correctly delegates to
runeLiteObject.getWorldView()and is appropriately package-private for internal plugin use.runelite-client/src/main/java/net/runelite/client/plugins/itemidentification/ItemIdentification.java (1)
328-328: LGTM!The new
SURGE_POTIONconstant follows the established pattern for potion definitions, with the correct constructor signature (Type.POTION, display name, shorthand, and four dose-variant ItemIDs). The placement within the POTION section is appropriate, and the shorthand "Sur" is unique.runelite-client/src/main/java/net/runelite/client/plugins/grounditems/GroundItem.java (1)
51-53: No action needed: GroundItem.itemLayer is always set via the builder
Builder usages consistently call.itemLayer(...)and there are no directnew GroundItem(...)calls, so the NPE risk is unfounded.Likely an incorrect or invalid review comment.
| /** | ||
| * Get the {@link WorldEntity} this spotanim is on. | ||
| * @return | ||
| */ | ||
| WorldView getWorldView(); |
There was a problem hiding this comment.
Correct the javadoc to reference WorldView instead of WorldEntity.
The javadoc incorrectly references {@link WorldEntity} when the method returns WorldView. Additionally, the @return tag is empty and should describe what is returned.
Apply this diff:
/**
- * Get the {@link WorldEntity} this spotanim is on.
- * @return
+ * Get the {@link WorldView} this spotanim is on.
+ * @return the world view
*/
WorldView getWorldView();📝 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.
| /** | |
| * Get the {@link WorldEntity} this spotanim is on. | |
| * @return | |
| */ | |
| WorldView getWorldView(); | |
| /** | |
| * Get the {@link WorldView} this spotanim is on. | |
| * @return the world view | |
| */ | |
| WorldView getWorldView(); |
🤖 Prompt for AI Agents
In runelite-api/src/main/java/net/runelite/api/GraphicsObject.java around lines
35 to 39, the Javadoc incorrectly references {@link WorldEntity} and has an
empty @return; update the Javadoc to reference {@link WorldView} and provide a
concise description for the @return (e.g., "the WorldView this spotanim is on"),
keeping formatting consistent with surrounding comments.
| for (GroundItem item : groundItemList) | ||
| { | ||
| item.setOffset(offsetMap.compute(item.getLocation(), (k, v) -> v != null ? v + 1 : 0)); | ||
| item.setOffset(offsetMap.compute(item.getItemLayer().getWorldLocation(), (k, v) -> v != null ? v + 1 : 0)); |
There was a problem hiding this comment.
Stacking offset key ignores WorldView (offsets can collide across views).
offsetMap keys by WorldPoint only. WorldPoint lacks WorldView identity, so items in different WorldViews at identical world coords share the same offset, causing inconsistent text stacking.
Recommend keying by (worldViewId, WorldPoint).
- private final Map<WorldPoint, Integer> offsetMap = new HashMap<>();
+ private final Map<java.util.AbstractMap.SimpleEntry<Integer, WorldPoint>, Integer> offsetMap = new HashMap<>();- item.setOffset(offsetMap.compute(item.getItemLayer().getWorldLocation(), (k, v) -> v != null ? v + 1 : 0));
+ var key = new java.util.AbstractMap.SimpleEntry<>(wv.getId(), item.getItemLayer().getWorldLocation());
+ item.setOffset(offsetMap.compute(key, (k, v) -> v != null ? v + 1 : 0));- : offsetMap.compute(item.getItemLayer().getWorldLocation(), (k, v) -> v != null ? v + 1 : 0);
+ : offsetMap.compute(new java.util.AbstractMap.SimpleEntry<>(wv.getId(), item.getItemLayer().getWorldLocation()),
+ (k, v) -> v != null ? v + 1 : 0);Also applies to: 286-286
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/grounditems/GroundItemsOverlay.java
around lines 131 and 286, the offsetMap currently keys by WorldPoint only which
allows collisions across different WorldViews; change the map key to include the
WorldView identity (e.g., use a composite key of worldViewId and WorldPoint or a
small ValueObject/record like (int worldViewId, WorldPoint point)) and update
compute(...) calls to use that composite key when setting and reading offsets so
offsets are isolated per WorldView.
| final WorldView wv = item.getItemLayer().getWorldView(); | ||
| final LocalPoint groundPoint = LocalPoint.fromWorld(wv, item.getItemLayer().getWorldLocation()); | ||
|
|
||
| if (groundPoint == null || localLocation.distanceTo(groundPoint) > MAX_DISTANCE | ||
| if (groundPoint == null | ||
| || (groundPoint.getWorldView() == WorldView.TOPLEVEL && localLocation.distanceTo(groundPoint) > MAX_DISTANCE) | ||
| || !plugin.shouldDisplayItem(ownershipFilterMode, item.getOwnership(), accountType)) |
There was a problem hiding this comment.
Distance filter mixes world views (can compute wrong distances).
You compare player LocalPoint (in their current view) to a TOPLEVEL groundPoint. If the player is in a sub-WorldView, distanceTo is meaningless and may hide/show items incorrectly. Prefer a world-point distance check gated by the item’s WorldView.
Apply this diff:
- if (groundPoint == null
- || (groundPoint.getWorldView() == WorldView.TOPLEVEL && localLocation.distanceTo(groundPoint) > MAX_DISTANCE)
- || !plugin.shouldDisplayItem(ownershipFilterMode, item.getOwnership(), accountType))
+ if (groundPoint == null
+ || !plugin.shouldDisplayItem(ownershipFilterMode, item.getOwnership(), accountType))
{
continue;
}
+ // Only apply distance gating in TOPLEVEL; compare in world tiles to avoid cross-WorldView mismatch
+ if (wv.isTopLevel())
+ {
+ WorldPoint playerWp = player.getWorldLocation();
+ if (playerWp == null || item.getItemLayer().getWorldLocation().distanceTo(playerWp) > 20) // ~20 tiles
+ {
+ continue;
+ }
+ }📝 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.
| final WorldView wv = item.getItemLayer().getWorldView(); | |
| final LocalPoint groundPoint = LocalPoint.fromWorld(wv, item.getItemLayer().getWorldLocation()); | |
| if (groundPoint == null || localLocation.distanceTo(groundPoint) > MAX_DISTANCE | |
| if (groundPoint == null | |
| || (groundPoint.getWorldView() == WorldView.TOPLEVEL && localLocation.distanceTo(groundPoint) > MAX_DISTANCE) | |
| || !plugin.shouldDisplayItem(ownershipFilterMode, item.getOwnership(), accountType)) | |
| final WorldView wv = item.getItemLayer().getWorldView(); | |
| final LocalPoint groundPoint = LocalPoint.fromWorld(wv, item.getItemLayer().getWorldLocation()); | |
| if (groundPoint == null | |
| || !plugin.shouldDisplayItem(ownershipFilterMode, item.getOwnership(), accountType)) | |
| { | |
| continue; | |
| } | |
| // Only apply distance gating in TOPLEVEL; compare in world tiles to avoid cross-WorldView mismatch | |
| if (wv.isTopLevel()) | |
| { | |
| WorldPoint playerWp = player.getWorldLocation(); | |
| if (playerWp == null || item.getItemLayer().getWorldLocation().distanceTo(playerWp) > 20) // ~20 tiles | |
| { | |
| continue; | |
| } | |
| } |
| collectedGroundItems.values().removeIf(g -> g.getItemLayer().getWorldView() == wv); | ||
| lootbeams.values().removeIf(l -> l.getWorldView() == wv.getId()); |
There was a problem hiding this comment.
Inconsistent WorldView comparison approach.
Line 270 compares WorldView objects (getItemLayer().getWorldView() == wv), while Line 271 compares what appears to be WorldView IDs (getWorldView() == wv.getId()). For consistency, both should use the same comparison approach—either compare objects throughout or compare IDs throughout.
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/grounditems/GroundItemsPlugin.java
around lines 270-271, the code inconsistently compares WorldView by object on
line 270 and by ID on line 271; make the comparisons consistent by comparing
WorldView IDs for both removals (e.g., use getItemLayer().getWorldView().getId()
== wv.getId()), and add null-safety where needed (ensure getItemLayer() and
getWorldView() are non-null before calling getId()).
| // auto-expand sections with matches | ||
| if (!anyVisible) continue; | ||
| // ensure expanded when filtering | ||
| if (!noFilter && !contents.isVisible()) contents.setVisible(true); |
There was a problem hiding this comment.
Dead code: condition is always false at this point.
Line 223 checks !contents.isVisible() after line 218 sets contents.setVisible(anyVisible) where anyVisible is guaranteed to be true (line 221 would have continued otherwise). Since the parent was also made visible on line 217, contents.isVisible() will return true, making the condition !contents.isVisible() always false. The body never executes.
- // auto-expand sections with matches
- if (!noFilter && !contents.isVisible()) contents.setVisible(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.
| if (!noFilter && !contents.isVisible()) contents.setVisible(true); |
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/microbot/ui/MicrobotConfigPanel.java
around line 223, the condition `if (!noFilter && !contents.isVisible())
contents.setVisible(true);` is dead because `contents.setVisible(anyVisible)`
just set it visible when anyVisible is true; remove this redundant if-statement
(or, if the intent was to set visibility before the parent change, move the
check earlier) so you either delete the line or adjust the ordering so the
visibility check is meaningful.
| public static int getNotedId(int itemId) { | ||
| ItemComposition composition = Microbot.getClientThread().runOnClientThreadOptional(() -> | ||
| Microbot.getClient().getItemDefinition(itemId) | ||
| ).orElse(null); | ||
|
|
||
| return getNotedItemId(composition); | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Static noted/unnoted helpers violate their own contract
Javadoc says “otherwise the original item ID,” but both return -1 when composition is null or on error. Return the original ID as fallback.
public static int getNotedId(int itemId) {
ItemComposition composition = Microbot.getClientThread().runOnClientThreadOptional(() ->
Microbot.getClient().getItemDefinition(itemId)
).orElse(null);
-
- return getNotedItemId(composition);
+ int res = getNotedItemId(composition);
+ return res == -1 ? itemId : res;
}
@@
public static int getUnNotedId(int itemId) {
ItemComposition composition = Microbot.getClientThread().runOnClientThreadOptional(() ->
Microbot.getClient().getItemDefinition(itemId)
).orElse(null);
-
- return getUnNotedId(composition);
+ int res = getUnNotedId(composition);
+ return res == -1 ? itemId : res;
}Also applies to: 457-464
| public String getAction(String partOfAction) { | ||
| return Arrays.stream(getInventoryActions()) | ||
| .filter(Objects::nonNull) | ||
| .filter(x -> x.toLowerCase().contains(partOfAction)) | ||
| .findFirst().orElse(null); | ||
| } |
There was a problem hiding this comment.
Case-insensitive search isn’t symmetric
Javadoc promises case-insensitive, but only action is lowercased; partOfAction isn’t. Lower both.
public String getAction(String partOfAction) {
- return Arrays.stream(getInventoryActions())
- .filter(Objects::nonNull)
- .filter(x -> x.toLowerCase().contains(partOfAction))
+ final String needle = partOfAction == null ? "" : partOfAction.toLowerCase();
+ return Arrays.stream(getInventoryActions())
+ .filter(Objects::nonNull)
+ .filter(x -> x.toLowerCase().contains(needle))
.findFirst().orElse(null);
}📝 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 String getAction(String partOfAction) { | |
| return Arrays.stream(getInventoryActions()) | |
| .filter(Objects::nonNull) | |
| .filter(x -> x.toLowerCase().contains(partOfAction)) | |
| .findFirst().orElse(null); | |
| } | |
| public String getAction(String partOfAction) { | |
| final String needle = partOfAction == null ? "" : partOfAction.toLowerCase(); | |
| return Arrays.stream(getInventoryActions()) | |
| .filter(Objects::nonNull) | |
| .filter(x -> x.toLowerCase().contains(needle)) | |
| .findFirst().orElse(null); | |
| } |
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2ItemModel.java
around lines 579 to 584, the case-insensitive search lowercases only the
inventory action but not the input partOfAction causing asymmetric matching;
update the code to lowercase partOfAction (and guard against null) before the
stream filter so both sides are compared in the same case (e.g., compute String
needle = partOfAction == null ? "" : partOfAction.toLowerCase() and use
x.toLowerCase().contains(needle)).
| public String getActionFromList(List<String> actions) { | ||
| return Arrays.stream(getInventoryActions()) | ||
| .filter(action -> action != null && actions.stream().anyMatch(keyword -> action.toLowerCase().contains(keyword.toLowerCase()))) | ||
| .min(Comparator.comparingInt(action -> | ||
| actions.stream() | ||
| .filter(keyword -> action.toLowerCase().contains(keyword.toLowerCase())) | ||
| .mapToInt(actions::indexOf) | ||
| .findFirst() | ||
| .orElse(Integer.MAX_VALUE) | ||
| )).orElse(null); | ||
| } | ||
| } |
There was a problem hiding this comment.
getActionFromList claims to ignore nulls but doesn’t
Null keywords in actions will NPE. Filter nulls up front.
public String getActionFromList(List<String> actions) {
- return Arrays.stream(getInventoryActions())
- .filter(action -> action != null && actions.stream().anyMatch(keyword -> action.toLowerCase().contains(keyword.toLowerCase())))
+ final List<String> needles = actions == null ? List.of() :
+ actions.stream().filter(Objects::nonNull).collect(Collectors.toList());
+ return Arrays.stream(getInventoryActions())
+ .filter(action -> action != null && needles.stream().anyMatch(keyword -> action.toLowerCase().contains(keyword.toLowerCase())))
.min(Comparator.comparingInt(action ->
- actions.stream()
- .filter(keyword -> action.toLowerCase().contains(keyword.toLowerCase()))
- .mapToInt(actions::indexOf)
+ needles.stream()
+ .filter(keyword -> action.toLowerCase().contains(keyword.toLowerCase()))
+ .mapToInt(needles::indexOf)
.findFirst()
.orElse(Integer.MAX_VALUE)
)).orElse(null);
}📝 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 String getActionFromList(List<String> actions) { | |
| return Arrays.stream(getInventoryActions()) | |
| .filter(action -> action != null && actions.stream().anyMatch(keyword -> action.toLowerCase().contains(keyword.toLowerCase()))) | |
| .min(Comparator.comparingInt(action -> | |
| actions.stream() | |
| .filter(keyword -> action.toLowerCase().contains(keyword.toLowerCase())) | |
| .mapToInt(actions::indexOf) | |
| .findFirst() | |
| .orElse(Integer.MAX_VALUE) | |
| )).orElse(null); | |
| } | |
| } | |
| public String getActionFromList(List<String> actions) { | |
| // Filter out null keywords (and handle a null actions list) | |
| final List<String> needles = actions == null | |
| ? List.of() | |
| : actions.stream() | |
| .filter(Objects::nonNull) | |
| .collect(Collectors.toList()); | |
| return Arrays.stream(getInventoryActions()) | |
| .filter(action -> | |
| action != null && | |
| needles.stream() | |
| .anyMatch(keyword -> action.toLowerCase().contains(keyword.toLowerCase())) | |
| ) | |
| .min(Comparator.comparingInt(action -> | |
| needles.stream() | |
| .filter(keyword -> action.toLowerCase().contains(keyword.toLowerCase())) | |
| .mapToInt(needles::indexOf) | |
| .findFirst() | |
| .orElse(Integer.MAX_VALUE) | |
| )) | |
| .orElse(null); | |
| } |
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2ItemModel.java
around lines 594 to 605, the stream logic assumes keywords in the passed-in
actions List are non-null and will NPE when they are; update the predicate and
comparator to filter out null keywords before calling toLowerCase by adding
keyword != null checks (e.g., change anyMatch to
actions.stream().filter(Objects::nonNull).anyMatch(...) and similarly filter
keywords in the comparator's inner stream) so null keywords are ignored and
won’t cause a NullPointerException.
| if (Rs2Pvp.isInWilderness() && (Rs2Pvp.getWildernessLevelFrom(Rs2Player.getWorldLocation()) > (transport.getMaxWildernessLevel() + 1))) | ||
| return false; |
There was a problem hiding this comment.
🧩 Analysis chain
Off-by-one in wilderness teleport restriction check
Using > (maxWildernessLevel + 1) permits teleports at exactly max+1. Should block when current level exceeds max, not max+1.
Please align similar checks elsewhere (e.g., spells) for consistency.
- if (Rs2Pvp.isInWilderness() && (Rs2Pvp.getWildernessLevelFrom(Rs2Player.getWorldLocation()) > (transport.getMaxWildernessLevel() + 1)))
+ if (Rs2Pvp.isInWilderness() && Rs2Pvp.getWildernessLevelFrom(Rs2Player.getWorldLocation()) > transport.getMaxWildernessLevel())
return false;🏁 Script executed:
#!/bin/bash
rg -n --type java "getMaxWildernessLevel" -C3
rg -n --type java "getWildernessLevelFrom" -C3Length of output: 10601
Fix off-by-one in wilderness teleport restriction
In both handleTeleportSpell (line 1812) and handleTeleportItem (line 1835), the check
if (isInWilderness() && getWildernessLevelFrom(...) > (transport.getMaxWildernessLevel() + 1))allows teleports at exactly max+1. It should block any level above max:
- … > (transport.getMaxWildernessLevel() + 1)
+ … > transport.getMaxWildernessLevel()🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java
around lines 1835-1836, the wilderness teleport restriction uses a comparison
that allows teleports at exactly max+1; change the condition so it blocks any
wilderness level greater than the transport's max (i.e., replace the current >
(transport.getMaxWildernessLevel() + 1) check with a comparison against
transport.getMaxWildernessLevel() so teleports are disallowed when wilderness
level > transport.getMaxWildernessLevel()).
| private final List<DespawnedNpc> despawnedNpcsThisTick = new ArrayList<>(); | ||
|
|
||
| @AllArgsConstructor | ||
| private static class DespawnedNpc | ||
| { | ||
| WorldPoint coord; | ||
| int index; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
WorldView-aware tagging/memorization still keyed by NPC index only (collisions across views).
Index-only keys (npcTags.contains(npc.getIndex()), memorizedNpcs.get(npc.getIndex())) can collide between worldviews. Result: wrong NPCs get (un)tagged or respawn timers mis-assigned when multiple worldviews are active. Despawn records also lack worldViewId.
Scope keys by worldviewId+index and carry worldviewId through DespawnedNpc.
Apply targeted diffs to changed hunks:
@@
- private final List<DespawnedNpc> despawnedNpcsThisTick = new ArrayList<>();
+ private final List<DespawnedNpc> despawnedNpcsThisTick = new ArrayList<>();
@@
- @AllArgsConstructor
- private static class DespawnedNpc
+ @AllArgsConstructor
+ private static class DespawnedNpc
{
- WorldPoint coord;
- int index;
+ WorldPoint coord;
+ int worldViewId;
+ int index;
}
@@
- final boolean idMatch = npcTags.contains(npc.getIndex());
+ final boolean idMatch = npcTags.contains(key(menuEntry.getWorldViewId(), npc.getIndex()));
@@
- if (npcTags.contains(npc.getIndex()))
+ if (npcTags.contains(key(wv.getId(), npc.getIndex())))
{
memorizeNpc(npc);
highlightedNpcs.put(npc, highlightedNpc(npc));
spawnedNpcsThisTick.add(npc);
return;
}
@@
- if (memorizedNpcs.containsKey(npc.getIndex()))
+ if (memorizedNpcs.containsKey(key(client.findWorldViewFromWorldPoint(npc.getWorldLocation()).getId(), npc.getIndex())))
{
- despawnedNpcsThisTick.add(new DespawnedNpc(npc.getWorldLocation(), npc.getIndex()));
+ WorldView wv = client.findWorldViewFromWorldPoint(npc.getWorldLocation());
+ despawnedNpcsThisTick.add(new DespawnedNpc(npc.getWorldLocation(), wv != null ? wv.getId() : -1, npc.getIndex()));
}
@@
- for (DespawnedNpc npc : despawnedNpcsThisTick)
+ for (DespawnedNpc npc : despawnedNpcsThisTick)
{
...
- final MemorizedNpc mn = memorizedNpcs.get(npc.index);
+ final MemorizedNpc mn = memorizedNpcs.get(key(npc.worldViewId, npc.index));
...
}Add helpers and update key types (outside these hunks):
// Helper
private static long key(int worldViewId, int index) {
return ((long) worldViewId << 32) | (index & 0xFFFFFFFFL);
}
// Change types:
private final Set<Long> npcTags = new HashSet<>();
private final Map<Long, MemorizedNpc> memorizedNpcs = new HashMap<>();
@Getter(AccessLevel.PACKAGE)
private final Map<Long, MemorizedNpc> deadNpcsToDisplay = new HashMap<>();
// Update all .add/.remove/.contains/.put/.get calls to use key(wvId, index)Note: Use the actual API you have to resolve a WorldView for an NPC (e.g., via world point). If method names differ, adapt accordingly.
Also applies to: 312-314, 756-769, 610-611, 803-817
🏁 Script executed:
#!/bin/bash
set -e
FILE="runelite-client/src/main/java/net/runelite/client/plugins/npchighlight/NpcIndicatorsPlugin.java"
echo "=== DespawnedNpc class definition ==="
rg -n -C2 "class DespawnedNpc" "$FILE"
echo "=== npcTags.contains usages ==="
rg -n -C2 "npcTags\.contains" "$FILE"
echo "=== memorizedNpcs.get/.contains usages ==="
rg -n -C2 "memorizedNpcs\.(get|contains)" "$FILE"
echo "=== worldViewId references ==="
rg -n "worldViewId" "$FILE"Length of output: 1514
The review comment is correct: keys lack worldview scoping, risking collisions across worldviews.
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/npchighlight/NpcIndicatorsPlugin.java
around lines 189-196, the DespawnedNpc entries do not include worldview scoping
which can cause collisions across different worldviews; add a worldview
identifier field (e.g. int world or UUID worldviewId) to the DespawnedNpc class,
populate it when creating/despawning NPC entries using the current worldview
value from the client/context, and update any places that compare or look up
DespawnedNpc instances to include the worldview in equality/identity checks so
entries are only considered equal within the same worldview.
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/equipment/Rs2Equipment.java (1)
314-341: Inconsistent usage of thetargetvariable.The
targetvariable is initialized tors2Item.getName()on line 314, then conditionally set to an empty string on line 339 for sub-menu actions. However, line 379 always passesrs2Item.getName()directly to theNewMenuEntryconstructor instead of using thetargetvariable.Apply this diff to use the
targetvariable consistently:- Microbot.doInvoke(new NewMenuEntry(param0, param1, menuAction.getId(), identifier, -1, rs2Item.getName()), rectangle); + Microbot.doInvoke(new NewMenuEntry(param0, param1, menuAction.getId(), identifier, -1, target), rectangle);Also applies to: 379-379
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2Inventory.java (1)
1921-1921: Three logic issues in invokeMenu: NPE risk, default action not resolved, and fragile sub-action main index lookup
- Line 1921: Optional.orElseGet(null) will NPE. Use orElse(null).
- Lines 1920-1936 (inside the action handling): When action is empty, no fallback is chosen. This contradicts interact(int id)’s javadoc (“first available action”) and yields identifier=-1 with option="". Resolve the first non-null inventory action and set identifier accordingly.
- Lines 1938-1941: mainMenuIndex is derived with Arrays.asList(actions).indexOf(subActionMap.getKey()) without stripping color tags or case-insensitive matching. If not found, mainMenuIndex becomes -1, leading to an invalid identifier. Normalize actions before matching, and hard-fail if unresolved.
Apply this diff:
- var itemWidget = Arrays.stream(inventoryWidgets).filter(x -> x != null && x.getIndex() == rs2Item.getSlot()).findFirst().orElseGet(null); + var itemWidget = Arrays.stream(inventoryWidgets) + .filter(x -> x != null && x.getIndex() == rs2Item.getSlot()) + .findFirst() + .orElse(null); @@ - String[] actions = itemWidget != null && itemWidget.getActions() != null ? - itemWidget.getActions() : - rs2Item.getInventoryActions(); + String[] actions = (itemWidget != null && itemWidget.getActions() != null) + ? itemWidget.getActions() + : rs2Item.getInventoryActions(); + String[] actionsStripped = stripColTags(actions); @@ - int simpleIndex = indexOfIgnoreCase(stripColTags(actions), action); - if (simpleIndex != -1) { - identifier = simpleIndex + 1; - } else { + if (action.isEmpty()) { + // Resolve first available action as default + for (int i = 0; i < actionsStripped.length; i++) { + if (actionsStripped[i] != null && !actionsStripped[i].isEmpty()) { + action = actions[i]; + identifier = i + 1; + break; + } + } + if (identifier == -1) { + Microbot.log("No available actions for item=" + rs2Item.getName(), Level.ERROR); + return; + } + } else { + int simpleIndex = indexOfIgnoreCase(actionsStripped, action); + if (simpleIndex != -1) { + identifier = simpleIndex + 1; + } else { // We could not find the action in the item widget's actions, so we try to find it in the sub-menu actions Map.Entry<String, Integer> subActionMap = rs2Item.getIndexOfSubAction(action); if (subActionMap == null) { Microbot.log("Item=" + rs2Item.getName() + " does not have action=" + action, Level.ERROR); return; } - // The main menu index depends on the inventory interface from which this item is interacted with - int mainMenuIndex = java.util.Arrays.asList(actions).indexOf(subActionMap.getKey()); + // Resolve main menu index robustly (strip color tags, ignore case) + int mainMenuIndex = -1; + for (int i = 0; i < actionsStripped.length; i++) { + if (actionsStripped[i] != null && actionsStripped[i].equalsIgnoreCase(subActionMap.getKey())) { + mainMenuIndex = i; + break; + } + } + if (mainMenuIndex == -1) { + Microbot.log("Could not resolve main menu index for sub-action '" + subActionMap.getKey() + "' on item=" + rs2Item.getName(), Level.ERROR); + return; + } identifier = NewMenuEntry.findIdentifier(subActionMap.getValue() + 1, mainMenuIndex + 1); target = ""; + } }Also applies to: 1927-1941, 1878-1887
runelite-api/src/main/java/net/runelite/api/coords/WorldArea.java (1)
420-439: Fix inverted extraCondition logic in diagonal movement checksDiagonal checks should mirror the x/y branches: block when there is a collision OR when extraCondition fails. Current code returns false only if both collision and extraCondition are true, letting movement pass when extraCondition fails. Align with earlier checks.
- if (width == 1) - { - if ((collisionDataFlags[checkX][checkY - dy] & xFlags) != 0 && - extraCondition.test(WorldPoint.fromScene(wv, checkX, startY, plane))) - { - return false; - } - } + if (width == 1) + { + if ((collisionDataFlags[checkX][checkY - dy] & xFlags) != 0 || + !extraCondition.test(WorldPoint.fromScene(wv, checkX, startY, plane))) + { + return false; + } + } if (height == 1) { - if ((collisionDataFlags[checkX - dx][checkY] & yFlags) != 0 && - extraCondition.test(WorldPoint.fromScene(wv, startX, checkY, plane))) + if ((collisionDataFlags[checkX - dx][checkY] & yFlags) != 0 || + !extraCondition.test(WorldPoint.fromScene(wv, startX, checkY, plane))) { return false; } }runelite-client/src/main/java/net/runelite/client/plugins/grounditems/GroundItemsPlugin.java (1)
194-201: WorldView collisions: keys are WorldPoint-only (items and lootbeams), causing cross-view conflation.You’ve made handling WorldView-aware, but both:
- collectedGroundItems is keyed by WorldPoint only, and
- lootbeams map key is WorldPoint only.
When identical WorldPoints exist in multiple WorldViews, items/lootbeams collide: lookups return the wrong GroundItem, and only one lootbeam survives per WorldPoint. The unload filter cannot prevent conflicts while multiple views are loaded.
Refactor to include WorldView identity in the key.
Apply targeted changes within the touched areas:
- private static final Table<WorldPoint, Integer, GroundItem> collectedGroundItems = Tables.synchronizedTable(HashBasedTable.create()); + private static final Table<WorldKey, Integer, GroundItem> collectedGroundItems = Tables.synchronizedTable(HashBasedTable.create());@@ public void onItemSpawned(ItemSpawned itemSpawned) - ItemLayer layer = tile.getItemLayer(); + ItemLayer layer = tile.getItemLayer(); @@ - GroundItem groundItem = buildGroundItem(layer, item); - GroundItem existing = collectedGroundItems.get(tile.getWorldLocation(), item.getId()); + GroundItem groundItem = buildGroundItem(layer, item); + var key = new WorldKey(layer.getWorldView().getId(), tile.getWorldLocation()); + GroundItem existing = collectedGroundItems.get(key, item.getId()); @@ - collectedGroundItems.put(tile.getWorldLocation(), item.getId(), groundItem); + collectedGroundItems.put(key, item.getId(), groundItem);@@ public void onMenuEntryAdded(MenuEntryAdded event) - final WorldPoint worldPoint = WorldPoint.fromScene(wv, sceneX, sceneY, wv.getPlane()); - GroundItem groundItem = collectedGroundItems.get(worldPoint, itemId); + final WorldPoint worldPoint = WorldPoint.fromScene(wv, sceneX, sceneY, wv.getPlane()); + var key = new WorldKey(wv.getId(), worldPoint); + GroundItem groundItem = collectedGroundItems.get(key, itemId);- private final Map<WorldPoint, Lootbeam> lootbeams = new HashMap<>(); + private final Map<WorldKey, Lootbeam> lootbeams = new HashMap<>();@@ private void handleLootbeam(WorldPoint worldPoint) - removeLootbeam(worldPoint); + // supply the view id along with the point when calling this method + removeLootbeam(key);@@ private void addLootbeam(WorldPoint worldPoint, Color color) - Lootbeam lootbeam = lootbeams.get(worldPoint); + Lootbeam lootbeam = lootbeams.get(key); @@ - lootbeam = new Lootbeam(client, clientThread, worldPoint, color, config.lootbeamStyle()); - lootbeams.put(worldPoint, lootbeam); + lootbeam = new Lootbeam(client, clientThread, worldPoint, color, config.lootbeamStyle()); + lootbeams.put(key, lootbeam); @@ - lootbeam.setColor(color); + lootbeam.setColor(color);@@ private void removeLootbeam(WorldPoint worldPoint) - Lootbeam lootbeam = lootbeams.remove(worldPoint); + Lootbeam lootbeam = lootbeams.remove(key);Add a lightweight key type (outside the shown ranges):
// Helper key to disambiguate the same WorldPoint across multiple WorldViews private static final class WorldKey { final int worldViewId; final WorldPoint point; WorldKey(int worldViewId, WorldPoint point) { this.worldViewId = worldViewId; this.point = point; } @Override public boolean equals(Object o) { if (this == o) return true; if (!(o instanceof WorldKey)) return false; WorldKey k = (WorldKey) o; return worldViewId == k.worldViewId && point.equals(k.point); } @Override public int hashCode() { return 31 * worldViewId + point.hashCode(); } }Also update other call sites (despawn/quantity change/handleLootbeam callers) to construct and pass WorldKey consistently (use the item’s ItemLayer.getWorldView().getId() or the event/menu-derived WorldView).
Also applies to: 266-272, 514-519, 757-824, 843-866
runelite-client/src/main/java/net/runelite/client/plugins/npchighlight/NpcIndicatorsPlugin.java (3)
533-557: Tag/Un-tag toggles against “highlighted” state, not id-tag state (wrong behavior when highlight comes from name/wildcard).Clicking “Tag” on an NPC already highlighted via name/wildcard can untag/no-op instead of adding an id tag. Toggle should be based on npcTags.contains(id), not highlightedNpcs.containsKey(npc).
Apply this fix:
if (entry.getOption().equals(TAG) || entry.getOption().equals(UNTAG)) { - final boolean exists = highlightedNpcs.containsKey(npc); + final boolean idTagged = npcTags.contains(id); - if (exists) + if (idTagged) { npcTags.remove(id); if (!highlightMatchesNPCName(npc.getName())) { highlightedNpcs.remove(npc); memorizedNpcs.remove(npc.getIndex()); } } else { - if (!wv.isInstance()) - { - memorizeNpc(npc); - npcTags.add(id); - } + if (!wv.isInstance()) + { + memorizeNpc(npc); + npcTags.add(id); + } highlightedNpcs.put(npc, highlightedNpc(npc)); } npcOverlayService.rebuild(); }
803-829: Respawn validation ignores WorldView membership.Despawn checks compare NPC coord vs player location without ensuring the coord is in the player’s WorldView, potentially starting timers for NPCs in other views.
Gate processing by player WV containment:
- for (DespawnedNpc npc : despawnedNpcsThisTick) + for (DespawnedNpc npc : despawnedNpcsThisTick) { + // Only consider despawns in the player's current world view + if (!client.getTopLevelWorldView().contains(npc.coord)) + { + continue; + } if (!teleportGraphicsObjectSpawnedThisTick.isEmpty()) { if (teleportGraphicsObjectSpawnedThisTick.contains(npc.coord)) { // NPC teleported away, so we don't want to add the respawn timer continue; } } if (isInViewRange(client.getLocalPlayer().getWorldLocation(), npc.coord)) { final MemorizedNpc mn = memorizedNpcs.get(npc.index);If contains(WorldPoint) isn’t available, add an equivalent WV check here.
592-600: Align instance checks between spawn and rebuild paths
Spawn handler in NpcIndicatorsPlugin (lines 595–600) usesclient.isInInstancedRegion(), whilerebuildWorldviewfilters viaWorldView.isInstance(), leading to instanced child views being processed incorrectly. Replace- if (!client.isInInstancedRegion()) + if (!client.getTopLevelWorldView().isInstance())or, if available, use each NPC’s own
getWorldView().isInstance().runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java (1)
3344-3355: Bank withdrawal ignores current inventory count (over-withdraws)amountToWithdraw uses amountNeeded, but the subsequent wait condition expects a delta (currentCount + amountToWithdraw). This over-withdraws and mismatches the wait.
Fix to withdraw the missing amount only:
- int currentCount = Rs2Inventory.count(itemId); - int amountToWithdraw = Math.max(0, amountNeeded ); + int currentCount = Rs2Inventory.count(itemId); + int amountToWithdraw = Math.max(0, amountNeeded - currentCount);
🧹 Nitpick comments (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java (1)
1918-1939: Wearable teleports only handled when displayInfo contains “:”; single-destination wearables are ignoredCurrent logic returns false for wearables that don’t require a destination (e.g., single “Teleport”/“Operate” actions). This skips valid teleports.
Extend the method to handle single-destination/operate wearables:
- private static boolean handleWearableTeleports(Transport transport, int itemId) { + private static boolean handleWearableTeleports(Transport transport, int itemId) { Rs2ItemModel rs2Item = Rs2Equipment.get(itemId); if (rs2Item == null) return false; - if (transport.getDisplayInfo().contains(":")) { + if (transport.getDisplayInfo().contains(":")) { String[] values = transport.getDisplayInfo().split(":"); String destination = values[1].trim().toLowerCase(); if (transport.getDisplayInfo().toLowerCase().contains("slayer ring")) { Rs2Equipment.invokeMenu(rs2Item, "teleport"); Rs2Dialogue.sleepUntilSelectAnOption(); Rs2Dialogue.clickOption(destination); } else { Rs2Equipment.invokeMenu(rs2Item, destination); if (transport.getDisplayInfo().toLowerCase().contains("burning amulet")) { Rs2Dialogue.sleepUntilInDialogue(); Rs2Dialogue.clickOption("Okay, teleport to level"); } } log.info("Traveling to {} - ({})", transport.getDisplayInfo(), transport.getDestination()); return true; - } - return false; + } else { + // Handle single-destination/operate actions on worn items + String action = Optional.ofNullable(rs2Item.getAction("teleport")) + .orElse(rs2Item.getActionFromList(Arrays.asList("operate", "invoke", "teleport"))); + if (action != null && Rs2Equipment.invokeMenu(rs2Item, action)) { + // Confirm wilderness warning if applicable + boolean wilderness = PathfinderConfig.isInWilderness(WorldPointUtil.packWorldPoint(transport.getDestination())); + if (wilderness) { + Rs2Dialogue.sleepUntilInDialogue(); + return Rs2Dialogue.clickOption("Yes", "Okay"); + } + log.info("Traveling to {} - ({})", transport.getDisplayInfo(), transport.getDestination()); + return true; + } + } + return false; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (51)
cache/pom.xml(1 hunks)pom.xml(1 hunks)runelite-api/pom.xml(1 hunks)runelite-api/src/main/java/net/runelite/api/Actor.java(1 hunks)runelite-api/src/main/java/net/runelite/api/CameraFocusableEntity.java(1 hunks)runelite-api/src/main/java/net/runelite/api/Client.java(1 hunks)runelite-api/src/main/java/net/runelite/api/GraphicsObject.java(1 hunks)runelite-api/src/main/java/net/runelite/api/Perspective.java(14 hunks)runelite-api/src/main/java/net/runelite/api/WorldEntity.java(1 hunks)runelite-api/src/main/java/net/runelite/api/WorldView.java(3 hunks)runelite-api/src/main/java/net/runelite/api/coords/LocalPoint.java(2 hunks)runelite-api/src/main/java/net/runelite/api/coords/WorldArea.java(3 hunks)runelite-api/src/main/java/net/runelite/api/coords/WorldPoint.java(1 hunks)runelite-api/src/main/java/net/runelite/api/events/WorldEntityDespawned.java(1 hunks)runelite-api/src/main/java/net/runelite/api/events/WorldEntitySpawned.java(1 hunks)runelite-api/src/main/java/net/runelite/api/events/WorldViewLoaded.java(1 hunks)runelite-api/src/main/java/net/runelite/api/events/WorldViewUnloaded.java(1 hunks)runelite-client/pom.xml(2 hunks)runelite-client/src/main/java/net/runelite/client/game/AgilityShortcut.java(6 hunks)runelite-client/src/main/java/net/runelite/client/game/npcoverlay/NpcOverlayService.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/grounditems/GroundItem.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/grounditems/GroundItemsOverlay.java(6 hunks)runelite-client/src/main/java/net/runelite/client/plugins/grounditems/GroundItemsPlugin.java(6 hunks)runelite-client/src/main/java/net/runelite/client/plugins/grounditems/Lootbeam.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/groundmarkers/GroundMarkerMinimapOverlay.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/groundmarkers/GroundMarkerOverlay.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/groundmarkers/GroundMarkerPlugin.java(12 hunks)runelite-client/src/main/java/net/runelite/client/plugins/interacthighlight/InteractHighlightOverlay.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/interacthighlight/InteractHighlightPlugin.java(3 hunks)runelite-client/src/main/java/net/runelite/client/plugins/interfacestyles/InterfaceStylesPlugin.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/itemidentification/ItemIdentification.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/ui/MicrobotConfigPanel.java(6 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/ActorModel.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/dialogues/Rs2Dialogue.java(8 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/equipment/Rs2Equipment.java(3 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2Inventory.java(3 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2ItemModel.java(21 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java(5 hunks)runelite-client/src/main/java/net/runelite/client/plugins/npchighlight/NpcIndicatorsPlugin.java(10 hunks)runelite-client/src/main/java/net/runelite/client/plugins/objectindicators/ObjectIndicatorsOverlay.java(3 hunks)runelite-client/src/main/java/net/runelite/client/plugins/objectindicators/ObjectIndicatorsPlugin.java(12 hunks)runelite-client/src/main/java/net/runelite/client/plugins/playerindicators/PlayerIndicatorsService.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/tileindicators/TileIndicatorsOverlay.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/xpupdater/XpUpdaterConfig.java(0 hunks)runelite-client/src/main/java/net/runelite/client/plugins/xpupdater/XpUpdaterPlugin.java(3 hunks)runelite-client/src/main/java/net/runelite/client/ui/overlay/outline/ModelOutlineRenderer.java(17 hunks)runelite-client/src/main/java/net/runelite/client/util/GameEventManager.java(3 hunks)runelite-client/src/test/java/net/runelite/client/plugins/loottracker/LootTrackerPluginTest.java(1 hunks)runelite-client/src/test/java/net/runelite/client/plugins/timetracking/farming/CompostTrackerTest.java(1 hunks)runelite-jshell/pom.xml(1 hunks)runelite-maven-plugin/pom.xml(1 hunks)
💤 Files with no reviewable changes (1)
- runelite-client/src/main/java/net/runelite/client/plugins/xpupdater/XpUpdaterConfig.java
🧰 Additional context used
🧠 Learnings (2)
📚 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/inventory/Rs2ItemModel.java
📚 Learning: 2025-08-26T13:22:16.518Z
Learnt from: Voxsylvae
PR: chsami/Microbot#1415
File: runelite-client/src/main/java/net/runelite/client/plugins/microbot/pluginscheduler/tasks/requirements/PrePostScheduleRequirements.java:1215-1221
Timestamp: 2025-08-26T13:22:16.518Z
Learning: The Rs2Equipment.get(slot) method in the Microbot framework returns Rs2ItemModel instances with slot indices that correspond to valid EquipmentInventorySlot values, as items are retrieved by iterating through the enum values themselves. When processing equipment items that were originally obtained via Rs2Equipment.get(slot), the slot indices are guaranteed to be valid for EquipmentInventorySlot.values()[index] access.
Applied to files:
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2ItemModel.java
🧬 Code graph analysis (13)
runelite-api/src/main/java/net/runelite/api/events/WorldEntityDespawned.java (3)
runelite-api/src/main/java/net/runelite/api/events/WorldEntitySpawned.java (1)
Value(36-40)runelite-api/src/main/java/net/runelite/api/events/WorldViewLoaded.java (1)
Value(33-37)runelite-api/src/main/java/net/runelite/api/events/WorldViewUnloaded.java (1)
Value(33-37)
runelite-api/src/main/java/net/runelite/api/events/WorldViewLoaded.java (3)
runelite-api/src/main/java/net/runelite/api/events/WorldEntityDespawned.java (1)
Value(33-37)runelite-api/src/main/java/net/runelite/api/events/WorldEntitySpawned.java (1)
Value(36-40)runelite-api/src/main/java/net/runelite/api/events/WorldViewUnloaded.java (1)
Value(33-37)
runelite-client/src/main/java/net/runelite/client/plugins/groundmarkers/GroundMarkerMinimapOverlay.java (1)
runelite-api/src/main/java/net/runelite/api/Perspective.java (1)
Perspective(62-1159)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/equipment/Rs2Equipment.java (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/menu/NewMenuEntry.java (1)
NewMenuEntry(11-317)
runelite-api/src/main/java/net/runelite/api/events/WorldViewUnloaded.java (3)
runelite-api/src/main/java/net/runelite/api/events/WorldEntityDespawned.java (1)
Value(33-37)runelite-api/src/main/java/net/runelite/api/events/WorldEntitySpawned.java (1)
Value(36-40)runelite-api/src/main/java/net/runelite/api/events/WorldViewLoaded.java (1)
Value(33-37)
runelite-client/src/main/java/net/runelite/client/plugins/npchighlight/NpcIndicatorsPlugin.java (1)
runelite-client/src/main/java/net/runelite/client/ui/overlay/outline/ModelOutlineRenderer.java (2)
AllArgsConstructor(60-65)AllArgsConstructor(67-74)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2Inventory.java (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/menu/NewMenuEntry.java (1)
NewMenuEntry(11-317)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java (6)
runelite-api/src/main/java/net/runelite/api/coords/WorldArea.java (1)
WorldArea(45-784)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2Inventory.java (1)
Rs2Inventory(46-2313)runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/WorldPointUtil.java (1)
WorldPointUtil(15-274)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/dialogues/Rs2Dialogue.java (1)
Rs2Dialogue(19-756)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/equipment/Rs2Equipment.java (1)
Rs2Equipment(25-392)runelite-api/src/main/java/net/runelite/api/gameval/InterfaceID.java (1)
Bookofscrolls(21155-21238)
runelite-api/src/main/java/net/runelite/api/events/WorldEntitySpawned.java (3)
runelite-api/src/main/java/net/runelite/api/events/WorldEntityDespawned.java (1)
Value(33-37)runelite-api/src/main/java/net/runelite/api/events/WorldViewLoaded.java (1)
Value(33-37)runelite-api/src/main/java/net/runelite/api/events/WorldViewUnloaded.java (1)
Value(33-37)
runelite-client/src/main/java/net/runelite/client/plugins/grounditems/GroundItemsOverlay.java (1)
runelite-api/src/main/java/net/runelite/api/Perspective.java (1)
Perspective(62-1159)
runelite-client/src/main/java/net/runelite/client/plugins/groundmarkers/GroundMarkerOverlay.java (1)
runelite-api/src/main/java/net/runelite/api/Perspective.java (1)
Perspective(62-1159)
runelite-client/src/main/java/net/runelite/client/ui/overlay/outline/ModelOutlineRenderer.java (1)
runelite-api/src/main/java/net/runelite/api/Perspective.java (1)
Perspective(62-1159)
runelite-client/src/main/java/net/runelite/client/game/AgilityShortcut.java (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/gameobject/ObjectID.java (1)
ObjectID(10-33)runelite-api/src/main/java/net/runelite/api/gameval/DBTableID.java (1)
Quest(8-404)
⏰ 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). (2)
- GitHub Check: Build
- GitHub Check: Build
🔇 Additional comments (11)
runelite-client/src/main/java/net/runelite/client/plugins/interfacestyles/InterfaceStylesPlugin.java (1)
181-189: LGTM!The worldViewId propagation is consistent with how other menu entry properties are being copied to the submenu entry. The logic correctly preserves the WorldViewId context when condensing player options into submenus.
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/equipment/Rs2Equipment.java (2)
383-391: LGTM!The
getSafeBoundshelper method correctly retrieves widget bounds and provides a sensible fallback to the full canvas rectangle when bounds are unavailable.
343-377: LGTM!The safe bounds computation is consistently applied across all equipment slots, replacing the previous static full-canvas rectangle approach.
runelite-client/src/main/java/net/runelite/client/plugins/objectindicators/ObjectIndicatorsOverlay.java (1)
86-107: LGTM!The WorldView-aware filtering logic is well-structured with proper null guards and plane checks before rendering. The overlap detection via
WorldEntity.isHiddenForOverlap()appropriately prevents rendering objects that should be hidden.runelite-api/src/main/java/net/runelite/api/Client.java (1)
2419-2432: LGTM!Clean API method declarations extending the Client interface to support camera focus retrieval and WorldView lookup from WorldPoint.
runelite-client/src/main/java/net/runelite/client/plugins/itemidentification/ItemIdentification.java (1)
328-328: LGTM!The SURGE_POTION entry follows the established pattern for potion identification, consistent with other potion types in the enum.
runelite-client/src/main/java/net/runelite/client/plugins/interacthighlight/InteractHighlightOverlay.java (1)
96-100: LGTM!The worldId extraction and parameter addition correctly extends the tile object lookup to be WorldView-aware.
runelite-api/src/main/java/net/runelite/api/WorldEntity.java (1)
27-50: LGTM!The interface extension to
CameraFocusableEntityand the new location/transform methods are well-documented and align with the WorldView-aware architecture introduced in this PR.runelite-api/src/main/java/net/runelite/api/events/WorldViewLoaded.java (1)
30-37: LGTM!Standard Lombok event class following the established pattern used for other WorldView/WorldEntity lifecycle events.
runelite-api/src/main/java/net/runelite/api/events/WorldEntitySpawned.java (1)
30-40: LGTM!Standard event class with helpful documentation clarifying that this fires before the WorldView is loaded, setting clear expectations for event ordering.
runelite-api/src/main/java/net/runelite/api/events/WorldViewUnloaded.java (1)
30-37: LGTM!Standard event class completing the WorldView lifecycle event pair with WorldViewLoaded.
| public static LocalPoint fromWorld(Client client, WorldPoint point) | ||
| { | ||
| return fromWorld(client.getTopLevelWorldView(), world); | ||
| WorldView wv = client.findWorldViewFromWorldPoint(point); | ||
| return fromWorld(wv, point); | ||
| } |
There was a problem hiding this comment.
Handle null WorldView lookup before delegating
client.findWorldViewFromWorldPoint(point) returns null when the tile isn’t backed by a loaded WorldView (eg, outside the scene). Calling fromWorld(null, …) then dereferences wv and will throw. Add a null guard and return null instead.
WorldView wv = client.findWorldViewFromWorldPoint(point);
- return fromWorld(wv, point);
+ if (wv == null)
+ {
+ return null;
+ }
+ return fromWorld(wv, point);📝 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 static LocalPoint fromWorld(Client client, WorldPoint point) | |
| { | |
| return fromWorld(client.getTopLevelWorldView(), world); | |
| WorldView wv = client.findWorldViewFromWorldPoint(point); | |
| return fromWorld(wv, point); | |
| } | |
| public static LocalPoint fromWorld(Client client, WorldPoint point) | |
| { | |
| WorldView wv = client.findWorldViewFromWorldPoint(point); | |
| if (wv == null) | |
| { | |
| return null; | |
| } | |
| return fromWorld(wv, point); | |
| } |
🤖 Prompt for AI Agents
In runelite-api/src/main/java/net/runelite/api/coords/LocalPoint.java around
lines 68 to 72, the method calls client.findWorldViewFromWorldPoint(point) and
passes the result into fromWorld(...) without checking for null; add a null
guard: if the WorldView lookup returns null, return null immediately instead of
delegating, otherwise call fromWorld(wv, point). Ensure the method signature and
callers handle a possible null return value.
| private final JTextField searchField = new JTextField(); | ||
| private final Map<JPanel, String> itemIndex = new HashMap<>(); // item panel -> lowercased name | ||
| private final Map<String, JPanel> sectionContentByKey = new HashMap<>(); // section key -> contents panel | ||
| private final Map<ConfigSectionDescriptor, JPanel> sectionPanelByDesc = new HashMap<>(); // whole section panel |
There was a problem hiding this comment.
Unused map: sectionPanelByDesc is never read.
The map is populated during rebuild but never queried. The applyFilter method uses contents.getParent() directly at line 217 instead of looking up the section panel via this map.
If the map is not needed for future functionality, remove it:
- private final Map<ConfigSectionDescriptor, JPanel> sectionPanelByDesc = new HashMap<>(); mainPanel.removeAll();
itemIndex.clear();
sectionContentByKey.clear();
- sectionPanelByDesc.clear(); sectionWidgets.put(csd.getKey(), sectionContents);
- sectionPanelByDesc.put(csd, section);
sectionContentByKey.put(csd.getKey(), sectionContents);Also applies to: 279-279, 340-340
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/microbot/ui/MicrobotConfigPanel.java
around lines 114 (and also references at ~279 and ~340), the field
sectionPanelByDesc is populated but never read; remove the unused Map
declaration and any code that only exists to populate it (e.g., put calls during
rebuild) and update applyFilter to continue using contents.getParent() as it
already does; also remove any imports or references that become unused after
deleting the field to keep the file clean.
| contents.getParent().setVisible(anyVisible); // whole section panel | ||
| contents.setVisible(anyVisible); // keep contents open when filtering | ||
|
|
||
| // auto-expand sections with matches | ||
| if (!anyVisible) continue; | ||
| // ensure expanded when filtering | ||
| if (!noFilter && !contents.isVisible()) contents.setVisible(true); |
There was a problem hiding this comment.
Dead code: redundant visibility check.
Line 218 sets contents.setVisible(anyVisible). When anyVisible is true (the only case that reaches line 223 due to the continue at line 221), contents is already visible. The condition at line 223 !contents.isVisible() can never be true.
Apply this diff to remove the dead code:
contents.getParent().setVisible(anyVisible); // whole section panel
contents.setVisible(anyVisible); // keep contents open when filtering
- // auto-expand sections with matches
- if (!anyVisible) continue;
- // ensure expanded when filtering
- if (!noFilter && !contents.isVisible()) contents.setVisible(true);
+ if (!anyVisible) continue;
}📝 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.
| contents.getParent().setVisible(anyVisible); // whole section panel | |
| contents.setVisible(anyVisible); // keep contents open when filtering | |
| // auto-expand sections with matches | |
| if (!anyVisible) continue; | |
| // ensure expanded when filtering | |
| if (!noFilter && !contents.isVisible()) contents.setVisible(true); | |
| contents.getParent().setVisible(anyVisible); // whole section panel | |
| contents.setVisible(anyVisible); // keep contents open when filtering | |
| if (!anyVisible) continue; |
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/microbot/ui/MicrobotConfigPanel.java
around lines 217-223, remove the redundant visibility check by replacing the
final conditional so it no longer tests contents.isVisible(); specifically,
change the dead branch `if (!noFilter && !contents.isVisible())
contents.setVisible(true);` to `if (!noFilter) contents.setVisible(true);` (or
remove the conditional entirely if you prefer the call to be unconditional when
filtering) so the code no longer relies on a condition that can never be true.
| } | ||
| int mainMenuIndex = actions.indexOf(subMenuEntry.getKey()); | ||
| if (mainMenuIndex < 0) { | ||
| Microbot.log("Cannot find action=%s, in main actions=%s, mainMenuIndex=%s", subMenuEntry.getKey(), String.join(", ", actions), mainMenuIndex, Level.ERROR); |
There was a problem hiding this comment.
Inconsistent logging pattern.
Line 336 uses a format-string style with placeholders (%s), while other logging calls in the file (lines 306, 331) use string concatenation. This inconsistency may cause issues if Microbot.log does not support this format-string signature with the Level parameter at the end.
Apply this diff to match the logging pattern used elsewhere in the file:
- Microbot.log("Cannot find action=%s, in main actions=%s, mainMenuIndex=%s", subMenuEntry.getKey(), String.join(", ", actions), mainMenuIndex, Level.ERROR);
+ Microbot.log("Cannot find action=" + subMenuEntry.getKey() + ", in main actions=" + String.join(", ", actions) + ", mainMenuIndex=" + mainMenuIndex, Level.ERROR);📝 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.log("Cannot find action=%s, in main actions=%s, mainMenuIndex=%s", subMenuEntry.getKey(), String.join(", ", actions), mainMenuIndex, Level.ERROR); | |
| Microbot.log("Cannot find action=" + subMenuEntry.getKey() + ", in main actions=" + String.join(", ", actions) + ", mainMenuIndex=" + mainMenuIndex, Level.ERROR); |
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/equipment/Rs2Equipment.java
around line 336, the Microbot.log call uses printf-style placeholders with Level
passed as the last argument which is inconsistent with surrounding calls that
use string concatenation and pass Level explicitly; replace the call with the
same concatenation pattern used on lines ~306 and ~331 (build the message by
concatenating subMenuEntry.getKey(), the joined actions string, and
mainMenuIndex) and pass Level.ERROR as the explicit level argument to match the
file's logging pattern.
| public static int getNotedId(int itemId) { | ||
| ItemComposition composition = Microbot.getClientThread().runOnClientThreadOptional(() -> | ||
| Microbot.getClient().getItemDefinition(itemId) | ||
| ).orElse(null); | ||
|
|
||
| return getNotedItemId(composition); | ||
| } |
There was a problem hiding this comment.
Static getNotedId/getUnNotedId can return -1; contradicts method contract
Javadoc promises “otherwise the original item ID,” but when composition is null helpers return -1. This leaks -1 to callers.
public static int getNotedId(int itemId) {
ItemComposition composition = Microbot.getClientThread().runOnClientThreadOptional(() ->
Microbot.getClient().getItemDefinition(itemId)
).orElse(null);
-
- return getNotedItemId(composition);
+ int noted = getNotedItemId(composition);
+ return noted == -1 ? itemId : noted;
}
@@
public static int getUnNotedId(int itemId) {
ItemComposition composition = Microbot.getClientThread().runOnClientThreadOptional(() ->
Microbot.getClient().getItemDefinition(itemId)
).orElse(null);
-
- return getUnNotedId(composition);
+ int unnoted = getUnNotedId(composition);
+ return unnoted == -1 ? itemId : unnoted;
}Also applies to: 457-463, 472-491, 500-519
| public String getAction(String partOfAction) { | ||
| return Arrays.stream(getInventoryActions()) | ||
| .filter(Objects::nonNull) | ||
| .filter(x -> x.toLowerCase().contains(partOfAction)) | ||
| .findFirst().orElse(null); | ||
| } |
There was a problem hiding this comment.
Case-insensitive search not actually case-insensitive
Javadoc says “ignoring case sensitivity,” but partOfAction isn’t lower-cased. Uppercase input can fail.
- public String getAction(String partOfAction) {
- return Arrays.stream(getInventoryActions())
- .filter(Objects::nonNull)
- .filter(x -> x.toLowerCase().contains(partOfAction))
- .findFirst().orElse(null);
- }
+ public String getAction(String partOfAction) {
+ if (partOfAction == null) return null;
+ final String needle = partOfAction.toLowerCase();
+ return Arrays.stream(getInventoryActions())
+ .filter(Objects::nonNull)
+ .filter(x -> x.toLowerCase().contains(needle))
+ .findFirst()
+ .orElse(null);
+ }📝 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 String getAction(String partOfAction) { | |
| return Arrays.stream(getInventoryActions()) | |
| .filter(Objects::nonNull) | |
| .filter(x -> x.toLowerCase().contains(partOfAction)) | |
| .findFirst().orElse(null); | |
| } | |
| public String getAction(String partOfAction) { | |
| if (partOfAction == null) return null; | |
| final String needle = partOfAction.toLowerCase(); | |
| return Arrays.stream(getInventoryActions()) | |
| .filter(Objects::nonNull) | |
| .filter(x -> x.toLowerCase().contains(needle)) | |
| .findFirst() | |
| .orElse(null); | |
| } |
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2ItemModel.java
around lines 579 to 584, the method claims to perform a case-insensitive search
but only lower-cases the inventory action strings; lower-case the incoming
partOfAction (preferably using partOfAction.toLowerCase(Locale.ROOT)) before the
stream filter so both sides are compared in the same case, or alternatively use
String::toLowerCase on both operands inside the filter to ensure true
case-insensitive contains matching.
| if (Rs2Inventory.interact(itemId, itemAction)) { | ||
| log.debug("Traveling with genericAction={}, to {} - ({})", itemAction, transport.getDisplayInfo(), transport.getDestination()); | ||
|
|
||
| log.info("Traveling to {} - ({})", transport.getDisplayInfo(), transport.getDestination()); | ||
| return sleepUntilTrue(() -> Rs2Player.getWorldLocation().distanceTo2D(transport.getDestination()) < OFFSET, 100, 8000); | ||
| if (itemAction.equalsIgnoreCase("open") && itemId == ItemID.BOOKOFSCROLLS_CHARGED) { | ||
| return handleMasterScrollBook(destination); | ||
| } else if (wildernessTransport) { | ||
| Rs2Dialogue.sleepUntilInDialogue(); | ||
| return Rs2Dialogue.clickOption("Yes", "Okay"); | ||
| } else { | ||
| log.info("Unsure how to handle this itemTransport={} action={}", transport, itemAction); | ||
| } | ||
| } |
There was a problem hiding this comment.
Inventory teleports with multiple destinations don’t select the destination after generic “rub/teleport/break”
When itemAction falls back to a generic keyword on multi-destination items, the code doesn’t pick the target option, unlike wearable teleports. This can leave the teleport unresolved and return false, even though the submenu is open.
Apply this diff to select the destination when hasMultipleDestination is true and handle wilderness confirmation afterward:
- if (Rs2Inventory.interact(itemId, itemAction)) {
+ if (Rs2Inventory.interact(itemId, itemAction)) {
log.debug("Traveling with genericAction={}, to {} - ({})", itemAction, transport.getDisplayInfo(), transport.getDestination());
- if (itemAction.equalsIgnoreCase("open") && itemId == ItemID.BOOKOFSCROLLS_CHARGED) {
+ if (itemAction.equalsIgnoreCase("open") && itemId == ItemID.BOOKOFSCROLLS_CHARGED) {
return handleMasterScrollBook(destination);
- } else if (wildernessTransport) {
+ }
+ // Multi-destination inventory items (e.g., games/skills necklace): select the destination
+ else if (hasMultipleDestination) {
+ Rs2Dialogue.sleepUntilSelectAnOption();
+ boolean selected = Rs2Dialogue.clickOption(destination, true);
+ if (!selected) return false;
+ if (wildernessTransport) {
+ Rs2Dialogue.sleepUntilInDialogue();
+ return Rs2Dialogue.clickOption("Yes", "Okay");
+ }
+ return true;
+ } else if (wildernessTransport) {
Rs2Dialogue.sleepUntilInDialogue();
return Rs2Dialogue.clickOption("Yes", "Okay");
} else {
log.info("Unsure how to handle this itemTransport={} action={}", transport, itemAction);
}
}📝 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.
| if (Rs2Inventory.interact(itemId, itemAction)) { | |
| log.debug("Traveling with genericAction={}, to {} - ({})", itemAction, transport.getDisplayInfo(), transport.getDestination()); | |
| log.info("Traveling to {} - ({})", transport.getDisplayInfo(), transport.getDestination()); | |
| return sleepUntilTrue(() -> Rs2Player.getWorldLocation().distanceTo2D(transport.getDestination()) < OFFSET, 100, 8000); | |
| if (itemAction.equalsIgnoreCase("open") && itemId == ItemID.BOOKOFSCROLLS_CHARGED) { | |
| return handleMasterScrollBook(destination); | |
| } else if (wildernessTransport) { | |
| Rs2Dialogue.sleepUntilInDialogue(); | |
| return Rs2Dialogue.clickOption("Yes", "Okay"); | |
| } else { | |
| log.info("Unsure how to handle this itemTransport={} action={}", transport, itemAction); | |
| } | |
| } | |
| if (Rs2Inventory.interact(itemId, itemAction)) { | |
| log.debug("Traveling with genericAction={}, to {} - ({})", itemAction, transport.getDisplayInfo(), transport.getDestination()); | |
| if (itemAction.equalsIgnoreCase("open") && itemId == ItemID.BOOKOFSCROLLS_CHARGED) { | |
| return handleMasterScrollBook(destination); | |
| } | |
| // Multi-destination inventory items (e.g., games/skills necklace): select the destination | |
| else if (hasMultipleDestination) { | |
| Rs2Dialogue.sleepUntilSelectAnOption(); | |
| boolean selected = Rs2Dialogue.clickOption(destination, true); | |
| if (!selected) { | |
| return false; | |
| } | |
| if (wildernessTransport) { | |
| Rs2Dialogue.sleepUntilInDialogue(); | |
| return Rs2Dialogue.clickOption("Yes", "Okay"); | |
| } | |
| return true; | |
| } | |
| else if (wildernessTransport) { | |
| Rs2Dialogue.sleepUntilInDialogue(); | |
| return Rs2Dialogue.clickOption("Yes", "Okay"); | |
| } else { | |
| log.info("Unsure how to handle this itemTransport={} action={}", transport, itemAction); | |
| } | |
| } |
| @Subscribe | ||
| public void onWorldViewUnloaded(WorldViewUnloaded event) | ||
| { | ||
| GameState gameState = gameStateChanged.getGameState(); | ||
| if (gameState == GameState.LOADING) | ||
| var wv = event.getWorldView(); | ||
| objects.removeIf(c -> c.getTileObject().getWorldView() == wv); | ||
| // TODO remove points when the last boat using it despawns? | ||
| if (wv.isTopLevel()) | ||
| { | ||
| // Reload points with new map regions | ||
| objects.clear(); | ||
| reloadPoints(); | ||
| Arrays.stream(wv.getMapRegions()).forEach(points::remove); | ||
| } |
There was a problem hiding this comment.
Null-safety: guard getMapRegions() on unload.
onWorldViewUnloaded assumes wv.getMapRegions() is non-null; at unload time it can be null, causing an NPE.
- if (wv.isTopLevel())
- {
- Arrays.stream(wv.getMapRegions()).forEach(points::remove);
- }
+ if (wv.isTopLevel())
+ {
+ int[] regions = wv.getMapRegions();
+ if (regions != null)
+ {
+ Arrays.stream(regions).forEach(points::remove);
+ }
+ }📝 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.
| @Subscribe | |
| public void onWorldViewUnloaded(WorldViewUnloaded event) | |
| { | |
| GameState gameState = gameStateChanged.getGameState(); | |
| if (gameState == GameState.LOADING) | |
| var wv = event.getWorldView(); | |
| objects.removeIf(c -> c.getTileObject().getWorldView() == wv); | |
| // TODO remove points when the last boat using it despawns? | |
| if (wv.isTopLevel()) | |
| { | |
| // Reload points with new map regions | |
| objects.clear(); | |
| reloadPoints(); | |
| Arrays.stream(wv.getMapRegions()).forEach(points::remove); | |
| } | |
| @@ -247,10 +247,15 @@ public void onWorldViewUnloaded(WorldViewUnloaded event) | |
| - if (wv.isTopLevel()) | |
| - { | |
| - Arrays.stream(wv.getMapRegions()).forEach(points::remove); | |
| if (wv.isTopLevel()) | |
| { | |
| int[] regions = wv.getMapRegions(); | |
| if (regions != null) | |
| { | |
| Arrays.stream(regions).forEach(points::remove); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/objectindicators/ObjectIndicatorsPlugin.java
around lines 247 to 256, onWorldViewUnloaded calls wv.getMapRegions() without
guarding for null which can NPE during unload; fix by checking if
wv.getMapRegions() != null (or using a null-safe guard) before streaming/forEach
to remove points so the removal is skipped when getMapRegions() returns null.
6 participants
fix on rswalker with sub menu items
update with latest runelite