Hi,

> int sound_init_pipes(struct _state *s);
> int sound_map_instruments(struct _state *s);
> 
> Are these now redundant also?

Yes. sound_init_pipes() has been moved to _make_pipes(),
sound_map_instruments is now map_MIDI_instruments().

> Should a pointer to the sound_server_state_t struct be a part of state_t? It
> would be really useful but I'm not sure if it's appropriate.

It's potentially in a different address space, so it'd be pointless in
general.

> I can see that the currently playing notes are being kept track of. What is
> the purpose of this?

This was an attempt to control some problems with MIDI sequencers running
out of timbres (or similar) to play on.

> Similarly for the current instrument mappings. Is this
> for debugging purposes?

It's primarily so that we can restore them together with the rest of the
game state (soundserver.c, around L580).

> The next couple of questions relate to save games:
> 
> Why isn't the current_instrument array, storing the current instrument
> mappings, saved in a save game and restored?

I'm afraid that I couldn't find this array. The song->instruments
structure is definitely being saved and restored, though.

> When each song is saved, we have:
>   fd = creat(filename, 0600);
> ...
>   if (write(fd, seeker->data, seeker->size) < seeker->size)
> ...
> How do we know if fd has been opened in ASCII or binary mode? Why isn't
> fopen or open used here?

Because creat() is easier to read, IMO. I don't know if any additional
stuff has to be done on Win32; when trying to debug some sound problems on
it, Matt tested this and reported that changing it didn't help.
Of course this doesn't mean that it's neccessarily correct; you might want
to have a look at the Win32 docs and change it, if neccessary (please
notify us if you do so that we know how/why creat() should be called
differently/avoided on Win32).

> In SOUND_COMMAND_RESTORE_STATE, the following is not used:
>       long fadeticks_obsolete; /* FIXME: Keeps API compatibility */
> What is its purpose?

It's purpose is to keep API compatibility, and it's tagged as a FIXME. In
short, I was in a rush and didn't want to change the headers and recompile
everything when I got rid of its meaning.

If you see FIXMEs like this with variables not being used, you can just
remove them (and change the function implementations and declarations 
appropriately).


I hope this helps,

llap,
 Christoph


Reply via email to