Skip to content

Fix item duplication, trade item loss and vendor buy validation - #4503

Open
Y0oshi wants to merge 5 commits into
ACEmulator:masterfrom
Y0oshi:fix/stack-revalidation
Open

Y0oshi wants to merge 5 commits into
ACEmulator:masterfrom
Y0oshi:fix/stack-revalidation

Conversation

@Y0oshi

@Y0oshi Y0oshi commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Fixes a few inventory, trade and vendor issues where client input could duplicate items, lose items, or create an unbounded number of objects.

  • Give: HandleActionGiveObjectRequest validates the item and stack size when the request comes in, then gives the item in the MoveTo callback using the original item and container references. Give doesn't set IsBusy, and HandleActionStackableMerge within the player's own inventory runs immediately, so the stack can be fully merged into another stack (and destroyed) before the callback runs. When giving part of a stack, RemoveItemForGive then subtracts from the destroyed stack, whose StackSize was never cleared, and creates a new stack, duplicating the amount. Same for NPC gives.
  • SplitTo3D: same issue during the pickup animation, since StartPickupChain doesn't set IsBusy.
  • Both callbacks now look the item up again in the player's inventory and equipped items and recheck the stack size, similar to the revalidation HandleActionStackableSplitToContainer does after its move and pickup. This also refreshes the container references (and for Give, the equipped state) in case the item was moved in the meantime.
  • Trade: FinalizeTrade removes items from both players, then adds them to the other player after a short delay, ignoring the result. It sets IsBusy, but in-inventory actions such as HandleActionStackableSplitToContainer don't check it, so the recipient can use up free slots during the delay and the item ends up with no container. Items that can't be added are now returned to their original owner (or logged as lost if that also fails), the same fallback GiveObjectToPlayer uses.
  • Vendor buy: vendor services count as 0 slots in ItemsToReceive, and ItemProfileToWorldObjects creates one object per unit for non-stackable items before the busy, unique and price checks, so a large amount for a service would create that many objects on the world thread. A service can now only be bought with an amount of 1. Repeated item guids in the same request now reject the whole purchase (sell already drops duplicates); previously a unique item listed twice was charged twice, and if the main pack was full it could be added to two side packs.
  • docker-compose: MySQL was published on all interfaces, with root allowed from any host (MYSQL_ROOT_HOST=%) and the default root password committed in docker.env. It's now bound to localhost; the server container connects over the compose network and is unaffected. Remote DB tools would need an SSH tunnel or a compose override.

@gmriggs

gmriggs commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator

(verified the first 3 fixes, waiting for some other devs to chime in with their knowledge on the last 3 fixes)

Great job with finding these bugs, the code to fix them, and the PR notes!

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.

2 participants