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

Unequipping NPC Clothing Causes a Crash To Desktop #23394

Closed
ChronicDankDisasterAlgorithm opened this issue Apr 6, 2018 · 12 comments

Comments

Projects
@ChronicDankDisasterAlgorithm
Copy link

commented Apr 6, 2018

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

This comment has been minimized.

Copy link
Contributor

commented Apr 6, 2018

Possible culprit: #23306

@Leland Leland added <Crash / Freeze> and removed <Bug> labels Apr 6, 2018

@KongMD-Steam

This comment has been minimized.

Copy link

commented Apr 6, 2018

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

This comment has been minimized.

Copy link
Author

commented Apr 6, 2018

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

This comment has been minimized.

Copy link

commented Apr 7, 2018

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.
remove_armor_bug1

@ChronicDankDisasterAlgorithm

This comment has been minimized.

Copy link
Author

commented Apr 7, 2018

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

@SolsticeTheTechMarine

This comment has been minimized.

Copy link

commented Apr 10, 2018

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.

image

image

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

This comment has been minimized.

Copy link

commented Apr 10, 2018

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

This comment has been minimized.

Copy link

commented Apr 23, 2018

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

This comment has been minimized.

Copy link
Contributor

commented May 26, 2018

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.

@kevingranade kevingranade added this to Need Confirmation in 0.D Release via automation Jan 10, 2019

@kevingranade kevingranade added this to the 0.D milestone Jan 10, 2019

@kevingranade kevingranade moved this from Need Confirmation to Fix Proposed in 0.D Release Jan 10, 2019

@ifreund

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2019

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

This comment has been minimized.

Copy link
Member

commented Jan 25, 2019

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

0.D Release automation moved this from Fix Proposed to Closed Issues Jan 25, 2019

@kevingranade

This comment has been minimized.

Copy link
Member

commented Jan 25, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.