From ef2edfd13dae861168069bfe4812ba63fd044617 Mon Sep 17 00:00:00 2001
From: John Bytheway <jbytheway@gmail.com>
Date: Sun, 28 Apr 2019 20:50:45 +0100
Subject: [PATCH] Properly clear NPCs in NPC tests

The npc_can_target_player test was sometimes failing due to the hostile
NPC choosing to target another NPC instead of the player.

This, in turn, was caused by the clearing of NPCs not being done
properly.  They were unloaded, but then loaded again, and not all were
killed.

Write a new clear_npcs in map_helpers.h which works more reliably, and
call that instead.
---
 tests/map_helpers.cpp | 10 +++++++++
 tests/map_helpers.h   |  1 +
 tests/npc_test.cpp    | 47 +++++++++++++++++--------------------------
 3 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/tests/map_helpers.cpp b/tests/map_helpers.cpp
index c9d66cc33d66d..cea8c4f715412 100644
--- a/tests/map_helpers.cpp
+++ b/tests/map_helpers.cpp
@@ -8,10 +8,12 @@
 #include "map.h"
 #include "mapdata.h"
 #include "monster.h"
+#include "npc.h"
 #include "player.h"
 #include "field.h"
 #include "enums.h"
 #include "game_constants.h"
+#include "overmapbuffer.h"
 #include "pimpl.h"
 
 void wipe_map_terrain()
@@ -36,6 +38,14 @@ void clear_creatures()
     g->unload_npcs();
 }
 
+void clear_npcs()
+{
+    for( npc &n : g->all_npcs() ) {
+        n.die( nullptr );
+        overmap_buffer.remove_npc( n.getID() );
+    }
+}
+
 void clear_fields( const int zlevel )
 {
     const int mapsize = g->m.getmapsize() * SEEX;
diff --git a/tests/map_helpers.h b/tests/map_helpers.h
index f3f4cdacd868c..91ce349a50b1a 100644
--- a/tests/map_helpers.h
+++ b/tests/map_helpers.h
@@ -10,6 +10,7 @@ class monster;
 
 void wipe_map_terrain();
 void clear_creatures();
+void clear_npcs();
 void clear_fields( int zlevel );
 void clear_map();
 void clear_map_and_put_player_underground();
diff --git a/tests/npc_test.cpp b/tests/npc_test.cpp
index e3b5292cbdf6d..7ad05fb805ef4 100644
--- a/tests/npc_test.cpp
+++ b/tests/npc_test.cpp
@@ -11,6 +11,7 @@
 #include "field.h"
 #include "game.h"
 #include "map.h"
+#include "map_helpers.h"
 #include "npc.h"
 #include "npc_class.h"
 #include "overmapbuffer.h"
@@ -69,6 +70,17 @@ npc create_model()
     return model_npc;
 }
 
+std::string get_list_of_npcs( const std::string &title )
+{
+
+    std::ostringstream npc_list;
+    npc_list << title << ":\n";
+    for( const npc &n : g->all_npcs() ) {
+        npc_list << "  " << &n << ": " << n.name << '\n';
+    }
+    return npc_list.str();
+}
+
 TEST_CASE( "on_load-sane-values", "[.]" )
 {
     SECTION( "Awake for 10 minutes, gaining hunger/thirst/fatigue" ) {
@@ -305,20 +317,8 @@ TEST_CASE( "npc-movement" )
     g->place_player( tripoint( 60, 60, 0 ) );
 
     // kill npcs before removing vehicles so they are correctly unboarded
-    for( int y = 0; y < height; ++y ) {
-        for( int x = 0; x < width; ++x ) {
-            const tripoint p = g->u.pos() + point( x, y );
-            Creature *cre = g->critter_at( p );
-            if( cre != nullptr && cre != &g->u ) {
-                npc *guy = dynamic_cast<npc *>( cre );
-                cre->die( nullptr );
-                if( guy ) {
-                    overmap_buffer.remove_npc( guy->getID() );
-                }
-            }
-        }
-    }
-    g->unload_npcs();
+    clear_npcs();
+    clear_creatures();
     // remove existing vehicles
     VehicleList vehs = g->m.get_vehicles( g->u.pos(), g->u.pos() + point( width - 1, height - 1 ) );
     for( auto &veh : vehs ) {
@@ -439,21 +439,8 @@ TEST_CASE( "npc_can_target_player" )
 
     g->place_player( tripoint( 10, 10, 0 ) );
 
-    // kill npcs before removing vehicles so they are correctly unboarded
-    for( int y = 0; y < height; ++y ) {
-        for( int x = 0; x < width; ++x ) {
-            const tripoint p = g->u.pos() + point( x, y );
-            Creature *cre = g->critter_at( p );
-            if( cre != nullptr && cre != &g->u ) {
-                npc *guy = dynamic_cast<npc *>( cre );
-                cre->die( nullptr );
-                if( guy ) {
-                    overmap_buffer.remove_npc( guy->getID() );
-                }
-            }
-        }
-    }
-    g->unload_npcs();
+    clear_npcs();
+    clear_creatures();
 
     const auto spawn_npc = []( const int x, const int y, const std::string & npc_class ) {
         const string_id<npc_template> test_guy( npc_class );
@@ -471,6 +458,8 @@ TEST_CASE( "npc_can_target_player" )
     hostile->set_attitude( NPCATT_KILL );
     hostile->name = "Enemy NPC";
 
+    INFO( get_list_of_npcs( "NPCs after spawning one" ) );
+
     hostile->regen_ai_cache();
     REQUIRE( hostile->current_target() != nullptr );
     CHECK( hostile->current_target() == static_cast<Creature *>( &g->u ) );