breaking wielded fragile item against iron bars

Reported by entrez, wielding something fragile (potion of acid
perhaps), and using F to smash it against iron bars called breaktest()
directly, then a second time indirectly through hero_breaks() via
hit_bars().  There is a random chance to resist breaking (99% for
artifacts, 1% for other items) so breaktest() might say that something
will break on the first call and that it will not break on the second
call, or vice versa.  That could remove uwep from inventory then leave
it in limbo without destroying it, or destroy uwep without removing it
from inventory first triggering impossible "obfree: deleting worn obj".

diff --git a/doc/fixes37.0 b/doc/fixes37.0
index 0096e62df31a16bb325d1a64b29ffce3af3f024d..0a81d02d1aff1f90508fe4fe16c05bdf337c6190 100644 (file)
--- a/doc/fixes37.0
+++ b/doc/fixes37.0
@@ -655,6 +655,9 @@ if vault guard arrives on a boulder in a breach in the vault wall when coming
 when vault walls are repaired, destroy any rocks or boulders at their spots
 melting ice timer could persist after the ice was gone from digging or from an
        exploding land mine
+using 'F'orcefight against iron bars while wielding something breakable could
+       yield erratic outcome because non-deterministic breaktest() was being
+       called twice and could yield results that conflicted
 
 
 Fixes to 3.7.0-x Problems that Were Exposed Via git Repository

diff --git a/include/extern.h b/include/extern.h
index f59df229c448e2fc10fa5090ebd1462f0cb25d4e..cba3270fd743e7d3db16b0a93f99f86ead230756 100644 (file)
--- a/include/extern.h
+++ b/include/extern.h
@@ -565,7 +565,7 @@ extern boolean throwing_weapon(struct obj *);
 extern void throwit(struct obj *, long, boolean, struct obj *);
 extern int omon_adj(struct monst *, struct obj *, boolean);
 extern int thitmonst(struct monst *, struct obj *);
-extern int hero_breaks(struct obj *, xchar, xchar, boolean);
+extern int hero_breaks(struct obj *, xchar, xchar, unsigned);
 extern int breaks(struct obj *, xchar, xchar);
 extern void release_camera_demon(struct obj *, xchar, xchar);
 extern void breakobj(struct obj *, xchar, xchar, boolean, boolean);
@@ -1637,7 +1637,7 @@ extern int breamm(struct monst *, struct attack *, struct monst *);
 extern void m_useupall(struct monst *, struct obj *);
 extern void m_useup(struct monst *, struct obj *);
 extern void m_throw(struct monst *, int, int, int, int, int, struct obj *);
-extern void hit_bars(struct obj **, int, int, int, int, boolean, boolean);
+extern void hit_bars(struct obj **, int, int, int, int, unsigned);
 extern boolean hits_bars(struct obj **, int, int, int, int, int, int);
 
 /* ### muse.c ### */

