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

["ThMO" <thmo-13@xxxxxx>]

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

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)

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.

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

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

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

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

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

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.

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.

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 ?

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

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

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?

---

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

THX for listening.

CU Tom.
(Thomas M.Ott)
Germany

--- freeciv-2.0.8/utility/shared.h.orig 2006-03-02 07:33:38.000000000 +0100
+++ freeciv-2.0.8/utility/shared.h      2006-04-22 14:30:50.000000000 +0200
@@ -218,7 +218,11 @@
 int cat_snprintf(char *str, size_t n, const char *format, ...)
      fc__attribute((__format__ (__printf__, 3, 4)));
 
+#if    __GNUC__ == 2 && __GNUC_MINOR__ == 7
+#define die( args...) real_die(__FILE__, __LINE__ , ## args)
+#else
 #define die(...) real_die(__FILE__, __LINE__, __VA_ARGS__)
+#endif
 void real_die(const char *file, int line, const char *format, ...)
       fc__attribute((__format__ (__printf__, 3, 4)));
 
--- freeciv-2.0.8/utility/log.h.orig    2006-03-02 07:33:38.000000000 +0100
+++ freeciv-2.0.8/utility/log.h 2006-04-22 14:32:12.000000000 +0200
@@ -88,12 +88,21 @@
     }                                                                       \
   } while(FALSE)
 #else
+#if    __GNUC__ == 2 && __GNUC_MINOR__ == 7
+#  define freelog(level, args...)                                           \
+  do {                                                                      \
+    if ((level) != LOG_DEBUG) {                                             \
+      real_freelog((level) , ## args);                                  \
+    }                                                                       \
+  } while(FALSE) 
+#else
 #  define freelog(level, ...)                                             \
   do {                                                                      \
     if ((level) != LOG_DEBUG) {                                             \
       real_freelog((level), __VA_ARGS__);                                   \
     }                                                                       \
   } while(FALSE) 
+#endif
 #endif  /* DEBUG */
 
 #endif  /* FC__LOG_H */
--- freeciv-2.0.8/utility/fciconv.h.orig        2006-03-02 07:33:38.000000000 
+0100
+++ freeciv-2.0.8/utility/fciconv.h     2006-04-22 14:32:54.000000000 +0200
@@ -34,7 +34,11 @@
 char *local_to_internal_string_buffer(const char *text,
                                      char *buf, size_t bufsz);
 
+#if    __GNUC__ == 2 && __GNUC_MINOR__ == 7
+#define fc_printf( args...) fc_fprintf(stdout , ## args)
+#else
 #define fc_printf(...) fc_fprintf(stdout, __VA_ARGS__)
+#endif
 void fc_fprintf(FILE *stream, const char *format, ...)
       fc__attribute((__format__ (__printf__, 2, 3)));
 
--- freeciv-2.0.8/client/text.c.orig    2006-03-02 07:33:38.000000000 +0100
+++ freeciv-2.0.8/client/text.c 2006-04-22 14:46:32.000000000 +0200
@@ -51,8 +51,13 @@
                     const char *format, ...)
 fc__attribute((__format__(__printf__, 3, 4)));
 
+#if    __GNUC__ == 2 && __GNUC_MINOR__ == 7
+#define add_line( args...) real_add_line(&out,&out_size , ## args)
+#define add( args...) real_add(&out,&out_size , ## args)
+#else
 #define add_line(...) real_add_line(&out,&out_size, __VA_ARGS__)
 #define add(...) real_add(&out,&out_size, __VA_ARGS__)
+#endif
 
 #define INIT                   \
   static char *out = NULL;     \
--- freeciv-2.0.8/configure.orig        2006-03-06 08:04:47.000000000 +0100
+++ freeciv-2.0.8/configure     2006-04-22 14:20:15.000000000 +0200
@@ -1,4 +1,4 @@
-#! /bin/sh
+#!/bin/bash
 # Guess values for system-dependent variables and create Makefiles.
 # Generated by GNU Autoconf 2.59.
 #
@@ -8954,7 +8957,7 @@
   echo "$as_me: failed program was:" >&5
 sed 's/^/| /' conftest.$ac_ext >&5
 
-ac_cv_c99_variadic_macros=no
+ac_cv_c99_variadic_macros=yes  ###no
 fi
 rm -f conftest.err conftest.$ac_objext conftest.$ac_ext
 fi