[Freeciv-Dev] Re: Sound patches to be added (diffs'll come later)

[Jason Dorje Short <jshort@xxxxxxxxxxxxx>]

David Weisgerber wrote:
> Am Samstag, 18. August 2001 10:25 schrieben Sie:

<snip sound.[ch] files for adding sound support>

> > I'm also not quite sure what strcoll() is supposed to be doing.  Could
> > you add a comment on that one?
> 
> The strcoll() function compares the two strings s1 and s2.
>        It returns an integer less than, equal to, or greater than
>        zero  if  s1  is  found, respectively, to be less than, to
>        match, or be greater than s2.  The comparison is based  on
>        strings  interpreted as appropriate for the program's cur­
>        rent locale for category LC_COLLATE.  (See  setlocale(3)).

Hmmm.  It seems odd that sound code should be checking translated
strings.  There should be a comment as to why it's better to do so.

> > You should probably check the return value of the system() call that
> > plays the sound.
> 
> and what sould I do with the return value ??? That doesn't bring something as
> you can see from the discussion. Either play doesn't fail than there is a
> sound play or play fails and it is like it was until this patch came.

My man page says that if the return value is 127 (and errno is set
appropriately) it means the program failed to execute.  An error message
(to freelog or possibly even stderr) should be sent in this case. 
Otherwise sound will silently fail!

Of course, even if it fails we're no worse off than we are now, but
that's no reason not to do things properly...

> > Naming all of the units shouldn't be done in the sound file.  If you
> > want to do it outside of the sources, it should ultimately be put into
> > some other spec file.  In fact, a lot of the options you've put into the
> > sound spec file are inappropriate - for instance, defining the play
> > command in this file means anyone using an incompatible platform must
> > use a different sound spec file!
> 
> No, I don't think so. First point: Naming the units in the sound file is good
> for mod makers.

Ugh.

> Second point: Maybe you define a specfile which doesn't play wave. Instead it
> plays MP3s or AC3s than you have to use another program.

Hmmm, good point.  How, then, will you deal with different platforms??? 
Can we have a separate config file that specifies different executables
to be used for different file types?  This could then be changed between
platforms without having to maintain separate versions of the main sound
spec file.  (This sounds like something for later patch to accomplish.)

jason