From 70f263f0cee404335e4f5248f75324ca2184c4f1 Mon Sep 17 00:00:00 2001
From: Michael Meyer <me@entrez.cc>
Date: Fri, 14 Jan 2022 09:38:00 -0500
Subject: [PATCH] Fix: unpaid_cost regression

Recent commit 5c14f3e fixed an issue with unpaid_cost when the hero is
in two shops at once (e.g. phasing through a shared wall), but
introduced new problems with finding the cost of a container with unpaid
contents -- an unpaid item in a player-owned container would generate
the same "unpaid_cost: object wasn't on any bill" impossible as the
issue the commit fixed.

The main issue was that finding the bill entry used to depend on finding
the shopkeeper, but after the change, finding the shopkeeper depended on
finding the bill entry; for a container with unpaid items which has a
shopkeeper, but no bill entry, this caused issues.

Revert part of 5c14f3e and tackle the problem another way, which should
correctly handle unpaid items in inventory or a container, even if they
belong to a mixture of different shops and/or the hero is on a space
shared between multiple shops.
---
 src/shk.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/src/shk.c b/src/shk.c
index 06501cba5..3b8dfc953 100644
--- a/src/shk.c
+++ b/src/shk.c
@@ -2421,6 +2421,7 @@ unpaid_cost(
 {
     struct bill_x *bp = (struct bill_x *) 0;
     struct monst *shkp;
+    char *shop;
     long amt = 0L;
 
 #if 0   /* if two shops share a wall, this might find wrong shk */
@@ -2430,22 +2431,28 @@ unpaid_cost(
         ox = u.ux, oy = u.uy; /* (shouldn't happen) */
     if ((shkp = shop_keeper(*in_rooms(ox, oy, SHOPBASE))) != 0) {
         bp = onbill(unp_obj, shkp, TRUE);
-    } else /* didn't find shk?  try searching bills */
+    } else {
+        /* didn't find shk?  try searching bills */
+        for (shkp = next_shkp(fmon, TRUE); shkp;
+             shkp = next_shkp(shkp->nmon, TRUE))
+            if ((bp = onbill(unp_obj, shkp, TRUE)) != 0)
+                break;
+    }
 #endif
-    for (shkp = next_shkp(fmon, TRUE); shkp;
-         shkp = next_shkp(shkp->nmon, TRUE))
-        if ((bp = onbill(unp_obj, shkp, TRUE)) != 0)
-            break;
+    for (shop = u.ushops; *shop; shop++) {
+        if ((shkp = shop_keeper(*shop))) {
+            if ((bp = onbill(unp_obj, shkp, TRUE)))
+                amt = unp_obj->quan * bp->price;
+            if (include_contents && Has_contents(unp_obj))
+                amt = contained_cost(unp_obj, shkp, amt, FALSE, TRUE);
+            if (bp || (!unp_obj->unpaid && amt))
+                break;
+        }
+    }
 
     /* onbill() gave no message if unexpected problem occurred */
-    if (!shkp || (unp_obj->unpaid && !bp)) {
+    if (!shkp || (unp_obj->unpaid && !bp))
         impossible("unpaid_cost: object wasn't on any bill.");
-    } else {
-        if (bp)
-            amt = unp_obj->quan * bp->price;
-        if (include_contents && Has_contents(unp_obj))
-            amt = contained_cost(unp_obj, shkp, amt, FALSE, TRUE);
-    }
     return amt;
 }