Fix using variables for spawning items

[Ramza13]

Summary

None

Purpose of change

Fix #68747

Describe the solution

Describe alternatives you've considered

Testing

Additional context

[andrei8l]

This just makes it ignore the variable/math with no error, doesn't it? count and item_name need to be evaluated inside the lambda wherever likely_rewards are used (mission UI??)

[Ramza13]

This just makes it ignore the variable/math with no error, doesn't it? count and item_name need to be evaluated inside the lambda wherever likely_rewards are used (mission UI??)

Yes you are right. That should only matter in a small number of cases and you can sort of get around it in those by using a default value.

I've looked into a proper solution but it would be huge. The issue is that any solution I try seems to cause an infinite loop. talk_effect_fun_t would need to have its likely_rewards or function calls replaced by dbl_or_var and str_or_var objects. However those contain talk_effect_fun_t so it can't be done like that.

At least to me it seems like this is a tiny bug compared to the effort required to rewrite the whole dialog mess. Maybe there is a solution I'm missing, but I think at least for now it makes sense to merge this and then possibly open up the mission rewards display as a new bug.

[andrei8l]

I've looked into a proper solution but it would be huge. The issue is that any solution I try seems to cause an infinite loop. talk_effect_fun_t would need to have its likely_rewards or function calls replaced by dbl_or_var and str_or_var objects. However those contain talk_effect_fun_t so it can't be done like that.

I tried it quickly and it seems to work fine. I don't see any path for infinite loops. talk_effect_fun_t is used in dbl_or_var only for arithmetic which is going away in the next couple of months.

Most of this is just moving code around

diff --git a/src/dialogue_helpers.h b/src/dialogue_helpers.h
index 7690900414..da7e3d3680 100644
--- a/src/dialogue_helpers.h
+++ b/src/dialogue_helpers.h
@@ -18,11 +18,36 @@ using talkfunction_ptr = std::add_pointer_t<void ( npc & )>;
 using dialogue_fun_ptr = std::add_pointer_t<void( npc & )>;
 
 using trial_mod = std::pair<std::string, int>;
+struct dbl_or_var;
 
