Cataclysm-dda: Unequipping NPC Clothing Causes a Crash To Desktop

[ChronicDankDisasterAlgorithm]

Game version:
0.C-27229-gae5bdc0 Build 7286
Operating system:
Win 10 Home 64-bit
Tiles or curses:
Tiles
Mods active:
Mining Mod, More Survival Tools, StatsThroughSkills, Vehicle Additions Pack, Tanks and Other Vehicles, Boats, Folding Parts Pack, Necromancy, Disable NPC Needs, Extended Realistic Guns

Expected behavior

(u)nequiping NPC clothing should take off the clothing item

Actual behavior

(u)nequipping NPC clothing causes a Crash To Desktop

Steps to reproduce the behavior

Examine an NPC then select (r) 'sort armor' and then press (u) on a piece of clothing

When I do it to safety glasses it correctly unequipped them, but cowboy hat and leather pants did cause the crash. I haven't tried other articles of clothing besides the ones mentioned.

[Leland]

Possible culprit: #23306

[KongMD-Steam]

I can reproduce the issue on build 7285. The CTD occurred when I tried to unequip the NPC's pants. This is the last output in my debug.log file. The same message repeats over more lines that what I've posted here.

18:7:59.601 GAME : src/npctalk.cpp:2634: called calc_chance with invalid talk_trial value: 0
18:7:59.601 GAME : src/npctalk.cpp:2634: called calc_chance with invalid talk_trial value: 0
18:7:59.601 GAME : src/npctalk.cpp:2634: called calc_chance with invalid talk_trial value: 0
18:7:59.601 GAME : src/npctalk.cpp:2634: called calc_chance with invalid talk_trial value: 0
18:7:59.601 GAME : src/npctalk.cpp:2634: called calc_chance with invalid talk_trial value: 0
18:7:59.601 GAME : src/npctalk.cpp:2634: called calc_chance with invalid talk_trial value: 0
18:7:59.601 GAME : src/npctalk.cpp:2634: called calc_chance with invalid talk_trial value: 0

[ChronicDankDisasterAlgorithm]

After I gave them some different clothing through the dialogue screen (I want you to use this item) I was then able to unequip the clothes that previously had caused the crash. So I'm not sure what was going on.

[KongMD-Steam]

There's a race condition in the remove armor code. I fired up the same save 10 times and removed all armor from the NPC, starting from the top. The only crashes I got were when removing pants and removing military rucksack. 2/10 attempts crashed, and the other 8 attempts I was able to remove all armor successfully.

Mods: Base game only.

In a different save, I got this error when removing the NPC's boots. The game did not crash after the error.

[ChronicDankDisasterAlgorithm]

Yeah I think it dupes the item you take off as well.

[SolsticeTheTechMarine]

Can confirm that attempting to remove clothing from an npc causes a crash. Appears to also happen when applying clothing to npc, though this could also be the npc removing clothing and thus causing a crash.

EDIT: Another thing I have noticed is that while I am healing any broken limbs on an npc, it causes a sort of ticking time bomb effect. Eventually when the npc attempts to remove the splint, which causes a crash. Had I backed up the save, I could test Lamandus's theory, but it seems it doesn't happen. Will update if I have more info.

EDIT 2: Got some followers rather badly crippled and was still having the issue. Both npcs having gone to sleep, one of whome had healed their broken limbs to usable status while sleeping. Once I woke up one of the npcs the game would crash. This was a perfect moment for testing Lamandus's theory. I first woke the npc up without a storage space on them. This crashed the game. I tried the edit npc option in debug mode to force add a backpack onto the sleeping npc. I proceed to wake the npc up with no problems. The game should be checking to see if there is inventory space to store and if not dropping the item, but somewhere in between the game crashes, presumably from a loop without a break.

TLDR: To solve the issue temporarily, debug and force a backpack onto the npc. The issue is the lack of storage space on the npc.

[Lamandus]

It may be happening when the amount of possible volume he can carry of the rest he wears exceeds the amount already in his inventory. Especally removing rucksacks (for me) causes this crash (when the NPC is transporting stuff, of course).

[Lazasar]

Definitely an inventory capacity issue. If you use the "let's trade items" dialogue, the error checking there prevents it from happening, but using the armor interface can cause it when you unequip NPC armor. It also can happen if you give an item that adds inventory space, like a backpack, to the NPC via the "use this item" option

[Maeyanie]

I did some debugging on this as part of issue #23799 (without realizing it was a duplicate), but this thread seems more complete so I'll summarize it here. Backtraces and other long stuff can be found there.

Basically the problem seems to be that when an NPC tries to takeoff() an item, which would put it over its capacity, it drop()s it. However, within drop(), it tells the NPC to process the activity.
This causes the NPC to call takeoff() again, while still in the middle of the first call. That one succeeds (because it's called with res != null), modifies the equipment list and invalidates the iterator in the outer call, causing a crash when it is then used.

Commenting out the section doing that did solve the crash and NPCs dropping items seemed to work, but I don't know if it causes any other problems so I'm reluctant to send this in as a PR.

Edit:
That change does seem to break telling NPCs to unequip things via their Sort Armour screen, so seems it needs the attention of someone who knows the code better than me to craft a proper fix.

[ifreund]

I can't seem reproduce this crash on the latest build. It's been over half a year, maybe this got fixed somewhere down the line. Can someone else reproduce? If so I will try to fix it.

[Night-Pryanik]

Can't confirm either on latest experimental. Closing for now, if the issue still exists, ping me.

[kevingranade]

It looks like #23663 fixed this, or at least applied a workaround of having the outer takeoff() exit after calling drop() to avoid dereferencing the invalidated iterator.