Skip to content
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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kphoenix137
Copy link
Collaborator

@kphoenix137 kphoenix137 commented Mar 7, 2024

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 in OnPutItem() 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 in OnPutItem()

@kphoenix137 kphoenix137 changed the title Fix delete dropped item Bugfix: Prevent item deletion when dropping items Mar 7, 2024
@kphoenix137 kphoenix137 added the fix fix for a bug label Mar 7, 2024
WorldTilePosition &position = MyPlayer->position.tile;

// Common function to attempt opening based on input method.
auto attemptOpen = [&]() -> bool {
Copy link
Member

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) {
Copy link
Member

@AJenbo AJenbo Apr 13, 2024

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.

@AJenbo
Copy link
Member

AJenbo commented Apr 13, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix fix for a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Issue Report]: Ctrl+Click can delete items when the player is moving
2 participants