From 4bce58f66560941a321b266181fee20859c8f928 Mon Sep 17 00:00:00 2001 From: PatR Date: Sun, 17 Jun 2018 16:59:58 -0700 Subject: [PATCH] fix github issue #106 - polymorph panic Fixes #106 If dipping a worn amulet into a potion of polymorph turns it into an amulet of change, the game panics while trying to use up that amulet when the new one hasn't replaced the old one in inventory yet. Simply reordering the relevant code isn't sufficient to fix things: once it is in inventory and can be successfully used up, later code would end up deferencing a stale pointer because it was unaware of the deletion. --- doc/fixes36.2 | 1 + include/extern.h | 1 + src/potion.c | 22 +++++++++------------- src/worn.c | 16 +++++++++++++++- src/zap.c | 39 ++++++++++++++++++++++++++------------- 5 files changed, 52 insertions(+), 27 deletions(-) diff --git a/doc/fixes36.2 b/doc/fixes36.2 index ea039baaa..a24195e83 100644 --- a/doc/fixes36.2 +++ b/doc/fixes36.2 @@ -38,6 +38,7 @@ internals for 'sortloot' option have been changed to not reorder the actual full-pack identify won't result in possibly skipping some items give vault guards a cursed tin whistle since there is a shrill whistling sound if hero teleports out of vault while being confronted by guard +polymorphing worn amulet triggers panic if it turns into amulet of change Fixes to Post-3.6.1 Problems that Were Exposed Via git Repository diff --git a/include/extern.h b/include/extern.h index a7ed1fef6..aa990d10b 100644 --- a/include/extern.h +++ b/include/extern.h @@ -2830,6 +2830,7 @@ E int FDECL(wseg_at, (struct monst *, int, int)); E void FDECL(setworn, (struct obj *, long)); E void FDECL(setnotworn, (struct obj *)); +E struct obj *FDECL(wearmask_to_obj, (long)); E long FDECL(wearslot, (struct obj *)); E void FDECL(mon_set_minvis, (struct monst *)); E void FDECL(mon_adjust_speed, (struct monst *, int, struct obj *)); diff --git a/src/potion.c b/src/potion.c index 28a73aaf8..d90beb898 100644 --- a/src/potion.c +++ b/src/potion.c @@ -1920,26 +1920,22 @@ dodip() 5, 95)) { pline1(nothing_happens); } else { - boolean was_wep, was_swapwep, was_quiver; short save_otyp = obj->otyp; /* KMH, conduct */ u.uconduct.polypiles++; - was_wep = (obj == uwep); - was_swapwep = (obj == uswapwep); - was_quiver = (obj == uquiver); - obj = poly_obj(obj, STRANGE_OBJECT); - if (was_wep) - setuwep(obj); - else if (was_swapwep) - setuswapwep(obj); - else if (was_quiver) - setuqwep(obj); - - if (obj->otyp != save_otyp) { + /* + * obj might be gone: + * poly_obj() -> set_wear() -> Amulet_on() -> useup() + * if obj->otyp is worn amulet and becomes AMULET_OF_CHANGE. + */ + if (!obj) { + makeknown(POT_POLYMORPH); + return 1; + } else if (obj->otyp != save_otyp) { makeknown(POT_POLYMORPH); useup(potion); prinv((char *) 0, obj, 0L); diff --git a/src/worn.c b/src/worn.c index a2819c4e3..828e6a902 100644 --- a/src/worn.c +++ b/src/worn.c @@ -29,7 +29,8 @@ const struct worn { { W_TOOL, &ublindf }, { W_BALL, &uball }, { W_CHAIN, &uchain }, - { 0, 0 } }; + { 0, 0 } +}; /* This only allows for one blocking item per property */ #define w_blocks(o, m) \ @@ -142,6 +143,19 @@ register struct obj *obj; update_inventory(); } +/* return item worn in slot indiciated by wornmask; needed by poly_obj() */ +struct obj * +wearmask_to_obj(wornmask) +long wornmask; +{ + const struct worn *wp; + + for (wp = worn; wp->w_mask; wp++) + if (wp->w_mask & wornmask) + return *wp->w_obj; + return (struct obj *) 0; +} + /* return a bitmask of the equipment slot(s) a given item might be worn in */ long wearslot(obj) diff --git a/src/zap.c b/src/zap.c index aa902d70c..f157926b0 100644 --- a/src/zap.c +++ b/src/zap.c @@ -1392,7 +1392,8 @@ struct obj *obj; int id; { struct obj *otmp; - xchar ox, oy; + xchar ox = 0, oy = 0; + long old_wornmask, new_wornmask = 0L; boolean can_merge = (id == STRANGE_OBJECT); int obj_location = obj->where; @@ -1573,10 +1574,10 @@ int id; of polymorph can produce side-effects but those won't yield out of sequence messages because current polymorph is finished */ if (obj_location == OBJ_INVENT && obj->owornmask) { - long old_wornmask = obj->owornmask & ~(W_ART | W_ARTI), - new_wornmask = wearslot(otmp); boolean was_twohanded = bimanual(obj), was_twoweap = u.twoweap; + old_wornmask = obj->owornmask & ~(W_ART | W_ARTI); + new_wornmask = wearslot(otmp); remove_worn_item(obj, TRUE); /* if the new form can be worn in the same slot, make it so [possible extension: if it could be worn in some other @@ -1595,13 +1596,21 @@ int id; u.twoweap = TRUE; } else if ((old_wornmask & new_wornmask) != 0L) { new_wornmask &= old_wornmask; + /* + * Defer this until later; set_wear() might result in otmp + * being destroyed (using up an amulet of change, for instance). + * setworn(otmp, new_wornmask); - set_wear(otmp); /* Armor_on() for side-effects */ + set_wear(otmp); + */ } } - /* ** we are now done adjusting the object ** */ + /* + * ** we are now done adjusting the object ** + */ + (void) get_obj_location(obj, &ox, &oy, BURIED_TOO | CONTAINED_TOO); /* swap otmp for obj */ replace_object(obj, otmp); if (obj_location == OBJ_INVENT) { @@ -1614,8 +1623,14 @@ int id; freeinv_core(obj); addinv_core1(otmp); addinv_core2(otmp); + if (new_wornmask) { + setworn(otmp, new_wornmask); + /* set_wear() might result in otmp being destroyed if + worn amulet has been turned into an amulet of change */ + set_wear(otmp); + otmp = wearmask_to_obj(new_wornmask); /* might be Null */ + } } else if (obj_location == OBJ_FLOOR) { - ox = otmp->ox, oy = otmp->oy; /* set by replace_object() */ if (obj->otyp == BOULDER && otmp->otyp != BOULDER && !does_block(ox, oy, &levl[ox][oy])) unblock_point(ox, oy); @@ -1624,11 +1639,9 @@ int id; block_point(ox, oy); } - if ((!carried(otmp) || obj->unpaid) - && get_obj_location(otmp, &ox, &oy, BURIED_TOO | CONTAINED_TOO) - && costly_spot(ox, oy)) { - register struct monst *shkp = - shop_keeper(*in_rooms(ox, oy, SHOPBASE)); + /* note: if otmp is gone, billing for it was handled by useup() */ + if (((otmp && !carried(otmp)) || obj->unpaid) && costly_spot(ox, oy)) { + struct monst *shkp = shop_keeper(*in_rooms(ox, oy, SHOPBASE)); if ((!obj->no_charge || (Has_contents(obj) @@ -1636,8 +1649,8 @@ int id; && inhishop(shkp)) { if (shkp->mpeaceful) { if (*u.ushops - && *in_rooms(u.ux, u.uy, 0) - == *in_rooms(shkp->mx, shkp->my, 0) + && (*in_rooms(u.ux, u.uy, 0) + == *in_rooms(shkp->mx, shkp->my, 0)) && !costly_spot(u.ux, u.uy)) { make_angry_shk(shkp, ox, oy); } else {