Apply extra care with monster dragging code to avoid segfaults

Based upon my interpretation of the stack-trace in CleverRaven#33104, it appears
that `monster::store` is trying to use a dragged_foe that's not null,
but is no longer valid, either. This may happen if the foe is dead, or
any other condition that may cause the dragged critter object to be destroyed.

This commit:

- Always tracks the `dragged_foe_id` (an integer) rather than
  `dragged_foe` (a pointer) directly.
- Uses `critter_by_id()` to check the validity of the `dragged_foe_id`
  on the occasions when it needed. This is the same function used by the
  savegame loading code to restore the dragged foe.
- Moves the dragged_foe-finding functionality to its own method for
  clarity.
- Ensures the dragged_foe-finding code clears relevant status effects
  and members if we're no longer dragging a valid critter.
- Moves the nursebot operation code to its own method, also for clarity.

Note that I don't have a reproducable way to induce the bug, so I don't
have a great test for this. However it's hard to have a segfault if you
don't have a stale pointer to deference in the first place.

Fixes CleverRaven#33051. Fixes CleverRaven#33141.

src/monattack.cpp

@@ -2951,7 +2951,7 @@ bool mattack::nurse_operate( monster *z )
         } else {
             grab( z );
             if( target->has_effect( effect_grabbed ) ) { // check if we succesfully grabbed the target
-                z->dragged_foe = target;
+                z->dragged_foe_id = target->getID();
                 z->add_effect( effect_dragging, 1_turns, num_bp, true );
                 return true;
             }

src/monmove.cpp

@@ -601,54 +601,11 @@ void monster::move()
         }
     }
 
-    // dragged_foe: restore pointer by saved id if required
-    if( dragged_foe_id >= 0 ) {
-        dragged_foe = g->critter_by_id<player>( dragged_foe_id );
-        dragged_foe_id = 0;
-    }
-    // defective nursebot surgery code
-    if( type->has_special_attack( "OPERATE" ) && has_effect( effect_dragging ) &&
-        dragged_foe != nullptr ) {
-
-        if( rl_dist( pos(), goal ) == 1 && g->m.furn( goal ) == furn_id( "f_autodoc_couch" ) &&
-            !has_effect( effect_operating ) ) {
-            if( dragged_foe->has_effect( effect_grabbed ) && !has_effect( effect_countdown ) &&
-                ( g->critter_at( goal ) == nullptr || g->critter_at( goal ) == dragged_foe ) ) {
-                add_msg( m_bad, _( "The %1$s slowly but firmly puts %2$s down onto the autodoc couch." ), name(),
-                         dragged_foe->disp_name() );
-
-                dragged_foe->setpos( goal );
-
-                add_effect( effect_countdown, 2_turns );// there's still time to get away
-                add_msg( m_bad, _( "The %s produces a syringe full of some translucent liquid." ), name() );
-            } else if( g->critter_at( goal ) != nullptr && has_effect( effect_dragging ) ) {
-                sounds::sound( pos(), 8, sounds::sound_t::speech,
-                               string_format(
-                                   _( "a soft robotic voice say, \"Please step away from the autodoc, this patient needs immediate care.\"" ) ) );
-                // TODO: Make it able to push NPC/player
-                push_to( goal, 4, 0 );
-            }
-        }
-        if( get_effect_dur( effect_countdown ) == 1_turns && !has_effect( effect_operating ) ) {
-            if( dragged_foe->has_effect( effect_grabbed ) ) {
-
-                bionic_collection collec = *dragged_foe->my_bionics;
-                int index = rng( 0, collec.size() - 1 );
-                bionic target_cbm = collec[index];
-
-                //8 intelligence*4 + 8 first aid*4 + 3 computer *3 + 4 electronic*1 = 77
-                float adjusted_skill = static_cast<float>( 77 ) - std::min( static_cast<float>( 40 ),
-                                       static_cast<float>( 77 ) - static_cast<float>( 77 ) / static_cast<float>( 10.0 ) );
-
-                g->u.uninstall_bionic( target_cbm, *this, *dragged_foe, adjusted_skill );
+    // Check if they're dragging a foe and find their hapless victim
+    player *dragged_foe = find_dragged_foe();
 
-                dragged_foe->remove_effect( effect_grabbed );
-                remove_effect( effect_dragging );
-                dragged_foe = nullptr;
-
-            }
-        }
-    }
+    // Give nursebots a chance to do surgery.
+    nursebot_operate( dragged_foe );
 
     // The monster can sometimes hang in air due to last fall being blocked
     const bool can_fly = has_flag( MF_FLIES );
@@ -929,6 +886,81 @@ void monster::move()
     }
 }
 
