fix confused remove curse bug

Reported directly to devteam, player observed that objects on the
floor had their bless/curse state change when reading a blessed
scroll of remove curse while confused.  Message feedback mentioned
a silver saber being dropped.  I didn't attempt to view the ttyrec
playbacks; what I'm sure happened was that the saber was secondary
weapon for dual wielding and had been uncursed; the confused remove
curse effect cursed it, which in turn caused it to be dropped.  The
saber's 'next object' pointer became the [previous] top of the pile
at that spot and further object traversal intended to process the
rest of hero's inventory ended up processing floor objects there
instead.

This bug has been present for over 20 years (since 3.3.0 came out
in late 1999, when dual wielding was introduced and cursing of the
secondary weapon forced it to be dropped since making it become
welded was deemed to be too complicated) and never been reported.
Most likely players keep secondary weapons blessed so the scroll
effect doesn't touch them and simple object traversal sticks with
inventory.  Or items at the spot have unknown BUC state so having
them be affected wouldn't be particularly noticeable.

diff --git a/doc/fixes37.0 b/doc/fixes37.0
index 8b766ebb282cb584473bf740b9dac2dfcbbaa24f..8c3b51b2eb53b8b92576ae1892d716b4c864b2bd 100644 (file)
--- a/doc/fixes37.0
+++ b/doc/fixes37.0
@@ -634,6 +634,11 @@ if a Rider or displacer beast swapped places with a single-segment long worm
        the segment co-located with the head wasn't moved with that head;
        if sanity_checking was enabled a warning could be triggered:
        mon (000000) at seg location is not worm (123abc)
+blessed scroll of remove curse read while confused blesses or curses any
+       uncursed items in inventory, but if hero was dual-wielding and the
+       scroll cursed the secondary weapon, that would be dropped and further
+       object traversal would process items on the floor at hero's spot
+       instead of the rest of inventory
 
 
 Fixes to 3.7.0-x Problems that Were Exposed Via git Repository

diff --git a/src/read.c b/src/read.c
index 2d8c4ecba00814e3aeee19b6f8c239ad0ee196b2..2f69fa985c12555d291e071ddd9bf49cfa6618b2 100644 (file)
--- a/src/read.c
+++ b/src/read.c
@@ -1310,12 +1310,13 @@ seffect_scare_monster(struct obj **sobjp)
 static void
 seffect_remove_curse(struct obj **sobjp)
 {
-    struct obj *sobj = *sobjp;
+    struct obj *sobj = *sobjp; /* scroll or fake spellbook */
     int otyp = sobj->otyp;
     boolean sblessed = sobj->blessed;
     boolean scursed = sobj->cursed;
     boolean confused = (Confusion != 0);
-    register struct obj *obj;
+    register struct obj *obj, *nxto;
+    long wornmask;
 
     You_feel(!Hallucination
              ? (!confused ? "like someone is helping you."
@@ -1326,9 +1327,14 @@ seffect_remove_curse(struct obj **sobjp)
     if (scursed) {
         pline_The("scroll disintegrates.");
     } else {
-        for (obj = g.invent; obj; obj = obj->nobj) {
-            long wornmask;
-
+        /* 3.7: this used to use a straight
+               for (obj = invent; obj; obj = obj->nobj) {}
+           traversal, but for the confused case, secondary weapon might
+           become cursed and be dropped, moving it from the invent chain
+           to the floor chain at hero's spot, so we have to remember the
+           next object prior to processing the current one */
+        for (obj = g.invent; obj; obj = nxto) {
+            nxto = obj->nobj;
             /* gold isn't subject to cursing and blessing */
             if (obj->oclass == COIN_CLASS)
                 continue;