+struct var_info {
+    var_info( var_type in_type, std::string in_name ): type( in_type ),
+        name( std::move( in_name ) ) {}
+    var_info( var_type in_type, std::string in_name, std::string in_default_val ): type( in_type ),
+        name( std::move( in_name ) ), default_val( std::move( in_default_val ) ) {}
+    var_info() : type( var_type::global ) {}
+    var_type type;
+    std::string name;
+    std::string default_val;
+};
+
+template<class T>
+struct abstract_str_or_var {
+    std::optional<T> str_val;
+    std::optional<var_info> var_val;
+    std::optional<T> default_val;
+    std::optional<std::function<T( const dialogue & )>> function;
+    std::string evaluate( dialogue const & ) const;
+};
+
+using str_or_var = abstract_str_or_var<std::string>;
 struct talk_effect_fun_t {
+    public:
+        using likely_reward_t = std::pair<dbl_or_var, str_or_var>;
+        using likely_rewards_t = std::vector<likely_reward_t>;
     private:
         std::function<void( dialogue &d )> function;
-        std::vector<std::pair<int, itype_id>> likely_rewards;
+        likely_rewards_t likely_rewards;
 
     public:
         talk_effect_fun_t() = default;
@@ -107,7 +132,7 @@ struct talk_effect_fun_t {
         void set_bulk_trade_accept( const JsonObject &jo, std::string_view member, bool is_npc = false );
         void set_npc_gets_item( bool to_use );
         void set_add_mission( const JsonObject &jo, std::string_view member );
-        const std::vector<std::pair<int, itype_id>> &get_likely_rewards() const;
+        const likely_rewards_t &get_likely_rewards() const;
         void set_u_buy_monster( const JsonObject &jo, std::string_view member );
         void set_learn_recipe( const JsonObject &jo, std::string_view member, bool is_npc = false );
         void set_forget_recipe( const JsonObject &jo, std::string_view member, bool is_npc = false );
@@ -141,31 +166,10 @@ struct talk_effect_fun_t {
         }
 };
 
-struct var_info {
-    var_info( var_type in_type, std::string in_name ): type( in_type ),
-        name( std::move( in_name ) ) {}
-    var_info( var_type in_type, std::string in_name, std::string in_default_val ): type( in_type ),
-        name( std::move( in_name ) ), default_val( std::move( in_default_val ) ) {}
-    var_info() : type( var_type::global ) {}
-    var_type type;
-    std::string name;
-    std::string default_val;
-};
-
 std::string read_var_value( const var_info &info, const dialogue &d );
 
 var_info process_variable( const std::string &type );
 
-template<class T>
-struct abstract_str_or_var {
-    std::optional<T> str_val;
-    std::optional<var_info> var_val;
-    std::optional<T> default_val;
-    std::optional<std::function<T( const dialogue & )>> function;
-    std::string evaluate( dialogue const & ) const;
-};
-
-using str_or_var = abstract_str_or_var<std::string>;
 using translation_or_var = abstract_str_or_var<translation>;
 
 struct eoc_math {
diff --git a/src/mission.cpp b/src/mission.cpp
index 8f825ba6c6..3321df990d 100644
--- a/src/mission.cpp
+++ b/src/mission.cpp
@@ -767,7 +767,7 @@ character_id mission::get_npc_id() const
     return npc_id;
 }
 
-const std::vector<std::pair<int, itype_id>> &mission::get_likely_rewards() const
+const talk_effect_fun_t::likely_rewards_t &mission::get_likely_rewards() const
 {
     return type->likely_rewards;
 }
diff --git a/src/mission.h b/src/mission.h
index 27d741a34c..95ca40d7ba 100644
--- a/src/mission.h
+++ b/src/mission.h
@@ -201,7 +201,7 @@ struct mission_type {
         bool has_generic_rewards = true;
 
         // A limited subset of the talk_effects on the mission
-        std::vector<std::pair<int, itype_id>> likely_rewards;
+        talk_effect_fun_t::likely_rewards_t likely_rewards;
 
         // Points of origin
         std::vector<mission_origin> origins;
@@ -349,7 +349,7 @@ class mission
         int get_id() const;
         const itype_id &get_item_id() const;
         character_id get_npc_id() const;
-        const std::vector<std::pair<int, itype_id>> &get_likely_rewards() const;
+        const talk_effect_fun_t::likely_rewards_t &get_likely_rewards() const;
         bool has_generic_rewards() const;
         void register_kill_needed() {
             monster_kill_goal++;
diff --git a/src/mission_ui.cpp b/src/mission_ui.cpp
index 876814ce54..e86bf642d7 100644
--- a/src/mission_ui.cpp
+++ b/src/mission_ui.cpp
@@ -97,13 +97,25 @@ void game::list_missions()
         draw_scrollbar( w_missions, selection, entries_per_page, umissions.size(), point( 0, 3 ) );
 
         mission_row_coords.clear();
+        dialogue d( get_talker_for( get_avatar() ), nullptr, {} );
+        auto format_tokenized_description = [&d]( const std::string & description,
+        const talk_effect_fun_t::likely_rewards_t &rewards ) {
+            std::string formatted_description = description;
+            for( const auto &reward : rewards ) {
+                std::string token = "<reward_count:" + itype_id( reward.second.evaluate( d ) ).str() + ">";
+                formatted_description = string_replace( formatted_description, token, string_format( "%g",
+                                                        reward.first.evaluate( d ) ) );
+            }
+            parse_tags( formatted_description, get_avatar(), get_avatar() );
+            return formatted_description;
+        };
         for( int i = top_of_page; i <= bottom_of_page; i++ ) {
             mission *miss = umissions[i];
             const nc_color col = u.get_active_mission() == miss ? c_light_green : c_white;
             const int y = i - top_of_page + 3;
             trim_and_print( w_missions, point( 1, y ), MAX_CHARS_PER_MISSION_ROW_NAME,
                             static_cast<int>( selection ) == i ? hilite( col ) : col,
-                            miss->name() );
+                            format_tokenized_description( miss->name(), miss->get_likely_rewards() ) );
             inclusive_rectangle<point> rec( point( 1, y ), point( 1 + MAX_CHARS_PER_MISSION_ROW_NAME - 1, y ) );
             mission_row_coords.emplace( i, rec );
         }
@@ -121,16 +133,6 @@ void game::list_missions()
 
             int y = 3;
 
-            auto format_tokenized_description = []( const std::string & description,
-            const std::vector<std::pair<int, itype_id>> &rewards ) {
-                std::string formatted_description = description;
-                for( const auto &reward : rewards ) {
-                    std::string token = "<reward_count:" + reward.second.str() + ">";
-                    formatted_description = string_replace( formatted_description, token, string_format( "%d",
-                                                            reward.first ) );
-                }
-                return formatted_description;
-            };
             y += fold_and_print( w_missions, point( 41, y ), getmaxx( w_missions ) - 43, col,
                                  format_tokenized_description( miss->name(), miss->get_likely_rewards() ) + for_npc );
             y++;
diff --git a/src/npctalk.cpp b/src/npctalk.cpp
index 62b69d8a1a..0f6909df67 100644
--- a/src/npctalk.cpp
+++ b/src/npctalk.cpp
@@ -3039,9 +3039,7 @@ void talk_effect_fun_t::set_spawn_item( const JsonObject &jo, std::string_view m
         receive_item( iname, count.evaluate( d ), container_name.evaluate( d ), d, use_item_group,
                       suppress_message, add_talker, target_location, force_equip );
     };
-    dialogue d( get_talker_for( get_avatar() ), nullptr, {} );
-    likely_rewards.emplace_back( static_cast<int>( count.evaluate( d ) ),
-                                 itype_id( item_name.evaluate( d ) ) );
+    likely_rewards.emplace_back( count, item_name );
 }
 
 void talk_effect_fun_t::set_u_buy_item( const JsonObject &jo, std::string_view member )
@@ -3856,7 +3854,7 @@ void talk_effect_fun_t::set_add_mission( const JsonObject &jo, std::string_view
     };
 }
 
