[Freeciv-Dev] Re: (PR#16688) v2.0.8: patch + bug report

["Jason Dorje Short" <jdorje@xxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=16688 >

ThMO wrote:
> <URL: http://bugs.freeciv.org/Ticket/Display.html?id=16688 >
> 
> Hello folks,
> 
> I'd like to state some things I've noticed.
> 
> · bugs related to compiling:
> ============================
> 
> 1.)  variable macro arguments:
> I've attached a small unified diff, patching 3 header files and one
> C file, in order to correctly compile freeciv with gcc v2.7.2.1 .
> Please notice the space-surrounded comma before the pre-processor
> concat operator, which is mandatory, so it can be eliminated in case
> no more arguments are given to the macro.
> I don't know, if gcc v2.95.3 is able to handle C99 variable macro
> arguments - if not, the pre-processor #if instruction could be
> reduced to  `#if __GNUC__ < 3' .

What systems can't use gcc3 that use gcc2?  Varargs is a C99 standard 
and should be supported by all modern compilers.

> 2.)  configure using a *real* bourne shell:
> The configure script uses some constructs, namely == for checking
> equality, which isn't understood by an older bourne shell (the C-like
> equality operators have been added by POSIX lately, but the single =
> sign works even on a brand new POSIX-like bourne shell).
> My change for requesting /bin/bash is just a hack to let it work.
> (IMHO even /bin/ksh might work, but I haven't tested that)

I don't know much about shell scripting.  Do you have a patch for this?

> 3.)  unrespected CFLAGS:
> One thing I really hate is the fact, some configure script *ignores*
> my CFLAGS settings - configure does honor my CC settings, but overrides
> CFLAGS with it's own compilation flags - it's really painful to hand-edit
> all Makefiles before compiling, only to get *my* flags used.

Add CFLAGS to the make line.  In my .bashrc I have

alias make="nice -n 15 make CFLAGS=\"-g -O0 -Wall -Werror -Wcast-align 
-Wmissing-prototypes -Wmissing-declarations\""

> A word to append some warning flags: it's O.K. to append those flags:
>   -Wall -Wpointer-arith -Wcast-qual -Wcast-align -Winline
> (-Wcast-qual isn't appended by configure, but I'm using this warning
>  option frequently)
> but it is *not* O.K. to append the following:
>   -Wmissing-prototypes -Wmissing-declarations
> Reason: in an optimized compilation with -ffast-math appended, all math
> functions residing in libm are declared using `static inline __<func>',
> for which no prototypes are necessary, forcing the compiler to give use=
> less warnings for those functions, which aren't normally declared inside
> math.h, e.g. __fsincos(), etc. pp.
> That way, I need to link in libm only, if the address of a math function
> is needed...

AFAIR the extra flags are only added if --enable-debug is given.

> BTW, your overridden CFLAGS setting include -O3, which will lead to
> problems for newer gcc's like v3.x and especially v4.x, since those com=
> pilers often miscompile things when given -O3 ...
> And since `-O3 == -O2 -finline-functions' this isn't necessary, since inline
> unconditionally all functions doesn't produce faster programs - mostly
> they will run somewhat slower, bug getting bigger - IMHO the best way
> is to include some functions specifically using `inline' then...

Every "miscompilation" I've ever seen is actually a bug in the program 
that just doesn't show up under regular optimization.  However I have no 
objection to changing the default flags.

> 4.)  Xaw[3d]: Freeciv.h -> resource.c:
> Personally I would prefer a configure option, to omit including the
> app-defaults inside the client, as I always do install the app-default
> file in order to customize it, that way reducing the size of the
> client from 820k -> 680k ...

If that's the case it shouldn't be a configure option - it should just 
always be done that way.  I have no objection to it.

> 5.)  compiling against libXaw3d:
> Many "problems" arise, because the false set of header files are used
> for compiling, since the headers do differ from Xaw - for that, I always
> do the following before starting the compilation:
>   cd client/gui-xaw
>   mkdir X11
>   cd X11
>   ln -s /usr/X11R6/include/X11/Xaw3d Xaw
> By using -I. this will ensure, that the correct header files be included.
> This step could be done by configure...

I haven't compiled with xaw3d in a long long time.  Can you provide a 
patch to fix its compilation?

> · bugs related to running:
> ==========================
> 
> 6.)  NLS using -lintl and -liconv:
> Since the data files are in utf-8 format, I'm using the following ENV:
>   LANG=de
>   FREECIV_LOCAL_ENCODING=iso-8859-1
>   FREECIV_INTERNAL_ENCODING=iso-8859-1
>   FREECIV_DATA_ENCODING=utf-8

There should be no need to set the encoding environment variables; they 
are intended for debugging use.  Freeciv is supposed to detect the 
correct values all the time.  If you run into a problem with the regular 
detection you can try fiddling with them but you should also report it 
as a bug.

> This will lead to messages like `coudn't convert from utf-8 to latin1'
> *several* times.  I've tracked down the problem, since I'm using the
> german translations, that an already converted word like `Königin' will
> get converted a second time from utf-8 to latin-1, which will force this
> error.  The reason for that is, that -lintl already uses -liconv, if it
> is present, which is the reason, why the gettext package needs to be
> compiled a 2nd time, since -liconv may use -lintl, unless --disable-nls
> has been given.

Gettext is told to use the internal encoding for its output (of course 
libintl uses iconv to convert to this encoding).  It is possible there 
are some places in the code where such a string is treated as a data 
encoding string, and therefore converted again.  If you can trigger a 
crash (add an abort call to the error message in fcintl.c) and get a 
backtrace we can track down such erronous flow.

