Add unlimited scrolling server chests (0.1.86)

[jneb802]

Server Chests currently reject deliveries beyond 64 stacks. This change removes that cap, adds rows as deliveries arrive, and uses the native inventory scrollbar. Version increases from 0.1.85 to 0.1.86.

The server chest keeps normal item stack limits and withdrawal-only player access. Deliveries refresh while the chest is open. Saves above 2,048 stacks use grouped native inventories to avoid the game's byte-sized row coordinates; smaller and existing saves retain the native format. Failed grouped reads do not replace live contents. Send and status commands return a failure response when chest contents cannot load; failed deliveries do not change ownership or save data. Ordinary containers keep their existing save/load behavior.

Feature validation completed before the command error-handling follow-up on Valdev and Valnet client 01 using the deployed Season 8 release 8.0.29 profile:

• Release build passed: zero errors, 84 existing warnings.
• Real-game save/load checks passed through 2,049 stacks, including item metadata and truncated-save rejection. A separate save-only check preserved 65,537 stacks.
• Live delivery, scrolling to row 257, withdrawals, blocked deposits, and ordinary chest layout passed.
• Server restart and client reconnect preserved all 2,049 stacks. Withdrawing back to 2,048 stacks also saved and loaded correctly.
• No ServerChest exceptions in the final test flow. Existing startup graphics/asset warnings remain.
• Test objects were removed, profiles and character restored, original file hashes verified, devices stopped, and leases released.

Install matching versions on server and clients. Older versions cannot read saves above 2,048 stacks. Memory, rendering, save size, and network traffic still limit practical capacity; a 1,985-item delivery took about 34 seconds in this environment.

Production was not changed.

Command error-handling follow-up validated live on Valdev with the deployed Season 8 release 8.0.30 on 2026-09-30. Before installing the candidate, all 48 plugin/patcher DLL hashes matched production. Normal RCON serverchest_status and serverchest_send commands returned clear failure messages for both truncated native and grouped saves. After each failed command, the fixture's stored bytes, ownership, and data revision were unchanged. A valid delivery of 10 Wood then succeeded and status reported the correct count. The temporary chest was removed. The server logged the four expected load warnings, with no unhandled command exceptions. Startup graphics warnings and Expand World Data configuration-generation warnings remained. Release build passed with zero errors and 84 existing warnings. No test or documentation files are included in this PR.

After validation, all 866 original profile files were restored and hash-verified. The temporary helper was removed. The restored server loaded PraetorisClient 0.1.82 and answered a player-count query, then was returned to its original stopped state.

[github-actions]

No blocking issues found.

The implementation correctly handles all the key behavioral surfaces:

Delivery path (ServerChestService.SendItems / TryAddItemAmount): ResizeToFit(inventory, 1) is called before each AddItem call in the while loop, ensuring m_height is always large enough for AddItem to find a free slot. Failure mid-delivery exits before SaveInventoryToZdo, leaving the ZDO unchanged. The long totalAmount cast (items.Sum(item => (long)item.Amount)) correctly prevents int overflow for large deliveries.

Save/load format boundary (ServerChestStorage): The chunk size of Columns * 256 = 2048 keeps each embedded vanilla inventory within 256 rows, respecting the byte-row constraint. Chests with ≤ 2048 stacks fall through to inventory.Save(package) unchanged (backwards-compatible). The staged-inventory approach in Load (decode into loaded, then bulk-replace inventory only on success) correctly prevents corrupt ZDO data from overwriting live contents.

Container.Load patch (ServerChestContainerLoadPatch): The first early return (DataRevision == lastRevision → return false without setting __result) matches what the vanilla Container.Load would return in the same case (nothing changed since last load). The remaining three exit paths that represent actual data state changes all set __result = true correctly. The ___m_loading = true guard prevents m_onChanged from triggering a recursive save when InventoryChanged.Invoke fires after loading items.

Container.Save patch: After calling SaveInventoryToZdo, chest.LoadedItemData is updated to the freshly-written bytes and lastRevision is updated to the new DataRevision, so the subsequent Load call on the same revision will hit the SequenceEqual fast path and return true with no re-decode.

Withdrawal path: Withdrawals fire Container.Save() via the Container's own m_onChanged subscription, which is intercepted by ServerChestContainerSavePatch and serialised with the custom format. The LoadedItemData / lastRevision cache is updated in the same Prefix, so the immediate follow-up Load call correctly short-circuits.

Exception re-throw in LoadInventoryFromZdo: The old code silently swallowed load errors and would have delivered items into an empty in-memory copy of a corrupt chest, overwriting it. The new re-throw propagates to the SendItems caller, preventing a silent data-loss delivery. This is a correct behavioral improvement.

Minor observation (not blocking): CHANGELOG.md has no entry for 0.1.85, going directly from 0.1.84 to 0.1.86. No behavior impact.

[github-actions]

Now I have a complete picture. Let me write the review.

---

Blocking issues found.

Behavior Bug: LoadInventoryFromZdo re-throw propagates as unhandled exception through service commands

File: Source/ServerChest/ServerChest.cs:220–224

The catch block in LoadInventoryFromZdo now re-throws after logging:

catch (Exception ex)
{
    PraetorisClientPlugin.Log.LogWarning("...");
    throw;  // added in this PR
}

