From 4bce58f66560941a321b266181fee20859c8f928 Mon Sep 17 00:00:00 2001 From: PatR Date: Sun, 17 Jun 2018 16:59:58 -0700 Subject: [PATCH 1/2] 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 { From 1822f54c41641737e1a196b73fcb1510a0f692bc Mon Sep 17 00:00:00 2001 From: PatR Date: Sun, 17 Jun 2018 19:09:36 -0700 Subject: [PATCH 2/2] more polymorph of worn item The earlier change for 'fix github issue #106' could result in a polymorphed weapon being worn in multiple weapon/alt-weapon/quiver slots. Reorganize the relevant code more thoroughly this time. --- src/zap.c | 83 +++++++++++++++++++++++++------------------------------ 1 file changed, 37 insertions(+), 46 deletions(-) diff --git a/src/zap.c b/src/zap.c index f157926b0..b2ff4c77a 100644 --- a/src/zap.c +++ b/src/zap.c @@ -1568,49 +1568,12 @@ int id; /* update the weight */ otmp->owt = weight(otmp); - /* handle polymorph of worn item: stone-to-flesh cast on self can - affect multiple objects at once, but their new forms won't - produce any side-effects; a single worn item dipped into potion - 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) { - 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 - slot which is currently unfilled, wear it there instead] */ - if ((old_wornmask & W_QUIVER) != 0L) { - setuqwep(otmp); - } else if ((old_wornmask & W_SWAPWEP) != 0L) { - if (was_twohanded || !bimanual(otmp)) - setuswapwep(otmp); - if (was_twoweap && uswapwep) - u.twoweap = TRUE; - } else if ((old_wornmask & W_WEP) != 0L) { - if (was_twohanded || !bimanual(otmp) || !uarms) - setuwep(otmp); - if (was_twoweap && uwep && !bimanual(uwep)) - 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); - */ - } - } - /* - * ** we are now done adjusting the object ** + * ** we are now done adjusting the object (except possibly wearing it) ** */ (void) get_obj_location(obj, &ox, &oy, BURIED_TOO | CONTAINED_TOO); + old_wornmask = obj->owornmask & ~(W_ART | W_ARTI); /* swap otmp for obj */ replace_object(obj, otmp); if (obj_location == OBJ_INVENT) { @@ -1623,13 +1586,41 @@ 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 */ - } + /* + * Handle polymorph of worn item. Stone-to-flesh cast on self can + * affect multiple objects at once, but their new forms won't + * produce any side-effects. A single worn item dipped into potion + * of polymorph can produce side-effects but those won't yield out + * of sequence messages because current polymorph is finished. + */ + if (old_wornmask) { + boolean was_twohanded = bimanual(obj), was_twoweap = u.twoweap; + + /* wearslot() returns a mask which might have multiple bits set; + narrow that down to the bit(s) currently in use */ + new_wornmask = wearslot(otmp) & old_wornmask; + remove_worn_item(obj, TRUE); + /* if the new form can be worn in the same slot, make it so */ + if ((new_wornmask & W_WEP) != 0L) { + if (was_twohanded || !bimanual(otmp) || !uarms) + setuwep(otmp); + if (was_twoweap && uwep && !bimanual(uwep)) + u.twoweap = TRUE; + } else if ((new_wornmask & W_SWAPWEP) != 0L) { + if (was_twohanded || !bimanual(otmp)) + setuswapwep(otmp); + if (was_twoweap && uswapwep) + u.twoweap = TRUE; + } else if ((new_wornmask & W_QUIVER) != 0L) { + setuqwep(otmp); + } else 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 */ + } + } /* old_wornmask */ } else if (obj_location == OBJ_FLOOR) { if (obj->otyp == BOULDER && otmp->otyp != BOULDER && !does_block(ox, oy, &levl[ox][oy]))