From 47076c36255ac8b152f76803ea6391f10d2e1191 Mon Sep 17 00:00:00 2001 From: Michael Meyer Date: Tue, 18 Oct 2022 22:55:43 -0400 Subject: [PATCH] Fix: duplicate invlet from throwing obj with count Specifying a count of 1 when throwing an object could leave you with two stacks sharing one inventory letter. The second stack gets split off when the player specifies a count (e.g. 't1o'), but keeps its original invlet. Some early returns, like trying to throw at yourself with '.', could fail to unsplit the stack. Theoretically, specifying multiple items to multishot and then failing to throw them all could also leave a partial stack; I don't think this is actually possible right now with 't' but I tried to make sure it won't become a problem if greater counts than 1 are ever allowed. The fix doesn't affect 'f', which can be a combined "create a quiver stack and throw" action and doesn't have the issue with duping invlets. Specifying a count to split off a new quiver stack with 'f' shouldn't be reverted if the throwing fails or only part of the stack is thrown, because the newly created stack may be intended for continued use as the quiver in future turns. This slightly changes the behavior of the existing unsplit when cancelling the throw (which previously unsplit the newly created quiver and quivered the entire parent stack), but I think this actually makes more sense -- the player only declined to throw the new stack, not to create it (as if they canceled earlier in the action). I routed a couple early returns through the stack unsplitting that shouldn't actually need it (like Mjollnir and welded items) for consistency's sake; I don't think it hurts anything. --- src/dothrow.c | 56 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/src/dothrow.c b/src/dothrow.c index f086697f1..a83ef827e 100644 --- a/src/dothrow.c +++ b/src/dothrow.c @@ -85,20 +85,14 @@ throw_obj(struct obj *obj, int shotlimit) schar skill; long wep_mask; boolean twoweap, weakmultishot; + int res = ECMD_TIME; + struct obj_split save_osplit = g.context.objsplit; /* ask "in what direction?" */ if (!getdir((char *) 0)) { - /* No direction specified, so cancel the throw; - * might need to undo an object split. - * We used to use freeinv(obj),addinv(obj) here, but that can - * merge obj into another stack--usually quiver--even if it hadn't - * been split from there (possibly triggering a panic in addinv), - * and freeinv+addinv potentially has other side-effects. - */ - if (obj->o_id == g.context.objsplit.parent_oid - || obj->o_id == g.context.objsplit.child_oid) - (void) unsplitobj(obj); - return ECMD_CANCEL; /* no time passes */ + /* No direction specified, so cancel the throw */ + res = ECMD_CANCEL; /* no time passes */ + goto unsplit_stack; } /* @@ -109,24 +103,31 @@ throw_obj(struct obj *obj, int shotlimit) * If the gold is in quiver, throw one coin at a time, * possibly using a sling. */ - if (obj->oclass == COIN_CLASS && obj != uquiver) + if (obj->oclass == COIN_CLASS && obj != uquiver) { + /* throw_gold will unsplit the stack itself if necessary and may have + freed the object, so don't route through unsplit_stack here */ return throw_gold(obj); /* check */ + } if (!canletgo(obj, "throw")) { - return ECMD_OK; + res = ECMD_OK; + goto unsplit_stack; } if (is_art(obj, ART_MJOLLNIR) && obj != uwep) { pline("%s must be wielded before it can be thrown.", The(xname(obj))); - return ECMD_OK; + res = ECMD_OK; + goto unsplit_stack; } if ((is_art(obj, ART_MJOLLNIR) && ACURR(A_STR) < STR19(25)) || (obj->otyp == BOULDER && !throws_rocks(g.youmonst.data))) { pline("It's too heavy."); - return ECMD_TIME; + res = ECMD_TIME; + goto unsplit_stack; } if (!u.dx && !u.dy && !u.dz) { You("cannot throw an object at yourself."); - return ECMD_OK; + res = ECMD_OK; + goto unsplit_stack; } u_wipe_engr(2); if (!uarmg && obj->otyp == CORPSE && touch_petrifies(&mons[obj->corpsenm]) @@ -141,7 +142,8 @@ throw_obj(struct obj *obj, int shotlimit) } if (welded(obj)) { weldmsg(obj); - return ECMD_TIME; + res = ECMD_TIME; + goto unsplit_stack; } if (is_wet_towel(obj)) dry_a_towel(obj, -1, FALSE); @@ -242,6 +244,9 @@ throw_obj(struct obj *obj, int shotlimit) if (otmp->owornmask) remove_worn_item(otmp, FALSE); oldslot = obj->nobj; + /* obj will leave inventory and may be freed by throwit, don't + try to unsplit it from potential parent stack below */ + obj = (struct obj *) 0; } freeinv(otmp); throwit(otmp, wep_mask, twoweap, oldslot); @@ -251,7 +256,22 @@ throw_obj(struct obj *obj, int shotlimit) g.m_shot.o = STRANGE_OBJECT; g.m_shot.s = FALSE; - return ECMD_TIME; + unsplit_stack: + /* might need to undo an object split. + * We used to use freeinv(obj),addinv(obj) here, but that can + * merge obj into another stack--usually quiver--even if it hadn't + * been split from there (possibly triggering a panic in addinv), + * and freeinv+addinv potentially has other side-effects. + */ + if (obj && obj != uquiver + && (obj->o_id == save_osplit.parent_oid + || obj->o_id == save_osplit.child_oid)) { + /* futureproofing: objsplit will have been affected if partial stack + was thrown; objects will have been split off stack to throw. */ + g.context.objsplit = save_osplit; + (void) unsplitobj(obj); + } + return res; } /* common to dothrow() and dofire() */