[Freeciv-Dev] (PR#1870) FreecivAC: borders patch

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

[ben@xxxxxxxxxxxxxxxxxxxxxx - Mon Jul  7 11:28:07 2003]:

> Are there any other issues that need to be fixed before CVS inclusion?

A few notes; some may be wrong (let me know):

- How come owners are sent across the network as a Uint16, but
MAP_TILE_OWNER_NULL is MAX_UINT8?

- map_get_adjc_city should probably use map_get_city not map_get_tile.

- The comment "(N.B. will probably need modifications to deal with other
topologies)." isn't true; since this code just uses adjc_iterate no
modifications will be needed. (This is in is_claimed_ocean.)

- is_claimed_ocean is buggy; what if there is continent A with 1
adjacent tile and continent B with 5 adjacent tiles to the ocean, but
continent A happens to be found first?  We'll see that numland==1 and
the tile will be unclaimed.  The rule could be changed so that that if
two different continents touch a tile it cannot be claimed by either
(making for a simple code change instead of a harder one, and probably
better results).

- Why is map_get_adjc_city required for map_get_closest_city?  Is this
just to speed things up?

- Recalculating borders is hideously expensive.  A better algorithm
should be possible, but using square distance makes things tricky.

- map_update_borders_landmass_change is problematic.  Both because it
recalculates the whole world and because it has to resend the whole
world (I think).

- map_calculate_territory looks like it is O(n*n*m*m) in time (n=# of
cities, m=map.borders).  This can definitely be improved on.

- Why is civ_score completely rewritten, again?  Surely this could be
avoided?

- If map.borders==0, the "hack" to use MAP_DEFAULT_BORDERS to calculate
landarea is too ugly, IMO.  Just use an approximation, calculate the
area by hand, or use the old landarea calculation.

  whole_map_iterate(x, y) {
    if ((pcity = map_get_closest_city(x, y))
         && sq_map_distance(x, y, pcity->x, pcity->y) < cl_sq_dist) {
      area[pcity->owner]++;
    }
  } whole_map_iterate_end;

- Moving civ_score and associated functions into the server is a task in
its own IMO.  We might as well have a separate patch.

Looking at the patch I'm even more convinced we should start with
omniscient-POV borders (as your patch does).  It will make debugging
much easier.

jason