[Freeciv-Dev] Re: PATCH: borders update

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Fri, Oct 05, 2001 at 12:37:52AM +0100, Stewart Adcock wrote:
> Whoops.  It is attached now.

> +Unresolved issues
> +-----------------
> +To do or fix:
> +* Several portions of code are 'less-than-optimal'.  These will be recoded 
> in due course.
> +* fog-of-war artifacts.  Maybe fixed?  Needs more playtesting.
> +* Civ score uses different analysis of territory.  Should be easy to alter, 
> but is it desirable?
> +* Define territorial claim radius in ruleset.  i.e. Replace 
> MAP_CITY_INFL_DISTANCE constant.
> +* Occasional buggy worker states for tiles.  The sanity checks pick up 
> occasional problems.  Needs more thought.

IMHO these should be sorted out before inclusion.

> +* Optional minimap view coloured to show territories.

This would nice.

> +* Only the GTK client has been modified.

No problem

> +* Only the isometric view has been tested.

This should also be fixed.


> +  int thisowner, leftowner, aboveowner;      /* Nation claiming the tile. */
>  
> +    /* Draw national borders. */
> +    if (draw_borders && !citymode) {
> +      thisowner = map_get_owner(x,y);
> +      leftowner = map_get_owner(x-1,y);
> +      aboveowner = map_get_owner(x,y-1);
> +
> +      /* left edge: */
...
> +      /* top edge: */

You may want to look at the patch from "Sat, 22 Sep 2001 16:30:04
+0200" with subject "Re: [Freeciv-Dev] Re: Grid color". We want to
maximize the code that is shared between the clients.

> +    /* the nation colour. */
> +    colbuf[0] = '#';   /* NOTE: Should replace with a pixmap. */
> +    colbuf[1] = '\0';
> +    row[3] = colbuf;

Will this be fixed?

> +  row[10] = (char *) player_addr_hack(&game.players[i]);     /* Fixme */

What should be fixed?

> +  int foundation_date;        /* Date this city was founded -- used only in 
> server */

IMHO you should use the turn and not the year.

>  /***************************************************************
> +Return Player_no for nation owning this tile, return
> +-1 for unclaimed tiles.
> +***************************************************************/
> +int map_get_owner(int x, int y)
> +{
> +  if (!normalize_map_pos(&x, &y))
> +    return MAP_TILE_OWNER_NULL;
> +  else
> +    return (map.tiles + x + y * map.xsize)->owner;
> +}
> +
> +/***************************************************************
> +Set the owner of a tile.
> +***************************************************************/
> +void map_set_owner(int x, int y, const int owner)
> +{
> +  assert(is_real_tile(x, y));
> +  normalize_map_pos(&x, &y);
> +  (map.tiles + x + y * map.xsize)->owner = owner;
> +}

> +  int owner;            /* Player_no owning this tile, or -1 for unclaimed. 
> */

Since we want to avoid ids it would be nice if the owner is saved as a
"struct player *".

> +#define MAP_TILE_OWNER_NULL   MAX_UINT8

What is this? Is this -1? Why haven't you used MAP_TILE_OWNER_NULL
instead of -1?

> +  iget_uint8(&iter, &packet->owner);

There should be a "has_capability(...)" call.

> +  cptr=put_uint8(cptr, pinfo->owner);

Same here.

> +/***************************************************************
> +Return pointer to the oldest adjacent city to this tile.  If
> +there is a city on the exact tile, that is returned instead.
> +***************************************************************/
> +struct city *map_get_adjc_city(int x, int y)
> +{
> +  struct city *closest=NULL;   /* Closest city */
> +  struct tile *ptile;
> +
> +  assert(is_real_tile(x, y));
> +  normalize_map_pos(&x, &y);
> +
> +  ptile = map_get_tile(x, y);
> +  if (ptile->city) return ptile->city;
> +
> +  adjc_iterate(x,y,xp,yp) {
> +    ptile = map_get_tile(xp, yp);
> +    if ( ptile->city && 
> +         (!closest || ptile->city->foundation_date<closest->foundation_date) 
> )
> +      closest = ptile->city;
> +  } adjc_iterate_end;
> +
> +  return closest;
> +}
> +
> +/***************************************************************
> +Return pointer to the closest city to this tile on the same
> +continent.  If two or more cities are equally distant, then
> +return the oldest (i.e. the one with the lowest id).  This
> +also correctly works for water bases in SMAC mode.
> +***************************************************************/
> +struct city *map_get_closest_city(int x, int y)

You may want to unifiy this with ai/aitools.c:dist_nearest_city

> +/***************************************************************
> +Return the id of the closest city to this tile.  If two cities
> +are the same distance, then return the oldest.
> +Return '-1' in event of not finding a city.
> +***************************************************************/
> +int map_get_closest_city_id(int x, int y)

Don't use int but "struct city *"

game.borders isn't used.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Programming today is a race between software engineers striving to
  build bigger and better idiot-proof programs, and the Universe trying
  to produce bigger and better idiots. So far, the Universe is winning."
    -- Rich Cook