diff --git a/include/hack.h b/include/hack.h
index 34eafe7a7d58c3e23ca189e79dd04d3433e89f4b..0d90867a34eff92c30855db244d7996f13679bdb 100644 (file)
--- a/include/hack.h
+++ b/include/hack.h
@@ -522,6 +522,15 @@ enum bodypart_types {
                                exiting early with "You don't have anything to
                                foo" if nothing in inventory is valid) */
 
+/* flags for hero_breaks() and hits_bars(); BRK_KNOWN* let callers who have
+   already called breaktest() prevent it from being called again since it
+   has a random factor which makes it be non-deterministic */
+#define BRK_BY_HERO        1
+#define BRK_FROM_INV       2
+#define BRK_KNOWN2BREAK    4
+#define BRK_KNOWN2NOTBREAK 8
+#define BRK_KNOWN_OUTCOME  (BRK_KNOWN2BREAK | BRK_KNOWN2NOTBREAK)
+
 /* values returned from getobj() callback functions */
 enum getobj_callback_returns {
     /* generally invalid - can't be used for this purpose. will give a "silly

diff --git a/src/dokick.c b/src/dokick.c
index 48199babac0224d391ef1384e95fd39c3905cb0a..b55395dcb8ffd0885b5b73a67447b844caf3c087 100644 (file)
--- a/src/dokick.c
+++ b/src/dokick.c
@@ -618,7 +618,7 @@ really_kick_object(xchar x, xchar y)
     }
 
     /* fragile objects should not be kicked */
-    if (hero_breaks(g.kickedobj, g.kickedobj->ox, g.kickedobj->oy, FALSE))
+    if (hero_breaks(g.kickedobj, g.kickedobj->ox, g.kickedobj->oy, 0))
         return 1;
 
     /* too heavy to move.  range is calculated as potential distance from

diff --git a/src/dothrow.c b/src/dothrow.c
index b170e29697ede4fabc989fb9a9edda0ce8fdc41e..14b0c14e83ff86d055701d642b38fdeff003df56 100644 (file)
--- a/src/dothrow.c
+++ b/src/dothrow.c
@@ -550,7 +550,7 @@ hitfloor(struct obj *obj,
         pline("%s %s the %s.", Doname2(obj), otense(obj, "hit"), surf);
     }
 
-    if (hero_breaks(obj, u.ux, u.uy, TRUE))
+    if (hero_breaks(obj, u.ux, u.uy, BRK_FROM_INV))
         return;
     if (ship_object(obj, u.ux, u.uy, FALSE))
         return;
@@ -2053,13 +2053,20 @@ gem_accept(register struct monst *mon, register struct obj *obj)
 int
 hero_breaks(struct obj *obj,
             xchar x, xchar y, /* object location (ox, oy may not be right) */
-            boolean from_invent) /* thrown or dropped by player;
-                                    maybe on shop bill */
+            unsigned breakflags)
 {
-    boolean in_view = Blind ? FALSE : (from_invent || cansee(x, y));
-
-    if (!breaktest(obj))
+    /* from_invent: thrown or dropped by player; maybe on shop bill;
+       by-hero is implicit so callers don't need to specify BRK_BY_HERO */
+    boolean from_invent = (breakflags & BRK_FROM_INV) != 0,
+            in_view = Blind ? FALSE : (from_invent || cansee(x, y));
+    unsigned brk = (breakflags & BRK_KNOWN_OUTCOME);
+
+    /* only call breaktest if caller hasn't already specified the outcome */
+    if (!brk)
+        brk = breaktest(obj) ? BRK_KNOWN2BREAK : BRK_KNOWN2NOTBREAK;
+    if (brk == BRK_KNOWN2NOTBREAK)
         return 0;
+
     breakmsg(obj, in_view);
     breakobj(obj, x, y, TRUE, from_invent);
     return 1;

diff --git a/src/hack.c b/src/hack.c
index 207b4740b57fbffee729470c53eae4404b5c0678..410c69d0c15b6256d3421d1635ea43c4bb0192a6 100644 (file)
--- a/src/hack.c
+++ b/src/hack.c
@@ -1770,30 +1770,29 @@ domove_core(void)
         if (g.context.forcefight || !mtmp->mundetected || sensemon(mtmp)
             || ((hides_under(mtmp->data) || mtmp->data->mlet == S_EEL)
                 && !is_safemon(mtmp))) {
-
             /* target monster might decide to switch places with you... */
-            if (mtmp->data == &mons[PM_DISPLACER_BEAST] && !rn2(2)
-                && mtmp->mux == u.ux0 && mtmp->muy == u.uy0
-                && mtmp->mcanmove && !mtmp->msleeping && !mtmp->meating
-                && !mtmp->mtrapped && !u.utrap && !u.ustuck && !u.usteed
-                && !(u.dx && u.dy
-                     && (NODIAG(u.umonnum)
-                         || (bad_rock(mtmp->data, x, u.uy0)
-                             && bad_rock(mtmp->data, u.ux0, y))
-                         || (bad_rock(g.youmonst.data, u.ux0, y)
-                             && bad_rock(g.youmonst.data, x, u.uy0))))
-                && goodpos(u.ux0, u.uy0, mtmp, GP_ALLOW_U))
-                displaceu = TRUE;
-
-            /* try to attack; note that it might evade;
+            displaceu = (mtmp->data == &mons[PM_DISPLACER_BEAST] && !rn2(2)
+                         && mtmp->mux == u.ux0 && mtmp->muy == u.uy0
+                         && mtmp->mcanmove && !mtmp->msleeping
+                         && !mtmp->meating && !mtmp->mtrapped
+                         && !u.utrap && !u.ustuck && !u.usteed
+                         && !(u.dx && u.dy
+                              && (NODIAG(u.umonnum)
+                                  || (bad_rock(mtmp->data, x, u.uy0)
+                                      && bad_rock(mtmp->data, u.ux0, y))
+                                  || (bad_rock(g.youmonst.data, u.ux0, y)
+                                      && bad_rock(g.youmonst.data, x, u.uy0))))
+                         && goodpos(u.ux0, u.uy0, mtmp, GP_ALLOW_U));
+            /* if not displacing, try to attack; note that it might evade;
                also, we don't attack tame when _safepet_ */
-            else if (do_attack(mtmp))
+            if (!displaceu && do_attack(mtmp))
                 return;
         }
     }
 
     if (g.context.forcefight && levl[x][y].typ == IRONBARS && uwep) {
         struct obj *obj = uwep;
+        unsigned breakflags = (BRK_BY_HERO | BRK_FROM_INV);
 
         if (breaktest(obj)) {
             if (obj->quan > 1L)
@@ -1801,8 +1800,12 @@ domove_core(void)
             else
                 setuwep((struct obj *)0);
             freeinv(obj);
+            breakflags |= BRK_KNOWN2BREAK;
+        } else {
+            breakflags |= BRK_KNOWN2NOTBREAK;
         }
-        hit_bars(&obj, u.ux, u.uy, x, y, TRUE, TRUE);
+
+        hit_bars(&obj, u.ux, u.uy, x, y, breakflags);
         return;
     }
 

