Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Another 'consolidate small changes' PR #54

Closed
wants to merge 45 commits into from

Conversation

entrez
Copy link
Contributor

@entrez entrez commented Oct 12, 2021

Various things I have noticed while playing xnethack recently. Despite the name of the branch only some are bugfixes.

Fixes #56 (along with the merest handful of other things)

Don't count concealed/disguised mimics as a 'seen/detected' monster for
monster_census(TRUE); as a result, the 'a giant mimic appears from
nowhere!' message won't be printed when a hidden mimic is generated
from a wand or scroll of create monster.
Simple mixup between magr and mdef.

Even now that the parameters are in the correct order, this could cause
problems if mdef == magr, since mon_nam_too's '(him|her|it)self' won't
work nicely with s_suffix (would produce '...himself's attack' etc).  I
wrote some code to turn 'himself' into 'his own', etc, in s_suffix, but
I think the combo of s_suffix and mon_nam_too only happens in this one
spot, and I'm not sure if it's actually possible for a monster to attack
itself in that way, so I didn't include it.  If it turns out that it can
happen, I can dig that back out later, but it might be simpler to
rephrase the pline.  If someone sees 'himself's' while playing it will
be a good indicator to revisit this.
Removal of armor by a foocubus increases its weight and may push the
hero over the limit to the next encumberance category, but the
associated message wasn't appearing until the next turn.
Item material names were removed from any use of 'distant_oname' in
bb90455, intended to target a specific bug in cursed_gear_welds;
this made it impossible to determine the material of an item from a
distance, since farlook uses distant_oname.

Fix the bug which prompted bb90455 in a slightly different way, by
copying over the material for the fake object in minimal_xname if it's
an item type which includes its default materials in its name.  This
means the cursed_gear_welds message will be 'the plastic gloves weld
themselves...' rather than 'the gloves weld themselves...' for objects
which force the inclusion of material names, but I think it's worth the
slight loss of minimalism for those particular item types to regain the
ability to judge object materials from a distance.
A false positive match with '[material] ring' when wishing for
'[material] ring mail' meant that the wish wouldn't be recognized and
the player would get a 'nothing fitting that description exists' msg.
Add a specific check for 'ring mail' in not_actually_specifying_material
so that it won't produce false positives in this case.

Of course, it's impossible to wish for a particular material outside of
wizard mode anyway, so this is unlikely to impact a normal game.
...due to only locked containers being available

The message "there's no other container to put items in" suggests that
no other container existed on the spot (or in player inventory) at all,
but would also be printed if the only other containers available were
known to be locked.  Clarify this a bit by rephrasing the message and
specifying that no 'unlocked containers' exist, if the only ones
accessible are locked.
Show the option to transfer items from one container to another only
when there are actually other valid containers to transfer to.  This is
in line with way other take out/put in options work, since they are
shown only when they could actually be used.

In the process fix a bug affecting 'traditional' menustyle, where 't'
showed up when there were items available to put into the container
rather than items available to take out.

The previous commit which rephrased the 'no valid containers' transfer
failure feedback isn't made entirely moot by this, since when using
'traditional' menustyle options can be selected even if they aren't
included in the prompt, but this does mean the failure message won't be
seen nearly as often.
@entrez entrez force-pushed the more-small-fixes branch 4 times, most recently from 19174b4 to a295648 Compare October 14, 2021 18:17
Use "transfer" as the verb associated with the action, and adjust the
transfer container selection menu so that it doesn't have a bunch of
extra empty space when a valid container exists in inventory or on the
ground, but not both.
'It crumbles' is ambiguous about which of the two items involved is
crumbling, so make the message more specific by naming the rock.

Also distinguish between a rock 'crumbling', producing flint and
destroying the rock, and 'crumbling slightly', which has no effect.
Although the commit message in b5a46e3 seems to suggest otherwise, the
rock is only destroyed when flint is produced: make it clear that the
rock isn't completely crumbling in cases where nothing happens.
When generating a spellbook as a boon from prayer, the object is
generated, and then the otyp may be changed if it is an already-known
spell or some other undesireable spellbook type.  This could result in
the spellbook's material being mismatched, if the original spellbook was
vellum or parchment (which are leather) and the otyp was changed to
another spellbook (others are paper), triggering an 'invalid material'
sanity check.  Make sure to reset the spellbook material to default
after potentially re-rolling the otyp.
Rock-breaking for flint added the ability to #rub two rocks together to
produce flint, but the callbacks for those actions added in the getobj
refactor didn't permit selecting rocks.  Restore the ability to choose
a rock, but don't suggest it (as was the case prior to the changes to
getobj, as far as I can see).
When a maze level room had its walls removed, secret doors were left
alone, resulting in secret doors hanging out by themselves without any
nearby connecting walls.  Ensure secret doors are also deleted when
removing the room's walls.
'Skeleton key' distinctly refers to an object rather than a monster.
Ensure the material of the low boots created by polymorphing a crocodile
corpse is set to leather, rather than creating knockoffs of the shoes
Lady Gaga wore to the 2010 VMAs.
'%' was represented as '%%' in a tin label that is printed verbatim,
without going through printf-style formatting -- although this is the
right way to represent a single '%' in tin labels which are formatted,
in this case both '%' chars were printed.
@entrez entrez force-pushed the more-small-fixes branch 2 times, most recently from 249dd13 to 7c0b8ba Compare October 21, 2021 12:34
Being killed by an explosion from a thrown potion of oil or a
FIRE_BLAST door trap would give something like "caught herself in her
own fireball" as the cause of death.  Revise those messages so they
don't blame the poor dead hero for something she didn't do.
Differing oerodeproof values only prevented merging for weapons and
armor, but now that confused enchant weapon can be used to make a
variety of other items fixed, the difference needs to block merging for
them as well, to prevent 'free' erodeproofing being provided via merge.
The oeroded bit pulls double duty as a way to track dilution for
potions; so that glass weapons can have 'shatterproof' affixed to their
name glass objects were permitted to continue through add_erosion_words,
so potions had 'burnt' added alongside 'diluted'.

