Check for NPC dialogue leading directly to TALK_MISSION_OFFER (#48813)

* Check for dialogues leading to TALK_MISSION_OFFER

Inspired by Cataclysm-BN#494, this checks for dialogues that lead
directly to TALK_MISSION_OFFER when they should instead go via
TALK_MISSION_LIST.

* Fix NPCs with respond direct to TALK_MISSION_OFFER

The just-added check found these three examples of NPCs whose initial
dialogue option has an unconditional response leading to
TALK_MISSION_OFFER.  This can cause problems once the mission is
complete.  Fix the three.

data/json/npcs/TALK_CITY_COP.json

@@ -18,7 +18,7 @@
       },
       { "text": "Wanna get outta here?", "topic": "TALK_SUGGEST_FOLLOW" },
       { "text": "Let's trade items.", "topic": "TALK_CITY_COP", "effect": "start_trade" },
-      { "text": "Can I do anything for you?", "topic": "TALK_MISSION_OFFER" },
+      { "text": "Can I do anything for you?", "topic": "TALK_MISSION_LIST" },
       { "text": "I gotta go.", "topic": "TALK_DONE" }
     ]
   },

src/dialogue.h

@@ -368,6 +368,7 @@ class json_talk_response
     private:
         talk_response actual_response;
         std::function<bool( const dialogue & )> condition;
+        bool has_condition_ = false;
         bool is_switch = false;
         bool is_default = false;
 
@@ -378,6 +379,11 @@ class json_talk_response
         json_talk_response() = default;
         explicit json_talk_response( const JsonObject &jo );
 
+        const talk_response &get_actual_response() const;
+        bool has_condition() const {
+            return has_condition_;
+        }
+
         /**
          * Callback from @ref json_talk_topic::gen_responses, see there.
          */
@@ -445,6 +451,8 @@ class json_talk_topic
          * responses will be added (behind those added here).
          */
         bool gen_responses( dialogue &d ) const;
+
+        cata::flat_set<std::string> get_directly_reachable_topics( bool only_unconditional ) const;
 };
 
 void unload_talk_topics();

src/npc.cpp

@@ -22,6 +22,7 @@
 #include "cursesdef.h"
 #include "damage.h"
 #include "debug.h"
+#include "dialogue.h"
 #include "dialogue_chatbin.h"
 #include "effect.h"
 #include "enums.h"
@@ -50,6 +51,7 @@
 #include "mtype.h"
 #include "mutation.h"
 #include "npc_class.h"
+#include "npctalk.h"
 #include "options.h"
 #include "output.h"
 #include "overmap.h"
@@ -259,6 +261,18 @@ void npc_template::check_consistency()
         if( !guy.myclass.is_valid() ) {
             debugmsg( "Invalid NPC class %s", guy.myclass.c_str() );
         }
+        std::string first_topic = guy.chatbin.first_topic;
+        if( const json_talk_topic *topic = get_talk_topic( first_topic ) ) {
+            cata::flat_set<std::string> reachable_topics =
+                topic->get_directly_reachable_topics( true );
+            if( reachable_topics.count( "TALK_MISSION_OFFER" ) ) {
+                debugmsg(
+                    "NPC template \"%s\" has dialogue \"%s\" which leads unconditionally to "
+                    "\"TALK_MISSION_OFFER\", which doesn't check for an available mission.  "
+                    "You should probably prefer \"TALK_MISSION_LIST\"",
+                    e.first.str(), first_topic );
+            }
+        }
     }
 }
 

src/npctalk.cpp

@@ -2549,6 +2549,7 @@ json_talk_response::json_talk_response( const JsonObject &jo )
 
 void json_talk_response::load_condition( const JsonObject &jo )
 {
+    has_condition_ = jo.has_member( "condition" );
     is_switch = jo.get_bool( "switch", false );
     is_default = jo.get_bool( "default", false );
     read_condition<dialogue>( jo, "condition", condition, true );
@@ -2562,6 +2563,11 @@ bool json_talk_response::test_condition( const dialogue &d ) const
     return true;
 }
 
+const talk_response &json_talk_response::get_actual_response() const
+{
+    return actual_response;
+}
+
 bool json_talk_response::gen_responses( dialogue &d, bool switch_done ) const
 {
     if( !is_switch || !switch_done ) {
@@ -2862,6 +2868,29 @@ bool json_talk_topic::gen_responses( dialogue &d ) const
     return replace_built_in_responses;
 }
 
+cata::flat_set<std::string> json_talk_topic::get_directly_reachable_topics(
+    bool only_unconditional ) const
+{
+    std::vector<std::string> result;
+
+    auto add_reachable_for_response = [&]( const json_talk_response & json_response ) {
+        const talk_response &response = json_response.get_actual_response();
+        if( !only_unconditional || !json_response.has_condition() ) {
+            result.push_back( response.success.next_topic.id );
+            result.push_back( response.failure.next_topic.id );
+        }
+    };
+
+    for( const json_talk_response &r : responses ) {
+        add_reachable_for_response( r );
+    }
+    for( const json_talk_repeat_response &r : repeat_responses ) {
+        add_reachable_for_response( r.response );
+    }
+
+    return cata::flat_set<std::string>( result.begin(), result.end() );
+}
+
 std::string json_talk_topic::get_dynamic_line( const dialogue &d ) const
 {
     return dynamic_line( d );
@@ -2959,3 +2988,12 @@ bool npc::item_whitelisted( const item &it )
     const auto to_match = it.tname( 1, false );
     return item_name_whitelisted( to_match );
 }
+
+const json_talk_topic *get_talk_topic( const std::string &id )
+{
+    auto it = json_talk_topics.find( id );
+    if( it == json_talk_topics.end() ) {
+        return nullptr;
+    }
+    return &it->second;
+}

src/npctalk.h

@@ -5,6 +5,7 @@
 #include "type_id.h"
 
 class item;
+class json_talk_topic;
 class npc;
 class time_duration;
 
@@ -100,4 +101,7 @@ time_duration calc_proficiency_training_time( const npc &, const proficiency_id
 int calc_proficiency_training_cost( const npc &p, const proficiency_id &proficiency );
 time_duration calc_ma_style_training_time( const npc &, const matype_id & /* id */ );
 int calc_ma_style_training_cost( const npc &p, const matype_id & /* id */ );
+
+const json_talk_topic *get_talk_topic( const std::string &id );
+
 #endif // CATA_SRC_NPCTALK_H