exit_nhwindows() is called before terminate(), and the tty incarnation
destroys all windows--including 'pickinv_cache_win'--without setting
the various index variables used to access them to WIN_ERR, then
terminate() calls freedynamicdata() which calls free_pickinv_cache()
which tries to destroy 'pickinv_cache_win' since it isn't WIN_ERR (if
the perm_invent option has been enabled during that playing session).
Some of the other <interface>_exit_nhwindows() also tear things down
without resetting the variables used to track them, so fixing this in
exit_nhwindows() would have been pretty messy.
Call free_pickinv_cache() before exit_nhwindows() in done(). At the
moment it's only called from done(), so other exit paths won't release
the small chunk(s) of memory used for the alternate inventory window
(if it got created for perm_invent support).
Replace the code that uses strcat with two pointers into the same buffer.
Treated separately, they point at distinct strings (no overlap possible),
but the C standard does disallow that in order to enable optimizations
using block transfer or such, so the tool that complained about it isn't
wrong. The characters getting appended to the output pointer can end
up overlapping the beginning of the other input pointer, conceivably
breaking an implementation that didn't use simple left-to-right byte-at-
a-time copying.
Also, I noticed that wishing for "luck stone" gave me a random gem.
There's code to strip off " stone" and compare against gem types, but it
prevents other code that accepts "foo bar" as a match for "foobar" and
vice versa from finding a match, since "luck" doesn't match anything
once "stone" is gone. So add the four gray stones into the array of
alternate spellings.
Another bit: it now accepts " gem" in addition to " stone" as optional
gem suffix, so "ruby", "ruby stone", and "ruby gem" all yield ruby.
("luck gem" won't work; you'll end up with a random gem-class object.)
And a last other bit: wishing for "lamp (lit) named foo" would yield
an unlit, unnamed lamp because "(lit)" followed by anything didn't match
"(lit)" and threw away everything past the opening paren. Now it will
produce "lamp named foo (lit)"--a lit lamp named "foo". (Wishing for
"lamp named foo (lit)" produces an unlit lamp named "foo (lit)". That's
acceptable to me... I'm not crawling any farther down this hole. Maybe
object formatting should be changed to keep the lit attribute in front
of the name?)
The memory leak (monst->mextra->edog, monst->mextra->mname,
monst->mextra for some monster were not released) I noticed recently
was due to recording a pet's full monster attributes with its corpse.
During save and restore, obj->oextra->omonst was being treated as a
full-fledged monster so worked as intended, but when freed, omonst
was treated as a black box and its mextra details weren't handled.
Changes to be committed:
modified: src/objects.c
modified: win/share/tilemap.c
Warnings during tile builds (and incorrect tile mappings
at run time when MAIL wasn't defined):
Creating 16x16 binary tile files (this may take some time)
warning: for tile 325 (numbered 325) of objects.txt,
found 'ETAOIN SHRDLU' while expecting 'stamped / mail'
warning: for tile 326 (numbered 326) of objects.txt,
found 'LOREM IPSUM' while expecting 'ETAOIN SHRDLU'
The recent addition of the first new extra scroll descriptions in a
very long time caused this problem to show up when MAIL was undefined.
There was a magic number in use that made an assumption that there
were only 4 such extra scroll descriptions, those being
"FOOBIE BLETCH", "TEMOV","GARVEN DEH","READ ME"
This isn't urgent, but I figure that until the mac build stuff gets
merged in, the core is still fair game....
'O' command's autopickup_exceptions was freeing a menu pick-list even
when it hadn't been allocated (for the list case, and for the remove
case if nothing was chosen for removal). That code was evidently used
as the model for msgtype and menucolors; they had the same situation.
I think ANSI and ISO sanction free(NULL) as a no-op, but pre-ANSI free
implementations don't necessarily handle that benignly. Even if they
all do, freeing something--even if that 'something' is nothing--which
hasn't been allocated is a bug on our end.
Replace several 'foo = alloc(strlen(bar)+1), strcpy(foo,bar)' sequences
with 'foo = dupstr(bar)' calls.
Change 'free(foo)' into 'free((genericptr_t) foo)' to possibly pacify
'lint' and/or really old compilers.
Add braces around 'if something;' when 'else { otherwise; }' has braces.
Simplify option value formatting for 'sortloot'.
Free sysopt.shellers and sysopt.explorers when releasing the memory used
for other sysopt fields.
Also some formatting stuff since sys.c was previously untouched.
This one has a couple of code changes included, but they shouldn't
produce any change in game play. If anyone adds a new shirt or shield
they'll have to update the corresponding foo_on() and foo_off() routines
to avoid an 'impossible' when putting on or taking off the new item(s),
the same situation as already happens for other subclasses of armor.
The automated reformatting put a space in casts of the form
'(type)(expression)', yielding '(type) (expression)', but it didn't
do that for '(typedef)(expression)'. There are lots of instances of
'(boolean)(expression)'; (uchar) and (xchar) also occur. I haven't
noticed other types, but I haven't looked in very many files yet.
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.
Fixing up mis-indented block comments, but hit some files that hadn't
had the earlier mixture of tab replacement, etc, so it's bigger than I
expected. If I get to it, they'll be another round of this tomorrow.
The intended change was that you'd get an increase to max energy if
current was sufficiently close to max rather than only when it was
already at max. It wasn't intended that you'd fail to have current
boosted all the way to max in the case where it wasn't sufficiently
close. That's fixed here.
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.
The corridors used to head towards the goal using the straightest
possible line, often making a zig-zag beeline. Allow some slight variance,
sometimes going straight instead of turning, reducing the predictability,
and making those monotonously turning corridors less likely.
While the topic of strprepend() is current, make good use of it.
Simplify code which inserts "the Nth " in front of "<arrow,&c>".
I'm pretty sure there are one or two other places where I assumed that
the outpuf of xname() was a char array which is BUFSZ in length rather
than BUFSZ-PREFIX, and reused the buffer, but I don't know where they
occur. (BUFSZ-PREFIX is still big enough to hold most things, so it
might not lead to trouble.)
Fix the reported bug that attempting to cast an expired spell, which
causes confusion and/or stun, was replacing the duration of any existing
confusion or stun with the new amount rather than increasing it by that
amount.
Attempting to cast any spell while stunned will now fail immediately,
and casting an expired spell while confused will increase confusion
duration (and/or set stun duration) rather than override it.
Same sort of stuff as before: some continuation lines with operator
followed by end of line comment (only a few files with those still to
go...), plus tab replaced by spaces in comments, excess parenthesis
removal for return statements, and force function name to be in column
one in function definitions:
type name(args) /* comment */
argtype args;
{
to
/* comment */
type
name(args)
argtype args;
{
I've been spotting those by eye rather than rexexp, so probably missed
some.
When doing some more reformatting I came across something I've been
meaning to tweak for a long time, and since the change is only a couple
of lines I'm putting it in now instead of waiting. Make potions of
gain energy more useful for actually regaining energy so they might not
be relegated to alchemy all the time. The adjustment is probably too
low to really achieve that, but I didn't want to risk going too high.
Increase to max energy is only a little higher (average 10.5 vs 9 for
blessed, 7 vs 4 for uncursed) but to current energy is noticeably higher
(31.5 vs 9 for blessed, 21 vs 4 for uncursed; capped by max energy so
bigger increase only matters if current is below max when quaffing).
This should address the issue that the problem patch to display_pickinv()
was trying to deal with: releasing the inventory window before exiting
the program so Pasi's memory checker doesn't think it's a memory leak.
Not related, but in the same file:
The older qsort comparison routines are tagged with CFDECLSPEC to deal
with some C vs C++ interaction issue. I added that to the relatively
recently added 'sortloot' qsort compare callback.
I also changed worn_wield_only(), although it isn't actually called.
(display_minventory() has provisions to call it, but both of the latter's
callers pass in MINV_ALL so allow_all() gets used instead.)
Noticed while going through more reformatting: can_carry() was changed
to return a number rather than yes/no, but it's trying to return a long
value (obj->quan) as an int.
Gold is the only thing likely to exceed LARGEST_INT in actual play
(although rocks could manage it if somebody tried hard enough). This
makes sure that the value returned doesn't exceed LARGEST_INT, but only
tame monsters honor the resulting subset value (at least for gold) and
split the stack. The proper fix is to convert can_carry() and all its
uses to long, but I'd rather spend my time on other stuff.
Reformat some trailing &&, || operators followed by end-of-line comment,
missed by the earlier continuation formating.
An
#if 0
something {
#else
something_else {
#endif
construct in rhack(cmd.c) confused the automated reformatter, resulting
in some code from inside a function ending up in column 1.
