[Freeciv-Dev] Re: Changing interface for generate_warmap (PR#1108)

[Gregory Berkolaiko <gberkolaiko@xxxxxxxxxxx>]

 --- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote: 
> On Thu, Dec 13, 2001 at 06:10:33PM +0000, Gregory Berkolaiko wrote:
> > No "default" warmap is a jump too big for now.
> > There is a lot of confusion in the code now, with functions
> generating
> > warmap being few calls away from the functions using it etc.
> 
> To help you I have changed your patch. It now tracks an id of the
> generator. Note that 3 autogames have run without a message and there
> was also no message during a quick interactive testing. However this

Very interesting experiment.
There are, however, easy explanations to the observed irregularities:


> doesn't mean that the dependencies are complete. Interesting:
> 
> diff -u -r1.115 settlers.c
> --- server/settlers.c   2001/12/06 11:59:07     1.115
> +++ server/settlers.c   2001/12/14 07:58:08
> @@ -825,9 +833,9 @@
>          unit_types[pcity->currently_building].transport_capacity &&
>          !unit_flag(pcity->currently_building, F_CARRIER) &&
>         !unit_flag(pcity->currently_building, F_MISSILE_CARRIER)) {
> -      if (warmap.cost[pcity->x][pcity->y] < best) {
> +      if (get_warmap_land(NULL, pcity->x, pcity->y,0,0,0) < best) {
>          id = pcity->id;
> -        best = warmap.cost[pcity->x][pcity->y];
> +        best = get_warmap_land(NULL, pcity->x, pcity->y,0,0,0);
>          *x = pcity->x;
>          *y = pcity->y;
>        }
> 
> which means that the get_warmap_land calls are never reached.

Look at the broader picture:
=============================================
#ifdef ALLOW_VIRTUAL_BOATS
  city_list_iterate(pplayer->cities, pcity)
    if (pcity->is_building_unit &&
        unit_types[pcity->currently_building].transport_capacity &&
        !unit_flag(pcity->currently_building, F_CARRIER) &&
        !unit_flag(pcity->currently_building, F_MISSILE_CARRIER)) {
      if (get_warmap_land(NULL, pcity->x, pcity->y,0,0,0) < best) {
        id = pcity->id;
        best = get_warmap_land(NULL, pcity->x, pcity->y,0,0,0);
        *x = pcity->x;
        *y = pcity->y;
      }
    }
  city_list_iterate_end;
#endif
==========================================

Now it's clear.


> Also note that the maps generated by id 3001, 4000 and 5000 aren't
> used.

3001:
================
/* Something went wrong; fall through. This should almost never happen.
*/
  if (punit->x != x || punit->y != y)
    generate_warmap(NULL, punit->x, punit->y, punit,3001);
  x = punit->x; y = punit->y;
  /* Fallthough to next fase */
================
So I guess, the comment is correct (and the map is never generated -- I
tested it on AI-only run, could be different if you actively auto-explore
in human mode)

4000: (gotohand.c)
=================
  generate_warmap(NULL, punit->x, punit->y, punit,4000);

  if (is_sailing_unit(punit))
    return warmap->seacost[dest_x][dest_y];
  else /* ground unit */
    return warmap->cost[dest_x][dest_y];
=================
Mea culpa.  I should not use direct access to warmap.

5000:
=================
  if (*ferryboat)
    generate_warmap(NULL, punit->x, punit->y, *ferryboat,5000);

  generate_warmap(NULL, punit->x, punit->y, punit,5001);
=================
You see, first call generates sea-map.  Second call generates land-map
and _overwrites_ the id of the sea-map.  Both are used later, but sea-map
thinks that it's id is 5001

My biggest worry is that in find_boat a wrong map is sometimes used.  It
seems that sometimes find_boat looks for a transport closest to the
home-town of the unit, not caring whether the unit is there or miles
away.

But using warmap for finding boat is conceptually complicated: you have
to take into account both unit's current position and it's destination.

G.

__________________________________________________
Do You Yahoo!?
Everything you'll ever need on one web page
from News and Sport to Email and Music Charts
http://uk.my.yahoo.com