diff --git a/src/mthrowu.c b/src/mthrowu.c
index 3644ebf0dc6c4dfe7fec9e3e6a993948087cd587..b8c3ba174e582d7a1f0b1b0e58f07afd33e9a3b7 100644 (file)
--- a/src/mthrowu.c
+++ b/src/mthrowu.c
@@ -227,9 +227,10 @@ monshoot(struct monst* mtmp, struct obj* otmp, struct obj* mwep)
                      mtarg ? mtarg->mx : mtmp->mux,
                      mtarg ? mtarg->my : mtmp->muy),
         multishot = monmulti(mtmp, otmp, mwep);
-        /*
-         * Caller must have called linedup() to set up g.tbx, g.tby.
-         */
+
+    /*
+     * Caller must have called linedup() to set up <g.tbx, g.tby>.
+     */
 
     if (canseemon(mtmp)) {
         const char *onm;
@@ -445,7 +446,7 @@ ohitmon(
     return 0;
 }
 
-#define MT_FLIGHTCHECK(pre) \
+#define MT_FLIGHTCHECK(pre,forcehit) \
     (/* missile hits edge of screen */                                  \
      !isok(g.bhitpos.x + dx, g.bhitpos.y + dy)                          \
      /* missile hits the wall */                                        \
@@ -459,7 +460,7 @@ ohitmon(
          && hits_bars(&singleobj,                                       \
                       g.bhitpos.x, g.bhitpos.y,                         \
                       g.bhitpos.x + dx, g.bhitpos.y + dy,               \
-                      ((pre) ? 0 : !rn2(5)), 0))                        \
+                      ((pre) ? 0 : forcehit), 0))                       \
      /* Thrown objects "sink" */                                        \
      || (!(pre) && IS_SINK(levl[g.bhitpos.x][g.bhitpos.y].typ))         \
      )
