Remove requirement of object probs adding to 1000

When discussing the recent commit that removed makedefs -o from the
build process, nhmall pointed out that a sanity check ensuring all
objects within one class add up to 1000 probability had been removed as
well. This requirement was a perennial thorn in the side for anyone
doing anything that touches object probabilities, because allocating
probability to something meant deciding what to take it away from,
without a good way to evenly distribute that across all the other
members of the object class.

I had gotten around this in xNetHack by removing the sanity check and
making mkobj() total up the probability within an object class and then
using that instead of 1000. This commit takes a similar approach, but
instead of inefficiently recalculating the sum every time mkobj() is
called, it instead computes it at the start of the game or when
restoring the save file and stores it in a global variable.

This fixes a slight bias problem with rings - they are all supposed to
be of equal probability, but there are 28 of them and 1000 is not evenly
divisible by that, so the old formula made the later rings slightly more
likely. Now instead of a 35/1000 or 36/1000 chance, they are all
uniformly 1/28. (Internally they have a oc_prob of 1 now, not 0).

Gems are also weird, because their oc_prob values change every level.
This ought to have still worked without a change, because the arcane
formula for assigning the probabilities would still end up with them
adding to 1000. But I added in code to reset the total gem probability
anyway; this may help make the formula less arcane in the future.

There is still a sanity check against object classes having a nonzero
number of objects but zero total probability, in which case an
impossible will be thrown and every member of the class will be given
equal probability. I also downgraded the "probtype error" panic in
mkobj() to an impossible because it has a reasonable failure case -
return the first item in that class.

