BONES_POOLS fix and bonesid fix

The BONES_POOLS implementation added an extra dot to the bones file
name (only when enabled) which would be a problem on some filesystems.
This changes the name from "bonD0.15.3" to "bon3D0.15" which avoids
the second dot and also fits within 8.3 characters.  To enforce that,
the maximum value for BONES_POOLS is now 10 (yielding single-digit pool
numbers 0 through 9).

BONES_POOLS==1 will omit the pool number (that's not a change, just a
reminder), yielding "bonD0.15" and so on.  Right now, BONES_POOLS==0
is equivalent to BONES_POOLS=1, but it could be changed someday to
mean that bones files shouldn't be used if we decide to support that.

The pool number as a suffix was being included in content validation,
so it wasn't possible to move "bonD0.15.3" to pool 2 by renaming it to
"bonD0.15.2".  I'm not sure whether that was intentional, but it seems
overly draconian.  "bon3D0.15" can be renamed to "bon2D0.15" and then
be loaded by a game assigned to pool 2.  Also, pre-pool bones can be
retained by renaming to any valid pool and should still work.

The three letter filecode for quest bones has made the bonesid be
broken since 3.3.0 introduced it (the three letter code, not bones-id).
"QArc.2" for level 2 of the Archeologist quest was being written into
the bones file as "rc.2", but worked as intended because validation
when loading bones had the same mistake.  This fixes it to use "QArc.2"
when saving and accept either "QArc.2" or "rc.2" when loading, so 3.6.0
bones files (and existing to-be-3.6.1 bones) will continue to work.

diff --git a/doc/fixes36.1 b/doc/fixes36.1
index 530523f43b083e3f6877cf0f0763fae430287498..7324c9eac8e4d6317d067a733a06b5b2e216c9ff 100644 (file)
--- a/doc/fixes36.1
+++ b/doc/fixes36.1
@@ -410,6 +410,10 @@ throne room's throne is occupied by a king
 using a grappling hook and getting pulled toward the target into water would
        drown hero without any chance to crawl out
 blinded monster who eats a carrot will have blindness cured (hero already did)
+the "bonesid" written into bones files when they're created so that they can
+       be validated when loaded was incorrect for bones in the quest branch
+       ("QBar.2" ended up being "ar.2", "QKni.4" ended up as "ni.4", and so
+       forth) but worked anyway, because validation used same incorrect value
 
 
 Fixes to Post-3.6.0 Problems that Were Exposed Via git Repository

diff --git a/src/bones.c b/src/bones.c
index 6f35d26311e22c862b0a2e49197032b8bc93fe97..4d3f6713826b48f634bee5600f8c62520bac8b5c 100644 (file)
--- a/src/bones.c
+++ b/src/bones.c
@@ -1,4 +1,4 @@
-/* NetHack 3.6 bones.c $NHDT-Date: 1450432756 2015/12/18 09:59:16 $  $NHDT-Branch: NetHack-3.6.0 $:$NHDT-Revision: 1.68 $ */
+/* NetHack 3.6 bones.c $NHDT-Date: 1503309019 2017/08/21 09:50:19 $  $NHDT-Branch: NetHack-3.6.0 $:$NHDT-Revision: 1.70 $ */
 /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985,1993. */
 /* NetHack may be freely redistributed.  See license for details. */
 
@@ -549,7 +549,7 @@ getbones()
 {
     register int fd;
     register int ok;
-    char c, *bonesid, oldbonesid[10];
+    char c, *bonesid, oldbonesid[40]; /* was [10]; more should be safer */
 
     if (discover) /* save bones files for real games */
         return 0;
@@ -581,7 +581,15 @@ getbones()
         }
         mread(fd, (genericptr_t) &c, sizeof c); /* length incl. '\0' */
         mread(fd, (genericptr_t) oldbonesid, (unsigned) c); /* DD.nnn */
