From ebe82db22f1140f7f7629593ded04ec01cdf4dfd Mon Sep 17 00:00:00 2001
From: BrooklynS <brooklyntstine@yahoo.com>
Date: Tue, 5 Sep 2017 02:21:33 -0700
Subject: [PATCH] Remove duplicate warning prompts for Amulet and Ring removals
 with negative props (*Contam, *Drain, etc).

In item-use.cc, puton_ring and remove_ring were re-running the same checks when JewelleryOnDelay was finished. Instead of having check_warning_inscriptions check the previous slot item when running OPER_PUTON, let the OPER_REMOVE call do the checking for itself. puton_ring already had a bunch of logic for determining a viable slot, so let it do the heavy lifting instead of check_warning_inscription.

Renamed puton_ring and remove_ring to puton_jewellery and remove_jewellery respectively.

This issue could be duplicated by having an Amulet already equipped and attempting to equip another Amulet with *Contam.
---
 crawl-ref/source/delay.cc    |  5 +++-
 crawl-ref/source/describe.cc |  4 +--
 crawl-ref/source/god-abil.cc |  3 ++-
 crawl-ref/source/invent.cc   | 19 ++------------
 crawl-ref/source/item-use.cc | 60 +++++++++++++++++++++++++++-----------------
 crawl-ref/source/item-use.h  |  4 +--
 crawl-ref/source/items.cc    |  2 +-
 crawl-ref/source/l-item.cc   |  4 +--
 crawl-ref/source/main.cc     |  4 +--
 9 files changed, 54 insertions(+), 51 deletions(-)

diff --git a/crawl-ref/source/delay.cc b/crawl-ref/source/delay.cc
index 2c20ee8328..d5ac27c134 100644
--- a/crawl-ref/source/delay.cc
+++ b/crawl-ref/source/delay.cc
@@ -728,7 +728,10 @@ void JewelleryOnDelay::finish()
 #ifdef USE_SOUND
     parse_sound(WEAR_JEWELLERY_SOUND);
 #endif
-    puton_ring(jewellery.link, false);
+
+    // Since the player has already been warned about putting on this
+    // item, skip subsequent warning checks.
+    puton_jewellery(jewellery.link, false, false);
 }
 
 void ArmourOnDelay::finish()
diff --git a/crawl-ref/source/describe.cc b/crawl-ref/source/describe.cc
index 8f095c590a..79e129aa24 100644
--- a/crawl-ref/source/describe.cc
+++ b/crawl-ref/source/describe.cc
@@ -2304,8 +2304,8 @@ static bool _do_action(item_def &item, const vector<command_type>& actions, int
     case CMD_EVOKE:            evoke_item(slot);                    break;
     case CMD_EAT:              eat_food(slot);                      break;
     case CMD_READ:             read(&item);                         break;
-    case CMD_WEAR_JEWELLERY:   puton_ring(slot);                    break;
-    case CMD_REMOVE_JEWELLERY: remove_ring(slot, true);             break;
+    case CMD_WEAR_JEWELLERY:   puton_jewellery(slot);               break;
+    case CMD_REMOVE_JEWELLERY: remove_jewellery(slot, true, true);  break;
     case CMD_QUAFF:            drink(&item);                        break;
     case CMD_DROP:             drop_item(slot, item.quantity);      break;
     case CMD_INSCRIBE_ITEM:    inscribe_item(item);                 break;
diff --git a/crawl-ref/source/god-abil.cc b/crawl-ref/source/god-abil.cc
index 7cc721d3e6..a78c919fbd 100644
--- a/crawl-ref/source/god-abil.cc
+++ b/crawl-ref/source/god-abil.cc
@@ -5786,7 +5786,8 @@ static void _extra_sacrifice_code(ability_type sac)
                      ring->name(DESC_YOUR).c_str(),
                      (you.species == SP_OCTOPODE ? "another" : "your other"),
                      you.hand_name(true).c_str());
-                puton_ring(ring_inv_slot, false);
+
+                puton_jewellery(ring_inv_slot, false, true);
             }
         }
     }
