isqrt adds some noticeable distortion artifacts to gradients (test case
I used is to draw a line from 10,10 to 20,15 with mindist = maxdist = 2
and see how the gradient is biased towards the upper right); changing
the distance calculations to use the square of the distance rather than
the raw distance avoids this. This makes radial gradients more radial,
and square gradients more square.
There still appears to be a bit of bias, but I think this is due to the
line algorithm not lining up perfectly with the tiles.
selection.gradient has some pretty unintuitive behavior, in that it
selects points that are NOT close to the defined center. I've used
gradient selections several times and so far all of them have had to be
negated, because I wanted to select points close to the center with a
decreasing probability further out.
This implements that behavior, and also fixes a bug in which the x,y
coordinates of the gradient center(s) were not converted properly when
used within a des.room or des.map. Also updated the lua documentation
for gradient.
I removed the "limited" argument, as it was previously used to control
whether the rest of the map outside the max given distance would be
included in the selection; now that the area beyond maxdist is naturally
never in the selection, it doesn't have much use. (And I can't think of
a reasonable use case for the inverse: wanting to select points close to
the center, with decreasing chance towards maxdist, but then select the
entire map beyond maxdist.)
Currently this does not affect any special levels or themed rooms
because none of them use selection.gradient.
The intuitive behavior of des.levregion or des.teleport_region when
"exclude" is left unspecified is that there is no exclusion area.
However, this wasn't actually the case: since l_get_lregion defaulted
the exclusion area to (0,0,0,0) and exclude_islev to 0, this meant that
the 0,0 space on the map would always be excluded from regions. In cases
where a region was specified with its inclusion area constrained to the
0,0 space of the map, this would create a "Couldn't place lregion"
impossible message.
This fixes that issue by defaulting the exclusion area to (-1,-1,-1,-1),
and if the exclusion area is left unspecified, forces exclude_islev=1.
This means that the exclusion zone will be outside the walkable space of
the level where it can't cause any problems.
If a level designer puts negative coordinates in their inclusion or
exclusion parameters, this might not work correctly, but negative region
coordinates aren't currently used anywhere and probably shouldn't be
supported anyway.
Reported by every for xNetHack. This bug is latent in vanilla, but can
easily start to present itself if themed rooms of a certain shape are
added. Ultimately, it comes from an assumption that shops will always be
rectangles of at least size 2x2, and the shopkeeper will always be able
to step diagonally backwards from their normal position just inside the
door in order to get out of the player's way.
Themed rooms introduce the possibility of shops where the shopkeeper has
only 1 square adjacent to their normal position to move to -
effectively, the shop entrance is a narrow corridor. When this happens,
they have nowhere to go to allow the player to enter or leave the shop,
leaving it permanently blocked unless the hero teleports or falls in or
out.
This fixes that by adjusting the shop algorithm to detect when a shop
candidate room is set up like this, and excludes it from becoming a
shop.
Noticed that an attempted terrain replacement wasn't taking hold even
though 'w' is supposed to mean "match any stone or wall"; this was
because w converts into non-terrain-type MATCH_WALL and replace_terrain
was doing a simple comparison on whether the potentially replaced
terrain matches that type. Add a special check here for w so it will
match the terrain types it's supposed to.
Note that using replace_terrain with 'w' now WILL match stone, since
this is the documented behavior of w, to match IS_STWALL rather than
just IS_WALL. If a level designer really wants to exclude stone, they
can work around this by either making a selection and filter out stone
terrain, or doing two replace_terrains with '-' and '|'.
The Tourist locate and goal levels have 2 shops each, and also have
various traps randomly placed on the level. Unfortunately, this does not
account for placing them in the shops, so it was possible to wind up
with a magic trap or falling rock trap or whatever inside the shop,
which the shopkeeper would not clean up.
This commit makes selections that exclude the shop areas, and picks the
traps from those selections, keeping the shop floors trap-free as their
customers expect.
Calling des.mineralize() with no arguments was equivalent to calling it
and manually specifying gem_prob = 0, gold_prob = 0, etc. Which meant
that no mineralization would actually happen.
Instead, make this match the intuitive behavior, and pass in -1
probabilities as defaults -- which the mineralize() function interprets
as the caller wanting to use the standard probabilities for a level of
that depth, as if it were not a special level.
This change does not affect any special level files since des.mineralize
is not currently used in any of them.
Perennial problem: since Bustling Town consists of a fixed map which was
shorter than the moveable area overlaid onto a cavern fill, sometimes
the cavern fill made spaces above or below the walled edges of the town,
which then got cut off when the town was placed. Since one of the valid
ways to generate a way out of an inaccessible space is to create a door
connecting it to an accessible space, this meant rooms in Minetown,
including shops, could get a second door leading into that tiny space.
And if it was a shop, the shopkeeper could not block exit from the
second door.
This fixes that issue by expanding the map vertically so that it will
overwrite the whole cavern fill in the town area of the map segment
where it might create cut-off spaces like this. (Not the whole area -
since we now have the 'x' mapchar to leave existing terrain in place,
areas adjoining open space where inaccessible pockets won't get
created can retain any existing fill to keep the town from being exactly
the same every time).
This is a large iteration on a previous implementation of making
nh.getmap() parse its coordinates as relative to the last defined map or
room rather than absolute to the entire level. Now, everything in the
nh.* and obj.* functions interprets coords as relative rather than
absolute. (By default; if no map or room has been defined, or if the lua
code is executing after level creation is done, they will interpret the
coordinates as absolute).
The general motivation is basically the same - routines that use
absolute coordinates are difficult to use in level creation routines,
because then the designer has to remember to convert the relative
coordinate to an absolute one (and that was impossible before
nh.abscoord was added, particularly in themed rooms). And once
nh.getmap() takes relative coordinates, it would be very strange to have
all the other functions (setting timers, burying objects, etc) remain
with absolute ones.
In a couple places, code is changed to account for coordinates that are
relative to a *room* (which uses g.coder->croom->[lx,ly] as an offset,
instead of relative to a *map*, which uses [xstart,ystart].
Specifically, selection.iterate did not account for this, and without
this the ice themed room timer was not being started in the proper
place.
All tests are updated to respect the new behavior. Most of the modified
functions are not actually used anywhere in level files; the one
exception is starting a timer in a themed room, and that has been
adjusted.
Documentation updated as well to clarify when various things are tossing
around relative and absolute coordinates, both in comments and in
lua.adoc.
There are many possible use cases for nh.getmap during level creation,
but it's rendered mostly unusable by virtue of always returning data
about the exact x,y coordinate in g.level.locations. (In particular,
it can't currently be used in themed rooms at all, because the themed
room could be anywhere on the level.) This is inconsistent with how most
other coordinate-based functions work following a des.map, which use
coordinates relative to the 0,0 point of the map.
This changes it so that during level creation only, if nh.getmap is used
following a des.map statement, it will look up the coordinates relative
to the origin of the map, consistent with the other functions.
I was never too happy with how this was a silent effect that required
you to watch the map to see if anything changed. It might count as
an accessibility issue as well, not sure.
This change adds specific feedback for all the possible things that
might get revealed by detecting unseen. If you reveal a secret door or a
trap, you now get a message indicating that.
One slight behavior change here: if the only thing detected is invisible
monsters, the game previously did not return a result of "detected
a non-zero number of things" to the caller of findit(); now it does.
(This allows the wand to be automatically identified when it prints a
message about detecting invisible monsters.)
If you had both warning and ESP, you would get the message "Your danger
sense causes you to take a second look close by" when moving next to a
monster that is technically "undetected" (according to mundetected) but
was actually apparent to the player via ESP. For instance, moving next
to an eel hiding in the water would produce this.
Since there was no follow-up message ("You find a <monster>".) and no
new information being given to the player, the "danger sense" message
was pointless, and so I removed it in this case when the warning doesn't
lead you to find anything new.
These bugs are currently latent only because no role's starting
inventory happens to satisfy the conditions under which it would show
up. This commit contains the xNetHack fixes for them.
Bug 1: if a role received a specific charged ring (versus a random
ring), it would be possible for that ring's charge to be <= 0, since the
code currently only caught this case for random rings. Move that clause
outside the if (UNDEF_TYP) clause.
Bug 2: non-ring oc_charged items of UNDEF_TYP that randomly rolled a
starting charge/enchantment <= 0 would always be enchanted, sometimes
very highly, because their enchantment was getting set to rne(3) by
hitting this case, which is only intended for rings as a comment
indicates. In particular, Archeologists' starting pick-axe would hit
this case after moving it out of the UNDEF_TYP block, but it would also
apply to any role receiving a random tool or weapon-tool at the start
of the game.
find_skates was still in use for its one intended case, but objdescr_is
has been around for a few years now and can do just as good a job
without having to hardcode the first and last boots in objects[].
You can create steam clouds that are gray and deal 0 damage when zapping
fire at or over water sources. If you happen to be in the cloud, it
produced the "You are enveloped in a cloud of noxious gas!" line, and
steam isn't noxious.
Other uses of "gas cloud" could have potentially been changed, but I
left them alone.
This parameter appears to have been in the code for a very long time,
but never used, since no version of NetHack I can find had mazes with
randomly placed fountains in them. Certainly isn't used now, so this can
be reduced to the same call to find_okay_roompos used by similar
functions such as mksink.
Andrio pointed out at some point that the "below 25% HP: pet will not
attack at all" mentioned in this comment was wrong. It will not attack
*peaceful monsters* at all, but will still attack hostile monsters.
Also, the math behind the balk variable has confused several people,
thinking it's off by one and allowing the pet to attack one level higher
than stated. This is not the case, since it's the lowest level they
*won't* attack. Clarify that.
This is a descendent of an earlier patch I wrote. The main idea is still
to clearly communicate to the player *what* something is turning into,
without the need to farlook afterwards, and give them the opportunity to
add MSGTYPE for when something jumps on a polymorph trap and becomes an
arch-lich. If it happens out of sight, the player also might get a whiff
of the monster's smell, giving a bit of advance warning.
There is one new case in here, in normal_shape(), which came about
because I noticed a weird message sequence: "The magic-absorbing blade
cancels the python! You kill the chameleon!" with no intervening
message indicating the python reverted to a chameleon.
Running #wizloadlua to run Lua scripts that use coordinates in any way
would work differently if you were on certain levels for the first time
versus leaving and returning to them. This is because various bits of
level creation routines can leave xstart and ystart set to non-zero
values, which are then zeroed at some point when leaving and returning
to the level.
Since xstart and ystart are only relevant to level creation and lua
commands, this fixes the problem by zeroing them after leaving mklev
routines. (Saving them with the level doesn't work because xstart and
ystart are relative to the last used des.map, of which there could be
multiple, e.g. in Asmodeus's level or if two map-based themed rooms
happen to generate. I can envision a more complex solution in which
every des.map used in the level can be associated with an identifier,
whose xstart and ystart are saved for use by later post-level-creation
lua scripts, but currently I just want to make them consistent between
level visits.)
Noticed when I tried to create a male monster of a species that permits
both males and females (i.e. not a single-gender or neuter species),
half the time the monster ended up female anyway. This was because
get_table_montype picks a random monster gender for such species, and
lspo_monster just sets it to that, making it impossible to deliberately
have a monster of a certain gender.
This fixes that by defaulting the "female" table argument to random
instead of false, and then checking to see whether the level file set it
to something other than random. If so, it uses that value.
I debated whether this should allow a level designer to make a monster
of a gender that conflicts with their species, such as a male nymph, but
erred on the side of respecting the species. So attempting to specify a
male nymph, etc. will still result in a female one.
Noticed that when I set a selection to grow in a random direction, it
instead grew in all directions, which is not what I wanted. Turns out
the -1 random dir ended up being passed straight to the code which
checks bitmasks, without any form of randomizing among directions.
So this adds code to do that, and defines W_RANDOM as -1 rather than
using a magic number. In the process I also noticed that specifying
"random" as the wall for a door in a room made it rerandomize the
direction every iteration of its loop, essentially rolling two rn2(4)s
and only proceeding if they matched. That was pointless so I cleaned it
up a bit.
Also added safety checks in the form of an impossible for des.corridor()
being called with "random" as either walldir, because this is not
implemented currently.
And lastly, I noticed that create_secret_door was entirely unused
(secret door creation is handled in create_door), so I deleted it.
The only behavior change caused by this is that the Valkyrie quest lava
pools will be a little smaller, which is the only place grow is
currently used. If it's desired to keep them the same, that should be
changed to "all".
Eucalyptus leaves are famously inedible except by certain animals such
as koalas. I consider it very strange that a single leaf in NetHack
gives you six meatballs' worth of calories.
I considered making it 0 nutrition, but am not sure if a 0-nutrition
comestible would end up violating some assumption that all food is at
least 1 nutrition.
Selection difference is something I have found myself wanting a lot when
working on levels, and have had to defer to a clunkier xor-then-and
approach. This commit implements the TODO-ed addition and subtraction
operators on two sets.
I don't see how the addition operator would be any different from
logical or, so it just calls l_selection_or rather than implement a new
function.
trycall() is a short docall() wrapper that is a no-op if the item is
already identified or the player has called the object type already. For
some reason, many calls to docall() did those same exact checks
beforehand.
This commit eliminates that redundancy by converting those calls into
trycall(), which is now made extern rather than local to do.c. No
behavior should be changed by this commit; I've checked that none of the
affected places could take a different code path now that the
oc_name_known and oc_uname checks are removed.
From EvilHack, under the basis that anyone trained in martial arts (or
is in a powerful kicking polyform or wearing kicking boots) should be
immune from landing such a poor kick. This bypass used to happen only
50% of the time; now it happens all the time.
Note that this only averts the "Your clumsy kick does no damage" case:
it remains possible for a powerfully kicking player to kick clumsily and
have the monster evade or block, for example if they are fumbling or
wearing bulky armor.
Also, documentation: I added a comment explaining what the incredibly
dense and confusing logic is with i and j happening here, for the next
poor soul that has to read that code.
In commit db68395, most of the instances of xdir and ydir here were
changed to u.dx and u.dy, but not all of them. The remaining ones are
out-of-bounds on xdir and ydir, because i is always set to 12 from an
earlier loop and is no longer involved in handling user input. They
should be u.dx and u.dy like the rest.
This is because ice was being treated as a type of corridor rather than
a ROOM space, and running has rules for following similar terrain. In
reality it's not a corridor and should behave like normal room for
running purposes.
This was obvious in the valkyrie quest upper levels with ice fields, in
which running into the edges of the map obliquely, or into the corners
of the map, would send the hero flying around the edge in a different,
probably unintended direction.
Currently weapons are set up as piercing, slashing, or whacking, using
their object's oc_dir field, with the intention that certain weapons
can classify as both. However, since oc_dir is only 2 bits and WHACK is
0, there's no way to unambiguously express some of these combinations.
Certain weapons such as the lucern hammer are defined as combination
piercing/blunt weapons, but the game just sees it as a piercing weapon.
This commit adds a third bit to oc_dir and promotes the WHACK constant
to its own bit. Nothing should be affected by this (wand directions and
the like should remain working as usual) other than the
blunt-and-something-else weapons being defined properly.
It's redundant with g.moves, so there is no more need for it.
Way, way back, it looks like g.moves and g.monstermoves can and did
desync, where g.moves would track the amount of moves the player had
gotten (and would therefore increase faster if the player were hasted)
and g.monstermoves would track the amount of monster move cycles, aka
turns. But this has not been the case for a long time, and they both
increment together in the same location in allmain.c. There are no
longer any cases where they will not be the same value.
This is a save-breaking change because it changes struct
instance_globals, but I have not updated the editlevel in this commit.
When discussing the recent commit that removed makedefs -o from the
build process, nhmall pointed out that a sanity check ensuring all
objects within one class add up to 1000 probability had been removed as
well. This requirement was a perennial thorn in the side for anyone
doing anything that touches object probabilities, because allocating
probability to something meant deciding what to take it away from,
without a good way to evenly distribute that across all the other
members of the object class.
I had gotten around this in xNetHack by removing the sanity check and
making mkobj() total up the probability within an object class and then
using that instead of 1000. This commit takes a similar approach, but
instead of inefficiently recalculating the sum every time mkobj() is
called, it instead computes it at the start of the game or when
restoring the save file and stores it in a global variable.
This fixes a slight bias problem with rings - they are all supposed to
be of equal probability, but there are 28 of them and 1000 is not evenly
divisible by that, so the old formula made the later rings slightly more
likely. Now instead of a 35/1000 or 36/1000 chance, they are all
uniformly 1/28. (Internally they have a oc_prob of 1 now, not 0).
Gems are also weird, because their oc_prob values change every level.
This ought to have still worked without a change, because the arcane
formula for assigning the probabilities would still end up with them
adding to 1000. But I added in code to reset the total gem probability
anyway; this may help make the formula less arcane in the future.
There is still a sanity check against object classes having a nonzero
number of objects but zero total probability, in which case an
impossible will be thrown and every member of the class will be given
equal probability. I also downgraded the "probtype error" panic in
mkobj() to an impossible because it has a reasonable failure case -
return the first item in that class.
This is caused by the bones-pile-making routine using artifact_light()
as a test for whether it needs to call end_burn. Gold dragon scale mail
uses artifact_light(), but only returns true when its owornmask is set.
But owornmask was getting zeroed right before artifact_light() is
called. Fix is to move it right after instead.
Tested that Sunsword is not affected by this (created bones while
wearing gold dragon scales and wielding Sunsword in a dark area; when
returning to them, no light was emitted from the gravesite) because it
always returns true in artifact_light() irrespective of owornmask.
Another SliceHack feature. However, the math implemented by SliceHack
seemed incorrect, so I tweaked it.
Pets previously attacked monsters of up to one level higher than them as
long as they were above 25% health. Now, they will attack monsters as
follows:
100%: up to level + 2 (pets could not attack this high before)
80%+: up to level + 1
60%+: up to same level
40%+: up to level - 1
25%+: up to level - 2
The case that prevents any attacks below 25% health still exists.
des.region() accepted booleans for the joined field, whereas des.room
accepted xchars. These were only being used as truth values, so this
converts the room ones into booleans for consistency. I don't think
accidentally using an int or a boolean wrongly would actually crash the
level generator, but consistency is good.
This converts an schar field in struct mkroom into a boolean; on most
systems these are probably 1-byte types and save files won't be broken,
but it might be best to treat this as a save breaker anyway.
Its value is only used as a boolean, so there's no real need to keep it
as a confusing int.
Shouldn't be a save-breaking change; it doesn't look like g.coder is
saved.
Due to a logic bug introduced when engraving became an occupation - the
code that tests to see whether the player is writing with a weapon that
will get dulled wasn't correctly checking that they were actually
carving an engraving.
Instead of inexplicably paralyzing the player for the duration of their
engraving. Many a character has died by trying to engrave something and
then sitting there diligently writing it while monsters surround and
attack them. (This was especially prominent back in the 3.4.3 era when
repeated Elbereths were viable, but it still occurs today with e.g.
using a hard stone to engrave Elbereth). There were also some other
oddities - for instance, if something teleported the player away while
they were engraving, they would continue to "engrave" (be paralyzed) on
their new location, but would not produce any text there; the full
engraving would be placed on their initial position.
In this commit, I have converted engraving to use the occupation
framework, which treats it as an interruptible activity. This
necessitated some logical restructuring, mostly involving the engraving
being written out in chunks as the player spends more uninterrupted time
on it.
I've tried to keep this free of regressions except for those inherent to
the occupation system.
What has NOT changed:
- The rate of engraving is still 10 characters per turn, or 1 character
using slow methods.
- The formulas for determining how much a bladed weapon or marker can
engrave before getting exhausted are kept. Though this is a bit
convoluted, and if it's not considered important to preserve the
existing behavior, I would recommend simplifying it by decreasing the
maximum engraving length for weapons by 1 so that each point of
enchantment simply gets you 2 characters' worth of engraving (e.g. a
-2 weapon will only engrave 1 or 2 characters before dulling to -3,
rather than giving it a third "grace character".
- The input buffer is still modified based on confusion/blindness/etc
only at the time when the player inputs it (if they gain a
debilitating status while engraving, it will not affect the text). My
personal preference is to make the text affected in scenarios like
that, but it's not strictly necessary to do here, so I didn't.
- Wand messages such as "The floor is riddled by bullet holes", and
blinding from engraving lightning, still appear before the hero starts
to take any time engraving. As noted above, getting blinded by the
wand still has no effect on accurately engraving the text, unless the
hero was already blind or impaired.
What has changed:
- Moving off the engraving or losing the object being engraved with
causes the player to stop engraving.
- Wands can still engrave an arbitrary amount of text using a single
charge, but if the hero is interrupted and decides to start engraving
again, they will consume a second charge.
As it adds a new field to g.context, this is a save-breaking change.
The code for doing this (basically an obj_extract_self() call plus
handling if the object was worn or wielded) was duplicated all over, and
inconsistent - for instance, though all of them updated the monster's
misc_worn_check to indicate it was no longer wearing something in
whatever slot, only one call also set the bit that flags the monster to
consider putting on other gear afterwards.
Under a new function, extract_from_minvent, all this extra handling is
checked in one function, which can simply replace the obj_extract_self
call.
A few callers (such as stealing) have some common code *after* the
object is extracted and some other things happen such as message
printing, such as calling mselftouch if the object was worn
gloves. extract_from_minvent does not handle these cases.
This replaces the arcane system previously used by getobj where the
caller would pass in a "string" whose characters were object class
numbers, with the first up to four characters being special constants
that effectively acted as flags and had to be in a certain order.
Because there are many places where getobj must behave more granularly
than just object class filtering, this was supplemented by over a
hundred lines enumerating all these special cases and "ugly checks", as
well as other ugly code spread around in getobj callers that formatted
the "string".
Now, getobj callers pass in a callback which will return one of five
possible values for any given object in the player's inventory. The
logic of determining the eligibility of a given object is handled in the
caller, which greatly simplifies the code and makes it clearer to read.
Particularly since there's no real need to cram everything into one if
statement.
This is related to pull request #77 by FIQ; it's largely a
reimplementation of its callbacks system, without doing a bigger than
necessary refactor of getobj or adding the ability to select a
floor/trap/dungeon feature with getobj. Differences in implementation
are mostly minor:
- using enum constants for returns instead of magic numbers
- 5 possible return values for callbacks instead of 3, due to trying to
make it behave exactly as it did previously. PR #77 would sometimes
outright exclude objects because it lacked semantics for invalid
objects that should be selectable anyway, or give slightly different
messages.
- passing a bitmask of flags to getobj rather than booleans (easier to
add more flags later - such as FIQ's "allow floor features" flag, if
that becomes desirable)
- renaming some of getobj's variables to clearer versions
- naming all callbacks consistently with "_ok"
- generally more comments explaining things
The callbacks use the same logic from getobj_obj_exclude,
getobj_obj_exclude_too and getobj_obj_acceptable_unlisted (and in a few
cases, from special cases still within getobj). In a number of them, I
added comments suggesting possible further refinements to what is and
isn't eligible (e.g. should a bullwhip really be presented as a
candidate for readying a thrown weapon?)
This also removed ALLOW_COUNT and ALLOW_NONE, relics of the old system,
and moved ALLOW_ALL's definition into detect.c which is the only place
it's used now (unrelated to getobj). The ALLOW_ALL functionality still
exists as the GETOBJ_PROMPT flag, because its main use is to force
getobj to prompt for input even if nothing is valid.
I did not refactor ggetobj() as part of this change.
This probably won't happen in practice, but it is a good safeguard
if this ever does happen (it happened for me in debugging when I wished
to have no "regular" rooms and only generate themed rooms).
This sets the minimum level depth of "Spider nest" to 10, somewhat above
the difficulty of an individual giant spider, because a whole room full
of them is a tougher challenge. Note that this isn't the only possible
fix to this problem; another solution would be to alter the special case
in mktrap that hardcodes a giant spider to generate with each web to
produce cave spiders if giant spiders would be too tough. Even then, a
lower difficulty cutoff is probably still warranted for this room, since
a large number of cave spiders might be too tough for level 1 or 2.
This also sets the minimum level depth of "Boulder room" to 4, based on
the fact that individual rolling boulder traps normally can't appear
until level 2, and having a bunch of them in one place which may be
required to reach the downstairs could be problematic.
This doesn't do anything to address the "Mausoleum" room problem, in
which a master or arch-lich can generate and immediately warp out and
attack the player. Even with a high difficulty threshold, it won't fix
the problem of these liches generating out of their normal difficulty
and Gehennom constraints.
Other potential candidates for difficulty thresholds:
- "Trap room": This room might be perilous on the first few levels,
especially if the level generates with it blocking the way to the
downstairs.
- "Massacre": Doesn't have any particular hazards, but might be
interesting if it only generated at deeper levels.
The system of themed rooms currently makes it so that any themed room
can potentially generate anywhere a themed room can be placed. This is
problematic in the long run, since it makes it difficult to design new
rooms that are an appropriate amount of challenge at all levels of the
dungeon. (A few themed rooms already have this problem: a hero starting
out on level 1 probably won't live very long when the neighboring room
is full of giant spiders, or an arch-lich has generated in a mausoleum
nearby).
This commit adds optional "mindiff" and "maxdiff" properties for
themerooms defined as tables and exposes level_difficulty() to Lua. A
themeroom whose mindiff exceeds the current level difficulty, or whose
maxdiff is lower than the current level difficulty, is prevented from
being selected.
Because the set of rooms eligible to generate on a given level is no
longer fixed, the total frequency of all the rooms can't be computed
once per game when the file is first parsed, as it was before. In place
of this, the themerooms_generate() function now uses a reservoir
sampling algorithm to choose a room from among the eligible rooms,
weighted by frequency.
Consider the following scenario: There's a level where there's a zone of
des.replace_terrain() between the stairs and some other objective, and
the terrain is something non-walkable like trees. There's a chance that
the path is entirely blocked off by random replace_terrain, so you
make the level set terrain to '.' along a randline (or normal line, or
whatever) between the randomly placed stairs and the other side of the
replace_terrain zone. The problem: this overwrote the stairs with a '.'
as well.
This can be worked around in the lua file by first picking the desired
location of the stairs, then setting the terrain that overlaps with the
stairs, then doing des.stair() after that, but this is awkward and hard
to read.
So this makes it impossible for anything calling SET_TYPLIT (only called
in sp_lev.c) to overwrite stairs. I can't really think of a situation
where a level designer would want to define stairs, then maybe overwrite
them.
This was leading to problems with special themed rooms which were
irregular. Walls of ordinary rooms count as part of the room, and
irregular rooms should be no different.
This also makes doors on the room edge be considered as part of the
room, which affected special room entry messages and filling.
All irregular rooms' walls getting marked as SHARED instead of their own
room is probably a latent bug in upstream NetHack, but will prevent
future issues for when/if themed rooms that involve special
rooms/subrooms get added.
This is a simple && vs || bug. The clear intention of the code is that
stairs aren't supposed to generate in themed rooms unless there is no
other choice.
Fixes#348
Aimed at fixing the problem where the player knows they're going to
forget a spell in a few thousand turns, so they go back and get the
book... only to find out that they "know it quite well already", and
need to wait an indeterminate amount of time until they are on the verge
of forgetting it (< 2000 turns) before the book will let them read it
again.
This commit simply removes that 2000 turn limit, so the player can fully
restore their memory at any time with the spellbook. Naturally, this
still consumes a read charge, so the book won't ultimately last as long
if you keep rereading it early.
If you do have more than 2000 turns left, the game will prompt you to
confirm that you do want to refresh your memory anyway. As before,
rereading with fewer turns will not prompt.
This is an omission in the filled/prefilled unification. The default for
filled on regions now being 0 meant that regions that had previously had
no need for any fill declaration at all (regions' prefilled defaulted to
0 before this, the effect being to fill them) now failed to get filled.
The rule of thumb is that all des.regions with a type for which filled
is meaningful (e.g. special rooms) should declare the fill status. I
added it to a bunch of temples even though this doesn't really seem to
affect anything there (the priest and altar come with the altar
definition). I assigned temples filled=1 and filled=2 loosely based on
if there is ever being some other generation that would put other
furniture or items in a temple, but the distinction should not affect
anything right now.
Cases fixed where non-temple regions weren't getting filled:
- Barracks, a graveyard, and shops in Tou-goal
- The beehive in the Wizard's Tower
This unifies the two separate special-room-stocking code paths, one in
the standard dungeon generator and one in the special level generator
(neither of which reacted to themed rooms, which is the reason for this
commit) into the end of makelevel(), placing the special room stocking
as the very last step of level creation.
Under the new system, when a regular or special level decides to create
a special room, it sets that room's rtype, but the room is not stocked
until later. It already worked this way for special levels, so the main
difference here is in the normal level generation, where the mkroom
family of functions identifies and marks a room as a special room, but
stops short of filling it. (I suppose perhaps the mkroom, mkzoo, mkshop
family of functions would be better off changing their names to
"pickroom" and so on.)
This also restructures makelevel() itself a bit, but the only real
change is that the paths that call makemaz don't return immediately
afterward; they continue to the special room stocking code. Also, this
code was lifted from fill_special_rooms, which is now not used
anywhere, so it has been deleted.
I don't really like how fill_ordinary_room is in mklev.c and
fill_special_room is in sp_lev.c; they seem like they'd be better off in
mkroom.c, but in the interest of not making unnecessary code changes,
I'll just recommend it.
The plan is to unify special room filling code and cause special rooms
to be filled as the very last stage of level creation. Since this will
occur after fixup_special, it was necessary to address the one remaining
piece of code in there that affects special room filling. (The Medusa
code remaining in there doesn't have to do with special rooms.)
There is code in fixup_special for stocking Medusa's lair with statues
of players from the leaderboard. It makes two assumptions: that there
will always be at least one room defined on Medusa's level, and that
the statues should be placed in the first room defined. In the process
of removing prefilled, some of these rooms suddenly became non-rooms,
and this caused problems. This commit ensures that the regions for
turning into rooms to hold the statues are present and come first.
In the process of writing this commit, I discovered a bug: the statue
stocking code for medusa in fixup_special naively chooses the spot at
which to place its final statue by selecting independent x and y
coordinates with somex and somey. This is responsible for a statue
occasionally being embedded in a wall or in iron bars on medusa-2 and
medusa-4: the rooms defined to receive statues are irregular, and some
of the possible coordinates happen to be walls, bars, and water.
The proper fix here is to add lua functionality so that the level
designer can specify that they want a leaderboard corpse or statue, and
remove the medusa special case from fixup_special, but that's rather
out of scope for what I'm doing here.
The fill_special_rooms function was only stocking two types of rooms:
top-level rooms in g.rooms, and their immediate subrooms. If there were
a special room 2 or more levels down, it would not get filled. (No
special levels currently define such a special room, so this bug is
latent.)
To address this, I changed fill_special_rooms to iterate only through
the top level rooms, and fill_special_room to recurse through all of its
subrooms (if any) before filling itself.
The existing system was a confusing mess of competing names (filled,
needfill, prefilled, etc) that had varying semantics, with prefilled
being the worst offender as it meant at least three different things in
various contexts. This commit unifies everything in the code under
"needfill", and everything in Lua under "filled", which defaults to 0
everywhere.
This also removes the second argument to fill_special_room; that
function now just checks the needfill of the room it's passed. As
before, a filled == 2 value is used for a special room to indicate that
the room should set the appropriate level flag, but shouldn't actually
be stocked with anything (for instance, King Arthur's throne room); the
difference is that this now comes directly from the lua script instead
of being manipulated within sp_lev.c.
The prefilled argument had one use case that is occasionally used in the
level files: if the level designer had specified an ordinary region with
prefilled = 1, it would become a room to control monster arrivals on a
level -- monsters that arrive within the bounds of a room are supposed
to stay there.
However, not all of the places where the comments indicated this was
being used were using it correctly; I tested this by letting a few
monsters fall through the knox portal (they're supposed to be
constrained to the entry room) and waiting a hundred turns, then going
through the portal; they were not constrained to the room and had
"wandered" through its walls.
Instead of trying to maintain this special case, I have added an
optional "arrival_room" boolean argument to des.region, which forces it
to create a room for the purposes of constraining monster arrival.
I have gone through and replaced occurrences of prefilled in lua files
with the appropriate filled option (or arrival, as needed). In some
cases, that resulted in questionable regions such as a filled ordinary
area in a non-themeroom (I just dropped the filled=1), or an area which
didn't do anything, not even lighting (which I deleted).
No code in this function would actually do anything if it were called on
a themed room, but since it is pretty clearly intended for the "regular"
special rooms, it's probably best to explicitly avoid themed rooms as
well.
I noticed that any subrooms created within a themed room were bare -
they never had any monsters, objects, traps, or anything really,
regardless of whether filled = 1 was set on them. As a result, they're
pretty boring.
It turns out that the code in makelevel() responsible for stocking
ordinary rooms with stuff only looped through g.rooms, and completely
ignored subrooms. (Subrooms would not get stocked with items by virtue
of being part of the larger room; I tested this by dialing the item
generation in rooms way up, and none of those items ever got placed in a
subroom.)
To fix this, I've extracted the code that populates an ordinary room
into its own function, fill_ordinary_room, and made it recurse into its
own subrooms. (I also renamed fill_rooms and fill_room to include the
word "special" in their names, because they only deal with special
rooms.) Note that since special rooms follow a separate codepath, an
ordinary subroom of a special room won't get stocked; perhaps these
functions should be unified in the future.
The fill_ordinary_room code is pretty much a verbatim cut and paste from
makelevel, so there is not currently any consideration for the size of
the subroom or the fact that it is a subroom with respect to how many
monsters, traps, objects, etc get placed.
I'm not sure whether other things such as stair selection will ever
select themed room subrooms, or whether they too only look at g.rooms.
This commit is intended to fix the bug where a pet will get fixated on
an unmoving monster and stop moving itself. I described the cause in the
github issue; the gist is that the pet AI chooses the unmoving monster
as its ranged target, doesn't do anything when it calls mattackm
(because it doesn't have ranged attacks), then returns a value
indicating it didn't move and can't take further actions.
I initially implemented a fix that refactored mattackm to distinguish
between "attacker missed" and "attacker did nothing", which the pet AI
could then use to determine whether the pet could continue doing things.
But then I realized that if mattackm is called with non-adjacent
monsters, a return of MM_MISS more or less unambiguously indicates that
the attacker did nothing (because the ranged functions it calls like
breamm don't actually check to see whether the target was hit, just
whether the monster initiated the attack.) So, this only really needed
to check whether mattackm returned with MM_MISS.
I also found a probable bug in mattackm, in that the thrwmm call isn't
treated the same as breamm or spitmm. In the latter two, mattackm
returns MM_HIT even though it doesn't check whether the ranged attack
actually hit its target. But there was no logic doing the same for
thrwmm, so this commit also adds that. (Otherwise, a pet could possibly
use a ranged weapon attack and then get to keep moving on its turn.)
It doesn't mention anywhere that newmextra() is responsible for
initializing struct mextra with null pointers to the various
sub-structs. So, when one follows the instructions and doesn't know or
remember to do this, they'll get segfaults when something tries to read
the uninitialized pointer to their new struct.
After some discussion with Alex Smith, it seems like a good change for
both gameplay and realism that candles' light radius should decay
quadratically instead of exponentially. Now a light radius of 4 from
candles can be accomplished by burning 9 candles, and players who find a
lot of candles might even be able to get up to 5 (16 candles) or 6 (25
candles).
The main impetus for this change is that with the existing formula, the
more candles -> more light mechanic was more or less useless outside of
wizard mode, because you needed 49 candles to do better than a lamp.
The bug was that you could easily displace peaceful sleeping monsters.
The source of this was that there was no check for sleeping in the
attack() function that checks for immobilization.
As well as adding the sleeping check, this logic is modified so that it
makes more sense: if a monster is immobilized at all (sleeping, frozen,
or mcanmove = 0) it always "doesn't seem to move!" You can't randomly
displace it anyway 1/6 of the time.
Sessile monsters who are otherwise not immobilized don't move 5/6 of the
time, but can be displaced with the other 1/6.
Pets can no longer be displaced out of a trap, because that was
inconsistent with peaceful monsters refusing to be displaced out of a
trap. The untaming-via-displacement-out-of-trap code is removed.
Pets also now have a better survival instinct: they follow the code for
peaceful displacement into a bad position, and refuse to swap places.
This means it's no longer possible to accidentally kill a pet by
levitating/walking over water and displacing it.
A side effect of making is_safepet() count peacefuls. Now checks
directly for a trapped, peaceful monster and says "they can't move out
of the trap"; this is inconsistent with pet behavior, and pet behavior
should probably be changed to be in line with it (ie they can't be
displaced out of a trap at all.)
Also refactor the code here a bit: a bunch of different if statements
have the exact same resetting code and steed resetting code in them.
Change this to a boolean flag and put the resetting code in one place
checked by that flag.
Shouldn't be able to displace priests since that could theoretically
eventually force them out of their temple, which would probably cause
problems. The Oracle doesn't usually move anyway, but seems like she
should "not want to swap places" in any circumstance.
Changes domove() code to allow displacing peaceful monsters.
Specifically, is_safepet() now returns true if the monster is peaceful.
Peacefuls are slightly pickier than pets about whether they consent to
being displaced: they will not displace if a goodpos() check fails for
the displaced space, or if there is a trap on the displaced space, or if
they are your quest leader. is_safepet should probably be renamed to
something else.
In the process of doing this, some other changes were made: the code now
checks whether the player and monster should be swapping places at all
first (previously it ran some code for displacing pets out of traps
first, which was a little weird if the displacement didn't actually
happen.)
In the original commit for this, I needed to guard the spoteffects()
call made in domove with a clause testing whether the player actually
moved; it was previously possible to fail to displace a monster and then
re-trigger a trap on the space you were still standing on. However, the
devteam has apparently put in an if (u.umoved) clause in the same place
and serving the same purpose.
This was already allowed for the other montype-compatible objects like
statues, corpses, eggs and tins. I don't see a reason why figurines
shouldn't be part of this group; perhaps it was an oversight.
Intended to simplify many of the math.random calls currently in use, and
make them more semantic and thus more readable.
The dice function d() takes either a two-argument form which is the same
as in the C source (number of dice, faces per die) or a one-argument
form that rolls a single die.
The percent(N) function returns true N% of the time.
Uncomments and makes available selection.gradient in Lua. (The backend C
code for this still existed, it just wasn't used.)
The only valid way to specify a gradient is with a table. I considered
adding non-table versions, but decided that there are too many
independent variables that can be optional. A non-table version, without
named parameters, would be confusing to read, especially since most of
the arguments are ints.
Also adds an impossible in the (possibly unreachable) case that
selection_do_gradient gets called with a bad gradient type.
Rather than spitting out an error code number that is not particularly
useful. The string contains the line number and the nature of the error,
which is much more useful for debugging or reporting an issue.
The impetus for this was to avoid ugly constructions such as the one
below (none of which currently appear in vanilla NetHack):
mongets(mtmp, LONG_SWORD);
struct obj* sword = m_carrying(mtmp, LONG_SWORD);
if (sword)
/* do thing to sword */
Most cases where mongets is used discard the returned value (which used
to be the created object's spe); the only places that do use it are the
series of statements that give various human monsters armor and prevent
them from getting too much armor. These statements included hardcoded
constants representing the base AC of the armor, which would have caused
discrepancies if armor's base AC were ever changed.
With mongets now returning a pointer to the created object, it can just
be passed into ARM_BONUS instead, which covers both the base AC and the
enchantment. (It will also cover erosion, if anyone ever decides that
armor should rarely generate as pre-eroded).
The overall algorithm is not changed by this; human monsters should
receive armor with the same probabilities as before.
Another SliceHack feature. It's possible for you to eat the chameleon
tin and turn into a fiery monster that burns off the slime in its
natural form, either extremely luckily at random or if you have
polymorph control.
This buffs the blessed effect of the teleport scroll by providing the
reader control over their destination even if they lack teleport
control. This seems like it makes the blessed/uncursed distinction
actually meaningful, rather than mostly pointless.
Ported from SpliceHack, and generalized to all shapeshifters (Splice
only implemented it for chameleons). It's very aggravating when your
powerful but hungry pet chows down on a shapeshifter before you can stop
them and then turns into something much more useless, so this aims to
prevent that.
The extreme circumstances under which a pet will eat a shapeshifter are:
1. The pet is starving, and prefers polymorph to starvation
2. The pet's tameness is 1
The reasoning behind the second condition is that if you mistreat your
pet almost to the point of untaming it, it might want to take a chance
on turning into something that might get some more respect from you.
Practically, whenever this happens, this will result in the player now
owning a newly polymorphed and *still* nearly feral pet.
This makes a lot of sense. Why would they hate one artifact sword so
much and not really care about the one that is especially designed to
kill their type personally?
The scroll of remove curse is trivially identified by checking inventory
after reading it to see whether anything became uncursed. This leads to
annoying tactics like remembering which scroll you just read so you can
go call it "remove curse" on the discoveries list.
This simply autoidentifies it when an item that was known to be cursed
has its curse removed.
This is aimed at providing a little quality of life in the form of not
having to divest yourself of your sources of magic resistance before
using a level teleporter. The player is already able to use regular
teleport traps while Antimagic; there's no reason why it should be
different for level teleporters.
This ultimately comes from "Stevie-O's level teleporter jump patch", by
way of SliceHack. I simplified it a bit: deliberately jumping onto the
trap always takes time even if it fails to levelport you (which would
only happen with level teleporters in the End Game, which don't exist).
Another feature from SliceHack. Randomly averting an instadeath might
seem a little too generous, but the only time you get food poisoning is
if you're a new player who hasn't learned about tainted corpses yet or
if you just did something stupid. So, be a little nicer in those
scenarios.
If you survive, your Con silently decreases by 1. Hey, it's better than
dying.
This is also from SliceHack, but with the odds of enlightenment toned
down a bit, to 4/9 for a blessed potion and 1/6 for an uncursed potion
(SliceHack had it at 50% blessed, 20% cursed, and strangely, 0%
uncursed). It gives a much-needed use to one of the potions that's
commonly blanked or discarded.
Jumping, Newton's 3rd Law hurtling, and throwing an iron ball:
attempting to do any of these in such a way that you would diagonally
pass between boulders/walls causes the Luck penalty. However, none of
these actually get you through the diagonal gap, thus they can't be used
to cheat and the penalty doesn't make sense.
For much the same reason as the horizontal teleport message: various
forms of level teleportation produce no message indication at all that
you just ended up somewhere else.
It's useful to get a message as indication you suddenly moved somewhere.
For instance, MSGTYPE=stop can be used on this to avoid bumbling in the
wrong direction after a spontaneous uncontrolled teleport.
This feature is originally from SliceHack, but the original code
directly edited the monmove code, whereas I thought it was cleaner to
use the existing mtrapseen code. Thus, this commit just marks trapdoors
as "seen" for all non-mindless monsters generated in the Castle level
(the same way all monsters in Sokoban are marked aware of pits and
holes).
This change prevents these Castle monsters from moving onto trapdoors
97.5% of the time. (A determined player can still patiently sit and wait
for everyone in the castle to plunge like lemmings into the trapdoors,
but it will now take 40 times as long.) Also unlike SliceHack, this only
excludes mindless monsters - not all non-humanoids. There are plenty of
intelligent non-humanoid monsters generated right next to the trapdoors,
after all.
This is aimed at better flavor (the inhabitants of the castle should
know about the traps in their own area) and better scenery in the Valley
(doesn't seem as much of a valley of the dead if there are hordes of
soldiers milling around down there).
I considered sticking an in_mklev condition onto this if statement, so
that monsters spawned into the Castle after its creation will fall down
the trapdoors, but ultimately decided against it.
Rather than just informing the player that saving and reloading might
fix the problem, they are now encouraged to report the problem to the
value of DEVTEAM_EMAIL. If the sysconf specifies SUPPORT, that is also
presented as an option.
Message is a reference to The Silver Chair. Most of the other races had
their own messages already, but gnomes would just default to discussing
dungeon exploration, which doesn't make that much sense most of the
times when you would be chatting to them in their own mines.
The quotation is edited from the source to reflect the dungeon
environment, but the sentiment is actually pretty spot-on given the
average player's win ratio.
Note: this doesn't interfere with the South Park gnome speech added to
3.6 a while ago; that only occurs when hallucinating and this only
occurs when not hallucinating.
From SliceHack. Note that this refers to the description of the physical
bottle; it's a substitute for "phial", "carafe", "flask", etc. such as
are seen when a potion crashes on someone's head. They don't obscure the
randomized appearance or actual potion identity.
The SliceHack version evidently went through several revisions; just
take the current one.
The general idea here came from SpliceHack -- give each alignment a
unique effect in what happens to its sacrifices -- but the "puff of
smoke" in Splice seemed too small.
Triggers when you feel more confident in your skills. This is to address
a problem I have heard about several times from newer players: unless
you pay close attention to the guidebook, nothing in the game actually
indicates that you can level up your abilities and how to do it.
Experienced players don't need this message, of course; they can hide it
via MSGTYPE if they really hate it, but I additionally added a clause
that prevents this message from being displayed more than once per game
session. (It didn't seem important enough to make a save field for.)
Backported from TNNT. Only affects dumplog pline history, not any other
form of pline history.
The impetus for this is to avoid dumplogs full of "Unknown command foo."
messages which don't provide any value for people reading the file. In
many cases, these messages crowd out the actual message history, making
it hard to reconstruct what happened.
It's confusing and served no purpose; a spoiled player knew what it is,
an unspoiled player might think it was a hook-hand or something. Now
they all show up as grappling hook.
That message implied something to do with an effect happening to the
hero that causes them to feel cold, such as taking cold damage.
Change it to "You hear a deep cracking sound" instead.
copperwater