basecamp, faction camp: start merging faction camps into basecamps

[mlangsdorf]

Summary

SUMMARY: Infrastructure "basecamp, faction camp: start merging faction camps into basecamps"

Purpose of change

Faction camps use the omt_terrain of the overmap tile containing the camp overseer to determine camp progress. This is bad and needs to be changed in order to do any serious improvement of faction camps, especially to make them more JSON friendly and to allow alternate upgrade paths.

Step 1 is to find some alternate persistent data structure that can hold faction camp data. The existing basecamp class is barely used. This PR moves some of the faction camp data into basecamp, with the short term goal of removing all uses of omt_terrain to determine faction camp upgrade process, aside from a single migration function for existing basecamps.

Describe the solution

the basecamp class gets heavily revised, picking up several members:

• expansions is a sorted map of direction keys and expansion_data. Each expansion_data contains a string describing the expansion type (camp, kitchen, garage, blacksmith, or farm), it's overmap pos3, and the current level.
• directions is a vector of strings, containing the direction keys for all the expansions except the center, initial camp. The current faction camp code makes a distinction between the center camp and the expansions, and it's useful to preserve that logic.
• sort_points is a vector containing the sort points that used to be stored in the camp overseer/manager NPC.

Various functions for manipulating these structures are introduced in this PR, and some existing talk_function:: code dealing with faction camps have been moved inside basecamp:: if those functions needed access to basecamp:: data internals.

Describe alternatives you've considered

Like everything else in faction camps, there's a bunch of different ways to start this process. I have to start somewhere.

Using the overmap terrain id as a data store is not great design for a bunch of reasons. It's just not compatible with a flexible faction camp design that includes the option to upgrade parts but not all of an overmap tile or take over existing buildings. So while it would be possible to continue with faction camps using the overmap terrain id to determine upgrade process, doing so is not my plan.

I could have moved more functions into basecamp::, but this PR is large enough as it is.

The map of string dir to expansion data isn't as useful as I hoped. A future commit will probably replace it with a fixed array indexed via an enum, but the current string indexing is built fairly heavily into the code and I don't want to do the necessary surgery at this time.

Additional context

This PR is going to be the first in an extended series that will eventually see the removal of the camp overseer role, the removal of the restriction on upgrading with vehicles on the map, and eventually JSON defined faction camps with multiple upgrade paths, including the ability to take over existing buildings. All of that is gated on NOT using the omt_terrain ID to measure faction camp upgrade progress.

The data structures introduced in this PR are not intended to be the final design of faction camps/base camps. They're being introduced to smooth the transition from the current state to the long term improved state.

[kevingranade]

Don't know if this is related yet, started a camp and upgraded it to the level that has crates, dispatched menial laborer, and encountered a segfault when I recovered them:

