Used up items moved to the billobjs list still have obj->unpaid set.
That should probably be cleared since it has no meaning there, but
this hasn't done that.
For those keeping score: unpaid checking has triggered three false
positives (so far) and found one bug.
If you were on a level teleporter, the spoteffects() call after
the hero gets expelled could end up going to a new level and
freeing all the monst chains from the level you were originally
engulfed on.
#0 0xba0507 in free
#1 0x87feda in dealloc_monst src/mon.c:2369
#2 0x880a02 in dmonsfree src/mon.c:2194
#3 0x9a7aa2 in savelev_core src/save.c:507
#4 0x9a7a21 in savelev src/save.c:466
#5 0x71eb9d in goto_level src/do.c:1483
#6 0x71833f in deferred_goto src/do.c:1903
#7 0xa2533f in level_tele src/teleport.c:1117
#8 0xa2567b in level_tele_trap src/teleport.c:1198
#9 0xa5c007 in trapeffect_level_telep src/trap.c:1861
#10 0xa5f856 in trapeffect_selector src/trap.c:2497
#11 0xa47497 in dotrap src/trap.c:2586
#12 0x7d669b in spoteffects src/hack.c:2859
#13 0x89d495 in xkilled src/mon.c:3187
The latter parts of xkilled() after the spoteffects() call would
then attempt to dereference the free'd monst pointer.
Save a copy of the monst struct prior to spoteffects() if you were
expelled, then point at the reference copy afterwards.
Resolves#938
Autodescribe was not updating during browse_map() when the cursor was
moved over a gold glyph that was actually a trap, causing the trap to
be described as the previous square that the cursor was on (probably
"unexplored area") instead of as gold pieces. This was especially
noticeable when using OPTIONS=whatis_coord:m, because the coordinate
was not updating when moving the cursor over the trap.
Split some lines that began with one sentence and then continued with
another so that each sentence has its own line as per 'roff guidelines.
Change the continuation lines in the files and environment sections
to begin with <backslash><space><tab> instead of just <tab>, again to
meet the guidelines (don't begin lines with whitespace). This had a
side-effect of suppressing some space insertion for justifying right
margin of right-hand column on some of the lines. That hadn't looked
very good anyway.
Add new file 'usagehlp' to the files section.
Also add previously unmentioned Guidebook[.txt], but it's hard to
explain why it probably won't be present....
Update the bones file entry.
'cmdhelp' isn't used anymore. Should it be moved to outdated/dat and
the Makefile install steps be updated to stop bothering with it?
Pull request from entrez: memory freed when changing levels could be
accessed if the level change happened when hero caused an engulfer to
expel him onto a level teleporter. Wouldn't happen when monsters are
moving because hero's level change will be deferred. Wouldn't happen
for trap doors and holes, but could happen for magic portal if hero
got swallowed while on one after coming through from other side.
Fixes#937
An engulfing monster can expel you onto a level teleporter or other
level-changing trap, in which case it may (under highly specific
circumstances[1]) no longer have been in memory by the time mtmp->mx/my
were accessed to see whether the "Brrooaa" message should be printed.
It also doesn't make much sense to print that message by the time you've
already fallen through a portal, trapdoor, etc, onto another level, so I
think moving it before the spoteffects() call kills two birds with one
stone.
[1] The highly specific circumstances: you must die due to illness or
some other timeout (or generally die on your own turn rather than the
monsters' turn, since this ensures the level change isn't deferred until
the end of the turn), while engulfed above a level teleporter [or maybe
another similar trap -- I tested with a level teleporter], and be
lifesaved, while positioned such that the engulfer can't follow you
through the levelport after expulsion (e.g. surrounded by other
monsters). It may happen under some other conditions too, but even if
so it's pretty rare and was tough to reproduce.
Pull request #936 took away the destruction of potions of acid ("acid
and water don't mix") if they survived water_damage(). Restore that
by forcing them to not survive. Exception: if they're greased and
pass the 50:50 chance of retaining the grease, they aren't destroyed.
Pull request from entrez: if a greased item loses its grease after
being affected by water, say so.
Also, the post-water code could access freed memory for an item that
had been destroyed by the water (potion of acid).
Fixes#936
A potion of acid could be destroyed and freed by dipping into a
fountain, then dereferenced after the fact -- both when checking its
type immediately after the water_damage() call (as was noticed by
hackemslashem and amateurhour on IRC), and also in the later switch/case
a few lines further down in dipfountain().
I basically reversed the original 'er != ER_DESTROYED' test here: as it
was before this, I think the only thing that could hit it was a greased
potion of acid, which would survive the initial dip due to the grease.
Such a potion would be silently deleted. Potions of acid which were
actually destroyed by water_damage, on the other hand, could be allowed
to continue down to the switch/case of further effects (and associated
dereferences). I think this makes more sense in reverse, with potions
that were protected by grease actually being protected and producing
normal dip effects, and potions of acid which exploded causing an early
return with no further effects. This effectively prevents the various
use-after-free scenarios that were possible, too.
This was totally silent, which -- at least for me -- has led to quite a
few cases of believing my bag or cloak is still greased when it actually
wore off the last time I took a dip. I think telling the player that
the grease has worn off would be helpful, and is consistent with other
types of water damage.
The message is printed even if you are blind, since that seems to be
true of all the other messages in water_damage(). I am not sure if that
makes complete sense (especially for ones like a scroll fading -- some
like water getting into a bag could be sensed by touch) but I didn't
change anything there.
Pull request from vultur-cadens: using autodescribe with cursor
movement to browse the map during gold detection didn't give any
feedback for fake piles of gold that get drawn at trap locations.
(After detection finishes, using autodescribe with ; or // reports
on such piles normally.)
Fixes#940
Autodescribe was not updating during browse_map() when the cursor was
moved over a gold glyph that was actually a trap, causing the trap to
be described as the previous square that the cursor was on (probably
"unexplored area") instead of as gold pieces. This was especially
noticeable when using OPTIONS=whatis_coord:m, because the coordinate
was not updating when moving the cursor over the trap.
The consolidation of global variables from scattered source
files into decl.c and declared in decl.h was begun in 3.7.0.
Their placement in common files was done for centralized
initialization and potential re-initialization during a
"play again" scenario.
It wasn't really necessary for all of them to be housed in a
single huge structure to meet the "play again" requirement,
and the single huge structure has been a little unwieldy when
it comes to maintenance.
Following this commit, instead of one single extremely large structure
named 'g' to house all of the relocated global variables, they
are distributed into several ga through gz.
To make things easy for the developer, each variable is placed
into the struct corresponding to the starting letter of the variable.
That way, no lookup is required in order to know which struct houses
a particular variable, it is a simple match to the starting letter
for all the centralized global variables.
A global variable named 'amulets', would be found in ga.
ga.amulets
^ ^
A global varable named 'move', would be found in gm.
gm.moves
^ ^
A global variable named 'val_for_n_or_more' would be found in gv.
gv.val_for_n_or_more
^ ^
A global variable named 'youmonst' would be found in gy.
gy.youmonst
^ ^
It turns out that there are some objects marked unpaid that aren't
carried by the hero, so the recent sanity check for unpaid/no_charge
could complain. Unpaid items dropped on the shop boundary (gap in
shop wall, doorway, shk's free spot) stayed unpaid when dropped onto
the floor, similar to recent change for pushed shop-owned boulders.
Don't give sanity complaints for those. They could be all the way
inside a shop too, where unpaid items in a gap in the shop wall got
pushed into the shop when the wall was repaired. (Possibly those
should come off the bill instead of remaining unpaid.)
Teleporting items out of a shop was marking them unpaid instead of
treating that as robbery. That's a bug caught by the sanity check.
rloco() was also marking shop items which got teleported from one
spot inside the shop to another spot inside the same shop as unpaid.
Fix both of those things. Also, if an unpaid item on the boundary
gets teleported all the way inside, take it off the bill.
Change 'I u' to mention whether there are additional unpaid items on
the floor somewhere since they won't be part of unpaid inventory and
they're not on the used-up bill either. It might occasionally help
the player figure out why the shopkeeper won't let the hero out of
the shop.
Issue reported by AndrioCelos: bullwhip using monster was able to
snatch hero's weapon when hero was engulfed.
Fix is trivial: when a monster is choosing an item to use, don't
pick bullwhip if hero is engulfed. Regular attack attempts already
skip engulfed hero.
Fixes#935
Issue reported by Melon2007: when non-deaf hero heard an unseen
monster read a scroll, the monster's type was identified accurately
(unless distorted by hallucination). That was intentional but it
doesn't seem plausible for the hero's hearing to be that acute.
Change it to report the monster type accurately if not hallucinating
and monster is the same species as the hero (as the current form if
hero is poly'd), otherwise report it as "someone" when it's humanoid,
otherwise as "something".
Also, if the monster is heard at a spot that would be visible if
hero could see, draw a "remembered, unseen monster" glyph there.
Fixes#934
If paranoid_confirm settings include praying, don't put the answer
to "are you sure you want to pray?" into the do-again buffer where ^A
would use it to ignore confirmation if prayer is repeated. And for
wizard mode, when confirmation is 'y' then the answer to "force the
gods to be pleased?" has to be suppressed from the do-again buffer too
or it would be used by subsequent ^A to answer "are you sure?".
This is basically a band-aid just for #pray. There are probably other
confirmations that should be suppressed from do-again instead of being
reusable. The rest of the paranoid_confirm ones should be ok because
they require "yes" and that doesn't end up in the do-again buffer, but
there are bound to be other confirmations that shouldn't automatically
be re-used during repetition.
Issue reported by Meklon2007: typing arrow keys when a menu is open
can end up with hidden counts. That's a Windows thing and this
makes no attempt to address it. (That's also a user error since
menus don't support arrow key use.) It shows up more for throwing
that for other things because fetching an object from inventory for
throwing attempts to enforce a count limit during item selection
that other actions don't.
But feedback could also be odd if you explicitly specify a count
since the rejection wasn't attempting to distinguish throwing more
than one from throwing more than you have. This changes things so
that with invent of
|$ - 3 gold pieces
|a - a dagger
|b - 3 darts
t4$ now yields "You only have 3." instead of throwing all 3
t4a now yields "You only have 1." instead of "you can only throw one"
t2b still yields "You can only throw one at a time."
t4b now yields "You only have 2 and can only throw one at a time."
In each case, it will reprompt rather than terminate the throw.
"Only one at a time" was already in place when multi-shot throwing/
shooting was introduced and became iffy then, but the way to try to
throw a specific amount is via a repeat count before t rather than
by choosing a subset when selecting the inventory item for t. The
count prefix method also works for f which doesn't otherwise provide
an opportunity to specify count since inventory item is preselected
via quiver.
Someone might want to reopen the arrow behavior as a Windows issue
but I'm not sure how that would be fixed other than by eliminating
its attempt to be user-friendly in converting arrows into movement
direction keystrokes.
Closes#933
Move the mention of viewing usage info via 'nethack --usage | more'
to the end, where it should remain visible if text has scrolled off
the top of the screen (which is nearly certain since it ended up
being much longer than originally intended).
Also, rephrase the text at the start about restore vs new game since
the previous description said "in all cases" which isn't applicable
for 'nethack --scores' or --version or --showpaths or --usage.
Move 'nethack --usage' to last so that 'nethack --scores' is first
among the non-playing command variants since -s is of more interest.
For 'nethack --scores', move -v before the other options since it
has to be next after -s|--scores to be processed correctly. Also,
avoid using "present" twice in the same sentence.
Although gcc specifies support for declaring a function as
noreturn after the function name and parameters, other compilers
do so via an attribute at the start of the declaration. Add some
macro support for the attribute-at-the-beginning method:
o MS Visual Studio compiler
o Upcoming C23 standard (untested at this point)
../win/curses/cursinit.c:102:9: warning: variable 'min_message_height' set but not used [-Wunused-but-set-variable]
int min_message_height = 1;
^
1 warning generated.
Handle items in gaps of a wall shared between adjacent shops.
Make handling of shop boundaries more explicit: walls, the door,
and the "free spot" by the door aren't classified as 'costly' but
obj->unpaid and obj->no_charge are valid there.
Move unpaid/no_charge checking into its own routine to unclutter
objlist_sanity().
Pushing a shop-owned boulder to the free spot or doorway or gap in
wall triggers the sanity check for the time being.
When dist2() got changed to use coordxy parameters, a macro that uses
it in its definition was overlooked and it had (int) casts in it.
That caused a warning about possible data loss when the int
then got converted to coordxy for the dist2() call.
Give online2() coordxy parameters instead of int, like its bretheren.
Avoid a couple of implicit conversion warnings where ints were being assigned
to smaller uchar or ints being assigned to smaller short.
A couple of signed vs unsigned warnings on some rumor processing.
Avoid some signed vs unsigned warnings in mdlib/makedefs where a signed int
param eventually got used in an external call that took size_t.
Eliminate all of it by just having the outer NetHack routine also take
a size_t.
Lastly, insert some default C99 alternative time-related code
in mdlib/makedefs since asctime() and ctime() are being flagged as
deprecated in the upcoming C23 standard and will now start to trigger
warnings for anyone using a C23-compliant compiler.
The test system is Slackware 14.2, which uses Qt 4.8.7.
* WANT_WIN_QT4 is defined, and has the expected meaning. Qt 5 is still
the default.
* The QT_NO_SOUND macro now excludes all headers and declarations
relating to sound; the multimedia package is not needed to build
(on any Qt 4, 5 or 6).
* A new function, nh_qsprintf, replaces QString::asprintf, for Qt
older than 5.5. These versions do not have QString::asprintf.
* DYNAMIC_STATUSLINES is disabled for Qt older than 5.9. These versions
do not have QSplitter::replaceWidget.
Pull request from chasonr: a symbols file with more than one set
having the unicode attribute effectively merged all unicode sets when
loading any of them. Also, freeing unicode glyphmap entries for gems
would attempt to free some of them more than once for those that had
colors cloned from other gems.
[This new code compiles but is otherwise untested by me.]
Closes#924
Shuffling gem appearances can cause mappings from object to
appearance that are not one-to-one. Copy any multiple mappings and
free any mappings that are left unused.
Using '-u name' rather than '-uname' was being treated as '--usage'
for any value of 'name'.
'-uname' worked as intended unless name was 'sage' (or leading
substring of it). That's still the case after this fix, where the
space after -u is now necessary for that special case name.
Change Qt's 6x3 grid of worn/wielded equipment so that it is facing
the player: hero's right hand side is shown in the grid's left column
and left hand side is shown in its right column. Middle column is
unchanged.
Pull request from argrath: remove a bunch of '#ifdef LINT' code
snippets that no longer serve any useful purpose.
If a lint that handles C99 is ever produced, persumably it won't
need the fairly ridiculous hacks for 'static' and 'long'/'long *'.
Closes#928
Pull request from entrez: don't list undiscovered or unseen (picked
up while blind, still blind) tools as likely candidates for charging.
They're still eligible to be chosen for charging but using a scroll
to charge something else won't reveal not-yet-known tools as being
magic.
Fixes#927
