Conversation
WalkthroughAdds a new commit-guidelines doc at .github/git-commit-instructions.md. Bumps microbot.version in runelite-client/pom.xml from 2.0.7 to 2.0.8. Adjusts RuneScapeProfileType: RAGING_ECHOES_LEAGUE becomes parameterless; adds GRID_MASTER using TOURNAMENT_WORLD. Updates HiscoreEndpoint TOURNAMENT display name to "Grid Master" and replaces SEASONAL with TOURNAMENT in HiscorePanel. Introduces a configurable RS2 cache feature: config section/item, flag in Microbot, conditional cache usage in Microbot and Script, and guarded init/shutdown in MicrobotPlugin. Removes Volcanic Ash Miner plugin, config, overlay, and script. Switches a varbit read in Rs2Player to Microbot.getVarbitValue. Makes Rs2Walker debug printing conditional. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/player/Rs2Player.java (1)
1429-1434: Anti‑poison guard is inverted — returns success without drinking when neededCurrent logic returns true when anti‑poison is NOT active. It should only return early when protection is already active (anti‑venom or anti‑poison).
public static boolean drinkAntiPoisonPotion() { - if (!hasAntiPoisonActive() || hasAntiVenomActive()) { - return true; - } - return usePotion(Rs2Potion.getAntiPoisonVariants().toArray(new String[0])); + if (hasAntiVenomActive() || hasAntiPoisonActive()) { + return true; + } + return usePotion(Rs2Potion.getAntiPoisonVariants().toArray(new String[0])); }runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java (1)
1652-1670: Trapdoor open/closed mapping used backwardsMap is OPEN→CLOSED, but the variables are assigned as if it were CLOSED→OPEN. This flips the handling and can break interactions.
- for (Map.Entry<Integer, Integer> entry : OPEN_TO_CLOSED_MAPPINGS.entrySet()) { - final int closedTrapdoorId = entry.getKey(); - final int openTrapdoorId = entry.getValue(); + for (Map.Entry<Integer, Integer> entry : OPEN_TO_CLOSED_MAPPINGS.entrySet()) { + final int openTrapdoorId = entry.getKey(); + final int closedTrapdoorId = entry.getValue();runelite-client/src/main/java/net/runelite/client/hiscore/HiscoreEndpoint.java (1)
56-66: Endpoint mapping inconsistent with UI (SEASONAL tab removed, but mapping still returns SEASONAL)fromWorldTypes can still return SEASONAL, yet the panel no longer exposes it. Map SEASONAL worlds to TOURNAMENT (Grid Master) or align UI otherwise to avoid invalid tab selection.
public static HiscoreEndpoint fromWorldTypes(Set<WorldType> worldTypes) { if (worldTypes.contains(WorldType.SEASONAL)) { - return HiscoreEndpoint.SEASONAL; + return HiscoreEndpoint.TOURNAMENT; }runelite-client/src/main/java/net/runelite/client/plugins/hiscore/HiscorePanel.java (1)
692-698: Defensive fallback needed when plugin returns an endpoint not present in ENDPOINTSIf plugin.getWorldEndpoint() returns SEASONAL, idx becomes -1 and tab selection may break. Add a safe fallback.
private void resetEndpoints() { // Select the correct tab based on the world type. HiscoreEndpoint endpoint = plugin.getWorldEndpoint(); - int idx = ArrayUtils.indexOf(ENDPOINTS, endpoint); - tabGroup.select(tabGroup.getTab(idx)); + int idx = ArrayUtils.indexOf(ENDPOINTS, endpoint); + if (idx < 0) { + endpoint = HiscoreEndpoint.NORMAL; + idx = ArrayUtils.indexOf(ENDPOINTS, endpoint); + } + tabGroup.select(tabGroup.getTab(idx)); }runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotPlugin.java (1)
299-313: Order-of-ops bug: Cache-dependent read before cache handlers are registered.Rs2RunePouch.fullUpdate() uses Microbot.getVarbitValue(), which will hit Rs2VarbitCache when enabled, but event handlers are registered afterward. Swap the order to avoid stale/zero reads.
- Microbot.setLoginTime(Instant.now()); - Rs2RunePouch.fullUpdate(); - if (microbotConfig.isRs2CacheEnabled()) { - Rs2CacheManager.registerEventHandlers(); - } + Microbot.setLoginTime(Instant.now()); + if (Microbot.isRs2CacheEnabled()) { + Rs2CacheManager.registerEventHandlers(); + } + Rs2RunePouch.fullUpdate();Also prefer the unified gate:
- if (microbotConfig.isRs2CacheEnabled()) { + if (Microbot.isRs2CacheEnabled()) {
🧹 Nitpick comments (4)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotPlugin.java (1)
534-542: Shutdown inconsistency: double-close risk and lazy init during shutdown.ClientShutdown closes caches behind a config gate, but plugin shutDown() always calls shutdownCacheSystem(), which internally getInstance()’s the manager even if caching was never enabled. Align gates to avoid instantiating on shutdown and double-closing.
Change the ClientShutdown gate to the runtime flag:
- if (microbotConfig.isRs2CacheEnabled()) { + if (Microbot.isRs2CacheEnabled()) { Rs2CacheManager.savePersistentCaches(); Rs2CacheManager.getInstance().close(); }Additionally, guard plugin shutDown() (outside this hunk) to mirror the same logic:
- // Shutdown the cache system - shutdownCacheSystem(); + // Shutdown the cache system (only if enabled/initialized) + if (Microbot.isRs2CacheEnabled()) { + shutdownCacheSystem(); + }runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotConfig.java (1)
102-113: Config wording vs implementation scope.Description says “cache ingame entities (npcs, objects,...)”, but enabling this also switches varbit/varp reads to cache-backed paths. Consider clarifying the description to reflect state/varbit/varp caching too, or scope the switch narrowly.
runelite-client/src/main/java/net/runelite/client/plugins/microbot/Script.java (1)
89-93: Comment contradicts behavior (cache wait is feature‑gated).The comment implies unconditional waiting after login, but the guard waits only when RS2 cache is enabled.
Apply this diff to align the comment with behavior:
- //when we log in, we must wait for the cache to be loaded before doing anything + // When RS2 cache is enabled and we log in, wait for the cache to be loaded before doing anything.github/git-commit-instructions.md (1)
11-11: Fix missing language specifiers in fenced code blocks.The markdownlint warnings are valid - fenced code blocks should specify the language for proper syntax highlighting and accessibility.
Apply this diff to fix the language specifiers:
-``` +```text <type>(scope): <short summary>-
+text
feat(auth): add token refresh to api client
fix(ui): correct sidebar collapse state on mobile
refactor(store): move user preferences into pinia
style: apply prettier formatting
docs: add usage section for useApiClient composable
test(api): add retry logic tests for error handling
build: upgrade nuxt to 3.14 and update lockfile
chore: remove unused eslint-disable rules
perf(router): lazy-load admin routes- ``` + ```text fix(api): handle 401 on refresh token Closes #123Also applies to: 52-52, 74-74 </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 5036f77e3906d9fc8a049e822ecd3dbfe07113d2 and 88164e4a42c68a0c98b6622d4f1fe4bc2d52a3d9. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `runelite-client/src/main/resources/net/runelite/client/plugins/hiscore/tournament.png` is excluded by `!**/*.png` </details> <details> <summary>📒 Files selected for processing (15)</summary> * `.github/git-commit-instructions.md` (1 hunks) * `runelite-client/pom.xml` (1 hunks) * `runelite-client/src/main/java/net/runelite/client/config/RuneScapeProfileType.java` (1 hunks) * `runelite-client/src/main/java/net/runelite/client/hiscore/HiscoreEndpoint.java` (1 hunks) * `runelite-client/src/main/java/net/runelite/client/plugins/hiscore/HiscorePanel.java` (1 hunks) * `runelite-client/src/main/java/net/runelite/client/plugins/microbot/Microbot.java` (2 hunks) * `runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotConfig.java` (3 hunks) * `runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotPlugin.java` (10 hunks) * `runelite-client/src/main/java/net/runelite/client/plugins/microbot/Script.java` (2 hunks) * `runelite-client/src/main/java/net/runelite/client/plugins/microbot/TaF/VolcanicAshMiner/VolcanicAshMinerConfig.java` (0 hunks) * `runelite-client/src/main/java/net/runelite/client/plugins/microbot/TaF/VolcanicAshMiner/VolcanicAshMinerOverlay.java` (0 hunks) * `runelite-client/src/main/java/net/runelite/client/plugins/microbot/TaF/VolcanicAshMiner/VolcanicAshMinerPlugin.java` (0 hunks) * `runelite-client/src/main/java/net/runelite/client/plugins/microbot/TaF/VolcanicAshMiner/VolcanicAshMinerScript.java` (0 hunks) * `runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/player/Rs2Player.java` (1 hunks) * `runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java` (3 hunks) </details> <details> <summary>💤 Files with no reviewable changes (4)</summary> * runelite-client/src/main/java/net/runelite/client/plugins/microbot/TaF/VolcanicAshMiner/VolcanicAshMinerOverlay.java * runelite-client/src/main/java/net/runelite/client/plugins/microbot/TaF/VolcanicAshMiner/VolcanicAshMinerConfig.java * runelite-client/src/main/java/net/runelite/client/plugins/microbot/TaF/VolcanicAshMiner/VolcanicAshMinerScript.java * runelite-client/src/main/java/net/runelite/client/plugins/microbot/TaF/VolcanicAshMiner/VolcanicAshMinerPlugin.java </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (2)</summary> <details> <summary>📚 Learning: 2025-08-23T16:17:37.615Z</summary>Learnt from: g-mason0
PR: #1423
File: runelite-client/src/main/java/net/runelite/client/plugins/microbot/externalplugins/MicrobotPluginManager.java:366-389
Timestamp: 2025-08-23T16:17:37.615Z
Learning: The ScheduledExecutorService injected into MicrobotPluginManager (and other RuneLite classes) is configured as a single-threaded executor via Executors.newSingleThreadScheduledExecutor() at RuneLiteModule.java line 130, wrapped in ExecutorServiceExceptionLogger. This ensures all async tasks execute sequentially, preventing race conditions.**Applied to files:** - `runelite-client/src/main/java/net/runelite/client/plugins/microbot/Script.java` </details> <details> <summary>📚 Learning: 2025-08-25T15:51:39.272Z</summary>Learnt from: runsonmypc
PR: #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/walker/Rs2Walker.java` </details> </details><details> <summary>🧬 Code graph analysis (1)</summary> <details> <summary>runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotPlugin.java (1)</summary><blockquote> <details> <summary>runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2RunePouch.java (1)</summary> * `Rs2RunePouch` (28-596) </details> </blockquote></details> </details><details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>.github/git-commit-instructions.md</summary> 11-11: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 52-52: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 74-74: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>⏰ 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)</summary> * GitHub Check: Build </details> <details> <summary>🔇 Additional comments (2)</summary><blockquote> <details> <summary>runelite-client/pom.xml (1)</summary><blockquote> `44-44`: **Version bump looks clean and consistent.** The microbot version increment from 2.0.7 to 2.0.8 aligns with the PR objectives and follows semantic versioning practices. The version is used in the Maven Shade plugin's output filename (Line 450), maintaining consistency throughout the build process. </blockquote></details> <details> <summary>runelite-client/src/main/java/net/runelite/client/config/RuneScapeProfileType.java (1)</summary><blockquote> `48-49`: **RAGING_ECHOES_LEAGUE missing predicate — confirm or fix** RAGING_ECHOES_LEAGUE is declared parameterless while GRID_MASTER has a Client predicate (client -> client.getWorldType().contains(WorldType.TOURNAMENT_WORLD)); parameterless enum entries use the no-arg constructor and will not be matched by RuneScapeProfileType.getCurrent. Add the appropriate predicate (e.g. check WorldType.SEASONAL) or restore the previous parameterized form. Location: runelite-client/src/main/java/net/runelite/client/config/RuneScapeProfileType.java (constants around lines 46–49). </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
|
|
||
| @Setter | ||
| @Getter | ||
| private static boolean isRs2CacheEnabled = false; | ||
|
|
There was a problem hiding this comment.
Config/runtime flag drift: Microbot.isRs2CacheEnabled vs MicrobotConfig.isRs2CacheEnabled lead to inconsistent behavior after toggling.
The static flag is set only at startup. Elsewhere (in MicrobotPlugin) gating uses the config directly, while getVarbit/varp here uses the static flag. If the user flips the setting at runtime, the two paths diverge.
Fix by updating the static flag and (un)initializing caches on config change, and then consistently gate on Microbot.isRs2CacheEnabled() in MicrobotPlugin. See suggested patch in MicrobotPlugin comments.
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/microbot/Microbot.java
around lines 198 to 202, the static flag isRs2CacheEnabled is only set at
startup which causes drift against MicrobotConfig.isRs2CacheEnabled when the
user toggles the setting at runtime; update the static flag whenever the config
changes and (un)initialize any RS2 cache state there so both code paths remain
consistent, and ensure MicrobotPlugin uses Microbot.isRs2CacheEnabled() as the
single source of truth for gating; implement a config-change listener that sets
Microbot.setRs2CacheEnabled(config.isRs2CacheEnabled()), initializes caches when
true, clears/unloads caches when false, and remove direct reads of
MicrobotConfig.isRs2CacheEnabled() elsewhere so all checks go through
Microbot.isRs2CacheEnabled().
| case MicrobotConfig.keyEnableCache: | ||
| break; | ||
| default: |
There was a problem hiding this comment.
No-op on cache toggle; implement runtime (un)toggle to keep config and runtime in sync.
When keyEnableCache changes, nothing updates the runtime flag nor (un)initializes handlers/caches. Add handling:
case MicrobotConfig.keyEnableCache:
- break;
+ {
+ final boolean enable = microbotConfig.isRs2CacheEnabled();
+ Microbot.setRs2CacheEnabled(enable);
+ if (enable) {
+ initializeCacheSystem();
+ if (Microbot.isLoggedIn()) {
+ Rs2CacheManager.registerEventHandlers();
+ }
+ } else {
+ Rs2CacheManager.unregisterEventHandlers();
+ shutdownCacheSystem();
+ }
+ break;
+ }Also unify other gates in this class to use the runtime flag:
- if (microbotConfig.isRs2CacheEnabled()) {
+ if (Microbot.isRs2CacheEnabled()) {Apply at: startUp init (optional), onRuneScapeProfileChanged, onGameStateChanged, logout path, and onClientShutdown.
Committable suggestion skipped: line range outside the PR's diff.
2 participants
feat: migrated volcanicAshMinerScript
@chsami
chsami committed 12 hours ago
refactor: reorganize imports and enhance debug logging in Rs2Walker
@chsami
chsami committed 12 hours ago
docs: add commit instructions for Vue/Nuxt 3 projects
@chsami
chsami committed 12 hours ago
feat: implement toggle for enabling/disabling caching ingame data
@chsami
chsami committed 3 hours ago