Pull request from mkuoppal: avoid integer overflow when user types
digits and they're combined into a number by successively multiplying
intermediate value by 10 and adding new digit. Needed to avoid
triggering undefined behavior if the value overflows the largest
signed integer (actually long int).
This is a much more general fix than the code in the pull request,
which imposed an arbitrary limit for one aspect of tty input.
I'm not convinced that integer.h was the right place to add the new
AppendLongDigit() macro. I may not have caught all the places where
it is needed. files.c accumulates a value from digits but uses
unsigned int, so overflow won't trigger undefined behavior (although
it presumably ends up with a different value than what was intended).
options.c and coloratt.c accumulate smaller integers and have a limit
on the number of digits they'll use, so can't overflow.
Fixes#1254
Containers can't become fireproofed so the line of code in
fire_damage() which tested for that led to confusion.
Also, add missing handling for statues as containers.
I was trying to examine gi.invent (via
print ((struct instance_globals_i *)&gi).invent
don't ask...) and gdb gave me very confusing rejections that don't
occur with other g<letter>.variables. It turned out that gdb was
treating 'gi' as a type rather than as a variable reference.
I suspect it was because Qt was included so linking used C++ mode,
and C++ handles struct names differently from C. Anyway, renaming
'struct gi' to 'struct glyphinfo' made the mystery problem go away.
Restoring was hiding unhidden mimics and if that chose an object
other than boulder of gold pieces, it called mkobj() before an
array used by that routine was initialized. The result was warning
"rnd(0) attempted" when NH_DEVEL_STATUS wasn't set to 'released' or
divide by 0 crash if it was set to that.
Restore should not be catching up for lost time when unpacking a
save file into individual level files, and if it hadn't done that
it wouldn't hide mimics who aren't currently hidden.
In addition to avoiding that, this also moves the initialization of
the offending mkobj array sooner.
[3.6 didn't use that array so wasn't susceptible to this. It is
hiding unhidden mimics during what should be a strictly bookkeeping
operation though.]
I happened to restore a save file left over from testing yesterday's
attempt to fix github issue #1253 and had a werejackal change from
human to beast while next to my hero on "Elbereth", then attack me.
This should be sufficient to prevent that. I'm not sure why the
previous fix attempt seemed to be adequate when testing it. The new
fix relies on the previous one.
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
Fetching a value of DEBUGFILES from the environment to enable
debugpline() messages was intended to operate in wizard mode only
but that wasn't enforced.
Honor the objection to an earlier change to tty_curs(), despite the
fact that it seems delusional. Four integer comparisons per cursor
positioning call affect the throughput of cmov(), but only if the
program is built without DEBUG defined?
More analyzer induced hackery. If tty_curs() discovers that the x,y
passed to it are no good, don't use them, even if that results in
the next output being in the wrong place.
The old issue of behaving differently when built with DEBUG defined
versus when it is not defined is fixed in a different way.
Check getenv("DEBUGFILES") unconditionally during startup rather
than waiting until it's needed.
Might prevent the static analyzer from getting so aggravated.
Most likely not though, but should make debugpline() less fragile
if it gets called during a panic or a signal.
Since croom and maxroom both point to elements in the rooms[] array,
the complaint that the memcpy() in
| if (croom != maxroom)
| memcpy(croom, maxroom, sizeof (struct room));
might have overlapping source and destination is bogus. The fix is
trivial but not vetted by the analyzer.
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.
Extend PR #1240 by giving the three brides names.
It looks a bit odd when they're in bat/fog/wolf form rather than in
base vampire lady form but I think that we can live with that.
In the original Bram Stoker novel, Dracula has three brides,
colloquially referred to as the [Brides of Dracula]
(https://en.wikipedia.org/wiki/Brides_of_Dracula). I thought it would
be a nice touch for NetHack to quietly reflect this by ensuring three
of the vampires of his court are vampire ladies. While this
technically increases the difficulty of Vlad's Tower, the vampires in
Vlad's court are not necessary to fight, and the experience level of
a hero at this point in the game is likely to cause them to spawn as
vampire lords or ladies in any case.
No established names exist for these characters, so I have left them
unnamed.
[Cherry picked rather than merged into order to reformat the commit's
log message. PatR]
Closes#1240
Pull request from argrath: replace 'goto nothing_special' with calls
to new routine nothing_special().
Needed some conflict resolution from 'git rebase' after sitting for
such long time.
Closes#1168
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.
I think that using two arrays in tandem like give_u_to_m_resistances
originally did is frequently considered a bad idea, because one can
easily be added to or subtracted from without modifying the other --
that could obviously cause problems. Combining the arrays like this
avoids that potential problem.
Landmine blew up, scatter exploded a potion of oil, which melted
the ice on which the landmine was, resulting in the landmine trap
being deleted. The code then tried to access it to make a pit.
Issue reported by Umbire: if hero dies by drowning on the Plane
of Water, cause of death was reported as "drowning in a limitless
water".
Reported for livelogging but applied to tombstone and logfile too.
Omit the article "a" in this situation.
For 3.6.7, it would have started as "drowning in a water" and been
updated on the fly to be "drowning in deep water". 3.7 changed
terrain type WATER to be "wall of water", where "a" is expected,
and also added "limitless water" for Plane of Water, but it was
neglecting to include a similar fixup for the latter. The "deep
water" fixup is still present but doesn't get triggered anymore.
Fixes#1248
A monster swallowed hero, hero teleported the monster away, but the level
was full of monsters, so the swallower ended up not moving anywhere,
so it was in the same location as hero, but hero wasn't swallowed by it.
Move the monster into limbo instead.
A pet titan dropped a boulder over a pool, and the code killed
the titan, because the code was considering the titan to be in
the water.
If a flyer or a floater is over the pool, don't kill them off
if a boulder is dropped in there. Also make the pet movement
code handle the case where the pet might've died while dropping
items.
The revised code returned an uninitialized variable if the new test
failed.
The cloneu() case was ok but this changes it to keep parallel to the
clone_mon() case.
Noticed while doing rudimentary testing of the mnearto() fix for
mimics. #migratemons wouldn't add monsters to migrating_mons if
getlin() was compiled with EDIT_GETLIN defined unless you manually
deleted the invisible default value that wiz_migrate_mons() was
passing. It took a while to puzzle that one out.
The command list has conditional description of what #migratemons
does and it was using the wrong spelling of the macro used to
control that. So '# ?' described the behavior of #migratemons as
it operates without DEBUG_MIGRATING_MONS enabled even when that is
enabled, adding to the getlin() confusion.
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.
Issue reported by chadministratorwastaken: persistent inventory
window didn't update if reading a spellbook caused that book to fade
to blank paper.
Issue was reported for 'curses' but it was general. The core made
the possibly-revised book type become discovered but that wouldn't
update perm_invent unless the type of book hadn't been discovered.
So if plain spellbook wasn't known yet, the persistent inventory
window did update as intended, but if spellbook of blank paper was
already discovered, the window continued to show the old book type
instead of changing to reflect the revised type (until subsequent
activity triggered another perm_invent update).
Fixes#1246
|w - wield this item in your hands
is "suboptimal" when poly'd into some form which lacks hands. Don't
offer 'w' as a choice if in a form what can't wield anything, and
substitute "claws" for "hands" when in forms that warrant it.
Include iron ball as a type of item leading to 'w - wield as weapon'
instead of falling through to 'w - wield in hands'.
Since inventory actions can be used as teaching aids for regular
command usage, change picking a potion in inventory from
|q - Quaff this potion
to
|q - Quaff (drink) this potion
When picking the uquiver item in inventory, add
|f - {Shoot|Throw} this item
and for the shoot case, tack on "with wielded <launcher>".
That makes 't - {Shoot|Throw} this item' nearly redundant (but only
for an item in uquiver slot) so tack on "(same as 'f')".
For the 'Q' choice (only offered for weapons or rocks/gems), have it
mention what the quiver is for rather than just that choosing that
menu entry would put the stack in the quiver slot.
For 'E - Write on the floor with this object', change the verb to
"Engrave" when the object is a bladed weapon, hard gem, hard gem
ring, or wand. It also changes 'floor' to surface() which might
need some refinement.
For non-weapons, change 'w - Hold this tin opener...' and 'w - Hold
this item in your hands' to 'w - Wield this...'. The concept of
holding an item in hero's hands isn't applicable in general, and in
particular any "held" item replaces wielded weapon.
For a bunch of instances of "this item", when the quantity of the
selected inventory entry is more than 1, change to "one of these
items" or "this stack" depending on how the specified action deals
with stacks.
Coming up to medusa-3 from the level below revealed that Perseus's
statue was not there. It was placed randomly in one of the three
chambers where Medusa might be rather than in the one where she
actually was. Medusa-4 behaved similarly.
Initially I was forcing the statue to be adjacent to Medusa rather
than on her spot, but medusa-1 and medusa-2 don't do that so I've
gone with a simplified revision.
Wake up a sleeping target that gets hit by chain lightning. Also,
bring mimics and hiders out of hiding.
Don't let chain lightning pass into the water on Plane of Water (or
water-wall elsewhere); not sure whether that makes sense physics-wise
but it would have feedback problems due to visibility issues. Do let
it pass over lava.
