feat(death): Rs2Death API for grave and Death's Office recovery - #1834
Open
infuse21 wants to merge 8 commits into
Open
feat(death): Rs2Death API for grave and Death's Office recovery#1834infuse21 wants to merge 8 commits into
infuse21 wants to merge 8 commits into
Conversation
Fix the walker
Adds util/death/Rs2Death, a static facade for handling a normal death: locate the grave (an NPC, ids 9856-10367), loot free and paid items, and optionally fall back to Death's Office once the grave expires. Death's Office recovery is opt-in — its fee is unreadable before it is charged, so it is never entered without an explicit flag. Scripts poll hasDeathToHandle() and drive recoverItems(budget[, office]), or compose the primitives directly. No automatic behaviour and no config coupling: callers pass plain scalars, matching Rs2Bank/Rs2Walker. Also reads the "Items Kept on Death" panel for the game's own numbers (getPredictedGraveFee, getRiskValue, getItemsKeptOnDeath) and estimates the office fee from wiki prices per the confirmed per-unit 100k rule. Verified against a live client: grave/office interface groups and components, the entrance object and reclaim dialogue, the GRAVESTONE_* varbit encodings (VISIBLE is non-zero not boolean; DURATION is ticks), and that both grave and office charge on per-unit value, not stack or cumulative. Details and footguns in docs/entity-guides/death.md. Wires onActorDeath/onVarbitChanged in MicrobotPlugin and regenerates the client-thread guardrail baseline for the two event handlers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… low Checked the recovery rules against the OSRS wiki's own tables. Both schedules are confirmed as implemented: a grave charges flat coin amounts per item by tier (1k / 10k / 100k for 100k-1m / 1m-10m / 10m+) capped at 500k, and Death's Office charges a flat 5% on items worth 100k or more, uncapped; ironmen get 50% off both. The wiki says "each reclaimed item", matching the per-unit behaviour observed in game (862 coal at 146 each reclaimed free from both grave and office). It also documents exceptions "to which the above rules do not neatly apply" — notably that stacks of amulet of glory (6) worth over 100,000 are charged 10% at Death's Office, double the rate and assessed on the stack's value rather than per unit. estimateReclaimFee applies the per-unit rule, so it predicts free for such a stack and reads LOW — the one direction a ceiling must not fail, since reclaimAll(maxEstimatedFee) spends real gold against it. Deliberately not special-cased: hardcoding glory would imply the exception list is complete, and the wiki states it is not. Instead estimateReclaimFee and reclaimAll(int) now state plainly that the estimate can read low, name glory as the known case, and tell callers to leave real headroom rather than treat the ceiling as a guarantee. Also removes a stale "biased high" claim and a duplicated javadoc block. Still unobserved in game: an actual non-zero charge. Only the free-below-100k case has been watched happen. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Removes estimateReclaimFee() and reclaimAll(int) — about 80 lines, over half of it caveats, with no callers outside the class. The office never publishes its fee before charging, so any estimate is a guess, and this one guessed LOW on documented exceptions: a stack of amulet of glory (6) over 100,000 is charged 10% on the stack rather than 5% per unit, so it is billed where the per-unit rule predicts free. A ceiling that can be quietly exceeded is worse than no ceiling, because reclaimAll(maxEstimatedFee) spent real gold against it. Special- casing glory was rejected: the wiki states the exception list is not exhaustive, so hardcoding one entry would imply a completeness that cannot be verified. What remains covers the same ground honestly: getPredictedGraveFee() reads the figure the game itself computed on the Items Kept on Death panel, reclaimAll() is unbounded and says so, and walk/enter/open plus closeInterfaces() let a script inspect the office and decline without paying — the trip is free, only the reclaim costs. A script that insists on its own cap can price the contents itself and owns that assumption. The fee schedules move from dead constants into the class javadoc as reference, since nothing computes them any more. Also corrects DeathsOfficeLocation's provenance note using the wiki's map data: every x matches exactly and every y sits a constant two tiles south of the wiki figure (four at Lumbridge, the one entry verified in-game against the real object). A uniform offset on the entry with known ground truth indicates the wiki centres its map north of the object, so these coordinates are the better estimate. Immaterial either way — enterDeathsOffice() resolves the entrance by id, never by coordinate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Every looting path was Take-All: lootGraveFreeItems/lootGravePaidItems click the grave's section buttons and reclaimAll clicks the office's, so a script that only wanted its gear back had to take everything or nothing. The interfaces support picking individual slots; the API did not expose it. Adds: - getGraveFreeItems / getGravePaidItems / getDeathsOfficeItems — read what is waiting, with slot indices preserved (the slot is the param0 needed to click it). - lootGraveItems(Predicate) — takes matching slots from both halves via the per-slot Take action. - reclaimItems(Predicate) — takes matching slots from the office. The office selects first and only then reveals its quantity buttons, so each slot is a two-step click: Select, wait for ALL to become visible, then ALL. Slots are clicked highest-index first, because taking one re-packs the container and would otherwise invalidate the indices still to come. Both paths stop when the inventory fills rather than clicking into a full backpack. Also makes the Take-All paths report what they left behind: the office holds up to 120 stacks against 28 inventory slots, so a full reclaim can simply not fit. Nothing is lost there — Death keeps the remainder indefinitely — but a grave expires, so the grave warning includes the time left on the timer. Note the asymmetry, documented on the methods: the office charges per item reclaimed so taking less costs less, whereas a grave's fee covers its whole paid half at once. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/entity-guides/death.md`:
- Around line 20-23: Update the Death’s Office example around
Rs2Death.reclaimAll() to inspect the reclaim fee before committing: call the
appropriate inspection action and close the interfaces without reclaiming, or
only invoke reclaimAll() after an explicit caller decision to accept the
uncapped fee.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/death/DeathsOfficeLocation.java`:
- Around line 56-61: Update DeathsOfficeLocation.getNearest to compare each
entrance’s horizontal distance using distanceTo2D(from) instead of
distanceTo(from), while preserving the existing null handling and
nearest-location selection.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/death/Rs2Death.java`:
- Around line 459-466: Move the “Claims the half of the grave that costs
nothing” Javadoc block so it immediately documents lootGraveFreeItems, leaving
getGraveFreeItems with only its item-list description. Move the reclaimAll
Javadoc block, including its spending-limit and return details, from before
getDeathsOfficeItems to immediately before reclaimAll, without changing either
method’s implementation.
- Around line 705-748: The reclaimItems method currently always reads and
interacts with the DeathOffice container; update it to resolve the container and
quantity-button interfaces based on the active retrieval variant, matching
reclaimAll()’s logic. Ensure GravestoneRetrieval uses its corresponding
container and controls while preserving DeathOffice behavior, filtering,
inventory checks, and reverse-order processing.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 42d93be9-6e1c-47ea-a81b-8b2e7d7cfcaa
📒 Files selected for processing (6)
docs/entity-guides/README.mddocs/entity-guides/death.mdrunelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotPlugin.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/death/DeathsOfficeLocation.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/util/death/Rs2Death.javarunelite-client/src/test/resources/threadsafety/client-thread-guardrail-baseline.txt
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>
getDeathsOfficeItems always read InterfaceID.DeathOffice.ITEMS, but isDeathsOfficeOpen accepts either retrieval variant. With the GravestoneRetrieval variant up it returned an empty list, so an office still holding items looked empty — both to callers inspecting it and to reclaimAll's inventory-full warning, which would report "still holds 0 item(s)" while items remained. It now resolves the container from whichever interface is visible, matching how reclaimAll already picks between takeall and button. reclaimItems is unaffected: its guard has already established that the DeathOffice variant is the open one before it reads anything. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
ℹ️ Skretzo pushed Evidence it is not caused by these changes:
I deliberately have not made it green. The tracked scope declares Suggested handling: merge this on
🤖 Posted by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
Rs2Death— a static utility for handling a normal death: find the grave, loot it,and optionally reclaim from Death's Office once the grave has expired.
Microbot currently has no death handling beyond
DeathEvent, which only covers theone-off first-death Death's Domain tutorial. Every script that can die either breaks or
reimplements recovery itself. This fills that gap.
Usage
Nothing runs automatically. A script polls and decides:
recoverItemsand thelootGrave*/reclaimAllmethods take everything. To takeonly some of it, inspect and filter:
getGraveFreeItems(),getGravePaidItems()andgetDeathsOfficeItems()show what iswaiting. The office charges per item reclaimed so taking less costs less, whereas a
grave's fee covers its whole paid half at once — and anything left in a grave is only
safe until the timer expires.
Death's Office is opt-in (
recoverItems(budget, true)) because its fee is uncapped andunreadable before it charges. Primitives (
walkToGrave,openGrave,getGraveFee,lootGraveFreeItems,lootGravePaidItems,walkToDeathsOffice,enterDeathsOffice,reclaimAll,closeInterfaces) are public for scripts that want their own flow.Also reads the game's own numbers off the Items Kept on Death panel:
getPredictedGraveFee(),getRiskValue(),getItemsKeptOnDeath().Banking and re-gearing are deliberately absent. Scripts already have banking state;
a second one here would fight it. The expected flow is
recover → bank → resupply from inventory setup → grind, and this API owns only step one.
Testing
./gradlew :client:runUnitTestsgreen. The 5 new guardrail-baseline entries are the two@Subscribehandlers, which the client fires on the client thread — same pattern asthe existing
Rs2Player.handleAnimationChanged/Rs2RunePouch.onVarbitChangedentries.Known gaps
Takeon grave slots,Selecton office slots) were observed live, but driving them has not been exercisedin game. The Take-All paths are live-verified end to end.
proven; rates are wiki-confirmed.