Infinite loop when NPC lacks access to required crafting proficiency

[EvanBalster]

Describe the bug

I'm currently subjecting some newly-recruited NPCs to a training montage and in the process of relieving them and sending them to bed my game froze with an infinite loop in do_activity. The backtrace implicates some "can craft" checks executed while my senior NPC is moving away from her work area.

Rosann is more experienced than my other NPCs, so this may have to do with one of the other four NPCs depending on her knowledge. Yusuke in particular is relying on her Metalworking proficiency to finish our anvil and will become unable to continue his work once she steps away.

Attach save file

GitHub wouldn't take the upload so self-hosting it. https://vermin.city/cdda/Bauxite-NPC-Move-CanCraft-Crash.zip

Steps to reproduce

1. Load attached save.
2. Walk right or order Rosann to move to the right side of the swimming pool.
3. As soon as Rosann (in the orange hard hat) tries to move beyond the position depicted below, the game will freeze.

Or:

1. Load attached save.
2. Order Yusuke to take it easy. (C,A,-,Yusuke Taylor)
3. Lead Rosann away.
4. Order Yusuke to work on the scavenger's anvil. (C,A,C,work on craft ... confirm)

Expected behavior

No freeze.

Screenshots

If you set up a sweatshop in a swimming pool, is it a swimshop or a sweatpool?

Versions and configuration

OS: Windows 11 x64

Experimental (Tiles) 2025-11-27-1313

Default configuration + bionic slots mod

Additional context

No log file is updated when manifesting the crash.

Here's a backtrace from my debugger (attached to an experimental release build

