Promote development with RuneLite startup and looting fixes#1803
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe PR updates ground-item action reflection so failed extraction returns a default Take-based action, reworks ground-item interaction callers and smoke-test timing, changes camera floating-point accessors and Perspective trig/projection math to use new 14-bit tables, and bumps Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/api/tileitem/models/Rs2TileItemModel.java`:
- Around line 251-253: The no-arg click path in Rs2TileItemModel is still
selecting the first slot even when it is null, so it never reaches the fallback
ground-item action. Update the action resolution logic in click() and the nearby
index selection for the empty-action case to choose the first non-blank entry
from groundActions instead of defaulting to index 0, or make click() call the
explicit action that matches the resolved fallback. Make sure the warning path
around MenuAction.CANCEL remains only for truly unsupported actions, and verify
the no-arg interaction now resolves to the Take option rather than
GROUND_ITEM_FIRST_OPTION.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/example/ExampleScript.java`:
- Around line 321-323: The post-drop settle wait in ExampleScript is currently
ignored, so the smoke test can continue even if the player is still animating or
moving. In the ExampleScript flow, capture the result of the sleepUntil call
after the item appearance check, assert that it succeeds, and only then proceed
to Rs2GroundItem.loot so the test fails fast when the player never settles.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/grounditem/Rs2GroundItem.java`:
- Around line 108-111: The new CANCEL failure path in
Rs2GroundItem.interact(InteractModel, String) is being ignored by the public
helpers, so they still report success after a failed interaction. Update
Rs2GroundItem.interact(RS2Item, String) to return and propagate the boolean
result from the underlying interact call instead of always returning true, and
make sure loot(...), pickup(...), and interact(RS2Item) all surface that false
result consistently.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/reflection/Rs2Reflection.java`:
- Around line 156-169: Cache misses in Rs2Reflection.extractWithCache still
return an empty action array on the early-exit paths, which bypasses the
intended default Take fallback. Update the early returns in extractWithCache so
they either call defaultGroundItemActions() directly or invalidate the cache and
re-discover before returning, and keep the normalization behavior consistent
with groundItemActionsOrDefault and defaultGroundItemActions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9f609494-f964-47fb-a2bb-d71039b85c2f
📒 Files selected for processing (8)
docs/entity-guides/items.mdgradle.propertiesrunelite-api/src/main/java/net/runelite/api/Client.javarunelite-api/src/main/java/net/runelite/api/Perspective.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/api/tileitem/models/Rs2TileItemModel.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/example/ExampleScript.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/grounditem/Rs2GroundItem.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/reflection/Rs2Reflection.java
| if (menuAction == MenuAction.CANCEL) { | ||
| log.warn("Unable to interact with ground item '{}' using action '{}'; actions={}", getName(), action, java.util.Arrays.toString(groundActions)); | ||
| return false; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
click() still resolves the wrong slot under the new fallback.
This guard never fires for the no-arg path, because Lines 227-229 force index = 0 even when groundActions[0] is null. With the new fallback {null, null, "Take", null, null}, click() now dispatches GROUND_ITEM_FIRST_OPTION instead of the fallback Take third option. Resolve the first non-blank action when action.isEmpty(), or have click() delegate to an explicit action.
Suggested fix
int index = -1;
if (action.isEmpty()) {
- action = groundActions[0];
- index = 0;
+ for (int i = 0; i < groundActions.length; i++) {
+ String groundAction = groundActions[i];
+ if (groundAction == null || groundAction.isBlank()) continue;
+ action = groundAction;
+ index = i;
+ break;
+ }
} else {
for (int i = 0; i < groundActions.length; i++) {
String groundAction = groundActions[i];📝 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 (menuAction == MenuAction.CANCEL) { | |
| log.warn("Unable to interact with ground item '{}' using action '{}'; actions={}", getName(), action, java.util.Arrays.toString(groundActions)); | |
| return false; | |
| int index = -1; | |
| if (action.isEmpty()) { | |
| for (int i = 0; i < groundActions.length; i++) { | |
| String groundAction = groundActions[i]; | |
| if (groundAction == null || groundAction.isBlank()) continue; | |
| action = groundAction; | |
| index = i; | |
| break; | |
| } | |
| } else { | |
| for (int i = 0; i < groundActions.length; i++) { | |
| String groundAction = groundActions[i]; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/api/tileitem/models/Rs2TileItemModel.java`
around lines 251 - 253, The no-arg click path in Rs2TileItemModel is still
selecting the first slot even when it is null, so it never reaches the fallback
ground-item action. Update the action resolution logic in click() and the nearby
index selection for the empty-action case to choose the first non-blank entry
from groundActions instead of defaulting to index 0, or make click() call the
explicit action that matches the resolved fallback. Make sure the warning path
around MenuAction.CANCEL remains only for truly unsupported actions, and verify
the no-arg interaction now resolves to the Take option rather than
GROUND_ITEM_FIRST_OPTION.
| boolean itemAppeared = sleepUntil(() -> Rs2GroundItem.exists(itemName, 5), 5000); | ||
| assertTrue(itemAppeared, "item did not appear on ground after drop"); | ||
| sleepUntil(() -> !Rs2Player.isAnimating() && !Rs2Player.isMoving(), 5000); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fail the smoke test if the player never settles.
The new wait is fire-and-forget right now. If the player is still animating or moving after 5s, the check continues anyway and can fail for the same race this change was added to avoid. Assert the sleepUntil(...) result before calling Rs2GroundItem.loot(...).
Suggested fix
boolean itemAppeared = sleepUntil(() -> Rs2GroundItem.exists(itemName, 5), 5000);
assertTrue(itemAppeared, "item did not appear on ground after drop");
- sleepUntil(() -> !Rs2Player.isAnimating() && !Rs2Player.isMoving(), 5000);
+ boolean settled = sleepUntil(() -> !Rs2Player.isAnimating() && !Rs2Player.isMoving(), 5000);
+ assertTrue(settled, "player never settled after drop");
boolean looted = Rs2GroundItem.loot(itemName, 5);📝 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.
| boolean itemAppeared = sleepUntil(() -> Rs2GroundItem.exists(itemName, 5), 5000); | |
| assertTrue(itemAppeared, "item did not appear on ground after drop"); | |
| sleepUntil(() -> !Rs2Player.isAnimating() && !Rs2Player.isMoving(), 5000); | |
| boolean itemAppeared = sleepUntil(() -> Rs2GroundItem.exists(itemName, 5), 5000); | |
| assertTrue(itemAppeared, "item did not appear on ground after drop"); | |
| boolean settled = sleepUntil(() -> !Rs2Player.isAnimating() && !Rs2Player.isMoving(), 5000); | |
| assertTrue(settled, "player never settled after drop"); | |
| boolean looted = Rs2GroundItem.loot(itemName, 5); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/example/ExampleScript.java`
around lines 321 - 323, The post-drop settle wait in ExampleScript is currently
ignored, so the smoke test can continue even if the player is still animating or
moving. In the ExampleScript flow, capture the result of the sleepUntil call
after the item appearance check, assert that it succeeds, and only then proceed
to Rs2GroundItem.loot so the test fails fast when the player never settles.
| if (menuAction == MenuAction.CANCEL) { | ||
| log.warn("Unable to interact with ground item '{}' using action '{}'; actions={}", groundItem.getName(), action, Arrays.toString(groundActions)); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Propagate this new failure path to the public helpers.
interact(InteractModel, String) now returns false here, but interact(RS2Item, String) at Lines 47-55 still discards that result and returns true after the try block. loot(...), pickup(...), and interact(RS2Item) therefore still report success even when this branch is hit.
Suggested fix
private static boolean interact(RS2Item rs2Item, String action) {
if (rs2Item == null) return false;
try {
- interact(new InteractModel(rs2Item.getTileItem().getId(), rs2Item.getTile().getWorldLocation(), rs2Item.getItem().getName()), action);
+ return interact(new InteractModel(rs2Item.getTileItem().getId(), rs2Item.getTile().getWorldLocation(), rs2Item.getItem().getName()), action);
} catch (Exception ex) {
Microbot.logStackTrace("Rs2GroundItem", ex);
+ return false;
}
- return true;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (menuAction == MenuAction.CANCEL) { | |
| log.warn("Unable to interact with ground item '{}' using action '{}'; actions={}", groundItem.getName(), action, Arrays.toString(groundActions)); | |
| return false; | |
| } | |
| private static boolean interact(RS2Item rs2Item, String action) { | |
| if (rs2Item == null) return false; | |
| try { | |
| return interact(new InteractModel(rs2Item.getTileItem().getId(), rs2Item.getTile().getWorldLocation(), rs2Item.getItem().getName()), action); | |
| } catch (Exception ex) { | |
| Microbot.logStackTrace("Rs2GroundItem", ex); | |
| return false; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/grounditem/Rs2GroundItem.java`
around lines 108 - 111, The new CANCEL failure path in
Rs2GroundItem.interact(InteractModel, String) is being ignored by the public
helpers, so they still report success after a failed interaction. Update
Rs2GroundItem.interact(RS2Item, String) to return and propagate the boolean
result from the underlying interact call instead of always returning true, and
make sure loot(...), pickup(...), and interact(RS2Item) all surface that false
result consistently.
| if (cachedStringField == null) return groundItemActionsOrDefault(toStringArray(list)); | ||
| return groundItemActionsOrDefault(extractFromBeans(list, cachedStringField)); | ||
| } | ||
|
|
||
| private static String[] groundItemActionsOrDefault(String[] actions) { | ||
| if (actions != null) { | ||
| for (String action : actions) { | ||
| if (action != null && !action.isBlank()) { | ||
| return actions; | ||
| } | ||
| } | ||
| } | ||
| return defaultGroundItemActions(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Cache misses still bypass the default Take fallback.
extractWithCache() now normalizes successful reads, but it still returns new String[]{} on the early exits at Lines 148 and 153. Once the cache is populated, those branches can hand callers an empty action array again and reintroduce the CANCEL/no-op path this change is meant to remove. Either invalidate the cache there and rediscover, or return defaultGroundItemActions() instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/reflection/Rs2Reflection.java`
around lines 156 - 169, Cache misses in Rs2Reflection.extractWithCache still
return an empty action array on the early-exit paths, which bypasses the
intended default Take fallback. Update the early returns in extractWithCache so
they either call defaultGroundItemActions() directly or invalidate the cache and
re-discover before returning, and keep the normalization behavior consistent
with groundItemActionsOrDefault and defaultGroundItemActions.
2 participants
Summary
Takemaps toGROUND_ITEM_THIRD_OPTIONValidation
./gradlew :client:compileJava:client:runstartup reached RuneLite 1.12.30 without the previousNoSuchFieldError: SINE14Drop item from inventorypassed,Loot item from groundpassed, full result35/35 PASSEDGROUND_ITEM_THIRD_OPTION) with tile params matching the successful runtime log