It's possible to get a strange command result of ECMD_CANCEL+ECMD_TIME.
If/when that happens don't just treat it as cancel, make sure that time
elapses.
If an invalid command is provided, clear the do-again queue so that ^A
won't attempt to try the same invalid command again.
This is enough to prevent abuse by denying access to functions and
denial of service (RAM and instruction step limits), but not enough
to allow restricted use of things that require finer control (e.g.
filesystem access).
If something goes wrong, the whole thing can be turned off, for
now, in config.h (see NHL_SANDBOX).
None of the current functionality requires changes to build systems;
some of the possible future functionality may require some #defines
- TBD. There is lots of dead code (#ifdef notyet) for bits of that
additional functionality; we can rip it out if we don't want those
additions or we can complete (parts of) it depending on our needs.
All current uses of Lua are connected to sandboxes and guarded with
nhl_pcall (sandbox and lua_pcall wrapper); options and limits can
be set at the callsites in the passed nhl_sandbox_info. Some of
the error handling may be wrong - panic() vs. impossible() vs
silence.
Memory and instruction step limits should be tuned prior to release;
there's no point tuning them now.
The air bubbles on the Plane of Water and the clouds on the Plane of
Air were being saved and restored as part of the current level's state
(which is the 'u' struct and invent and such) rather than with the
current level itself. That was ok for normal play, but for wizard
mode's ^V allowing you to return to a previously visited endgame level
after moving to a different one it meant a new set of bubbles for
Water and new set of clouds for Air. Even that was ok since it only
applied to wizard mode, but using #wizmakemap to recreate Water or Air
while you were on it added a new set of bubbles or clouds to the
existing ones. If repeated, eventually there wouldn't be much water
or air left.
Instead of just adding a hack to #wizmakemap, change save/restore to
keep the bubbles/clouds with the level rather than with the state.
That wasn't trivial and now I know why the old odd arrangement was
chosen. Saving hides u.uz by zeroing it out for levels that the hero
isn't on and it is zero during restore so simple checks for whether a
given level is water or air won't work.
This also adds another non-file/non-debugpline() use of DEBUGFILES:
DEBUGFILES=seethru nethack -D
will make water and clouds be transparent instead of opaque. It also
makes fumaroles and other light-blocking gas clouds be transparent
which wasn't really intended, but avoiding it would be extra work that
doesn't accomplish much.
Increments EDITLEVEL for the third time this week....
Noticed when testing the forcefight against obscured furniture fix:
if you attempted forcefight against the edge of the map, you got
feedback about having already moved as far in <direction> as possible
rather than about forcefight failing to attack anything. Have the
can't move out bounds test check for forcefight before deciding to
give to its normal feedback.
Pull request #746 by entrez: give better feedback when 'F' prefix
is used toward furniture covered by objects and towards solid rock
terrain on arboreal levels.
Michael Meyer [Wed, 27 Apr 2022 01:58:51 +0000 (21:58 -0400)]
Fix: force-fight 'unknown obstacle' descriptions
This is intended to address a couple quirks with force-fighting an
unoccupied spot that I noticed:
* Now that furniture is considered 'solid', an object is much more
likely to be sitting on the square, obscuring the terrain glyph. As
a result, the current glyph is no longer sufficient to accurately
describe the contents of the spot -- e.g., an altar with a corpse on
top of it was being described as "an unknown obstacle", even when the
hero knew exactly what furniture was there.
* When blind and attacking an unexplored 'solid' square, the attacked
position would always be described as 'the stone', even something
like a fountain or sink which didn't seem likely to be confused with
a stone wall.
* The feedback for attacking stone was previously changed from 'solid
rock' to 'stone' in order to be consistent with the feedback for
attacking an unseen wall, but they still weren't quite the same
("stone" vs "the stone").
* The 'stone' feedback for all STONE/SCORR spots was incorrect on
levels flagged as arboreal, where stone is rendered and described as
trees.
This relies on back_to_glyph for positions where the hero is aware of
the terrain and certain other spots (like stone, walls, etc) for which
back_to_glyph produces good results even if they're unseen, and falls
back to the generic "unknown terrain" in other cases.
Pretty long commit message for such a small commit, but oh well...
Use the new traps and their tiles when confused gold detection finds
trapped doors and trapped chests. (Large boxes can be trapped too;
they use the trapped chest trap and corresponding tile rather than
have their own.)
Usually these pseudo-traps go away when as soon as they are within
line of sight. (While testing, I noticed that seeing a trapped door
from outside its room rather than inside didn't behave that way.
The door was created by wizard mode wishing; I don't know whether
that was a factor.)
I also discovered that secret doors weren't being handled correctly.
They can't be trapped because of their use of both the doormask and
wall_info overlays of levl[][].flags, but I had a secret door be
falsely displayed as a trap. This fixes that.
We should have obj->tknown and rm->D_TRAPKNOWN so that the hero won't
forget about these traps after declining to attempt to untrap them.
But that's more work than I care to tackle.
"add glyphs+tiles for door+chest traps",
commit d1217b9f2565a5db7572c7e80c36372115465a1d,
missed a couple of S_vibrating_square references, resulting in
screwed up rendering of zaps and explosions.
When trap detection finds trapped doors and trapped chests, it shows
those as bear traps. When the hero comes within view, they revert to
normal and the detected trap is forgotten. This doesn't change that,
it is just groundwork to be able to show them distinctly. Like the
TT_BEARTRAP patch, it increments EDITLEVEL so this seemed like a good
time to put the groudwork in place.
There shouldn't be any visible changes even though internal glyph and
tile values have been renumbered after inserting two new entries.
Adding traps after S_vibrating_square was quite a hassle and suffered
though a couple of off-by-one errors that weren't trivial to find and
fix.
Issue #745 by k2: when using two-weapon combat, the second attack
would still take place even if the first attack caused the hero to
become paralyzed (hitting a floating eye or g.cube).
Cleaver's up-to-three attacks had the same problem but did stop if
the hero underwent life-saving after some fatally damaging counter-
attack. Two-weapons didn't. Make them both stop early if either
paralysis or life-save occurs.
Multiple attacking monster against monster and against poly'd hero
already deal with paralysis; life-saving doesn't apply.
- Microsoft Visual Studio 2017 Community Edition v 15.9.47
- Microsoft Visual Studio 2019 Community Edition v 16.11.13
- Microsoft Visual Studio 2022 Community Edition v 17.1.5
When not wielding anything, ^X reports "you are empty handed" if
wearing gloves or "you are bare handed" if no gloves. The ')',
'w-', and 'A)' commands were using "empty handed" unconditionally.
Make them be consisitent with ^X.
After this, body part HANDED is no longer used anywhere except in
body_part().
Reported directly to devteam: when a Rider revived, its corpse
didn't get used up.
The change to have delobj() never destroy Rider corpses, like it
won't destroy the Amulet or invocation items, didn't take into
account that they should be destroyed when Riders revive from them.
Add delobj_core() to be able to do that without changing existing
delobj() usage.
I'm surprised hardfought players haven't been all over this one.
Reported directly to devteam: changing levels while riding gave an
impossible warning, "no monster to remove".
mon_leaving_level() was trying to take hero's steed off the map but
that isn't on the map in the first place. Only noticable if built
with EXTRA_SANITY_CHECKS enabled. Normally remove_monster(x,y) just
sets level.monsters[x,y] to Null but with extra checks enabled it
first checks whether that is already Null.
That affected shop repair messages but wasn't enough to prevent a
shop wall repair display glitch. This seems to make things work
properly but is little iffy.
Pasi Kallinen [Sun, 24 Apr 2022 10:03:43 +0000 (13:03 +0300)]
Monster list iterator
Add some basic functions to iterate through the monster list,
ignoring dead monsters. Mainly just to allow splitting up code
into discrete functions.
Not quite happy with the get_iter_mons_xy - should probably have
a pointer to iterator data struct, which gets passed through instead,
but this works for now.
Like the commit message for #743, the followup to it was misIDed
as #744. This one is for #744. There's no need to check a name
against all the "<class> of typename" unless it contains " of ".
Pull request #744 from vultur-cadens: the object specification
supported by des.object() was ambiguous for items whose name in
objects[] doesn't include the object class name. For instance ring
and spellbook of protection are just named "protection" (unlike
"cloak of protection" which has its full name).
Fixes #744
The commit message for pull request #743 misidentified it as #744,
so didn't get marked closed properly. Do that now...
Allow specifying object classes in the object name given to des.object()
and actually do so in the lua files.
Before this, it was not possible to specify (for example) "scroll of
teleportation" in des.object() because there is actually no object
defined in objects.h named "scroll of teleportation", so
find_objtype() failed to find it. Instead, one had to request
"teleportation", but that is ambiguous, and find_objtype() would find
the first defined item with that name instead (ring of teleportation).
In cases of ambiguity, I referred to the des files from 3.6.6 (before
the lua conversion).
remove unnecessary null-check on wc_set_window_colors()
mungspaces() returns its argument itself, so `newop` is assigned to `buf`, and always non-null.
`tfg` and `tbg` is assigned to (some addition of) `newop`, so these are also always non-null.
If you have 'fixinv' set to Off and and an inventory of three items,
they'll always be a and b and c. #adjust had you pick 'from' slot
among [abc] and the prompt for 'to' was supposed to show the letter
you picked plus 'd' for 'move to last'. But it was only showing
the 'from' letter itself as likely candidate, omitting the last+1
choice. (Anything after the last letter in use could be picked and
yield the right result, only the list of likely candidate slots in
the prompt wrong.)
Fixed more by trial and error than by understanding why the old code
didn't do what was intended.
There's no 'w-' or 'Q-' for alternate weapon, but context-sensitive
inventory is starting from the object rather than the command so can
finesse that. 'A' does allow alternate weapon to be directly unset
(aka reset to bare/gloved hands) but is not friendly to being passed
queued input.
This adds an extra internal command which only handles unset uswapwep,
even though that is something which is awfully specialized to get it's
own command. Users don't see this command so that shouldn't matter.
For context-sensitve inventory, if player picks the item occupying
the quiver slot, offer '-' as a choice. Like for wielded weapon,
picking that will clear the slot. Also, don't include the 'Q' for
quivering it choice since it is already quivered.
Unlike nethack4, we don't currently have a way to explicitly clear
uswapwep. The sequence #swap, #wield '-', #swap won't work if
primary weapon is cursed and isn't safe to use if alternate one is.
It would be simple enough to just directly call setuswapwep(NULL)
and untwoweapon() in the item-action routine instead of queuing up
some command to do it but that feels a bit unclean. Adding yet
another internal command for it would work too but this one wouldn't
be for variant user-interaction of an existing command.
The fire command could claim that time passed when it hadn't (fill
quiver with ammo, which takes no time, then queue commands to switch
to matching launcher, which should also take no time while queueing,
only during subsequent execution).
If quiver is empty or has ammo in it, give wielded thrown-and-return
weapon (aklys) priority over filling quiver or switching to ammo's
launcher. Don't do that if quiver has non-ammo in it, otherwise
players running Valks who wield Mjollnir with super strength but
want to throw quivered daggers would complain.
When player is being asked what to fill the quiver with, use the
\#quiver command to do that. Using it honors a count to split a
stack, handles switching uwep or uswapwep to uquiver, and gives
feedback. This is actually a fairly substantial change.
For 'fireassist', when switching to a launcher that isn't already
uswapwep pick one known to be blessed or uncursed over one having
unknown BUC status. But use the latter as last resort.
Reported by vultur-cadens: slow digestion from wearing white dragon
scales/mail blocked per-turn hunger and didn't cause any hunger,
unlike ring of slow digestion which blocks per-turn hunger but still
causes some hunger as a worn ring itself. If no rings or amulet
were worn, wearing the suit prevented the hero from ever burning any
nutrition.
Change to treat wearing the suit to be quite a bit like wearing a
ring, unless hero is also wearing an actual ring of slow digestion
(then the hunger cost of the suit is 'free').
Wearing a ring of slow digestion and another ring consumes two units
of nutrition every 20 turns, no matter what suit. Wearing white
dragon scales/mail and two non-slow digestion rings now consumes three
units of nutrition every 20 turns. Using the suit to effectively get
an extra ring finger isn't free.
Issue #729 by argrath points out that one of the checks in
edibility_prompts() couldn't work.
For the next-meal effect after reading a blessed scroll of food
detection, the check whether a corpse was tainted but not dangerous
because the hero had sickness resistance could never be evaluated.
An earlier condition would cause the routine to return before
reaching that check.
Move it sooner, even though doing so violates the "order by most
to least dangerous" guideline. It was either that or eliminate it
altogether.
edibility_prompts() had a lot of repetitive code that has now been
condensed.
Simplify acid blob corpse handling--for all of eating, not just for
edibility_prompts()--by treating that as "never rots" so that the
'rotted' variable always stays 0. Now checks for that variable
being greater than some threshold don't need to include "and not an
acid blob corpse" as an exception. A side-effect of this change is
that not only do they never become tainted, they'll no longer yield
the "you feel sick" outcome when they're old but not old enough to
exceed the tainted threshold.
Bug fix: edibility testing stopped warning about green slimes.
That worked in 3.6.x, but 3.7 changed the 'cadaver' variable to
exclude them so the check for eating a glob of green slime could no
longer be reached.
Reported by entrez. Don't make 50% of neuter monsters be flagged as
female. It doesn't matter for live monsters but gets inherited by
their corpses, where female and non-female corpses stack separately.
Noticed earlier when testing the status_hilite_menu_fld() changes:
if you enter a duplicate rule (with different highlighting), it didn't
replace the previous one and the old rule continued to be used.
This still doesn't do replacement, but by adding the new rule at the
end of the list of rules for the specified field instead of inserting
it at the beginning, the new rule gets used.
A change made 5 or 6 weeks ago that was meant to enhance tracking of
artifact creation had an unintended side-effect of making every object
obtained via wishing have its dknown flag be set. That made them
behave differenly from items picked up off the floor, so revert to the
old behavior.
Pull request #740 by argrath removes some redundant code. Instead
of adopting that, this rewrites the section of code in question.
The menu involved allows the user to select both "delete hilites"
and "add new hilite" but if both are selected it would only do
deletion. It also falsely claimed to have done something if delete
was chosen but no highlights were selected to be deleted. And it
only added one new highlight for the specified field when adding.
This supports both add and delete on the same menu invocation, for
addition it keeps adding until no new highlight is specified, and
has caller do the highlight updating if any changes are made.
More context-sensitive inventory support. While examining inventory,
if you pick an item other than gold and it has a quantity of more
than 1, "I - Adjust inventory by splitting this stack" will be one
of the menu choices.
Breaking doorganize() into two parts was much easier than expected,
but the new internal command added to be an alternate for the first
part had more niggling details than anticipated.
Message history only shows the first digit with "Split off how many?"
if the player enters more than that.
Add context-sensitive inventory item-action 'i' to run the #adjust
command. Groundwork for item-action 'I' to use #adjust to split a
stack is included but non-functional.
Also reorders some of the enums and switch cases to be alphabetical
by their menu selection letter, dip moves from 'd' (which is drop)
to 'a' and offer/sacrifice moves from last to 'O'.
Michael Meyer [Mon, 18 Apr 2022 21:11:15 +0000 (17:11 -0400)]
Fix: "in the the purple worm's entrails"
The message printed if the hero threw gold while swallowed by an animal
used "the <mon_nam>'s entrails", which produced a doubled 'the'. It
could also use the wrong possessive form, since it doesn't take
advantage of any of the special case handling in s_suffix. I think the
only way that could ever be a problem with the current cast of engulfers
is if the hero was swallowed by a purple worm while hallucinating, but I
changed it to use s_suffix anyway.
hero when hero is [impatiently waiting...] on stairs
My attempts to cherry-pick this failed, so this was done manually.
It is a reimplementation of
NullCGT:feature/monster-item-use:dc2cef0562542fece1732dd2d4c4f0775308faff
] Pets approach the player if they are standing on the stairs.
]
] One of the most frequent complaints I have seen is that pets refuse
] to follow their owners down the stairs. While this can be resolved by
] waiting, most players, especially new ones, are not willing to spend
] multiple dozens of turns waiting for their pets to approach closely
] enough to follow them. This simple commit makes pets react to a player
] standing on stairs as if the player is holding a tripe ration. Simple,
] non-disruptive, and should solve many headaches.
Something I noticed when testing the item-action handling for name and
call; applies to the C/#name command too. You were allowed to call the
real Amulet something and allowed to call fake ones something [else].
If you did that, xname/doname didn't show it but the discoveries list
did, giving away information when the player had access to more than
one unID'd Amulet of Yendor. Rather than messing about with discovery
handling, make real and fake Amulet be ineligible from being given a
type name. (They can still be given individual names.)
Mostly attempting to clean up potential error handling but I don't
have any error cases to test with. Doesn't seem to break anything
when there aren't any errors....
Switch the verb from "name" to "call" for type-naming. Format menu
choices for name or call of unique items more carefully.
Read actions got some extra details so move them out of the main
item-action routine to avoid cluttering it up.
Avoid "r - Study the spellbook" for novels. It's changed for known
Book of the Dead too, but I'm not sure "r - Examine the tome" conveys
"read" sufficiently. ("Read" and "peruse" seem too mundane for it;
perhaps it should revert to "study" even though the hero isn't
attempting to learn a spell from it.)
Change "r - cast the spell on this scroll" to "r - read this scroll
to activate its magic". Leave off the last phrase if it's a known
scroll of blank paper or scroll of mail.
Pasi Kallinen [Tue, 19 Apr 2022 11:56:51 +0000 (14:56 +0300)]
Fix panic on thrown potion of acid when swallowed
When you're swallowed and throw a missile that kills the swallower,
the thrown missile is picked up by the swallower before death.
This caused a panic when you threw a potion, which was first picked up
by the swallower, which upon death dropped the potion on the floor,
but then throw code was trying to destroy the potion ...
From 6 year old email: m_detach (monster death or removal from play)
and relmon (monster migrating to another level) both take a monster
off the map but they weren't consistent with each other. Change them
to use a common routine for that.
I'm not sure whether the inconsistencies resulted in any bugs. The
email was concerned about handling for monsters that emit light, but
those aren't actually common to the two removal methods and turned
out to be ok.
This attempts to make item-actions, #herecmd, and #therecmd be more
robust. When rhack() or yn_function() take queued input off cmdq
and get something unexpected, discard the rest of the queue.
It also fixes the two crash cases that entrez reported. There are
bound to be others though.
I think a lot of actions that can be executed by queued input are
going to need nomul(0) calls to handle repeat counts that should be
ended early if something unexpected happens or something expected
fails to happen. But that clears cmdq so may be tricky to decide
where to use.
Fix eat floor food and drink from dungeon feature via #herecmdmenu.
That uses queued commands, but those two actions were changed to
skip the floor when queued input was present because asking about
floor items interfered with context-sensitive inventory item-actions.
I was misled by a comment that says it couldn't insert an m-prefix;
that was for treating the 'm' key as typed text rather than as a
command. There's no problem with inserting a #reqmenu command which
is what 'm' is these days. So item actions can force 'm' to skip the
floor and go directly to inventory, also the #eat and #quaff commands
don't have to alter their behavior when queued input is pending so
the #herecmdmenu usage for them gets fixed.
For Qt with 'popup_dialog' off, if I typed "#quit" then the prompt
"Really quit?" was displayed in the message window as expected.
But if I typed "#" and then clicked on [quit], sometimes the prompt
wasn't visible even though the program was waiting for me to answer
it. I've noticed this a time or two in the past and just pretended
that it hadn't happened; since I usually enter extended commands by
typing their name, it didn't stand out much. Testing #repeat with
extended commands brought it back to mind.
I don't understand what's going on, but this one-line patch seems
to solve it. (With a multi-line message window, clear_nhwindow()
doesn't erase anything, but for Qt it does update the most recent
entry.)
Using '&#' or '?f#' showed "# perform an extended command (##)".
The "(##)" part looks rather silly and is not helpful. Expand the
text a little and omit command name for that particular command:
"# enter and perform an extended command".
The comment preceding new 'savech_extcmd()' said that the core didn't
care whether it was given the full command name or just enough leading
substring to be unambiguous. Then it tested the string against
"repeat" which contradicts that comment. Didn't seem to be an actual
problem because "#repeat" is not flagged for auto-completion, but fix
the code to match the intent of the comment and reword the comment to
match the code.
Fix '#repeat' for tty; both it and ^A can repeat an extended command.
Fix both for curses; they can repeat an extended command instead of
just repeating the initial '#' to start getting an extended command.
X11 (tested), Qt (tested), and probably Windows GUI (not tested)
behave the same as before: ^A (or #repeat) after an extended command
just repeats the # to run the dialog to get an extended command.
I hope this introduces fewer bugs than it fixes but I don't think I'd
bet on that....
Picking a potion from inventory and then picking 'quaff this potion'
from the context menu needed handling similar to eat/offer/tin: skip
floor candidates. If you were on a fountain or sink, picked a potion
from inventory and then the quaff option for it, you weren't prompted
to drink from the fountain but you were prompted for what potion to
drink instead of using the one that had already been picked to
initiate drinkig.
If you used ^A to repeat a command which had taken no time, the
repeat execution would take time. This fixes that. Also, give some
feedback when trying to repeat an invalid command.
Internals bit: don't use 'X == cmd_from_func(do_repeat)' to decide
whether key X is the key for #repeat. Both X and Y might be bound to
that action and cmd_from_func() could return Y rather than X.
There is another ^A bug that I haven't figured out how to fix:
t ESC start to throw but don't finish
^A nothing seems to happen
^A "You don't have that object."
The first ^A repeats 't', doesn't display a prompt for what to throw,
but does request input for it. The second ^A fulfills that input and
doesn't match any inventory item. Either 't' shouldn't have been put
into the do-again buffer or do-again handling should have ceased when
it was taken out and there was no further remembered input, so that
normal prompting would resume. My tentative attempts for both those
approaches didn't work.
Issue #734 reported as "parse function" by Meklon2007: the change
yesterday intended to make ^A work for commands that were preceded
by a prefix was triggering a crash if used after a keystroke that's
not assigned to any command.
'M^A' or '~^A' would segfault by derefencing a null pointer when
checking whether 'M' or '~' was a prefix. This prevents the check
attempt from doing that, but a better fix would be to not put the
invalid command keystroke into the do-again buffer in the first
place.
Pasi Kallinen [Fri, 15 Apr 2022 15:52:46 +0000 (18:52 +0300)]
Revisit the Valkyrie goal level hack
Instead of hardcoding the lava terrain change in core, if the stairs
are created in a fixed location, force the terrain to room floor first.
Move the surrounding lava changing to room floor to the Val-goal lua
file.
Reported by luxidream via the web contact form and also as github
issue #732: using the repeat command after F+direction would take a
step in direction if there was no target to fight.
The direction was being repeated without the F prefix. It wasn't
specific to F; m+dir misbehaved too. This fix seems to work but it
should be replaced with something more robust.
Reported by k2, arriving at the final level of the Valkyrie quest
can issue a recently added impossible
| mkstairs: placing stairs up on molten lava at <68,13>
The report said it was easy to reproduce, but it took me multiple
tries (so not hard to do, but not a sure thing on any given attempt).
The stairs on that level are placed at specific coordinates that
are outside the pre-mapped area, so there's no guarantee that their
spot will be suitable for stairs. The underlying terrian changes
from lava to stair, but only after the warning about molten lava.
This hack solves that particular level but is not a general solution
for this type of thing. When about to make stairs on a lava spot,
change that spot to normal floor first. Plus 50:50 chance to change
each adjacent lava spot to floor too so that there's decent chance
for some elbow room upon arrival.
Also, turn the no-flip attribute off for that level so that 'fixed'
location of the stairs can occur in four different places.
Reported by gebulmer: if charging exploded a ring, the ring's memory
got freed but the stale pointer was passed to cap_spe() which accessed
it again. Fix by setting the object pointer to Null after using up
the ring. This was a post-3.6 bug.
Make being offered floor food and declining behave similarly to
being offered a chance to drink from or dip into a fountain and
declining: insert "else" into
| "You don't have anything [else] to {eat | offer | tin}."
when there is nothing applicable in inventory.
Simplify the recently added handling for inserting "else" into
| You don't have anything [else] to {drink | dip into}.
after declining to use a fountain/pool/sink at the spot when not
carrying any potions.
Allow the player to precede q/#quaff or M-d/#dip with the 'm' prefix
to skip asking about fountains, sinks, or pools if one of those
happens to be present, similar to how using it for e/#eat skips food
on the floor and goes straight to inventory.
If you use it and don't have any potions, you'll get "you don't have
anything to drink" or "you don't have anything to dip into", same as
when there is no suitable dungeon feature present combined with no
potions. However, if an applicable dungeon feature is present and
you don't use the prefix but answer 'no' to drink from fountain,&c
and you don't have any potions, "else" will be inserted into the
message: "you don't have anything else to drink".
A big part of the diff is just a change in indentation level for
code that is now inside 'if (!iflags.menu_requested) {' ... '}'.
rhack() normally calls parse(), parse() sets context.move to True
assuming that the player's next action will take game time, then
when it returns, rhack() sets context.move back to False if the
assumption turned out to be incorrect. But when performing actions
after picking something in inventory, rhack() doesn't call parse()
so context.move is left at False.
This was hidden by making the inventory command take game time if
the player picked an item and set up an action to be done with it
even though the action hadn't taken place yet. So time was being
accounted for but if the hero didn't get consecutive moves then
monsters got their turn between the shouldn't-take-time inventory
command and the ought-to-behave-like-normal-command queued action.
My initial attempt to fix this (before figuring out how context.move
works) by stopping inventory from taking time didn't work because
queued item-actions stopped taking time too, or rather the fact that
they took no time became exposed. This second attempt doesn't have
that problem and I think it is correct.
Normally dipping gets the thing to dip first and what to dip it
into second and the item-action handling knows that. I'm not sure
why that wasn't working as intended and I couldn't figure out how
to make it do that, so went another way: this adds an internal
extended command that executes an alternate dip routine which gets
the potion to dip into first and the thing to dip into it second.
The #dip command should allow an 'm' prefix to skip fountains and
pools, similar to how eating accepts it to skip food on the floor.
But this doesn't implement that.
Picking a corpse while looking at inventory issued a menu that had
entry for eating that and if on an altar another one for offering
that. Picking the eat or offer choice worked as long as there
weren't any other corpses on the ground or altar. If there were
others, they'd be skipped but you'd get prompted for which item in
inventory to eat or offer instead of operating on the one that was
used to initiate the action.