Not Flagged	>	27328	0	Main Thread	Main Thread	cataclysm-tiles.exe!inventory::add_item
 	 	 	 	 	 	cataclysm-tiles.exe!inventory::add_item(class item,bool,bool,bool)
 	 	 	 	 	 	cataclysm-tiles.exe!inventory::form_from_map(class map &,class std::vector<class coords::coord_point_ob<struct tripoint,5,0>,class std::allocator<class coords::coord_point_ob<struct tripoint,5,0> > >,class Character const *,bool)
 	 	 	 	 	 	cataclysm-tiles.exe!inventory::form_from_map(class map *,class coords::coord_point_ob<struct tripoint,5,0> const &,int,class Character const *,bool,bool)
 	 	 	 	 	 	cataclysm-tiles.exe!Character::crafting_inventory(class map *,class coords::coord_point_ob<struct tripoint,5,0> const &,int,bool)
 	 	 	 	 	 	cataclysm-tiles.exe!Character::crafting_inventory(class coords::coord_point_ob<struct tripoint,5,0> const &,int,bool)
 	 	 	 	 	 	cataclysm-tiles.exe!multi_activity_actor::craft_can_do(class string_id<class activity_type> const &,class Character &,class coords::coord_point_ob<struct tripoint,5,0> const &)
 	 	 	 	 	 	cataclysm-tiles.exe!can_do_activity_there()
 	 	 	 	 	 	cataclysm-tiles.exe!generic_multi_activity_handler(class player_activity &,class Character &,bool)
 	 	 	 	 	 	cataclysm-tiles.exe!activity_type::call_do_turn(class player_activity *,class Character *)
 	 	 	 	 	 	cataclysm-tiles.exe!player_activity::do_turn(class Character &)
 	 	 	 	 	 	cataclysm-tiles.exe!npc::do_player_activity(void)
 	 	 	 	 	 	cataclysm-tiles.exe!npc::execute_action(enum npc_action)
 	 	 	 	 	 	cataclysm-tiles.exe!npc::move(void)
 	 	 	 	 	 	cataclysm-tiles.exe!`anonymous namespace'::monmove()
 	 	 	 	 	 	cataclysm-tiles.exe!do_turn(void)
 	 	 	 	 	 	cataclysm-tiles.exe!WinMain()
 	 	 	 	 	 	cataclysm-tiles.exe!__scrt_common_main_seh()
 	 	 	 	 	 	kernel32.dll!00007ffdfb0de8d7()
 	 	 	 	 	 	ntdll.dll!00007ffdfcd0c53c()

[EvanBalster]

Spent a while longer messing with this. Telling Yusuke to stop crafting before Rosann walks away definitely prevents the crash. Revised the issue title accordingly.

EDIT: The freeze also occurs if an NPC is ordered to resume work on the item without the proficient NPC nearby to advise. In this case the loop includes this callstack:

 	cataclysm-tiles.exe!std::vector<class mission *,class std::allocator<class mission *> >::_Emplace_reallocate<class mission * &>(class mission * * const,class mission * &)	Unknown
 	cataclysm-tiles.exe!item_contents::get_pockets(class std::function<bool (class item_pocket const &)> const &)	Unknown
 	cataclysm-tiles.exe!item_contents::get_all_contained_pockets(void)	Unknown
 	cataclysm-tiles.exe!item::get_all_contained_pockets(void)	Unknown
 	cataclysm-tiles.exe!`anonymous namespace'::_stacks_whiteblacklist()	Unknown
 	cataclysm-tiles.exe!item::stacks_with(class item const &,bool,bool,bool,int,int,bool)	Unknown
 	cataclysm-tiles.exe!inventory::add_item(class item,bool,bool,bool)	Unknown
 	cataclysm-tiles.exe!inventory::form_from_map(class map &,class std::vector<class coords::coord_point_ob<struct tripoint,5,0>,class std::allocator<class coords::coord_point_ob<struct tripoint,5,0> > >,class Character const *,bool)	Unknown
 	cataclysm-tiles.exe!inventory::form_from_map(class map *,class coords::coord_point_ob<struct tripoint,5,0> const &,int,class Character const *,bool,bool)	Unknown
 	cataclysm-tiles.exe!Character::crafting_inventory(class map *,class coords::coord_point_ob<struct tripoint,5,0> const &,int,bool)	Unknown
 	cataclysm-tiles.exe!Character::crafting_inventory(class coords::coord_point_ob<struct tripoint,5,0> const &,int,bool)	Unknown
 	cataclysm-tiles.exe!multi_activity_actor::craft_can_do(class string_id<class activity_type> const &,class Character &,class coords::coord_point_ob<struct tripoint,5,0> const &)	Unknown
 	cataclysm-tiles.exe!can_do_activity_there()	Unknown
 	cataclysm-tiles.exe!generic_multi_activity_handler(class player_activity &,class Character &,bool)	Unknown
 	cataclysm-tiles.exe!activity_type::call_do_turn(class player_activity *,class Character *)	Unknown
 	cataclysm-tiles.exe!player_activity::do_turn(class Character &)	Unknown
 	cataclysm-tiles.exe!npc::do_player_activity(void)	Unknown
 	cataclysm-tiles.exe!npc::execute_action(enum npc_action)	Unknown
 	cataclysm-tiles.exe!npc::move(void)	Unknown
 	cataclysm-tiles.exe!`anonymous namespace'::monmove()	Unknown
 	cataclysm-tiles.exe!do_turn(void)	Unknown
 	cataclysm-tiles.exe!WinMain()	Unknown

[EvanBalster]

The infinite loop appears to happen in do_activity. It can occur if there is no line of sight between the two NPCs, even if the NPC with the proficiency is also adjacent to the in progress craft.

Two possible culprits for the regression:

• https://github.com/CleverRaven/Cataclysm-DDA/pull/83818, part of a series of crafting refactors by @ShnitzelX2
• https://github.com/CleverRaven/Cataclysm-DDA/pull/80858, introduced required proficiencies which are a precondition for the infinite loop

[ShnitzelX2]

Probably #83922, though I'd need to verify whether this was an issue prior. My test case for these PRs is having an NPC craft a pipe cleaner, so I know crafting works in general.

[EvanBalster]

@ShnitzelX2 That save file was made with a recent-enough build that it experimental 2025-11-24 or so should load it right up.

Or, on a fresh file, try using a scavenger's anvil as your test case. You'll need to spawn a few tools, though: hammer, adjustable wrench, angle grinder, steel frame. And you'll need one PC/NPC with the proficiency to start the crafting work (eg, spawn yourself as a handyman). Then remove your proficiency or have some underqualified minion continue the work as you wander away.

This is just programmers' intuition based on source-less debug stepping, but: I'll bet there are two different pieces of code checking the requirements for the crafting job. One of those (controlling the loop) is not accounting for the relatively new possibility of a required proficiency, and erroneously assumes the other check will behave identically.

[ShnitzelX2]

/confirmed

Bug is present on:

• cdda-windows-with-graphics-x64-2025-07-10-1658
• cdda-windows-with-graphics-x64-2025-11-24-2054
• master build as of posting this

So not from my PRs.

The problem:

1. craft_activity_actor::do_turn() runs check_if_craft_okay for Yusuke when the player moves to the right, failing and setting the activity to null. This seems correct to me given the info provided.
2. The end of the activity backlog is restored, and because the ACT_MULTIPLE_CRAFT "do" stage assigns individual NPCs to do craft_activity_actor, ACT_MULTIPLE_CRAFT was the last backlogged and is restored
3. Because the requirements for craft_activity_actor and ACT_MULTIPLE_CRAFT are different, ACT_MULTIPLE_CRAFT continues running despite the fact that it presumably shouldn't
4. The ACT_MULTIPLE_CRAFT "do" stage ends up assigning another craft_activity_actor, which is not processed because... only a legacy activity or activity actor can be processed, not both. The newly assigned actor is cleared because the legacy activity ACT_MULTIPLE_CRAFT was set to null at the start of generic_multi_activity_handler, and we loop back to step 2, forever.

TL;DR: legacy multi-activity system bad. This justifies #83818 very nicely, and I can try to fix this when porting ACT_MULTIPLE_CRAFT to an activity actor. Thanks!