@@ -474,6 +475,7 @@ m_throw(
 {
     struct monst *mtmp;
     struct obj *singleobj;
+    boolean forcehit;
     char sym = obj->oclass;
     int hitu = 0, oldumort, blindinc = 0;
 
@@ -525,7 +527,7 @@ m_throw(
         }
     }
 
-    if (MT_FLIGHTCHECK(TRUE)) {
+    if (MT_FLIGHTCHECK(TRUE, 0)) {
         (void) drop_throw(singleobj, 0, g.bhitpos.x, g.bhitpos.y);
         return;
     }
@@ -666,7 +668,9 @@ m_throw(
             }
         }
 
-        if (!range || MT_FLIGHTCHECK(FALSE)) { /* end of path or blocked */
+        forcehit = !rn2(5);
+        if (!range || MT_FLIGHTCHECK(FALSE, forcehit)) {
+            /* end of path or blocked */
             if (singleobj) { /* hits_bars might have destroyed it */
                 /* note: pline(The(missile)) rather than pline_The(missile)
                    in order to get "Grimtooth" rather than "The Grimtooth" */
@@ -826,12 +830,12 @@ breamm(struct monst* mtmp, struct attack* mattk, struct monst* mtarg)
             return MM_MISS;
         }
 
-       /* if we've seen the actual resistance, don't bother, or
-        * if we're close by and they reflect, just jump the player */
-       if (m_seenres(mtmp, cvt_adtyp_to_mseenres(typ))
-           || (m_seenres(mtmp, M_SEEN_REFL)
+        /* if we've seen the actual resistance, don't bother, or
+           if we're close by and they reflect, just jump the player */
+        if (m_seenres(mtmp, cvt_adtyp_to_mseenres(typ))
+            || (m_seenres(mtmp, M_SEEN_REFL)
                 && monnear(mtmp, mtmp->mux, mtmp->muy)))
-           return MM_HIT;
+            return MM_HIT;
 
         if (!mtmp->mspec_used && rn2(3)) {
             if ((typ >= AD_MAGM) && (typ <= AD_ACID)) {
@@ -990,7 +994,8 @@ linedup_callback(
     if (!g.tbx && !g.tby)
         return FALSE;
 
-    if ((!g.tbx || !g.tby || abs(g.tbx) == abs(g.tby)) /* straight line or diagonal */
+    /* straight line, orthogonal to the map or diagonal */
+    if ((!g.tbx || !g.tby || abs(g.tbx) == abs(g.tby))
         && distmin(g.tbx, g.tby, 0, 0) < BOLT_LIM) {
         dx = sgn(ax - bx), dy = sgn(ay - by);
         do {
@@ -1026,7 +1031,8 @@ linedup(
     if (!g.tbx && !g.tby)
         return FALSE;
 
-    if ((!g.tbx || !g.tby || abs(g.tbx) == abs(g.tby)) /* straight line or diagonal */
+    /* straight line, orthogonal to the map or diagonal */
+    if ((!g.tbx || !g.tby || abs(g.tbx) == abs(g.tby))
         && distmin(g.tbx, g.tby, 0, 0) < BOLT_LIM) {
         if ((ax == u.ux && ay == u.uy) ? (boolean) couldsee(bx, by)
                                        : clear_path(ax, ay, bx, by))
@@ -1094,45 +1100,52 @@ m_carrying(struct monst* mtmp, int type)
 
 void
 hit_bars(
-    struct obj **objp,      /* *objp will be set to NULL if object breaks */
-    int objx, int objy, int barsx, int barsy,
-    boolean your_fault, boolean from_invent)
+    struct obj **objp,    /* *objp will be set to NULL if object breaks */
+    int objx, int objy,   /* hero's spot (when wielded) or missile's spot */
+    int barsx, int barsy, /* adjacent spot where bars are located */
+    unsigned breakflags)  /* breakage control */
 {
     struct obj *otmp = *objp;
     int obj_type = otmp->otyp;
-    boolean unbreakable = (levl[barsx][barsy].wall_info & W_NONDIGGABLE) != 0;
+    boolean nodissolve = (levl[barsx][barsy].wall_info & W_NONDIGGABLE) != 0,
+            your_fault = (breakflags & BRK_BY_HERO) != 0;
 
     if (your_fault
-        ? hero_breaks(otmp, objx, objy, from_invent)
+        ? hero_breaks(otmp, objx, objy, breakflags)
         : breaks(otmp, objx, objy)) {
         *objp = 0; /* object is now gone */
         /* breakage makes its own noises */
         if (obj_type == POT_ACID) {
-            if (cansee(barsx, barsy) && !unbreakable)
+            if (cansee(barsx, barsy) && !nodissolve)
                 pline_The("iron bars are dissolved!");
             else
-                You_hear(Hallucination ? "angry snakes!" : "a hissing noise.");
-            if (!unbreakable)
+                You_hear(Hallucination ? "angry snakes!"
+                                       : "a hissing noise.");
+            if (!nodissolve)
                 dissolve_bars(barsx, barsy);
         }
+    } else {
+        if (!Deaf)
+            pline("%s!", (obj_type == BOULDER || obj_type == HEAVY_IRON_BALL)
+                         ? "Whang"
+                         : (otmp->oclass == COIN_CLASS
+                            || objects[obj_type].oc_material == GOLD
+                            || objects[obj_type].oc_material == SILVER)
+                           ? "Clink"
+                           : "Clonk");
     }
-    else if (obj_type == BOULDER || obj_type == HEAVY_IRON_BALL)
-        pline("Whang!");
-    else if (otmp->oclass == COIN_CLASS
-             || objects[obj_type].oc_material == GOLD
-             || objects[obj_type].oc_material == SILVER)
-        pline("Clink!");
-    else
-        pline("Clonk!");
 }
 
 /* TRUE iff thrown/kicked/rolled object doesn't pass through iron bars */
 boolean
 hits_bars(
-    struct obj **obj_p, /* *obj_p will be set to NULL if object breaks */
-    int x, int y, int barsx, int barsy,
-    int always_hit, /* caller can force a hit for items which would fit through */
-    int whodidit)   /* 1==hero, 0=other, -1==just check whether it'll pass thru */
+    struct obj **obj_p,   /* *obj_p will be set to NULL if object breaks */
+    int x, int y,
+    int barsx, int barsy,
+    int always_hit,       /* caller can force a hit for items which would
+                           * fit through */
+    int whodidit)         /* 1==hero, 0=other, -1==just check whether it
+                           * will pass through */
 {
     struct obj *otmp = *obj_p;
     int obj_type = otmp->otyp;
@@ -1180,7 +1193,8 @@ hits_bars(
         }
 
     if (hits && whodidit != -1) {
-        hit_bars(obj_p, x,y, barsx,barsy, whodidit, FALSE);
+        hit_bars(obj_p, x, y, barsx, barsy,
+                 (whodidit == 1) ? BRK_BY_HERO : 0);
     }
 
     return hits;

diff --git a/src/zap.c b/src/zap.c
index 31ebc3b6cceb8778e75971cef3db8cc3a27fe48d..0ae62798d34c91949864293f3b9c4108065eabbd 100644 (file)
--- a/src/zap.c
+++ b/src/zap.c
@@ -2108,7 +2108,7 @@ bhito(struct obj *obj, struct obj *otmp)
                 int ooy = obj->oy;
                 if (g.context.mon_moving
                         ? !breaks(obj, obj->ox, obj->oy)
-                        : !hero_breaks(obj, obj->ox, obj->oy, FALSE))
+                        : !hero_breaks(obj, obj->ox, obj->oy, 0))
                     maybelearnit = FALSE; /* nothing broke */
                 else
                     newsym_force(oox,ooy);