diasurgical / devilutionX

Diablo build for modern operating systems
Other
7.68k stars 751 forks source link

Item seed collisions can cause items to be picked up other than the target #3510

Open ephphatha opened 2 years ago

ephphatha commented 2 years ago

Important information Win 11 / DevilutionX master / compiled

Describe The way vanilla Diablo/Hellfire handle picking up items means they always do a search over the list of dropped items, even when the coordinates provided in the RequestGetItem message are valid. This can result in a different item getting picked up, leaving the target item on the ground and un-interactable (as it has been flagged as requested).

To Reproduce This is easiest to reproduce with gold.

  1. Use give gold to fill your inventory with gold
  2. Drop it all onto the ground
  3. Start picking up items
  4. Pay attention to which gold pile disappears off the ground, eventually it'll be one away from where you clicked.
  5. Try pick up the pile you originally clicked, the player character will path to the item but will not collect it from the ground.

Expected behavior Picking up items should pick up the one that was targeted.

Screenshots https://user-images.githubusercontent.com/357684/141663183-203e0766-e3e2-4194-a812-73b6c2296afd.mp4 Keep an eye on item 105 (the gold pile left of the rock) and item 60 (the gold pile right of the rock).

Additional context This is a combination of issues. Item seeds for gold are frequently rerolled which with the dodgy default RNG means collisions are more likely, and the logic for picking up an item ignores the position data that gets sent with the message.

AJenbo commented 2 years ago

This has also caused other issues like quest items and "unique" items not reacting to being picked up.