improve dropping items

[qndel]

Allows dropping items further than only tiles adjacent to the character and prevents them from dropping in unreachable places.
@Chance4us can you test? : P

Resolves #3514 in a super ugly way 🙃
Fixes #4124

bool CanPut(Point position)
{
	// TODO: remove after collision of houses has been fixed - see https://github.com/diasurgical/devilutionX/issues/3514
	// clang-format off
	if (IsAnyOf(position,
		Point { 53, 45 }, Point { 46, 45 }, Point { 46, 44 }, // house near portal location
		Point { 27, 16 }, Point { 29, 18 }, Point { 27, 27 }, // cathedral
		Point { 38, 67 }, Point { 38, 66 }, Point { 41, 67 }, Point { 41, 64 }, Point { 40, 64 }, // gillian's house
		Point { 71, 81 }, Point { 68, 81 }, Point { 68, 80 }, Point { 70, 78 }, Point { 71, 78 }, // house near farnham
		Point { 73, 71 }, // house near spawn
		Point { 60, 61 }, Point { 60, 60 }, Point { 62, 59 }, Point { 62, 58 }, // griswold's house
		Point { 9, 45 } // stones around wirt
	))
	// clang-format on
		return false;

[Chance4us]

Allows dropping items further than only tiles adjacent to the character and prevents them from dropping in unreachable places. @Chance4us can you test? : P

I tested but can't decide.

What has always irritated me in general with this logic is the fact that if I aim at the top, then the item is dropped at the bottom. Or if I aim to the right, the item may end up on the left or on the right. The behavior is generally somewhat unpredictable. That was always kind of strange in this game.

I think the best thing to do would be to drop the item where the mouse pointer is, paying attention to whether this tile is free. But I don't know if it's that easy to do.

[qndel]

Then nothing would stop you from accidentally dropping your stuff in unreachable places.

[Chance4us]

Then nothing would stop you from accidentally dropping your stuff in unreachable places.

Yes, we should merge this PR quickly, because it is an improvement in any case.
The other thing I suggested is too complicated and would possibly have other undesirable side effects.

[ephphatha]

The adjacency check currently bails if the search area contains a tile that isn't adjacent to an item, this causes the search to fail far quicker than necessary. I think this could be changed to continue searching if any tile in the current search radius contains an item.

In this screenshot the tile under the cursor is not adjacent to an item so I can't drop anything more, but removing the if (adjacentFails) check lets me continue dropping up/left/over the bridge.

[AJenbo]

The adjacency check currently bails if the search area contains a tile that isn't adjacent to an item, this causes the search to fail far quicker than necessary. I think this could be changed to continue searching if any tile in the current search radius contains an item.

In this screenshot the tile under the cursor is not adjacent to an item so I can't drop anything more, but removing the if (adjacentFails) check lets me continue dropping up/left/over the bridge.

Woudn't that let you drop things in areas that are impossible to reach or on the other side of walls?

[ephphatha]

Woudn't that let you drop things in areas that are impossible to reach or on the other side of walls?

Dropping on the other side of walls is only possible if there's an unbroken line of items to the space. This could potentially be an unpathable line given the item adjacency check counts diagonals and player pathing requires clear spaces on both sides of a diagonal for it to be a valid path. Fixing that would be feasible by only checking orthogonal (non-diagonal) tiles in the item adjacency code.

https://user-images.githubusercontent.com/357684/141664550-73450d0c-26f2-44d3-b233-313c6f328005.mp4
(with just the continueFloodFill check)

https://user-images.githubusercontent.com/357684/141664635-d4cb76f6-8c8c-4229-8fb0-930da72846b1.mp4
(with continueFloodFill and CanPutAdjacent updated to only check { Direction::NorthEast, Direction::NorthWest, Direction::SouthEast, Direction::SouthWest })

Edit: Actually, this is an incomplete fix as well. If an item is already on the ground on the other side of a wall the player will start dropping items there (which should eventually be reachable, but it will be non-obvious to the player). To do it properly we'd need to track which tiles are reachable with an unbroken chain from the starting point, requiring an additional data structure/different logic for finding the next tile to drop.

[qndel]

How's this then?

[Chance4us]

How's this then?

Looks nice. I would also like to own that much gold.

[qndel]

Looks nice. I would also like to own that much gold.

give gold in debug build ;)

