fix broken migrating monster arrival

If the first monster on the migrating_mons list couldn't arrive and
was put back on the list to try again later, 'later' would happen
immediately and the program looped forever trying and failing to
bring that monster to the level.

Defer repeat attempts at migration until losedogs() has been through
the whole migrating_mons list.  mon_arrive() now populates a new
list called failed_arrivals and losedog() moves its contents, if any,
to migrating_mons prior to returning.

diff --git a/doc/fixes3-7-0.txt b/doc/fixes3-7-0.txt
index a19f030a2741ae31780b1013a89daad6148db05c..ebbaf82ba78780cf55b25c156292d8c3c54be59f 100644 (file)
--- a/doc/fixes3-7-0.txt
+++ b/doc/fixes3-7-0.txt
@@ -979,6 +979,8 @@ scared hostile monster which cannot move away will attack
 prevent a fog cloud that has engulfed the hero from moving under closed doors
 allow cutting a known spider web with wielded weapon by force-fighting the web
 holes and trapdoors have a fixed exit level
+recent changes to losedogs() could result in an infinite loop when migrating
+       monsters try to arrive as hero moves to a different level
 
 
 Fixes to 3.7.0-x Problems that Were Exposed Via git Repository

diff --git a/src/dog.c b/src/dog.c
index ecf40a3c5d8ec54dd16a1b8f2146071dbdf5a31d..9e9854f2cfc2a2a70a3c3689b47adbf837e21f7e 100644 (file)
--- a/src/dog.c
+++ b/src/dog.c
@@ -11,10 +11,11 @@ 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 */
+    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 */
+    Wiz_arrive = -1  /* resurrect(wizard.c) */
 };
 
 void
@@ -224,12 +225,16 @@ update_mlstmv(void)
     iter_mons(set_mon_lastmove);
 }
 
+/* note: always reset when used so doesn't need to be part of struct 'g' */
+static struct monst *failed_arrivals = 0;
+
 void
 losedogs(void)
 {
     register struct monst *mtmp, **mprev;
     int dismissKops = 0, xyloc;
 
+    failed_arrivals = 0;
     /*
      * First, scan g.migrating_mons for shopkeepers who want to dismiss Kops,
      * and scan g.mydogs for shopkeepers who want to retain kops.
@@ -279,15 +284,17 @@ losedogs(void)
         make_happy_shoppers(TRUE);
 
     /* put monsters who went onto migrating_mons in order to be accessible
-       when other levels are active back to the positions on this level;
+       when other levels are active back to their 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] */
+       hero can't steal the spot that belongs to them; these migraters
+       should always be able to arrive because they were present on the
+       level at the time the hero left [if they can't arrive for some
+       reason, mon_arrive() will put them on the 'failed_arrivals' list] */
     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) {
+            /* remove mtmp from migrating_mons */
             *mprev = mtmp->nmon;
             mon_arrive(mtmp, Before_you);
         } else {
@@ -303,19 +310,36 @@ losedogs(void)
         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) but that doesn't interfere with traversal */
+    /* time for migrating monsters to arrive; monsters who belong on
+       this level but fail to arrive get put on the failed_arrivals list
+       temporarily [by mon_arrive()], then back onto the migrating_mons
+       list below */
     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) {
+            /* remove mtmp from migrating_mons */
             *mprev = mtmp->nmon;
+            /* note: if there's no room, it ends up on failed_arrivals list */
             mon_arrive(mtmp, After_you);
         } else {
             mprev = &mtmp->nmon;
         }
     }
+
+    /* put any monsters who couldn't arrive back on migrating_mons,
+       clearing out the temporary 'failed_arrivals' list in the process */
+    while ((mtmp = failed_arrivals) != 0) {
+        failed_arrivals = mtmp->nmon;
+        /* mon_arrive() put mtmp onto fmon, but if there wasn't room to
+           arrive, relmon() was used to take it off again; put it back now
+           because m_into_limbo() expects it to be there */
+        mtmp->nmon = fmon;
+        fmon = mtmp;
+        /* set this monster to migrate back this level if hero leaves
+           and then returns */
+        m_into_limbo(mtmp);
+    }
 }
 
 /* called from resurrect() in addition to losedogs() */
@@ -497,8 +521,13 @@ mon_arrive(struct monst *mtmp, int when)
     else
         failed_to_place = !rloc(mtmp, RLOC_NOMSG);
 
-    if (failed_to_place)
-        m_into_limbo(mtmp); /* try again next time hero comes to this level */
+    if (failed_to_place) {
+        if (when != Wiz_arrive)
+            /* losedogs() will deal with this */
+            relmon(mtmp, &failed_arrivals);
+        else
+            m_into_limbo(mtmp);
+    }
 }
 
 /* heal monster for time spent elsewhere */

diff --git a/src/makemon.c b/src/makemon.c
index 4cde2f46c6db80d88b56d8ddfe8f232d67b9c62c..d9a4d667620586b8a86aa824f80a2a7f0b87fa64 100644 (file)
--- a/src/makemon.c
+++ b/src/makemon.c
@@ -1511,9 +1511,10 @@ mbirth_limit(int mndx)
 /* used for wand/scroll/spell of create monster */
 /* returns TRUE iff you know monsters have been created */
 boolean
-create_critters(int cnt,
-                struct permonst *mptr, /* usually null; used for confused reading */
-                boolean neverask)
+create_critters(
+    int cnt,
+    struct permonst *mptr, /* usually null; used for confused reading */
+    boolean neverask)
 {
     coord c;
     coordxy x, y;

diff --git a/src/vault.c b/src/vault.c
index 063130025c9485d62eb182cb00e83942654a34c8..c4ef28df1579a8e50252c208d049e2b055ef560c 100644 (file)
--- a/src/vault.c
+++ b/src/vault.c
@@ -215,10 +215,12 @@ findgd(void)
     for (mprev = &g.migrating_mons; (mtmp = *mprev) != 0;
          mprev = &mtmp->nmon) {
         if (mtmp->isgd && on_level(&EGD(mtmp)->gdlevel, &u.uz)) {
-            /* take out of migrating_mons and place at <0,0> */
+            /* take out of migrating_mons and place at <0,0>;
+               simplified mon_arrive(); avoid that because it would
+               send mtmp into limbo if no regular map spot is available */
             *mprev = mtmp->nmon;
-            /* simplified mon_arrive(); avoid that because it would send
-               mtmp into limbo if no regular map spot is available */
+            mtmp->nmon = fmon;
+            fmon = mtmp;
             mon_track_clear(mtmp);
             mtmp->mux = u.ux, mtmp->muy = u.uy;
             mtmp->mx = mtmp->my = 0; /* not on map (note: mx is already 0) */

diff --git a/src/wizard.c b/src/wizard.c
index cbf179c866e4c803d75be16c7b7874649245a69e..50137c763acb6cdba5547f65614603464bfde343 100644 (file)
--- a/src/wizard.c
+++ b/src/wizard.c
@@ -708,7 +708,7 @@ resurrect(void)
                     mtmp->mfrozen = 0, mtmp->mcanmove = 1;
                 if (!helpless(mtmp)) {
                     *mmtmp = mtmp->nmon;
-                    mon_arrive(mtmp, 0); /* 0: not With_you (1) */
+                    mon_arrive(mtmp, -1); /* -1: Wiz_arrive (dog.c) */
                     /* note: there might be a second Wizard; if so,
                        he'll have to wait til the next resurrection */
                     break;