Clear "next" boulder so that when pushing a pile of boulders, only
the first message for each of the 2nd, 3rd, &c will be formatted as
"next boulder". If any of them trigger additional messages, those
messages will use normal "boulder".
This cleans up analyzer feedback in shk.c, based on the set of
warnings specified by hints/MacOS.370 and the lower level hints it
applies, rather than anything specified during periodic 'onefile'
processing.
shk.c is the only file I've analyzed, to try again to figure out
how to suppress the old complaint that has been causing a special
case in Genonefile. It isn't triggering during my testing but that
might be due to something in use by onefile but not by normal hints.
I'm running ccc-analyzer from llvm-16; I don't know whether that
matches Genonefile.
Use ESHK(shkp)->bill_p consistently. The bill[] array field is used
to initialize the bill_p pointer field rather than be used directly
when manipulating shop bills.
While in there, get rid of some '#if DUMB' from pre-standard C days.
This time the no_charge object was being carried by a pet.
To trigger, drop an object into shop, decline to sell it,
pick up an object belonging to shopkeeper, decline to buy it,
wait for your pet to pick up the no_charge object you dropped,
teleport out of the shop, wait for shopkeeper to walk out of
the shop.
Sanity checking was complaining about a no_charge obj in untended shop.
Angry shopkeeper was accepting thrown items as no_charge objects:
To reproduce the impossible, kick down the shop door angering
the shopkeeper. While the shopkeeper is still in their shop,
throw an item they don't want into the shop. Wait for the shopkeeper
to get out of the shop.
Move the anger checking before the sell auto-accept code,
so the shopkeeper will charge for the object.
Aka issue #1339 take II
For hero-owned container with some unpaid items, the itemized
shopping bill had a spurious index into the traditional shopping
bill (since it wasn't in that bill due not being unpaid). When mixed
with unpaid items that weren't in the container, that could cause
bill corruption while updating the traditional bill during payment,
leading to impossible warnings.
Fixes#1339Fixes#1350
Issue reported by ars3niy: with the relatively new container
handling, buying multiple items when some were inside a container
sometimes triggered impossible "unpaid_cost: object wasn't on any
bill" warnings and not buy all intended items. Once that occurred,
subsequent inventory display would repeat the warning.
A couple weeks back, I managed to produce a save file which would
reproduce the problem when 'p' was issued, but failed to figure
out why that was happening. I accidentally deleted that save file
and it took quite a lot of further attempts to get another one.
I still don't understand why this fix is needed, but with it in
place the save file no longer triggers any problems. I'm marking
the issue fixed but that could be premature.
Fixes#1339
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.
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
Since save and bones files just got clobbered, make another clobbering
change. The 'queuedpay' field added to shop's ESHK(shkp)->bill[] was
replaced last week by a similar field in the transient itemizing bill.
At the time, the old field was left in place to avoid incrementing
EDITLEVEL.
shkp->mextra->bill[] is saved and restored, so its queuedpay field was
too. Eliminate that no longer used field.
Unrelated: bill[]->useup is declared as boolean but being assigned 1
or 0. Change the assignments to use TRUE or FALSE.
Finish shop changes begun in 2674a9904d.
Fix the longstanding bug where shop paying with itemized buying would
reveal container contents if any unpaid items were inside containers,
regardless of whether the containers' contents were known yet, even
when the container was a locked box/chest or a cursed bag of holding.
Paying by menu made that be more noticeable but it has been present
ever since itemized paying was introduced. I can't find any old bug
reports for it though. I did find an old message of mine that claims
it's in bugzilla with a "#Hnnnn" tag.
This changes how buying containers and their contained items behaves.
It's now an all or nothing operation. Itemized billing will list
the container but not contents, and to buy what is inside you need
to pay for the whole thing as a single unit. If the container itself
is unpaid then its price is part of that item's total cost. If it is
hero-owned than it is listed as an item to buy but doesn't increase
the cost derived from its unpaid contents. (If you decline to pay,
hero will still own the container and still owe for unpaid contents.)
Issue reported by ars3niy: unpaid containers containing one or
more other unpaid items appear on the menu for 'p' along with
separate menu entries for those other items. Buying the container
buys the contents too, then if any of the contents were also picked
in the menu, an attempt will be made to pay for them separately.
Since they are no longer on the bill by that point, that triggers
an impossible warning.
This fixes that by paying for the container but not its contents.
(Temporarily, until more extensive changes get implemented.) Then
those contents will still be on the bill. It they've been chosen
in the 'p' menu, paying for them will work as expected.
This also fixes the pay menu for the case where the bill contains
any instances of a partly used up portion of some stack and also
the unpaid intact portion of the same stack. With itemized billing,
you are allowed to buy the used up portion separately, then maybe
drop the unpaid portion. (If you try to pay for the intact portion,
the shk won't accept the payment and will tell you to pay for the
used up portion first.) With the recently added menu for billing,
they were lumped together as a single item and you had to pay for
their whole stack.
And it fixes a much older bug dealing with the cheapest item on
the shop bill. If you don't have enough to pay for that, the rest
of buying gets skipped. But stacks that had used up and intact
portions were lumping those together instead of separately checking
the two portions for possibly being the cheapest, so it was possible
to have enough gold to pay for one portion but be told that you
couldn't affort to pay for anything. If it was the intact portion,
you wouldn't be able to buy it anyway, but if the cheapest item was
used up portion, being told you didn't have enough gold was wrong.
This commit does not fix the longstanding bug that itemized billing
reveals the contents of containers which haven't been opened. It
was intended to do so but I've run out of steam.
(There is groundwork for that, where buying a container would
include payment for its unpaid contents without revealing what those
are, and they couldn't be purchased separately unless they get taken
out of the container. Uncommenting '#define CONTAINED_BUYING' will
enable it, with updated pay menu handling but without being able to
pay for non-empty containers.)
Fixes#1249
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
Note: Original change is from xNetHack by copperwater <aosdict@gmail.com>,
but this commit comes from HACKEM-MUCHE by Erik Lunna, with
some minor code formatting.
From xNetHack commit a0a6103bea:
'The original goal: nerf item destruction using a method I initially
proposed for SpliceHack, in which the number of items subject to
damage from any single source is limited by the amount of damage the
effect caused. The intent was to be more fair all around and prevent
aggravating situations where, for instance, a chest shock trap zaps
you for 4 damage and immediately ten of your rings and wands blow up.
Problem 1: no easy way to limit the items destroyed without biasing
heavily towards the start of the invent chain. The old code was able
to get away without bias by just indiscriminately destroying
everything eligible with a 1/3 chance. Here, I had to introduce
reservoir sampling in a somewhat more complex form than I've applied
it elsewhere, since there are a pool of potential items.
Problem 2: destroy_item no longer worked remotely like destroy_mitem,
which still destroyed 1/3 of items indiscriminately. Commence the
process of squishing them into one function that handles both the
player and monsters. (Which required making a lot of adjustments to
destroy_one_item, now named maybe_destroy_item, on nits such as
messaging and when to negate damage. An annoying consequence of the
merge is that in the player case, their HP is deducted and they can
be killed directly, but for monsters they need to add up the
destruction damage and return it.)
Unifying destroy_item and destroy_mitem has some advantages: in
addition to the obvious code duplication removal, it ensures monsters
now take the same damage as players for destruction (previously they
took a piddly 1 damage per destroyed item). Now when you hit
something with Mjollnir and their coveted wand of death breaks apart
and explodes, you at least get the satisfaction of knowing they took
the standard amount of damage from it. Monsters also now get
symmetry with players in having extrinsic elemental resistance
protect them from item destruction, and damage negation from item
destruction if they were appropriately resistant.
Problem 3: a lot of callers didn't preserve the "amount of incoming
damage" that this refactor relies on. E.g. if the defender resisted
that element, the local dmg variable would be set to 0. So I had to
do some wrangling with callers to save that original damage
value. The rule of thumb is: all *incoming* damage counts. So that
includes the player's spellcasting bonus if applicable, but not
things like half damage, negation due to resistance, or extra damage
due to being vulnerable to cold/fire.
Then I figured, while I'm here let's get rid of all those silly cases
where destroy_items is called multiple times for various different
object classes, and cut the object class parameter out of it. This
has a few minor effects:
- Places where different object classes previously rolled
independently for destruction to happen at all now roll
once. (Which, by my calculation, generally means less incidences of
destruction - a fire attack now won't have three separate chances
to hit your scrolls, potions, and spellbooks. On the flip side, a
lucky roll will no longer save an entire object class in your
inventory.)
- Callers can no longer specify different probabilities for
destroying different object classes. The only place this was really
used was to call destroy_item with a slightly lower probability on
SPBOOK_CLASS. With the nerf in this commit, less of them ought to
be destroyed anyway.
- A very edge case of where explosion-vs-monster damage was totted up
differently for golems, which could result in differences of a hit
point here or there.
- All object classes being processed in one go means that less items
are destroyed than would be if they were still processed
independently. This is not really visible compared to the old
baseline of just destroying 33% of everything, but would be a
marked difference versus a copy of the game that still called
destroy_items separately for different object classes. To
compensate, I adjusted my planned damage-to-destruction-limit
scaling factor down from 8 to 5.
Not done: merging in ignite_items(), though that would probably be
really easy now.'
Notes from porting from xNetHack:
- It might be necessary to reexamine at all the conditional checks for
calling destroy_items. Because item destruction is much more
restrained and uses the actual damage from an effect, we might now
need to check 'if (!rn2(3))' and similar in all the places item
destruction occurs.
This reverts commit 378648bd9c.
The problem was triggered by marking the 'objlist' argument in
merge_choice() prototype with __attribute__((nonnull)) when it
shouldn't have been, then a followup which relied on that. The
'objlist' argument might be Null. Instead of passing its address to
force it to be non-Null, remove the attribute.
Allow 'm p' to pay via menu when menustyle is traditional and to pay
via the old sequence when it's combination, full, or partial. Also
revise the "Itemized billing?" prompt to accept 'm' as well as 'ynq'.
Answering 'm' will switch from the old sequence to the menu (whether
you got to that prompt via m-less 'p' for traditional or 'm p' for
other styles).
... and have more than 1 billed item, and using non-traditional
menustyle.
I opted to add an extra field to the bill struct, because
that made the code cleaner.
Breaks saves and bones.
Delete engravings made in a breach of a shop's wall or of a vault's
wall or in the guard's temporary corridor when the wall is repaired
or the corridor removed. If 'sanity_check' was On, those would
trigger impossible warning "engraving sanity: illegal surface (x)"
where x was the terrain type code for solid rock or relevant walls.
Adding del_engr_at() calls to the shop code was straightforward.
The vault code is very complicated and I'm not sure that all the
calls I added were actually necessary.
Neither my fix for #969 nor the followup by entrez dealt with this:
if you were in one shop and dug the wall it shared with another shop,
sometimes the shopkeeper for the room your were in would teleport to
you and demand payment--or attack if you lacked funds--other times the
one from the far room would do so. For the latter, if you maneuvered
to the gap in the wall (possibly declining to die if angry shopkeeper
managed to kill you) you would get "this shop is deserted" (which is
accurate) but if you subsequently died there, you could get "Welcome
to so-and-so's shop" when the shopkeeper who abandoned her shop was
returned to occupancy after one of them took possession of invent.
And the welcome message might come from the shop that hadn't been
deserted and that you had never left. (Perhaps always from that one;
I'm not sure.)
Possibly the shopkeeper for the room you're in should get priority
when demanding payment for repairs so that the other one won't
abandon the far shop, but I didn't attempt to tackle that. This
just suppresses room entry messages when returning the shopkeeper to
her shop if the game is ending.
Not fixed, but amusing: in one of the tests, the 'far' shopkeeper
who had teleported into the near shop to demand payment for the dug
wall picked up an item from the near shop (in the case I noticed, a
hardware store shk picked up a food ration; just an ordinary item
owned by the stop) while pursuing me to the wall gap. One shk was
stealing from the other. :-)
inherits() only examined the first item in u.ushops, so some shopkeepers
that should have had first dibs were ignoring the hero, one of the
causes of #969. Examine the entire u.ushops array instead of just the
first item so that the hero's position within the shop will be correctly
identified (and do the same in set_repo_loc, though it's probably not
really necessary there).
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.
