-
Notifications
You must be signed in to change notification settings - Fork 814
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bugfix: Prevent item deletion when dropping items #7006
base: master
Are you sure you want to change the base?
Bugfix: Prevent item deletion when dropping items #7006
Conversation
WorldTilePosition &position = MyPlayer->position.tile; | ||
|
||
// Common function to attempt opening based on input method. | ||
auto attemptOpen = [&]() -> bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What need does the arrow function serve, it seems it should work the same with the code directly instead.
Direction dir = useCursorPosition ? GetDirection(myPlayer.position.tile, cursPosition) : myPlayer._pdir; | ||
std::optional<Point> itemTile = FindAdjacentPositionForItem(myPlayer.position.tile, dir); | ||
|
||
if (!itemTile) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets call it freeTile
, avalibleTile
or something like that instead.
This looks like a good direction to move in, but I would like to iterate over it a little bit. I dropped you some initial comments to look into. |
Fixes: #6937
TryDropItem()
attempted to use the future position of the player to validate the drop position of an item, which would be invalidated in certain cases inOnPutItem()
which uses the player's current position. This mismatch would result in the item being removed from the player's hand, but failing to be dropped.I deduplicated and unified the logic used for handling using the mouse to drop items and other methods (Ctrl + click or non-mouse controls), and set the player's position to current position universally instead of future position. Additionally, I've given the same treatment to the logic for opening dungeons.
For the cherry on top, I removed a redundant
else
inOnPutItem()