don't rebuild the item drop list

[KorGgenT]

Summary

SUMMARY: Bugfixes "instead of rebuilding the item drop list, remove invalid pointers"

Purpose of change

Fixes #36554

Describe the solution

as noted in the attached issue, the push_back function appears to be invalidating the item_locations in the entire item list called items. I have noticed that act.targets has all valid pointers, and that the pointers do not become invalid until p.moves becomes negative. I couldn't quite find a reasonable solution that involves keeping the way the item list gets rebuilt at the end of this function, so i implemented a for loop that just removes the targets and their associated value when it finds invalid pointers.

Describe alternatives you've considered

First I thought copying the item_location was invalidating them, but I couldn't find a specific location at which they were being invalidated.

Testing

Tested with 300 plastic chunks: drop with d, drop with D (directional) and using advanced inventory to drop at my location from my inventory.

Additional context

I spent a long time trying to find a way to fix this, so admittedly this is the first solution I found that worked. I have a hunch that there is a more elegant solution out there, but maybe reworking the drop activity even more might be on that particular horizon.

[ghost]

I have noticed that act.targets has all valid pointers, and that the pointers do not become invalid until p.moves becomes negative.

But why do they get invalidated? Could this have something to do with how activity resuming became quite inconsistent of late?

Good to have this annoyance patched, but the underlying problem seems to still be there

[kevingranade]

I have another candidate for the cause, but my guess needs some verification, and I'm not clear where exactly it goes wrong.

1. activity is constructed, target list is valid
2. First turn worth of dropping occurs
   2a. target list moved to item list by convert_to_locations() (because it uses emplace_back())
   2b. targets.clear() invoked, this "should not" cause a problem, but who knows.
   2c. target list moved back to targets list by first moving entries into an ephemeral list (emplace_back() invoked by the other convert_to_locations()), then copying entries into targets.

If anything touches any entries in targets between 2a and 2c, that is in fact undefined behaviour, additionally this is an egregiously complicated sequence to run these shared pointers through, alternately copying and moving them across multiple containers.

My recommendation is to make sure everything that touches an item_location uses push_back() instead of emplace_back() and see if that clears anything up.

[Voideka]

After this update, dropping large amounts of items hangs the game.

[Unrepentant-Atheist]

I agree with Voideka. Fix is no bueno. Large amount of items hangs the game. Even small amout of items take a long time now.

The dropping worked in build 10090. SOmething since then fucked shite up.

[ZhilkinSerg]

What amount of items do you consider "large"? Do you have a savegame and/or reproduction scenario?

[Voideka]

Save attached. Hit shift+D, select any tile, then hit tab for category select, drop all of the Other category. After a couple of items it slows down, then it grinds to a halt.
save - Copy.zip

Total items dropped is about 46. I've even tried moving to another map tile without a stack of existing items, same issue.

[eilaattwood]

What amount of items do you consider "large"? Do you have a savegame and/or reproduction scenario?

I can confirm this bug. My game also hangs.