Limit Server Chests and register builders automatically

[jneb802]

Building a Server Chest automatically registers it to its builder. Each player can place one chest per world, including when their existing chest is unloaded. A dedicated placement RPC asks the server to verify the builder and reject duplicate chests. The client retries this request for up to 15 seconds if object synchronization has not arrived. Accepted repeat requests do not change the chest. The client blocks further chest placement while registration is pending.

The feature no longer patches ZDOMan.RPC_ZDOData or ZDO.Deserialize. Validation applies to clients that send the placement request; a modified client that bypasses it is outside this enforcement path.

The chest clones TreasureChest_dvergrtower directly. Its build cost is set to 10 wood with a workbench requirement. It keeps the ServerChest prefab identity and inventory format, disables treasure loot, treasure-discovery statistics, and empty-chest destruction, and generates its build icon from the clone. The wooden-chest dependency and repair code have been removed. Manual registration remains available for old unregistered chests.

Live validation passed on Valdev and Valnet client 02 with the deployed Season 8 8.0.29 runtime, Valheim 1.0.16, and the candidate on both sides:

• Building registered the chest automatically; seven coins arrived without manual registration.
• A second placement was blocked with the one-chest message.
• Registration and the seven coins survived restarting both sides.
• Reconnecting about 580 metres away still blocked placement; no objects were loaded at the original chest location.
• Destroying the original allowed a replacement, which registered automatically.
• A normal mouse-click build consumed exactly 10 wood with a workbench nearby. Normal hammer removal succeeded.

Validation found inherited treasure-discovery behavior sending a missing vanilla RPC when opening the clone. Setting m_discoverStat to None fixed it. After that change, placement, registration, delivery, and opening were repeated; both final logs contained zero exceptions, zero missing-RPC warnings, and zero Server Chest errors. Existing shader, headless-video, and other mod/config warnings remain. Earlier automation errors came from using an obsolete helper RPC or interacting with a build preview; neither appeared in the final run.

The build passed with zero errors and 84 existing warnings. This was a one-client validation; most placement checks used the normal Player.TryPlacePiece method with costs disabled, with a separate normal mouse-input cost/removal check. Screenshots and logs are retained outside the repository; no documentation or test files were added.

Cleanup: all test chests and the test workbench were removed. Original profile files, metadata, and links were restored and verified against their backup hashes. Both restored profiles loaded successfully. Valdev was returned to its stopped state. Valnet client 02 was powered off, and both device leases were released.

[github-actions]

Now I have enough context. Let me write the review.

---

No blocking issues found.

PR #114 — Limit Server Chests and register builders automatically

ServerChestIdentity.cs — else { return false }

The new branch fires only when peer == null && sender != ZNet.GetUID(), i.e. a sender that is neither a connected peer nor the local player. Both relevant callers (OnCountRequest, AcceptNewChest) pass empty strings for requestedName/requestedPlatformId, so the old code already returned false via the final !IsNullOrWhiteSpace check for this case. The change is correct and plugs a theoretical path where a caller supplying non-empty strings could previously slip through as an unidentified sender.

ServerChestPlacement.cs — placement-limit and auto-registration

CanPlace() / Update() / OnCountRequest / OnCountResponse flow — Clients poll the server every 2 s while the ServerChest piece is selected. The 6-second staleness window blocks placement and immediately re-requests; the Math.Max(localCount, serverCount) correctly catches chests in unloaded zones. On a listen server Update() skips the RPC path (IsServer() guard) and CanPlace() calls Count() directly against the full ZDO database. Both paths are sound.

IsSenderCreator — Correctly distinguishes the local player (sender == ZNet.GetUID() → check Player.m_localPlayer) from remote peers (ZDO lookup via peer.m_characterID).

ServerChestCreatorPatch — The __state = IsChest && GetCreator() == 0L guard fires only for a fresh placement, never for repeated SetCreator calls. On a listen server the local-player branch calls AcceptNewChest(zdo, ZNet.GetUID()); TryGetSenderIdentity resolves this via the else if (sender == ZNet.GetUID()) branch (local platform ID + game profile name), so the chest is accepted and registered correctly. Remote clients' pieces arrive through ServerChestDeserializePatch, not this patch, keeping the responsibility cleanly separated.

ServerChestReceivePatch + ServerChestDeserializePatch — The Prefix/Finalizer pattern safely pushes/pops ReceivingPeer around RPC_ZDOData. The GetPrefab() == 0 pre-deserialization gate correctly restricts AcceptNewChest to newly arriving ZDOs, not world-load replays or existing-ZDO updates. The redundant ReceivingPeer != null check in Postfix is harmless.

AcceptNewChest duplicate gate — The server-authoritative Count(creator, platform, zdo.m_uid) check excludes the ZDO being evaluated and covers both creator-ID and platform-lookup axes, so a client that places two chests in rapid succession will have the second rejected even if the client-side CanPlace() allowed both.

ServerChestPiece.cs — prefab change

Replacing piece_chest_wood with TreasureChest_dvergrtower removes the entire vanilla-chest-restore machinery, which is correctly cleaned up in ServerChestPatches.cs and ServerChest.cs (ForgetInventory was only used from the now-deleted TryRestoreVanillaWoodChest). Replacing m_defaultItems.m_drops.Clear() with new DropTable() and adding m_autoDestroyEmpty = false are correct — the treasure chest prefab would otherwise auto-destroy on empty.

Minor notes (non-blocking)

• The PR description flags that "the latest setup [prefab simplification] has not been retested in game." The render-icon path (RenderManager.Instance.Render) and CraftingStation/Requirements wiring in pieceConfig are not exercised by the build, but no code-level defect is apparent.
• Count() using OR over creator-ID and platform-lookup can count one chest matching both conditions only once (short-circuit), which is correct.

