From 5a90688a46111d6f0a89febd56f80549752d0c9c Mon Sep 17 00:00:00 2001
From: Eric Pierce <wapcaplet88@gmail.com>
Date: Mon, 10 Feb 2020 22:11:52 -0700
Subject: [PATCH] Transfer inhaler to NPC inventory on mission end (#37454)

* Let asthmatic NPC keep inhaler after quest

I recruited an asthmatic NPC after completing their inhaler mission and
was startled to see they had already lost it. This allows them to keep
the inhaler you find for them.

Attempted fix for #31773. I have detailed some problems with this commit
in comments on that issue.

* Not take away all the player's inhalers

This may be a case of the workaround being worse than what's being
worked around. Will find a better solution.

* Don't sell inhaler by individual charges

When using this function to sell an inhaler to an NPC, `u.has_charges`
is (apparently) returning true (since player has inhaler charges),
although the inhaler itself is not counted by charges according to
@BevapDin.

This change makes `set_u_sell_item` follow more closely the pattern of
`set_u_buy_item`, by merging the two separate `if/else` blocks into one,
handling both the charge adjustment and `p.i_add` to transfer them item.

Most importantly, the first condition for doing charge-based adjustment
is whether `count_by_charges()` is true. I've preserved the check for
`u.has_charges`, because we need to ensure the requested number of
charges are available.
---
 data/json/npcs/missiondef.json |  6 +++---
 src/npctalk.cpp                | 26 ++++++++++----------------
 2 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/data/json/npcs/missiondef.json b/data/json/npcs/missiondef.json
index f656fa048fd06..8ac4bf289a13c 100644
--- a/data/json/npcs/missiondef.json
+++ b/data/json/npcs/missiondef.json
@@ -28,11 +28,11 @@
     "id": "MISSION_GET_INHALER",
     "type": "mission_definition",
     "name": "Find Inhaler",
-    "goal": "MGOAL_FIND_ITEM",
+    "goal": "MGOAL_CONDITION",
+    "goal_condition": { "u_has_item": "inhaler" },
     "difficulty": 2,
     "value": 150000,
     "urgent": true,
-    "item": "inhaler",
     "origins": [ "ORIGIN_OPENER_NPC", "ORIGIN_ANY_NPC" ],
     "deadline_low": 30,
     "deadline_high": 48,
@@ -55,7 +55,7 @@
         { "clear_npc_rule": "investigate_noises" }
       ]
     },
-    "end": { "effect": [ "stop_guard", { "set_npc_rule": "investigate_noises" }, "npc_thankful" ] },
+    "end": { "effect": [ "stop_guard", { "set_npc_rule": "investigate_noises" }, { "u_sell_item": "inhaler" }, "npc_thankful" ] },
     "fail": { "effect": "npc_die" }
   },
   {
diff --git a/src/npctalk.cpp b/src/npctalk.cpp
index 48e4a95873beb..057d64175f359 100644
--- a/src/npctalk.cpp
+++ b/src/npctalk.cpp
@@ -1970,31 +1970,25 @@ void talk_effect_fun_t::set_u_sell_item( const std::string &item_name, int cost,
     function = [item_name, cost, count]( const dialogue & d ) {
         npc &p = *d.beta;
         player &u = *d.alpha;
-        item old_item = item( item_name, calendar::turn );
-        if( u.has_charges( item_name, count ) ) {
-            u.use_charges( item_name, count );
+        if( item::count_by_charges( item_name ) && u.has_charges( item_name, count ) ) {
+            for( const item &it : u.use_charges( item_name, count ) ) {
+                p.i_add( it );
+            }
         } else if( u.has_amount( item_name, count ) ) {
-            u.use_amount( item_name, count );
+            for( const item &it : u.use_amount( item_name, count ) ) {
+                p.i_add( it );
+            }
         } else {
             //~ %1$s is a translated item name
-            popup( _( "You don't have a %1$s!" ), old_item.tname() );
+            popup( _( "You don't have a %1$s!" ), item::nname( item_name ) );
             return;
         }
-        if( old_item.count_by_charges() ) {
-            old_item.mod_charges( count - 1 );
-            p.i_add( old_item );
-        } else {
-            for( int i_cnt = 0; i_cnt < count; i_cnt++ ) {
-                p.i_add( old_item );
-            }
-        }
-
         if( count == 1 ) {
             //~ %1%s is the NPC name, %2$s is an item
-            popup( _( "You give %1$s a %2$s." ), p.name, old_item.tname() );
+            popup( _( "You give %1$s a %2$s." ), p.name, item::nname( item_name ) );
         } else {
             //~ %1%s is the NPC name, %2$d is a number of items, %3$s are items
-            popup( _( "You give %1$s %2$d %3$s." ), p.name, count, old_item.tname() );
+            popup( _( "You give %1$s %2$d %3$s." ), p.name, count, item::nname( item_name, count ) );
         }
         p.op_of_u.owed += cost;
     };