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

Unloading adjacent ammo belts throws error, leaves empty belt behind #35368

Closed
Nerezza opened this issue Nov 6, 2019 · 1 comment · Fixed by #37023
Closed

Unloading adjacent ammo belts throws error, leaves empty belt behind #35368

Nerezza opened this issue Nov 6, 2019 · 1 comment · Fixed by #37023
Labels
<Bug> This needs to be fixed Items: Magazines Ammo holding items and objects. (S2 - Confirmed) Bug that's been confirmed to exist
Projects
Milestone

Comments

@Nerezza
Copy link
Contributor

Nerezza commented Nov 6, 2019

Describe the bug

src/visitable.cpp:506 [item visitable<T>::remove_item(item&) [with T = Character]] Tried removing item from object which did not contain it

If you unload an ammo belt from an adjacent tile, the above error appears and the belt is not removed from the game, leaving behind an ammo belt with 0 ammunition which shouldn't be a thing since you've deconstructed the ammo links it consists of.

Steps To Reproduce

  1. Spawn an ammo belt adjacent to the player
  2. "U"nload the ammo belt without picking it up

Expected behavior

The ammo belt should disappear once empty, and the game attempts to do just that. I believe this bug is an oversight from when the ability to interact with adjacent objects without picking them up was implemented, the code seems to assume that the ammo belt is being moved to the player's inventory to unload it.

Versions and configuration

This has happened under 3 different versions/configurations already, but here's my current one anyway:

  • OS: Windows
    • OS Version: 10.0 1903
  • Game Version: 0.D-9137-g1e35a58 [64-bit]
  • Graphics Version: Tiles
  • Mods loaded: [
    Dark Days Ahead [dda],
    Disable NPC Needs [no_npc_food],
    Aftershock [aftershock],
    More Survival Tools [More_Survival_Tools],
    Modular Turrets [modular_turrets],
    Salvaged Robots [Salvaged_Robots],
    Vehicle Additions Pack [blazemod],
    Tanks and Other Vehicles [Tanks],
    Disable Religious Texts [no_religious_Texts],
    No Acid Zombies [No_Acid_Zombies],
    No Ants [No_Anthills],
    No Big Zombies [No_Big_Zombies],
    No Explosive Zombies [No_Explosive_Zombies],
    No Fungal Monsters [No_Fungi],
    Safe Autodoc [safeautodoc],
    Prevent Zombie Revivication [no_reviving_zombies],
    Stats Through Skills [StatsThroughSkills],
    Alternative Map Key [alt_map_key],
    Fuji's Military Profession Pack [fuji_mpp]
    ]

Additional context

It's pretty clear the code used to unload ammo belts expects them to be in the player's inventory. It just needs to be changed so that it'll remove the belt no matter where it is.

@Night-Pryanik Night-Pryanik added (S2 - Confirmed) Bug that's been confirmed to exist <Bug> This needs to be fixed Items: Magazines Ammo holding items and objects. labels Nov 7, 2019
@kevingranade kevingranade added this to Confirmed in 0.E Release via automation Jan 13, 2020
@kevingranade kevingranade added this to the 0.E milestone Jan 13, 2020
@KorGgenT
Copy link
Member

probably need to obtain the item, didn't really do much digging but i'm pretty sure item_location::obtain invalidates the pointer... and returns an index.

@KorGgenT KorGgenT moved this from Confirmed to Fix Proposed in 0.E Release Jan 13, 2020
@kevingranade kevingranade moved this from Fix Proposed to In Progress in 0.E Release Jan 13, 2020
0.E Release automation moved this from In Progress to Done Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bug> This needs to be fixed Items: Magazines Ammo holding items and objects. (S2 - Confirmed) Bug that's been confirmed to exist
Projects
No open projects
0.E Release
  
Done
Development

Successfully merging a pull request may close this issue.

4 participants