fix #H4418/#H4419 - #tip of gold inside shops

The #tip command tries to reduce verbosity by formatting drop messages
with just the object name instead of with full sentences, yielding
  Objects spill out: obj1, obj2, obj3, ..., objN.
where the trailing comma or period is included with each successive
object.  If an intervening message occurs, such "25 zorkmids are
added to your credit", the rest of the objects will no longer be
extending the original sentence and end up looking silly.
  Objects spill out: obj1, obj2,--More--
  25 zorkmids are added to your credit.  obj3, ..., objN.
This fix causes the post-interruption messages to revert to verbose
format.
  Objects spill out: obj1, obj2,--More--
  25 zorkmids are added to your credit.--More--
  obj3 drops to the floor.--More--
...
  objN drops to the floor.

The interrupting message still follows the comma of the partial
sentence, but I don't see any sane way to fix that other than to
abandon the terse format altogether, and doing that makes #tip way
too verbose when the container has a lot of items in it.  But #tip
inside shops now does that, since there will always be buy/not-
interested feedback interrupting the terse format in that situation.
For other situations, a full sentence message might end up following
a partial sentence list of dropped items.

There was a more significant bug.  Dropping a hero-owned container
with gold in it onto shop floor sold the gold to shk, giving hero
credit.  Subsequent #tip gave the hero credit for that same gold
when it spilled out.  addtobill(obj) relies on obj->ox,oy to
determine whether events are taking place in a shop, and #tip was
relying on placement onto floor to set those, too late for shop
billing.  The fix yields suboptimal results:  you're given credit
when you drop the container, then during #tip when you spill the
contents, credit for the gold is removed, then new credit for it
is given.  That's down to shop insanity, not tipping behavior.

diff --git a/doc/fixes36.1 b/doc/fixes36.1
index a7d320c31a2bca9c545807e4928effa76c5a74af..3b8bd6ca165a4ebf32a28055c793ca1c9d23391e 100644 (file)
--- a/doc/fixes36.1
+++ b/doc/fixes36.1
@@ -305,6 +305,13 @@ make fireballs or cones of cold cast a skilled or higher not go through walls
 prevent flying monsters from hovering over unreachable underwater objects
 lembas wafer gives increased nutrition to elves, reduced nutrition to orcs;
        cram ration gives increase nutrition to dwarves
+when #tip's terse object drop format got interrupted by a regular message,
+       it continued using "obj2, obj3, ..." for subsequent objects, where
+       the sentence grammar no longer made sense (the interrupting message
+       still follows the comma of a partial sentence--attempting to fix that
+       seems hopeless; leaving it is better than always using verbose format)
+for #tip inside shop, credit was incorrectly given for spilled gold if that
+       gold's stale location coordinates didn't happen to be inside the shop
 
 
 Fixes to Post-3.6.0 Problems that Were Exposed Via git Repository

diff --git a/include/flag.h b/include/flag.h
index a8a6d85939ba566a48987679f56243121ef413ac..55a544471bbb7d6cdf518e597d2a06ee21274f1b 100644 (file)
--- a/include/flag.h
+++ b/include/flag.h
@@ -382,6 +382,7 @@ extern NEARDATA struct instance_flags iflags;
 #define PLNMSG_TOWER_OF_FLAME      2 /* scroll of fire */
 #define PLNMSG_CAUGHT_IN_EXPLOSION 3 /* explode() feedback */
 #define PLNMSG_OBJ_GLOWS           4 /* "the <obj> glows <color>" */