Also allow other potentially-'indestructible' items such as wands and
rings to get the proper adjective included in their name.
Confused enchant weapon was expanded to work on potions, rings, etc, but
because oeroded tracks dilution for potions, it was resetting potion
dilution in the process of making the potion 'indestructible'. Only
reset oeroded/2 for erosion_matters objects, since other objects may
use those bits for other purposes which don't make sense for fooproofing
to reset.
I noticed some strange-looking weapon onames caused by capitalizing only
the first word in the weapon name, e.g. 'Wicked Worm tooth'.  Use
up_all_words instead of upstart so that multi-word weapon names have
each of their words capitalized.  This means the special definition for
'Morning Star' can be removed, since it will render that way by default.

Also add 'staff' as a shorter term for 'quarterstaff' (akin to 'hose'
for 'rubber hose'), and 'blade' as a alternative term which may be used
for swords.
A worm tooth produced by killing a long worm probably shouldn't be given
a random name like 'Worm Tooth of Sorrow' -- after all, who is supposed
to have named it?  (Admittedly, though, 'Hungry Worm Tooth' fits rather
well.)
not_fully_identified wasn't updated to account for the fact that glass
items, nontraditional classes like scrolls and rings, and various other
previously-impossible targets can now be fooproofed with confused
enchant weapon.  Make sure the function is updated to reflect the types
of items where rknown matters, so that you can formally learn their
oerodeproof status.
Allow wishing for 'indestructible' rings, wands, scrolls.  Also retain
'indestrucitble' status when polymorphing one of these items.
@entrez entrez force-pushed the more-small-fixes branch 2 times, most recently from 2a64df9 to 4a973fb Compare November 10, 2021 01:13
Produces an impossible when a maze fails to cover the entire level.
walkfrom's recursion worked in such a way that it could go back to every
level _but_ the very first position it started from. This meant that if
its first move could get it stuck between a room and a corner of the
map, the rest of the map would go unfilled.

