Reverts 690e072, which changed the various is_foo macros from this:
| #define is_elf(ptr) ((((ptr)->mflags2 & M2_ELF) != 0L)
to this:
| #define is_elf(ptr) ((((ptr)->mflags2 & M2_ELF) != 0L) \
| || ((ptr) == g.youmonst.data && \
| !Upolyd && Race_if(PM_ELF)))
This is a problem because g.youmonst.data is not unique to the hero:
the '(ptr) == g.youmonst.data' test will also be true of all player
monsters of the same role. For this reason, any of those player
monsters will be treated as sharing the hero's race, producing strange
results. For example, if the player is an elven ranger, any ranger
player monster generated will be considered 'elven' too (so will get a
to-hit bonus when attacking orcs, etc) -- but only while the hero is
unpolymorphed.
There are already other ways of checking the hero's race in addition to
her current polyform, most notably the maybe_polyd() macro. maybe_polyd
or something similar is already used in nearly all the cases where the
hero's race is being evaluated, meaning Race_if gets used instead when
the hero is in her natural form. So I think the check of the hero's
race in is_foo had very little effect except for the unintended
side-effects on player monsters.
In reviewing all the uses of is_{elf,dwarf,gnome,orc,human}, I noticed
only one case that relied on the hero-race-checking behavior. That has
been changed in this commit to use maybe_polyd (there's another 'raw'
is_human(g.youmonst.data) a few lines down, but it doesn't need
maybe_polyd since it already distinguishes between 'hero in nonhuman
polyform' vs 'nonpolyd or human polyform'). same_race(mondata.c) is
another case where &g.youmonst.data can be passed to is_foo, but
everywhere that calls it for the hero also calls your_race() or
same_race(&mons[Race_switch]) to handle the racial case.
Make changes similar to the suggested patch from entrez: support
for 'youmonst' as the monster passed to m_carrying(). This doesn't
change carrying(otyp) to call m_carrying(&g.youmonst,otyp) though.
Also, treat being on the Plane of Air or in an air bubble on the
Plane of Water similar to flying or levitating: wielded Giantslayer
(or carried loadstone) doesn't prevent knockback there.
The #wizborn command shows 'E' for an extinct species and 'G' for a
genocided one, but if a species first becomes extinct and then later
gets genocided, instead of showing both flags it stopped showing
either. I was going to add a second flag column and show 'E' and 'G'
separately but decided to stick with one column and display 'X' for
the unlikely 'both extinct and genocided' case.
More PR #882; give a different message for empty input than "such
creatures do not exist".
The new message mentions 'none' (with single quotes) as a potential
choice so recognize "'none'" as well as "none" to decline.
If someone gets hit for a knockback effect but resists it due to
wielding Giantslayer or carrying a loadstone, give feedback saying
so, otherwise the lack of knockback is indistinguishable from an
ordinary hit.
The message is not likely to appear for non-hero target since that
target needs to have special equipment. A hero wielding Giantslayer
might see it enough for the player to become annoyed; if so,
MSGTYPE=hide could be used to suppress it.
mhitm.c: In function 'hitmm':
mhitm.c:583:30: warning: '%s' directive writing between 8 and 9 bytes into a region of size between 0 and 255 [-Wformat-overflow=]
583 | Sprintf(buf, "%s %s", magr_name,
| ^~
In file included from ../include/config.h:671,
from ../include/hack.h:10,
from mhitm.c:6:
../include/global.h:279:24: note: 'sprintf' output between 10 and 266 bytes into a destination of size 256
279 | #define Sprintf (void) sprintf
mhitm.c:583:13: note: in expansion of macro 'Sprintf'
583 | Sprintf(buf, "%s %s", magr_name,
| ^~~~~~~
Issue reported by vultur-cadens: one of the checks for whether a
shade would be harmed by an attack was erroneously inside a block
of code that only executed when you could see the attack. Basic
physical damage wasn't affected but some monster (or poly'd hero)
damage types that shouldn't affect shades didn't when seen but did
when unseen.
Could also get "attack passes harmlessly through the shade" when
an unseen attack for physical damage hit and failed to deal damage.
fixes#907
There was a TODO about this; not exactly a great challenge but it feels
like a worthwhile change since the name was misleading. I also updated
the name of the do_intrinsics parameter of extract_from_minvent(worn.c),
since it was in a similar situation (and directly related, since it
controls whether to call update_mon_{in/ex}trinsics).
Specifying a count of 1 when throwing an object could leave you with two
stacks sharing one inventory letter. The second stack gets split off
when the player specifies a count (e.g. 't1o'), but keeps its original
invlet. Some early returns, like trying to throw at yourself with '.',
could fail to unsplit the stack. Theoretically, specifying multiple
items to multishot and then failing to throw them all could also leave a
partial stack; I don't think this is actually possible right now with
't' but I tried to make sure it won't become a problem if greater counts
than 1 are ever allowed.
The fix doesn't affect 'f', which can be a combined "create a quiver
stack and throw" action and doesn't have the issue with duping invlets.
Specifying a count to split off a new quiver stack with 'f' shouldn't be
reverted if the throwing fails or only part of the stack is thrown,
because the newly created stack may be intended for continued use as the
quiver in future turns. This slightly changes the behavior of the
existing unsplit when cancelling the throw (which previously unsplit the
newly created quiver and quivered the entire parent stack), but I think
this actually makes more sense -- the player only declined to throw the
new stack, not to create it (as if they canceled earlier in the action).
I routed a couple early returns through the stack unsplitting that
shouldn't actually need it (like Mjollnir and welded items) for
consistency's sake; I don't think it hurts anything.
Pull request by Theyflower: carrying a loadstone prevents big
monsters from hitting their target for knockback effect, same as
wielding Giantslayer.
The PR code needed fixing (unintended switch from 'otmp' to 'obj')
so I didn't use the commeit. The PR code also required that the
loadstone be blessed which sounds nethackish but would mean that
nobody would ever notice. Allow carrying any loadstone to prevent
being knocked back. It will still be a rare accident or uncommon
tactical decision. (It doesn't happen if the target is flying or
levitating because those checks deliberately come first.)
supersedes #906closes#906
A giant mummy starts out with a mummy wrapping but couldn't wear it.
Allow humanoids who are bigger than human size (including poly'd hero
when applicable) to wear such cloaks. They won't do so if they are
invisible and the cloak would let hero start seeing them.
Some routines return ECMD_TIME|ECMD_CANCEL (for instance when 'a'pply
wields an item and player cancels the attempt to use it) so change
drop_menu() to test that properly. I don't think drop() ever returns
that combined mask value but be prepared to handle time passage if it
ever does.
When not carrying any contained gold, or the only contained gold is
inside container(s) whose contents aren't known, ^X writes one line
about the hero's "wallet". When known contained gold is present, it
writes two lines for gold, first one about wallet with the second
one about contained gold being a continuation of the first. Move
the conjunction that combines them from the start of the second line
to the end of the first.
So change
|Your wallet contains M zorkmids,
|and you have N more contained in your pack.
to
|Your wallet contains M zorkmids, and
|you have N more contained in your pack.
and
|Your wallet is empty,
|but you have N zorkmids contained in your pack.
to
|Your wallet is empty, but
|you have N zorkmids contained in your pack.
It evens out the line lengths a little bit and starting the second
line with uncapitalized "you" seems slightly less jarring than with
"and" or "but".
Reported by entrez: using ^A instead of #retravel after interrupted
travel can pick wrong location if cursor was previously positioned
with movement commands rather than feature targeting because it
won't be starting from the original spot. Also, ^A after ';' will
just redescribe whatever was examined previously instead of having
the player pick a new spot.
This suppresses cursor positioning from the do-again queue so that
repeating travel or quick-look or other command that needs player
to choose a position will repeat the command but then need to have
a position chosen. For interrupted #travel, the cursor will already
be placed on the previous destination so that's relatively painless,
but also allows a different destination to be chosen.
It adds iflags.remember_getpos that callers of getpos() could set to
be able to restore the old behavior but none do so far.
Fixes#905
Issue reported by vultur-cadens: Elbereth used to be effective in
inhibiting monster movement when an object was present on the same
spot, but since 3.6.0 it isn't. It only functions that way when the
hero--or hero's displaced image--is present these days. So special
levels that have been using engraved Elbereth to try to protect
objects from monsters haven't been providing any useful protection.
This makes Elbereth that's engraved during level creation work like
it used to in 3.4.3 and earlier: when there's at least one object
on the engraving's spot, monsters who are affected by Elbereth will
be affected. [I'm fairly sure that that behavior started out
unintentionally, as a side-effect of an optimization to only check
for scroll of scare monster when there was at least one item present
which is a necessary condition for such a scroll.]
Old-style Elbereth includes Elbereth chosen as a random engraving
during level creation in addition to engravings specified in special
level definitions. Engravings by the player don't have the required
attribute and player-engraved Elbereth behaves in the 3.6 way.
This ought to be replaced by something more general. Perhaps a new
engraving type not usable by the player?
Fixes#900
Shorten the name of the recently added debug command that validates
monster difficulty values. 'wizcheckmdifficulty' was 19 characters
long, the next longest is 14 ('wiztelekinesis'). The extra width
messed up the Qt interface's extended command selection dialog when
wizard mode commands are included. It sizes the button for every
command to fit the longest name; the increase in size from 14 to 19
made the button grid become too big for the screen.
Add monsters' base difficulty level to the #wizmondiff output.
Add #wizmondiff and #wizdispmacros to 'wizhelp'.
Force windowtype to be the first option written to new RC file since
its value can affect how other options are processed. (Only saved if
comes from existing RC file, not command line.) doset() lists a few
compound options before the rest too. Combine the two sets of want-
to-be-first and move the handling for that to optlist.h where the only
cost is that the options are no longer in alphabetical order.
I hadn't ever used #saveoptions before and when I checked to see
whether the autounlock:none changes were being handled properly, I
discovered that options set via 'm O' weren't being handled at all.
This includes some miscellaneous reformatting of things noticed
while tracking down the problem.
Because the existing error was the default case in a switch/case
statement only reachable if the option matched one of the expected ones
in the list, it wasn't actually reachable: something totally out of
left-field wouldn't match one of the expected options so never hit the
switch, and something that did match one of the expected options would
by definition have a first character handled by one of the cases in the
switch/case.
Do it a slightly different way that should successfully raise an
unexpected value error for 'OPTIONS=autounlock:foobar'. I didn't remove
the default case entirely, because it could still catch an error if
some new value is added to unlocktypes[] without a corresponding case
being added to the switch statement.
The autounlock handler included an explicit 'none' option, a choice that
gave it a different UX from similar existing compound option handlers
(e.g. paranoid_confirm or pickup_types), which set 'none' simply by
deselecting all options. It didn't make the menu any easier to use (at
least in my experience), since in order to go from some combination of
options to 'none', you'd have to deselect everything anyway (which on
its own was enough to set 'none', so there was no reason to explicitly
select it after doing so).
Make the autounlock handler work like other compound option handlers,
such that deselecting all options is the way to set 'none', and there is
no explicit 'none' option included in the list.
The line got a lot longer than most other #attributes lines when the
hero had gold both in open inventory and in stashed containers, so break
it up into two lines (using the same approach as the pantheon info in
the first section). Maybe this isn't necessary but it does make it
stand out less.
The #showgold command now mentions (known) gold socked away in
containers in your inventory as of 706b1a9. Since the gold info in the
attributes display and dumplog matches the output of #showgold
otherwise, update it to do the same thing. Also refactored doprgold a
bit to be a little more compact, as opposed to enumerating all the
different combinations of gold/no gold in open inventory/containers.
This eliminated some string constants that were broken up into multiple
constants/lines (like "line 1 " "line 2"), which NetHack code style
seems to prefer to avoid.
Add a stack of 2 tins of spinach near the leader on the monk quest
start level and another stack of 2 blessed tins of spinach at a
random spot on the monk quest locate level, to compensate for the
inability to gain strength from giant corpses if they adhere to
vegan or vegetarian conduct. paxed supplied the 'tinplace' magic.
4 tins of spinach aren't nearly enough to get to 18/100, but by
uncursing the first pair, if necessary, and waiting until strength
is at least 18, they can be eaten to add 4..40 (average 22) points
of exceptional strength. (Players choosing either of those conducts
for other roles or foodless for any role are on their own as far as
boosting Str goes, same as before.)
The special level loader needed to be modified to handle tins of
spinach. It now accepts "spinach" as a fake monster type for an
object of type "tin". Also added support for empty tins since it
involved the same code, and use of fake monster type "empty" with
object type "egg" to be able to create generic (unhatchable) eggs.
(Wishing for "egg" produces those by default but it also accepts
explicit "empty egg" by coincidence.)
Handle alternate values for hero poly'd into a 'strongmonst' form
more thoroughly by propagating max values other than 18/100 to the
attribute manipulation routines.
ATTRMAX(A_STR), which used to be a relatively simple expression, now
contains a function call.
Along the way, change the races[] terminator's value for 'mnum' from
0 (giant ant) to NON_PM.
When strength loss is so big as to cause HP damage, but reduce
strength if the damage causes hero to revert to normal form. There's
no point in adjusting strength before rehumanization and not fair to
do so afterward.
Also, validate strength and its intended adjustment before doing
anything else. (Just paranoia; there's no reason to suspect that any
bad data ever gets passed in.)
Try again to make losestr() do what's intended. If it would take
strength below 3, it takes away HP and max HP instead. If hero is
poly'd, those come from the hero-as-monst values. If hero was
poly'd but isn't any more, hero-as-monst died and rehumanized as
previous self; leave max HP alone. If hero wasn't poly'd, take
HP and max HP from their usual values, but don't take max HP below
the threshold of minimum max HP (experience level times 1). The old
check for max HP going below minimum can't happen anymore, unless
hero was below that threshold already (which shouldn't happen; if it
does somehow, don't punish hero further).
If this still isn't right, I'll throw up my hands and my lunch.
Refine pull request #802 by entrez. Applying damage within a loop
could potentially damage the hero multiple times, maybe using up
an amulet of life saving and then killing hero anyway, or causing
rehumanization and taking further HP from normal form, or both,
causing rehumanization and then using up amulet of life saving.
Accumulate the damage in the loop and then apply it as a unit.
src/mkroom.c(1068) :
warning C4701: potentially uninitialized local variable 'insidex' used
src/mkroom.c(1070) :
warning C4701: potentially uninitialized local variable 'insidey' used
The warning is because the insidex and insidey variables only get
assigned a value conditionally within a for-loop, but contain random
values if that for-loop is not executed, and they are used unconditionaly
later on in the code.
Initializing them changes that from containing random values to containing
zeros, whether that is appropriate or not.
In this particular case, insidex and insidey look to be riding on the coat
tails of insidect and there is a check for invalid insidect in the code,
so that should catch the situation.
Reported by every for xNetHack. This bug is latent in vanilla, but can
easily start to present itself if themed rooms of a certain shape are
added. Ultimately, it comes from an assumption that shops will always be
rectangles of at least size 2x2, and the shopkeeper will always be able
to step diagonally backwards from their normal position just inside the
door in order to get out of the player's way.
Themed rooms introduce the possibility of shops where the shopkeeper has
only 1 square adjacent to their normal position to move to -
effectively, the shop entrance is a narrow corridor. When this happens,
they have nowhere to go to allow the player to enter or leave the shop,
leaving it permanently blocked unless the hero teleports or falls in or
out.
This fixes that by adjusting the shop algorithm to detect when a shop
candidate room is set up like this, and excludes it from becoming a
shop.
A monster which has grabbed you could move away without becoming unstuck
if it hit the "move and shoot" or "helpless" conditions in the dochug
MMOVE_MOVED case (since those lead to early return or break), leaving
the hero stuck to a monster which is no longer adjacent. Put the
'grabber moved away -> become unstuck' stuff at the top of the block so
that it will always be evaluated if a grabber has moved.
I would have liked to move the whole "grabber checks" block up, but I
think it'd change behavior to have the u.uswallow attack come before the
early return for a helpless monster, so I split it up instead.
Min Str is typically 3 no matter the hero's race, but could be higher
(at least in theory?). Using ATTRMIN makes losestr respect the same
minimum as other kinds of attribute loss (I'm operating under the
assumption that this wasn't an intentional move to fix the minimum at 3
regardless of other factors).
Since losestr and losehp calls go together most of the time, this feels
like it probably makes more sense than repeating the killer name/format
twice in a row all over the place.
Remove callers' responsibility to deal with possible hero death when
calling losestr. This is less fragile and error-prone than leaving it
in the caller's hands, but it means that death from the monster spell
'weaken target' no longer goes through done_in_by, and the death reason
is no longer "killed by <monster name>".
Killer format isn't a boolean, since it has 3 possible values
(KILLED_BY_AN, KILLED_BY, NO_KILLER_PREFIX). It shouldn't make any
difference behind the scenes, but it's confusing to use 'boolean' for
it.
...could leave hero in creature form with negative u.mh
losestr can subtract HP, but doesn't directly kill its target. The
caller is responsible for possibly killing the hero if losestr reduces
her HP to 0 or lower; most callers do this by combining losestr with a
losehp call, which can kill off the hero if necessary.
MGC_WEAKEN_YOU calls done_in_by if u.uhp < 1 after losestr, but didn't
handle the Upolyd u.mh case, so could leave a polymorphed hero with
negative health. Add a rehumanize call in that case.
This could also be done by changing losestr to call losehp itself for
the HP loss it deals out, but this would interfere with
cast_wizard_spell's use of done_in_by to generate the death reason:
either all strength loss is described one way ("terminal frailty" or
something -- not great) or else losestr must be passed a death reason
and is described a different way than other attack spells (because it
wouldn't go through done_in_by).
Monster purple worms can now gain intrinsics from swallowing foes whole,
so maybe the hero should be able to do so too. Intrinsics aren't
granted immediately upon swallowing (that would probably have been
easier), but only once a corpse is created and then entirely digested.
I'm not sure if this is too powerful and was being avoided deliberately
for that reason, since it includes potential level gain from wraith
corpses in addition to other intrinsics. That's consistent with monster
purple worms but may be a bit too much in the hands of the hero, though
it is limited by needing the corpse creation roll to succeed.