[github-actions]

No blocking issues found.

ServerChestPlacement.cs — new placement RPC

The core mechanics look correct. The chestId.UserID != sender check in OnPlacementRequest (line 346) prevents a client from sending a placement request for a ZDO they didn't create. The idempotency check in AcceptNewChest (lines 482–484) correctly returns true on repeated requests once the chest is already registered under the server session, guarding against double-registration or destruction of an accepted chest.

The Count function's OR logic (lines 384–386) is intentional: it catches chests found by ZDO creator field OR by registered platform owner, handling the case where an admin reassigns a chest to a different player than the one who placed it.

Client-side count validation

CanPlace combines a local ZDO scan with the server-polled _count via Math.Max (line 461). The local scan only covers loaded zones, so the server poll is needed for unloaded-zone chests — this is correctly handled. The 6-second staleness window (line 455) prevents placement while the count is out of date. Resetting _nextRequest = 0f on both staleness (line 458) and on placement response (line 362) triggers immediate re-polling in both cases.

ServerChestIdentity.cs change

The new else { return false; } (lines 37–40) makes the function reject unknown senders early. All existing callers in ServerChestRpc.cs use known peer or local UIDs; callers in the new placement code pass "" for both requested values, so the old fall-through path would also have returned false. No existing code regresses.

Timeout edge case

If the 15-second retry window expires without the server confirming the ZDO, _pendingChest is cleared but the orphaned chest remains on the server. Subsequent CanPlace calls still count it via ZDOVars.s_creator, blocking duplicate placement — so the player is stuck at one chest (requiring manual registration), consistent with the PR description.

Removal of vanilla wood chest restoration

The removal of TryRestoreVanillaWoodChest and its call site in ServerChestPatches.cs is safe because the plugin no longer patches piece_chest_wood. Players with chests from prior versions of the mod that were built on the old wood-chest prefab will not have them automatically restored; the PR does not address backwards-compatibility for those, which matches the stated scope.

Minor observation (non-blocking)

AcceptNewChest calls ZDOMan.instance.DestroyZDO (line 490) without an explicit null guard on ZDOMan.instance, relying on the earlier ZNet.instance.IsServer() check to guarantee it's non-null. This is a reasonable assumption for in-game execution but would throw if ever called outside that context.

[github-actions]

Now I have enough context for a complete review. Let me write the final analysis.

---

No blocking issues found.

ServerChestPlacement.cs — placement and registration flow

The new RPC handshake is logically sound. The Piece.SetCreator patch guards correctly: __state is only true when the creator slot was 0 before the call and equals the local player afterward, so registration fires exactly once per legitimate placement. The 15-second retry loop in UpdatePlacementRequest forces a ForceSendZDO on every attempt, which correctly handles the case where object-sync arrives after the first request.

AcceptNewChest ordering is correct. The "already accepted" path (zdo.GetOwner() == ZDOMan.GetSessionID() && OwnerLookup == NormalizeLookup(platform)) can only be reached after IsSenderCreator passes, which verifies the ZDO's s_creator field matches the sender's character ID. A chest registered to a different player would have a different OwnerLookup and fall through to the zdo.GetOwner() != sender guard (the server now owns it), correctly returning false.

ServerChestIdentity.TryGetSenderIdentity — the new else { return false; }

The new branch triggers only when FindPeerBySender returns null and the sender is not the local user. For the new placement path (always called with "", ""), behavior is identical to before — the prior fallthrough would have produced !IsNullOrWhiteSpace("") && !IsNullOrWhiteSpace("") = false anyway.

For manual registration (ServerChestService.RegisterChest), the caller provides non-empty requestedName and requestedPlatformId. If FindPeerBySender ever returns null for a still-connected remote peer (e.g. a timing gap between peer-list registration and RPC delivery), the server now returns "Could not resolve player identity." where it previously would have accepted the client-provided values. This is a security tightening — unverified identity data is rejected — but it narrows the fallback for manual registration in edge cases. Given the PR description explicitly preserves manual registration, monitor whether PlayerResolver.FindPeerBySender can transiently miss a peer on session join; if it can, the old chests workflow becomes unreliable during that window.

Count function — OR-logic between s_creator and OwnerLookup

Counting by either ZDO creator or registered platform ID is intentional: it catches both newly placed unregistered chests (matched by creator) and registered chests in unloaded zones (matched by platform). The exclude parameter correctly omits the chest being evaluated, so AcceptNewChest counts only pre-existing chests.

Client-side stale-count window

The 6-second _receivedAt freshness check means a player who destroys their chest and immediately selects the chest piece could be temporarily blocked with "please try again." _nextRequest = 0f in CanPlace forces an immediate refresh, so the window is bounded by one server round trip. This is correct behavior, not a bug.

OnPlacementRequest — chestId.UserID != sender guard

ZDOID.UserID holds the session ID of the ZDO's creator, which equals the RPC sender's session ID for a legitimately placed chest. This is a correct spoofing guard.

Prefab change from piece_chest_wood to TreasureChest_dvergrtower

The vanilla wood chest restoration and repair code (TryRestoreVanillaWoodChest, CaptureVanillaWoodChestShape, RestoreVanillaWoodChestPrefab) is fully removed. The old base prefab is no longer cloned or modified, so removing these is correct. Setting m_defaultItems = new DropTable(), m_autoDestroyEmpty = false, and m_discoverStat = PlayerStatType.None directly in ConfigurePrefab prevents the treasure-chest behaviors (loot generation, empty-chest self-destruction, the missing-RPC discovery stat) that would otherwise be inherited from the clone.