-        if (strcmp(bonesid, oldbonesid) != 0) {
+        if (strcmp(bonesid, oldbonesid) != 0
+            /* from 3.3.0 through 3.6.0, bones in the quest branch stored
+               a bogus bonesid in the file; 3.6.1 fixed that, but for
+               3.6.0 bones to remain compatible, we need an extra test;
+               once compatibility with 3.6.x goes away, this can too
+               (we don't try to make this conditional upon the value of
+               VERSION_COMPATIBILITY because then we'd need patchlevel.h) */
+            && (strlen(bonesid) <= 2
+                || strcmp(bonesid + 2, oldbonesid) != 0)) {
             char errbuf[BUFSZ];
 
             Sprintf(errbuf, "This is bones level '%s', not '%s'!", oldbonesid,

diff --git a/src/files.c b/src/files.c
index abda2a7704ff073b6606296d7d7d4cf9b1459ec1..a8efcb5d6a903195604d516ea9c5fad808e23cb3 100644 (file)
--- a/src/files.c
+++ b/src/files.c
@@ -1,4 +1,4 @@
-/* NetHack 3.6 files.c $NHDT-Date: 1502581476 2017/08/12 23:44:36 $  $NHDT-Branch: NetHack-3.6.0 $:$NHDT-Revision: 1.211 $ */
+/* NetHack 3.6 files.c $NHDT-Date: 1503309020 2017/08/21 09:50:20 $  $NHDT-Branch: NetHack-3.6.0 $:$NHDT-Revision: 1.215 $ */
 /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
 /* NetHack may be freely redistributed.  See license for details. */
 
@@ -697,21 +697,47 @@ d_level *lev;
     s_level *sptr;
     char *dptr;
 
-    Sprintf(file, "bon%c%s", dungeons[lev->dnum].boneid,
+    /*
+     * "bonD0.nn"   = bones for level nn in the main dungeon;
+     * "bonM0.T"    = bones for Minetown;
+     * "bonQBar.n"  = bones for level n in the Barbarian quest;
+     * "bon3D0.nn"  = \
+     * "bon3M0.T"   =  > same as above, but for bones pool #3.
+     * "bon3QBar.n" = /
+     *
+     * Return value for content validation skips "bon" and the
+     * pool number (if present), making it feasible for the admin
+     * to manually move a bones file from one pool to another by
+     * renaming it.
+     */
+    Strcpy(file, "bon");
+#ifdef SYSCF
+    if (sysopt.bones_pools > 1) {
+        unsigned poolnum = min((unsigned) sysopt.bones_pools, 10);
+
+        poolnum = (unsigned) ubirthday % poolnum; /* 0..9 */
+        Sprintf(eos(file), "%u", poolnum);
+    }
+#endif
+    dptr = eos(file); /* this used to be after the following Sprintf()
+                         and the return value was (dptr - 2) */
+    /* when this naming scheme was adopted, 'filecode' was one letter;
+       3.3.0 turned it into a three letter string (via roles[] in role.c);
+       from that version through 3.6.0, 'dptr' pointed past the filecode
+       and the return value of (dptr - 2)  was wrong for bones produced
+       in the quest branch, skipping the boneid character 'Q' and the
+       first letter of the role's filecode; bones loading still worked
+       because the bonesid used for validation had the same error */
+    Sprintf(dptr, "%c%s", dungeons[lev->dnum].boneid,
             In_quest(lev) ? urole.filecode : "0");
-    dptr = eos(file);
     if ((sptr = Is_special(lev)) != 0)
-        Sprintf(dptr, ".%c", sptr->boneid);
+        Sprintf(eos(dptr), ".%c", sptr->boneid);
     else
-        Sprintf(dptr, ".%d", lev->dlevel);
-#ifdef SYSCF
-    if (sysopt.bones_pools > 1)
-        Sprintf(eos(file), ".%d", (ubirthday % sysopt.bones_pools));
-#endif
+        Sprintf(eos(dptr), ".%d", lev->dlevel);
 #ifdef VMS
     Strcat(dptr, ";1");
 #endif
-    return (dptr - 2);
+    return dptr;
 }
 
 /* set up temporary file name for writing bones, to avoid another game's
@@ -2283,8 +2309,13 @@ int src;
             free((genericptr_t) sysopt.genericusers);
         sysopt.genericusers = dupstr(bufp);
     } else if (src == SET_IN_SYS && match_varname(buf, "BONES_POOLS", 10)) {
+        /* max value of 10 guarantees (N % bones.pools) will be one digit
+           so we don't lose control of the length of bones file names */
         n = atoi(bufp);
-        sysopt.bones_pools = (n < 0) ? 0 : n;
+        sysopt.bones_pools = (n <= 0) ? 0 : min(n, 10);
+        /* note: right now bones_pools==0 is the same as bones_pools==1,
+           but we could change that and make bones_pools==0 become an
+           indicator to suppress bones usage altogether */
     } else if (src == SET_IN_SYS && match_varname(buf, "SUPPORT", 7)) {
         if (sysopt.support)
             free((genericptr_t) sysopt.support);