Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR updates microbot.version in pom.xml. It adds ConfigInventorySetupDataManager and wires it into ConfigManager to handle inventory setup defaults based on stored V3 setups. It introduces MicrobotConfigManager with resetProfilePluginProperties. AutoLoginScript removes a forced inventory tab switch. MicrobotConfigPanel is refactored, integrates MicrobotConfigManager, and stores full InventorySetup objects via a combo box. MicrobotPluginHubPanel switches filtering to MicrobotPluginSearch. GrandExchangeOfferDetails adds a GrandExchangeSlots slot field and updates its constructor. Rs2Walker adjusts path selection logic when comparing teleport-enabled vs non-teleport paths. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Tip 🧪 Early access (models): enabledWe are currently testing Sonnet 4.5 code review models, which should lead to better review quality. However, this model may result in higher noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
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: 2
🧹 Nitpick comments (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotConfigManager.java (1)
63-65: Include the exception stack trace in error logging.Line 64 only logs
e.getMessage(), which may not provide sufficient debugging context. Consider passing the exception itself to enable stack trace logging.- Microbot.log("Error resetting profile plugin properties: ", Level.ERROR, e.getMessage()); + Microbot.log("Error resetting profile plugin properties: " + e.getMessage(), Level.ERROR, e);Note: Verify that
Microbot.logaccepts an exception parameter for stack trace logging. If not, consider usinglog.error("Error resetting profile plugin properties", e)directly.runelite-client/src/main/java/net/runelite/client/plugins/microbot/ui/MicrobotConfigPanel.java (1)
547-597: Remove unnecessarysendConfig()calls.Lines 553 and 576 call
configManager.sendConfig()immediately aftersetConfiguration(). TheConfigManageralready persists changes periodically (see line 143 in ConfigManager.java), so these explicit calls are unnecessary and may cause performance issues if inventory setups are changed frequently.if (setups.isEmpty()) { // If there are no setups, return an empty combo box configManager.setConfiguration(cd.getGroup().value(), cid.getItem().keyName(), ""); - configManager.sendConfig(); return new JComboBox<>(); } JComboBox<InventorySetup> box = new JComboBox<>(new DefaultComboBoxModel<>(setups.toArray(new InventorySetup[0]))); // Set the renderer to display the setup name box.setRenderer(new DefaultListCellRenderer() { @Override public Component getListCellRendererComponent(JList<?> list, Object value, int index, boolean isSelected, boolean cellHasFocus) { super.getListCellRendererComponent(list, value, index, isSelected, cellHasFocus); if (value instanceof InventorySetup) { InventorySetup setup = (InventorySetup) value; setText(setup.getName()); } return this; } }); // 1) Retrieve dezerialized InventorySetup from config InventorySetup dezerialized = configManager.getConfiguration(cd.getGroup().value(), cid.getItem().keyName(), InventorySetup.class); if (dezerialized == null) { dezerialized = setups.get(0); configManager.setConfiguration(cd.getGroup().value(), cid.getItem().keyName(), dezerialized); - configManager.sendConfig(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
runelite-client/pom.xml(1 hunks)runelite-client/src/main/java/net/runelite/client/config/ConfigManager.java(4 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotConfigManager.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/accountselector/AutoLoginScript.java(0 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/inventorysetups/ConfigInventorySetupDataManager.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/ui/MicrobotConfigPanel.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/ui/MicrobotPluginHubPanel.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/grandexchange/models/GrandExchangeOfferDetails.java(5 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java(1 hunks)
💤 Files with no reviewable changes (1)
- runelite-client/src/main/java/net/runelite/client/plugins/microbot/accountselector/AutoLoginScript.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-03T03:59:10.180Z
Learnt from: g-mason0
PR: chsami/Microbot#1462
File: runelite-client/src/main/java/net/runelite/client/plugins/microbot/ui/MicrobotPluginHubPanel.java:343-344
Timestamp: 2025-09-03T03:59:10.180Z
Learning: In MicrobotPluginManager, the public methods installPlugin(), removePlugin(), and update() already use executor.submit() internally to perform their operations asynchronously, making them non-blocking on the EDT. These are wrapper methods that delegate to the actual implementation methods (install(), remove(), refresh()) via the executor.
Applied to files:
runelite-client/src/main/java/net/runelite/client/plugins/microbot/ui/MicrobotPluginHubPanel.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/ui/MicrobotConfigPanel.java
🧬 Code graph analysis (3)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/ui/MicrobotPluginHubPanel.java (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/ui/search/MicrobotPluginSearch.java (1)
MicrobotPluginSearch(10-22)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotConfigManager.java (1)
runelite-client/src/main/java/net/runelite/client/plugins/Plugin.java (1)
Plugin(33-80)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/ui/MicrobotConfigPanel.java (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotConfigManager.java (1)
MicrobotConfigManager(15-67)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (2)
runelite-client/src/main/java/net/runelite/client/config/ConfigManager.java (1)
119-120: LGTM!The injected
ConfigInventorySetupDataManageris correctly wired for use in the inventory setup default-value logic.runelite-client/src/main/java/net/runelite/client/plugins/microbot/ui/MicrobotConfigPanel.java (1)
342-361: LGTM!The reset button correctly integrates
MicrobotConfigManager.resetProfilePluginPropertiesto clear profile-specific plugin properties alongside the existing config reset flow.
| boolean isInventorySetup = item.keyName().toLowerCase().contains("inventorysetup"); | ||
| if (Objects.equals(current, valueString) || (Strings.isNullOrEmpty(current) && Strings.isNullOrEmpty(valueString))) | ||
| { | ||
| if (isInventorySetup) { | ||
| var setups = configInventorySetupDataManager.loadV3Setups(this); | ||
| if (!setups.isEmpty()) { | ||
| setConfiguration(group.value(), item.keyName(), gson.toJson(setups.get(0), InventorySetup.class)); | ||
| } | ||
| } | ||
| continue; // already set to the default value | ||
| } | ||
|
|
||
| if (current != null && !current.isBlank()) { | ||
| if (isInventorySetup) { | ||
| var setups = configInventorySetupDataManager.loadV3Setups(this); | ||
| InventorySetup currentObj = gson.fromJson(current, InventorySetup.class); | ||
| if (!setups.isEmpty() && setups.stream() | ||
| .noneMatch(x -> x.getName().equalsIgnoreCase(currentObj.getName()))) { | ||
| setConfiguration(group.value(), item.keyName(), ""); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Verify the inventory setup default-value logic.
The inventory setup handling introduces several control-flow inconsistencies:
-
Lines 1246-1251: This block runs when
currentalready equalsvalueString(or both are empty), yet it overwrites the config with the first loaded setup. If the values already match, why overwrite? -
Line 1252: The
continuestatement skips the normal default-setting logic (lines 1266-1275), meaning inventory setup keys bypass the standard flow entirely when current equals default. -
Lines 1247 & 1257:
loadV3Setupsmay be called twice for the same key if both conditions are true, which is inefficient. -
Line 1243: Using
.toLowerCase().contains("inventorysetup")is fragile—it will match any key containing that substring (e.g.,"myInventorySetupHelper"). Consider an exact key-suffix match or an annotation-based approach. -
Lines 1255-1264: The validation logic (clearing config if the setup name doesn't match any loaded setup) is reasonable, but its placement after the first block creates confusing flow. Should this validation happen before setting defaults?
Consider refactoring to:
- Load setups once and cache the result
- Clarify when defaults should be set vs. when validation should clear stale values
- Use a more precise key-matching strategy (e.g., check for a specific annotation or key prefix/suffix)
- boolean isInventorySetup = item.keyName().toLowerCase().contains("inventorysetup");
+ boolean isInventorySetup = item.keyName().endsWith("InventorySetup") || item.keyName().endsWith("inventorySetup");
if (Objects.equals(current, valueString) || (Strings.isNullOrEmpty(current) && Strings.isNullOrEmpty(valueString)))
{
- if (isInventorySetup) {
+ // If inventory setup, ensure the stored setup still exists
+ if (isInventorySetup && !Strings.isNullOrEmpty(current)) {
var setups = configInventorySetupDataManager.loadV3Setups(this);
- if (!setups.isEmpty()) {
- setConfiguration(group.value(), item.keyName(), gson.toJson(setups.get(0), InventorySetup.class));
+ InventorySetup currentObj = gson.fromJson(current, InventorySetup.class);
+ if (currentObj != null && !setups.isEmpty() && setups.stream()
+ .noneMatch(x -> x.getName().equalsIgnoreCase(currentObj.getName()))) {
+ // Stored setup no longer exists, clear it
+ setConfiguration(group.value(), item.keyName(), "");
}
}
continue; // already set to the default value
}
- if (current != null && !current.isBlank()) {
+ // Set default if not already set
+ if (isInventorySetup && Strings.isNullOrEmpty(current)) {
+ var setups = configInventorySetupDataManager.loadV3Setups(this);
+ if (!setups.isEmpty()) {
+ setConfiguration(group.value(), item.keyName(), gson.toJson(setups.get(0), InventorySetup.class));
+ continue;
+ }
+ }
+
+ if (current != null && !current.isBlank() && !isInventorySetup) {
- if (isInventorySetup) {
- var setups = configInventorySetupDataManager.loadV3Setups(this);
- InventorySetup currentObj = gson.fromJson(current, InventorySetup.class);
- if (!setups.isEmpty() && setups.stream()
- .noneMatch(x -> x.getName().equalsIgnoreCase(currentObj.getName()))) {
- setConfiguration(group.value(), item.keyName(), "");
- }
- }
+ // Non-inventory-setup handling (if needed)
}
-This refactor:
- Validates stale setups when
currentequals default (lines 1246-1251 replacement) - Sets the first setup as default only when
currentis empty (new block after the continue) - Removes the duplicate
loadV3Setupscall - Clarifies the control flow
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In runelite-client/src/main/java/net/runelite/client/config/ConfigManager.java
around lines 1243 to 1264, the inventory-setup handling has confusing control
flow, duplicated setup loads, and a fragile key match; refactor so that you (1)
replace the toLowerCase().contains(...) check with a precise match (e.g.,
keyName endsWith or explicit suffix/prefix constant or annotation-based flag),
(2) loadV3Setups once into a local variable before any branching, (3) perform
validation of the current value first (if current is non-blank and a setup name
no longer exists, clear it), and (4) only set the first setup as the default
when current is blank (not when current already equals the default), ensuring
the normal default-setting logic after the continue remains intact; this removes
the duplicate load, clarifies flow, and prevents overwriting identical values.
| // Derive the key prefix from class name by stripping the 'Config' suffix | ||
| String name = plugin.getSimpleName().replaceFirst("Plugin$", "").toLowerCase(); |
There was a problem hiding this comment.
Fix the incorrect comment.
Line 37 states "stripping the 'Config' suffix" but line 38 actually strips the "Plugin" suffix. Update the comment to match the code.
- // Derive the key prefix from class name by stripping the 'Config' suffix
+ // Derive the key prefix from class name by stripping the 'Plugin' suffix
String name = plugin.getSimpleName().replaceFirst("Plugin$", "").toLowerCase();📝 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.
| // Derive the key prefix from class name by stripping the 'Config' suffix | |
| String name = plugin.getSimpleName().replaceFirst("Plugin$", "").toLowerCase(); | |
| // Derive the key prefix from class name by stripping the 'Plugin' suffix | |
| String name = plugin.getSimpleName().replaceFirst("Plugin$", "").toLowerCase(); |
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotConfigManager.java
around lines 37 to 38, the comment incorrectly says "stripping the 'Config'
suffix" while the code actually strips the "Plugin" suffix; update the comment
to accurately describe the operation (e.g., "Derive the key prefix from class
name by stripping the 'Plugin' suffix") so the comment matches the code.
2 participants
inventory setup gets loaded on client startup
plugin hub search fixed
resetting a plugin will remove all the values from the .json file
improved grand exchange API