Sitelet https://github.com/NetHack/NetHack/issues/1682
Skip to content

Polymorphing can trigger a 2nd form change that is mishandled #1682

Description

@davidbau

There are three bugs in src/polyself.c that happen while processing the
consequences of a polymorph that in turn cause the hero to take some damage
that cause the hero to change form a second time.

I ran into one of these scenarios in a game, and careful testing (thanks AI)
found three related reproducible bugs sitting in the polymorph code:

what you see bundle
A light source is deleted that was never created del_light_source: not found, Program in disorder!, a request to mail the DevTeam bug 07
Cleanup runs twice for one polymorph one polymorph, two artifact blasts and two damage rolls bug 06
Decisions keep being made by the superseded form's rules a human is stripped of their water walking boots because a newt could not wear them, while standing in lava; fatal bug 08

The affected code is src/polyself.c (polymon(), polyself() and
break_armor()) plus one line in src/timeout.c. All three are present at
the NetHack-5.0 tip c63ee6ac7,
checked 2026-09-18, and all three repros were recorded against
NetHack/NetHack@16ff59115, NetHack 5.0.0 as released.

A companion pull request with three commits, one per defect, is #1681. It
carries the reasoning for each part of the change and why it is correct.

The three scenarios

Below are recorded session repros for each of the three problems, and for the
fixed behaviour, viewable in a browser.

1. A light source is deleted before being created

Polymorph into a form that glows (a yellow light, a fire vortex) while
carrying a cross-aligned artifact. polymon()'s own retouch_equipment(2)
blasts you, the frail glowing form dies, and rehumanize() runs. It tries to
delete the light source for the form you currently have. But the light source
for a polymorphed hero is not created by polymon(); it is created by
polyself(), after polymon() returns. There is nothing to delete yet.

Buggy
·
Fixed, same step

2. Post-polymorph cleanup runs twice for double damage

Polymorph into a level-0 form while standing on lava, or onto a land mine
while swallowed. The form dies inside polymon(), and rehumanize() does its
own end-of-form-change cleanup: encumber_msg() and retouch_equipment(2).
Control returns to polymon(), which is about to do exactly those two things
itself. The artifact blasts you a second time, for a second roll of damage.

Buggy, the second blast
·
Fixed, same step
·
second route, buggy
·
second route, fixed

3. After flipping back to human, still treated as if turning into a newt

Polymorph into a newt while levitating over lava on an #invoked artifact.
break_armor() decides what to strip by asking the form it captured when it
started. Its gloves block drops your weapon, which ends the levitation, which
drops you in the lava, which kills the newt, which reverts you to human. The
function then carries on asking the newt what a human may wear, and takes
the water walking boots off a human standing in lava.

Buggy, the fatal strip
·
Fixed, hero survives

The proposed invariant for polymorphs

The current polymorph code seems try to keep the following invariant.
So the proposed changes add the needed logic to clean this up:

Every effect applied during a form change must be appropriate to the form the
hero has now, so if the hero re-changes, it should stop applying effects that
were consequences of the previous polymorph form.

Note that re-entrancy itself is not what causes these bugs, (re-entrancy is not
avoidable here: dropping armour can put the hero in water, dropping an artifact
can end levitation; then damage taken while polymorphed reverts your form for
a nested polymorph).

The problem in the code is that state examined before a nested form change is
still used after the form is out-of-date: it keeps working with the form polymon()
was originally asked to install, the form break_armor() decided to strip by, etc.

Following two principles could fix the problems within and after a nested
form change.

If you make changes inherent to the form, leave the world consistent with it
before you return.
This is the ownership issue behind the light source crash.
The light source is part of what it means to be a glowing monster, so it belongs
to the code that installs the form, not to a caller several frames up who will get
around to it later. So we should move the light-source handling up much earlier
to be applied immediately before it can be aborted or reversed.

If you apply ramifcations after a call that might have changed the form,
check that the form is still active before you act.
This is the other bugs
about double-effects and obsolete-effects.


The full analysis, the recorded sessions, the two controls and a proposed patch
that would fix all these are here:

https://github.com/davidbau/nethack-bugreport/tree/main/bugs/10-polyself-reentrant-form-changes

It's an AI-augmented change, and I'm happy to clean this up more by hand if it's helpful.

-- @davidbau

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions