This was discovered when a game of xNetHack crashed with stack smashing
detected during dumplog creation after an ascension. I traced the
problem to a wish with a very long string the player had made much
earlier in the game ("greased very blessed holy rustproof unlit historic
thoroughly +5 very cloak of protection named it would be a shame if
something happened to me wearing this cloak"), which is further recorded
in an even longer form in the chronicle as 'wished for "X", got "Y"'.
That string does get truncated, but since the gamelog strings are
dynamically allocated, they can be longer than BUFSZ.
When show_gamelog was subsequently called, it didn't use any bounds
checking, which allowed its stack-allocated buffer to overflow. Changing
the offending sprintf to snprintf and limiting it to the buffer size
appears to fix this issue. It will truncate the string at BUFSZ-1
characters and therefore will be expressed in the dumplog as an
incomplete string, but 1) that was happening anyway because the gamelog
string already doesn't capture the entire "wished for X, got Y" message
on such a wish, and 2) this should only ever happen for very long
wishes.
Most things that can be dug or chopped can only have that done by one
of the two types of digging/chopping tools: pick-axe or axe. Since
closed door can be broken open via either type, mention the type of
implement in the final "you break through the door" message by adding
"with your <uwep>."
After updating the --dumpweights code in hack.c to insert "pair of"
for gloves and boots and "set of" for dragon scales, I've switched
it to use simple_typename() instead.
Turns out that that routine also lacked handling for 'pair|set of'.
And it was generating "coin of gold piece". Fix those.
Roughly half of the gems are "<gem>" and the others "<gem> stone",
so the --dumpweights output is different by more than just pair/set.
Experience-level and experience-points, if enabled, could be
highlighted via 'up' or 'changed' rules in initial display after
restore. I tried 'down' rule too but didn't produce with that.
I don't understand what was going on but was able to reproduce it
and then fix it via the trial and error method....
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.
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.
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.
Bug description of #1386 by @copperwater on GitHub:
"When generating a random monster from a class using des.monster(),
the G_NOGEN in their statblock is suppressed, but because every monster
of this class has frequency 0, none of them are actually eligible to get
picked. mkclass ends up returning a null pointer and create_monster has
to pick a random monster instead.
This affects the following levels (all the ones that use random sea monsters):
Healer quest start
Healer quest locate
Plane of Water (difficult to notice, since it has lots of specific sea monsters and only 5 random ones)
This can be pretty easily viewed by going to the Healer quest start and
detecting monsters: there is a shark and a giant eel, which are
specifically defined, but the remaining random sea monster that should
be there is absent."
Add a tracking array mclass_maxf[MAXMCLASSES] (about 61 entries, the
first not being used), and fill it one time in init_mongen_order() with
the maximum frequency value seen of any monster in that class.
Any mclass_maxf[] entry of zero represents that entire class of monsters
having no positive frequency value.
Detect that in mkclass_aligned(), and use it to work around the situation
to produce the monster being sought by the Lua level description file.