diff --git a/crawl-ref/source/invent.cc b/crawl-ref/source/invent.cc
index c33bb331e3..19cbe9a947 100644
--- a/crawl-ref/source/invent.cc
+++ b/crawl-ref/source/invent.cc
@@ -1480,23 +1480,8 @@ bool check_old_item_warning(const item_def& item,
     }
     else if (oper == OPER_PUTON) // can we safely remove old item?
     {
-        if (item.base_type != OBJ_JEWELLERY)
-            return true;
-
-        if (jewellery_is_amulet(item))
-        {
-            int equip = you.equip[EQ_AMULET];
-            if (equip == -1 || item.link == equip)
-                return true;
-
-            old_item = you.inv[equip];
-            if (!needs_handle_warning(old_item, OPER_TAKEOFF, penance))
-                return true;
-
-            prompt += "Really remove ";
-        }
-        else // rings handled in prompt_ring_to_remove
-            return true;
+        // Jewellery always returns true. It manages its own checks in puton_jewellery.
+        return true;
     }
     else // anything else doesn't have a counterpart
         return true;
diff --git a/crawl-ref/source/item-use.cc b/crawl-ref/source/item-use.cc
index fc83b1efdb..b5ed12b992 100644
--- a/crawl-ref/source/item-use.cc
+++ b/crawl-ref/source/item-use.cc
@@ -1139,7 +1139,7 @@ static bool _can_takeoff_armour(int item)
 }
 
 // TODO: It would be nice if this were made consistent with wear_armour,
-// wield_weapon, puton_ring, etc. in terms of taking a default value of -1,
+// wield_weapon, puton_jewellery, etc. in terms of taking a default value of -1,
 // which has the effect of prompting for an item to take off.
 bool takeoff_armour(int item)
 {
@@ -1527,7 +1527,7 @@ static bool _swap_rings(int ring_slot)
     // ring slot (even if we still have empty slots).
     else if (available == 1 && !Options.jewellery_prompt)
     {
-        if (!remove_ring(unwanted, false))
+        if (!remove_jewellery(unwanted, false, true))
             return false;
     }
     // We can't put a ring on without swapping - because we found
@@ -1545,7 +1545,7 @@ static bool _swap_rings(int ring_slot)
             return false;
         }
 
-        if (!remove_ring(unwanted, false))
+        if (!remove_jewellery(unwanted, false, true))
             return false;
     }
 
@@ -1617,7 +1617,7 @@ static equipment_type _choose_ring_slot()
 // - item is not already equipped in a jewellery slot
 static bool _can_puton_jewellery(int item_slot)
 {
-    // TODO: between this function, _puton_item, _swap_rings, and remove_ring,
+    // TODO: between this function, _puton_jewellery, _swap_rings, and remove_jewellery,
     // there's a bit of duplicated work, and sep. of concerns not clear
     item_def& item = you.inv[item_slot];
     if (item_slot == you.equip[EQ_WEAPON])
@@ -1686,25 +1686,33 @@ static bool _can_puton_jewellery(int item_slot)
     }
 }
 
