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.
Monstly changing '<type>* <var>' to '<type> *<var>'. Wrap or shorten
a few wide lines.
One of the rolling boulder routines accessed gb.bhitpos.x,y many times;
change that to use local x,y instead.
If a termcap entry for ending attributes and color also contains
the code to switch from secondary font back to primary (HE_resets_AS
hack), maybe strip the AS code out of HE (and clear the HE_resets_AS
flag) when setting up DECgraphics. Affects whether nethack sends
extra AS sequences while rendering a run of VT line-drawing chars.
My HE doesn't reset AS so that aspect hasn't been exercized.
When switching back and forth between normal and line-drawing,
defer the switch away from line-drawing if the character will be
rendered the same in both character sets (uppercase letter, digit,
most punctuation). That might just defer the AE, but could skip it
and next AS depending on what characters are written. The cycle
might repeat an arbitrary number of times, avoiding sending many
AS+AE combinations rather than just one.
Both of these optimizations are pretty small but reducing the number
of characters sent from a server to a remote user is worthwhile.
If a punished hero's inventory is empty, have a nymph usually take
off the attached chain and indirectly fix punishment. If hero is
trapped by being chained to a buried iron ball, sometimes take off
the implicit chain and indirectly release hero from the trap.
Monkeys won't do this and nymphs will only do so if there is no
inventory (aside from gold and/or embedded dragon scales which they
aren't allowed to target).
It's a lot simpler than I was expecting when I wrote the TODO about
it recently.
If a trap is blocking levitation or flight and you use wand of opening
or spell of knock to get out, you'd get things like
|You start to float in the air!
|You are released from the bear trap.
Give the release message before the actual untrapping.
Add a comment about something that occurred to me when fixing the
^C-during-DECgraphics-output situation several days ago. I don't
think there's any compelling reason to avoid this optimization, but
this only describes it without actually implementing it.
Simplify the valid-position highlighting via '$' by combining the
tmp_at(start) and tmp_at(populate) steps.
Add highlighting for a couple of targetting operations that would
give valid/invalid feedback via autodescribe but weren't displaying
highlight markers for $.
I made several jumping changes, most of them dealing with picking
hero's own spot.
Redo getpos() highlighting. If bgcolors is Off, '$' toggles between
no highlighting and showing dollar signs at valid spots. ^R removes
those if they're present. When bgcolors is On, '$' cycles among
three settings: highlighting via background color, no highlighting,
and highlighting by drawing dollar sign characters. ^R switches to
background color mode.
This doesn't directly solve the problem of background color causing
conflict with the foreground color of some objects or monsters, but
being able to get rid of the background with a keystroke should be an
improvement.
'bgcolors' defaults to On, which was a problem for tty when 'color'
was Off. Turn it Off (in the core) for text map if color is Off and
bgcolors hasn't been explicitly set. Conversely, Qt was leaving
color Off and then using color with abandon. Turn in On (in the core)
for tiled map if it hasn't been given an explicit value. Those two
changes should cope with most situations and still let the player
override.
Change the goodpos symbol, which is used to mark valid locations for
some operations when getpos() is having the player pick a spot, from
green question mark to blue dollar sign. Dollar sign is the default
keystroke to toggle those markers off and on.
end.c:177:16: warning: unused parameter 'why' [-Wunused-parameter]
177 | NH_abort(char *why)
| ~~~~~~^~~
end.c: At top level:
end.c:1191:1: warning: 'get_saved_pline' defined but not used [-Wunused-function]
1191 | get_saved_pline(int lineno){
| ^~~~~~~~~~~~~~~
options.c:1263:1: warning: 'optfn_crash_urlmax' defined but not used [-Wunused-function]
1263 | optfn_crash_urlmax(int optidx UNUSED, int req, boolean negated UNUSED, char *opts, char *op)
| ^~~~~~~~~~~~~~~~~~
options.c:1240:1: warning: 'optfn_crash_name' defined but not used [-Wunused-function]
1240 | optfn_crash_name(int optidx UNUSED, int req, boolean negated UNUSED, char *opts, char *op)
| ^~~~~~~~~~~~~~~~
options.c:1217:1: warning: 'optfn_crash_email' defined but not used [-Wunused-function]
1217 | optfn_crash_email(int optidx UNUSED, int req, boolean negated UNUSED, char *opts, char *op)
| ^~~~~~~~~~~~~~~~~
Add the 'dump' argument to the existing '--version' command-line
option to display the magic numbers used when validating save and
bones files for compatibility.
Nothing exciting, just a line of 5 hex values. I was going to also
list the values for however many save and bones files are specified
on the command line but it seems to need more effort than I care to
expend. And I hadn't made up my mind whether that should be done by
nethack, recover, or some new standalone program. [Single line of
relatively raw output is so that they could be compared more easily.]
nethack --version:bad-argument was writing a message to stdout and
then starting play--which immediately overwrites stdout. Have it
quit instead. Player wasn't trying to start a game and quitting is
what it does with --version:good-argument.
'uskin' isn't part of worn[] so wasn't being checked with the other
slot pointers. Add that, although it'll be excluded by default and
need to have EXTRA_SANITY_CHECKS defined to include it.
Default to INTERNET_AVAILABLE=Y. If the build computer is not
online with the internet available, an explicit
INTERNET_AVAILABLE=N
will be required for the build, if pdcurses and Lua are not already
staged in the lib folder. That can be done by editing the appropriate line
in the Makefile, or by altering the command line:
nmake INTERNET_AVAILABLE=N install
With Makefile.nmake:
Store pdcurses wingui and pdcurses wincon object files in separate
directories.
Rename the two pdcurses static libraries.
Because of the static library changes, the following will be required
prior to the next build:
nmake spotless
Check the various uarm, uwep, and so forth pointers to make sure that
they point to items in hero's inventory and that those items have the
corresponding W_ARM, W_WEP, &c bit set in their owornmask field.
Also check whether any other items in inventory have the same bit set.
[Some of this is already handled by sanity_check_worn() in mkobj.c.]
Also validate two-weapon combat mode. I don't recall ever seeing any
problems reported about it though.
Does not validate ball and chain. Those should have their own sanity
checks that validate a bunch of other stuff besides just worn slots.
They already get some checking by the normal object tests.
This works ok with 'sanity_check' set and items worn and wielded
normally. The only insane situation tested was by reverting the
confused-looting-with-quivered-gold fix from earlier today. I haven't
used a debugger to force other such problems so this isn't very
thoroughly tested.
Noticed while testing confused #loot: when using 'Q' to populate
quiver, or 'f' when quiver is empty, don't bother asking what to
ready/fire if inventory is empty.
And when inventory isn't empty, don't list '-' as a likely candidate
if quiver is already empty.
'w' behaves differently. '-' is treated as a likely candidate when
already not wielding anything, and even when inventory is completely
empty.
