Skip to content

Commit e317b5d

Browse files
infuse21claude
andcommitted
fix(death): address review findings
Three of the four findings were valid: 1. getNearest used WorldPoint.distanceTo, which returns Integer.MAX_VALUE across planes. Every entrance is on plane 0, so a player on any upper floor scored MAX_VALUE for all eight and min() silently returned the first constant — Lumbridge — however far away it was. Switched to distanceTo2D. 2. Two Javadoc blocks were orphaned when the item-reader methods were inserted ahead of the methods they described: the grave "claims the half that costs nothing" block landed on getGraveFreeItems, and the detailed reclaimAll block (spending-limit rationale and @return) landed on getDeathsOfficeItems. Both moved to the methods they document; no implementation change. 3. The Death's Office example in the guide called reclaimAll() unconditionally under a comment about pricing the office first — stale since the fee estimator was removed. It now reads the contents, leaves the decision to the caller, and shows closeInterfaces() as the free way to decline. The fourth — make reclaimItems resolve container and quantity buttons per retrieval variant, mirroring reclaimAll — is not implementable as described. Confirmed against the game cache (iftypes): death_office (669) has 1/5/x/all/takeall, while gravestone_retrieval (602) has no quantity controls at all, only button / button_bank / discard. There is nothing to resolve to. The real defect underneath it was that reclaimItems read the DeathOffice container unconditionally even though isDeathsOfficeOpen accepts either variant, so on 602 it would read an empty container and report "took nothing". It now detects the variant and fails loudly, pointing the caller at reclaimAll(). Both interfaces' component lists are documented in the guide. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 5971e54 commit e317b5d

3 files changed

Lines changed: 51 additions & 26 deletions

File tree

