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
The getobj prompt for charging was presenting any chargeable tool in the
hero's inventory as a suggested charging target, even tools which were
unidentified and undistinguishable from their mundane counterparts
(e.g. bag of tricks, magic harp, horn of plenty...). This leaked
information about the identity of these items and made it possible to
determine whether a generic 'harp' was magic or not.
When suggesting chargeable tools, include only those which are actually
known to be chargeable (unidentified or unseen chargeable tools can
still be selected, they just won't be suggested targets). Basically the
same as what's done for a potion of oil in the apply prompt.
Pull request from entrez: boulders owned by shops could be used up
(plugging hole in floor) or stolen (pushed through unrepaired gap in
shop wall) without cost. Not very common because shops rarely have
boulders in them.
Fixes#925
Pushing a shop-owned boulder out of the shop wouldn't charge the hero
anything. Remedy this (and remove the boulder from the bill if the hero
then pushes it back in). Also tried to handle a couple other uncharged
boulder "theft" scenarios: pushing a boulder into lava or water, into a
trapdoor or hole, or into a level teleporter (various other traps
already charged for the boulder -- it was pretty inconsistent).
I externified onbill() for this, since relying on otmp->unpaid by itself
impossibles if you push a boulder through a gap in a wall between two
adjoining shops.
Short for distu(mtmp->mx, mtmp->my) (i.e. the distance between the hero
and the specified monster), which is a very common use of distu(). The
idea is that this would be a convenient shorthand for it; I actually
thought it (or something very similar) existed already, but couldn't
find it when I tried to use it earlier. Based on the number of uses of
fully-spelled-out 'distu(mtmp->mx, mtmp->my)' replaced in this commit
I'm guessing I just imagined it.
