Add support for filtering by unpaid status for container-in and
container-out actions. When taking out of a container, it works as
expected if you're carrying the container while in a shop, but won't
find any unpaid items if the container is on the floor. That's
because they're only flagged as unpaid while in the hero's inventory.
(And when it doesn't find any unpaid items it won't list 'unpaid' as
a category of item to manipulate, so while that might be suboptimal
for taking items out of shop containers, it shouldn't be a problem.
Typically all the contents are shop-owned anyway, so using unpaid as
a filter wouldn't gain any advantage over just taking stuff out.)
The different menustyle settings have been offering different degrees
of support for BUCX filtering:
Full : multi-drop, container-in, container-out
Trad, Combo: multi-drop
Partial : none (to be expected; it explicitly jumps past class
filtering, where BUCX gets handled, to a menu of all objects)
This adds pickup, container-in, container-out, multi-unwear/unwield,
and object-ID for Trad and Combo, and multi-unwear/unwield for Full.
(Full behaves like Partial for pickup--not sure why--and for object-ID,
bypassing filters to go straight to a menu of all applicable items.)
There are probably several new bugs--this stuff is very convoluted....
Report #5426 was classified as not-a-bug, but the underlying issue
can be improved.
For item selection where BUCX (bless/curse state) filtering is
supported (mostly for menustyle:Full, but there are a few actions
where Traditional and Combination handle BUCX too), 3.4.3 took the
union of object class and bless/curse state (so ?!B gave all scrolls
and all potions and every blessed item from other classes) but 3.6.0
changed that to the intersection (so ?!B gives blessed scrolls and
blessed potions, period). Since gold is inherently not blessed or
cursed it has been getting excluded during intersection handling
when that includes BUCX filtering. Report #5426 was from a player
who was used to choosing $X when putting newly acquired loot into a
container asking to have the old behavior reinstated.
The ideal fix would be to support both union ($ | X) and intersection
(?! & B), but implementation would be bug prone and the interface,
especially when done for menus, would be cumbersome. Instead, this
adds new boolean option, goldX, to allow the player to decide whether
gold is classified as uncursed--even though it is never described as
such--or unknown. The new-loot-into-container issued can be solved
either via $abcX, where abc lists all classes that have any X items
(when gold is included as one of the classes, its BUCX state is now
ignored for the current selection), or by setting the goldX option
and then just picking X for the types of items to put into the
container (or drop or whatever other action supports BUCX filtering).
The situations where menustyle:Full allows BUCX filtering during
object class specification and styles Traditional and Combination
don't should to be fixed (by extending BUCX support to Traditional
and Combination rather than removing it from Full, obviously).
The tie-breaker for the qsort comparison routine used for 'sortloot'
had its logic backwards. Instead of retaining the original relative
order for tied items, it inadvertently reversed them. So a chest
containing
club
dagger named A
dagger named B
dagger named C
long sword
would alternate between that order and
club
dagger named C
dagger named B
dagger named A
long sword
each time the chest's contents were examined. This fixes that, and
also simplifies the unnecessarily convoluted bless/curse state handling.
The other half of the report was a request that 'sortloot:n' not do
any sorting. Right now, if the player has 'sortpack' set then
'sortloot:n' results in grouping into object classes within pack order
rather than not sorting at all. (Also, armor and weapons are further
ordered within their groups: armor by slot [helms, gloves, shields, &c]
and weapons by function [ammo, launchers, missiles, 'ordinary', pole-
arms].) I think the proper fix is to add a new setting for 'sortpack'
which yields the current behavior before changing 'n' to leave things
in their unsorted order.
Update DUMPLOG's message history to include player responses to
most queries. For tty, both getlin() and yn_function(). For other
interfaces, only yn_function() is covered. (It's intercepted by a
core routine that can take care of the logging; getlin() isn't.)
Also includes saved messages from previous session(s), for the
interfaces which support that (tty), to fill out the logging when
a game ends shortly after a save/restore cycle.
The tty interface was using pline() to display prompt strings.
Having 'MSGTYPE=hide "#"' or 'MSGTYPE=hide "yn"' in .nethackrc
would suppress many prompt strings (in the two examples mentioned,
entering extended commands or the vast majority of yes/no questions,
respectively) and generally lead to substantial confusion even if
done intentionally, so switch to putstr(WIN_MESSAGE) instead.
Bug report was:
> "Completed sokoban" achievement was logged when picking up
> a randomly generated bag of holding in the gnomish mines.
The picking-up code was missing checks for the branches, so
you could get the achievements outside the correct branches.
With menustyle set to "full" or "partial", using 'D' when not
carrying anything gave no feedback. (Modes "traditional" and
"combination" give "you have nothing to drop" via ggetobj().)
Also, there's no need to reset in-progress armor removal, lock
picking, or trap setting if you don't actually drop anything.
The inventory they're set to operate on or with stays intact.
"Your pair of speed boots glow silver for a moment." should be
"Your pair of speed boots glows silver for a moment.". The fix
reverses a post-3.6.0 change to is_plural(). Also, add new
pair_of() to test for object formatted as "pair of Bars". For verb
usage, that's definitely singular, but for pronoun usage, sometimes
plural seems better (although it might actually be incorrect).
I fixed up the formatting of a block comment in obj.h, but it is
still a candidate for tab cleanup.
This ended up being more elaborate than I anticipated. 'Q' will
now accept the wielded weapon as a choice of item to quiver. If
that item is a stack of more than one, it will offer to split N-1
into the quiver and leave 1 wielded. If the offer is declined, or
if there is already just 1, it will require confirmation to move the
item from the weapon slot to the quiver slot. The alternate weapon
is handled similarly, with different phrasing when in twoweapon
combat mode.
Just to be crystal clear: a single object cannot be in more than
one weapon, alt-weapon, or quiver slot at the same time. 'Q's old
behavior of rejecting the wielded weapon was to avoid accidentally
becoming empty-handed, not anything to do with multiple worn/wield
slots.
'Q' will also accept a count when picking an inventory item, then
put 'count'-many into the quiver, leaving N-count in original stack.
Except when the chosen item is already in the quiver; it that case,
it undoes the stack split and leaves things as they were. (That
restriction may not have been necessary but I'm not planning to
revisit it....)
Change sort ordering of
diluted potion of bar
diluted potion of foo
potion of bar
potion of foo
potion of fruit juice
to
potion of bar
diluted potion of bar
potion of foo
diluted potion of foo
potion of fruit juice
so that potions of the same type are grouped together. Bless/curse
state (when known) takes precedence over dilution, so "blessed
diluted potion of foo" will come out before "uncursed potion foo".
When items were sorted alphabetically, holy water ended up in the H's
and unholy water in the U's. Force them to get placed with water in
the W's, as would happen if water wasn't given an alternate name when
blessed or cursed.
getobj() was caching 'invent' in 'firstobj', dating from the days
of the !GOLDOBJ configuration, and sortloot() could change the value
of invent, making firstobj end up pointing somewhere into the midst
of the inventory list. So collecting letters of applicable items
could miss things (typically right after restoring a saved game).
Repeating the command would operate on already sorted invent, making
firstobj remain valid and things mysteriously reappear after having
been missed before.
Just get rid of 'firstobj' since it's no longer useful.
Force the menu for the look-here command when 'here' is the inside
of an engulfer to be PICK_NONE. That way '>' won't exit the menu
by choosing the extra inventory item "> - hero".
For menustyle:Traditional, the object class prompt for 'D' includes
an entry choice of 'm' to request a menu. Supplying real classes
and 'm' resulted in a menu limited to those classes, as intended,
but any of BUCX for curse/bless state and 'm' without any actual
classes resulted in a menu of entire invent. A one-line fix once
the proper place for that fix was located.
For menustyle:Traditional or menustyle:Combination, 'A' includes an
extra choice of 'i' to examine relevant inventory prior to choosing
object classes (and object identification also offers that), but
the inventory display showed everything rather than just the items
applicable to 'A' (or to object ID). Figuring out where to apply
the fix was trivial, but the fix itself was a bit more involved and
it exposed a latent bug in display_pickinv(): "" was supposed to be
the same as NULL when passed in as list of target inventory letters,
but it wasn't being handled correctly.
A patch in late January to suppress suits as likely candidates for
the 'R' command when wearing a cloak also unintentionally suppressed
rings when wearing non-cursed (or not yet known to be cursed) gloves.
Cloak always blocks suit removal; gloves only block ring removal if
cursed.
The "sortloot revamp" patch six or seven weeks ago broke filtering
for '?' menu in display_pickinv() called by getobj(). The old code
handled 'lets' when building an array of object pointers to be
sorted. The revamp code did away with that to sort the linked list
instead, but neglected to put 'lets' handling into the subsequent
menu creation loop which is now operating on full invent rather
than the filtered subset.
After some permutation of commands which displayed items, the 'd'
command presented a prompt with the list of letters scrambled (in
loot order or pack order rather than invlet order), so explicitly
sort when getobj operates. Done for ggetobj too.
For menustyle:Traditional, ',' followed by 'm' presented a pickup
list in pile order even when sortloot was 'l' or 'f'. That was an
unintentional change during the 'revamp'.
Fix some typos in the sort-by-invlet code and a logic error in the
lately added subclass sorting for sort-by-pack. Regular inventory
display only works correctly for the latter if invlet is the tie-
breaker within object classes. When helmet/gloves/boots/&c and
ammo/launcher/missile/&c sub-categories already break ties for armor
and weapon classes, inventory ended up out of alphabetical order.
Change the sortloot option to use qsort() instead of naive insertion
sort. After sorting, it reorders the linked list into the sorted
order, so might have some subtle change(s) in behavior since that
wasn't done before.
pickup.c includes some formatting cleanup.
modified:
include/extern.h, hack.h, obj.h
src/do.c, do_wear.c, end.c, invent.c, pickup.c
Report states that using OSX Xcode IDE results in use of 'clang
Modules', whatever those are, and role.c's 'filter' struct ends up
conflicting with a function declared by <curses.h> (or possibly
<ncurses.h> since one includes the other). src/role.c does not
include <curses.h>, so this smacks of the problems caused by using
precompiled headers on pre-OSX Mac.
Instead of trying to import nethack into Xcode, I temporarily
inserted '#include <curses.h>' at the end of unixconf.h. gcc did
complain about 'filter' in role.c (but not in invent.c, despite
-Wshadow), and then complained about termcap.c using TRUE when it
wasn't defined (after in had been #undef'd, where there's a comment
stating that it won't be used in the rest of that file), and also
complained about static function winch() in wintty.c conflicting
with external winch() in curses.
This renames 'filter' and 'winch()' to things that won't conflict.
Also, our winch() is a signal handler but had the wrong signature
for one. And the troublesome use of TRUE was in code that was
supposed to be dealing with int rather than boolean.
Requested during beta testing last year, include a menu entry of
"- - your bare hands" (or "your gloved hands") for wielding,
"- - empty quiver" for readying quiver,
"- - your fingertip" for engraving, or
"- - your fingers" for applying grease
if the user responds with '?' or '*' at the
"What do you want to {wield|ready|write with|grease}? [- abc or ?*]"
getobj prompt. (First dash is inventory selector 'letter', second
dash is menu separator between the letter and its choice description.)
Globs on the floor used different criteria (anything goes) than globs
in inventory (mostly requiring same ownership when in shops and same
curse/bless state--other stuff generally isn't applicable) when
deciding whether two globs should merge. That was okay as long as
the globs on the floor were from being left behind when a pudding or
ooze was killed, but not if the player had picked some up, dipped
them in holy or unholy water, and dropped them again. This changes
things so that globs on the floor use the same criteria as globs in
inventory when deciding whether to coallesce.
Also, my earlier fix was modifying globs in the mergeable() test (to
make bknown and rknown match) rather than during actual merge, which
would be a problem if the merger didn't take place for some reason.
Bug report subject was "menu interface things" but this isn't related
to menus, just getobj(). Make the requested change to not list worn
suit as a likely candidate for T (and R) if a worn cloak is going to
prevent it from being removed. (Suit can still be picked, but the
take-off operation for it will be refused, same as when it was being
listed as a candidate.)
In discussion about the request, there was a separate suggestion that
cloaks shouldn't interfere with removing things underneath since
they're generally sleeveless. I haven't done that; I think it is
better to keep the layering as it is.
The bug report also asked for the likely candidates when writing with
a magic marker to exclude non-blank scrolls and books. That has been
implemented already (post-3.6.0).
This is more robust than the previous hack. The issue of whether to
use it in other places is still unexplored. Ultimately it's the user's
fault if overzealous message suppression hides something important.
[For an eerie game, try 'MSGTYPE=hide .'.]
User had
MSGTTYPE=norep "You see here"
and complained that once the message had been given while walking
over an object, using ':' to intentionally look at something would
end up doing nothing if its feedback was a repeat of "You see here".
Trying to classify which actions should deliberately override
no-repeat (or no-show) will be an ordeal. This fixes the case for
the ':' command where the user obviously expects feedback. I think
it could be done better but am settling for something quick and easy.
Simplify some conditionally excluded, obsolete but not yet
discarded code.
This is something else I thought I'd checked in a long time ago.
I must have clobbered pending changes to invent.c at some point.
When applying a magic marker, only list known blank scrolls and known
blank spellbooks as likely candidates to write on. Accepts any scroll
or book (but non-blank ones will get rejected by the writing code).
Attempting to choose some other class of item yields "that is a silly
thing to write on", same as before.
This was requested during beta testing and I'd swear that I checked it
in a long time ago, but it wasn't here.
I think there was also a report about this during beta testing.
Killing an ooze, slime, or pudding left a glob of same which had its
bknown flag pre-set so was immediately shown as "uncursed" even to
non-priests. Use another way to maximize glob mergability: allow
globs to merge even when one has bknown set and the other doesn't.
exit_nhwindows() is called before terminate(), and the tty incarnation
destroys all windows--including 'pickinv_cache_win'--without setting
the various index variables used to access them to WIN_ERR, then
terminate() calls freedynamicdata() which calls free_pickinv_cache()
which tries to destroy 'pickinv_cache_win' since it isn't WIN_ERR (if
the perm_invent option has been enabled during that playing session).
Some of the other <interface>_exit_nhwindows() also tear things down
without resetting the variables used to track them, so fixing this in
exit_nhwindows() would have been pretty messy.
Call free_pickinv_cache() before exit_nhwindows() in done(). At the
moment it's only called from done(), so other exit paths won't release
the small chunk(s) of memory used for the alternate inventory window
(if it got created for perm_invent support).
This should address the issue that the problem patch to display_pickinv()
was trying to deal with: releasing the inventory window before exiting
the program so Pasi's memory checker doesn't think it's a memory leak.
Not related, but in the same file:
The older qsort comparison routines are tagged with CFDECLSPEC to deal
with some C vs C++ interaction issue. I added that to the relatively
recently added 'sortloot' qsort compare callback.
I also changed worn_wield_only(), although it isn't actually called.
(display_minventory() has provisions to call it, but both of the latter's
callers pass in MINV_ALL so allow_all() gets used instead.)
Relatively small number of continuation fixes needed for this subset.
Quite a bit of mangling to engrave.c unrelated to continuation lines,
with three or four coding changes.
Replace the code that Dean objected to with something a little bit more
robust. It doesn't rely on the two stacks being adjacent or having the
same inventory letter. It is still vulnerable to having another
splitobj() occur between the offending split and its attempted unsplit,
or to either of the two halves of a split being extracted from their
object chain. As before, failure to unsplit only results in the two
halves of the split remaining separate stacks, not anything more drastic
like the panic() that prompted all this.
Simplification of hallucinated currency names got mixed in with this
patch. I haven't bothered separating it back out.
Whoever reset PATCHLEVEL to 0 jumped the gun. This patch increments it
since change to the 'context' structure breaks save file compatibility,
so it will need to undergo another reset before release.
Allow 'P' and 'R' commands to accept armor and wear/take-off the chosen
item, and 'W' and 'T' commands to accept accessories and put-on/remove
the item. The which-object prompt only lists the type(s) of items that
traditionally go with each command, as does an inventory menu if the
user picks '?', but items of the alternate type(s) can be chosen, by
unshown letter or by the inventory menu given for '*'.
There shouldn't be much difference if you continue picking items that
go with the original commands, although you will somestimes get
"which object? [*]" when the only choices are for alternate command.
And you won't see the all-four-accessories-are-already-worn message
for 'P' unless you also have something worn in all seven armor slots.
The Guidebook.mn changes have been tested (that's how/why I noticed
the preface glitch) but the corresponding Guidebook.tex ones haven't.
Add macros W_WEAPON and W_ACCESSORY, similar to existing W_ARMOR, bitmask
of all the relevant worn bits. Just for code readability; there should
be no change in behavior.
Also, reformat the "ugly checks" portion of getobj(). Slightly better
readability and fewer continuation lines, but only a modest improvement.
Replace instances of strings split across lines which rely on C89/C90
implicit concatenation of string literals to splice them together
with single strings that are outdented relative to the code that uses
them. It's uglier but it won't break compile for pre-ANSI compilers.
This covers many files in src/ that only have one or two such split
strings. There are several more files which have three or more. Those
will eventually be '(2 of 2)'.
Noticed along the way: the fake mail message/subject
Report bugs to devteam@nethack.org.
wasn't using its format string of "Report bugs to %s.", so would have
just shown our email address. Doesn't anybody enable fake mail anymore?
I modified that format to enclose the address within angle brackets and
made a similar change for the 'contact' choice of the '?' command.
Changes to be committed:
modified: include/context.h
modified: include/extern.h
modified: src/files.c
modified: src/invent.c
modified: src/sounds.c
modified: src/spell.c
Add a couple more tribute easter eggs.
- can lead to a remark by Death if you happen to have a pratchett book on
your person, as suggested by M. Stephenson (fat chance you will, or
think to #chat if you do, but it could be a tournament novelty or something
obscure to strive for).
- can draw some additional Death quotes from the tribute file. (There's two
in there right now. If anyone wants to add or suggest some more, please go
ahead. The Death quotes are at the end of the tribute file. One-liners
only please or the code will only pull the last line.
