rnd_otyp_by_namedesc() had an off by one error when choosing which
matching item to return, making it impossible to successfully wish
for the Amulet of Yendor, always yielding the plastic imitation.
n == 2, maxprob == 2
prob = rn2(maxprob); => 0 or 1
i = 0; while (i < n - 1 && (prob -= ... + 1) > 0) i++;
always exited the loop on the first test of the condition because
subtracting 1 from 0 or 1 never yielded a result greater than 0.
It's still suboptimal: "amulet of yendor" should find an exact match
and should never return "cheap plastic imitation of amulet of yendor"
from the partial match.
I think biasing the choice among multiple candidates based on their
generation probabilities only makes sense when all the candidates are
within the same class. If scroll of light occurred in 5% of scrolls
and spellbook of light occurred in 10% of spellbooks (both percentages
pulled out of thin air), having "light" get a 2 out 3 chance to be a
spellbook doesn't seem right because scrolls are four times more
likely than spellbooks (in most of the dungeon; books aren't randomly
generated at all on the rogue level or in Gehennom).
...by thrown potion. The reported case was a shopkeeper killed by
system shock from thrown potion of polymorph, but any death (acid,
burning oil explosion, water against iron golem, holy water against
undead, demon, or werecritter) to any peaceful monster could cause
similar result.
When confused gold detection finds a door trap or a chest trap, it
puts a bear trap glyph/tile on the map at that location. (They
disappear once they're within sight.) Those should be given their
own glyphs so that they can have their own tiles, but this doesn't
do that. What it does do is describe such fake bear traps as
"trapped door" or "trapped chest" when examined with far-look.
The '^' command--if used while blind so that '^' hasn't disappeared
yet--needs to catch up: it says "I can't see a trap there" when the
adjacent '^' is a fake bear trap.
While testing something I noticed that moving the cursor to visible '^'
by typing '^' while getpos was asking me to pick a location, it didn't
always cycle through all visible traps. The most straightforward
culprit was after trap detection (via confused gold detection, not ^F)
had found a trap door or level teleporter in a closet that itself was
a secret corridor spot. But it turned out to be any location that
hadn't been seen yet. This is a substantial overhaul of the relevant
code and so far works for all the cases I've tried, but there are
bound to be cases I haven't tried yet and those may or may not work
correctly.
There's also a bunch of formatting cleanup, and some simplification of
the m/M/o/O/d/D/x/X handling.
When travel fails to reach its destination, it remembers the target
spot to use as default next time. But that spot is only meaningful
on the current level. Discard last travel destination when moving
to a different level.
Also, discard unlocking context when changing level unless the
context is for a container being brought along (after having been
picked up since you can't unlock a carried box). Previously, a
door pointer on the new level could happen to match the last one
being unlocked on the old level.
Discard trap setting context when changing level even if the trap
object is brought along.
Somehow the code for applying a touchstone got inserted in between
two sections of code for applying a trap (ages ago; probably since
touchstone was first introduced however many versions back), so
clean that up.
Add MG_BW_LAVA to mapglyph() instead of hijacking MG_DETECT. Used
to display lava in inverse video if color is disabled and lava is
using the same display character as water (which is the default).
(The use_inverse option must be enabled for tty to honor it. X11's
text mode doesn't care. Win32 does care but probably shouldn't--it's
not a case like tty where the hardware might not support it.)
This implements both MG_DETECT and MG_BW_LAVA for X11, but only if
the program is built with TEXTCOLOR enabled. Those should work even
when color is not supported, but I suspect that configuration is
unlikely to ever be used so didn't want to spend the time to figure
out how to do it. (The relevant data is overloaded on the color
data, so not available when TEXTCOLOR is disabled.)
The win32 revision is untested.
Reported almost exactly one year ago by a beta tester proofreading
the Guidebook, the number_pad setting to support the German keyboard
which swaps the Y and Z keys is for a keyboard that is used in other
places too. The report mentioned France and Belgium; Wikipedia's
"keyboard layout" entry mentions "Germany, Austria, Switzerland and
other parts of Central Europe". This changes references to "German
keyboard" (there were only a couple) into "QWERTZ keyboard".
While looking for things in core which might conceivably trigger the
Windows ctype assertion failure (haven't found any yet), I noticed
that help_dir() was still treating ^O as a wizard mode-only command.
Also, documentation about that command was never brought up to date.
I wish this change to ^O hadn't been done. #overview already has
a meta/alt M-O shortcut and overloading wizard mode commands makes
documentation more complicated since wizard mode stuff traditionally
has been left unmentioned.
If the recently added release routine ever gets called twice for
some reason, don't free already freed memory, or worse, was freed
here and then allocated for something else which is still in use.
When a stack of N corpses is hit by wand or spell of undead turning,
1 revives and N-1 remain corpses. If owned by a shop, a fee for
using up all N corpses was charged and if carried at the time, the
extra N-1 became owned by the player but if on the floor, they
remained owned by the shop. Feedback was schitzophrenic as to
whether the whole stack was involved:
One of the <foo> corpses glows irridescently.
You owe <shk> X zormids for them.
Split the stack so that revival explicitly operates on only 1 corpse.
It's done after the revival side of things has already succeeded or
given up, so the split will never need to be undone.
Zapping wand of undead turning at self while inside a shop and
carrying a corpse caused the shopkeeper to claim a use-up fee for
the corpse regardless of whether it was owned by the shop.
Not mentioned in the report: casting stone-to-flesh as self while
carrying a figurine or statue behaved similarly.
A few things which might conceivably pass negative values to ctype
routines. Some are post-3.6.0. None of them explain the sporadic
Windows assertion failure.
Using tolower() without verifying the argument isupper() should be
completely safe when tolower() is a function but might not be when
it's a macro. (Likewise for toupper() without islower().) NetHack's
lowc() function is always safe, at least for ASCII.
I've hunted for other instances where monster hit points were set
to zero or less without calling the routine that kills off the
monster (see recent mon_unslime() vs zhitm()) and didn't find any
for mhp subtraction. I haven't checked for direct assignment yet.
For a while I thought I'd found several cases where a monster was
intended to be killed but got left with positive hit points, but
it turned out that lifesaved_monster(), of all places, was setting
them to zero. I've moved that to its callers so that it isn't so
well hidden. And changed several ''if ((mon->mhp -= dmg) <= 0)''
into separate subtraction and 'if' just so the mhp manipulation is
a bit more visible.
I think the only actual change here is the message for monster
being killed by lava, where glass golems now melt instead of burn.
Change description of area outside of the swallow animation from
"interior of a monster" to "unreconnoitered". For the animation
characters themselves, don't suppress the list of other screen
features which use the same character ('/' for wand and so on).
Add a data.base entry for "unreconnoitered" in case someone tries
to use it to look up an unfamiliar term.
They aren't walls.
Noticed while working on a potential change for far-look while
swallowed, zap beams have a bogus description of "wall" that
showed up when looking at '/' and '\\'. I'm guessing that vbeam
and hbeam were cloned from actual wall symbols and then lslant
and rslant got cloned from them. There doesn't seem to be any
other reason for the description, which is both wrong and applies
to symbols which aren't on the screen at a time when the player
can use far-look to examine them. It's been this way since the
creation of the second cvs repository used to start the current
git one; I didn't attempt to go back any farther than that.
When --More-- was written to leftmost column of line 2 while the
hero was swallowed, after player acknowledged it and the top line
was cleared, the cursor ended up in the wrong place. I still
don't understand what in the world is going on here, but adding
'flush_screen(0)' after 'swallowed(1)' in docorner() makes the
problem go away. Why is the behavior different when --More-- is
in the first column than when it's anywhere else?
After that fix, I commented the whole thing out. The swallowed
optimization is just not significant enough to justify peeking at
core internals.
Core bit: prior to those two changes, I tried inserting 'bot()'
into swallowed(). It moved the mis-positioned cursor from the
end of the second status line to on the map just right of the
bottom right corner of the swallowed display. That didn't fix
anything, but I've left it in place. bot() to update status is
needed following cls(); now it happens before redrawing the map
instead of at some point after.
For OPTIONS=role:Valk,race:!human,align:!lawful (where first+second
contradicts third or vice versa), you'd get
Shall I pick your Dwarven Valkyrie's for you?
where the what-to-pick field names are empty. Now, align:!lawful
gets overridden, producing
Shall I pick your Dwarven Valkyrie's alignment for you?
and then you'll end up lawful regardless of whether you answer yes
or no. That may be suboptimal but does emphasize that the original
alignment constraint couldn't be honored. (Things just fell out
that way and I haven't tried to make it behave any other way.)
While testing the fix, I noticed that OPTIONS=role:Valk,race:random
prompted
Shall I pick your Valkyrie's race and alignment for you?
instead of honoring 'race:random' without asking, so I've tried to
fix that too.
Role selection has become insanely complex, so one or the other of
these fixes has probably broken some other permuation of partial
specification. Both of the changes here have been done in the core
without touching any interface-specific role selection code.
It was hard to test the attempting-to-revive-shopkeeper-corpse
fix when dying shopkeepers kept declining to leave corpses. Make
shopkeepers always leave corpses (modulo various circumstances
which prevent all corpses). I don't know whether or not temple
priests ought to receive the same treatment.
Reported directly to devteam, zapping wand of undead turning at a
shopkeeper's corpse would cause a crash. 'Traits' to fully recreate
the shk were attached to the corpse, but the temporary monster
created on the map intended to be relaced by the shk didn't have any
eshk struct, and the sequence replmon() -> replshk() -> inhishop()
attempted to access mtmp->mextra->eshk when trying to reattach the
shk to his/her shop. No other mextra structs involve pointer fixups,
so pets, priests, vault guards don't need extra handling.
I tested four cases. #1 and #3 had no shop bill at the time; I'm not
sure about #2. These all worked.
1) shk killed inside shop, resurrected there;
2) killed outside shop on the shop level, resurrected there;
3) killed inside his shop, corpse carried to different level before
being resurrected;
4) killed and resurrected on different level from shop after hero
stole something (teleported out of shop with unpaid item)--shk
left shop to chase hero and followed him/her up some stairs.
Implement the suggestion that a monster killing itself with acid
to avoid turning to stone or with fire to avoid turning into green
slime not break pacifist conduct even if the player caused the
"turning into" situation that triggered the accidental suicide.
Along the way I discovered a serious bug: zhitm() applies damage
to target monster but leaves it to caller to finish killing off
that monster when damage is fatal, but muse_unslime() called it
without checking whether the monster should die. For fire breath
that shouldn't matter since all fire breathers are immune to fire
damage, but when support for wands of fire and fire horns was
added later it just cloned the fire breath code and neglected to
check for fatal damage. The result was that a monster with 0 HP
would be left on the map, then impossible "dmonsfree: 1 removed
doesn't match 0 pending" would be given when taking it off fmon
list, but a stale monster symbol (presumably level.monsters[][]
pointer too) was left on the map which eventually led to monsndx
panic or arbitrary crash.
Replace "dark part of a room" with something more sensible when
examining the map while underwater where water/lava/ice within the
3x3 grid centered on the hero is all that can be seen. Adjacent
non-water, non-lava, non-ice spots are now described as "land".
(Note: this stuff doesn't apply on the Plane of Water where being
underwater gets handled differently.) Spots outside that 3x3 grid
are now described as "unreconnoitered", which sounds a bit odd but
I couldn't come up with anything better. "Not visible" is accurate
when the hero can see but needs adjusting when he can't, bringing
us right back to the current conundrum. I suppose "not accessible"
might be viable but nitpickers would consider it to be inaccurate
if hero has teleport capability. (There are a couple of references
to "unknown" from earlier versions of this revision. I think
"a ghost or unexplored or unknown or land or air (land)" is the only
place left where the player might see it, and it seems reasonable
there, although perhaps it ought to be changed to "unreconnoitered".)
Also fix farlook while swallowed and blind, where blindness was
overriding swallower Id even though it doesn't do so for mon_nam()
and things which use that like combat feedback.
Quiting without ever examining inventory caused the Qt interface
to issue an impossible(), then crash due to deferencing a Null
pointer. The prior fix was to suppress the validation code that
was crashing. This changes things so that the inventory window
always gets at least one use, allowing the Qt validation code to
succeed. tty and X11 are ok with it; win32 needs to be verified.
View the regular map, as originally intended, when using #terrain
while underwater. Doing so means you can't see an underwater object
underneath an adjacent critter by viewing map+traps+objs (without
monsters), but being able to see the level map instead of just the
8 adjacent squares is more valuable.
When underwater, map adjacent lava as lava rather than as "not water".
The Valkyrie quest has lava adjacent to water and the hero ought to
be able recognize it while immersed so she doesn't try to climb out
of water directly into lava.
When the color option is disabled and lava uses the same symbol as
pool or as water (which is the case for default ascii, DECgraphics,
and IBMgraphics), apply the detected-monster display effect to lava.
For tty, this will draw it in inverse video if the use_inverse
option is enabled. (Assumes non-tty will nearly always be using
color so not care.) Creating a separate mapglyph special attribute
for B&W lava instead of overloading MG_DETECT ought to be done but
I didn't want to modify any interface code.
