From 2b1f8a1b432911948cc6c40af2336b5d7f8a07b2 Mon Sep 17 00:00:00 2001 From: PatR Date: Mon, 13 Jan 2020 12:34:01 -0800 Subject: [PATCH] fix #K166 - role selection filtering pick_role() had a 5 year old copy+paste error where a pair of lines were cloned multiple times but one of the resulting lines didn't get the intended revision, preventing OPTIONS=align:!chaotic or !neutral or !lawful from working as intended when letting the game choose role randomly. The bad line should have been calling ok_align() but that routine turned out to have a bug too. Fixing those lead to other less obvious problems with role selection, particularly the tty menu version for picking manually. Roles and/or races which should have been excluded by partial specification weren't always kept out. Also, if any filtering was specified, trying to disable all filters (via choosing 'reset filtering' and de-selecting everything in the menu) was a no-op. Once any filtering was in place you had to leave at least one role or race or gender or alignment flagged as not acceptable in order to change any of the filtering. When that was fixed and it was possible to turn off all filtering, there was no way to turn it back on because the menu choice to reset the filters wasn't offered unless there was some filtering in place (that was intentional but turned out not to be a good idea). I checked curses and X11; they both offer less versatile selection capability that don't seem to need the tty-specific fixes. --- doc/fixes37.0 | 5 ++++- src/role.c | 14 +++++++++----- win/tty/wintty.c | 42 +++++++++++++++++++++++++++--------------- 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/doc/fixes37.0 b/doc/fixes37.0 index 63533bb38..988af40ac 100644 --- a/doc/fixes37.0 +++ b/doc/fixes37.0 @@ -1,4 +1,4 @@ -$NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.65 $ $NHDT-Date: 1578896120 2020/01/13 06:15:20 $ +$NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.66 $ $NHDT-Date: 1578947634 2020/01/13 20:33:54 $ General Fixes and Modified Features ----------------------------------- @@ -44,6 +44,7 @@ walking out of tethered-to-buried-object trap condition was supposed to attached chain that got dragged around but had no ball attached when poly'd into a giant and moving onto a boulder's spot, message given was confused about whether autopickup would occur so could be misleading +random role selection wasn't honoring unwanted alignment(s) properly Fixes to 3.7.0-x Problems that Were Exposed Via git Repository @@ -75,6 +76,8 @@ data.base lookup of an entry with any blank lines would falsely claim that Platform- and/or Interface-Specific Fixes ----------------------------------------- +tty: role and race selection menus weren't filtering out potential choices + which got excluded by OPTIONS=align:!lawful or !neutral or !chaotic General New Features diff --git a/src/role.c b/src/role.c index 1a1a88986..f93859da1 100644 --- a/src/role.c +++ b/src/role.c @@ -1,4 +1,4 @@ -/* NetHack 3.6 role.c $NHDT-Date: 1574648943 2019/11/25 02:29:03 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.65 $ */ +/* NetHack 3.6 role.c $NHDT-Date: 1578947634 2020/01/13 20:33:54 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.68 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985-1999. */ /*-Copyright (c) Robert Patrick Rankin, 2012. */ /* NetHack may be freely redistributed. See license for details. */ @@ -1047,8 +1047,8 @@ int racenum, gendnum, alignnum, pickhow; gendnum, alignnum) && ok_gend(i, racenum, (gendnum >= 0) ? gendnum : ROLE_RANDOM, alignnum) - && ok_race(i, racenum, - gendnum, (alignnum >= 0) ? alignnum : ROLE_RANDOM)) + && ok_align(i, racenum, + gendnum, (alignnum >= 0) ? alignnum : ROLE_RANDOM)) set[roles_ok++] = i; } if (roles_ok == 0 || (roles_ok > 1 && pickhow == PICK_RIGID)) @@ -1221,7 +1221,7 @@ int alignnum; /* random; check whether any selection is possible */ for (i = 0; i < ROLE_ALIGNS; i++) { if (g.rfilter.mask & aligns[i].allow) - return FALSE; + continue; allow = aligns[i].allow; if (rolenum >= 0 && rolenum < SIZE(roles) - 1 && !(allow & roles[rolenum].allow & ROLE_ALIGNMASK)) @@ -1909,9 +1909,13 @@ boolean preselect; add_menu(where, NO_GLYPH, &any, RS_menu_let[which], 0, ATR_NONE, buf, MENU_ITEMFLAGS_NONE); } else if (which == RS_filter) { + char setfiltering[40]; + any.a_int = RS_menu_arg(RS_filter); + Sprintf(setfiltering, "%s role/race/&c filtering", + gotrolefilter() ? "Reset" : "Set"); add_menu(where, NO_GLYPH, &any, '~', 0, ATR_NONE, - "Reset role/race/&c filtering", MENU_ITEMFLAGS_NONE); + setfiltering, MENU_ITEMFLAGS_NONE); } else if (which == ROLE_RANDOM) { any.a_int = ROLE_RANDOM; add_menu(where, NO_GLYPH, &any, '*', 0, ATR_NONE, "Random", diff --git a/win/tty/wintty.c b/win/tty/wintty.c index 0fbe47648..a6ad2f7a2 100644 --- a/win/tty/wintty.c +++ b/win/tty/wintty.c @@ -1,4 +1,4 @@ -/* NetHack 3.6 wintty.c $NHDT-Date: 1575245194 2019/12/02 00:06:34 $ $NHDT-Branch: NetHack-3.6 $:$NHDT-Revision: 1.227 $ */ +/* NetHack 3.6 wintty.c $NHDT-Date: 1578947635 2020/01/13 20:33:55 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.244 $ */ /* Copyright (c) David Cohrs, 1991 */ /* NetHack may be freely redistributed. See license for details. */ @@ -582,8 +582,7 @@ tty_player_selection() role_menu_extra(RS_RACE, win, FALSE); role_menu_extra(RS_GENDER, win, FALSE); role_menu_extra(RS_ALGNMNT, win, FALSE); - if (gotrolefilter()) - role_menu_extra(RS_filter, win, FALSE); + role_menu_extra(RS_filter, win, FALSE); role_menu_extra(ROLE_NONE, win, FALSE); /* quit */ Strcpy(pbuf, "Pick a role or profession"); end_menu(win, pbuf); @@ -681,8 +680,7 @@ tty_player_selection() role_menu_extra(RS_ROLE, win, FALSE); role_menu_extra(RS_GENDER, win, FALSE); role_menu_extra(RS_ALGNMNT, win, FALSE); - if (gotrolefilter()) - role_menu_extra(RS_filter, win, FALSE); + role_menu_extra(RS_filter, win, FALSE); role_menu_extra(ROLE_NONE, win, FALSE); /* quit */ Strcpy(pbuf, "Pick a race or species"); end_menu(win, pbuf); @@ -774,8 +772,7 @@ tty_player_selection() role_menu_extra(RS_ROLE, win, FALSE); role_menu_extra(RS_RACE, win, FALSE); role_menu_extra(RS_ALGNMNT, win, FALSE); - if (gotrolefilter()) - role_menu_extra(RS_filter, win, FALSE); + role_menu_extra(RS_filter, win, FALSE); role_menu_extra(ROLE_NONE, win, FALSE); /* quit */ Strcpy(pbuf, "Pick a gender or sex"); end_menu(win, pbuf); @@ -863,8 +860,7 @@ tty_player_selection() role_menu_extra(RS_ROLE, win, FALSE); role_menu_extra(RS_RACE, win, FALSE); role_menu_extra(RS_GENDER, win, FALSE); - if (gotrolefilter()) - role_menu_extra(RS_filter, win, FALSE); + role_menu_extra(RS_filter, win, FALSE); role_menu_extra(ROLE_NONE, win, FALSE); /* quit */ Strcpy(pbuf, "Pick an alignment or creed"); end_menu(win, pbuf); @@ -1032,6 +1028,7 @@ reset_role_filtering() winid win; anything any; int i, n; + char filterprompt[QBUFSZ]; menu_item *selected = 0; win = create_nhwindow(NHW_MENU); @@ -1058,10 +1055,12 @@ reset_role_filtering() "Unacceptable alignments", MENU_ITEMFLAGS_NONE); setup_algnmenu(win, FALSE, ROLE_NONE, ROLE_NONE, ROLE_NONE); - end_menu(win, "Pick all that apply"); + Sprintf(filterprompt, "Pick all that apply%s", + gotrolefilter() ? " and/or unpick any that no longer apply" : ""); + end_menu(win, filterprompt); n = select_menu(win, PICK_ANY, &selected); - if (n > 0) { + if (n >= 0) { /* n==0: clear current filters and don't set new ones */ clearrolefilter(); for (i = 0; i < n; i++) setrolefilter(selected[i].item.a_string); @@ -1093,7 +1092,11 @@ int race, gend, algn; /* all ROLE_NONE for !filtering case */ any = cg.zeroany; /* zero out all bits */ for (i = 0; roles[i].name.m; i++) { - role_ok = ok_role(i, race, gend, algn); + /* role can be constrained by any of race, gender, or alignment */ + role_ok = (ok_role(i, race, gend, algn) + && ok_race(i, race, gend, algn) + && ok_gend(i, race, gend, algn) + && ok_align(i, race, gend, algn)); if (filtering && !role_ok) continue; if (filtering) @@ -1137,7 +1140,10 @@ int role, gend, algn; any = cg.zeroany; for (i = 0; races[i].noun; i++) { - race_ok = ok_race(role, i, gend, algn); + /* no ok_gend(); race isn't constrained by gender */ + race_ok = (ok_race(role, i, gend, algn) + && ok_role(role, i, gend, algn) + && ok_align(role, i, gend, algn)); if (filtering && !race_ok) continue; if (filtering) @@ -1171,7 +1177,10 @@ int role, race, algn; any = cg.zeroany; for (i = 0; i < ROLE_GENDERS; i++) { - gend_ok = ok_gend(role, race, i, algn); + /* no ok_align(); gender isn't constrained by alignment */ + gend_ok = (ok_gend(role, race, i, algn) + && ok_role(role, race, i, algn) + && ok_race(role, race, i, algn)); if (filtering && !gend_ok) continue; if (filtering) @@ -1203,7 +1212,10 @@ int role, race, gend; any = cg.zeroany; for (i = 0; i < ROLE_ALIGNS; i++) { - algn_ok = ok_align(role, race, gend, i); + /* no ok_gend(); alignment isn't constrained by gender */ + algn_ok = (ok_align(role, race, gend, i) + && ok_role(role, race, gend, i) + && ok_race(role, race, gend, i)); if (filtering && !algn_ok) continue; if (filtering)