> I've tried out several combinations of the above ENV settings, but none
> of them worked, so my solution to the problem was, to convert all data
> files (namely those unter ./nations) by hand to latin-1 and hand-modified
> `de.po', after utf-8 to latin-1 conversion and using the ENV
>   FREECIV_DATA_ENCODING=iso-8859-1
> Now it runs without errors, after hand-converting the save files too.

Of course if all encodings are equal there should never be any problems 
no matter how many times a string is converted from one encoding to another.

> In order to solve the problem, the configure script needs to check, if
> libintl already has been linked against libiconv, so the second iconv()
> call could be avoided in that case.

We assume libintl is linked against libiconv.  If it doesn't, then 
libintl is broken.  I suppose we could check for that and fail the 
configure but I hope that would never happen.

> The best solution would surely be, to flag *each* message string inside
> the program with it's corresponding charset, so things will not get con=
> verted several times...
> This could be done by specifying in each data file, which charset will
> be used inside it - that way, only those files, which need a different
> charset like latin-1, needs to be processed by iconv(), thereby speeding
> things up.  In fact only the eastern european folks, using latin-2 to
> latin-9, can't use latin-1, since those characters can't be represented
> using latin-1 - and only those really need charset conversions, like the
> eastern countries do, e.g. japanese, chinese, a.s.o.

In most cases there is no problem because there are rules for what 
encoding each string is in.  All global strings are in the internal 
encoding.

HOWEVER, one thing I just remembered is that for the server, the 
internal encoding is defined to be equal to the data encoding.  This 
could cause problems if you try to override things using environment 
variables.  Try setting the encodings separately for server and client 
and see if that helps.

> 7.)  server options:
> Starting the server like that:
>   ./civserver -s svdir -f game
> should load the file `./svdir/game.sav[.gz]', but it doesn't - instead
> it must be called
>   ./civserver -s svdir -f svdir/game.sav[.gz]
> By contrast - saving a game inside the server with `save blubber' will
> correctly save it as `svdir/blubber.sav[.gz]'.
> This behaviour should be equal on the command line, which means, the
> load operations needs to be held pending, until all parameters has been
> processed in order to get a potential directory path for save-games.

The -s command-line parameter does not affect the interpretation of the 
-f savegame name.  I don't think this is a bug.

> 8.)  client options:
> The manual states, that for a Xaw[3d] client all Xt options are valid,
> but doesn't state, that the Xt options need to be separated by `--'.
> E.g.
>   ./civclient -geometry +2+1 -n Thommy &
> results in an error message, specifying `-geometry' is unknown.
> Instead it must be called
>   ./civclient -n Thommy -- -geometry +2+1 &
> This should be documented inside the man-page.

Patch wanted.

> 9.)  client menu `view other server options':
> This option does nothing...  Is that the correct behaviour?
> BTW, I'm using the Xaw3d client, as it looks *much* better than the
> gtk v1 client, especially wehn it comes to the technology help.
> And hopefully you'll not consider dropping the Xaw3d client from this
> package, since I'm not going to install gtk v2 ?

This menu entry was removed some time ago when all server options were 
merged into one dialog.  I don't see it anywhere...but if you find it 
somewhere we should remove it.

> · things related to playing:
> ============================
> 
> 10.)  novice players playing the first time:
> It would be nice, if the server would start a new game by default for
> novice players, since those usually are not knowing the various server
> options, e.g. setting the endyear by default to 5000, since it's frus=
> tating, if the game ends in the middle of the playing and it isn't clear
> at first sight, why the game shall be over. (experienced players could
> surely change this parameters, since they need to change some other
> parameters too...)

The tutorial scenario is intended to do this.  I intend to add a button 
to the startscreen to start the tutorial.  Unfortunately the tutorial is 
not yet fully playable.

> It would be nice too, if there could be a mode, where the game never
> ends automatically - personally I don't like that behaviour, since I'd
> like to explore the full map in order to get experienced after playing
> many times that way...

Hmm, maybe.

> For novices it would surely be helpful, if the online-help (which is BTW
> done *very* well!) could include some ``how-do-I-do-that'' messages,
> like e.g.
>   · build a city on another island (settlers need to be transported by
>     a ship)
>   · what to do, if people are starving in a city (I don't know)
>   · I tried to build a bridge over the ocean to the next island, but
>     that doesn't work, although I've researched building a bridge
>     (I think, this could only be done over rivers on the same island?)
>   · adding a notice, that it seems, that each city can have at most
>     4 Handelsrouten (routes established by trucks for making more money)
>   · etc.pp.

You mean a FAQ section?

> 11.)  downloadable handbooks for offline-viewing:
> It would be great, if the handbooks could be downloaded for offline
> viewing, since it could get expensive in order to view them online
> through a dialup-line using a slow modem connection, since it's a pain
> in the ass to fetch them using `wget', modify them and accordingly
> fetch the additional pages, not retrieved by `wget', hand-modify them
> and start over again, since they'll be generated by some php-scripts...

What handbooks are you talking about?  I think all documentation should 
just be included with the game.  However a lot has been added to the 
wiki that is not listed anywhere else.

> 12.)  comfortable context menues:
> Especially for guys, like myself, playing this game for the first time,
> it would be very helpful, if e.g. pressing the right (or the middle)
> mouse button over e.g. a settler would open a menu with commands, this
> unit can do, like building a city, building a road, etc.pp.
> Although that might be difficult to implement?

Not that difficult.  I would like to do this but haven't gotten to it 
yet - some complain that popup menus are a "bad" interface, and another 
problem is all mouse buttons already do something.

> Finally I'd like to say a big THX for developing such a great game,
> making me sad, that I haven't noticed this game earlier - in fact
> it has only one drawback - it's very, very addictive...  ;-)

Thanks.

-jason