-// Put on a particular ring or amulet
-static bool _puton_item(int item_slot, bool prompt_slot)
+/**
+* Put on a particular ring or amulet.
+*
+* @param item_slot      The jewellery item slot.
+* @param prompt_slot    Should the player be prompted which ring slot to use?
+* @param check_warning  Should the player be warned about any bad effects?
+*/
+static bool _puton_jewellery(int item_slot, bool prompt_slot, bool check_warning)
 {
     item_def& item = you.inv[item_slot];
 
     for (int eq = EQ_FIRST_JEWELLERY; eq <= EQ_LAST_JEWELLERY; eq++)
+    {
         if (item_slot == you.equip[eq])
         {
             // "Putting on" an equipped item means taking it off.
             if (Options.equip_unequip)
                 // TODO: why invert the return value here? failing to remove
                 // a ring is equivalent to successfully putting one on?
-                return !remove_ring(item_slot);
+                return !remove_jewellery(item_slot, false, check_warning);
             else
             {
                 mpr("You're already wearing that object!");
                 return false;
             }
         }
+    }
 
     if (!_can_puton_jewellery(item_slot))
         return false;
@@ -1712,10 +1720,13 @@ static bool _puton_item(int item_slot, bool prompt_slot)
     // It looks to be possible to equip this item. Before going any further,
     // we should prompt the user with any warnings that come with trying to
     // put it on.
-    if (!check_warning_inscriptions(item, OPER_PUTON))
+    if (check_warning)
     {
-        canned_msg(MSG_OK);
-        return false;
+        if (!check_warning_inscriptions(item, OPER_PUTON))
+        {
+            canned_msg(MSG_OK);
+            return false;
+        }
     }
 
     const bool is_amulet = jewellery_is_amulet(item);
@@ -1742,7 +1753,7 @@ static bool _puton_item(int item_slot, bool prompt_slot)
     else if (you.slot_item(EQ_AMULET, true))
     {
         // Remove the previous one.
-        if (!remove_ring(you.equip[EQ_AMULET], true))
+        if (!remove_jewellery(you.equip[EQ_AMULET], true, true))
             return false;
 
         // Check for stat loss.
@@ -1759,7 +1770,7 @@ static bool _puton_item(int item_slot, bool prompt_slot)
     // trying to equip.
 
     // Check for stat loss.
-    if (!_safe_to_remove_or_wear(item, false))
+    if (!_safe_to_remove_or_wear(item, false, !check_warning))
         return false;
 
     equipment_type hand_used = EQ_NONE;
@@ -1779,7 +1790,7 @@ static bool _puton_item(int item_slot, bool prompt_slot)
         // Allow swapping out a ring.
         else if (you.slot_item(hand_used, true))
         {
-            if (!remove_ring(you.equip[hand_used], false))
+            if (!remove_jewellery(you.equip[hand_used], false, true))
                 return false;
 
             start_delay<JewelleryOnDelay>(1, item);
@@ -1820,7 +1831,7 @@ static bool _puton_item(int item_slot, bool prompt_slot)
 }
 
 // Put on a ring or amulet. (If slot is -1, first prompt for which item to put on)
-bool puton_ring(int slot, bool allow_prompt)
+bool puton_jewellery(int slot, bool allow_prompt, bool check_warning)
 {
     int item_slot;
 
@@ -1850,12 +1861,12 @@ bool puton_ring(int slot, bool allow_prompt)
 
     bool prompt = allow_prompt ? Options.jewellery_prompt : false;
 
-    return _puton_item(item_slot, prompt);
+    return _puton_jewellery(item_slot, prompt, check_warning);
 }
 
 // Remove the amulet/ring at given inventory slot (or, if slot is -1, prompt
 // for which piece of jewellery to remove)
-bool remove_ring(int slot, bool announce)
+bool remove_jewellery(int slot, bool announce, bool check_warning)
 {
     equipment_type hand_used = EQ_NONE;
     int ring_wear_2;
@@ -1946,11 +1957,14 @@ bool remove_ring(int slot, bool announce)
         return false;
     }
 
-    if (!check_warning_inscriptions(you.inv[you.equip[hand_used]],
-                                    OPER_REMOVE))
+    if (check_warning)
     {
-        canned_msg(MSG_OK);
-        return false;
+        if (!check_warning_inscriptions(you.inv[you.equip[hand_used]],
+            OPER_REMOVE))
+        {
+            canned_msg(MSG_OK);
+            return false;
+        }
     }
 
     if (you.inv[you.equip[hand_used]].cursed())
@@ -3328,9 +3342,9 @@ void tile_item_use(int idx)
 
         case OBJ_JEWELLERY:
             if (equipped && !equipped_weapon)
-                remove_ring(idx);
-            else if (check_warning_inscriptions(item, OPER_PUTON))
-                puton_ring(idx);
+                remove_jewellery(idx);
+            else
+                puton_jewellery(idx);
             return;
 
         case OBJ_POTIONS:
diff --git a/crawl-ref/source/item-use.h b/crawl-ref/source/item-use.h
index 3f7a651753..d04c12b444 100644
--- a/crawl-ref/source/item-use.h
+++ b/crawl-ref/source/item-use.h
@@ -25,14 +25,14 @@ bool god_hates_brand(const int brand);
 
 bool safe_to_remove(const item_def &item, bool quiet = false);
 
-bool puton_ring(int slot = -1, bool allow_prompt = true);
+bool puton_jewellery(int slot, bool prompt_slot = true, bool check_warning = true);
 
 void read(item_def* scroll = nullptr);
 void read_scroll(item_def& scroll);
 bool player_can_read();
 string cannot_read_item_reason(const item_def &item);
 
-bool remove_ring(int slot = -1, bool announce = false);
+bool remove_jewellery(int slot = -1, bool announce = false, bool check_warning = true);
 
 bool wear_armour(int slot = -1);
 
diff --git a/crawl-ref/source/items.cc b/crawl-ref/source/items.cc
index 9dcc4e734e..d169cc355b 100644
--- a/crawl-ref/source/items.cc
+++ b/crawl-ref/source/items.cc
@@ -2451,7 +2451,7 @@ bool drop_item(int item_dropped, int quant_drop)
     {
         if (!Options.easy_unequip)
             mpr("You will have to take that off first.");
-        else if (remove_ring(item_dropped, true))
+        else if (remove_jewellery(item_dropped, true, true))
         {
             // The delay handles the case where the item disappeared.
             start_delay<DropItemDelay>(1, item);
diff --git a/crawl-ref/source/l-item.cc b/crawl-ref/source/l-item.cc
index 6c4b1dddf4..5efacdaab0 100644
--- a/crawl-ref/source/l-item.cc
+++ b/crawl-ref/source/l-item.cc
@@ -165,7 +165,7 @@ static int l_item_do_puton(lua_State *ls)
     if (!item || !in_inventory(*item))
         return 0;
 
-    lua_pushboolean(ls, puton_ring(item->link));
+    lua_pushboolean(ls, puton_jewellery(item->link));
     return 1;
 }
 
@@ -198,7 +198,7 @@ static int l_item_do_remove(lua_State *ls)
     if (eq == EQ_WEAPON)
         result = wield_weapon(true, SLOT_BARE_HANDS);
     else if (eq >= EQ_FIRST_JEWELLERY && eq <= EQ_LAST_JEWELLERY)
-        result = remove_ring(item->link);
+        result = remove_jewellery(item->link);
     else
         result = takeoff_armour(item->link);
     lua_pushboolean(ls, result);
diff --git a/crawl-ref/source/main.cc b/crawl-ref/source/main.cc
index e9de1c637b..c42a67530c 100644
--- a/crawl-ref/source/main.cc
+++ b/crawl-ref/source/main.cc
@@ -1834,12 +1834,12 @@ void process_command(command_type cmd)
     case CMD_QUAFF:                drink();                  break;
     case CMD_READ:                 read();                   break;
     case CMD_REMOVE_ARMOUR:        _do_remove_armour();      break;
-    case CMD_REMOVE_JEWELLERY:     remove_ring();            break;
+    case CMD_REMOVE_JEWELLERY:     remove_jewellery();       break;
     case CMD_SHOUT:                issue_orders();           break;
     case CMD_THROW_ITEM_NO_QUIVER: throw_item_no_quiver();   break;
     case CMD_WEAPON_SWAP:          wield_weapon(true);       break;
     case CMD_WEAR_ARMOUR:          wear_armour();            break;
-    case CMD_WEAR_JEWELLERY:       puton_ring(-1);           break;
+    case CMD_WEAR_JEWELLERY:       puton_jewellery(-1);      break;
     case CMD_WIELD_WEAPON:         wield_weapon(false);      break;
     case CMD_ZAP_WAND:             zap_wand();               break;
 
-- 
2.14.1.windows.1