Conversation
This rewrites aggregateRecords to include all sessionRecords, which is easier than trying to merge the sessionRecords in when they get culled. Additionally deduplicate the record tracking in LootTrackerBox by just holding a reference to the aggregate record.
[ci skip]
# Conflicts: # cache/pom.xml # pom.xml # runelite-api/pom.xml # runelite-client/pom.xml # runelite-client/src/main/java/net/runelite/client/plugins/loottracker/LootTrackerPanel.java # runelite-client/src/main/java/net/runelite/client/plugins/loottracker/LootTrackerRecord.java # runelite-jshell/pom.xml # runelite-maven-plugin/pom.xml
WalkthroughThis pull request updates the parent version to 1.11.23-SNAPSHOT across multiple modules. New content additions include tutorial-island quest entries, polling system interface definitions and varbits, and UI elements for inventory highlighting and widget flipping. The loot tracker undergoes substantial refactoring to employ mutable records with merge capabilities and session history management via deques. WorldHopper filtering extends seasonal treatment to tournament worlds. New frustum culling hooks and widget flipping methods are added to the API. Microbot's loot-related public methods are deprecated. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.39.6)runelite-api/src/main/java/net/runelite/api/NpcID.javarunelite-api/src/main/java/net/runelite/api/gameval/VarbitID.javarunelite-api/src/main/java/net/runelite/api/gameval/DBTableID.java
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
runelite-client/pom.xml (1)
467-476: Fix invalid characters breaking XML in shade plugin filters.There’s an extraneous "§y" after the closing exclude tag on Line 472. This corrupts the POM and will cause Maven parse/build failures. Remove it.
- <exclude>META-INF/*.DSA</exclude>§y + <exclude>META-INF/*.DSA</exclude>runelite-client/src/main/java/net/runelite/client/plugins/itemcharges/ItemChargePlugin.java (1)
699-701: Typo breaks low-warning color selection (and likely compilation).config.lowWarningolor() → config.lowWarningColor().
- color = config.lowWarningolor(); + color = config.lowWarningColor();runelite-client/src/main/java/net/runelite/client/plugins/loottracker/LootTrackerPanel.java (1)
640-647: “Reset” in single‑loot view removes the aggregate record unintentionallyWhen not grouped, match is r -> r.equals(record). Equals/hash are title+type only, so this predicate matches the aggregate entry too, removing it even though the intent is to delete a single kill. Result: aggregate view loses all history for that source.
Make single‑loot reset remove only the specific session entry and then recompute the aggregate for that source:
- Predicate<LootTrackerRecord> match = groupLoot - // With grouped loot, remove any record with this title - ? r -> r.matches(record.getTitle(), record.getType()) - // Otherwise remove specifically this entry - : r -> r.equals(record); - sessionRecords.removeIf(match); - aggregateRecords.values().removeIf(match); + if (groupLoot) + { + // Remove all records for this source + Predicate<LootTrackerRecord> match = r -> r.matches(record.getTitle(), record.getType()); + sessionRecords.removeIf(match); + aggregateRecords.values().removeIf(match); + } + else + { + // Remove only this session entry + sessionRecords.remove(record); // identity + // Rebuild the aggregate for this source from remaining session entries + aggregateRecords.values().removeIf(r -> r.matches(record.getTitle(), record.getType())); + LootTrackerRecord rebuilt = null; + for (LootTrackerRecord s : sessionRecords) + { + if (s.matches(record.getTitle(), record.getType())) + { + if (rebuilt == null) + { + rebuilt = new LootTrackerRecord(s.getTitle(), s.getSubTitle(), s.getType(), new LootTrackerItem[0], 0); + } + rebuilt.merge(s); + } + } + if (rebuilt != null) + { + aggregateRecords.put(rebuilt, rebuilt); + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
cache/pom.xml(1 hunks)pom.xml(1 hunks)runelite-api/pom.xml(1 hunks)runelite-api/src/main/interfaces/interfaces.toml(1 hunks)runelite-api/src/main/java/net/runelite/api/NpcID.java(1 hunks)runelite-api/src/main/java/net/runelite/api/Quest.java(1 hunks)runelite-api/src/main/java/net/runelite/api/gameval/DBTableID.java(3 hunks)runelite-api/src/main/java/net/runelite/api/gameval/InterfaceID.java(8 hunks)runelite-api/src/main/java/net/runelite/api/gameval/NpcID.java(1 hunks)runelite-api/src/main/java/net/runelite/api/gameval/SpriteID.java(1 hunks)runelite-api/src/main/java/net/runelite/api/gameval/VarClientID.java(1 hunks)runelite-api/src/main/java/net/runelite/api/gameval/VarbitID.java(3 hunks)runelite-api/src/main/java/net/runelite/api/hooks/DrawCallbacks.java(2 hunks)runelite-api/src/main/java/net/runelite/api/widgets/Widget.java(1 hunks)runelite-client/pom.xml(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/itemcharges/ItemChargePlugin.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/itemstats/ItemStatChanges.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/loottracker/LootTrackerBox.java(9 hunks)runelite-client/src/main/java/net/runelite/client/plugins/loottracker/LootTrackerPanel.java(13 hunks)runelite-client/src/main/java/net/runelite/client/plugins/loottracker/LootTrackerPlugin.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/loottracker/LootTrackerRecord.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/Microbot.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/worldhopper/WorldTableRow.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/worldhopper/WorldTypeFilter.java(1 hunks)runelite-client/src/test/java/net/runelite/client/plugins/itemcharges/ItemChargePluginTest.java(3 hunks)runelite-client/src/test/java/net/runelite/client/plugins/loottracker/LootTrackerBoxTest.java(0 hunks)runelite-jshell/pom.xml(1 hunks)runelite-maven-plugin/pom.xml(1 hunks)
💤 Files with no reviewable changes (1)
- runelite-client/src/test/java/net/runelite/client/plugins/loottracker/LootTrackerBoxTest.java
🧰 Additional context used
📓 Path-based instructions (4)
**/pom.xml
📄 CodeRabbit inference engine (AGENTS.md)
Target Java 11 via maven-compiler-plugin using 11 in all module POMs
Files:
pom.xmlcache/pom.xmlrunelite-api/pom.xmlrunelite-client/pom.xmlrunelite-maven-plugin/pom.xmlrunelite-jshell/pom.xml
**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
**/*.java: Use tabs for indentation, follow the brace placement style exemplified in MicrobotPlugin.java, and keep lines under 120 characters
Use UpperCamelCase for types and lowerCamelCase for members
Files:
runelite-client/src/main/java/net/runelite/client/plugins/itemstats/ItemStatChanges.javarunelite-client/src/main/java/net/runelite/client/plugins/worldhopper/WorldTypeFilter.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/Microbot.javarunelite-client/src/main/java/net/runelite/client/plugins/worldhopper/WorldTableRow.javarunelite-api/src/main/java/net/runelite/api/Quest.javarunelite-api/src/main/java/net/runelite/api/hooks/DrawCallbacks.javarunelite-api/src/main/java/net/runelite/api/gameval/NpcID.javarunelite-client/src/test/java/net/runelite/client/plugins/itemcharges/ItemChargePluginTest.javarunelite-api/src/main/java/net/runelite/api/gameval/VarClientID.javarunelite-api/src/main/java/net/runelite/api/gameval/SpriteID.javarunelite-api/src/main/java/net/runelite/api/NpcID.javarunelite-client/src/main/java/net/runelite/client/plugins/loottracker/LootTrackerRecord.javarunelite-client/src/main/java/net/runelite/client/plugins/loottracker/LootTrackerPlugin.javarunelite-api/src/main/java/net/runelite/api/widgets/Widget.javarunelite-client/src/main/java/net/runelite/client/plugins/loottracker/LootTrackerPanel.javarunelite-api/src/main/java/net/runelite/api/gameval/InterfaceID.javarunelite-client/src/main/java/net/runelite/client/plugins/itemcharges/ItemChargePlugin.javarunelite-api/src/main/java/net/runelite/api/gameval/DBTableID.javarunelite-api/src/main/java/net/runelite/api/gameval/VarbitID.javarunelite-client/src/main/java/net/runelite/client/plugins/loottracker/LootTrackerBox.java
runelite-client/src/main/java/net/runelite/client/plugins/microbot/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/**/*.java: Place all gameplay automation code under runelite-client/src/main/java/net/runelite/client/plugins/microbot
Prefix configuration interfaces with the plugin name (e.g., ExampleConfig) within microbot plugins
Files:
runelite-client/src/main/java/net/runelite/client/plugins/microbot/Microbot.java
runelite-client/src/test/java/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
runelite-client/src/test/java/**/*.java: Mirror production package structure for tests under runelite-client/src/test/java
Write JUnit 4 tests (junit:4.12) under matching package paths
Name test classes with the *Test suffix and split scenarios into focused @test methods asserting observable client state
Use Mockito (mockito-core:3.1.0) for client services and guice-testlib when event bus wiring is involved
Files:
runelite-client/src/test/java/net/runelite/client/plugins/itemcharges/ItemChargePluginTest.java
🧠 Learnings (2)
📚 Learning: 2025-10-16T17:14:29.745Z
Learnt from: CR
PR: chsami/Microbot#0
File: AGENTS.md:0-0
Timestamp: 2025-10-16T17:14:29.745Z
Learning: Applies to runelite-client/src/main/java/net/runelite/client/plugins/microbot/**/*.java : Place all gameplay automation code under runelite-client/src/main/java/net/runelite/client/plugins/microbot
Applied to files:
runelite-client/src/main/java/net/runelite/client/plugins/microbot/Microbot.java
📚 Learning: 2025-10-16T17:14:29.745Z
Learnt from: CR
PR: chsami/Microbot#0
File: AGENTS.md:0-0
Timestamp: 2025-10-16T17:14:29.745Z
Learning: Applies to runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/**/*.java : Put shared helpers under .../microbot/util
Applied to files:
runelite-client/src/main/java/net/runelite/client/plugins/microbot/Microbot.java
⏰ 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 (1)
runelite-client/src/main/java/net/runelite/client/plugins/itemcharges/ItemChargePlugin.java (1)
341-348: RoF break(+1) vs equipped-only decrement — verify event/order assumptionWith the new check requiring the ring to be worn, the break path still sets charges to 141 expecting the following USED message to decrement. If the ring is removed from WORN before that USED message arrives, the decrement is skipped and 141 persists. Please confirm the client’s message/item-container ordering guarantees WORN.contains(RING_OF_FORGING) remains true between BREAK and USED; otherwise, this flow is inconsistent and needs a fallback.
| default boolean zoneInFrustum(int zoneX, int zoneZ, int maxY, int minY) | ||
| { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
zoneInFrustum defaults to "cull all" and has inverted min/max parameter order
- tileInFrustum defaults to true (fail‑open), but zoneInFrustum defaults to false (fail‑closed). If the engine calls this when the flag is set but an implementation doesn’t override, all zones get culled.
- Parameter order is (maxY, minY), but elsewhere you use min→max (e.g., preSceneDraw: minLevel, …, maxLevel).
Suggest aligning both:
- default boolean zoneInFrustum(int zoneX, int zoneZ, int maxY, int minY)
+ default boolean zoneInFrustum(int zoneX, int zoneZ, int minY, int maxY)
{
- return false;
+ return true;
}Verification (check for any current usages before merging; expect none or easy fixes):
🏁 Script executed:
#!/bin/bash
# Find any usages/implementations to confirm arg order and default reliance
rg -nC2 -P '\bzoneInFrustum\s*\(' --type=javaLength of output: 955
zoneInFrustum defaults to false and has inverted parameter order
- Default return false means any un-overridden calls will cull all zones, contradicting tileInFrustum’s fail-open default (return true).
- Parameters are declared as (maxY, minY) but should follow the (minY, maxY) convention.
-default boolean zoneInFrustum(int zoneX, int zoneZ, int maxY, int minY)
+default boolean zoneInFrustum(int zoneX, int zoneZ, int minY, int maxY)
{
- 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.
| default boolean zoneInFrustum(int zoneX, int zoneZ, int maxY, int minY) | |
| { | |
| return false; | |
| } | |
| default boolean zoneInFrustum(int zoneX, int zoneZ, int minY, int maxY) | |
| { | |
| return true; | |
| } |
🤖 Prompt for AI Agents
In runelite-api/src/main/java/net/runelite/api/hooks/DrawCallbacks.java around
lines 119 to 122, the default implementation of zoneInFrustum currently returns
false and the y parameters are declared in the order (maxY, minY); change the
default to return true so un-overridden implementations are fail-open like
tileInFrustum, and flip the parameter order to (int zoneX, int zoneZ, int minY,
int maxY) (and update the method signature, javadoc/comments, and any call sites
to match the new order) so it follows the (minY, maxY) convention.
| // allocate a separate LootTrackerRecord for aggregate records to avoid later merge() calls | ||
| // mutating the session record | ||
| aggRecord = new LootTrackerRecord(eventName, subTitle, type, items, kills); | ||
| aggregateRecords.put(aggRecord, aggRecord); | ||
| } |
There was a problem hiding this comment.
Aggregate record still shares the session items array; deep-copy to prevent cross-record mutation
Comment says “allocate a separate LootTrackerRecord for aggregate records to avoid later merge() calls mutating the session record,” but this constructs aggRecord with the same LootTrackerItem[] and objects. Mutations (e.g., ignore toggles) will bleed between session and aggregate.
Deep-copy the items when creating aggRecord.
Apply this diff:
- aggRecord = new LootTrackerRecord(eventName, subTitle, type, items, kills);
+ LootTrackerItem[] itemsCopy = Arrays.stream(items)
+ .map(i -> new LootTrackerItem(
+ i.getId(), i.getName(), i.getQuantity(), i.getGePrice(), i.getHaPrice(), i.isIgnored()))
+ .toArray(LootTrackerItem[]::new);
+ aggRecord = new LootTrackerRecord(eventName, subTitle, type, itemsCopy, kills);📝 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.
| // allocate a separate LootTrackerRecord for aggregate records to avoid later merge() calls | |
| // mutating the session record | |
| aggRecord = new LootTrackerRecord(eventName, subTitle, type, items, kills); | |
| aggregateRecords.put(aggRecord, aggRecord); | |
| } | |
| // allocate a separate LootTrackerRecord for aggregate records to avoid later merge() calls | |
| // mutating the session record | |
| LootTrackerItem[] itemsCopy = Arrays.stream(items) | |
| .map(i -> new LootTrackerItem( | |
| i.getId(), i.getName(), i.getQuantity(), i.getGePrice(), i.getHaPrice(), i.isIgnored())) | |
| .toArray(LootTrackerItem[]::new); | |
| aggRecord = new LootTrackerRecord(eventName, subTitle, type, itemsCopy, kills); | |
| aggregateRecords.put(aggRecord, aggRecord); | |
| } |
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/loottracker/LootTrackerPanel.java
around lines 417 to 421, the aggregate record is constructed using the same
LootTrackerItem[] and item objects as the session record, so later mutations
(like toggling ignore) leak between records; fix by deep-copying the items array
and each LootTrackerItem when creating aggRecord: allocate a new
LootTrackerItem[] of the same length, iterate over the original items and create
a fresh LootTrackerItem for each entry (via an existing copy/clone constructor
or by copying fields into a new instance), then pass that new array into the
LootTrackerRecord constructor before putting aggRecord into aggregateRecords.
| private void initLoot(LootRecordType type, String name) | ||
| { | ||
| if (panel.hasRecord(type, name)) | ||
| { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Call panel.hasRecord on the EDT to avoid cross‑thread access to Swing state
initLoot() reads panel.hasRecord(...) from a non‑EDT thread. aggregateRecords is Swing‑owned; reading it off‑EDT risks races and stale visibility.
Wrap the check with invokeAndWait:
- if (panel.hasRecord(type, name))
- {
- return;
- }
+ final boolean alreadyLoaded;
+ if (SwingUtilities.isEventDispatchThread())
+ {
+ alreadyLoaded = panel.hasRecord(type, name);
+ }
+ else
+ {
+ final boolean[] res = new boolean[1];
+ try
+ {
+ SwingUtilities.invokeAndWait(() -> res[0] = panel.hasRecord(type, name));
+ }
+ catch (Exception e)
+ {
+ // fall through; treat as not loaded
+ }
+ alreadyLoaded = res[0];
+ }
+ if (alreadyLoaded)
+ {
+ return;
+ }📝 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.
| private void initLoot(LootRecordType type, String name) | |
| { | |
| if (panel.hasRecord(type, name)) | |
| { | |
| return; | |
| } | |
| private void initLoot(LootRecordType type, String name) | |
| { | |
| final boolean alreadyLoaded; | |
| if (SwingUtilities.isEventDispatchThread()) | |
| { | |
| alreadyLoaded = panel.hasRecord(type, name); | |
| } | |
| else | |
| { | |
| final boolean[] res = new boolean[1]; | |
| try | |
| { | |
| SwingUtilities.invokeAndWait(() -> res[0] = panel.hasRecord(type, name)); | |
| } | |
| catch (Exception e) | |
| { | |
| // fall through; treat as not loaded | |
| } | |
| alreadyLoaded = res[0]; | |
| } | |
| if (alreadyLoaded) | |
| { | |
| return; | |
| } |
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/loottracker/LootTrackerPlugin.java
around lines 651-656, initLoot() calls panel.hasRecord(...) off the EDT; wrap
the read in a synchronous EDT call (SwingUtilities.invokeAndWait or
EventQueue.invokeAndWait) to fetch the boolean result and then return if true.
Specifically, run a Runnable/Callable on the EDT that reads
panel.hasRecord(type, name) into a local boolean, handle/propagate
InterruptedException/InvocationTargetException, and use that boolean to decide
whether to return before proceeding with non-EDT work.
| items = Arrays.copyOf(items, items.length + 1); | ||
| items[items.length - 1] = item; | ||
| } | ||
|
|
||
| kills += record.kills; | ||
| } |
There was a problem hiding this comment.
Defensive-copy items to avoid aliasing between session and aggregate records
Appending the incoming item by reference (items[items.length - 1] = item) aliases LootTrackerItem across records. Subsequent ignore toggles or mutations on one record will leak into the other, contradicting the intent of isolated session vs aggregate state.
Use a copy when appending unmatched items.
Apply this diff:
- items = Arrays.copyOf(items, items.length + 1);
- items[items.length - 1] = item;
+ items = Arrays.copyOf(items, items.length + 1);
+ items[items.length - 1] = new LootTrackerItem(
+ item.getId(),
+ item.getName(),
+ item.getQuantity(),
+ item.getGePrice(),
+ item.getHaPrice(),
+ item.isIgnored()
+ );Optionally, add a small helper to centralize cloning:
// Place inside LootTrackerRecord
private static LootTrackerItem cloneItem(LootTrackerItem it) {
return new LootTrackerItem(it.getId(), it.getName(), it.getQuantity(), it.getGePrice(), it.getHaPrice(), it.isIgnored());
}🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/loottracker/LootTrackerRecord.java
around lines 92 to 97, the code appends the incoming LootTrackerItem by
reference which aliases items between records; change the append to store a
defensive copy of the item instead of the original reference (e.g., replace
items[items.length - 1] = item with a cloned/new LootTrackerItem created from
the incoming item's fields). Optionally add a private static helper
cloneItem(LootTrackerItem it) that constructs and returns a new
LootTrackerItem(it.getId(), it.getName(), it.getQuantity(), it.getGePrice(),
it.getHaPrice(), it.isIgnored()) and use that helper when appending unmatched
items.
4 participants
No description provided.