docs/entity-guides/death.md

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,15 @@ if (Rs2Death.hasDeathToHandle()) {
1717
// opt in to the Death's Office trip as well, if the script wants expired items back
1818
Rs2Death.recoverItems(config.deathBudget(), config.useDeathsOffice());
1919

20-
// or price the office yourself before committing — the trip is free, only the reclaim costs
20+
// or inspect before committing — walking there is free, only the reclaim costs
2121
if (Rs2Death.walkToDeathsOffice() && Rs2Death.enterDeathsOffice() && Rs2Death.openDeathsOffice()) {
22-
Rs2Death.reclaimAll(); // no cap is possible — see rule 11
23-
Rs2Death.closeInterfaces(); // or inspect first and close to decline without paying
22+
List<Rs2ItemModel> waiting = Rs2Death.getDeathsOfficeItems();
23+
24+
if (worthReclaiming(waiting)) { // the script's own call — see rule 11, no cap is possible
25+
Rs2Death.reclaimAll(); // takes everything, at whatever it costs
26+
// or: Rs2Death.reclaimItems(i -> i.getName().contains("rune"));
27+
}
28+
Rs2Death.closeInterfaces(); // declining is free; Death keeps them indefinitely
2429
}
2530
```
2631

@@ -353,6 +358,18 @@ What to use instead:
353358
reclaim costs. A script that insists on its own cap can price the contents itself and owns that
354359
assumption.
355360

361+
**Two different retrieval interfaces exist, and only one supports selective taking.** Confirmed against
362+
the game cache (`iftypes`):
363+
364+
| Group | Components | Selective? |
365+
|---|---|---|
366+
| `death_office` (669) | `items`, **`1` `5` `x` `all`**, `takeall`, `info` | yes — select a slot, then a quantity |
367+
| `gravestone_retrieval` (602) | `items`, `button`, `button_bank`, `discard`, `fee`, `info` | **no quantity controls at all** |
368+
369+
`isDeathsOfficeOpen()` accepts either, so `reclaimItems(filter)` checks which one is actually up and
370+
refuses on 602 rather than reading the wrong container and reporting "took nothing". `reclaimAll()`
371+
handles both, clicking `takeall` or `button` as appropriate.
372+
356373
**Where this applies:** `Rs2Death.reclaimAll`, `Rs2Death.recoverItems`.
357374

358375
## 12. Death's Office needs the entrance object, then a dialogue — not an NPC click

runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/death/DeathsOfficeLocation.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,11 @@ public static DeathsOfficeLocation getNearest() {
5555

5656
public static DeathsOfficeLocation getNearest(WorldPoint from) {
5757
if (from == null) return null;
58+
// distanceTo2D, not distanceTo: the latter returns Integer.MAX_VALUE across planes, and every
59+
// entrance is on plane 0. A player upstairs would score MAX_VALUE for all of them, so min()
60+
// would silently return the first constant (Lumbridge) however far away it is.
5861
return Arrays.stream(values())
59-
.min(Comparator.comparingInt(location -> location.entrance.distanceTo(from)))
62+
.min(Comparator.comparingInt(location -> location.entrance.distanceTo2D(from)))
6063
.orElse(null);
6164
}
6265
}

runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/death/Rs2Death.java

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -456,9 +456,6 @@ public static int getGraveFee() {
456456
return parseFee(Rs2Widget.getWidget(InterfaceID.GravestoneGeneric.FEE));
457457
}
458458

459-
/**
460-
* Claims the half of the grave that costs nothing. Items behind a fee are untouched and stay put.
461-
*/
462459
/**
463460
* The items in the grave's free half — everything that costs nothing to reclaim. Requires the grave
464461
* interface to be open ({@link #openGrave()}).
@@ -476,8 +473,8 @@ public static List<Rs2ItemModel> getGravePaidItems() {
476473
}
477474

478475
/**
479-
* Takes <b>everything</b> in the free half. Use {@link #lootGraveItems(Predicate)} to take only some
480-
* of it.
476+
* Takes <b>everything</b> in the free half; items behind the fee are untouched and stay put. Use
477+
* {@link #lootGraveItems(Predicate)} to take only some of it.
481478
*/
482479
public static boolean lootGraveFreeItems() {
483480
if (!isGraveOpen()) return false;
@@ -682,21 +679,6 @@ private static void advanceReclaimDialogue() {
682679
}
683680
}
684681

685-
/**
686-
* Reclaims everything Death is holding, into the inventory. Death's Office keeps items
687-
* indefinitely, so a partial reclaim caused by a full inventory is safe to resume later.
688-
* <p>
689-
* <b>There is deliberately no spending limit, because one is not possible.</b> The fee is never on
690-
* screen before it is charged — verified live, {@code INFO} reads "Select an item to retrieve."
691-
* whether the office is empty or holding items, the {@code 1}/{@code 5}/{@code X}/{@code All}
692-
* buttons stay hidden until an item is selected, and {@code Take-All} never selects. Any cap here
693-
* would be fiction.
694-
* <p>
695-
* Calling this authorises an unbounded charge against Death's Coffer, and the bank after that.
696-
* Death's Office holds items indefinitely, so declining to call it is always a safe alternative.
697-
*
698-
* @return {@code true} once the retrieval interface has closed with nothing left to collect.
699-
*/
700682
/**
701683
* The items Death is currently holding. Requires the retrieval interface to be open
702684
* ({@link #openDeathsOffice()}) — the office cannot be inspected from afar, though walking there and
@@ -723,6 +705,17 @@ public static List<Rs2ItemModel> getDeathsOfficeItems() {
723705
public static int reclaimItems(Predicate<Rs2ItemModel> filter) {
724706
if (!isDeathsOfficeOpen()) return 0;
725707

708+
// Selective reclaim is DeathOffice-only. isDeathsOfficeOpen also accepts the
709+
// GravestoneRetrieval variant, but that interface has no per-quantity controls at all — its
710+
// components are BUTTON / BUTTON_BANK / DISCARD, with no 1/5/X/All — so the select-then-take
711+
// flow below has nothing to click there. Fail loudly rather than reading the wrong container
712+
// and silently reporting "took nothing".
713+
if (!Rs2Widget.isWidgetVisible(InterfaceID.DeathOffice.ITEMS_CONTAINER)) {
714+
log.warn("Selective reclaim needs the Death's Office interface; the retrieval-service "
715+
+ "variant has no quantity controls. Use reclaimAll() instead.");
716+
return 0;
717+
}
718+
726719
List<Rs2ItemModel> items = getDeathsOfficeItems();
727720
int taken = 0;
728721
for (int i = items.size() - 1; i >= 0; i--) {
@@ -748,9 +741,21 @@ public static int reclaimItems(Predicate<Rs2ItemModel> filter) {
748741
}
749742

750743
/**
751-
* Reclaims <b>everything</b> Death is holding. Use {@link #reclaimItems(Predicate)} to take only
752-
* some of it.
744+
* Reclaims everything Death is holding, into the inventory. Death's Office keeps items
745+
* indefinitely, so a partial reclaim caused by a full inventory is safe to resume later.
746+
* <p>
747+
* <b>There is deliberately no spending limit, because one is not possible.</b> The fee is never on
748+
* screen before it is charged — verified live, {@code INFO} reads "Select an item to retrieve."
749+
* whether the office is empty or holding items, the {@code 1}/{@code 5}/{@code X}/{@code All}
750+
* buttons stay hidden until an item is selected, and {@code Take-All} never selects. Any cap here
751+
* would be fiction.
752+
* <p>
753+
* Calling this authorises an unbounded charge against Death's Coffer, and the bank after that.
754+
* Death's Office holds items indefinitely, so declining to call it is always a safe alternative.
755+
*
756+
* @return {@code true} once the retrieval interface has closed with nothing left to collect.
753757
*/
758+
754759
public static boolean reclaimAll() {
755760
if (!isDeathsOfficeOpen()) return false;
756761

0 commit comments

Comments
 (0)