Hi,
> Just to summarise the bugs I'm aware of:
>
> * Save games still don't work and haven't for a long time (at least
> regarding sound). I'm looking at the sound part of this currently.
>
> * _findfirst error message when saving/restoring games.
Isn't this directly related (or even the cause of) sound saves not
working on Win32?
> * Having the reverse_stereo variable specified in the config file causes a
> crash on init_midi_device() where the heap appears to become corrupt.
>
> * Cannot save under event sound server.
> I need Christoph's help here with adding the extra variables that the event
> sound server uses (and to remove the obsolete ticks_to_fade). Are there
> instructions somewhere? Unfortunately there may not be a way to save a game
> under the event sound server (for example) and then restore it under the
> polled ss, since the last_played variable wouldn't be correct.
This would imply insufficient abstraction of the sound server state. I'd
have to have a closer look at why ticks_to_fade is obsolete etc. first.
> * Does the SDL full screen crash bug still exist?
> Does Matt or anyone have comments or additions to this?
I have three:
* Suspending the main process causes the sound server process to die on
SIGPIPE (reported by Claudio). This will need two additional signal
handlers.
* Restoring a savegame doesn't reset the copy of the game's savegame
directory (A problem caused by the long savedir name hack).
* When building with dmalloc, I get a segfault early on.
This is in addition to the 59 (on last count) interpreter bugs on the BTS
that are slowly starting to decompose...
> Regarding the memset in song.c: isn't it just good practice to clear a
> struct before you use it, especially if not all of the struct's variables
> are initially initialised (sic)?
For engineering purposes it is (->practice); mathematically, however, it
adds unneccessary complexity (->theory): Being more complex, it makes
things harder to read.
For practical reasons speaking against it, consider the following
(slightly academical) example:
int
check_foo(int max_value)
{
int i1 = 0;
int i2 = 0;
for (i1 = 0; i1 < max_value; i1++)
if (foo(i1))
return i2; /* (mark) */
quux(&i2);
if (i2 > i1)
return i2;
else
return i1;
}
Clearly, 'i2' at the (mark) should've been i1- a simple typo. Normally,
a compiler would warn about this, but, here, the developer will get
incorrect results without any warning. Assigning an arbitrary (!) value to
the variable in order to prevent an uninitialized read has backfired by
provoking the read of an incorrect value, an error which is much harder to
trace.
(This is different with pointers, of course, where setting them to NULL
increases the chance of things breaking apart on error -> Good Thing).
Here it's a more or less direct cause of C (pre-C99) not allowing variable
declarations in arbitrary positions in the code and the programmer
choosing bad variable names.
Anyway, What I mean to say with this is that initializing is not
neccessarily good practice in all cases; think of the uninitialized
variables implicitly being set to an 'invalid' value to allow you tracking
their uninitialized use (with a sufficiently powerful compiler or
development support tool). Initializing variables "because it's good
practice" sounds a lot like masking other mistakes to me, and this
certainly isn't "good practice".
llap,
Christoph