+player *monster::find_dragged_foe()
+{
+    // Make sure they're actually dragging someone.
+    if( dragged_foe_id < 0 || !has_effect( effect_dragging ) ) {
+        dragged_foe_id = -1;
+        return nullptr;
+    }
+
+    // Dragged critters may die or otherwise become invalid, which is why we look
+    // them up each time. Luckily, monsters dragging critters is relatively rare,
+    // so this check should happen infrequently.
+    player *dragged_foe = g->critter_by_id<player>( dragged_foe_id );
+
+    if( dragged_foe == nullptr ) {
+        // Target no longer valid.
+        dragged_foe_id = -1;
+        remove_effect( effect_dragging );
+    }
+
+    return dragged_foe;
+}
+
+// Nursebot surgery code
+void monster::nursebot_operate( player *dragged_foe )
+{
+    // No dragged foe, nothing to do.
+    if( dragged_foe == nullptr ) {
+        return;
+    }
+
+    // Nothing to do if they can't operate, or they don't think they're dragging.
+    if( !( type->has_special_attack( "OPERATE" ) && has_effect( effect_dragging ) ) ) {
+        return;
+    }
+
+    if( rl_dist( pos(), goal ) == 1 && g->m.furn( goal ) == furn_id( "f_autodoc_couch" ) &&
+        !has_effect( effect_operating ) ) {
+        if( dragged_foe->has_effect( effect_grabbed ) && !has_effect( effect_countdown ) &&
+            ( g->critter_at( goal ) == nullptr || g->critter_at( goal ) == dragged_foe ) ) {
+            add_msg( m_bad, _( "The %1$s slowly but firmly puts %2$s down onto the autodoc couch." ), name(),
+                     dragged_foe->disp_name() );
+
+            dragged_foe->setpos( goal );
+
+            add_effect( effect_countdown, 2_turns );// there's still time to get away
+            add_msg( m_bad, _( "The %s produces a syringe full of some translucent liquid." ), name() );
+        } else if( g->critter_at( goal ) != nullptr && has_effect( effect_dragging ) ) {
+            sounds::sound( pos(), 8, sounds::sound_t::speech,
+                           string_format(
+                               _( "a soft robotic voice say, \"Please step away from the autodoc, this patient needs immediate care.\"" ) ) );
+            // TODO: Make it able to push NPC/player
+            push_to( goal, 4, 0 );
+        }
+    }
+    if( get_effect_dur( effect_countdown ) == 1_turns && !has_effect( effect_operating ) ) {
+        if( dragged_foe->has_effect( effect_grabbed ) ) {
+
+            bionic_collection collec = *dragged_foe->my_bionics;
+            int index = rng( 0, collec.size() - 1 );
+            bionic target_cbm = collec[index];
+
+            //8 intelligence*4 + 8 first aid*4 + 3 computer *3 + 4 electronic*1 = 77
+            float adjusted_skill = static_cast<float>( 77 ) - std::min( static_cast<float>( 40 ),
+                                   static_cast<float>( 77 ) - static_cast<float>( 77 ) / static_cast<float>( 10.0 ) );
+
+            g->u.uninstall_bionic( target_cbm, *this, *dragged_foe, adjusted_skill );
+
+            dragged_foe->remove_effect( effect_grabbed );
+            remove_effect( effect_dragging );
+            dragged_foe_id = -1;
+
+        }
+    }
+}
+
 // footsteps will determine how loud a monster's normal movement is
 // and create a sound in the monsters location when they move
 void monster::footsteps( const tripoint &p )

src/monster.h

@@ -419,8 +419,7 @@ class monster : public Creature
         tripoint wander_pos; // Wander destination - Just try to move in that direction
         int wandf;           // Urge to wander - Increased by sound, decrements each move
         std::vector<item> inv; // Inventory
-        player *dragged_foe; // player being dragged by the monster
-        int dragged_foe_id; // id of player being dragged by the monster (for save/load)
+        int dragged_foe_id = -1; // id of player being dragged by the monster
         cata::optional<item> tied_item; // item used to tie the monster
         cata::optional<item> battery_item; // item to power mechs
         // DEFINING VALUES
@@ -525,6 +524,9 @@ class monster : public Creature
         std::bitset<NUM_MEFF> effect_cache;
         cata::optional<time_duration> summon_time_limit = cata::nullopt;
 
+        player *find_dragged_foe();
+        void nursebot_operate( player *dragged_foe );
+
     protected:
         void store( JsonOut &jsout ) const;
         void load( JsonObject &jsin );

src/savegame_json.cpp

@@ -1811,7 +1811,6 @@ void monster::load( JsonObject &data )
 
     data.read( "inv", inv );
     data.read( "dragged_foe_id", dragged_foe_id );
-    dragged_foe = nullptr;
 
     if( data.has_int( "ammo" ) && !type->starting_ammo.empty() ) {
         // Legacy loading for ammo.
@@ -1889,10 +1888,7 @@ void monster::store( JsonOut &json ) const
     }
     json.member( "inv", inv );
 
-    if( dragged_foe ) {
-        json.member( "dragged_foe_id", dragged_foe->getID() );
-    }
-
+    json.member( "dragged_foe_id", dragged_foe_id );
     json.member( "path", path );
 }