From 0fe6a731dff521a7dd1c1a9f3c36d5438d24c22e Mon Sep 17 00:00:00 2001 From: PatR Date: Sun, 16 Dec 2018 14:21:30 -0800 Subject: [PATCH] fix #H2204 - mkclass() mon selection distribution That #H number isn't a typo. This finally fixes--at least improves-- something reported eight years ago. The monster types chosen by mkclass() could be way off in some circumstances. Cited example was repeated same-race sacrifice by chaotic hero on dungeon level 20; it produced about twice as many incubi as succubi even though they're the same as far as difficulty goes. (No changes in the intervening years had any discernable effect; that was still reproducible.) The report also mentioned that ndemon() threw away the result from mkclass() and retried quite often and suggested that mkclass() be taught to filter by alignment when caller cared about that. This seems to even things out, although it also made harder monsters chosen more often. A test program generated these numbers when picking a chaotic demon 10000 times (level 1 hero on dungeon level 20, so not realistic; actually probably level 0 hero since the program didn't initialize struct u.) Third column is the number of times the monster type was chosen with the old mkclass(), fourth is same for the new one. mkclass() calls 27315 10000 286 succubus 2800 3309 288 incubus 5552 3262 291 marilith 973 780 292 vrock 477 1617 293 hezrou 150 626 294 bone devil 46 247 295 ice devil 2 107 296 nalfeshnee 0 23 297 pit fiend 0 15 298 sandestin 0 4 299 balrog 0 10 Note that vrock has generation frequency 2 and marilith only 1, so getting twice as many vrocks as mariliths should be expected. I temporarily changed ndemon() to ask for lawful demons instead of chaotic ones and got this. mkclass() calls 15762 10000 287 horned devil 3197 3375 289 erinys 4991 3339 290 barbed devil 1812 3286 I also ran it for dragons with any alignment (so the outcome was never thrown away; 10000 calls were needed for 10000 picks) instead of demons of specific alignment and am suspicious of the outcome. mkclass() calls 10000 10000 140 baby yellow dragon 1124 0 141 gray dragon 1096 1096 142 silver dragon 1073 1099 143 red dragon 1061 1126 144 white dragon 1077 1128 145 orange dragon 1141 1118 146 black dragon 1154 1049 147 blue dragon 1137 1123 148 green dragon 1137 1154 149 yellow dragon 0 1107 There may be a flaw in the test program. Or else old mkclass() was not very good at picking dragons. --- doc/fixes36.2 | 2 ++ include/extern.h | 3 +- src/makemon.c | 86 ++++++++++++++++++++++++++++++++---------------- src/minion.c | 49 +++++++++++++++------------ 4 files changed, 90 insertions(+), 50 deletions(-) diff --git a/doc/fixes36.2 b/doc/fixes36.2 index 1d8176f7e..f7a3d5983 100644 --- a/doc/fixes36.2 +++ b/doc/fixes36.2 @@ -271,6 +271,8 @@ clairvoyance revealing underwater or under-lava objects left object displayed make it easier to clear 'pickup_types' (menustyles Traditional and Combination could do so by setting it to 'a'; now ' ' works too; Full and Partial allowed unselecting every object class; now 'all classes' is a choice) +distribution of monsters selected by mkclass() didn't match expected distrib + (cited example was ndemon() creating twice as many incubi as succubi) Fixes to Post-3.6.1 Problems that Were Exposed Via git Repository diff --git a/include/extern.h b/include/extern.h index 67a3d0d0e..df9477b62 100644 --- a/include/extern.h +++ b/include/extern.h @@ -1,4 +1,4 @@ -/* NetHack 3.6 extern.h $NHDT-Date: 1544669659 2018/12/13 02:54:19 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.667 $ */ +/* NetHack 3.6 extern.h $NHDT-Date: 1544998887 2018/12/16 22:21:27 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.671 $ */ /* Copyright (c) Steve Creps, 1988. */ /* NetHack may be freely redistributed. See license for details. */ @@ -1152,6 +1152,7 @@ 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)); E int FDECL(adj_lev, (struct permonst *)); E struct permonst *FDECL(grow_up, (struct monst *, struct monst *)); diff --git a/src/makemon.c b/src/makemon.c index 20912b8da..d78363130 100644 --- a/src/makemon.c +++ b/src/makemon.c @@ -1,4 +1,4 @@ -/* NetHack 3.6 makemon.c $NHDT-Date: 1542798623 2018/11/21 11:10:23 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.128 $ */ +/* NetHack 3.6 makemon.c $NHDT-Date: 1544998885 2018/12/16 22:21:25 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.131 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /*-Copyright (c) Robert Patrick Rankin, 2012. */ /* NetHack may be freely redistributed. See license for details. */ @@ -1635,59 +1635,89 @@ int mndx, mvflagsmask, genomask; } /* Make one of the multiple types of a given monster class. - * The second parameter specifies a special casing bit mask - * to allow the normal genesis masks to be deactivated. - * Returns Null if no monsters in that class can be made. - */ + The second parameter specifies a special casing bit mask + to allow the normal genesis masks to be deactivated. + Returns Null if no monsters in that class can be made. */ struct permonst * mkclass(class, spc) char class; int spc; +{ + return mkclass_aligned(class, spc, A_NONE); +} + +/* mkclass() with alignment restrictions; used by ndemon() */ +struct permonst * +mkclass_aligned(class, spc, atyp) +char class; +int spc; +aligntyp atyp; { register int first, last, num = 0; + int k, nums[SPECIAL_PM + 1]; /* +1: insurance for final return value */ int maxmlev, mask = (G_NOGEN | G_UNIQ) & ~spc; + (void) memset((genericptr_t) nums, 0, sizeof nums); maxmlev = level_difficulty() >> 1; if (class < 1 || class >= MAXMCLASSES) { impossible("mkclass called with bad class!"); return (struct permonst *) 0; } /* Assumption #1: monsters of a given class are contiguous in the - * mons[] array. + * mons[] array. Player monsters and quest denizens + * are an exception; mkclass() won't pick them. + * SPECIAL_PM is long worm tail and separates the + * regular monsters from the exceptions. */ for (first = LOW_PM; first < SPECIAL_PM; first++) if (mons[first].mlet == class) break; - if (first == SPECIAL_PM) - return (struct permonst *) 0; - - for (last = first; last < SPECIAL_PM && mons[last].mlet == class; last++) - if (mk_gen_ok(last, G_GONE, mask)) { - /* consider it */ - if (num && toostrong(last, maxmlev) - && mons[last].difficulty != mons[last - 1].difficulty && rn2(2)) - break; - num += mons[last].geno & G_FREQ; - } - if (!num) + if (first == SPECIAL_PM) { + impossible("mkclass found no class %d monsters", class); return (struct permonst *) 0; + } /* Assumption #2: monsters of a given class are presented in ascending * order of strength. */ - for (num = rnd(num); num > 0; first++) - if (mk_gen_ok(first, G_GONE, mask)) { - /* skew towards lower value monsters at lower exp. levels */ - num -= mons[first].geno & G_FREQ; - if (num && adj_lev(&mons[first]) > (u.ulevel * 2)) { - /* but not when multiple monsters are same level */ - if (mons[first].mlevel != mons[first + 1].mlevel) - num--; + for (last = first; last < SPECIAL_PM && mons[last].mlet == class; last++) { + if (atyp != A_NONE && sgn(mons[last].maligntyp) != sgn(atyp)) + continue; + if (mk_gen_ok(last, G_GONE, mask)) { + /* consider it; don't reject a toostrong() monster if we + don't have anything yet (num==0) or if it is the same + (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) + && mons[last].difficulty > mons[last - 1].difficulty + && rn2(2)) + break; + if ((k = (mons[last].geno & G_FREQ)) > 0) { + /* skew towards lower value monsters at lower exp. levels + (this used to be done in the next loop, but that didn't + work well when multiple species had the same level and + were followed by one that was past the bias threshold; + cited example was sucubus and incubus, where the bias + against picking the next demon resulted in incubus + being picked nearly twice as often as sucubus); + we need the '+1' in case the entire set is too high + level (really low level hero) */ + nums[last] = k + 1 - (adj_lev(&mons[last]) > (u.ulevel * 2)); + num += nums[last]; } } - first--; /* correct an off-by-one error */ + } + if (!num) + return (struct permonst *) 0; - return &mons[first]; + /* the hard work has already been done; 'num' should hit 0 before + first reaches last (which is actually one past our last candidate) */ + for (num = rnd(num); first < last; first++) + if ((num -= nums[first]) <= 0) + break; + + return nums[first] ? &mons[first] : (struct permonst *) 0; } /* like mkclass(), but excludes difficulty considerations; used when diff --git a/src/minion.c b/src/minion.c index 1a2b22d56..973572184 100644 --- a/src/minion.c +++ b/src/minion.c @@ -1,4 +1,4 @@ -/* NetHack 3.6 minion.c $NHDT-Date: 1432512773 2015/05/25 00:12:53 $ $NHDT-Branch: master $:$NHDT-Revision: 1.33 $ */ +/* NetHack 3.6 minion.c $NHDT-Date: 1544998886 2018/12/16 22:21:26 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.40 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /*-Copyright (c) Robert Patrick Rankin, 2008. */ /* NetHack may be freely redistributed. See license for details. */ @@ -256,7 +256,7 @@ register struct monst *mtmp; flags.female ? "Sister" : "Brother"); if (!tele_restrict(mtmp)) (void) rloc(mtmp, TRUE); - return (1); + return 1; } cash = money_cnt(invent); demand = @@ -292,7 +292,7 @@ register struct monst *mtmp; } } mongone(mtmp); - return (1); + return 1; } long @@ -323,7 +323,7 @@ struct monst *mtmp; } (void) money2mon(mtmp, offer); context.botl = 1; - return (offer); + return offer; } int @@ -336,9 +336,9 @@ aligntyp atyp; pm = rn1(PM_DEMOGORGON + 1 - PM_ORCUS, PM_ORCUS); if (!(mvitals[pm].mvflags & G_GONE) && (atyp == A_NONE || sgn(mons[pm].maligntyp) == sgn(atyp))) - return (pm); + return pm; } - return (dlord(atyp)); /* approximate */ + return dlord(atyp); /* approximate */ } int @@ -351,9 +351,9 @@ aligntyp atyp; pm = rn1(PM_YEENOGHU + 1 - PM_JUIBLEX, PM_JUIBLEX); if (!(mvitals[pm].mvflags & G_GONE) && (atyp == A_NONE || sgn(mons[pm].maligntyp) == sgn(atyp))) - return (pm); + return pm; } - return (ndemon(atyp)); /* approximate */ + return ndemon(atyp); /* approximate */ } /* create lawful (good) lord */ @@ -361,9 +361,9 @@ int llord() { if (!(mvitals[PM_ARCHON].mvflags & G_GONE)) - return (PM_ARCHON); + return PM_ARCHON; - return (lminion()); /* approximate */ + return lminion(); /* approximate */ } int @@ -375,7 +375,7 @@ lminion() for (tryct = 0; tryct < 20; tryct++) { ptr = mkclass(S_ANGEL, 0); if (ptr && !is_lord(ptr)) - return (monsndx(ptr)); + return monsndx(ptr); } return NON_PM; @@ -383,19 +383,26 @@ lminion() int ndemon(atyp) -aligntyp atyp; +aligntyp atyp; /* A_NONE is used for 'any alignment' */ { - int tryct; struct permonst *ptr; - for (tryct = 0; tryct < 20; tryct++) { - ptr = mkclass(S_DEMON, 0); - if (ptr && is_ndemon(ptr) - && (atyp == A_NONE || sgn(ptr->maligntyp) == sgn(atyp))) - return (monsndx(ptr)); - } - - return NON_PM; + /* + * 3.6.2: [fix #H2204, 22-Dec-2010, eight years later...] + * pick a correctly aligned demon in one try. This used to + * use mkclass() to choose a random demon type and keep trying + * (up to 20 times) until it got one with the desired alignment. + * mkclass_aligned() skips wrongly aligned potential candidates. + * [The only neutral demons are djinni and mail daemon and + * mkclass() won't pick them, but call it anyway in case either + * aspect of that changes someday.] + */ +#if 0 + if (atyp == A_NEUTRAL) + return NON_PM; +#endif + ptr = mkclass_aligned(S_DEMON, 0, atyp); + return (ptr && is_ndemon(ptr)) ? monsndx(ptr) : NON_PM; } /* guardian angel has been affected by conflict so is abandoning hero */