-const std::vector<std::pair<int, itype_id>> &talk_effect_fun_t::get_likely_rewards() const
+const talk_effect_fun_t::likely_rewards_t &talk_effect_fun_t::get_likely_rewards() const
 {
     return likely_rewards;
 }

I tried it with this EOC & mission

  {
    "id": "MISSION_EXPLODE",
    "type": "mission_definition",
    "name": { "str": "Explode <reward_count:hammer> <global_val:blorg>" },
    "description": "blorg",
    "goal": "MGOAL_FIND_ITEM",
    "difficulty": 2,
    "value": 0,
    "item": "novel_mystery",
    "count": 3,
    "origins": [ "ORIGIN_SECONDARY" ],
    "has_generic_rewards": false,
    "dialogue": {
      "describe": "I'd like to read some mystery novels.",
      "offer": "Sometimes it gets a little bit boring up here, and I like to read something to take my mind off of things.  Problem is, I've run out of books.  I've been wanting to read some mystery novels as of late, so could you get me three of them?",
      "accepted": "Just bring them to me when you have it.",
      "rejected": "Nevermind then.  I hope you'll change your mind.",
      "advice": "I'd loot libraries and bookstores.",
      "inquire": "So, have you found the books?",
      "success": "Thank you.  This will keep me busy for a while.  Please take this as a token of my gratitude.",
      "success_lie": "Could you give them to me?",
      "failure": "Fine.  I can read something else."
    },
    "end": {
      "effect": [
        { "u_spawn_item": { "global_val": "blorg" }, "count": { "math": [ "u_val('strength') + 1" ] } },
        {
          "u_spawn_item": { "global_val": "blorg" },
          "count": { "arithmetic": [ { "u_val": "strength" }, "+", { "const": 1 } ] }
        }
      ]
    }
  },
  {
    "type": "effect_on_condition",
    "id": "EOC_GAME_EXPLODES",
    "effect": [ { "set_string_var": "hammer", "target_var": { "global_val": "blorg" } }, { "assign_mission": "MISSION_EXPLODE" } ]
  },

screenshot

(the character had strength 9)

Obviously there are still deficiencies in mission_ui's parsing of <reward_count:...> (and it's pretty ugly in general), but that's completely separate from this PR.

[Ramza13]

Thanks! I just assumed I couldn't compile something like that. You learn something new everyday.