Those places that use get_table_str_opt() to get an optional string
value can instead use a function that returns a string.
This can be used for example in quest data lua table, or some table
fields in the lua api bindings, or the dungeon definition.
For example, in quest.lua
text = "You again sense %l pleading for help.",
could be replaced with
text = function() return "You again sense %l pleading for help."; end,
which of course allows using lua to build the string.
There were couple reports of doors being generated in a corner
of a room. This happened for randomly generated irregular rooms,
because the code that was deciding if a corridor starting or
ending location was good did not handle irregular rooms at all.
This changes the corridor code so it can now generate start and
end points properly for any valid position in irregular rooms,
instead of only on the edges. This means the corridor sometimes
meanders a bit more than before, because it tries to find
the end point away from the edge of the room rectangle.
Also added is sanity checking for the randomly generated rooms
and corridors level, testing for door placement and room connectivity.
And another fix for a rare special case where dig_corridor
created a zero-tile long corridor; the entrance door was placed,
but there was nothing behind it.
Fixes github #1269 and #1385
Fix warnings
objnam.c: In function ‘wizterrainwish’:
objnam.c:3536:54: warning: variable ‘didblock’ set but not used [-Wunused-but-set-variable]
3536 | boolean madeterrain = FALSE, badterrain = FALSE, didblock, is_dbridge;
| ^~~~~~~~
Disarming a chest trap was setting obj->tknown = 0 even though the
hero just discovered that it isn't trapped.
Triggering a chest trap behaved similarly. Since there are no
repeating chest traps, hero should know that the chest whose trap
just went off is no longer trapped.
chest_trap() didn't document its return value but was clearly meant
to return True if the chest was destroyed. It didn't handle that
correctly when the chest was being carried. However, none of the
callers actually use the return value. [This fix tracks whether the
chest gets deleted; a better fix would be to destroy an exploding
chest even when it is being carried.]
If 'autounlock' is set to test a chest for traps, skip "check for
traps?" when tknown is set; go directly to "disarm trap?" if the
chest is trapped, skip that too if it isn't.
If wand of probing hits a chest, set the tknown bit.
We've had reports of a couple of issues building against musl libc.
Issues reported:
- build procedures utilize col for Guidebook-creation, and col
is deprecated in distros that use musl libc
- some of the CRASHREPORT code is using library functions that
are not available in the musl libc environment. The reported
functions were backtrace() and backtrace_symbols(), which use
header file /usr/include/execinfo.h.
So we'll try to accommodate this. Since we don't have a means of
autodetecting the musl libc situation during the build (as of yet), the
builder will have to specify 'make musl=1' on the make command line.
Specifying 'musl=1' on the make command line will:
1. ensure that NOCRASHREPORT gets defined in the C preprocessor.
2. set COLCMD to be '../util/stripbs' instead of 'col -bx'.
Related to GitHub #1393
We've had reports of a couple of issues building against musl libc.
Issues reported:
- build procedures utilize cat for Guidebook-creation, and cat
is deprecated in distros that use musl libc.
- some of the CRASHREPORT code is using library functions that
are not available in the musl libc environment. The reported
functions were backtrace() and backtrace_symbols(), which use
header file /usr/include/execinfo.h.
So we'll try to accommodate this. Since we don't have a means of
autodetecting the musl libc situation during the build (as of yet), the
builder will have to specify 'make musl=1' on the make command line.
Specifying 'musl=1' on the make command line will:
1. ensure that NOCRASHREPORT gets defined in the C preprocessor.
2. set COLCMD to be '../util/stripbs' instead of 'col -bx'.
Closes#1393
Issue reported by elunna: Using the 'F' prefix against a displacer
beast prevented swapping places.
This doesn't use the suggested fix. It is quite short but there is
a large diff due to change in indentation and reformatting several
comments because of that.
Attacking a displacer beast either with or without 'F' might miss,
hit, or swap places. It won't "harmlessly attack thin air."
Fixes#1377
Protect carried items as well as hero when carrying the Mitre of
Holiness. Already handled when wearing that artifact.
This might make it be too strong. At the time that it was given
the carrued attribute, there was no such thing as carried items
providing any protection to other carried items.
"You have disabled loading of bones levels" (during play) and
"You disabled loading of bones levels" (end of game disclosure)
both clearly refer to the player rather than the hero.
"You have never encountered a bones level" is accurate for current
hero but not necessarily accurate for the player. Rephrase it.
Also, if OPTIONS=!bones is set and the hero just died, extend
"You disabled loading of bones levels" during disclosure to
"You disabled loading and storing of bones levels" (even in the
case where bones wouldn't be saved anyway).
Initially diagnosed in an xnethack fuzzer crash - unblock_point
shouldn't be called when a closed door becomes non-closed, because it's
possible that there's a gas cloud on the space which means it still
blocks vision. These always need to be recalc_block_point. A number of
them were fixed, but when I went through all the xnethack ones, I found
some that were unchanged from upstream NetHack. I reproduced the sanity
check impossibles usually by breathing gas at a door as an iron golem
and then opening or destroying the door to trigger the unblock_point
call.
The use of recalc_block_point in wizterrainwish was not triggering this
bug, but the previous code there basically duplicated
recalc_block_point.
It wasn't clear to me how selection.room() handles room edges and
unusual terrain in the room, so I looked at the code and wrote down how
it behaves for posterity.
I don't believe the one room currently capable of getting a random fill
while already containing some odd terrain, "Blocked center", actually
has unusual terrain in the room fill. This is because filler_region
creates an irregular region (i.e. a room containing the ROOM points in a
square ring around the blocked center). The points in the middle don't
share the same roomno, so they won't be returned in the selection
created by selection.room(). But there's no reason a room couldn't be
added in the future which does specify some nonstandard terrain and then
a themeroom fill.
Not sure how long this has existed without triggering any issues, but
when I was testing out a themed room wider than it was tall, I ran into
rn2-of-a-negative-number impossibles. Traced it to here, where it was
trying to subtract the width of the mapfrag from ROWNO to figure out
which y-value it should place the map on. The correct behavior is to
subtract the height of the mapfrag.
It is possible to create a bidirectional teleportation trap by making a
pair of teleportation traps with a fixed destination of each other's
coordinate. Moving or hurtling onto such a trap correctly materializes
the hero on top of the other trap without triggering it, but for some
reason I didn't dig into, sitting down to trigger the first trap does
also trigger the second one at the destination end, causing you to
counterintuitively teleport twice and end up back where you started.
Fix this by stopping tele_trap() from doing anything if it's called
recursively, using a static variable like spoteffects() does for the same
purpose. I had to adjust a bit of other tele_trap code to remove its
sole early return.
If a player initially goes petless then later obtains a pet, it's absent
from the game chronicle. Fix that by adding a livelog for it.
This required a bit of restructuring in create_familiar() that I wanted
to do anyway: removing the kludge of decrementing u.uconduct.pets when a
figurine has been deployed but isn't actually going to come out tame.
Calling initedog() /after/ deciding whether it's needed or not prevents
a first-pet livelog being produced for a figurine that didn't come out
tame.
The guardian angel code, which avoids calling initedog(), can never be
the hero's first pet anyway because it only appears tame when the hero
has already broken petless conduct. But while checking, I noticed a
duplicate comment, so I removed that.
I was working on another patch involving a message that could be printed
for either a monster or the player (using a struct monst * variable that
either holds the monster or &gy.youmonst), but wasn't able to easily use
pline_mon for the message since the mx and my of youmonst aren't kept
updated as the hero moves. (In my testing, they were always 0, but it's
not clear if they will remain 0 throughout the game, or if that's a bad
assumption to make.)
Allow this in the future by checking for youmonst in pline_mon and
setting the coordinates to 0,0 explicitly so no relative coordinate
message gets printed when it's about the hero.
I'm not sure if it's a reasonable assumption that no messages that could
ever be passed to pline_mon for a player would ever need to note
"(here)" when accessiblemsg is turned on. If that's the case, the
correct thing would be to set the coordinates to u.ux, u.uy instead of
0,0.
Most recent version of XQuartz, same as before. Unfortunately,
newer version of macOS => newer version of Xcode and its command
line tools => newer version of clang => emulating newer version of
gcc which defaults to a more recent version of StdC, I suppose, or
perhaps our hints are specifying that. Whichever, it has resulted
in a bunch of complaints about XtOffset() used in win/X11/winX.c:
|warning: performing pointer subtraction with a null pointer has\
undefined behavior [-Wnull-pointer-subtraction]
Adding -wno-null-pointer-subtraction to X11FLAGS silences them,
but that would require figuring out which versions of gcc and
clang added -Wnull-pointer-subtraction and its negation. Revising
XtOffset() to include the ptrdiff_t casts eliminates the warnings,
avoiding the need for version conditionals to deal with X11FLAGS.
