Skip to content

server/session: Resend inventories on item release to keep the client in sync - #1281

Closed
root-nat wants to merge 1 commit into
df-mc:masterfrom
root-nat:fix/release-item-inventory-sync
Closed

server/session: Resend inventories on item release to keep the client in sync#1281
root-nat wants to merge 1 commit into
df-mc:masterfrom
root-nat:fix/release-item-inventory-sync

Conversation

@root-nat

Copy link
Copy Markdown

When a player releases an item such as a bow, the client predicts the result and, for example, decrements the arrow it expects to be consumed. If the server does not actually consume anything (the release is cancelled, the bow is not drawn long enough, or there are no arrows), it never corrected the client, leaving the displayed inventory out of sync with the server.

The inventories are now resent before handling the release in both release paths, mirroring what handleUseItemTransaction already does for item use. The slot updates from an actual consumption are still sent afterwards, so a successful shot ends up with the correct count either way.

Fixes #1001.

🤖 Generated with Claude Code

… in sync

When a player releases an item such as a bow, the client predicts the result
and, for example, decrements the arrow it expects to be consumed. If the
server does not actually consume anything (the release is cancelled, the bow
is not drawn long enough, or there are no arrows), it never corrected the
client, leaving the displayed inventory out of sync with the server.

Resend the inventories before handling the release in both release paths,
mirroring what handleUseItemTransaction already does for item use. The
existing slot updates from an actual consumption are still sent afterwards,
so a successful shot ends up with the correct count either way.

Fixes df-mc#1001.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
// handleReleaseItemTransaction ...
func (h *InventoryTransactionHandler) handleReleaseItemTransaction(c Controllable) error {
func (h *InventoryTransactionHandler) handleReleaseItemTransaction(s *Session, c Controllable) error {
// The client predicts the result of releasing an item, such as an arrow being removed from the

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment could be 1 line

case protocol.PlayerActionStartItemUseOn:
// TODO: Properly utilize these actions.
case protocol.PlayerActionStopItemUseOn:
// The client predicts the result of releasing an item, such as an arrow being removed from

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment could be 1 line

// The client predicts the result of releasing an item, such as an arrow being removed from the
// inventory when a bow is fired. Resend the inventories so that the client stays in sync if the
// release does not actually consume the predicted items, for example when it is cancelled.
s.resendInventories()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong logic, It won't fix anything, it will only make things worse...

// the inventory when a bow is fired. Resend the inventories so that the client stays in sync
// if the release does not actually consume the predicted items, for example when it is
// cancelled.
s.resendInventories()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong logic...

@root-nat

Copy link
Copy Markdown
Author

You're right — resending before the release was the wrong placement (it re-sends the pre-release inventory on every successful shot and relies on the slot update to re-correct it, causing a redundant resend and a one-frame flicker). Reworked in #1289: it now resyncs only when the release consumes nothing, and leaves successful shots to the existing inventory slot-change callback (broadcastInvFunc).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Canceling HandleItemRelease when using a Bow will decrease arrows in the player inventory (client-side)

3 participants