Most shop messages accurately identify the shopkeeper even when he
or she can't be seen, but some also include a pronoun reference that
ended up as "it" or "its" when not seen. Extend pronoun selection
so that visibility can be ignored: noit_mhe(mon), noit_mhim(mon),
and noit_mhis(mon). Note that despite being called noit_foo(),
those will still return "it" if mon is neuter.
"Accurately identify shopkeeper" is misleading if the hero is
hallucinating; a random shopkeeper name is used then. noit_foo()
yields the pronoun applicable to the actual shopkeeper and might
not match the gender of a hallucinatory name. That could be fixed
in a couple of ways (add shk_mhe()/shk_mhim()/shk_mhis() and either
pass them the randomly chosen name so that they can figure out the
appropriate gender, or just have them use a random gender whenever
hallucinating) but I don't think that's worth bothering with.
A bunch of shop messages needed noit_foo(); only a couple of those
have actually been tested. A bunch more were using shkname() at
the beginning of a sentence where Shknam() should be used instead.
(All the existing shk names are already capitalized so there's no
noticeable difference.)
The three places outside shk.c and vault.c which directly use
pronoun_gender() have been successfully tested.
Fixes#121
Fix githib issue #121 - shopkeepers don't charge for consecutive
acts of vandalism on the same square. pay_for_damage() keys its
action on the 'when' field of the damage structure, and when a
second type of damage gets added to existing damage, that wasn't
being updated. Both bits of damage (broken door or dug wall plus
trap created at same spot) get repaired together but shopkeeper
wasn't challanging hero to pay for the second one (trap).
The repair process had issues too. If you broke a shop door, paid
off the shopkeeper, then left the level before the repair took
place and came back after (or rather, catching up for lost time
repaired it when you returned to the level), it didn't actually
get fixed and remained a doorless doorway that was considered to
have been successfully repaired (record of damage discarded).
Unless there was also a trap there; then the door did get properly
fixed when the trap was removed.
Object scattering during wall repair was bypassed if trap removal
took place.
Also, trap removal while off level still gave messages when it took
place after you returned. I didn't try to verify that; it's possible
that vision is in a state where you can't see the repair even if you
return to a spot where it would be visible. But the return value
from the repair routine was one which wanted a message instead of
the one to suppress messages.
Not addressed: digging a pit inside a shop (aside from in doorway
or breached wall) is not treated as damage which should be repaired.
This includes the case of digging up a grave which converts the spot
into ordinary floor (plus pit trap).
Stealing a shop object from outside the shop with a grappling hook
would result in that item being left marked 'unpaid' after the shop's
bill was treated as being bought and not yet paid for. This led to
"unpaid_cost: object wasn't on any bill" every time inventory was
examined. The problem was caused by handling the shop robbery after
removing the object from the floor but before adding it to inventory,
so it couldn't be found to have its unpaid bit cleared.
When investigating this I came across a more severe bug: if the hero
had never entered the shop, the shopkeeper's bill wasn't initialized
properly and add_one_tobill() could crash while attempting to execute
bp->bo_id = obj->o_id;
because 'bp' was Null.
The #tip command tries to reduce verbosity by formatting drop messages
with just the object name instead of with full sentences, yielding
Objects spill out: obj1, obj2, obj3, ..., objN.
where the trailing comma or period is included with each successive
object. If an intervening message occurs, such "25 zorkmids are
added to your credit", the rest of the objects will no longer be
extending the original sentence and end up looking silly.
Objects spill out: obj1, obj2,--More--
25 zorkmids are added to your credit. obj3, ..., objN.
This fix causes the post-interruption messages to revert to verbose
format.
Objects spill out: obj1, obj2,--More--
25 zorkmids are added to your credit.--More--
obj3 drops to the floor.--More--
...
objN drops to the floor.
The interrupting message still follows the comma of the partial
sentence, but I don't see any sane way to fix that other than to
abandon the terse format altogether, and doing that makes #tip way
too verbose when the container has a lot of items in it. But #tip
inside shops now does that, since there will always be buy/not-
interested feedback interrupting the terse format in that situation.
For other situations, a full sentence message might end up following
a partial sentence list of dropped items.
There was a more significant bug. Dropping a hero-owned container
with gold in it onto shop floor sold the gold to shk, giving hero
credit. Subsequent #tip gave the hero credit for that same gold
when it spilled out. addtobill(obj) relies on obj->ox,oy to
determine whether events are taking place in a shop, and #tip was
relying on placement onto floor to set those, too late for shop
billing. The fix yields suboptimal results: you're given credit
when you drop the container, then during #tip when you spill the
contents, credit for the gold is removed, then new credit for it
is given. That's down to shop insanity, not tipping behavior.
Zapping wand of undead turning at self while inside a shop and
carrying a corpse caused the shopkeeper to claim a use-up fee for
the corpse regardless of whether it was owned by the shop.
Not mentioned in the report: casting stone-to-flesh as self while
carrying a figurine or statue behaved similarly.
To test:
1. Get a level layout with two shops facing each other, e.g. minetn-4.
2. Sell a fragile object to one of the shops.
3. Dig a pit in the other shop's door space so its shopkeeper stays out
of the way.
4. Pick up an object in that other shop so it appears on your bill.
5. Zap a wand of striking at the first shop to break the fragile
object.
6. 'p'ay for the object picked up.
Expected result: Object gets the standard prompt to pay for it.
Actual result: "Paid object on bill??" followed by "Program in disorder
perhaps you'd better #quit." followed by the object being given to the
player for free.
The cause? This comment going all the way back to 2002:
> /* FIXME: object handling should be limited to
> items which are on this particular shk's bill */
Originally reported by PaRaD0xx in FreeNode's #NetHack IRC channel
whilst playing NAO343.
Based on DynaHack commit d995ed1 (Fix paid object on bill when angering
another shkp) by me.
It's obviously supposed to be the latter and not the former.
Interesting note: This same bug was found and fixed in NitroHack commit
4973ce4 (static checker day: fixes for scan-build and PVS warnings).
There's no capacity for the shop logic to handle gold without also
changing the credit/debit within it, so gold must always be handled in
`sellobj()`, even when the state of it is set to `SELL_DONTSELL`.
This is needed for an upcoming bug fix.
Based on DynaHack commit b0784c5 (Credit/debit gold in containers even
in sellobj_state SELL_DONTSELL) by me.
For a shop to NOT charge for an object, two conditions apply:
1. The object's `no_charge` flag must be set.
2. That `no_charge` flag must be set regardless of whether or not the
shop typically sells the object in question.
There are two places in `sellobj()` which ignore the second condition,
thus transferring object ownership from the player to the shop without
the player's consent:
1. A container is dropped in a shop that typically sells such
containers and `sellobj_state` is `SELL_DONTSELL`.
2. A zero-cost container holding nothing but gold is dropped in a shop
that typically sells such containers.
Neither occurs currently in NetHack: the latter because NetHack has no
zero-cost containers, but the former is needed for an upcoming bug fix.
This may be related to SC343-21: "Accounting is incorrect for containers
dropped in a shop that does not sell them."
Based on DynaHack commit 4e79b6a (Don't shop-donate non-empty bags
dropped in sellobj_state SELL_DONTSELL) by me.
Most shop messages use shkname() to give the shopkeeper's accurate
name (or hallucinatory substitute) even if he or she can't be seen.
stolen_value() was using mon_nam(), which calls shkname() if the
monster is a shopkeeper who can be seen, but produces "it" when not
seen. Change it to use shkname() like the rest of the shop routines.
Also, replace Monnam() (quite a few instances) with new Shknam() to
do the same duty when the name is at the start of a sentence.
There was also a very obscure bug where if you could see two
shopkeepers at the same time, you could probe the map one spot at
a time with repeated use of the 'p' command to locate monsters in
general and other shopkeepers in particular. Very tedious and not
very useful, but now fixed.
Putting gold into a hero-owned container on a shop's floot gave credit
for the amount of the gold but also set the gold object no_charge, so
it could be taken out without taking away the credit. Then put back
in and taken out as many times as the player liked, doubling the gold
each time until the shopkeeper was out of cash.
I think the proper fix would be to avoid giving credit instead of not
marking the gold no_charge, but that would require multiple additional
changes so I took the easy way out.
Most of the changes to pickup.c are reformatting that it escaped prior
to release. The changes to shk.c are cosmetic and not part of the fix.
Explicitly combine adjacent string literals so that pre-ANSI compilers
still have a chance to compile the code. I thought these had already
been dealt with, but I kept stumbling across them while reformatting,
so am trying to get them all out of the way now.
Last few && or || followed by end-of-line comments, plus tab replacement
and 'return' parentheses. Not as many of those; some of these files had
already had that done.
Also, tweaked non-cursed scroll of charging read while confused to be a
tiny bit more effective.
To do: find and fix block comments that immediately follow a line with
an end-of-line comment and got misindented to line up with that comment.
I couldn't figure out why walking over a container in a shop might
give the wrong price; the code looks correct. But I've reorganized
get_cost_from_price to perform the cheapest tests first. The u.ushops
check should probably be done in doname to avoid calling this routine
at all 99.99% of the time.
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!
* Replace variadic debugpline() with fixed argument debugpline0(str),
debugpline1(fmt,arg), and so on so that C99 support isn't required;
* showdebug() becomes a function rather than a macro and handles a
bit more;
* two debugpline() calls in light.c have been changed to impossible();
* DEBUGFILES macro (in sys.c) can substitute for SYSCF's DEBUGFILES
setting in !SYSCF configuration (I hope that's temporary).
Move debugging output into couple preprocessor defines, which
are no-op without DEBUG. To show debugging output from a
certain source files, use sysconf:
DEBUGFILES=dungeon.c questpgr.c
Also fix couple debug lines which did not compile.
This also includes fixes due to Derek Ray to depugpline to work better
on other platforms.
From a bug report, you get the message "this shop
appears to be deserted" when entering a shop whose shopkeeper is not there,
even if you can't see at the time. Change it to be
'this shop is deserted'
when you can see that the shop is empty,
'this shop is untended'
when there's no shopkeeper but you can see some other monster(s) inside,
'this shop seems to be deserted'
when you can see but there is a monster that you can't see (probably a
hidden mimic, but possibly an invisible monster or one standing behind
a boulder that's ended up in the shop), or
'this shop seems to be untended'
when you can't see (and don't have active telepathy or persistent
monster detection).
From a bug report, dropping and selling a container that had some things owned
by the hero and some already owned by the shop, you could get "You sold
some items inside <a container> for N gold piecess." Shop handing for
containers has been changed significantly since 3.4.3, but the typo
"pieces" that then optionally gets plural "s" appended was still there.
While testing the trivial fix, I noticed suboptional feedback in the
prompt about selling. For a container owned by the shop, it said "items"
even when there was just one hero owned item inside. Fortunately this
potentinal can of worns only seemed to have one tiny weeny worm in it....
The revised version of count_buc() that I've had laying around for
a while is also included.
The fixes entry is for "piecess", not escaped/captured/exterminated
worms, and goes into fixes34.4 despite this patch being labeled "trunk
only". Separate patch for trunk to follow.
Fix one of the issues noticed while investigating the report of a
shopkeeper sighing when a hero died without owing anything. If the death
takes place outside of any shop on a level with multiple shopkeepers, the
first one in the fmon list would be the one who had access to the dying
hero's inventory, even if nothing was owed to that one and something was
owed to another shopkeeper. Now the one who is owed gets first chance to
take the hero's possessions.
When dying inside a shop, the keeper of that shop takes control of
the possessions regardless of whether he or anyone else is owed anything.
That hasn't changed, except that if the hero is simutaneoulsy inside
multiple shops (within a wall spot shared by two or more shops) and owes
money to one of them, the one who is owed will take his inventory even if
the other shk is found first.
This doesn't include any changes to feedback given when the hero dies
in the presence of shopkeepers.
Fix the second of a couple of minor things I noticed when viewing
that guy's ttyrec about a hidden inventory item which turned out to be
scrolls of scare monster on the used-up items section of his shop bill.
He had a lot of scroll prices which started as $X00 and were changed
to $Y99 due to integer division roundoff when being hit with a 1/3 price
increase for unID'd items followed by 1/2 price increase for low charisma:
100 -> 133 -> 199
200 -> 266 -> 399
Forcing things to round up instead of truncate would help in some cases
but yield $Z01 in others. Rather than doing that, this accumulates the
adjustment factors and then uses them to make one calculation which gives
a better result:
100 -> 200
200 -> 400
Other values and/or other adjustments might not work out so well, but I
don't think they'll ever become prices which look worse than before.
For a single increase of 1/3, 100 still yields 133 but 200 now gives 267
(so ends up differing from the combined price for two 100->133 items).
Fix the first of a couple of minor things I noticed when viewing
that guy's ttyrec about a hidden inventory item which turned out to be
scrolls of scare monster on the used-up items section of his shop bill.
Be more precise when a shopkeeper gives the hero credit for dropped
gold. Instead of just displaying
N zorkmids are added to your credit.
show either
You have established N zorkmids credit.
when you had no previous credit, or
N zorkmids added to your credit; total is now X zorkmids.
when adding to existing credit.
