Implement beast of burden storage for summoning familiars#1042
Open
HarleyGilpin wants to merge 12 commits into
Open
Implement beast of burden storage for summoning familiars#1042HarleyGilpin wants to merge 12 commits into
HarleyGilpin wants to merge 12 commits into
Conversation
Re-lands the work from GregHib#1015 (by @maatheusgois-dd) with the two review comments from Greg addressed: - Drop the redundant familiar_leash timer (and the ensureFollowerNearby helper it drove). Follow mode already teleports a familiar back when it drifts >15 tiles every tick, so the timer duplicated that. The Follow teleport improvement and the callFollower mode-reset are kept. - Clarify ensureBeastOfBurdenInventory: the clear() call discards a stale undersized inventory instance so the engine recreates it at the definition size; the empty guard prevents losing stored items. Added a comment and tightened the condition. Implements Store / Take BoB for beast-of-burden familiars (e.g. pack yak) via the beast_of_burden (671) and summoning_side (722) interfaces, registers the beast_of_burden inventory (id 530, 6x5), wires examine handlers, and drops stored items to the floor on dismiss.
GregHib
reviewed
Jun 23, 2026
Greg noted the original PR could leave gaps in the beast of burden when depositing more items than were held. Root cause: when the requested amount exceeds the source count, MoveItemLimit's undo path (removed < added) removes the first `added` matches from the target — including items already present — then re-adds only `removed`, scattering the survivors into high slots and leaving empty gaps. A later deposit then fills those gaps, appearing to "skip" slots. Clamp the requested amount to what's actually held before moveToLimit in both store and withdraw, so removed == added and the undo path never runs. Adds BeastOfBurdenStoreLimitTest covering both directions.
Take BoB used moveAll, an atomic transaction that rolls back entirely if the inventory can't hold every carried item — so a player with fewer free slots than the familiar carried got "You don't have enough inventory space." and withdrew nothing. Use moveAllToLimit instead so the inventory fills up to capacity and the remainder stays on the familiar (still showing the full message when items are left behind), matching the single-item Withdraw behaviour. Adds BeastOfBurdenTakeTest covering the partial and full-fit cases.
When the familiar inventory is opened, reorganise the stored items so they fill the interface from the top with any empty slots pushed to the bottom, preserving order. Done in the beast_of_burden interfaceOpened handler so it covers every open path. Adds BeastOfBurdenCompactTest.
Storing only blocked a new item type once the first `capacity` slots were full, so storing more of the same non-stackable item (or stackables) could fill the whole shared 30-slot inventory — e.g. a war tortoise (18 slots) accepted up to 28, a thorny snail (3) accepted 5. Count all used slots and the remaining free slots against the familiar's capacity: items merging into an existing stack take no new slot, a new stack takes one, and each non-stackable item takes one. Clamp non-stackable stores to the free slots and tell the player when the limit is hit. The per-familiar capacities (thorny snail 3 … pack yak 30) already come from the cache NPC definitions. Adds BeastOfBurdenCapacityTest.
HarleyGilpin
force-pushed
the
feat/summoning/beast-of-burden
branch
from
4f2ed05 to
44c5e0c
Compare
Comparison against the RuneScape wiki surfaced four behaviours that were missing or diverged: - Only tradeable items can be stored. Reject untradeable/lent/dummy items in store(), mirroring the trade-screen restriction, with a message. - The familiar inventory can't be accessed in combat. Gate openBeastOfBurden() on the under_attack clock. - Dropped items are now owner-only and retrievable for five minutes before despawning (was: never despawn), and use the wiki's drop message. - A familiar slain in combat now drops its stored items and is dismissed. Tag the familiar NPC with its owner on summon and handle its death via npcDeath, reusing dismissFamiliar (without re-removing the despawning NPC). Adds BeastOfBurdenWikiTest. Out of scope (documented divergences): GE high-value rejection, abyssal essence carriers, and bank-interface access.
dropBeastOfBurdenItems() dropped onto the player's tile; on dismiss or familiar death the items should fall under the familiar instead. Use the follower's tile (falling back to the player if it's already gone). Updated the drop tests to place the familiar on a separate tile and assert the items land there, not under the player.
Wiki: a familiar can't carry an unstackable item worth more than 5m, nor a stack whose total value exceeds 5m. Check the item value (def["price", def.cost], same as the price checker) in store(): reject an unstackable item priced over the cap, and reject a stackable store whose resulting stack total (existing + stored) would exceed it. Coins are therefore capped at 5,000,000. Adds tests (red_partyhat over cap; coins at/over 5m).
GregHib
requested changes
Jun 23, 2026
GregHib
left a comment
GregHib
left a comment
Owner
There was a problem hiding this comment.
I think you should be able to use items on BoB familiars to store them? 🤔
- Move the 5m carry-value cap to game.properties (summoning.beastOfBurden.maxValue); add a Long accessor to Settings. - Inline familiarDef() to follower?.def. - Allow using an item on a familiar to store it (itemOnNPCOperate), reusing store() with its existing validation.
Contributor
Author
Added. Using an item on a familiar now stores it via |
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
2 participants
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.
Re-lands #1015 (originally by @maatheusgois-dd), which was closed with two unaddressed review comments. Both are now resolved. I also added familiar storage compaction when player opens familiar storage, which cause familiar held items to move to the top of the familiar's inventory, occupying any empty slots.
Summary
beast_of_burden(671) andsummoning_side(722) interfaces.beast_of_burdeninventory (id 530, 6×5) and wires examine handlers.Followteleport so familiars auto-call when they drift >15 tiles (skipped while in combat).Comparison against the RuneScape wiki (https://runescape.wiki/w/Beasts_of_Burden?oldid=4736259) surfaced several behaviors that were missing or diverged:
in store(), mirroring the trade-screen restriction, with a message.
openBeastOfBurden() on the under_attack clock.
despawning (was: never despawn), and use the wiki's drop message.
Tag the familiar NPC with its owner on summon and handle its death via
npcDeath, reusing dismissFamiliar (without re-removing the despawning NPC)
stack whose total value exceeds 5m.
Tests
./gradlew :game:test --tests "content.skill.summoning.BeastOfBurden*" --tests "content.skill.summoning.PackYakDefinitionTest"— all pass:BeastOfBurdenStoreTest— store/withdraw round-tripBeastOfBurdenZeroSizeTest— empty undersized inv recreated correctlyBeastOfBurdenDisplayTest— interface sync after storePackYakDefinitionTest— pack yak BoB capacity from NPC def