In the context of sanity checking, an extra pass though the inventory
of every monster wielding a weapon is completely negligible, but it
is trivial to avoid so take it out.
Have save_mtraits() clear wielded weapon when attaching monster
attributes to a corpse object.
And have monster sanity check verify that wielded weapon is in the
monster's inventory.
These were unbalancing the game a) in the Castle and b) if they
woke up unique monsters (most notably the Wizard of Yendor).
I considered adding a difficulty check, but this commit instead
just directly fixes the symptoms. (It doesn't make sense for the
Castle to contain a monster that would kill or be killed by its
inhabitants: they should have died long before the hero arrived.
So for liches/zombies to exist in the Castle at all, there must be
a truce.)
Some variants were already using a similar approach
using a struct called 'ebones', so adopt the same naming
so NetHack-3.7, hardfought, and some variants are using
the same name.
As before there are fields in the struct that are not
currently used by NetHack-3.7, but the intent is that
hardfought save and bones files can be loaded by
NetHack-3.7 without code modification, for debugging
bug reports.
This invalidates existing save and bones files.
There are two hardfought code additions that render save and bones files incompatible
with the upstream NetHack-3.7, and that makes testing with hardfought
save and bones files more challenging than it needs to be, when
investigating and troubleshooting bug reports.
Add some unused fields to advance towards achieving save file parity with
hardfought, which is a significant source of play-testing for NetHack-3.7.
1) the elbereth field addition to u_conduct
This adds an unused placeholder field named 'hf_reserved1', at the appropriate
place in u_conduct to achieve struct field parity with the one in use on
hardfought.
2) hardfought adds a field to struct monst:
char former_rank[25]; /* for bones' ghost rank in their former life */
Instead of adding that to every monst, this adds a new mextra struct
named 'former', which currently contains the equivalent 25-character
field called 'rank' which can hold the content that was in the
former_rank[25] field. That way, the field will only be added when it
is needed.
A pull request https://github.com/k21971/NetHack37/pull/2 has been
done on hardfought to do it the same way (untested there as of yet).
Even though NetHack-3.7 does not utilize that information presently,
this will be a further step toward allowing hardfought-generated save
and bones files to be used for troubleshooting, without modification,
on a similar architecture running stock NetHack-3.7 code.
That savefile parity won't be achieved until the after the
hardfought pull-request mentioned above (or equivalent) is merged.
As this change will not be compatible with existing save and bones
files, it will be accompanied with an EDITLEVEL increment.
Previously, the code for monster healing was repeated every time it
was needed; this commit sends it all through a common function, which
will make it easier to make changes to how monster healing works in
the future.
This is just a code reorganisation and won't have any gameplay
effect unless I made a mistake.
Remove the XP gain for tourist seeing a new type of monster
nearby, as it apparently made tourists a bit harder by forcing
early level gains.
Monsters next to hero are still marked as seen close-up, but
fix the code so it doesn't count undetected monsters.
Tourists still gain XP from "taking photos" of new types of monsters,
but only if they haven't seen the monster close up before.
(No actual photos are taken.)
There was a transcription error in the comments in cstd.h for
the standard list of header files, where only the description
remained for <stdlib.h>, not the name of the file itself.
Remove several extraneous inclusions of the standard C99 headers.
Tested on the following afterwards:
Linux (using hints/linux.370) including tty, curses, qt6, and X11
macOS (using hints/macOS.370) including tty, curses, qt5, and X11
Windows MSYS2 using sys/windows/GNUmakefile
Windows Visual Studio using sys/windows/Makefile.nmake
msdos cross-compile on Ubuntu using djgpp cross-compiler
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
If a vampire in fog cloud form moves under a closed door and then
before moving further gets killed and revives in vampire form,
destroy the door instead of moving the vampire to a nearby open
spot (which might be a distant spot if the map is crowded). If the
door is trapped, explode the trap. That will damage the vampire but
usually not by enough to kill it.
This probably ought to be generalized to be done for any shape change
at a closed location but I ran out of gas.
A pet with the hides-under attribute could hide under cursed objects
if it first moved reluctantly to their location. Also, the messages
seem contradictory:
| The cobra slithers reluctantly over a scroll labeled DUAM XNAHT.
| You see your cobra slither under a scroll labeled DUAM XNAHT.
First over, then under, but that was actually accurate; the monster
moved, then after it was on the pile it hid underneath.
Change hideunder() to not let pets hide under an object if it is
cursed or any object in its pile is cursed. Initially I was just
going to check the top item and the item directly beneath it, but
reluctance to move there extends to the whole pile so I made hiding
do so too.
Change the first message to move reluctantly "onto" an object since
"over" suggests that it has continued past the item, unless it is
actually flying or levitating so truly "over" the pile.
A recent change was setting mon->data prior to canspotmon(mon) but
that triggered a crash if the temporary mon->data was Null. All the
testing was done with vampshifers and this affects non-vampshifers.
Push even more shapechange messaging into newcham() by not trying to
determine whether requesting a message from it will oblige.
Messaging for vampires changing into fog clouds was inconsistent
in 3.6.x but already fixed in to-be-3.7.
However, maintaining an extra set of shape change messages (one
in newcham(), the other in vamp_shift() which turns out to only be
used when a vampshifter turns into a fog cloud in order to pass
under a closed door) was a nuisance. Redo vamp_shift() to use
newcham() feedback. Update that feedback to be accessibility-aware
and also to use "Your <mon>" instead of "The <mon>" when appropriate.
While in there, remove a couple of trailing spaces and eliminate
use of one dynamically constructed format string that necessitated
warning manipulation.
Fix misleading indentation that the polyfood() macro inherited from
the misformated polyfodder() macro.
Fix a case where a non-pet monster would avoid eating a cockatrice
corpse but would eat Medusa's corpse.
'Polyfodder' is already a term frequently used by players to describe
items which are useful to hang on to specifically to zap polymorph at
(for example, extra unicorn horns, which can be turned into magic
markers). Using it as the name of a macro which tests whether a food
item will polymorph the creature consuming it is somewhat confusing, and
I think 'polyfood' is a lot clearer.
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
If the quest leader observes the hero attacking a peaceful monster,
only become angry if that peaceful monster is a quest guardian. And
when becoming angry, stop waiting for the hero to approach.
If a quest guardian observes the hero attacking any peaceful monster,
don't run away.
if Wizard escapes the dungeon
Reported by vultur-cadens: a fix to prevent quest feedback when quest
nemesis is removed from the game during bones creation introduced a
regression for an earlier fix that kept context.no_of_wizards up to
date if the Wizard of Yendor escapes the dungeon without dying.
Change 'wizdead()' to 'wizdeadorgone()' and call it from m_detach()
for mongone() as well as for mondead().
Fixes#1256
Issue reported by chadministratorwastaken: were-creature that was
ignoring Elbereth while in human form would make one more attack
after changing into creature form.
Have new_were() make an onscary() check when changing to beast form
while next to the hero. Do likewise for polymorphing creature.
Fixes#1253
Another tweak to PR #1240. Vampires start out vampshifted to
bat/fog/wolf form. Marking them as 'waiting' forces them back to
vampire form. Testing that revealed one of the brides as a vampire
lord rather than a vampire lady. That could probably be construed
as being politically correct but doesn't match the source material.
The reversion from vampshifted to vampire entailed a 10% chance of
toggling gender, comparable to hero self-polymorph. Don't do that
for vampires. From the player's perspective they change randomly but
from their own perspective, the change is controlled.
This makes it easier to understand what is happening in each of those
places, and also much easier to find the places that conversion between
hero and monster resistances happens (since you can now just grep for
"res_to_mr"). I think I got all the instances where the macro should be
used but I'm not 100% sure since there wasn't a single unique term to
search for to find them all.
I replaced the modifications to give_u_to_m_resistances made in the
previous commit, so that it now uses the same macro as other
conversions, since otherwise it would be much harder to find the
complete list of places where conversions between hero and monster
properties happen.
When mnearto() places a monster, it might remove another one to make
room, then try to put that second one back. If it can't do that, it
puts that other monster onto the migrating monsters list. But having
it already be off the map interferred with regular take-mon-off-map
handling that happens when setting up a migration.
If the other monster was a hider or concealed mimic, it wouldn't be
brought out of hiding. With 'sanity_check' enabled, that triggers
a warning when the occupants of migrating_mons get checked.
Testing this requires more effort than I'm willing to expend so there
might be unexpected consequences.
I still haven't found any explanation for the report by a hardfought
player recently that going down some stairs with a pair of leashed
pets got one into a confused state where it was flagged as leashed
but the corresponding leash was no longer in use.
This adds some new object and monster sanity checks regarding leashes,
and it changes o_unleash(obj) to clear obj->leashmon even if/when the
monster can't be found.
It also changes behavior for dipping an attached leash into a potion
of polymorph when that happens to yield another leash--now the new
one will end up being pre-attached.
Take advantage of the ability of some compilers to warn if
not all values are covered in a switch statement, to draw
attention to the need to update make_corpse() when new
monsters are added to NetHack.
This adds explicit entries for the current "default" handling
in make_corpse().
It might be a good idea to review the explicit entries to see
if any of them represent others that are not being handled,
but should be.
To test this, I temporarily reverted 85c86444, and I did receive the
following warning:
mon.c:545:13: warning: enumeration value 'PM_GOLD_DRAGON' not handled in switch [-Wswitch]
545 | switch (mndx) {
| ^~~~
1 warning generated.
