adopt github pull request #286 - rndmonst()

Eliminate the cache that was supporting rndmonst() and pick a random
monster in a single pass through mons[] via "weighted reservoir
sampling", a term I'm not familiar with.

It had a couple of bugs:  if the first monster examined happened to
be given a weighting of 0, rn2() would divide by 0.  I didn't try
to figure out how to trigger that.  But the second one was easy to
trigger:  if all eligible monsters were extinct or genocided, it
would issue a warning even though the situation isn't impossible.

Aside from fixing those, the rest is mostly as-is.  I included a bit
of formatting in decl.c, moved some declarations to not require C99,
and changed a couple of macros to not hide and duplicate a call to
level_difficulty().

Fixes #286

doc/fixes37.0

@@ -199,6 +199,7 @@ correct the Guidebook descriptions for msdos video_width and video_height to
 	state that they work with video:vesa; the video:vga setting that was 
 	described there forces the 640x480x16 mode where video_width and
 	video_height don't operate (github #294)
+redo rndmonst() to operate in a single pass (github pull request #286)
 
 
 Code Cleanup and Reorganization

include/decl.h

@@ -886,18 +886,19 @@ struct instance_globals {
     boolean chosen_symset_end;
     int symset_which_set;
     /* SAVESIZE, BONESSIZE, LOCKNAMESIZE are defined in "fnamesiz.h" */
-    char SAVEF[SAVESIZE]; /* holds relative path of save file from playground */
+    char SAVEF[SAVESIZE]; /* relative path of save file from playground */
 #ifdef MICRO
     char SAVEP[SAVESIZE]; /* holds path of directory for save file */
 #endif
     char bones[BONESSIZE];
     char lock[LOCKNAMESIZE];
 
-
     /* hack.c */
     anything tmp_anything;
     int wc; /* current weight_cap(); valid after call to inv_weight() */
 
+    /* insight.c */
+
     /* invent.c */
     int lastinvnr;  /* 0 ... 51 (never saved&restored) */
     unsigned sortlootmode; /* set by sortloot() for use by sortloot_cmp(); 
@@ -905,7 +906,7 @@ struct instance_globals {
     char *invbuf;
     unsigned invbufsiz;
     /* for perm_invent when operating on a partial inventory display, so that
-       the persistent one doesn't get shrunk during filtering for item selection
+       persistent one doesn't get shrunk during filtering for item selection
        then regrown to full inventory, possibly being resized in the process */
     winid cached_pickinv_win;
     /* query objlist callback: return TRUE if obj type matches "this_type" */
@@ -916,15 +917,10 @@ struct instance_globals {
     /* light.c */
     light_source *light_base;
 
-
     /* lock.c */
     struct xlock_s xlock;
 
     /* makemon.c */
-    struct {
-        int choice_count;
-        char mchoices[SPECIAL_PM]; /* value range is 0..127 */
-    } rndmonst_state;
 
     /* mhitm.c */
     boolean vis;

include/extern.h

@@ -1216,7 +1216,6 @@ E void FDECL(dealloc_mextra, (struct monst *));
 E struct monst *FDECL(makemon, (struct permonst *, int, int, int));
 E boolean FDECL(create_critters, (int, struct permonst *, BOOLEAN_P));
 E struct permonst *NDECL(rndmonst);
-E void FDECL(reset_rndmonst, (int));
 E struct permonst *FDECL(mkclass, (CHAR_P, int));
 E struct permonst *FDECL(mkclass_aligned, (CHAR_P, int, ALIGNTYP_P));
 E int FDECL(mkclass_poly, (int));

include/monst.h

@@ -201,4 +201,13 @@ struct monst {
 #define is_obj_mappear(mon,otyp) (M_AP_TYPE(mon) == M_AP_OBJECT \
                                   && (mon)->mappearance == (otyp))
 
+/* Get the maximum difficulty monsters that can currently be generated,
+   given the current level difficulty and the hero's level. */
+#define monmax_difficulty(levdif) (((levdif) + u.ulevel) / 2)
+#define monmin_difficulty(levdif) ((levdif) / 6)
+
+/* Macros for whether a type of monster is too strong for a specific level. */
+#define montoostrong(monindx, lev) (mons[monindx].difficulty > lev)
+#define montooweak(monindx, lev) (mons[monindx].difficulty < lev)
+
 #endif /* MONST_H */

src/decl.c

@@ -464,8 +464,6 @@ const struct instance_globals g_init = {
     UNDEFINED_VALUES,
 
     /* makemon.c */
-    { -1, /* choice_count */
-     { 0 } }, /* mchoices */
 
     /* mhitm.c */
     UNDEFINED_VALUE, /* vis */

src/do.c

@@ -1453,7 +1453,6 @@ boolean at_stairs, falling, portal;
             || dunlev(&u.uz) < dunlev_reached(&u.uz))
             dunlev_reached(&u.uz) = dunlev(&u.uz);
     }
-    reset_rndmonst(NON_PM); /* u.uz change affects monster generation */
 
     /* set default level change destination areas */
     /* the special level code may override these */

src/exper.c

@@ -215,7 +215,6 @@ const char *drainer; /* cause of death, if drain should be fatal */
         pline("%s level %d.", Goodbye(), u.ulevel--);
         /* remove intrinsic abilities */
         adjabil(u.ulevel + 1, u.ulevel);
-        reset_rndmonst(NON_PM); /* new monster selection */
     } else {
         if (drainer) {
             g.killer.format = KILLED_BY;
@@ -315,7 +314,6 @@ boolean incr; /* true iff via incremental experience growth */
         if (u.ulevelmax < u.ulevel)
             u.ulevelmax = u.ulevel;
         adjabil(u.ulevel - 1, u.ulevel); /* give new intrinsics */
-        reset_rndmonst(NON_PM);          /* new monster selection */
     }
     g.context.botl = TRUE;
 }

src/makemon.c

@@ -27,8 +27,6 @@ static boolean FDECL(makemon_rnd_goodpos, (struct monst *,
 
 #define m_initsgrp(mtmp, x, y, mmf) m_initgrp(mtmp, x, y, 3, mmf)
 #define m_initlgrp(mtmp, x, y, mmf) m_initgrp(mtmp, x, y, 10, mmf)
-#define toostrong(monindx, lev) (mons[monindx].difficulty > lev)
-#define tooweak(monindx, lev) (mons[monindx].difficulty < lev)
 
 boolean
 is_home_elemental(ptr)
@@ -945,7 +943,6 @@ boolean ghostly;
                         makeplural(mons[mndx].mname));
         }
         g.mvitals[mndx].mvflags |= G_EXTINCT;
-        reset_rndmonst(mndx);
     }
     return result;
 }
