]> granicus.if.org Git - nethack/commitdiff
fix #K3633 - vault guard parked at <0,0> brought \
authorPatR <rankin@nethack.org>
Wed, 13 Jul 2022 22:19:51 +0000 (15:19 -0700)
committerPatR <rankin@nethack.org>
Wed, 13 Jul 2022 22:19:51 +0000 (15:19 -0700)
back into play with bad data

I don't have a test case to verify the fix, and I'm not absolutely
certain that the cause has been correctly diagnosed, but I think the
problem was caused by a guard being sent into limbo because the map
was too full to place it, then while it was on the migrating monsters
list waiting for a chance to come back the fuzzer executed #wizmakemap.
If the hero left the level and subsequently returned, the guard would
arrive back but monst->mextra->egd contained data for the previous
incarnation of the level that's invalid for wizmakemap's replacement.

Treat any shopkeeper, temple priest, or vault guard who is not on his
'home' level like the Wizard has been treated since 3.6.0.  When
leaving the level they're on, put them on the migrating monsters list
scheduled to return to present position instead of stashing them in
the level's data file.  That way they can be accessed from any dungeon
level, so wizmakemap can pull ones for the level it's replacing off
the migrating monsters list when removing the old level's monsters,
handling both migration-pending and already-arrived-on-another-level.

Bonus fix:  put monsters who are on the migrating_mons list solely in
order to be accessible from other levels back first when returning to
the level they're on so that pets and the hero can't hijack their spot
when those arrive.  The Wizard has been vulnerable to that.

Not fixed:  #wizfliplevel command needs to flip parts of shk->mextra->
eshk and priest->mextra->epri for shk or priest on migrating_mons.
Vault guards don't contain anything flippable when migrating, but do
have coordinates that need fixing up while they're maintaining a
temporary corridor to/from the vault.

doc/fixes3-7-0.txt
include/extern.h
src/cmd.c
src/dog.c
src/wizard.c

index 64dd2605b5813705556d48c0a59b04393f1999ef..a4a6afd828b18c1ab3afb42b89fb13661af05d5c 100644 (file)
@@ -953,6 +953,10 @@ praying on an altar with pet corpse on it can revive the pet
 applying a cursed oil lamp can make your hands slippery
 valkyries start with a spear instead of a long sword
 grid bugs don't have hands
+if #wizmakemap was used to generate a replacement level while any shopkeeper,
+       temple priest, or vault guard from the level was off of it at the
+       time, the monster's eshk, epri, or egd data became invalid and would
+       cause trouble if the monster returned to its 'home' level
 
 
 Fixes to 3.7.0-x Problems that Were Exposed Via git Repository
index 5efb5aa0bbc04b5311306a8936d66de23546706a..dcc64aa7197a3a7d148f5062d87f309c82a64af3 100644 (file)
@@ -592,7 +592,7 @@ extern struct monst *make_familiar(struct obj *, coordxy, coordxy, boolean);
 extern struct monst *makedog(void);
 extern void update_mlstmv(void);
 extern void losedogs(void);
-extern void mon_arrive(struct monst *, boolean);
+extern void mon_arrive(struct monst *, int);
 extern void mon_catchup_elapsed_time(struct monst *, long);
 extern void keepdogs(boolean);
 extern void migrate_to_level(struct monst *, coordxy, coordxy, coord *);
index 5d463413adcb9c0d47682166ed3467fa2bfb51e9..d937da0741c48f9fc213935a97ecce3abbaf1f2d 100644 (file)
--- a/src/cmd.c
+++ b/src/cmd.c
@@ -130,6 +130,8 @@ static int wiz_rumor_check(void);
 static int wiz_migrate_mons(void);
 #endif
 
+static void makemap_unmakemon(struct monst *);
+static void makemap_remove_mons(void);
 static void wiz_map_levltyp(void);
 static void wiz_levltyp_legend(void);
 #if defined(__BORLANDC__) && !defined(_WIN32)
@@ -987,6 +989,71 @@ wiz_identify(void)
     return ECMD_OK;
 }
 
