This fixes a couple of bugs: a long-standing bug in which writing a
scroll by label could fail even if you've already seen a scroll with
that label (due to the game not tracking whether or not you've seen a
scroll if it doesn't have a name); and a somewhat newer bug in which
spellbooks auto-identified by Wizard knowledge were marked as having
been encountered (rather than as known but not encountered).
Breaks save file compatibility, but not bones files.
There was only one point in the code at which this caching was
being done, and it was incorrect: it's possible for the result of
near_capacity to change during a monster turn because monster
actions can change either inventory weight or carry capacity.
The bug was particularly relevant in cases where a character
polymorphed into a slow weak monster gets attacked by a monster
that moves at normal speed: due to the polyform being slow, the
normal-speed monster gets in a lot of attacks and causes a
rehumanization, but due to the polyform being weak, it was
burdened at the start of the monster turn, and so when that
penalty is (due to the bug) applied to the next turn it can
mean that the character misses the next turn too, and may end up
dying as a result.
Using 'i'+menu choice for suit+'T' to try to take off a suit that is
covered by a cloak (or shirt covered by suit and/or cloak) wouldn't
do anything. It should report that you need to take off the outer
garment first and then not take the chosen item off.
There is probably a simpler fix. It took me a long time to figure
where things were going wrong and them cobble this together.
A big chunk of the diff for invent.c is just identation, surrounding
a one-line change there.
If water walking boots haven't been discovered yet and underwater
hero rises to the surface when putting a pair on, discover them.
(Sinking while removing such on water already discovers them.)
gcc has recognized various "magic comments" for white-listing
occurrences of implicit fallthrough in switch statements for
a long time:
The range and shape of "falls through" comments accepted are
contingent upon the level of the warning. (The default level is =3.)
-Wimplicit-fallthrough=0 disables the warning altogether.
-Wimplicit-fallthrough=1 treats any kind of comment as a "falls through" comment.
-Wimplicit-fallthrough=2 essentially accepts any comment that contains something
that matches (case insensitively) "falls?[ \t-]*thr(ough|u)" regular expression.
-Wimplicit-fallthrough=3 case sensitively matches a wide range of regular
expressions, listed in the GCC manual. E.g., all of these are accepted:
/* Falls through. */
/* fall-thru */
/* Else falls through. */
/* FALLTHRU */
/* ... falls through ... */
etc.
-Wimplicit-fallthrough=4 also, case sensitively matches a range of regular
expressions but is much more strict than level =3.
-Wimplicit-fallthrough=5 doesn't recognize any comments.
Plenty of other compilers did not recognize the gcc comment convention,
and up until now the compiler warning for detecting unintended
fallthrough had to be suppressed on other compilers. That's because the code
in NetHack has been relying on the gcc approach, and only the gcc approach.
The C23 standard introduces an attribute [[fallthrough]] for the
functionality, when implicit fallthrough warnings have been enabled.
Several popular compilers already support that, or a very similar attribute
style approach, today, even ahead of their C23 support:
C compiler whitelist approach
--------------------------- -------------------------------------
C23 conforming compilers [[fallthrough]]
clang versions supporting
standards prior to
C23 __attribute__((__fallthrough__))
Microsoft Visual Studio
since VS 2022 17.4.
The warning C5262 controls
whether the implict
fallthrough is detected and
warned about with
/std:clatest. [[fallthrough]]
This adds support to NetHack for the attribute approach by inserting a
macro FALLTHROUGH to the existing cases that require white-listing, so
other compilers can analyze things too.
The definition of the FALLTHROUGH macro is controlled in include/tradstdc.h.
The gcc comment approach has also been left in place at this time.
Issue reported by youkan700: shape change anomalies. Shapechangers
could change shape despite active protection-from-shape-changers if
hero wore two rings of protection from shape changers and took one
off. Shapechangers who migrated to a not-yet-visited level that
eventually got visited with protection from shape changers in effect
would be stuck in their current shape, even if the PfSC attribute
got toggled off and back on.
The issue included suggested fixes and those are what I've used. I
noticed a third case that only applies to wizard mode: if player
used #wizintrinsic to set a timed value for PfSC, monsters wouldn't
resume changing shape after it timed out, unless/until it got toggled
on and back off via a PfSC ring or hero left the level and returned.
Fixes#1312
Fix a FIXME in Amulet_off() for removing an amulet of magical
breathing when within a poison gas cloud.
Redo message sequencing for both Amulet_on() and Amulet_off().
Use up an amulet of change if put on while the Unchanging attribute
is active (via #wizintrinsic) instead of wearing it with no effect.
Don't discover amulet of strangulation if put on while already
Strangled (via #wizintrinsic).
This is additional groundwork related to
https://github.com/NetHack/NetHack/issues/1320
This additional groundwork just puts some safeguards
in place to make it rather tough to end up with an
instant death from handling a cockatrice corpse in
your inventory without appropriate protection.
At this point, still no actual petrification will occur.
GitHub issue #1315 points out that it is possible for
a downstream function to change an object's nobj field
to point to a completely different chain.
The cited example by @vultur-cadens was:
for (obj = gi.invent; obj; obj = obj->nobj)
if (obj->oclass != COIN_CLASS && !obj->cursed && !rn2(5)) {
curse(obj);
++buc_changed;
}
curse() drops the weapon with drop_uswapwep(),
which calls dropx(),
which calls dropy(),
which calls dropz(),
which calls place_object().
place_object alters the nobj pointer, to point to the floor chain:
otmp->nobj = fobj;
fobj = otmp;
The result was that the next loop iteration was then using floor
objects from the floor chain.
This alters several for-loops to use a more consistent approach,
particularly when the obj is being handed off to a function,
where a downstream function might, or might not, alter the nobj
field.
References:
https://github.com/NetHack/NetHack/issues/1315https://www.reddit.com/r/nethack/comments/1gkc9ub/even_if_you_drop_an_item_before_drinking_from_the/
Like stuck-in-wall, there might not be any place to move the hero
in order to escape a poison gas cloud. That could result in "fix
all troubles" becoming stuck in a loop. 3.6.something fixed the
stuck-in-wall case by temporarily conferring Passes_walls. Fix the
in-poison-region case by temporarily conferring Magical_breathing.
Not adequately tested...
I don't think that this fixes "#K4252: NH 3.7 Prayer Bug" where the
game hung during a couple out of scores of prayers. I didn't see
any other fix-trouble cases that seemed likely to remain unfixed
and end up with repair attempts retrying.
Issue reported by Meklon2007: some theft feedback during nymph
attacks refers to the attacker as "she" and others as "it" if hero
is blind.
The "she" references are intentional. However, mixing them with
"it" references when a series of messages occurs is jarring. This
changes "it" to "someone", which is still different from "she" but
hopefully enough less so to be tolerable.
That resulted in monkeys also being referred to as "someone" because
they're classified as humanoid. Change x_monnam()'s AUGMENT_IT
handling, which chooses between "someone" and "something" when the
monster is not seen, to override humanoid for animals (affects 'Y'
class) and for mindless (affects zombies, mummies, and golems).
So an unseen monkey will be "it" again.
The final message for current item was relying on a cached monster
name value. If an unseen nymph or monkey stole a worn blindfold
so that hero's vision was restored: "It <stole> item" before the
changes and "Someone|Something <stole> item" after. So update the
cached name if sight gets regained, to give "<Mon> stole <item>."
(If the item is worn armor and the thief is a nymph, it was and
still is "She stole <item>".)
The message for having any worn item be stolen, which got split
into two parts within the past year, was giving "<Mon> takes off
<alt weapon>". When not dual-wielded, the alternate weapon isn't
really worn. Rather than suppress it outright, change the message
for uwep/uswapwep/uquiver to say "disarms" instead of "takes off".
For accessories, change "takes off" to "removes". Those are more
or less interchangeable these days but "removes" matches R instead
of T.
While testing, my pet evidently killed a nymph (I was blinded and
couldn't see it happen) while she stole my gloves and the next
message I got was "You finish taking off your suit." The gloves
weren't worn anymore so equipname() defaulted to suit. Get rid of
equipname() altogether and switch to armor_simple_name() which
doesn't rely on the worn-armor pointers.
Fixes#1257
The g? structs had a mix of variables that were written to
the savefile, and those that were not.
For better clarity and to distinguish those that end up in
the savefile, relocate some g? variables that get written
directly to the savefile into different structs.
This updates EDITLEVEL, although technically it probably
didn't need to, since savefile contents are not changing.
Details:
gb.bases -> svb.bases
gb.bbubbles -> svb.bbubbles
gb.branches -> svb.branches
gc.context -> svc.context
gd.disco -> svd.disco
gd.dndest -> svd.dndest
gd.doors -> svd.doors
gd.doors_alloc -> svd.doors_alloc
gd.dungeon_topology -> svd.dungeon_topology
gd.dungeons -> svd.dungeons
ge.exclusion_zones -> sve.exclusion_zones
gh.hackpid -> svh.hackpid
gi.inv_pos -> svi.inv_pos
gk.killer -> svk.killer
gl.lastseentyp -> svl.lastseentyp
gl.level -> svl.level
gl.level_info -> svl.level_info
gm.mapseenchn -> svm.mapseenchn
gm.moves -> svm.moves
gm.mvitals -> svm.mvitals
gn.n_dgns -> svn.n_dgns
gn.n_regions -> svn.n_regions
gn.nroom -> svn.nroom
go.oracle_cnt -> svo.oracle_cnt
gp.pl_character -> svp.pl_character
gp.pl_fruit -> svp.pl_fruit
gp.plname -> svp.plname
gp.program_state -> svp.program_state
gq.quest_status -> svq.quest_status
gr.rooms -> svr.rooms
gs.sp_levchn -> svs.sp_levchn
gs.spl_book -> svs.spl_book
gt.timer_id -> svt.timer_id
gt.tune -> svt.tune
gu.updest -> svu.updest
gx.xmax -> svx.xmax
gx.xmin -> svx.xmin
gy.ymax -> svy.ymax
gy.ymin -> svy.ymin
Related note:
There are some pointer variables that are heads of chains that were not
moved from 'g?' to 'sv?', because they are not actually written to the
savefile directly, but the objects/monst/trap/lightsource/timer in the
chains they point to are. That can be changed, if desired.
Examples: gi.invent, gm.migrating_objs, gb.billobjs, gm.migrating_mons,
gf.ftrap, gl.light_base, gt.timer_base
add a static function maybe_destroy_armor() to replace
the DESTROY_ARM() macro.
The DESTROY_ARM() macro expanded to legal code, but did include
side-effects of making assignments to local variable otmp (not an
argument to the macro), referenced local variable atmp (not an
argument to the macro), and sometimes setting the in_use field
on the impacted armor obj.
The assignment statements within the if (...) caused some compilers
and code analyzers to complain and suggest that perhaps
'=' should have been '==', which was incorrect.
The maybe_destroy_armor() function provides the
caller with information about whether the armor resisted,
so that appropriate action can be taken within the caller.
Donning elven boots while riding and not already stealthy, you'd get
the message "you walk quietly" when not walking at all. Instead of
just changing the message, make riding a non-flying steed block
stealth. Riding a flying steed (or one you take aloft with an amulet
of flying) does not. It would have been quite a bit simpler to have
made riding anything block stealth, but the hard part is done.
A couple of things I noticed while running umpteen tests for tty
perm_invent. Remove the update_inventory() from unmul(), and limit
the one that deals with seeing inventory when recovering from blindness.
Just a drop in the bucket overall, and the screen updates nearly
instantly for update_inventory() except when debugging perm_invent so
players aren't likely to notice this.
Issue reported by vultur-cadens: changing helm of brilliance to
crystal made it stop being classified as "hard helmet" so it gave
less protection against things falling onto the hero's head.
Change the is_metallic() tests used on helmets to new hard_helmet().
Unlike when thrown, crystal helmets don't break when objects fall
on them.
Fixes#1060
Discussed a long time ago, change helm of brilliance from iron to
crystal so that it doesn't need to be a special case for metallic
armor's affect on spell casting. It now has a fixed description of
"crystal helmet" but is not pre-discovered.
Add new helm of caution to retain the "etched helmet" description
among the shuffled helms. Wearing it confers the Warning attribute.
That's fairly lame but not necessarily useless. It's iron and gets
half the former probability for a random piece of armor being helm
of brilliance so is not likely to be popular.
Helm of caution keeps the old helm of brilliance tile and new helm
of brilliance is basically the same image with different color and
pointed on the top.
Not changed: the etched helmet was marked as green and is drawn
that way for text but the tile doesn't actually use green for it.
Crystal helmet started as hi_glass (rendered as 'bright cyan') but
has been changed to clr_white to match crystal plate mail.
Old save and bones files are invalidated.
... unless explicitly specified to generate at a specific point or
within a specific area. But if they are permitted to generate anywhere
on the level, and it contains water, they always end up in the water. I
noticed this when trying to explicitly specify ghouls to generate
anywhere on a level with a minimal amount of water.
This was due to the definition of "amphibious" being conflated with
"breathless", such that all breathless monsters counted as amphibious.
There are plenty of breathless monsters in the game that decidedly don't
normally inhabit water, such as undead, but they would pass the
amphibious() check in pm_to_humidity and thus the game decides that they
must generate in wet terrain if there is any available.
This fix takes the approach of changing amphibious() so that it no
longer checks the M1_BREATHLESS flag and only considers M1_AMPHIBIOUS,
then updating the places where amphibious() and Amphibious are used
accordingly. I also added a new macro cant_drown() which wraps up
swimming, amphibiousness, and breathlessness because these three things
are frequently checked together in the context of whether something
should drown.
Places where amphibious() or Amphibious did NOT have an extra
breathless() or Breathless check added on, and thus where behavior has
been changed:
- The pm_to_humidity function (to fix the bug).
- Player vs water in goodpos; it didn't seem like being polymorphed into
a breathless non-amphibious monster should make it fair game to
randomly teleport into water even though it's technically safe.
- Awarding extra experience when killing an eel. (So the hero will get
the extra experience if they are polymorphed into a breathless
non-amphibious monster and don't have magical breathing. Very much an
edge case.)
Aklyses uniquely have a tether attached to one's wrist, which would stay
secure even when one's fingers are slippery. Therefore, it doesn't make
for the weapon to be dropped completely, tether and all, when Glib.
However, the game doesn't have a way to model a weapon that's on the
floor at your feet but still attached to your arm. (Technically the
uball and uchain code does do this sort of thing already, but it can't
be used here - what if you have an aklys slipping out of your grasp and
are punished at the same time?) So the next best thing is to give
aklyses immunity to slipping from one's hand.
Refine the fix #K3888 by moving multiple instances of the check for
cancel_don() from destroy_arm() to one instance in wornarm_destroyed().
Should be no change in behavior.
Report was for spell-casting monster using the destroy armor spell on
hero's levitation boots while hero was floating over lava. The boots
became unworn but still in inventory, hero dropped into lava, the
boots happened to be an inventory item which got burned up, then the
call stack unwound back to the destroy armor routine which tried to
finish by deleting them but they were already gone by then. Could
also happen for black dragon breath, hero reading scroll of destroy
armor, or overenchanting the boots with scroll of enchant armor, so
not so unlikely that nobody would be expected to notice.
Initially I couldn't reproduce the object lost panic. It only happens
if the memory used by the boots is cleared or clobbered during first
time it's freed, otherwise second free doesn't notice any problem.
The 'wornarm_destroyed()' portion of this commit is sufficient to fix
this. The other bits are things I tried before figuring out how to
reproduce it, plus zeroing out any object passed to dealloc_obj().
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
^ ^
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.
This replaces the old pushq/saveq arrays (which were used to save
the keys pressed by the user for repeating a previous command)
with a new command queue. This means there's no hard-coded limit
to the saved keys, and it can repeat extended commands which are
not bound to any key.
Switch to using a macro invocation Verbos(n, s) in place of the
flags.verbose checks.
Provide the mechanics for individual suppression of any of the
existing messages that were considered verbose.
Mechanics only - this code update does not provide any means of
setting the suppression bits.
iflags.verbose = 0
is still a master suppression of all the verbose messages.
iflags.verbose = 1
turns on the verbose messages only for those whose suppression
bit is 0 (not set).
When not wielding anything, ^X reports "you are empty handed" if
wearing gloves or "you are bare handed" if no gloves. The ')',
'w-', and 'A)' commands were using "empty handed" unconditionally.
Make them be consisitent with ^X.
After this, body part HANDED is no longer used anywhere except in
body_part().
There's no 'w-' or 'Q-' for alternate weapon, but context-sensitive
inventory is starting from the object rather than the command so can
finesse that. 'A' does allow alternate weapon to be directly unset
(aka reset to bare/gloved hands) but is not friendly to being passed
queued input.
This adds an extra internal command which only handles unset uswapwep,
even though that is something which is awfully specialized to get it's
own command. Users don't see this command so that shouldn't matter.