diff --git a/include/decl.h b/include/decl.h
index 82543e4ee25e6e93d4c74a8ebafb2569048ba516..941b45a095467258e6a12c537710c056a89171d7 100644 (file)
--- a/include/decl.h
+++ b/include/decl.h
@@ -1056,6 +1056,7 @@ struct instance_globals {
 
     /* o_init.c */
     short disco[NUM_OBJECTS];
+    short oclass_prob_totals[MAXOCLASSES];
 
     /* objname.c */
     /* distantname used by distant_name() to pass extra information to

diff --git a/include/extern.h b/include/extern.h
index 3305405b60e50bf048d9e6b462ee8e26e86a28f9..6bf82cd25bbb980bdcf5e70d036191b90fb0510e 100644 (file)
--- a/include/extern.h
+++ b/include/extern.h
@@ -1741,6 +1741,7 @@ extern void consoletty_exit(void);
 /* ### o_init.c ### */
 
 extern void init_objects(void);
+extern void init_oclass_probs(void);
 extern void obj_shuffle_range(int, int *, int *);
 extern int find_skates(void);
 extern boolean objdescr_is(struct obj *, const char *);

diff --git a/include/objects.h b/include/objects.h
index ba95a626ac93a60855616de69c9d7d969e578eb7..7018417d36ea6337ae6727bb881957179ccb069e 100644 (file)
--- a/include/objects.h
+++ b/include/objects.h
@@ -651,7 +651,7 @@ BOOTS("levitation boots", "snow boots",
     OBJECT(OBJ(name, stone),                                          \
            BITS(0, 0, spec, 0, mgc, spec, 0, 0, 0,                    \
                 HARDGEM(mohs), 0, P_NONE, metal),                     \
-           power, RING_CLASS, 0, 0, 3, cost, 0, 0, 0, 0, 15, color,sn)
+           power, RING_CLASS, 1, 0, 3, cost, 0, 0, 0, 0, 15, color,sn)
 RING("adornment", "wooden",
      ADORNED,                  100, 1, 1, 2, WOOD, HI_WOOD, RIN_ADORNMENT),
 RING("gain strength", "granite",

diff --git a/src/decl.c b/src/decl.c
index 796079961665aa4be26b3d88a0802c1dd0c4b126..3a7ee43f3b3689c1f07132488731e387a1b3a51c 100644 (file)
--- a/src/decl.c
+++ b/src/decl.c
@@ -528,6 +528,7 @@ const struct instance_globals g_init = {
 
     /* o_init.c */
     DUMMY, /* disco */
+    DUMMY, /* oclass_prob_totals */
 
     /* objname.c */
     0, /* distantname */

diff --git a/src/mkobj.c b/src/mkobj.c
index c2cd6a6523b6b110140a6bfbba9b12d77785be63..6d7902b7f5052c98ce318781ef1eafbad4b70e0e 100644 (file)
--- a/src/mkobj.c
+++ b/src/mkobj.c
@@ -206,7 +206,7 @@ mksobj_migr_to_species(
 struct obj *
 mkobj(int oclass, boolean artif)
 {
-    int tprob, i, prob = rnd(1000);
+    int tprob, i, prob;
 
     if (oclass == RANDOM_CLASS) {
         const struct icp *iprobs = Is_rogue_level(&u.uz)
@@ -223,13 +223,16 @@ mkobj(int oclass, boolean artif)
         i = rnd_class(g.bases[SPBOOK_CLASS], SPE_BLANK_PAPER);
         oclass = SPBOOK_CLASS; /* for sanity check below */
     } else {
+        prob = rnd(g.oclass_prob_totals[oclass]);
         i = g.bases[oclass];
         while ((prob -= objects[i].oc_prob) > 0)
             ++i;
     }
 
-    if (objects[i].oc_class != oclass || !OBJ_NAME(objects[i]))
-        panic("probtype error, oclass=%d i=%d", (int) oclass, i);
+    if (objects[i].oc_class != oclass || !OBJ_NAME(objects[i])) {
+        impossible("probtype error, oclass=%d i=%d", (int) oclass, i);
+        i = g.bases[oclass];
+    }
 
     return mksobj(i, TRUE, artif);
 }

diff --git a/src/o_init.c b/src/o_init.c
index 22dcab4453908f14187b1ab3ac9d98cc7526c6c6..ef1fd6036d576a3a7654badd0f0b60344457aed4 100644 (file)
--- a/src/o_init.c
+++ b/src/o_init.c
@@ -42,7 +42,7 @@ shuffle_tiles(void)
 static void
 setgemprobs(d_level* dlev)
 {
-    int j, first, lev;
+    int j, first, lev, sum = 0;
 
     if (dlev)
         lev = (ledger_no(dlev) > maxledgerno()) ? maxledgerno()
@@ -62,6 +62,11 @@ setgemprobs(d_level* dlev)
     }
     for (j = first; j <= LAST_GEM; j++)
         objects[j].oc_prob = (171 + j - first) / (LAST_GEM + 1 - first);
+
+    /* recompute GEM_CLASS total oc_prob - including rocks/stones */
+    for (j = g.bases[GEM_CLASS]; j < g.bases[GEM_CLASS + 1]; j++)
+        sum += objects[j].oc_prob;
+    g.oclass_prob_totals[GEM_CLASS] = sum;
 }
 
 /* shuffle descriptions on objects o_low to o_high */
@@ -106,7 +111,7 @@ shuffle(int o_low, int o_high, boolean domaterial)
 void
 init_objects(void)
 {
-    int i, first, last, sum, prevoclass;
+    int i, first, last, prevoclass;
     char oclass;
 #ifdef TEXTCOLOR
 #define COPY_OBJ_DESCR(o_dst, o_src) \
@@ -167,17 +172,6 @@ init_objects(void)
                 break;
             }
         }
- checkprob:
-        sum = 0;
-        for (i = first; i < last; i++)
-            sum += objects[i].oc_prob;
-        if (sum == 0) {
-            for (i = first; i < last; i++)
-                objects[i].oc_prob = (1000 + i - first) / (last - first);
-            goto checkprob;
-        }
-        if (sum != 1000)
-            error("init-prob error for class %d (%d%%)", oclass, sum);
         first = last;
         prevoclass = (int) oclass;
     }
@@ -207,6 +201,8 @@ init_objects(void)
             objects[i].oc_name_known = nmkn ? 0 : 1;
         }
     }
