[Freeciv-Dev] Re: Sound patches to be added (diffs

[col@xxxxxxxxx]

> 
> > > > > 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.
> 
> What if the freeciv log is redirected with "-l" and the user will only
> look at it and not the normal std/stderr?

Come on, I don't think that making a hundred lines of code is better than 
telling the user to have a look @ stderr if sound fails.
 
> > > > > 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 ?!
> 
> The question is still: Why? Why include the names at all? Why is this
> better for mod makers?

I'll have some more looks @ the sources to do better.

> > > > 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 ?!
> 
> As a configure option or maybe also as a command line argument.

Who makes the windows/beos/amiga-clients ? Can I mail them ???

CU,
David