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.
Caught by 'sanity_check': hides-under monster hiding under nothing
after the glob it was under coalesced with an adjacent one. Tricky
to test because dropped, thrown, or kicked globs will always merge
with the one being hidden under. Needed to kill the type of monster
that drops the type of glob since it gets created on the floor so
is eligible to draw in the existing one.
Reported by ars3niy, the curses interface could behave strangely on
the first turn if the 'pauper' option/conduct was specified.
There isn't any definitive flag indicating whether or not the game
has started. Since 'moves' has traditionally been initialized to 1
rather than to 0, there were several instances of
| if (moves <= 1 && invent != NULL)
being used to determine the starting state on the assumption that
once hero has inventory, the game has begun. Introduction of the
'pauper' option made the test for non-Null invent become unreliable.
For paupers, the program would behave as if the game hadn't started
yet until the player finally made a time-consuming move.
This changes compile-time initialization of 'moves' from 1 to 0,
then sets it to 1 when initial inventory would be bestowed (even
when 'pauper' inhibits that). That's probably not the best place
for it, but testing for 'moves==0' now should produce an identical
effect as 'moves<=1 && invent!=NULL' used to accomplish.
It would have been much simpler just to give paupers 1 gold piece,
or perhaps one rock, in place of usual starting gear so that their
initial inventory wouldn't be empty, but the moves+invent way of
checking for start-of-play has always bothered me.
Should 'pauper' be preventing 'nethack -X' from giving its starting
wand of wishing? Conducts and explore mode don't really overlap so
maybe it doesn't matter.
Fixes#1275
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
The list of possible object locations used when formatting obj->where
wasn't updated when the objs_deleted list was introduced. If object
sanity checking ever tried to report it for something, it would have
been described as "unknown[9]" rather than as the intended "deleted".
Give a different message from "obj not free" if attempting to delete
an already deleted object. Also, skip sanity checking of in_use/
bypass/nomerge bits for deleted objects instead of clearing them when
going onto the objs_deleted list.
Teach obj_sanity_check() and clear_bypasses() about the new obj list.
It should always be empty when sanity checks are performed. That
might not be the case when obj bypasses are cleared, although failing
to clear bypasses for deleted objects wouldn't make any difference,
so this is mainly cosmetic.
Make object deletion work similarly to monster deletion:
it's marked for deletion (by setting the where-field to OBJ_DELETED
and moved to specific deleted-objects chain), but they're actually
freed at the beginning of turn.
This may need some more tweaking, especially in places that iterate
over object chains, but fuzzing did not find any obvious problems.
Fix a case of accessing freed memory: a monster breathed at hero,
destroying some items. The code stored the next item in the chain
(a cloak), but a ring of levitation was destroyed, causing hero to
plop down into lava, destroying the cloak. The item destruction
code then tried to access the destroyed cloak object.
Make the code check the object where-field - which will be different
if the object was marked for deletion. Also removed an extra loop
going through the whole object chain looking for the items to
destroy.
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.
Add a way to request that unpaid_cost() produce the cost for a single
item, which is necessary for the price adjustment made in
bill_dummy_object. Another option would be to simply divide by quan in
bill_dummy_object, but this might be more future-proof in case
unpaid_cost ever involves more than simple multiplication by quan
(e.g. the use of alternate units vs the base price, as are used for
globs).
Fixes#1236
Update some potential weight issues. Eggs won't hatch when in
containers so they weren't affected but add some bulletproofing.
Corpse revival from inside containers was already ok too, so
effectively there's no change except for making container_weight() be
global instead of local to mkobj.c.
Changing the quantity to 2 (50:50 chance) when creating a potion of
healing (also 50:50 chance for each attempt) to place inside a supply
chest wasn't updating the potion stack's weight, resulting in the odd
encumbrance behavior that was reported last December.
Taking the stack out of the container doesn't fix the weight but
drinking one of the potions splits the stack of 2 into two stacks
of 1 and does update the weight for both. That gives the hero higher
encumbrance when the formerly weightless one has its proper weight.
Finishing drinking the potion uses it up, removing second potion's
weight again. When below an encumbrance threshold by the weight of
one potion or less, player will see encumbrance increase and then
decrease, with healing message given before both due to sequencing.
Supply chests weren't having their own weight updated when they were
populated, so would behave as if empty if hero carried them around.
Removing something, breaking something by kicking the chest, or adding
something would update its weight to match its contents.
I also noticed a refutation (or should that be rebuttable?) to my own
remarks in this:
| commit cd91d0630b
| Author: PatR <rankin@nethack.org>
| Date: Sat Dec 30 17:10:39 2023 -0800
|
| github issue #1180 - humans and murder
|
| Issue reported by Umbire: reviving a human corpse into a human
| monster and then killing it entails murder penalty even when it is
| hostile.
|
| This is probably a non-issue. Human monsters tend to not leave
| human corpses, they leave shopkeeper corpses or sergeant corpses
[...]
Dead fake hero corpses placed at trap locations on early levels are
leaving plain human|dwarf|elf|gnome|orc corpses rather than fake
player monster ones (which are always human but resurrect as player
monsters rather than as plain humans), so there are more plain human
corpses now than there were in 3.6.x or early to-be-3.7. I've added
a comment about the situation.
Issue reported by AmyBSOD: several actions change the object type of
a potion rather than force creation of a replacement one, and if/when
the type was changed to oil, the age wasn't converted from absolute
to relative. Relative age is the amount available and/or the number
of turns it will burn if applied. The later in a game a potion got
converted into oil, the longer it would burn. Not mentioned: reverse
situation was also the case, although that didn't have any noticeable
effect since incorrect absolute age of former oil doesn't matter.
Not thoroughly tested. I got a potion of oil from a horn of plenty
and it burned for 400 turns, but it might have been created directly
rather than be a rejected magic potion that was converted into oil.
Closes#1191
src/mkobj.c(419): warning: '((obj2))->oextra->omonst' could be '0'
: this does not adhere to the specification for the
function 'memcpy'.
src/mkobj.c(421): warning: Dereferencing NULL pointer
'((obj2))->oextra->omonst'.
See line 419 for an earlier location where this can occur
The analyzer was not aware that newoextra() sets up an oextra block:
if (!obj2->oextra)
obj2->oextra = newoextra();
The analyzer was also not aware that newomonst() was setting up a valid
OMONST pointer.
if (!OMONST(obj2))
newomonst(obj2);
Add an assert(has_omonst(obj2)) before copying the content from
OMONST(obj1) into OMONST(obj2).
Add pickup_stolen option to autopick items stolen from you by a nymph or
monkey, even if they don't match your normal autopickup settings.
Replace was_dropped, was_thrown with a 2-bit bitfield that can contain
values LOST_DROPPED, LOST_THROWN, and LOST_STOLEN (or 0), since they
should all be mutually exclusive anyway as they track the most recent
way the item left the hero's inventory.
[Rebase/merge conflict fixed up. PR]
From a reddit thread: statue weight is one and half times corpse
weight and some monsters that don't leave a corpse are defined as
having mons[].cwt==0 so statues of those weighed nothing.
Rather than assigning a non-zero corpse weight to every type of
creature, statues that weigh less than an arbitrary value based on
monster size have their weight increased to that value. The weight
of a statue of a killer bee jumps from 1 to 100, that of a leprechaun
increases from 90 to 100, that of a yellow light is changed from 0
to 300, wraith from 0 to 500, and air elemental from 0 to 900. That
last one is actually too low but making the formula more complex
doesn't seem worth it.
Instead of a 5% chance for crystal plate mail or crystal helmet to
break each time it's subjected to breakage, switch to a 10% chance
but the damage is treated as erosion rather than break/don't-break.
'crystal foo' will need to go through four stages of damage before
breaking: cracked crystal foo, very cracked crystal foo, thoroughly
cracked crystal foo, then gone. Crackproof handling is included,
described as tempered crystal foo.
It mostly still applies to throwing and kicking the item. Having
some hits trigger damage might be worthwhile but isn't implemented.
Object creation within lua code probably needs to be updated, and
when the Mitre of Holiness is created in the priest/priestess quest
it should start out as tempered (erodeproof). Perhaps it ought to
be erodeproof regardless of where/how it's created.
Making the zeroing out of memory used by an object that is about to
be freed unconditional, and do the same for monsters. Should never
matter aside from an undetectable amount of extra overhead.
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().
sound_verbal(char *text, int32_t gender, int32_t tone, int32_t vol,
int32_t moreinfo);
-- NetHack will call this function when it wants to pass text of
spoken language by a character or creature within the game.
-- text is a transcript of what has been spoken.
-- gender indicates MALE or FEMALE sounding voice.
-- tone indicates the tone of the voice.
-- vol is the volume (1% - 100%) for the sound.
-- moreinfo is used to provide additional information to the soundlib.
-- there may be some accessibility uses for this function.
It may be useful for accessibility purposes too.
A preliminary implementation has been attempted for macsound to test
the interface on macOS. No tinkering of the voices has been done.
Use of the test implementation requires the following at build time with make.
WANT_SPEECH=1
That needs to be included on the make command line to enable the test code,
otherwise just the interface update is compiled in.
I don't know for certain when AVSpeechSynthesizer went into macOS, but older versions
likely don't support it, and would just leave off the WANT_SPEECH=1.
If built with WANT_SPEECH=1, the 'voices' NetHack option needs to be enabled.
It was a bit strange, when I first started up the test, to hear Asidonhopo,
the shopkeeper, talking to me as I entered his shop and interacted with him.
Issue reported for a hardfought player by k2: dying in a shop wall
produced "place_object: <item> [0] off map <0,0>" when hero's invent
was dropped. It happened in Mine Town where multiple shopkeepers are
present and it is possible to have two shops share a wall.
I could not reprouce the problem, even after setting up--and dying
various times at a gap in--a wall shared by two shops.
paybill() -> inherits() -> set_repo_loc() sets up the destination
prior to disclosure and finish_paybill() -> drop_upon_death() later
places invent at the spot iff bones are going to he saved. inherits()
is convoluted and evidently took at least one path that failed to
call set_repo_loc(). Change it to always call set_repo_loc() when
returning 'True' so that the destination should always be set if
really_done() calls finish_paybill().
Some followups by entrez are probably still useful.
Closes#965
Items in initial hero inventory, or generated via lua in
special levels or themed rooms are not subject to this.
Code via xnethack by copperwater <aosdict@gmail.com>,
with some modifications.
Insert the calls to trigger a number of potential soundeffects
into the core.
If no additional soundlib support is integrated into the
build, then the Soundeffect macro (sndprocs.h) expands to nothing:
[#define Soundeffect(seid, vol)
]
If, however, at least one additional soundlib support is integrated
into the build, then the Soundeffect macro gets defined as this
in sndprocs.h:
[#define Soundeffect(seid, vol) \
do { \
if (!Deaf && soundprocs.sound_soundeffect \
&& ((soundprocs.sndcap & SNDCAP_SOUNDEFFECTS) != 0)) \
(*soundprocs.sound_soundeffect)(emptystr, (seid), (vol)); \
} while(0)
]
That macro definition checks for the hero not being Deaf; it checks
to ensure that the active soundlib interface has a non-null
sound_soundeffect() function pointer; and it checks to ensure
that the active soundlib interface has declared that it supports
soundeffects by setting the SNDCAP_SOUNDEFFECTS bit in its sndcap
entry. That just means that the interface routines are prepared to
accept and deal with the calls from the core, whether or not it
actually produces the desired soundeffect.
Try to fix a fuzzer issue. I wasn't able to reproduce it so am not
sure whether this actually fixes it. A mimic seemed to be mimicking
object #1 (generic ILLOBJ_CLASS object which shouldn't occur) rather
than #0 (strange object). Strange object always has dknown==1 and
generic objects should always have dknown==0 but farlook of mystery
object #1 had its dknown flag set.
An earlier fix to force non-Null oc_name when formatting objects in
order to pacify the static analyzer might have been the reason that
the problem couldn't be reproduced.
This includes a few miscellaneous changes made while unsuccessfully
hunting for the problem.