+#define PLNMSG_OBJNAM_ONLY         5 /* xname/doname only, for #tip */
     /* Usage:
      *  pline("some message");
      *    pline: vsprintf + putstr + iflags.last_msg = PLNMSG_UNKNOWN;

diff --git a/src/pickup.c b/src/pickup.c
index 1fccc65d6914b7e55d831d59fa4423a2644d87d0..0f8b5b5d467fc39ba03ab0b4ecfc2e32615df758 100644 (file)
--- a/src/pickup.c
+++ b/src/pickup.c
@@ -2902,20 +2902,27 @@ tipcontainer(box)
 struct obj *box; /* or bag */
 {
     xchar ox = u.ux, oy = u.uy; /* #tip only works at hero's location */
-    boolean empty_it = FALSE,
-            /* Shop handling:  can't rely on the container's own unpaid
-               or no_charge status because contents might differ with it.
-               A carried container's contents will be flagged as unpaid
-               or not, as appropriate, and need no special handling here.
-               Items owned by the hero get sold to the shop without
-               confirmation as with other uncontrolled drops.  A floor
-               container's contents will be marked no_charge if owned by
-               hero, otherwise they're owned by the shop.  By passing
-               the contents through shop billing, they end up getting
-               treated the same as in the carried case.   We do so one
-               item at a time instead of doing whole container at once
-               to reduce the chance of exhausting shk's billing capacity. */
-            maybeshopgoods = !carried(box) && costly_spot(ox, oy);
+    boolean empty_it = FALSE, maybeshopgoods;
+
+    /* box is either held or on floor at hero's spot; no need to check for
+       nesting; when held, we need to update its location to match hero's;
+       for floor, the coordinate updating is redundant */
+    if (get_obj_location(box, &ox, &oy, 0))
+        box->ox = ox, box->oy = oy;
+
+    /* Shop handling:  can't rely on the container's own unpaid
+       or no_charge status because contents might differ with it.
+       A carried container's contents will be flagged as unpaid
+       or not, as appropriate, and need no special handling here.
+       Items owned by the hero get sold to the shop without
+       confirmation as with other uncontrolled drops.  A floor
+       container's contents will be marked no_charge if owned by
+       hero, otherwise they're owned by the shop.  By passing
+       the contents through shop billing, they end up getting
+       treated the same as in the carried case.   We do so one
+       item at a time instead of doing whole container at once
+       to reduce the chance of exhausting shk's billing capacity. */
+    maybeshopgoods = !carried(box) && costly_spot(box->ox, box->oy);
 
     /* caveat: this assumes that cknown, lknown, olocked, and otrapped
        fields haven't been overloaded to mean something special for the
@@ -2978,7 +2985,7 @@ struct obj *box; /* or bag */
 
     if (empty_it) {
         struct obj *otmp, *nobj;
-        boolean verbose = FALSE, highdrop = !can_reach_floor(TRUE),
+        boolean terse, highdrop = !can_reach_floor(TRUE),
                 altarizing = IS_ALTAR(levl[ox][oy].typ),
                 cursed_mbag = (Is_mbag(box) && box->cursed);
         int held = carried(box);
@@ -2986,20 +2993,27 @@ struct obj *box; /* or bag */
 
         if (u.uswallow)
             highdrop = altarizing = FALSE;
+        terse = !(highdrop || altarizing || costly_spot(box->ox, box->oy));
         box->cknown = 1;
+        /* Terse formatting is
+         * "Objects spill out: obj1, obj2, obj3, ..., objN."
+         * If any other messages intervene between objects, we revert to
+         * "ObjK drops to the floor.", "ObjL drops to the floor.", &c.
+         */
         pline("%s out%c",
               box->cobj->nobj ? "Objects spill" : "An object spills",
-              !(highdrop || altarizing) ? ':' : '.');
+              terse ? ':' : '.');
         for (otmp = box->cobj; otmp; otmp = nobj) {
             nobj = otmp->nobj;
             obj_extract_self(otmp);
+            otmp->ox = box->ox, otmp->oy = box->oy;
 
             if (box->otyp == ICE_BOX) {
                 removed_from_icebox(otmp); /* resume rotting for corpse */
             } else if (cursed_mbag && !rn2(13)) {
                 loss += mbag_item_gone(held, otmp);
                 /* abbreviated drop format is no longer appropriate */
-                verbose = TRUE;
+                terse = FALSE;
                 continue;
             }
 
@@ -3012,14 +3026,18 @@ struct obj *box; /* or bag */
                 /* might break or fall down stairs; handles altars itself */
                 hitfloor(otmp);
             } else {
-                if (altarizing)
+                if (altarizing) {
                     doaltarobj(otmp);
-                else if (verbose)
+                } else if (!terse) {
                     pline("%s %s to the %s.", Doname2(otmp),
                           otense(otmp, "drop"), surface(ox, oy));
-                else
+                } else {
                     pline("%s%c", doname(otmp), nobj ? ',' : '.');
+                    iflags.last_msg = PLNMSG_OBJNAM_ONLY;
+                }
                 dropy(otmp);
+                if (iflags.last_msg != PLNMSG_OBJNAM_ONLY)
+                    terse = FALSE; /* terse formatting has been interrupted */
             }
             if (maybeshopgoods)
                 iflags.suppress_price--; /* reset */

diff --git a/src/shk.c b/src/shk.c
index 586f5de6e7e3029f190ad8cca8bdc3e9b21e7bd2..a914a56d7159cd6beb77a593049cca35da51a010 100644 (file)
--- a/src/shk.c
+++ b/src/shk.c
@@ -2130,9 +2130,10 @@ boolean unpaid_only;
     return price;
 }
 
+/* count amount of gold inside container 'obj' and any nested containers */
 long
 contained_gold(obj)
-register struct obj *obj;
+struct obj *obj;
 {
     register struct obj *otmp;
     register long value = 0L;
@@ -2867,12 +2868,12 @@ xchar x, y;
     boolean isgold = (obj->oclass == COIN_CLASS);
     boolean only_partially_your_contents = FALSE;
 
+    if (!*u.ushops) /* do cheapest exclusion test first */
+        return;
     if (!(shkp = shop_keeper(*in_rooms(x, y, SHOPBASE))) || !inhishop(shkp))
         return;
     if (!costly_spot(x, y))
         return;
-    if (!*u.ushops)
-        return;
 
     if (obj->unpaid && !container && !isgold) {
         sub_one_frombill(obj, shkp);