Make some progress on a couple of next minor release checklist
items, hopefully without introducing too many new bugs. This
is just the initial commit, and work continues.
Checklist items:
Savefiles compatible between Windows versions, whether 64-bit
or 32-bit in little-endian field format.
Selection of file formats:
historical (structlevel saves),
lendian (little-endian, fieldlevel saves),
and just for proof-of-concept, ascii fieldlevel saves
(the ascii is huge! 10x bigger than little-endian).
For the fieldlevel save, all complex data structures recursively
get broken down until until it is one of the simple types that
can't be broken down any further, and that gets when it gets
written to the output file.
New files needed for this build:
hand-coded:
include/sfprocs.h
src/sfbase.c - really a dispatcher to one of the
output/input format routines.
src/sflendian.c - little-endian output writer/reader.
src/sfascii.c - ascii text output writer/reader.
auto-coded (generated):
include/sfproto.h
src/sfdata.c
This is just one approach. I'm sure there are countless others
and they have different pros and cons.
For producing the auto-coded files a utility called
universal-ctags, that is actively maintained and evolving,
was used to do all the heavy-lifting of parsing the
NetHack C sources to tabulate the data fields, and store
them in an intermediate file called util/nethack.tags
(not required for building NetHack if you already have a
generated include/sfproto.h and src/sfdata.c)
util/readtags (also not required for building NetHack
itself) will decipher the nethack.tags file and produce
the functions that can deal with the NetHack struct data
fields.
You can obtain the source for universal-ctags by cloning it
from here:
https://github.com/universal-ctags/ctags.git
The combination universal-ctags + util/readtags has been
tried and tested under both Windows and Linux, so it is
not tied to a particular platform.
Note: util/readtags will work only with universal-ctags
output, so other ctags are unlikely to work as-is.
Universal-ctags can be build from source very easily
under Linux, or under Windows using visual studio.
Changing an inventory item's bknown flag wasn't followed by a call to
update_inventory() in many circumstances, so information which should
have appeared wasn't showing up until some other event triggered an
update.
Make the Plane of Water be water all the way to edge instead of having
stone on left, top, and right. The Plane of Air already has air all
the way to edge (including unused/unuseable column #0) but does so via
code rather than the level description file so Water does that now too.
The edges of the Plane of Air were cloudless (3 columns on the left,
2 rows on the top, and 2 columns on the right; don't recall about the
bottom) and that looked pretty strange. Those rows and columns are
beyond the range of bubble/cloud movement so just make some of those
spots randomly be sight-blocking cloud terrain instead of all open air.
It isn't integrated with the moving clouds but looks fairly good when
the hero moves along the edge of the level.
Using wizard mode to leave Water or Air and later return resulted in
no clouds on the Air level and bubbles as usual on the Water level.
I still don't understand why, but on return to those levels run the
bubble creation routine as if the old discarded bubbles or clouds were
being restored.
Leaving the Plane of Water to return to a previously visited endgame
level didn't free the air bubbles unless/until you visit a new level.
Returning to that level creates a new set of air bubbles, losing track
of the previous set. Likewise with Plane of Air and its clouds. (Not
an issue with actual save and restore when on those levels, or when
just moving forward to not-yet-visited levels.)
Not applicable to normal play where it isn't possible to return to a
previously visited endgame level.
For 3.7, bubble save/restore ought to become part of savlev() instead
of being handled by savegamestate().
Lock context wasn't being cleared if it was for a container and that
container got destroyed. Case discovered was forcelock() ->
breakchestlock() -> delobj() (sometimes the container is destroyed
rather than just breaking its lock) followed by #wizmakemap (replace
current level) and maybe_reset_pick() trying to check whether
xlock.box was being carried. But being interrupted, destroying the
container or dropping it down a hole to ship it to another level, then
attempting to resume picking the lock would also find a stale pointer.
This is branched from Alex's hallu-rng-stability branch,
with two build corrections (detect.c, zap.c), and merged
with the isaac64 branch that we have ready to go.
Alex's dual rng is supported by setting up the array
of multiple isaac64 contexts.
I stuck with Alex's approach of passing the rng function
name around as the parameter (rng or rn2_on_display_rng)
for the new additional parameter needed for
set_random(), init_random(), reseed_random(),
and init_isaac64().
For platforms that read from the system's random number generator,
reseed during level change, before the map of a new level is created and
after level creation has finished.
Reported 14 months ago, a monster reading a scroll of earth which
dropped a boulder that killed another monster in an adjacent pit
was giving credit/blame to the hero and could also trigger a panic.
If the monster was killed, the pit would be filled and deleted via
m_detach and then when flooreffects tried to delete the same trap,
it accessed freed memory and deltrap could panic.
The check I added to make sure that a monster was at the hero's
coordinates before deciding to move one or the other would have been
confused by a long worm's tail. Check that they're at that spot but
not by comparing monst.<mx,my> coordinates with <ux,uy>.
Also, don't have wiz_makemap() assume that each level of the Wizard's
Tower has the same boundary coordinates. Keep track of whether hero
is inside that tower before discarding the old level.
Both u_on_rndspot() and losedogs() might result in having a monster
and the hero be at the same location. Have wiz_makemap() use the
same fixup for that as goto_level().
Dropping an existing fragile item while levitating will usually
break it. Getting a new wished-for fragile item and dropping it
because of fumbling or overfull inventory never would.
Some callers of hold_another_object() held on to its return value,
others discarded that. That return value was unsafe if the item
was dropped and fell down a hole (or broke [after this change]).
Return Null if we can't be sure of the value, and make sure all
callers are prepared to deal with Null.
I don't know why we have two different functions which do exactly
the same thing (checking whether an item is unpaid or is a container
that holds at least one unpaid item), but switch the #H2504 fix to
use 'the other one' and reverse one of the changes made when using
the inventory one.
I thought that the earlier fix for #H2504 was too easy for anything
shop related. It didn't deal sensibly with containers owned by hero
but holding unpaid shop goods.
This one is only seven years old. Dropping an unpaid item inside an
engulfer leaves it unpaid and still on bill. If engulfer is killed,
it ends up unpaid when back on the shop's floor.
Treat dropping an unpaid item into engulfer's inventory as stealing
that item. You have to pay for it to leave the shop, and like any
other dying monster's inventory, the shopkeeper will take ownership
if it lands on the shop floor when the engulfer is killed.
The 'theft' doesn't anger the shopkeeper and the cost shows up on 'Ix'
as part of "usage fees/other charges" rather than as an itemized used
up item.
struct rm.flags in overloaded for a bunch of rm.typ -dependent things
(doormask, altarmask, throne/fountain/sink looted, a few others) and
wasn't being reset for various cases where rm.typ gets changed.
I've changed a lot, some no doubt unnecessarily, and probably missed
plenty. This compiles but has not been thoroughly tested.
When deciding whether to discard interrupted lock/unlock context while
changing levels, maybe_reset_pick() checks whether xlock.box is being
carried. But it was doing so after the old level had been saved and
memory for non-carried container there had been freed.
That led to a couple of other issues. context.travelcc was using -1
for 'no cached value', but the fields of travelcc have type 'xchar' and
shouldn't be given negative values. 0 should be fine for 'no cache'.
Failed partial restore which occurred after old game's context had been
loaded would begin a new game with old game's stale context. Restoring
goes out of its way to avoid that for 'flags' but didn't for 'context'.
During level change, when a monster from mydogs (monsters accompaying
hero, usually pets) couldn't be placed because the level was full, it
was set to migrate to that level (in order to get another chance to
arrive if hero left and returned). The code sequence
mon_arrive()-> mnexto()-> m_into_limbo()-> migrate_to_level()-> relmon()
tried to remove the monster from the map, but it wasn't necessarily on
the map (depending upon whether it couldn't arrive at all, or arrived
at the hero's spot and couldn't be moved out of the hero's way). The
EXTRA_SANITY_CHECKS for remove_monster() issued impossible "no monster
to remove". relmon() now checks whether monster is already off the map.
While investigating that, I discovered that pets set to re-migrate
to the same level to try again on hero's next visit didn't work at all.
migrating_mons gets processed after mydogs so moving something from
the latter to the former after arrival failure just resulted in
immediate second failure when the more general list was handled during
the hero's current arrival. And failure to arrive from migrating_mons
would kill the monster instead of scheduling another attempt.
The sanest fix for that turned out to be to have all monsters who
can't arrive be put back on the migrating_mons list to try again upon
hero's next visit. Pets still fail twice but are no longer discarded
during the second time, and now do arrive when hero leaves and comes
back provided he or she has opened up some space before leaving. If
there's still no space on the next visit, monsters who can't arrive
then are scheduled to try again on the visit after that.
Recent fix for invalid corpses becomes moot. Monsters aren't killed
during arrival failure so there are no resulting corpses to deal with.
When using rloc and friends to move monsters, and the monster
happens to be a long worm, the tail may get randomly placed
in the same place where the long worm was removed from.
In the cases where we expect the location to really be free,
explicitly recheck the location for a monster after rloc.
Use the make_foo() intrinsic set/reset routines instead of trying
to manipulate the intrinsics directly. Previous patch left Dex
down by 1 if stoning caused wounded legs to be fixed, and left
delayed killer allocated if stoning cured sliming or vice versa.
Add code to run a fuzz tester, simulating (more-or-less) random
keyboard mashing. There's no option to turn it on, you need to
set iflags.debug_fuzzer on via a debugger or something along
those lines.
Make being trapped in/on/over floor block Levitation and Flying, the
way that being inside solid rock already does, and the way levitating
blocks flight.
Blocked levitation still provides enhanced carrying capacity since
magic is attempting to make the hero's body be bouyant. I think that
that is appropriate but am not completely convinced.
One thing that almost certainly needs fixing is digging a hole when
trapped in the floor or tethered to a buried iron ball, where the
first part of digactualhole() releases the hero from being trapped.
If being released re-enables blocked levitation, the further stages
of digging might not make sense in some circumstances.
I recently realized that being held by a grabbing monster is similar
to being trapped so should also interfere with levitation and flying.
Nothing here attempts to address that.
Save files change, but in a compatible fashion unless trapped at the
time of saving. If someone saves while trapped prior to this patch,
then applies it and restores, the game will behave as if the patch
wasn't in place--until escape from trap is achieved. (Not verified.)
