state.c had put file_name into the save file. The saved value was used
to overwrite file_name on restore. If the save file had been renamed,
unlink(file_name) would then fail, because file_name held the old name.
To avoid breaking savefile compatibility, file_name is still saved, but
it is read back into a temporary buffer and then ignored.
I thought I fixed this already.
Alchemy jugs are refilled by the alchemy() fuse, which takes a pointer
to the jug object as an argument. When written to a save file and read
back out, the pointer is unlikely to point anywhere useful.
Instead, rs_write_daemons() now stores an index into the player's pack
or the list of objects on the floor. rs_read_daemons() uses this
number to locate the object when restoring.
This change should not cause any new issues with old savefiles, but it
is unable to make a broken alchemy jug work again.
Daemons and fuses take a single argument, nominally an int but either
ignored or unsafely cast to a pointer. Its type has now been changed
to void*.
The save/restore code no longer tries to store this argument in the
savefile. For doctor(), this is not a problem, because player is the
only argument it is ever given as a daemon. However, alchemy() will
fail to do anything when passed NULL. Fixing this would be complicated
but possible.
Summary: the code is slightly safer, but alchemy jugs are guaranteed to
stop working after save and restore, instead of just extremely likely.
The player name is stored in whoami[], which is length 80 in most games
(1024 in rogue5). Only the first 10 chars were used to create
file_name, because that buffer is the same length. Increasing the size
of file_name to 256 permits using all of whoami.
The name is also no longer truncated to 20 chars when writing the log.
All games should now be able to handle 79-character names without
collisions. Anything more would break save compatibility.
In rogue5/state.c, rs_read_daemons() zeroes out the argument and delay
if the daemon slot is empty. Unfortunately that code ended up on the
wrong side of the brace that closes the for loop, so instead of running
after each daemon, it got run once after the loop exited, when the
index was of course out of bounds.
This tended to manifest, when compiled with -O2, by overwriting hw and
setting it to NULL. When inventory() next ran, hw would be passed to
wgetch(), which returns ERR when it gets a NULL argument. This made
md_readchar() think something was wrong and autosave the game.
Upon investigation, rogue3 was found to commit the same mistake.
rogue4 and srogue don't zero the data. arogue5 already does it
properly.
Someday I am going to run all this through Valgrind. Someday when I
am a kinder person who will not be driven to invoke hordes of trolls
and centaurs upon the original authors.
The save/restore code took the pointer intended as an argument for the
doctor() daemon and wrote it to the savefile as an int. I don't know
why it took so long to fail horribly. The problem has been avoided by
replacing the value with &player when restoring. That seems to be the
only argument ever actually used.
The code also writes only four bytes for an unsigned long; if
sizeof(long) == 8, it casts to unsigned int first. It failed to do the
cast when reading back, with the result that four bytes were read and
the other half of the number was effectively uninitialized.
It apparently works now, but the save/restore code ought still to be
regarded as decidedly unfortunate.
Some .o files need to be rebuilt if config.h changes. Adding it to the
list of headers may still fail to solve the problem, because some of
the Makefiles use implicit rules or do not list dependencies properly.
When shell variables are unexpectedly empty, 'test' gets the wrong
number of arguments and becomes unhappy. Logical AND should not be
done with 'test EXPR1 -a EXPR2' in such cases, because 'test' logic
does not short-circuit. Replace with 'test EXPR1 && test EXPR2'.
Shell logic does short-circuit, and if the first test invocation
fails, the second will never occur, and will never encounter missing
arguments.
md_readchar() mapped KEY_BACKSPACE to CTRL-H, but get_str(), which
handles prompts for strings, only backs up when it receives the erase
character. The key should be mapped to md_erasechar().
This fixes Red Hat Bugzilla #847852.
rogue4 and rogue5 set the player's ISRUN flag upon exit from sleep or
holding. This is apparently supposed to indicate that the player can
move again. What it actually does is make it harder for monsters to
hit the player, until the flag is reset.
As this behavior makes little sense and seems like a cheat, it has
been deemed a bug and removed.
In all games, rs_write_room_reference() stored -1 for a nonexistent
room, but rs_read_room_reference() did not check for out-of-bounds
values, leading to pointers to rooms[-1], which sometimes caused
crashes. rs_read_room_reference() has now been modified to use NULL
instead.
Some of the games required further changes to replace NULL with the
pointer to the actual room. Others are capable of handling NULL for
objects not in any room.
Super-Rogue, like Rogue V4, stored data of machine-dependent length in
the savefile, to prevent cheating. This made saved games non-portable.
Also deleted was a check that used this data, and prevented restoring
savefiles from backup.
This change BREAKS SAVEFILE COMPATIBILITY, but old files can be
converted by removing the block at offset 0x1e with length
sizeof(ino_t) + sizeof(dev_t) + 2 * sizeof(time_t). That seems to be
0x14 on i686 and 0x20 on x86_64.
The save_file() function in save.c stored the savefile's device number,
inode number, creation time, and modification time in the file. The
restore() function read them back, and apparently used to compare them
to protect against cheaters.
Unfortunately, the types and sizes of these numbers differ from system
to system, which ruins the Roguelike Restoration Project's fine
portability work. So they have been removed from the savefile.
This BREAKS SAVEFILE COMPATIBILITY, but old files can be converted by
excising the chunk starting at offset 0x22 with length sizeof(ino_t) +
sizeof(dev_t) + 2 * sizeof(time_t). That's 0x14 on i686 and 0x20 on
x86_64, at least with current versions of Linux and glibc.
The spell-choosing and prayer-choosing routines, when the one-line
inventory option is set, displayed to cw instead of msgw. This caused
permanent corruption of the message line.
