Michael Meyer [Fri, 30 Sep 2022 00:22:00 +0000 (20:22 -0400)]
Fix: X11 extcmd menu heap buffer overflow
Instead of allocating space for ((n + 1) * size) (to make room for all
entries plus terminator), it allocated space for (n * size + 1).
init_extended_commands_popup would therefore write past the end of the
memory allocated for command_list and command_indx when trying to store
their respective terminator entries. This meant the X11 windowport
would crash when accessing the extended command menu, if NetHack had
been compiled with -fsanitize=address.
I also did some minor cleanup/refactoring to eliminate variables and
lines that were made redundant or useless by 9d64d13, which changed the
way the function worked and removed the need for things like tracking
indices in the source and destination arrays with separate variables.
A change to the curses interface from three years ago to make sure
that round-off didn't make the horizontal and vertical clipped map
indicators appear to not be clipped was using ROWNO for both instead
of COLNO for the horizontal one. For modest clipping the mistake
was unnoticeable; I don't know whether that remained true for more
extreme clipping.
[Not fixed: the curses scrollbar stuff ignores the fact that map
column 0 is unused.]
warning: for tile 250 (numbered 250) of objects.txt,
found 'huge chunk of meat' while expecting 'enormous meatball'
../win/share/objects.txt: 460 tiles
Make the prompt for single genocide be more precise
from: What monster do you want to genocide?
_to_: What type of monster do you want to genocide?
and the re-prompt for it be slightly more verbose
from: What... [type the name of a monster]
_to_: What... [enter the name of a type of monster]
Also, make the already verbose re-prompt for class genocide even
more verbose
from: What class... [type the symbol representing a class]
_to_: What class... [type the symbol or name representing a class]
Possibly should have changed 'type' to 'enter' on the last one to
keep them consistent but I left that as-is.
Pull request from entrez: genocide prompt has a short explanation
appended. Skip that on first try, and on all tries if 'cmd_assist'
option is Off.
For class genocide, don't retry an unlimited number of times if
player supplies an empty response. Any empty response will now
count toward the 5 chances available to supply a valid choice, as
it does with single genocide.
Michael Meyer [Wed, 28 Sep 2022 01:08:30 +0000 (21:08 -0400)]
Show genocide prompt help iff 'cmdassist' enabled
The "[type the name]" prompt seems appropriate for handling via
cmdassist, so experienced players who don't need the help can hide it.
I also added a corresponding help note for the class genocide prompt.
Michael Meyer [Wed, 28 Sep 2022 00:47:57 +0000 (20:47 -0400)]
Don't screen out empty input in class genocide
So that a blank line wouldn't use up one of the player's "tries" for
class genocide, the game would continue to prompt until the input was
non-blank (or the user hit <esc>), with a tight loop around the getlin
call that only exited when it got some non-empty input. This apparently
risked leaving the game endlessly looping under some worst-case-scenario
hangup conditions. It was also inconsistent with normal genocide, which
doesn't have special handling of blank lines. Make the class genocide
prompt behave like the normal genocide prompt by removing the "blank
input" loop (and consequently treating a blank line the same way as any
other attempt to write a name).
Pull request from chasonr: if the curses interface did a full screen
update while the game was ending, it could attempt to refresh windows
that had already been removed.
For #if PDCURSES, raw_print() could attempt to write to the message
window after it had been removed.
[Why is raw_print() using the message window at all? Isn't the main
point of it to bypass that?]
Ray Chason [Wed, 28 Sep 2022 00:57:09 +0000 (20:57 -0400)]
Fix undefined behavior when exiting Curses
* When saving: curses_exit_nhwindows calls curses_uncurse_terminal,
which calls endwin. curses_exit_nhwindows then calls raw_print,
which calls more Curses functions after endwin has been called.
Fix this by having curses_raw_print use puts if window_inited
is false.
* When dying, quitting, etc.: really_done opens the "Goodbye" window,
which refreshes the other windows when it closes. But the status
window (and possibly the map and message windows) are gone by that
point. The window pointers are properly NULLed, but the NULL is then
passed to touchwin. Fix this by checking window pointers for NULL.
Michael Meyer [Thu, 25 Aug 2022 01:38:56 +0000 (21:38 -0400)]
Fix: IS_SUBROOM_INDEX range
The macro (currently unused, I think) for checking whether a particular
index designates a subroom was off by one on the maximum allowable
value.
Because of the dedicated extra space for the g.rooms array terminator
flag (hx == -1), subroom indices in g.rooms are set out in the range
[MAXNROFROOMS+1, MAXNROFROOMS*2], inclusive.
Pull request from vultur-cadens: don't change unID'd gem prices
when source changes insert some new object before them or the game
gets rebuilt with different featues enabled such as MAIL.
This is more about keeping the spoiler table in the wiki accurate
than anything to do with game play. It definitely gives players
who use spoilers an advantage over ones who don't, but does so in
such an insignificant aspect of play that it likely doesn't matter.
I'm fairly sure that the remark that keeping the prices the same
is what the original implementor intended is not correct. I don't
recall who implemented this and the commit log for the first cvs
repository is long gone so it's moot.
Make the token selling prices for unidentified gems not depend on how
many items were defined before FIRST_GEM. Now the unidentified gem
selling prices will depend only on the number and defined order of the
types of gems, and won't inexplicably change when objects are added,
or depend on compile-time options such as MAIL.
Also don't do the regular item price reduction for unidentified gems,
since they are already not based on the actual value. This restores
the pre-3.6 behavior, allowing players to gain a bit more information
from the nominal selling prices of unidentified gems.
Whoever first introduced this special handling for gems probably
intended for players to be able to gain information from gem prices
this way, but probably nobody has been doing it since 3.6.
rename "huge chunk of meat" to "enormous meatball"
Pull request #607 by Vivit-R proposed renaming "huge chunk of meat"
to "giant meatball" to better reflect the similarity to meatball.
But an object name that contains a monster name prefix requires extra
work in the wishing code. I considered "huge meatball" which retains
more of the original name but decided to go with "enormous meatball"
becaues it seems more evocative.
> I'm not sure whether gcc 3 is really the right test for whether the
> returns_nonnull attribute setting is available.
The gcc.gnu.org website only goes back to 5.1, and searching the
documentation of that version for returns_nonnull finds it. I used
ftp to get gcc-core-3.0.0 and gcc-core-4.0.0 and their doc files don't
mention this attribute. It might have been added for some later 4.x
but that really doesn't matter for nethack's purposes.
Use __GNUC__ >= 5 instead of __GNUC__ >= 3 when testing whether
__attribute__(returns_nonnull) is available.
The menu to interactively set the windowborders option for curses
uses 'a'..'e' for choosing 0..4. Accept '0'..'4' (via unseen group
accellerator) too.
If environment had TTYINV=4 (perm_invent shows worn/wielded only),
disclosing inventory at end of game only showed worn and wielded
items instead of full inventory. Didn't matter whether perm_invent
is On or which interface is in use or whether game was built with
TTY_PERM_INVENT enabled.
This is an alternate way to deal with pull request #876, where
splitting a stack that has a name assigned updated perm_invent when
cloning the name and ran into trouble with shop billing when trying
to format for persistent inventory display.
The PR#876 fix has been left in place but wouldn't have been needed
if this had gone in first.
Reported by paxed 8 years ago: if a bones file contains a
doppelganger imitating a unique monster, when it gets loaded
that monster ends up being marked as having been created. The
doppelganger itself will shapechange to other forms, but the
unique monster won't be created when it should be because it has
become extinct.
Report involved creating a statue of a unique monster which
yields a doppelganger in that monster's shape, then using stone
to flesh to animate the statue, dying before it changes to some
other shape, and having bones be saved.
Pull request from copperwater: don't give "flash of light" feedback
when activating a level teleporter because it's too much like one of
the outcomes of a magic trap.
This doesn't use the suggested commit. Getting the "you feel
disoriented" feedback before being asked for destination level (when
having teleport control) seemed contradictory. This gives different
feedback and does so after the actual teleport.
Supersedes #850
and since nhcopier doesn't seem to understand "supersede"
Closes #850
Mark alloc()--also dupstr() and re_alloc()--for gcc and clang as
always returning non-Null. This should silence some of the static
analysis complaints.
Almost all the monster and object naming functions (anything that
returns an mbuf or an obuf) should be marked this way too but I'll
leave that for somebody else to deal with.
I didn't attempt to mark alloc() with the 'malloc' attribute because
macro definitions could end up causing trouble. Specifying its
deallocator would probably be useful but is at even bigger risk of
macro interference.
I'm not sure whether gcc 3 is really the right test for whether the
returns_nonnull attribute setting is available.
Year old issue from copperwater: 'open' directed at a non-door told
player that there isn't a door and took no time unless character was
blind and learned what type of terrain it is, applying a key gave
the same message but used a turn and didn't update map to reflect any
terrain discovery.
Attempting to open an adjacent door or applying a key to one while in
a pit had a similar issue: they produced the same "you can't reach
it from here" but had different time vs no-time outcome.
Noticed that an attempted terrain replacement wasn't taking hold even
though 'w' is supposed to mean "match any stone or wall"; this was
because w converts into non-terrain-type MATCH_WALL and replace_terrain
was doing a simple comparison on whether the potentially replaced
terrain matches that type. Add a special check here for w so it will
match the terrain types it's supposed to.
Note that using replace_terrain with 'w' now WILL match stone, since
this is the documented behavior of w, to match IS_STWALL rather than
just IS_WALL. If a level designer really wants to exclude stone, they
can work around this by either making a selection and filter out stone
terrain, or doing two replace_terrains with '-' and '|'.
Pull request from copperwater: the random traps specified by the
special level definitions of the tourist locate and goal levels could
be placed inside the shops present on those levels.
copperwater [Sun, 14 Aug 2022 00:04:25 +0000 (20:04 -0400)]
Fix: prevent traps in shops in the Tourist quest
The Tourist locate and goal levels have 2 shops each, and also have
various traps randomly placed on the level. Unfortunately, this does not
account for placing them in the shops, so it was possible to wind up
with a magic trap or falling rock trap or whatever inside the shop,
which the shopkeeper would not clean up.
This commit makes selections that exclude the shop areas, and picks the
traps from those selections, keeping the shop floors trap-free as their
customers expect.
Someone asked whether the 'magr' argument to artifact_hit() can be
Null or not since the code sometimes checks whether it is Null and
other times uses it unconditonally. The answer is "it depends."
Can't reply to asker due to forced anonymity when the contact form
was submitted.
Michael Meyer [Thu, 22 Sep 2022 02:19:42 +0000 (22:19 -0400)]
Prevent impossible fall dmg if falling up
The way hole destinations work now theoretically allows for a
cross-branch hole or trap door to move you across branches in a way that
decreases your overall depth. If this happened, it would cause an
impossible when the negative result of (depth(new) - depth(old)) was
used to calculate fall damage. Limit fall damage to 1d6 if dist <= 0.
Michael Meyer [Thu, 22 Sep 2022 01:25:25 +0000 (21:25 -0400)]
Apply trap door destination restrictions in dodown
Missed this way to use the trap door (in a block added in 05761ba) in
previous commits, though I'm a little confused about whether that block
in dodown is even reachable given how various trap scenarios are handled
with dotrap earlier in the function.
Michael Meyer [Thu, 22 Sep 2022 00:06:35 +0000 (20:06 -0400)]
Refine attempt to clamp trap door fall destination
This should prevent anyone from exploiting falling into the sanctum (by
taking advantage of a trap door in a bones file from a differently
laid-out dungeon, as described in the previous commit) to bypass the
invocation, in addition to falling past the actual end of the dungeon.
Michael Meyer [Wed, 21 Sep 2022 23:55:04 +0000 (19:55 -0400)]
Prevent hero from falling past end of dungeon
Similar story with saved trap door destinations: if a bones file near
the end of the dungeon came from a longer dungeon (i.e. with a
lower-depth castle) than the one the bones file is loaded into, the
trap door destination could be past the dungeon end. Clamp the
destination so it won't be lower than the bottom level of the dungeon.
Michael Meyer [Wed, 21 Sep 2022 23:44:27 +0000 (19:44 -0400)]
Have monsters' hole destination match the hero's
The fixed destination of a hole or trap door was being used for the hero
but not for monsters. Make everyone land in the same place, so you can
chase a monster into a hole and actually find it.
Michael Meyer [Wed, 21 Sep 2022 23:41:45 +0000 (19:41 -0400)]
Fix: antigravity trap doors
Trap doors saved their destinations as an absolute level, rather than a
relative one, so if you loaded bones from a special level their
destinations would reflect the dungeon layout from the bones player's
game. For example, die on the Oracle level, on dlvl5, with a trap door
that goes to dlvl6. Another player gets those bones on their Oracle
level, which is dlvl8... the trap door would still go to dlvl6. Pretty
amazing trap door -- something you might see in a funhouse!
Include relative rather than absolute destinations in save and bones
files, much like stairs do, to avoid this problem.
I bumped EDITLEVEL because although this won't break save files in an
obvious way, it will interpret the (absolute) destinations in existing
save and bones files as relative, leading to some crazy long falls. :)
Pull request from argrath nearly a year ago: an 'if' and corresponding
'else' have the same code so there's no point in testing for if/else.
I'm still not convinced that simply removing the if/else is the right
fix here but nothing else is going on. I've put part of the removed
code back inside '#if 0' in case it needs to be resurrected someday.
Pull request from entrez: the check for whether a pet in desperate
straits will eat a corpse that will cause it to polymorph used bad
logic. The suspect code was added post-3.6.
Michael Meyer [Tue, 20 Sep 2022 01:59:34 +0000 (21:59 -0400)]
Fix: starving herbivore pet vs polymorphing corpse
The special handling of polymorphing corpses included an "is acceptable
food if starving" rule which ignored the pet's other food preferences,
so a starving herbivorous pet would become willing to eat a
non-vegetarian corpse iff the corpse would polymorph it when eaten.
Along the same lines but latent: if there were a vegan polymorphing
corpse, a carnivorous pet would eat it not only if starving, but also if
maltreated and on the verge of becoming feral.
Instead of trying to fix this by reimplementing all the herbi vs carni
rules specifically for polymorphing corpses, just have a "don't eat
polymorphing corpses if neither starving nor almost untame" rule, and
fall back to the normal palatable corpses tests once it's been
determined that the pet is willing to eat even a polymorphing corpse.
I rephrased the comment just to make it negative (about avoiding
polymorph, rather than polymorph being OK under certain circumstances)
to match the new form of the rule.
Michael Meyer [Wed, 24 Aug 2022 17:08:46 +0000 (13:08 -0400)]
Describe engulf attacks a bit more consistently
Use verbiage for mon vs mon and hero (mostly hero) engulf attacks that
matches recent changes to monster vs hero engulf attacks more closely
(e.g. "swallows whole" instead of "engulfs" for purple worm, other
changes in b07fe59...). Also ensure non-AD_DGST engulf attacks
(e.g. from revamped trapper or lurker above polyforms) aren't treated as
"eating" (or as involving "debris").
Also change the enfolds and digests macros so they produce booleans
rather than attack pointers (I got a compiler warning about casting
struct attack * to boolean when I did 'boolean b = digests(ptr);').
fixes entry and tweaks for PR #871 - revive corpse
Pull requet from entrez: give better feedback than "it" when hero
observes a corpse reviving into a monster that can't be seen.
Tweak reviving from a container which was coded as if the container
was optional. That can lead to confusion when someone reads the
code so make the situation more explicit.
Michael Meyer [Sat, 10 Sep 2022 03:07:57 +0000 (23:07 -0400)]
Improve description of invisible mon revival
An invisible monster reviving would be called "it" (as in, "It rises
from the dead!"). Improve on this a bit by instead saying that "the
troll corpse disappears!" (similar to the messaging used when undead
turning is used on an invisible monster's corpse), or calling the
revived monster "something" (as in "something escapes from a sack!")
instead of "it". Distinguish between the original location of the
corpse and the location of the revived monster when describing seeing
things that have happened to the corpse, since revival may not place it
on the corpse's location.
Pull requet from vultur-cadens: use space instead of hyphen in
enlightenment and end-of-game feedback.
All the other resistances already use space. The inappropriate
hyphen in "disintegration-resistance" seems to have been present
since enlightenment was added (in 3.0; back then the relevant code
was in cmd.c; current insight.c didn't exist until 3.6).
when splitting a stack of named, shop-owned objects
Pull request from entrez: when perm_invent is on, splitting an unpaid
stack would issue impossible "unpaid_cost: object wasn't on any bill"
while cloning the new stack's name from the old stack.
Michael Meyer [Sat, 17 Sep 2022 02:50:39 +0000 (22:50 -0400)]
Fix: impossible from splitting named stack on bill
Trying to split an unpaid stack of named items in a shop, with
perm_invent enabled, would cause an impossible 'unpaid_cost: object
wasn't on any bill' because copy_oextra -> oname triggered an inventory
update while the newly created split stack was marked unpaid but before
the billing information had been split to match. Defer the copy_oextra
call until the billing info has already been split.
Change the handling for windowing system specific files so that
when building for more than one set, each gets compiled as a set
instead of some being interspersed among rival window systems.
Put differently, handle tile.o specially so that there's no need
for the hints to sort the WINOBJ list in order to avoid tile.o
duplication.
So the order of compilation is
common source files
unix-specific files
tty files
curses files
X11 files
Qt files
tile.c (if applicable), version.c, date.c
Previously, some of the X11 files were scattered around among the
others because of the spelling of their file names.
Only matters if you're watching the progress of a build.
Pasi Kallinen [Sun, 18 Sep 2022 09:35:30 +0000 (12:35 +0300)]
Themerooms: Engraving hints the location of buried treasure
Add two new themeroom functions that are called when generating
the level: pre_themerooms_generate and post_themerooms_generate,
calles before and after themerooms_generate.
Allow the buried treasure -themeroom to put down an engraving
anywhere on the level, hinting at the location of the treasure.
des.object contents function now gets the generated object passed
to it as a parameter.
options.c: In function ‘option_help’:
options.c:8820:55: warning: ‘%s’ directive writing up to 255 bytes into a region of size 220 [-Wformat-overflow=]
8820 | Sprintf(buf, "Set options as OPTIONS=<options> in %s", configfile);
| ^~ ~~~~~~~~~~
This reverts commit 94945a719afc0aa08eaff9aa1e0e2fbf159bf388.
It was too intrusive and can be handled by the 'onefile' script
by compiling isaac64.c before any source file that includes hack.h.
Issue reported by k2: tipping a wand of cancellation, bag of
holding, or bag of tricks from a non-magic container into a bag of
holding causes the bag of holding to explode but it wasn't dealing
out explosion damage nor being logged for livelog/chronicle the way
putting the item directly into a bag of holding is handled.
This fixes the described issues but bones handling leaves a lot to
be desired.
Reported by Umbire:
|You kill SpaceMannSpiff! SpaceMannSpiff puts on a dwarvish cloak.
|SpaceMannSpiff puts on a dwarvish iron helm.
|The seemingly dead SpaceMannSpiff suddenly transforms and rises as
| a Vampire.
This was tough to reproduce but I finally managed it. The issue
text mentions that it was fixed by copperwater in xNetHack with
commit 8c4af50f0aa3e72522f3eb98df039ff25c2a1ea0 to the repository
for that variant. My attempt to cherry-pick that failed--I'm not
even sure whether it should have been expected to work--and some of
the code has been impinged upon by changes, so I ended up applying
the contents of that commit manually.
The commit changes how/when monsters put on new armor rather than
anything directly related to vampires. Circumstances similar to
the example above now yield:
|You kill SpaceMannSpiff!
|The seemingly dead SpaceMannSpiff suddenly transforms and rises as
| a Vampire.
on one turn, then on the next turn the revived vampire produces:
|SpaceMannSpiff puts on a dwarvish cloak.
My test case only had one item of interest; I assume that the second
item of armor gets worn on a subsequent turn rather than at the same
time as the first one.
Require a free hand when tipping a container into another container.
Presumeably you need to open the destination container and possibly
keep holding it open.
If you try to tip a carried container into an unknown bag of tricks,
apply the bag (once) instead of performing the tip. (To 'open' the
destination as above.) Possibly slightly confusing if bag is empty.
When tipping a container, always ask for the destination instead of
doing that only when carrying other containers. Confirming floor
as destination can be annoying but having to do that sometimes and
skipping that sometimes is aggravating because it is error prone.
And floor is preselected so can be chosen with space or return.
(I wanted to change the selector letter for floor from '-' to '.'
and then keep '-' as an unseen group accelerator, but the latter
doesn't work for PICK_ONE so I've left '-' as-is.)
Don't display "monsters appear" after tipping a bag of tricks.
Monster creation gives feedback these days. (Comparable to recent
"summon nasties" fix.)
Pasi Kallinen [Thu, 15 Sep 2022 15:08:32 +0000 (18:08 +0300)]
Split themeroom shape from themeroom contents
Previously, the tetris-shaped rooms were always either
normal rooms, or turned into shops or other special rooms
in NetHack core. Now, the themed room lua code first picks
the themed room (which can be a themed or shaped), and some
of those will then pick a random filling (eg. ice floor,
traps, corpses, 3 altars).
Adds a new lua binding to create a selection picking locations
in current room.
The content-function in special level regions now get passed
the room data as a parameter.
For tipping purposes, a horn of plenty is treated like a container.
But using one as the source container in a container-to-container tip
wasn't supported. Implement that.
Also, #tip was offering carried bags of tricks as candidate containers
to tip some other carried container into. Only do that for ones which
aren't known to be bags of tricks (so when type not discovered yet, or
specific bag not seen yet due to blindness).
Ray Chason [Tue, 13 Sep 2022 23:49:56 +0000 (19:49 -0400)]
Sundry fixes for DOS 16-color VGA mode
To test 16-color mode, specify OPTIONS=video:vga explicitly;
autodetect will choose a VESA mode if it can.
* Draw tiles correctly when redrawing from panning or from changing
the map mode among text, tiles and overview. Previously, this would
draw everything with the tile at the hero's position.
* Draw corridor walls with the stone tile.
* Map the statue colors to the nearest neutral tone among the main
16 colors. This mainly affects altars and female cats.
* Fix the code that shows statues as the generic statue tile. This
code could be deleted, but the statues don't draw with the full
range of gray tones.
When tipping a magic-bag exploder from a sack or box into a bag of
holding, the choice of whether to call useup() or useupf() was
backwards. But nothing bad happened which is fishy.
Reported by k2: tipping one container's contents directly into
another container allowed transferring a wand of cancellation (not
mentioned: or a bag of holding or a bag of tricks) into a bag of
holding without blowing it up.
That's now fixed. There are other issues that this doesn't touch:
I think it's odd that you can transfer stuff from one carried
container to another but not from a carried container to a floor
container nor from one floor container to another one at same spot.
I didn't test shop billing so an not sure what happens when #tip
blows up a bag of holding and there are some unpaid items involved.
Using #tip on horn of plenty treats it like a container, but doing
that when it's carried doesn't offer the chance to tip its contents
directly into a carried container.
Tipping a carried container does not require free hands or even
limbs (for playability) but tipping such into another container
should require at least one free hand.
Renaming got_from_config[] (something that was done in the past)
to set_in_config[] (something to do in the future) is sufficient to
remove any confusion about why it is being set from 'O'. Since that
is the name of an enum value, use opt_set_in_config[] instead.
The definition of enc_stat[] got changed by a pull request nearly a
year ago ('const char *enc_stat[]' -> 'const char *const enc_stat[]')
but the separate declarations for it weren't changed to match.
Make the same change for hu_stat[]. Not sure why the pull request
didn't include it since the old declaration and the usage are same.
Reported seven and a half years ago: if you are in a vault but not
carrying any gold and the guard arrives, you're told "Follow me."
Then if you pick up gold while the guard is still in the wall breach
rather than out in the corridor, you would be told "I repeat, drop
that gold and follow me!" "Repeat" refers to the follow part but
sounds as if it refers to the drop-gold part which isn't actually
being repeated. Keep track of whether the guard has issued a drop
gold demand and use that to vary the wording of subsequent "I repeat"
message.
Modifies monst->mextra->egd so save and bones files are invalidated.
projects / nethack / log