Re: [Freeciv-Dev] [patch] kludge cleanup

[David Pfitzner <dwp@xxxxxxxxxxxxxx>]

Per Mathisen wrote:
> 
> There are a number of ugly kludges in the common/ code that needed to be
> cleaned up before CivWorld could be compiled successfully. Patch for
> this is below. These fixes do not change anything except how things are
> compiled, so there should be no surprises.
> 
> (These kludges were implicit calls in common/ to functions that were
> not implemented in common/ code, but were linked in from client or server
> code. This is the wrong thing to do, and very confusing. There is still
> one such kludge left, I will do it in some other time.)

> Content-Disposition: attachment; filename=kludgefixes

Comments:

> Index: client/xmain.c

> +  if ((d = XOpenDisplay(getenv("DISPLAY"))) == NULL) {
> +       fprintf(stderr, "Can't open X-Windows display.\n");
> +       exit(1);
> +  } else toplevel = XtVaAppInitialize(

Hmm, sneaking in other fixes?  :-)


> Index: common/city.c

> -    return (find_city_by_id(game.global_wonders[id]) != 0);
> +    return (game_find_city_by_id(game.global_wonders[id]) != 0);

This is bad.  In the server, find_city_by_id is O(1)
whereas game_find_city_by_id is O(N_all_cities), and
the above is in an often-used function.

There is a reason Syela wrote:

> -struct city *find_city_by_id(int id);
>  /* this funct is no longer in this module, but leaving it proto'd
>  here saves more headaches than you can imagine -- Syela */


> Index: common/player.c

> -    pcity = find_city_by_id(city_id);
> +    pcity = game_find_city_by_id(city_id);

Ditto.  The main point of the function containing this
bit was to use the fast find_city_by_id() where possible.


Solution?
Perhaps put a definition for find_city_by_id() in city.c,
as a simple wrapper to game_find_city_by_id(), but with
the definition controlled by a #define which is off in 
freeciv but can be turned on to be able to compile civworld?


> Index: common/shared.h

> +int is_server;

I consider declaring variables with storage in header files bad.
That is, the header should have "extern int is_server;"
and the "int is_server" should be in shared.c

Regards,
-- David