[FitzRoyX]

Here's a place you can drop items in vanilla that makes them irretrievable. Pro griefer strat, just throw quest items over there. This PR does seem to fix that.

[qndel]

@FitzRoyX how the hell do you drop items there in vanilla? These tiles aren't adjacent to character
Your screenshot doesn't look vanilla ...

[FitzRoyX]

Oh my bad. I assumed devx was vanilla in this regard since it's based on RE code. Was this behavior already altered once before?

[qndel]

Oh my bad. I assumed devx was vanilla in this regard since it's based on RE code. Was this behavior already altered once before?

yes ... that's exactly the point of this PR, to make the altered behavior usable (can't drop in unreachable places)
never assume something is vanilla unless you are testing in ... vanilla xD

[AJenbo]

Could have even tested in 1.3.0...

[Chance4us]

@qndel, @AJenbo
I have now tested the new behavior (335b0b9) over a longer period of time.
Due to the unpredictable dropping position, IMO the new logic is worse than vanilla, even if you can drop more items.
Unless the positioning could be made more logical and predictable. So the items should drop either in the north / east / west / south on the side of the mouse pointer direction.

[qndel]

@qndel, @AJenbo I have now tested the new behavior (335b0b9) over a longer period of time. Due to the unpredictable dropping position, IMO the new logic is worse than vanilla, even if you can drop more items. Unless the positioning could be made more logical and predictable. So the items should drop either in the north / east / west / south on the side of the mouse pointer direction.

335b0b9 is not related to this PR

[Chance4us]

335b0b9 is not related to this PR

That is clear. I'll put the comment there.

[qndel]

@Chance4us what about this one? is it fine?

[Chance4us]

@Chance4us what about this one? is it fine?

Where do I have to look? Is there a new PR? I probably missed something, didn't I?

[qndel]

@Chance4us what about this one? is it fine?

Where do I have to look? Is there a new PR? I probably missed something, didn't I?

This PR lol

[Chance4us]

@Chance4us what about this one? is it fine?

Where do I have to look? Is there a new PR? I probably missed something, didn't I?

This PR lol

The aforementioned PR behaves almost like vanilla. You cannot drop more than 9 items. But the positioning is better. It feels very good. But if you want to drop more item, it won't be enough for us.

[qndel]

Can we stop discussing other PRs here? Just tell me what you think about my one lol, I've changed it many times, have you tested the last version? Is it good?

[Chance4us]

Can we stop discussing other PRs here? Just tell me what you think about my one lol, I've changed it many times, have you tested the last version? Is it good?

Just tested. Feels pretty good.

[qndel]

Can we stop discussing other PRs here? Just tell me what you think about my one lol, I've changed it many times, have you tested the last version? Is it good?

Just tested. Feels pretty good.

Thanks :)

[ephphatha]

Even though this search needs to run twice each time an item is dropped, on my machine there's no noticeable lag. It'd be interesting seeing how this performs on handhelds/low power devices. I don't think it's a significant amount of extra work/space, the checkTiles array clocks in around 12kb and expected worst case for tilesToCheck seems to be around 40 queued positions after dropping a full inventory of gold.

Assuming no other surprising corner cases pop up like the stairs thing I reckon this is good after a rebase.

[ephphatha]

Another thing I noticed while testing... This check stops the player dropping items inside houses (which is great :D), but also stops ctrl+click dropping items if you're standing on a tile above an object (something where IsItemBlockingObjectAtPosition returns true). Relatively minor issue since the player can just move to a different tile, break/open the object, or drop with the mouse targeting a valid tile.

[qndel]

@Chance4us could you test again? :)

[qndel]

The only issue I've noticed so far was that sometimes I got a gold pile I couldn't pick up but seemed to only happen with auric amulet (was testing in multi - 2 players, both with auric amulet) so worth checking if this doesn't also happen with current item drop algorithm.

Doesn't seem related to pickup algorithm - just got this on master with auric amulet

[ephphatha]

Can confirm this fixes #4124

[Chance4us]

@Chance4us could you test again? :)

Of course I'll test soon

[Chance4us]

The items almost always fall on the position where you aim the mouse pointer. It feels really intuitive.