+    /* compute oclass_prob_totals */
+    init_oclass_probs();
 
     /* shuffle descriptions */
     shuffle_all();
@@ -216,6 +212,33 @@ init_objects(void)
     objects[WAN_NOTHING].oc_dir = rn2(2) ? NODIR : IMMEDIATE;
 }
 
+/* Compute the total probability of each object class.
+ * Assumes g.bases[] has already been set. */
+void
+init_oclass_probs(void)
+{
+    int i;
+    short sum;
+    int oclass;
+    for (oclass = 0; oclass < MAXOCLASSES; ++oclass) {
+        sum = 0;
+        for (i = g.bases[oclass]; i < g.bases[oclass + 1]; ++i) {
+            sum += objects[i].oc_prob;
+        }
+        if (sum <= 0 && oclass != ILLOBJ_CLASS
+            && g.bases[oclass] != g.bases[oclass + 1]) {
+            impossible("zero or negative probability total for oclass %d",
+                       oclass);
+            /* gracefully fail by setting all members of this class to 1 */
+            for (i = g.bases[oclass]; i < g.bases[oclass + 1]; ++i) {
+                objects[i].oc_prob = 1;
+                sum++;
+            }
+        }
+        g.oclass_prob_totals[oclass] = sum;
+    }
+}
+
 /* retrieve the range of objects that otyp shares descriptions with */
 void
 obj_shuffle_range(

diff --git a/src/restore.c b/src/restore.c
index 1afa8269364eeea0f5ba38eac6683083739d1ef6..ddabdab32b2439ff91921aacce7b1a4c054368d2 100644 (file)
--- a/src/restore.c
+++ b/src/restore.c
@@ -527,7 +527,7 @@ restgamestate(NHFILE* nhfp, unsigned int* stuckid, unsigned int* steedid)
 
     if (nhfp->structlevel)
         mread(nhfp->fd, (genericptr_t) &uid, sizeof uid);
-      
+
     if (SYSOPT_CHECK_SAVE_UID
         && uid != (unsigned long) getuid()) { /* strange ... */
         /* for wizard mode, issue a reminder; for others, treat it
@@ -582,7 +582,7 @@ restgamestate(NHFILE* nhfp, unsigned int* stuckid, unsigned int* steedid)
     if (nhfp->structlevel)
         mread(nhfp->fd, (genericptr_t) &u, sizeof(struct you));
     g.youmonst.cham = u.mcham;
-        
+
     if (nhfp->structlevel)
         mread(nhfp->fd, (genericptr_t) timebuf, 14);
     timebuf[14] = '\0';
@@ -692,7 +692,7 @@ restgamestate(NHFILE* nhfp, unsigned int* stuckid, unsigned int* steedid)
     }
     freefruitchn(g.ffruit); /* clean up fruit(s) made by initoptions() */
     g.ffruit = loadfruitchn(nhfp);
-  
+
     restnames(nhfp);
     restore_waterlevel(nhfp);
     restore_msghistory(nhfp);
@@ -857,6 +857,7 @@ dorecover(NHFILE* nhfp)
     substitute_tiles(&u.uz);
 #endif
     max_rank_sz(); /* to recompute g.mrank_sz (botl.c) */
+    init_oclass_probs(); /* recompute g.oclass_prob_totals[] */
     /* take care of iron ball & chain */
     for (otmp = fobj; otmp; otmp = otmp->nobj)
         if (otmp->owornmask)
@@ -1078,7 +1079,7 @@ getlev(NHFILE* nhfp, int pid, xchar lev)
         g.doorindex = g.rooms[g.nroom - 1].fdoor + g.rooms[g.nroom - 1].doorct;
     else
         g.doorindex = 0;
-  
+
     restore_timers(nhfp, RANGE_LEVEL, elapsed);
     restore_light_sources(nhfp);
     fmon = restmonchn(nhfp);