@@ -1517,98 +1514,75 @@ struct permonst *
 rndmonst()
 {
     register struct permonst *ptr;
-    register int mndx, ct;
+    register int mndx;
+    int weight, totalweight, selected_mndx, zlevel, minmlev, maxmlev;
+    boolean elemlevel, upper;
 
     if (u.uz.dnum == quest_dnum && rn2(7) && (ptr = qt_montype()) != 0)
         return ptr;
 
-    if (g.rndmonst_state.choice_count < 0) { /* need to recalculate */
+    zlevel = level_difficulty();
-        int zlevel, minmlev, maxmlev;
+    minmlev = monmin_difficulty(zlevel);
-        boolean elemlevel;
+    maxmlev = monmax_difficulty(zlevel);
-        boolean upper;
+    upper = Is_rogue_level(&u.uz); /* prefer uppercase only on rogue level */
+    elemlevel = In_endgame(&u.uz) && !Is_astralevel(&u.uz); /* elmntl plane */
 
-        g.rndmonst_state.choice_count = 0;
+    /* amount processed so far */
-        /* look for first common monster */
+    totalweight = 0;
-        for (mndx = LOW_PM; mndx < SPECIAL_PM; mndx++) {
+    selected_mndx = NON_PM;
-            if (!uncommon(mndx))
+
-                break;
+    for (mndx = LOW_PM; mndx < SPECIAL_PM; ++mndx) {
-            g.rndmonst_state.mchoices[mndx] = 0;
+        ptr = &mons[mndx];
-        }
+
-        if (mndx == SPECIAL_PM) {
+        if (montooweak(mndx, minmlev) || montoostrong(mndx, maxmlev))
-            /* evidently they've all been exterminated */
+            continue;
-            debugpline0("rndmonst: no common mons!");
+        if (upper && !isupper((uchar) def_monsyms[(int) ptr->mlet].sym))
-            return (struct permonst *) 0;
+            continue;
-        } /* else `mndx' now ready for use below */
+        if (elemlevel && wrong_elem_type(ptr))
-        zlevel = level_difficulty();
+            continue;
-        /* determine the level of the weakest monster to make. */
+        if (uncommon(mndx))
-        minmlev = zlevel / 6;
+            continue;
-        /* determine the level of the strongest monster to make. */
+        if (Inhell && (ptr->geno & G_NOHELL))
-        maxmlev = (zlevel + u.ulevel) / 2;
+            continue;
-        upper = Is_rogue_level(&u.uz);
-        elemlevel = In_endgame(&u.uz) && !Is_astralevel(&u.uz);
 
         /*
-         * Find out how many monsters exist in the range we have selected.
+         * Weighted reservoir sampling:  select ptr with a
+         * (ptr weight)/(total of all weights so far including ptr's)
+         * probability.  For example, if the previous total is 10, and
+         * this is now looking at acid blobs with a frequency of 2, it
+         * has a 2/12 chance of abandoning ptr's previous value in favor
+         * of acid blobs, and 10/12 chance of keeping whatever it was.
+         *
+         * This does not bias results towards either the earlier or the
+         * later monsters:  the smaller pool and better odds from being
+         * earlier are exactly canceled out by having more monsters to
+         * potentially steal its spot.
          */
-        for ( ; mndx < SPECIAL_PM; mndx++) { /* (`mndx' initialized above) */
+        weight = (int) (ptr->geno & G_FREQ) + align_shift(ptr);
-            ptr = &mons[mndx];
+        if (weight < 0 || weight > 127) {
-            g.rndmonst_state.mchoices[mndx] = 0;
+            impossible("bad weight in rndmonst for mndx %d", mndx);
-            if (tooweak(mndx, minmlev) || toostrong(mndx, maxmlev))
+            weight = 0;
-                continue;
+        }
-            if (upper && !isupper((uchar) def_monsyms[(int) ptr->mlet].sym))
+        /* was unconditional, but if weight==0, rn2() < 0 will always fail;
-                continue;
+           also need to avoid rn2(0) if totalweight is still 0 so far */
-            if (elemlevel && wrong_elem_type(ptr))
+        if (weight > 0) {
-                continue;
+            totalweight += weight; /* totalweight now guaranteed to be > 0 */
-            if (uncommon(mndx))
+            if (rn2(totalweight) < weight)
-                continue;
+                selected_mndx = mndx;
-            if (Inhell && (ptr->geno & G_NOHELL))
-                continue;
-            ct = (int) (ptr->geno & G_FREQ) + align_shift(ptr);
-            if (ct < 0 || ct > 127)
-                panic("rndmonst: bad count [#%d: %d]", mndx, ct);
-            g.rndmonst_state.choice_count += ct;
-            g.rndmonst_state.mchoices[mndx] = (char) ct;
         }
-        /*
-         *      Possible modification:  if choice_count is "too low",
-         *      expand minmlev..maxmlev range and try again.
-         */
-    } /* choice_count+mchoices[] recalc */
-
-    if (g.rndmonst_state.choice_count <= 0) {
-        /* maybe no common mons left, or all are too weak or too strong */
-        debugpline1("rndmonst: choice_count=%d", g.rndmonst_state.choice_count);
-        return (struct permonst *) 0;
     }
-
     /*
-     *  Now, select a monster at random.
+     * Possible modification:  if totalweight is "too low" or nothing
+     * viable was picked, expand minmlev..maxmlev range and try again.
      */
-    ct = rnd(g.rndmonst_state.choice_count);
+    if (selected_mndx == NON_PM || uncommon(selected_mndx)) {
-    for (mndx = LOW_PM; mndx < SPECIAL_PM; mndx++)
+        /* maybe no common monsters left, or all are too weak or too strong */
-        if ((ct -= (int) g.rndmonst_state.mchoices[mndx]) <= 0)
+        if (selected_mndx != NON_PM)
-            break;
+            debugpline1("rndmonst returning Null [uncommon 'mndx'=#%d]",
-
+                        selected_mndx);
-    if (mndx == SPECIAL_PM || uncommon(mndx)) { /* shouldn't happen */
-        impossible("rndmonst: bad `mndx' [#%d]", mndx);
         return (struct permonst *) 0;
     }
-    return &mons[mndx];
+    return &mons[selected_mndx];
-}
-
-/* called when you change level (experience or dungeon depth) or when
-   monster species can no longer be created (genocide or extinction) */
-void
-reset_rndmonst(mndx)
-int mndx; /* particular species that can no longer be created */
-{
-    /* cached selection info is out of date */
-    if (mndx == NON_PM) {
-        g.rndmonst_state.choice_count = -1; /* full recalc needed */
-    } else if (mndx < SPECIAL_PM) {
-        g.rndmonst_state.choice_count -= g.rndmonst_state.mchoices[mndx];
-        g.rndmonst_state.mchoices[mndx] = 0;
-    } /* note: safe to ignore extinction of unique monsters */
 }
 
 /* decide whether it's ok to generate a candidate monster by mkclass() */
@@ -1687,7 +1661,7 @@ aligntyp atyp;
                (or lower) difficulty as preceding candidate (non-zero
                'num' implies last > first so mons[last-1] is safe);
                sometimes accept it even if high difficulty */
-            if (num && toostrong(last, maxmlev)
+            if (num && montoostrong(last, maxmlev)
                 && mons[last].difficulty > mons[last - 1].difficulty
                 && rn2(2))
                 break;

src/polyself.c

@@ -299,7 +299,6 @@ newman()
         change_sex();
 
     adjabil(oldlvl, (int) u.ulevel);
-    reset_rndmonst(NON_PM); /* new monster generation criteria */
 
     /* random experience points for the new experience level */
     u.uexp = rndexp(FALSE);

src/read.c

@@ -2107,7 +2107,6 @@ do_class_genocide()
                      * have G_GENOD or !G_GENO.
                      */
                     g.mvitals[i].mvflags |= (G_GENOD | G_NOCORPSE);
-                    reset_rndmonst(i);
                     kill_genocided_monsters();
                     update_inventory(); /* eggs & tins */
                     pline("Wiped out all %s.", nam);
@@ -2324,7 +2323,6 @@ int how;
         } else if (ptr == g.youmonst.data) {
             rehumanize();
         }
-        reset_rndmonst(mndx);
         kill_genocided_monsters();
         update_inventory(); /* in case identified eggs were affected */
     } else {