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

[David Weisgerber <tnt@xxxxxxxxx>]

Am Samstag, 18. August 2001 15:04 schrieben Sie:
> 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.

I'll change it to strcmp() ...

> > > 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...

I think that play and the bash writes a good understandable error message to 
stdout / stderr so we don't have to do this.

> > > 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.

Why not ?!

> > 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.)

Maybe we have to add a play command for other platforms ?!

Good night,
David

-- 
Boykottiert Microsoft - Kauft Eure Fenster im Baumarkt.