Cleaver's ability to hit up to three adjacent targets could kill a
long worm and then try to cut it in two. When this was first reported
I was unable to reproduce it, but this time I've managed to do so.
But not reliably, so it's hard to claim that it's now fixed. However,
the new report's explanation of why it happens and suggested fix was a
big help. I had been trying to hit three tail segments, but you need
to attack a segment next to the head, then have Cleaver hit and kill
the head first (50:50 chance depending upon whether current swing is
clockwise or counter). Worm cutting would be looking at the location
of the targetted segment but there won't be any monster there when the
head dies. (Cleaver's attack itself already copes the situation where
its 2nd and/or 3rd potential targets aren't there any more by the time
it's ready to try to hit them.)
The curses interface maintains message history in a doubly linked list
with a capacity limit. Once capacity is reached, the list head is
advanced and the old head discarded, but it was leaving the new head's
'previous element' link pointing at that discarded element.
tmp_mesg = first_mesg->next_mesg;
(at this stage, tmp_mesg->prev_mesg points at first_mesg),
free(first_mesg);
first_mesg = tmp_mesg;
(with necessary 'first_mesg->prev_msg = NULL' missing). The situation
wasn't a significant problem because traversing the list was limited
by a counter. Going from tail back to head exhausted the counter
without ever accessing the stale pointer.
Since it wasn't noticeable, I haven't added a fixes entry for this.
I've also changed it to do fewer memory allocations and frees by
reusing the old list head instead of always allocating a new element
and freeing the one being replaced.
The unresolved "first problem" mentioned earlier in commit
382286cb99 was caused by stale values
in status fields which had become disabled. Polymorphing left an
old BL_XP value and returning to original form left an old BL_HD one.
They weren't displayed but the stale value was included in the line
length calculation, resulting in 4 or 5 columns being set aside for
a phantom value. That implicitly reduced the available length of the
line and could result in extra spaces separating other fields being
squeezed out while unused spaces remained at the end of the line.
Experience points, time, and score didn't trigger this problem because
they were being explicitly excluded if disabled. So stale values for
them when they had been enabled then later disabled didn't matter.
I noticed a couple of things wrong--that I was fairly sure that I
had working correctly before--and after fixing the second one, the
first has mysteriously disappeared.
First problem, which may or may not still be a problem: extra spaces
were being removed from the second line of 2-line status even though
there were still 4 or 5 available spaces to the right of the status
conditions. It was behaving as if it thought the line was narrower
than actual size, or conversely, that the sum of the widths of the
fields plus the extra spaces was bigger than it actually was.
Second problem, fixed here. The code to put '+' in the far right
column of the last status line when there is at least one condition
all the way off the display wasn't working right when windowborders
were displayed. That's down to curses wrapping to the next line but
user can't see it due to the window border overwriting. Single char
overflow stayed on same line, but two or more wrapped and then the
'x' coordinate didn't match tests for 'too wide'. Perform explicit
truncation instead of leaving that up to curses. Also truncate
encumbrance when warranted since it's feasible for it to overflow.
Anyone using a display narrower than 80 columns might still run
into odd status behavior because other fields than conditions and
encumbrance could go past the end of line. But they shouldn't be
wasting screen real estate with windowborders, and without borders,
curses will keep the cursor in the bottom right corner when the
program tries to go past, which should keep things reasonably sane.
curses uses 'reversed' (LIFO) style when displaying previous messages.
Use the existing (previously tty-only) 'msg_window' option to also
support 'full' (FIFO). The actual code needed as just a couple of
lines; tweaking options parsing and the documentation was more work.
Using ^P right after resize or 'O' of align_message, align_status,
statuslines, or windowborders would result in
'curses_display_nhmenu: attempt to display empty menu'
because some memory cleanup I added several weeks back was being
executed when the curses interface tore down and recreated its
internal windows.
This fixes ^P handling by making sure that that menu (which is just
text but uses a menu to support '>'/'<'/'^'/'|' scrolling) will never
be empty and it also fixes the window deletion to not throw away
message history until it's final deletion at exit time.
^P uses a popup window to display previous messages and it was never
deleting that window, just creating a new one each time. Same with
the routine which displays an external help file. Using either or
combination of both close to 5000 times would probably make internal
window creation get stuck in an infinite loop. Delete those windows
after they're used so it'll never be put to the test.
The memory cleanup I added for map/status/messages/invent was only
being preformed at end of game, not when saving. Fix that too.
I've overhauled the status display for curses. Horizontal layout
supports both 2 lines and 3 lines which can be changed dynamically
via using 'O' to set 'statuslines'. Fields are spread out a little
more than they used to be, making it more readable--at least to me--
but the extra spaces get squeezed out when lines become too long.
If 'showexp' is on and either conditions or hunger+encumbrance go
off the right edge, experience points are suppressed (but the option
is left on, so they'll come back once there is room).
For traditional 2-line hozizontal status, if hunger+encumbrance+
conditions go off the right edge even after experience points are
knocked out, there will be a '+' in the rightmost column if there
are any conditions that are all the way off. At present it doesn't
use the tty method of switching to abbreviated condition names to
reduce their legnth. I'll probably tackle that eventually if no one
beats me to it.
For 3-line horizonal status, there was an older implementation (but
disabled via #if 0) with gold and score moving to the third line.
(I'm not sure how status conditions were handled.) This one ignored
that and modified 2-line from scratch, moving alignment from line one
to line 2 and level description, time, and conditions from line 2 to
line 3. It looks like this (view with a fixed-width font...).
Wizard the Hatamoto St:16 Dx:15 Co:18 In:8 Wi:11 Ch:7 S:25
Lawful $:21 HP:25(25) Pw:6(6) AC:4 Xp:2/21 Hungry Burdened
Dlvl:1 T:36 Blind Lev
Score is actually right aligned with the edge but I've deleted several
spaces to keep the line shorter here. The status conditions line up
with the hunger slot as that shifts due to changes in gold/HP/power/AC/
experience, and conditions prefer that column even when hunger and/or
encumbrance are blank. Howver, if the number of conditions increase to
the point where they would go off the edge, the whole list shifts left
instead of trying to stay lined up with hunger. (It's just coincidence
that the lefthand parts of lines 2 and 3 seem to line up in this sample.
In general, they don't.)
The vertical layout has reordered most of the fields and now has a few
blank lines to separate those fields into some groups for readability.
Lines have the form of
Field-name : Value
and when highlights apply, now they only affect the value portion.
Single digit characteristics are padded with a leading space so that
all six of them line up (for "18/xx", "/xx" protrudes to the right).
HP and Pw are aligned with each other. Hunger and encumbrance share a
line. When there are more than three conditions, they're shown three
per line instead of wrapping across lines. And if too many lines are
present, it will squeeze out enough blank ones to fit.
To see the vertical status, you need a display size of at least 106
columns with 'windowborders' explicitly off, or 110 with them on; also
set option 'align_status' to 'right' or 'left'. (With borders on,
including the default 'auto' setting, the vertical status appears at
width of 108 columns, but does so by hiding 2 columns of the map; using
110 columns avoids that.) Resizing from outside the game or changing
align_status via 'O' both cause dynamic reconfiguration of the layout;
there's no need to save, make config changes, then restore.
I went through the places that set context.botl/botlx in src/a*.c to
see whether I could find any unnecessary status updates. I didn't
find anything interesting, mostly some minor sequencing issues plus
a couple of redundant sets (call foo() which includes setting botl,
then explicitly set botl after it returns). If you issue pline first
and then set context.botl, bottom lines won't be updated until the
next message or next pass through moveloop. If you order those the
other way around, status will be updated as the message describing
the reason for the update is being delivered.
I've noticed (with curses interface) that there are a lot of
update_status(BL_FLUSH) calls when nothing on the status lines
has changed. It happens while just wondering around, not due to
deliberate botlx update. This eliminates one of the causes.
Updating hero's experience points or score (via u.urexp) was
requesting a bottom lines refresh even if the relevant options to
see those changed values was off. The botl code would send a flush
directive even though nothing visible was modified. (I have a fix
for that too, but am holding back while hoping to find some of the
other causes of unnecessary botl requests.)
It took me a while to track this down: if you use 'O' to create
hilite_status rule(s) for status condition(s) and you specify multiple
attributes in the rule creation menu, it accepts them but it was
parsing the new rule(s) incorrectly and only supported one attribute.
if (mask & bit)
something;
else if (mask & other_bit)
something_else;
else if (mask & yet_another_bit) ...
effectively stops at the first bit matched. (At the time that that
was written, the menu leading to it only accepted a single attribute.)
More groundwork for overhauling the status display for curses, plus
a few functional changes. It was doing a full status update for
every changed field (except conditions), instead of waiting for a
flush directive after gathering multiple changes at a time. Since
it already does gather every change, the fix to wait is trivial.
This decouples 'hitpointbar' from 'statushilites'. When highlighting
is off, it uses inverse video only. When on, it behaves as before:
using inverse video plus the most recent color used to highlight HP
(which can vary if that has rules to highlight changes or percentage
thresholds) but ignoring any HP attribute(s). This also enables the
latent 'statuslines' option and changes 'windowborders' option from
being settable at startup only to changeable during play.
'statuslines' can have a value of 2 (the default) or 3 and applies to
'align_status:bottom' or 'top'; it's ignored for 'left' and 'right'.
At the moment, setting it to 3 only allows status condition overflow
to wrap from the end of line to 2 to the beginning of line 3, and if
window borders are drawn they'll clobber the last character on line 2
and first one on line 3. There's no point in trying to fix that
because it will go away when the main status overhaul changes go in.
Condition wrapping for vertical orientation (left or right placement)
was already subject to the same phenomenon and will be superseded too.
This also changes the meaning of the 'windowborders' value so could
impact players using source from git (or possibly beta binaries for
Windows, but not for OSX where curses interface wasn't included).
Old:
0 = unspecified, 1 = On, 2 = Off, 3 = Auto (On if display is big
enough, Off otherwise; reevaluated after dynamic resizing);
Unspecified got changed to 3 during curses windowing initialization.
New:
0 = Off, 1 = On, 2 = Auto;
0 gets changed to 2 for default value at start of options processing.
So old value of 2 is changing meaning and explicit old value of 3 is
becoming invalid. Implicit 3 changes to default 2. Explicit 3 could
be the subject of a fixup but there isn't much point since 2 can't
have a similar fix. Users who are using old 2 or explicit 3 will need
to update their run-time config files.
This adds 'statuslines' to the Guidebook and moves some other recently
added documentation of curses options from among the general options
(section 9.4) to "Window Port Customization options" (section 9.5).
None of them have been added to dat/opthelp which seems to be missing
all the wincap options.
Originally I made a lot of changes (mostly moving C99 declarations to
start of their blocks) to the old '#if 0' code at end of cursstat.c,
but have tossed those, except for one subtle bug that assumed 'int'
and 'long' are the same size.
Miscellaenous stuff either groundwork for or noticed while updating
curses status. The status changes themselves need some more testing.
One or two of the comments refer to that revised status which hasn't
been checked in yet.
decode_mixed() writes its output into a 'char *' buffer that's passed
in to it and then returns that buffer. Some excessively paranoid
error checking forced to return 'const char *'. Relax that and use
'char *' so callers can work with the result without extra casts.
Change the generic status line location "End Game" to relevant element
name "Earth", "Air", &c. ("Plane of <element>" might be too long if
hungry and encumbered and afflicted by conditions. "Astral Plane" is
already specific so not affected.)
Honor hilite_status rules specifying color even if curses-specific
option 'guicolor' is off.
Update status from scratch when 'O' is used to manipulate hilite_status
rules.
Fix:
../sys/winnt/nhraykey.c: In function 'CheckInput':
../sys/winnt/nhraykey.c:459:37: warning: type of 'mode' defaults to 'int' [-Wimplicit-int]
int __declspec(dllexport) __stdcall CheckInput(hConIn, ir, count, numpad,
^~~~~~~~~~
The curses interface was ignoring video attributes (bold, inverse, &c)
when color is toggled off or if built with TEXTCOLOR disabled. Honor
attributes regardless of whether color is displayed.
Also, toggling 'hilite_pet' On during play wouldn't do anything if the
curses-specific 'petattr' option had been left as None. (It worked as
intended if set in starting options.)
Change the test for whether fonts.dir exists (added to the script
in 3.6.0, for automatically setting up possible use of the NH10 font
under X11) from 'test -e file' to 'test -f file' since the latter
seems to be more universally available. When present, fonts.dir is
plain text, so a test for "exists and is a regular file" rather than
one for general existance is appropriate.
If nethack is built to use graphical tombstone but file rip.xpm is
missing from the playground, there would be a crash if the rip output
was shown. My first attempt to fix it prevented the crash but didn't
display any tombstone, just the last couple of lines of output which
follow the tombstone. This keeps that in case of some other Xpm
failure, but checks for rip.xpm via stdio and reverts to genl_outrip
for text tombstone if it can't be opened.
Hide a bug in the X11 interface by manually wrapping 'short' version
line (the first line of #version output) when runtime ID or git hash
and/or branch is present. Otherwise the very wide 'short' line causes
a horizontal scrollbar to be present at the bottom of the popup window,
but the window size doesn't take that into account so ends up being
one line too short. Worse, there's no vertical scrollbar to bring the
last line into view.
Even if somebody figures out how to fix that properly, the really wide
line above a couple of paragraphs which have been carefully wrapped
for a much narrower width looked strange.
Also, move the 'short' version's period to the end of the text.
old: basic version info. (extra info)
new: basic version info (extra info).
Twice I've gone through the curses code to deal with CHAR_P, BOOLEAN_P,
and so forth. Both times I eventually changed my mind. This time I'm
just adding an explanatory comment instead.
Extend the earlier support for Delete/Rubout in getline() to the
text entry for extended commands. In other words, treat <delete>
and <backspace> as synonyms in both places.
Some reformatting too, but only in a couple of the files.
When setting up an alternate definition for UCHAR_P to accommodate
DEC C's non-ANSI mode(s), leave the default definition to tradstdc.h.
DEC C's VAXC mode will end up expecting 'unsigned char' rather than
either 'int' or 'unsigned int'. Not applicable for Unix but this
makes similar change there as is being made for VMS.
Suppress Isaac64 on VAX were there isn't an easy way to do 64-bit
arithmetic. (Hard way isn't worth it for just an alternate RNG.)
Eliminate or suppress some diagnostics:
1) In strict ANSI mode, DEC C was reporting that '$' in identifier is
an extension (one time for each file in sys/vms/*.c). (It doesn't do
that for the default 'relaxed ANSI' mode.)
2) DEC C uses WIDENED_PROTOTYPES but widens uchar (unsigned char)
differently depending upon the mode it is operating in. (Applies to
Unix as well as VMS; based on documentation rather than testing.)
Update the comment in tradstdc.h about WIDENED vs UNWIDENED_PROTOTYPES.
An old comment in config1.h about a problem with the earliest version
of DEC C was probably based on an incorrect assumption of what was
really going, but I have no way to go back in time to verify that....
Kicking a stack splits off one item (except for gold coins) and
propels it, but the range for how far it would move was calculated
before the split using the entire stack's weight. So a large stack of
small items might fail with "thump" (which the report suggested hurt
the hero, but it doesn't) and none of the stack would move. Splitting
sooner looked complicated because of several potential early returns
between the range calculation and the eventual kick, so this hacks the
stack's quantity to get the intended weight instead.
The VMS compiler supports C99 and defines __STDC_VERSION__ to the
corresponding value, but it doesn't supply <stdint.h> so isaac64.c
wouldn't compile. It does supply another header containing what is
needed.
showed non-empty containers in inventory (including the one being
applied) with a 'for sale' suffix during put-in operations, as if the
shop was trying to sell it to the hero. Amount shown was cumulative
value of its contents. (Using /menustyle:T doesn't show the container
being applied so this wasn't visible with it unless other non-empty
containers were being carried.)
Two or three fix attempts solved one problem but introduced another.
This one seems to finally get things right but considering that there
was trial and error along the way, my confidence isn't great.
If steed ate a mimic corpse and started mimicking an object or dungeon
furniture, the hero was able to keep riding. Force a dismount when
that happens, even if steed takes on monster shape rather than object
or furniture. After that, #ride to remount non-monster will fail
unless using wizard mode's "force mount to succeed" action, in which
case steed's eating finishes immediately and it returns to normal.
This doesn't address the older report that mounted hero can continue
to move around while the steed is eating.
number_pad got removed from Guidebook.tex when mouse_support was added
by cc0e7a8750fd4e07d3a44592c38f5912d4e603de; put it back. Evidently
a cut-and-paste mishap when copy-and-paste was intended, probably
provoked by '\numbox{}'.
Remove \numbox{} from mouse_support; it isn't needed there since the
choices aren't trying to line up with '-1'.
Both Guidebook.mn and Guidebook.tex:
Remove number_pad's "for backward compatibility" from mouse_support
since the latter is brand new.
New petattr list of uppercase letters seemed intrusive when rendered
with a bold font, so switch to lowercase. (Option processing already
supports either case.)
Mention the '=' prefix for hilite_status 'absolute' threshold.
[As usual, the Guidebook.tex changes haven't been tested.]
Report forwarded from spam filter so not included in bugzilla list.
Make corpse revival feedback be more consistent. Some of the healer-
specific flavor is still there.
Generally, fish should lay their eggs in the water and
not on land, but the game was only allowing the opposite.
Eels are catadromous and lay their eggs in the Sargasso Sea,
not in the dungeon.
This started out as an attempt to document the curses options in the
Guidebook, but I didn't actually get that far. Instead, integrate
the curses options better via more consistent WC/WC2 usage. This
prevents 'guicolor' from showing up as a boolean option for non-curses
interface in curses+other binary.
For curses itself, let 'petattr' be set/reset via 'O'. Also, accept
'Dim' as a possible pet highlight attribute since it already handles
all the other ordinary attributes. I'm not sure what leftline and
rightline highlighting are supposed to do. They were missing for
ncurses (or maybe they're misspelled for PDcurses?) but adding them
didn't produce any visible effect (using TERM=xterm-256color on OSX
with default font/character set).
Not addressed:
1) general confusion about compile-time vs run-time option filtering;
2) curses pet highlighting only works if 'color' option is enabled.
Something else noticed while testing #H8271: toggling perm_invent on
with 'O' didn't show anything (at least with curses) until some later
action caused it to be updated. Make updating persistent inventory be
included with full redraw and set the need_redraw flag when toggling
perm_invent.
Realized while fixing #H8271: if persistent inventory got an update
while wearing or taking off was in progress (not within user's control
since hero is busy) the item in question was flagged as "(being worn)"
even though it wouldn't be worn if putting on got interrupted. Update
doname() to show "(being donned)" or "(being doffed)" instead of
"(being worn)" when corresponding operation is in progress. (During
testing, I was able to observe "being doffed" but never managed to see
"being donned".)
A relatively recent change moved 'obj->known = 1' when wearing armor
from before setworn(), which issues an update_inventory() call, to
afterwards. There wasn't any particular update then, so observing
the enchantment of armor by wearing it wasn't being reflected in the
persistent inventory window if that was enabled.
