-
-
Notifications
You must be signed in to change notification settings - Fork 486
Bank Fixes + Update latest runelite #1667
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
lost widget [bank]group_storage_button 12.8
[ci skip]
# Replaced reference to int BANK_ITEM_CONTAINER to a local int BANK_ITEM_CONTAINER with updated working value. ## Fixed Rs2Bank.isOpen() method. Now correctly identifies if bank is open properly again. ### May not fix all banking issues for other banking situations.
(fix) Basic Rs2Bank fix
# Conflicts: # gradle.properties
Add arkan blade to SpecialAttackWeaponEnum
WalkthroughThis pull request updates the RuneLite API and client to version 1.12.14, introducing new game data constants for group ironman game modes, sailing facility items, and facility bottles. It reindexes bank interface field IDs in the interface definitions and refactors the bank plugin to use updated script argument counts and new event handlers for popup tab drawing. Multiple skill calculator actions are added for new resources (camphor, ironwood, rosewood logs and related items), and the world hopper plugin gains TCP retransmit loss tracking on Windows. Build and microbot versions are incremented accordingly. Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.40.5)runelite-api/src/main/java/net/runelite/api/ItemID.javarunelite-api/src/main/java/net/runelite/api/gameval/ItemID.javarunelite-api/src/main/java/net/runelite/api/NullItemID.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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@runelite-api/src/main/java/net/runelite/api/gameval/ItemID.java`:
- Around line 86629-86637: The file adds SAILING_FACILITY_BOTTLE_EMPTY and
SAILING_FACILITY_BOTTLE_FULL but the Cert/Placeholder groups only define the
EMPTY variant, causing incomplete mappings; verify the canonical source for
cert/placeholder IDs for the FULL variant (and for the other occurrences
referenced at IDs 91219-91220 and 100672-100673) and either add the missing
SAILING_FACILITY_BOTTLE_FULL cert/placeholder constants with the correct IDs to
the corresponding Cert/Placeholder sections or, if no cert/placeholder exists,
remove or document the FULL top-level constant to keep tiered sets consistent
(search for SAILING_FACILITY_BOTTLE_EMPTY, SAILING_FACILITY_BOTTLE_FULL and the
Cert/Placeholder blocks to locate the spots to edit).
In
`@runelite-client/src/main/java/net/runelite/client/plugins/worldhopper/WorldHopperPingOverlay.java`:
- Around line 79-80: The text height calculation in WorldHopperPingOverlay uses
fm.getAscent() - fm.getDescent(), which underestimates the height; update the
calculation where FontMetrics fm is obtained (in WorldHopperPingOverlay, near
the render/paint code using fm) to use fm.getAscent() + fm.getDescent() so
textHeight reflects the full ascent+descent. Ensure any downstream layout that
uses textHeight continues to work with the corrected value.
In
`@runelite-client/src/main/java/net/runelite/client/util/ExecutorServiceExceptionLogger.java`:
- Around line 80-90: The one-shot schedule(Runnable, long, TimeUnit) and
schedule(Callable<V>, long, TimeUnit) overloads in
ExecutorServiceExceptionLogger currently delegate directly to
service.schedule(...) and therefore bypass the monitor(...) wrappers used by
scheduleAtFixedRate and scheduleWithFixedDelay; update the schedule(...)
implementations to pass the wrapped tasks (use monitor(command) for Runnable and
the Callable equivalent, e.g., monitor(callable) or a monitorCallable helper)
into service.schedule(...) so all scheduled tasks go through the same
exception-logging monitor in ExecutorServiceExceptionLogger.
| /** | ||
| * Facility bottle (empty) | ||
| */ | ||
| public static final int SAILING_FACILITY_BOTTLE_EMPTY = 33074; | ||
|
|
||
| /** | ||
| * Facility bottle (full) | ||
| */ | ||
| public static final int SAILING_FACILITY_BOTTLE_FULL = 33077; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent variant coverage for facility bottle IDs
Top-level adds both EMPTY and FULL, but Cert/Placeholder only add EMPTY. If FULL has cert/placeholder IDs, they’re missing and callers can’t map them. Please confirm the source data and either add FULL entries under Cert/Placeholder (with the correct IDs) or document/remove the FULL variant to keep the tiered sets consistent.
Also applies to: 91219-91220, 100672-100673
🤖 Prompt for AI Agents
In `@runelite-api/src/main/java/net/runelite/api/gameval/ItemID.java` around lines
86629 - 86637, The file adds SAILING_FACILITY_BOTTLE_EMPTY and
SAILING_FACILITY_BOTTLE_FULL but the Cert/Placeholder groups only define the
EMPTY variant, causing incomplete mappings; verify the canonical source for
cert/placeholder IDs for the FULL variant (and for the other occurrences
referenced at IDs 91219-91220 and 100672-100673) and either add the missing
SAILING_FACILITY_BOTTLE_FULL cert/placeholder constants with the correct IDs to
the corresponding Cert/Placeholder sections or, if no cert/placeholder exists,
remove or document the FULL top-level constant to keep tiered sets consistent
(search for SAILING_FACILITY_BOTTLE_EMPTY, SAILING_FACILITY_BOTTLE_FULL and the
Cert/Placeholder blocks to locate the spots to edit).
| final FontMetrics fm = graphics.getFontMetrics(); | ||
| final int textHeight = fm.getAscent() - fm.getDescent(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent text height calculation.
The formula fm.getAscent() - fm.getDescent() produces a value smaller than the actual text height. Standard text height should be fm.getAscent() + fm.getDescent() (both are positive values representing distance from the baseline).
Proposed fix
final FontMetrics fm = graphics.getFontMetrics();
- final int textHeight = fm.getAscent() - fm.getDescent();
+ final int textHeight = fm.getAscent() + fm.getDescent();📝 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 FontMetrics fm = graphics.getFontMetrics(); | |
| final int textHeight = fm.getAscent() - fm.getDescent(); | |
| final FontMetrics fm = graphics.getFontMetrics(); | |
| final int textHeight = fm.getAscent() + fm.getDescent(); |
🤖 Prompt for AI Agents
In
`@runelite-client/src/main/java/net/runelite/client/plugins/worldhopper/WorldHopperPingOverlay.java`
around lines 79 - 80, The text height calculation in WorldHopperPingOverlay uses
fm.getAscent() - fm.getDescent(), which underestimates the height; update the
calculation where FontMetrics fm is obtained (in WorldHopperPingOverlay, near
the render/paint code using fm) to use fm.getAscent() + fm.getDescent() so
textHeight reflects the full ascent+descent. Ensure any downstream layout that
uses textHeight continues to work with the corrected value.
| @Override | ||
| public ScheduledFuture<?> scheduleAtFixedRate(Runnable command, long initialDelay, long period, TimeUnit unit) | ||
| { | ||
| return service.scheduleAtFixedRate(monitor(command), initialDelay, period, unit); | ||
| } | ||
|
|
||
| @Override | ||
| public ScheduledFuture<?> scheduleWithFixedDelay(Runnable command, long initialDelay, long delay, TimeUnit unit) | ||
| { | ||
| return service.scheduleWithFixedDelay(monitor(command), initialDelay, delay, unit); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent wrapping: schedule() methods are not monitored.
The new scheduleAtFixedRate and scheduleWithFixedDelay methods correctly wrap commands with monitor(), but the schedule(Runnable, ...) and schedule(Callable, ...) methods at lines 149-158 still delegate directly without wrapping. This defeats exception logging for one-shot scheduled tasks.
🔧 Proposed fix to wrap schedule methods consistently
`@Override`
public ScheduledFuture<?> schedule(Runnable command, long delay, TimeUnit unit)
{
- return service.schedule(command, delay, unit);
+ return service.schedule(monitor(command), delay, unit);
}
`@Override`
public <V> ScheduledFuture<V> schedule(Callable<V> callable, long delay, TimeUnit unit)
{
- return service.schedule(callable, delay, unit);
+ return service.schedule(monitor(callable), delay, unit);
}Also applies to: 148-158
🤖 Prompt for AI Agents
In
`@runelite-client/src/main/java/net/runelite/client/util/ExecutorServiceExceptionLogger.java`
around lines 80 - 90, The one-shot schedule(Runnable, long, TimeUnit) and
schedule(Callable<V>, long, TimeUnit) overloads in
ExecutorServiceExceptionLogger currently delegate directly to
service.schedule(...) and therefore bypass the monitor(...) wrappers used by
scheduleAtFixedRate and scheduleWithFixedDelay; update the schedule(...)
implementations to pass the wrapped tasks (use monitor(command) for Runnable and
the Callable equivalent, e.g., monitor(callable) or a monitorCallable helper)
into service.schedule(...) so all scheduled tasks go through the same
exception-logging monitor in ExecutorServiceExceptionLogger.
Bank Fixes + Update latest runelite