Both callers in the service path—SendItems (ServerChestService.cs:108) and Status (ServerChestService.cs:133)—call LoadInventoryFromZdo with no surrounding try/catch:

// SendItems line 108
Inventory inventory = ServerChest.LoadInventoryFromZdo(zdo);

// Status line 133
Inventory inventory = ServerChest.LoadInventoryFromZdo(zdo);

Both methods return CommandResult. If LoadInventoryFromZdo throws (corrupt ZDO data, an InvalidDataException from the new chunked format, or a vanilla Inventory.Load failure), neither SendItems nor Status can return a CommandResult.Fail—the exception propagates up to the RPC/command handler. Depending on what the handler does with a raw exception from what it expects to be a CommandResult-returning function, this can crash the server-side command processing or silently drop the delivery with no user-facing error message.

Why the re-throw was added is clear and correct in intent: without it, a corrupt ZDO would result in LoadInventoryFromZdo returning an empty inventory, then SendItems delivering into that empty inventory and saving it back, destroying whatever was in the chest. The throw prevents that data loss. But the two service-command callers are not prepared to catch it.

The fix is to wrap the LoadInventoryFromZdo call in SendItems and Status with a try/catch that converts the exception into CommandResult.Fail, the same pattern used for every other failure in those methods.

---

Everything else in the diff looks correct:

• ResizeToFit called with additionalSlots = 1 before each AddItem in TryAddItemAmount ensures the inventory always has a free slot for the new stack before AddItem is called. Idempotent for items that merge into existing stacks.
• The ServerChestContainerLoadPatch try/finally correctly prevents a failed chunked load from updating chest.LoadedItemData or ___m_lastRevision, preserving live inventory contents on a bad read.
• The grouped-save format gate (items.Count <= ChunkSlots) and the chunk-count integrity check (chunk.NrOfItems() != Math.Min(ChunkSlots, count - start)) are both correct.
• The long cast for totalAmount (items.Sum(item => (long)item.Amount)) correctly avoids the int overflow that was possible at scale.
• Ordinary containers are unaffected: both new patches return true (run original) immediately when GetComponent<ServerChest>() is null.

[github-actions]

No blocking issues found.

ServerChest.cs — ResizeToFit arithmetic
checked(inventory.NrOfItems() + additionalSlots) prevents silent overflow; the ceiling-division expression slots / Columns + (slots % Columns == 0 ? 0 : 1) is correct.

ServerChestStorage.cs — format detection
FormatMarker = -1396918321 is highly unlikely to collide with the vanilla format's first int (a small version number). SetPos(position) correctly rewinds the package before falling back to vanilla Inventory.Load. Sound approach.

ServerChestStorage.cs — chunk validation
data.GetPos() != data.Size() || chunk.NrOfItems() != Math.Min(ChunkSlots, count - start) catches both under-reads and missing items before mutating the live inventory. The decode-then-commit pattern (all manipulation happens on loaded, then bulk-replaces the caller's inventory) upholds the PR's guarantee that failed reads leave live contents intact.

ServerChestStorage.cs — grid position reassignment
Items collected via GetAllItemsInGridOrder() per chunk are appended in serialization order, then global coordinates are assigned linearly. Both the vanilla and grouped paths produce the same canonical layout. Correct.

ServerChestStorage.cs — InventoryChanged.Invoke signature
AccessTools.Method(typeof(Inventory), "Changed") is called with new object[] { false, false }. If the current game version's Inventory.Changed() takes no parameters this throws TargetParameterCountException on every load, which the try/catch in LoadInventoryFromZdo (and both command handlers) would surface as a consistent failure — observable immediately in the live tests the PR reports passed. Accepted as matching the target API, but worth documenting as a version dependency.

ServerChestService.cs — partial-delivery safety
If TryAddItemAmount fails mid-loop, only the local inventory copy has been mutated; SaveInventoryToZdo is never called, so the ZDO data is unchanged. zdo.SetOwner persists regardless of delivery outcome, but that was also true in the previous implementation and is not a regression.

ServerChestService.cs — totalAmount promoted to long
items.Sum(item => (long)item.Amount) prevents overflow for very large deliveries. Correct improvement.

ServerChestContainerLoadPatch — DataRevision-unchanged early exit
When zdo.DataRevision == ___m_lastRevision, the prefix returns false without setting __result = true. All other non-passthrough paths explicitly set __result = true. This is the only asymmetry. The semantic distinction appears intentional: false means "no reload was necessary" rather than "reload failed." If any Valheim caller treats a false return as an error condition, the chest would never appear loaded during steady-state updates. The live test validates ordinary chest operation, so this does not appear to manifest, but the inconsistency warrants attention if callers are audited later.

ServerChestContainerLoadPatch — exception propagation on corrupt data
If ServerChestStorage.Load throws from the Container.Load patch context (UI path), the exception escapes the Harmony prefix unhandled. The PR's goal of preserving live contents is met because the decode-then-commit pattern means the inventory is not modified before the throw. The unhandled exception itself could surface as a game log error or container-UI failure on corrupt saves, but that is a pre-existing edge case, not a regression from this diff.

Ordinary containers
Both patches guard with if (chest == null) return true, so non-ServerChest containers are unaffected.