+/* used when wiz_makemap() gets rid of monsters for the old incarnation of
+   a level before creating a new incarnation of it */
+static void
+makemap_unmakemon(struct monst *mtmp)
+{
+    int ndx = monsndx(mtmp->data);
+
+    /* uncreate any unique monster so that it is eligible to be remade
+       on the new incarnation of the level; ignores DEADMONSTER() [why?] */
+    if (mtmp->data->geno & G_UNIQ)
+        g.mvitals[ndx].mvflags &= ~G_EXTINCT;
+    if (g.mvitals[ndx].born)
+        g.mvitals[ndx].born--;
+
+    /* vault is going away; get rid of guard who might be in play or
+       be parked at <0,0>; for the latter, might already be flagged as
+       dead but is being kept around because of the 'isgd' flag */
+    if (mtmp->isgd) {
+        mtmp->isgd = 0; /* after this, fall through to mongone() */
+    } else if (DEADMONSTER(mtmp)) {
+        return; /* already set to be discarded */
+    } else if (mtmp->isshk && on_level(&u.uz, &ESHK(mtmp)->shoplevel)) {
+        setpaid(mtmp);
+    }
+    mongone(mtmp);
+}
+
+/* get rid of the all the monsters on--or intimately involved with--current
+   level; used when #wizmakemap destroys the level before replacing it */
+static void
+makemap_remove_mons(void)
+{
+    struct monst *mtmp, **mprev;
+
+    /* keep steed and other adjacent pets after releasing them
+       from traps, stopping eating, &c as if hero were ascending */
+    keepdogs(TRUE); /* (pets-only; normally we'd be using 'FALSE') */
+    /* get rid of all the monsters that didn't make it to 'mydogs' */
+    for (mtmp = fmon; mtmp; mtmp = mtmp->nmon)
+        makemap_unmakemon(mtmp);
+    /* some monsters retain details of this level in mon->mextra; that
+       data becomes invalid when the level is replaced by a new one;
+       get rid of them now if migrating or already arrived elsewhere;
+       [when on their 'home' level, the previous loop got rid of them;
+       if they aren't actually migrating but have been placed on some
+       'away' level, such monsters are treated like the Wizard:  kept
+       on migrating monsters list, scheduled to migrate back to their
+       present location instead of being saved with whatever level they
+       happen to be on; see keepdogs() and keep_mon_accessible(dog.c)] */
+    for (mprev = &g.migrating_mons; (mtmp = *mprev) != 0; ) {
+        if (mtmp->mextra
+            && ((mtmp->isshk && on_level(&u.uz, &ESHK(mtmp)->shoplevel))
+                || (mtmp->ispriest && on_level(&u.uz, &EPRI(mtmp)->shrlevel))
+                || (mtmp->isgd && on_level(&u.uz, &EGD(mtmp)->gdlevel)))) {
+            *mprev = mtmp->nmon;
+            makemap_unmakemon(mtmp);
+        } else {
+            mprev = &mtmp->nmon;
+        }
+    }
+    /* release dead and 'unmade' monsters */
+    dmonsfree();
+    return;
+}
+
 void
 makemap_prepost(boolean pre, boolean wiztower)
 {
@@ -994,26 +1061,8 @@ makemap_prepost(boolean pre, boolean wiztower)
     struct monst *mtmp;
 
     if (pre) {
-        /* keep steed and other adjacent pets after releasing them
-           from traps, stopping eating, &c as if hero were ascending */
-        keepdogs(TRUE); /* (pets-only; normally we'd be using 'FALSE' here) */
-
-        rm_mapseen(ledger_no(&u.uz));
-        for (mtmp = fmon; mtmp; mtmp = mtmp->nmon) {
-            int ndx = monsndx(mtmp->data);
-            if (mtmp->isgd) { /* vault is going away; get rid of guard */
-                mtmp->isgd = 0;
-                mongone(mtmp);
-            }
-            if (mtmp->data->geno & G_UNIQ)
-                g.mvitals[ndx].mvflags &= ~(G_EXTINCT);
-            if (g.mvitals[ndx].born)
-                g.mvitals[ndx].born--;
-            if (DEADMONSTER(mtmp))
-                continue;
-            if (mtmp->isshk)
-                setpaid(mtmp);
-        }
+        makemap_remove_mons();
+        rm_mapseen(ledger_no(&u.uz)); /* discard overview info for level */
         {
             static const char Unachieve[] = "%s achievement revoked.";
 
index 0af7397e94224db28c9945afd084082a65c7fab3..7047c38c7c7014ba915141c440f2402d4f753330 100644 (file)
--- a/src/dog.c
+++ b/src/dog.c
@@ -8,6 +8,14 @@
 static int pet_type(void);
 static void set_mon_lastmove(struct monst *);
 static int mon_leave(struct monst *);
+static boolean keep_mon_accessible(struct monst *);
+
+enum arrival {
+    Before_you = 0, /* monsters kept on migrating_mons for accessibility;
+                     * they haven't actually left their level */
+    With_you   = 1, /* pets and level followers */
+    After_you  = 2  /* regular migrating monsters */
+};
 
 void
 newedog(struct monst *mtmp)
@@ -219,14 +227,16 @@ update_mlstmv(void)
 void
 losedogs(void)
 {
-    register struct monst *mtmp, *mtmp0, *mtmp2;
-    int dismissKops = 0;
+    register struct monst *mtmp, **mprev;
+    int dismissKops = 0, xyloc;
 
     /*
      * First, scan g.migrating_mons for shopkeepers who want to dismiss Kops,
      * and scan g.mydogs for shopkeepers who want to retain kops.
      * Second, dismiss kops if warranted, making more room for arrival.
-     * Third, place monsters accompanying the hero.
+     * Third, replace monsters who went onto migrating_mons in order to
+     * be accessible from other levels but didn't actually leave the level.
+     * Fourth, place monsters accompanying the hero.
      * Last, place migrating monsters coming to this level.
      *
      * Hero might eventually be displaced (due to the third step, but
@@ -268,38 +278,49 @@ losedogs(void)
     if (dismissKops > 0)
         make_happy_shoppers(TRUE);
 
-    /* place pets and/or any other monsters who accompany hero */
+    /* put monsters who went onto migrating_mons in order to be accessible
+       when other levels are active back to the positions on this level;
+       they're handled before mydogs so that monsters accompanying the
+       hero can't steal the spot that belongs to them
+       [note: mon_arrive() might fail and put mtmp back at the head of
+       migrating_mons; that doesn't interfere with our list traversal] */
+    for (mprev = &g.migrating_mons; (mtmp = *mprev) != 0; ) {
+        xyloc = mtmp->mtrack[0].x; /* (for legibility) */
+        if (mtmp->mux == u.uz.dnum && mtmp->muy == u.uz.dlevel
+            && xyloc == MIGR_EXACT_XY) {
+            *mprev = mtmp->nmon;
+            mon_arrive(mtmp, Before_you);
+        } else {
+            mprev = &mtmp->nmon;
+        }
+    }
+
+    /* place pets and/or any other monsters who accompany hero;
+       any that fail to arrive (level may be level) will be moved
+       to migrating_mons and immediately retry (and fail again) below */
     while ((mtmp = g.mydogs) != 0) {
         g.mydogs = mtmp->nmon;
-        mon_arrive(mtmp, TRUE);
+        mon_arrive(mtmp, With_you);
     }
 
     /* time for migrating monsters to arrive;
-       monsters who belong on this level but fail to arrive get put
-       back onto the list (at head), so traversing it is tricky */
-    for (mtmp = g.migrating_mons; mtmp; mtmp = mtmp2) {
-        mtmp2 = mtmp->nmon;
-        if (mtmp->mux == u.uz.dnum && mtmp->muy == u.uz.dlevel) {
-            /* remove mtmp from g.migrating_mons list */
-            if (mtmp == g.migrating_mons) {
-                g.migrating_mons = mtmp->nmon;
-            } else {
-                for (mtmp0 = g.migrating_mons; mtmp0; mtmp0 = mtmp0->nmon)
-                    if (mtmp0->nmon == mtmp) {
-                        mtmp0->nmon = mtmp->nmon;
-                        break;
-                    }
-                if (!mtmp0)
-                    panic("losedogs: can't find migrating mon");
-            }
-            mon_arrive(mtmp, FALSE);
+       monsters who belong on this level but fail to arrive get put back
+       onto the list (at head) but that doesn't interfere with traversal */
+    for (mprev = &g.migrating_mons; (mtmp = *mprev) != 0; ) {
+        xyloc = mtmp->mtrack[0].x;
+        if (mtmp->mux == u.uz.dnum && mtmp->muy == u.uz.dlevel
+            && xyloc != MIGR_EXACT_XY) {
+            *mprev = mtmp->nmon;
+            mon_arrive(mtmp, After_you);
+        } else {
+            mprev = &mtmp->nmon;
         }
     }
 }
 
 /* called from resurrect() in addition to losedogs() */
 void
-mon_arrive(struct monst *mtmp, boolean with_you)
+mon_arrive(struct monst *mtmp, int when)
 {
     struct trap *t;
     coordxy xlocale, ylocale, xyloc, xyflags;
@@ -340,7 +361,7 @@ mon_arrive(struct monst *mtmp, boolean with_you)
 
     if (mtmp == u.usteed)
         return; /* don't place steed on the map */
-    if (with_you) {
+    if (when == With_you) {
         /* When a monster accompanies you, sometimes it will arrive
            at your intended destination and you'll end up next to
            that spot.  This code doesn't control the final outcome;
@@ -619,6 +640,28 @@ mon_leave(struct monst *mtmp)
     return num_segs;
 }
 
+/* when hero leaves a level, some monsters should be placed on the
+   migrating_mons list instead of being stashed inside the level's file */
+static boolean
+keep_mon_accessible(struct monst *mon)
+{
+    /* the Wizard is kept accessible so that his harassment can fetch
+       him instead of creating a new instance but also so that he can
+       be put back at his current location if hero returns to his level */
+    if (mon->iswiz)
+        return TRUE;
+    /* monsters with special attachment to a particular level only need
+       to be kept accessible when on some other level */
+    if (mon->mextra
+        && ((mon->isshk && !on_level(&u.uz, &ESHK(mon)->shoplevel))
+            || (mon->ispriest && !on_level(&u.uz, &EPRI(mon)->shrlevel))
+            || (mon->isgd && !on_level(&u.uz, &EGD(mon)->gdlevel))))
+        return TRUE;
+    /* normal monsters go into the level save file instead of being held
+       on the migrating_mons list for off-level accessibility */
+    return FALSE;
+}
+
 /* called when you move to another level */
 void
 keepdogs(
@@ -699,10 +742,14 @@ keepdogs(
             mtmp->mx = mtmp->my = 0; /* mx==0 implies migating */
             mtmp->wormno = num_segs;
             mtmp->mlstmv = g.moves;
-        } else if (mtmp->iswiz) {
-            /* we want to be able to find him when his next resurrection
-               chance comes up, but have him resume his present location
-               if player returns to this level before that time */
+        } else if (keep_mon_accessible(mtmp)) {
+            /* we want to be able to find the Wizard when his next
+               resurrection chance comes up, but have him resume his
+               present location if player returns to this level before
+               that time; also needed for monsters (shopkeeper, temple
+               priest, vault guard) who have level data in mon->mextra
+               in case #wizmakemap is used to replace their home level
+               while they're away from it */
             migrate_to_level(mtmp, ledger_no(&u.uz), MIGR_EXACT_XY,
                              (coord *) 0);
         } else if (mtmp->mleashed) {
index 3f4acf884746bceec8f9917709d6ffc19e883222..cbf179c866e4c803d75be16c7b7874649245a69e 100644 (file)
@@ -708,7 +708,7 @@ resurrect(void)
                     mtmp->mfrozen = 0, mtmp->mcanmove = 1;
                 if (!helpless(mtmp)) {
                     *mmtmp = mtmp->nmon;
-                    mon_arrive(mtmp, TRUE);
+                    mon_arrive(mtmp, 0); /* 0: not With_you (1) */
                     /* note: there might be a second Wizard; if so,
                        he'll have to wait til the next resurrection */
                     break;