search

CleverRaven / Cataclysm-DDA

Cataclysm - Dark Days Ahead. A turn-based survival game set in a post-apocalyptic world.
http://cataclysmdda.org
Other
10.61k stars 4.17k forks source link

Inserting Items Segmentation Fault #48765

Closed veskveskvesk closed 1 year ago

veskveskvesk commented 3 years ago

Describe the bug

This Segmentation Fault is caused by trying to insert both a nested container (inside of a container) and it's contents on the ground into a wielded container, using the insert command (i). It hard-crashes to the desktop with a Segmentation Fault error.

Steps To Reproduce

  1. "w"ield a container (e.g. 'Body Bag')
  2. Stand atop a tile with a container with nested content, and secondary nested contents inside of it (e.g. firearm>magazine>bullets); "i"nsert both the magazine and bullets at the same time (but not firearm) into the held container. e.g. <RM103A automagnum (8/10 8x40mm caseless>8x40 10-round magazine (8/10 8x40mm caseless)>8x40mm caseless (8)"
  3. Game will hard crash with a Segmentation Fault.

Expected behavior

Inserting items from the ground into a wielded/worn container should move both of them to the target container, either leaving nested contents intact (e.g. ammo inside of a magazine, in the new container), or separate contents and container during the "i"nsert (perhaps with a warning popup, if contents are fluids).

Screenshots

Screenshot 2021-05-05 171943

Versions and configuration

Additional context

Crash Log and Debug Log crashanddebug.zip

Save File Union Gap.zip

The save is set up just before the crash, standing atop the necessary items to cause it, and wielding a body bag. The screenshot above shows how to replicate it: "i"nventory>select wielded Body Bag>"i"nsert>select one of the firearm's magazines and ammunition (but not the firearm itself) that my character is standing atop of>return>Segmentation Fault

p.s. I've included a second save file, which hard-crashed due to this segmentation fault error and which I have no backups of and am unable to play anymore (loading the save works, but as soon as the game is fully loaded, it Segmentation Faults and crashes immediately). If you have any advice on possible recovery of this character or world I'd be greatly appreciative. Marmarth.zip

actual-nh commented 3 years ago

Ping: @RoyBerube, @KorGgenT - any thoughts?

Zeropol commented 3 years ago

Loaded the save, inserted the 500-round in the wielded body bag with no crash. But I see an other difference : image I cannot do this. I cannot select both when inserting. Downloaded build 11582 with the same result. The magazine is inserted inside the wielded body bag with no crash, but I cannot select both the 500-round and its content when inserting. Also tried with the gun magazine there is on the character tile when loading the save.

Zeropol commented 3 years ago

Here a gif of what happen when I try to select both the magazine and its content for inserting inside the body bag : https://cdn.discordapp.com/attachments/613424936928804895/839666828426084372/cant_select_both.gif

actual-nh commented 3 years ago

Do you get a segmentation fault on loading the Marmarth.zip save, BTW?

Zeropol commented 3 years ago

Do you get a segmentation fault on loading the Marmarth.zip save, BTW?

Yes, both on build 11582 and on a build I built few hours ago. I mean, not a "segfault window", but the game exit after errors like "lost item during save load cycle" ToxiClay is working on restoring the savefile for Veskveskvesk.

ToxiClay commented 3 years ago

I've identified where the problem is, but not what the problem is or why.

In the #VmVzaw==.sav file, attempting to recover line 22, the "player" data, consistently crashes. I receive the following error message sequence:

 DEBUG    : Failed to find item_location owner with character_id 1

 FUNCTION : bool item_location::impl::item_on_person::ensure_who_unpacked() const
 FILE     : src/item_location.cpp
 LINE     : 290

 DEBUG    : Failed to find item_location owner with character_id 1

 FUNCTION : bool item_location::impl::item_on_person::ensure_who_unpacked() const
 FILE     : src/item_location.cpp
 LINE     : 290

 DEBUG    : item_location lost its target item during a save/load cycle

 FUNCTION : void item_location::impl::ensure_unpacked() const
 FILE     : src/item_location.cpp
 LINE     : 114

 DEBUG    : parent location does not point to valid item

 FUNCTION : void item_location::deserialize(JsonIn&)
 FILE     : src/item_location.cpp
 LINE     : 782

 DEBUG    : item_location lost its target item during a save/load cycle

 FUNCTION : void item_location::impl::ensure_unpacked() const
 FILE     : src/item_location.cpp
 LINE     : 114

I can't be sure which items are causing the problem, but I can try to dissect this line based on what Vesk was doing immediately before the crash, excise what I think are the offending items, and pray.

ToxiClay commented 3 years ago

This file is not expressly meant to be human-readable, which is...a serious pain in the behind.

ToxiClay commented 3 years ago

Oho. This maybe looks promising?

"activity": { "type": "ACT_UNLOAD", "actor": { "actor_type": "ACT_UNLOAD", "actor_data": { "moves_total": 755, "target": { "idx": 12, "type": "in_container", "parent": { "type": "character", "character": 1, "idx": 19 } } } }, "moves_left": 557, "index": 0, "position": 0, "coords": [  ], "coord_set": [  ], "name": "", "targets": [  ], "placement": [ 0, 0, 0 ], "values": [  ], "str_values": [  ], "auto_resume": false, "monsters": [  ] }

Bah, can't make it block-print. Maybe cuz it's all on one line.

This matches with line 9 of the stacktrace emitted by the original save when it faceplanted:

#8
    (dbghelp: @0x97f1a7[cataclysm-tiles.exe+0x57f1a7]), 
    (libbacktrace: item_contents::has_pocket_type(item_pocket::pocket_type) const+0x7@0x97f1a7),
    (libbacktrace: 0x97f1a7    [unknown src]:0    [unknown func]),
  #9
    (dbghelp: @0x433f41[cataclysm-tiles.exe+0x33f41]), 
    (libbacktrace: unload_activity_actor::unload(Character&, item_location&)+0x31@0x433f41),
    (libbacktrace: 0x433f41    [unknown src]:0    [unknown func]),
  #10
    (dbghelp: @0xf28849[cataclysm-tiles.exe+0xb28849]), 
    (libbacktrace: player_activity::do_turn(player&)+0xab9@0xf28849),
    (libbacktrace: 0xf28849    [unknown src]:0    [unknown func]),

I'm gonna do what's called a pro gamer move.

ToxiClay commented 3 years ago

YES!! Deleting that Activity line is what I needed! I am invincible!

Or, rather, not "deleting" it but replacing it with

"activity": { "type": "ACT_NULL" }

I still don't know why it occurred -- that'll take some more noodling -- but at least it's solved.

RoyBerube commented 3 years ago

The problem shown in the screenshot - essentially selecting the same item twice - was fixed in #48580. The build 0E.10395 is from before that PR was merged.

Anyway, nice catch on the atomicity problem @ToxiClay

actual-nh commented 3 years ago

This file is not expressly meant to be human-readable, which is...a serious pain in the behind.

Should an issue be put in as to this (also)?

KorGgenT commented 3 years ago

This file is not expressly meant to be human-readable, which is...a serious pain in the behind.

Should an issue be put in as to this (also)?

no, human readability is not really important for save files

ToxiClay commented 3 years ago

This file is not expressly meant to be human-readable, which is...a serious pain in the behind.

Should an issue be put in as to this (also)?

no, human readability is not really important for save files

Modifying the serialization of the save file to promote human readability would be a significant amount of work in exchange for gains only in edge cases like this one.

anothersimulacrum commented 3 years ago

The .sav files are not human readable because whitespace would significantly increase the size of saves. It is fairly easy to remove the version header, lint the file, then add the version header back to get a human readable .sav file.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not \'bump\' or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered.