Items disappear when dropping partial stack

Issue #249 closed
cadmal created an issue

Retrieving an item stack from the pack, followed by the split/drop (ALT-D key) command with the "some" option (S key) causes the remaining items in the stack that were not dropped to disappear. That is, if you retrieve a stack of 10 from your pack and drop 7, there will be 7 on the floor but the remaining 3 will vanish and the "in the air" slot will be empty.

This didn't seem to happen if the stack was in a non-pack slot before swapping to "in the air", but I didn't test that extensively. I'm able to reproduce this on a new character with any of the item stacks in the backpack from the starting inventory.

Comments (10)

  1. RMTEW FULL NAME repo owner

    This is likely part of a larger problem with the inventory system we have, there's another bug report about it around somewhere. Hopefully however, it might be possible to work around the larger problem and fix this.

    Thanks for the bug report.

  2. cadmal reporter

    Alright, thanks. I only had a quick look at the source, but I suspect the bug is related to Managers.cpp line 781 where the OneSomeAll() function call returns a pointer to the dropped item stack (it2) and overwrites the pointer to the original item stack (it) with it. I'd think the scary-sounding comment on line 786 about a crash is likely related to this as well. But I don't know if I want to get into compiling the source myself with all those dependencies. Maybe if I get bored enough...

  3. RMTEW FULL NAME repo owner

    If you're using Windows & Visual Studio, there's a prebuilt set of dependencies in an archive in the downloads page which you can just download extract in place.

    Instructions are on the bitbucket overview page.

  4. cadmal reporter

    I built the project and can see precisely why this problem happens:

    1. When you take an item out of the pack, that item's parent object pointer is blanked out (from its previous value of the pack object) instead of being set to the player object as it should be. (If you swap an item from a non-pack slot to "in the air", its parent remains the player object.)
    2. The drop command logic in InventoryManager() has a band-aid fix/hack that attempts to prevent a crash due to the blank parent but ends up making the item disappear instead in this particular case.

    I'll need to do some more digging to determine the correct fix. Either Container::TakeOut() should set the parent after the call to Item::Remove(), or it should be done by the caller to avoid altering the behavior of taking items out of containers other than the pack (i.e. chests). I haven't played enough of Incursion to find a chest yet, so I don't know if taking an item out of one is supposed to move the item directly to the player's inventory or to the floor. I would have guessed the former, but the AutoLoot macro logic makes it seem like the latter if I'm reading it correctly.

  5. RMTEW FULL NAME repo owner

    I'm pretty sure taking an item from a chest puts it in the in air slot, but it's been a while since I played, so can't guarantee it. Otherwise it sounds like you're more on top of this than I am.

    Feel free to do a pull request if you get it to the level you're happy with your fix. Put "closes issue #<issue num>" in the commit comment or pull request comment, and it'll link to and close this issue when it goes in.

  6. cadmal reporter

    Sorry, I'm new to Bitbucket/Git and trying to figure this out. I cloned the repo on my local machine using SourceTree and committed my changes. Do I need to push my local repo to Bitbucket first (for example cadmal/incursion-roguelike) before I can do the pull request?

  7. Log in to comment