This behavior can be reproduced consistently by placing a room with
roompos.x = 5, roompos.y = 3, then starting the mazewalk with mm.x = 3
and mm.y = 5.

Adjust walkfrom so that it doesn't irrevocably commit to a direction
before recursing, which should solve this problem.  Fixes copperwater#56.
Mimics generated during level creation inside a special room were using
random disguises rather than contextually-appropriate ones (e.g. based
on the pool of valid shop inventory), because set_mimic_sym was checking
orig_rtype, which isn't initialized until level creation is finished.
As a result all rooms were treated as though they had type OROOM for the
purposes of selecting an appropriate disguise.
When merging the objects.c -> objects.h move, the 'elven leather helm'
name was changed to 'elven helm', but its alternate 'leather hat'
description wasn't removed.
Kicking an iron secret door would print "your kick uncovers a secret
door" but would fail to actually update the symbol on the map until an
update was forced some other way.
Because secret doors fall into the 'rock' case -- which didn't check for
door material -- rather than the 'door' case in mfndpos(mon.c), all
secret doors, iron and wood, were permitted as a valid next move for
digging monsters. And since mdig_tunnel(dig.c) returns early without
opening or destroying the door if it's iron, the monster would appear to
move through a solid iron door.  Disallow digging through secret doors
in mfndpos if they're made of iron.
The visit counter of a shopkeeper persists in a bones file; as a result,
bones which included a shop visited by a dearly departed hero would
insert the shopkeeper's name into the 'cursing shoplifters' message,
since the only criteria for that to happen was a positive visitct.

When entering a pre-visited shop in a bones file, the shopkeeper will
treat you like a returning customer if you were the most recent visitor
before the bones were saved (effectively, if they are your own bones),
and otherwise will not.  Use the same criteria when determining whether
to insert their name in the level sound message.
Include wands in the "default magical" class list.
Looting a container made of a hated material would hurt the hero and
deal damage even if wearing gloves.
When mold 'grows' on a corpse under a boulder, the mold shows up on top
of the boulder, which looks sort of weird and doesn't make a ton of
sense.
src/apply.c Outdated
pline("It crumbles.");
You("bang %s%s on %s.", ((obj->quan > 1L) ? "one of " : ""),
the(xname(obj)), the(xname(tstone)));
pline_The("rock crumbles%s.", flint_made > 0 ? "" : " slightly");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a oversight in the initial implementation of this feature. The rock should crumble completely even if no flint is made.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense -- we had a conversation about the intended behavior a while back and I think I misunderstood what you said (you talked about an issue with the current way it works, something like "if the flint doesn't break you can just keep trying until you get some flint", and I thought you were describing the intended behavior). This makes more sense to me.

g.killer.format = KILLED_BY_AN;
Snprintf(g.killer.name, sizeof g.killer.name,
"exploding %s",
olet == BURNING_OIL ? "fire bomb" : "door");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can catch yourself in your own burning oil though, in which case the hero should be blamed in the death. "caught themself in their own fire bomb" or something similar. This could probably be differentiated with a g.mon_moving check here?

@copperwater
Copy link
Owner

copperwater commented Feb 13, 2022

Summary of commits cherry picked into 7.0-savebreaking branch:

src/mon.c Outdated Show resolved Hide resolved
src/mon.c Outdated Show resolved Hide resolved
@copperwater
Copy link
Owner

Commits fall into 3 basic categories:

  1. merged and pushed to 7.0-savebreaking branch, these can be removed from the PR
  2. obsoleted by changes in the 7.0-savebreaking branch, these can be removed from the PR
  3. I had questions or feedback

@entrez
Copy link
Contributor Author

entrez commented Feb 16, 2022

I've uploaded some of these 'fixup' commits without autosquashing them so you can see and evaluate the changes more easily.

@entrez entrez closed this Mar 10, 2022
@entrez entrez deleted the more-small-fixes branch March 10, 2022 00:57
@entrez entrez mentioned this pull request Jun 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gehennom level with no maze
2 participants