Program terminated with signal SIGSEGV, Segmentation fault.
#0  map::inbounds (this=this@entry=0x2835080, p=...) at src/map.cpp:7127
7127        return ( p.x >= 0 && p.x < SEEX * my_MAPSIZE &&
Missing separate debuginfos, use: debuginfo-install glibc-2.26-28.amzn2.0.1.x86_64 libgcc-7.3.1-5.amzn2.0.2.x86_64 libstdc++-7.3.1-5.amzn2.0.2.x86_64 ncurses-libs-6.0-8.20170212.amzn2.1.2.x86_64
(gdb) bt
#0  map::inbounds (this=this@entry=0x2835080, p=...) at src/map.cpp:7127
#1  0x00000000006f693e in map::furn (this=0x2835080, p=...) at src/map.cpp:1147
#2  0x0000000000566edd in basecamp::menial_return (this=this@entry=0x76fce78, p=...) at src/faction_camp.cpp:1751
#3  0x00000000005719d8 in talk_function::handle_camp_mission (cur_key=..., p=...) at src/faction_camp.cpp:829
#4  0x00000000007a4e0a in talk_function::companion_mission (p=...) at src/mission_companion.cpp:121
#5  0x0000000000865012 in std::function<void (dialogue const&)>::operator()(dialogue const&) const (__args#0=..., this=0x848f050) at /usr/include/c++/7/bits/std_function.h:706
#6  talk_response::effect_fun_t::operator() (d=..., this=0x848f050) at src/dialogue.h:123
#7  talk_response::effect_t::apply (this=0x7ffc53a7aa30, d=...) at src/npctalk.cpp:1973
#8  0x00000000008766c4 in dialogue::opt (this=this@entry=0x7ffc53a7ac10, d_win=..., topic=...) at src/npctalk.cpp:1772
#9  0x0000000000876dd5 in npc::talk_to_u (this=0x7657f80, text_only=text_only@entry=false) at src/npctalk.cpp:359
#10 0x000000000087752e in game::chat (this=this@entry=0x2834c60) at src/npctalk.cpp:230
#11 0x00000000005e30fb in game::handle_action (this=this@entry=0x2834c60) at src/handle_action.cpp:1463
#12 0x00000000005ab145 in game::do_turn (this=0x2834c60) at src/game.cpp:1548
#13 0x000000000041fc59 in main (argc=<optimized out>, argv=<optimized out>) at src/main.cpp:674

[kevingranade]

In basecamp_menial_return(), p.companion_mission_points is empty, but it gets dereferenced.

[mlangsdorf]

I think I've figured out the upgrade issue: editmap::mapgen_set creates a new tinymap at the existing overmap tile position and copies stuff over from the existing overmap tile's submaps to the new tinymap's submaps. It's supposed to copy over the basecamp:: but the implicit assignment operator isn't working or something.

[mlangsdorf]

Okay, what was happening with expansions:

• when you upgraded your central camp, the submap basecamps were being copied over unconditionally and invalid basecamps would overwrite the active basecamp pointer. That had a bunch of weird effects. I fixed that, and the camp manager now returns to his little green tile correctly.
• Separately, the survey_return function would update the expansions vector with wrong omt_pos3, using the central camp's omt_pos3 for the expansion. This caused the weird overwrite behavior Kevin was reporting.

I still need to write test cases. That's going to be a few days but the rest can be reviewed. I'd be fine with submitting them as a separate PR, actually.

[kevingranade]

I'd be fine with submitting them as a separate PR, actually.

I'll accept this without unit tests, just pointing out that the cost/benefit ratio seems likely to be favorable since it's so hard to test manually.

[mlangsdorf]

Started work on the unit tests, but the actual code flow depends on pop-ups and the test harness handles those poorly as far as I know. Refactoring everything to be easier to test would be a large task. It will probably happen as part of moving this stuff to JSON but I don't see an easy way to do it now.

[kevingranade]

Not sure if this one is related, it might be pre-existing.

I debugged in some NPCs and used mind control on them. I picked one and told them to be camp manager, then they were bugging me about not being able to breathe so I accepted their mission and gave them an inhaler. Now they're marked as a Camp Manager, but they don't have any dialog options for it.

Teleported to an entirely different zone, made sure to satisfy the new NPCs mission needs, then started a camp. I can enter the manager dialog now, but if I select "What needs to be done?" I get "There are no missions at this colony.".

Possibly pertinent minutiae, I'm mind controlling NPCs like crazy, teleporting all over the place, and I think I have 6 active camps scattered around at this point.

[mlangsdorf]

No missions at this colony probably means that something went crazy with finding the basecamp. I'll see if I can reproduce it.

[mlangsdorf]

The logic for finding a basecamp was indeed messed up. I have unmessed it up, I hope.

[mlangsdorf]

Jenkins rebuild

[kevingranade]

This pull request has been mentioned on Cataclysm: Dark Days Ahead. There might be relevant details there:

https://discourse.cataclysmdda.org/t/aircraft-in-game/17669/6

[kevingranade]

After many many weeks of upgrades, I encountered this segfault when retrieving a blacksmith from a crafting job:

Program terminated with signal SIGSEGV, Segmentation fault.
#0  std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, expansion_data>, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, expansion_data> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const>, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, expansion_data> > >::find (this=this@entry=0x68, __k=...) at /usr/include/c++/7/bits/stl_tree.h:2536
2536          const_iterator __j = _M_lower_bound(_M_begin(), _M_end(), __k);
Missing separate debuginfos, use: debuginfo-install glibc-2.26-28.amzn2.0.1.x86_64 libgcc-7.3.1-5.amzn2.0.2.x86_64 libstdc++-7.3.1-5.amzn2.0.2.x86_64 ncurses-libs-6.0-8.20170212.amzn2.1.2.x86_64
(gdb) bt
#0  std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, expansion_data>, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, expansion_data> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const>, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, expansion_data> > >::find (this=this@entry=0x68, __k="[B]") at /usr/include/c++/7/bits/stl_tree.h:2536
#1  0x000000000055f274 in std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, expansion_data, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const>, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, expansion_data> > >::find (__x="[B]", this=0x68) at /usr/include/c++/7/bits/stl_map.h:1189
#2  basecamp::recruit_evaluation (this=0x0, sbase=@0x7ffeb53f4a4c: 0, sexpansions=@0x7ffeb53f4a50: -1254143216, sfaction=@0x7ffeb53f4a54: 32766, sbonus=@0x7ffeb53f4a58: 1942447816) at src/faction_camp.cpp:2973
#3  0x000000000056a85e in talk_function::camp_recruit_start[abi:cxx11](npc&) (p=...) at src/faction_camp.cpp:3032
#4  0x000000000056d7f0 in talk_function::camp_missions (mission_key=..., p=...) at src/faction_camp.cpp:488
#5  0x00000000007a77ac in talk_function::companion_mission (p=...) at src/mission_companion.cpp:95
#6  0x0000000000868006 in std::function<void (dialogue const&)>::operator()(dialogue const&) const (__args#0=..., this=0xcff1820) at /usr/include/c++/7/bits/std_function.h:706
#7  talk_response::effect_fun_t::operator() (d=..., this=0xcff1820) at src/dialogue.h:123
#8  talk_response::effect_t::apply (this=0x7ffeb53f5370, d=...) at src/npctalk.cpp:1974
#9  0x00000000008796c4 in dialogue::opt (this=this@entry=0x7ffeb53f5550, d_win=..., topic=...) at src/npctalk.cpp:1773
#10 0x0000000000879dd5 in npc::talk_to_u (this=0x6d98cf0, text_only=text_only@entry=false) at src/npctalk.cpp:359
#11 0x000000000087a52e in game::chat (this=this@entry=0x1bf8c20) at src/npctalk.cpp:230
#12 0x00000000005e3a8d in game::handle_action (this=this@entry=0x1bf8c20) at src/handle_action.cpp:1468
#13 0x00000000005ab9f9 in game::do_turn (this=0x1bf8c20) at src/game.cpp:1549
#14 0x000000000041fd8d in main (argc=<optimized out>, argv=<optimized out>) at src/main.cpp:674

[mlangsdorf]

Dangling basecamp pointer resolved.

[kevingranade]

I was testing with a basecamp, spent hours on it.
I typoed in the debug menu and summoned an artifact in the camp leader tent, which started spawning FIRE throughout the tent.
Looking at the prospect of more hours of testing, yea good enough.