Report described this as a panic triggered by the sanity_check
option, but that's because it was running under the fuzzer, which
escalates any impossible() to panic(), rather than because nethack
panicked.
I couldn't find anything wrong--which doesn't mean that there
isn't something wrong--with place_worm_tail_randomly() and
random_dir(). They use xchar for map coordinates which should be
fine as long as no negative values are generated and I couldn't
discover any such. The suggested fix of changing xchar to int
might indicate a compiler bug (although the odds of that are low).
The bogus coordinate of -15000 in the report suggests that
typedef short int schar;
(which changes xchar too) is being used in the configuration but
I don't recall having any problems attributable to that.
This switches from xchar to int as a side-effect of replacing the
offending code entirely. The new code might produce an 'ny' of -1
before goodpos() rejects it, so xchar would be inappropriate now.
The old code is commented out via #if 0 _after_ changing it from
xchar to int.
This also adds an extra sanity_check for worm tails, unrelated to
the current bug. I'm not aware of any instance where it fails.
EXTRA_SANITY_CHECKS needs to be defined for it to do anything.
An earlier change resulted in place_worm_tail_randomly() sometimes
removing a long worm from the map unintentionally. It was still on
the monster list so if wizard mode sanity_check option was On, there
would be warnings of a monster which isn't on the map.
The change which triggered this was necessary so I'm inclined to
blame place_worm_tail_randomly() laziness.
This is a superset of the pull request's fix.
Fixes#377
montraits() didn't have any handling for long worm tails, makemon()
didn't have any provision for creating a long worm without a tail,
replmon() uses place_wegs() to put tail segments on the map when
replacing a dummy new monster with the mtraits one but place_wsegs()
wasn't updating the head segment since it isn't put on the map.
That turned out to be key because there is always an extra segment
co-located with the monster and when its coordinates were wrong,
worm_known() gave bad results for visibility checking. The
statue-goes-away message was the one for not being able to see the
monster that it just animated into, even though 'w' appeared at the
spot. It took quite a while to track down what was going on there.
Sanity checking for worms has been updated and could conceivably
start triggering complaints about things that it used ignore.
When a special level is created, there's a chance it gets flipped
horizontally and/or vertically.
Add new level flags "noflip", "noflipx", and "noflipy" to prevent
flipping the level. Add a wiz-mode command #wizlevelflip to test
the flipping on current level - although this doesn't flip everything,
as level flipping is meant to happen during level creation.
After casting a spell, a monster got a chance to make a regular attack
despite the apparent attempt to set up a return value indicating that
it wouldn't move.
When looking over the return value situation, I noticed 'wormhitu()'
for the first time. It gives worms additional attacks when the hero
is adjacent to some of the tail, that only works if the head is within
reach of a melee attack. The hidden tail segment at head's location
always met that criterium so gave an extra attack that didn't make
sense; change wormhitu() to skip that segment.
Do some formatting in mcastu.c; no change in actual code there.
Fixes#285
Make long worms grow more slowly (although that didn't seem to make
as much a difference as expected) and limit the amount of HP they
acculate if they shrink and then re-expand. Shrinking doesn't take
away max HP but growing used to always add to max. Now it won't add
to max HP unless the number of segments is at that worm's peak, so
shrinking will inhibit the exhorbitant HP expansion that idle worms
have had.
Mostly removing 'register', but also clear stale wgrowtime[] values
at the time they become obsolete instead of waiting for initworm()
to replace them when eventually re-used.
random_dir() for picking a spot to put a segment of a long worm's tail
(when it is being placed on the map, perhaps after teleport, not when
movement allows it to grow) had mistakes in three of the four compass
directions. The one for the top of the map was benign; it just
neglected to use the top row (#0). But it could pick a value off the
edge of the map for bottom and right or both.
This doesn't explain a couple of long worm [segment] oddities on the
Astral level because that level doesn't go all the way to the edge.
Make some progress on a couple of next minor release checklist
items, hopefully without introducing too many new bugs. This
is just the initial commit, and work continues.
Checklist items:
Savefiles compatible between Windows versions, whether 64-bit
or 32-bit in little-endian field format.
Selection of file formats:
historical (structlevel saves),
lendian (little-endian, fieldlevel saves),
and just for proof-of-concept, ascii fieldlevel saves
(the ascii is huge! 10x bigger than little-endian).
For the fieldlevel save, all complex data structures recursively
get broken down until until it is one of the simple types that
can't be broken down any further, and that gets when it gets
written to the output file.
New files needed for this build:
hand-coded:
include/sfprocs.h
src/sfbase.c - really a dispatcher to one of the
output/input format routines.
src/sflendian.c - little-endian output writer/reader.
src/sfascii.c - ascii text output writer/reader.
auto-coded (generated):
include/sfproto.h
src/sfdata.c
This is just one approach. I'm sure there are countless others
and they have different pros and cons.
For producing the auto-coded files a utility called
universal-ctags, that is actively maintained and evolving,
was used to do all the heavy-lifting of parsing the
NetHack C sources to tabulate the data fields, and store
them in an intermediate file called util/nethack.tags
(not required for building NetHack if you already have a
generated include/sfproto.h and src/sfdata.c)
util/readtags (also not required for building NetHack
itself) will decipher the nethack.tags file and produce
the functions that can deal with the NetHack struct data
fields.
You can obtain the source for universal-ctags by cloning it
from here:
https://github.com/universal-ctags/ctags.git
The combination universal-ctags + util/readtags has been
tried and tested under both Windows and Linux, so it is
not tied to a particular platform.
Note: util/readtags will work only with universal-ctags
output, so other ctags are unlikely to work as-is.
Universal-ctags can be build from source very easily
under Linux, or under Windows using visual studio.
This is based on the multiple-RNGs code fron NetHack4, but using
only the parts relevant to the display RNG (and with substantial
changes, both because of post-3.4.3 changes, and because Nethack4's
display code is based on Slash'EM's rather than NetHack's).
hmon() can destroy the weapon being used, and known_hitum() would
still pass the pointer to the freed object to cutworm(). Remember the
relevant weapon attribute before using and maybe freeing the object,
then pass that attribute instead of the whole weapon. Also pass
'more-likely-to-cut' for axes in addition to blades.
thimonst() behaved similarly, although due to much different code
paths none of the objects that might get to hmon() were then passed to
cutworm(), so it wasn't vulnerable. But pass 'more-likely-to-cut'
for axes instead of for blades when thrown.
When a long worm was removed from the map, the segments for that worm
retained their map location info. This caused problems later on if
wormgone (or toss_wsegs) was called, because it would try to remove the
segments of that worm from the map.
If long worm attacks you, and you have a passive response that may
kill the worm (eg. blue jelly), the worm may die in the middle of
a loop checking for a worm segment that can attack. This'll cause
a segfault because the worm segments will get freed.
If the worm dies, just exit the loop.
When using a stethoscope or wand of probing on a long worm, report
the number of segments it has in the feedback given.
Some of the extra bhitpos and/or notonhead assigments may not be
necessary. They were added when I was trying to figure out the
question of why probing of a tail segment revealed a long worm's
inventory even though the code explicitly prevents that. (Answer:
it didn't; I had misinterpreted bz 12 to think that that was what
was being reported. You need to use wand of probing--or "insigtful"
Magicbane hit--on the head in order to see its inventory or be told
"not carrying anything".)
The automated reformatting put a space in casts of the form
'(type)(expression)', yielding '(type) (expression)', but it didn't
do that for '(typedef)(expression)'. There are lots of instances of
'(boolean)(expression)'; (uchar) and (xchar) also occur. I haven't
noticed other types, but I haven't looked in very many files yet.
End of first pass, but '[&|?:][ \t]*$' doesn't catch trailing operater
followed by end-of-line comment so more needs to be done. As with the
past couple of batches, I've removed redundant parentheses from 'return'
statements but only for files that had continuation fix-ups.
I've also removed tabs from comments in some of the files, but didn't
start until part way through this subset of the sources.
I'll push a formatting guide at some point. There may still be
outstanding changes, but please feel free to resolve those as you arrive
a them.
To the best of my knowledge, there is no changes to the actual code
content, but the formatter does have the occasional bug. If you run into
an issue, please fix it!
From a bug report, a long worm with 0 HP
was observed via stethoscope after cutting one or more worms in half many
times, followed by an unspecified crash. Cutting a worm doesn't reduce
its level below 3, but if a worm is drained to level 0 by some other means
and then gets cut in half (and still has at least 2 HP left), cutworm()
would give the new level 0 worm 0d8 (hence 0) for current and max HP.
That could confuse end-of-move monster cleanup, which thinks 0 HP is a
dead monster who has been removed from the map but not yet purged from the
fmon list. Purging it would then leave a stale monster pointer on the map.
cutworm() should have special cased level 0 to use 1d4 for HP, but
instead I've changed it to not produce a cloned worm if the source one is
lower than level 3.
Fix the crash From a bug report, where
having the hit that cuts a long worm into two also take the original down
to 1 HP would result in clone_mon() returning null and nethack crashing due
to a segmentation fault or access violation. The same thing could happen
if there's been enough long worms created to get them flagged as extinct.
This bug was only present in 3.4.3. Prior to that, cut_worm() did
its own monster creation inline instead of calling clone_mon(), ignoring
extinction and too-low hit points.
This is one of the items from "#Q397: List of Bugs from #nethack" sent
in Janurary by <email deleted> and containing a list
of things collected from the IRC channel associated with nethack.alt.org's
public server. Moving diagonally between segments of a worm tail is
conceptually passing right through the worm's body. This patch prevents
moving in such a fashion for both the hero and monsters (it's still
possible to fight in that position though). It only applies when the two
tail segments are consecutive.
|...... In the diagram here, where tail segments are represented by
|.w1?.. digits indicating relative sequence number, the @ can still
|..@2.. move between segments 2 and 5 to reach !, but can no longer
|.65!3. move between 1 and 2 to reach ?. [However, if there is a
|...4.. monster at the ? spot, it can still hit @ and vice versa.]
Missiles and wand zaps still pass through such diagonals without
noticing or affecting the worm. I'm not sure whether this ought to be
extended to change that--it might get pretty messy since it would need
to be considered during monsters' targetting as well as during the path
traversal itself.
I've gotten tired of seeing newsgroup claims along the lines of
"since devteam is aware of this and has chosen not to eliminate it, they
must endorse it", so weaken the tactic of "pudding farming". It is still
possible to gain unlimited experience (past level 15 or so there's not
much point), but will be less effective for gaining items and for providing
sacrifice fodder. Keep track of which monsters have been created via
cloning (mostly puddings; gremlins and blue jellies are affected too but
nobody's likely to care much about them) so that they can receive special
handling. Make cloned monsters progressively less likely to leave corpses
as the number killed for a particular type goes up, and also much less
likely to drop random items at death. This is sure to need some tuning
once hard core farmers point out how they can still abuse it. For the
absurdly extreme case, see
http://scavenger.homeip.net/farmbot/HomePage
FYI, farmbot/PuddingFarmingHOWTO includes an impressive screen shot of a
dungeon level where rampant farming is taking place.
Cutting a shopkeeper poly'd as a long worm would generate strange messages
and could result in a crash. cutworm didn't deal with all the intricacies
of duplicating a monster. Fixed by changing cutworm() to use clone_mon()
to do most of its dirty work. It seems to me that without this change,
cutting a tame long worm could also have similar bad effects.
Other side effects of this change:
- clone_mon now takes x,y coordinates, 0,0 results in previous behavior
- clone_mon no longer always makes cloned monsters tame/peaceful if player
caused the clone, using the same formula previously in cutworm. Someone
else may wish to tweak this for gremlins.
- clone_mon will christen the new mon with the old shopkeeper's name, even
though clones are never shopkeepers (game can't handle 2 for a shop)
- cutworm can now be called during conflict or pet combat, although I
added no such calls (yet)
<Someone> wrote:
- You currently appear to be able to specify '~' and have it try
to detect monsters (though it won't detect any long worms)
Show the entire worm when specifying either '~' or 'w' now.
Pat Rankin wrote:
> collect them all into some new struct and
> save that separately rather than jamming more non-option stuff
> into struct flags.
This patch:
- collects all context/tracking related fields from flags
into a new structure called "context."
It also adds the following to the new structure:
- stethoscope turn support
- victual support
- tin support
>More worrying is the fact that applying a figurine over water lets
>the monster wait until its next move before it drowns (giving
>you time to teleport it to safety, or whatever) [...]
>Should there be a minliquid() check as part of make_familiar()?
Applying at the water location next to you was easy. But
applying it at your own location (triggering BY_YOU) could
end up placing the figurine at the far side of the level if
there was lots of water.
Correcting that required the ability to pass a flag from
make_familiar to makemon() telling it to not rule out
water locations as good positions. The flag had to
be passed on down to goodpos() and enexto().
The bulk of this patch is just adding an additional
argument to goodpos() in all of the callers.
