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

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Sun, Dec 16, 2001 at 11:20:08AM +0000, Gregory Berkolaiko wrote:
>  --- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote: 
> > On Sat, Dec 15, 2001 at 02:56:57PM +0000, Gregory Berkolaiko wrote:
> > >
> > > Keeping in mind our more recent discussions I propose the following
> > > interface (which is practically implemented in the patch).
> > > 
> > > struct move_cost_map *cost_map_allocate();
> > 
> > > void cost_map_generate(struct move_cost_map *p, 
> > >                  int x, int y, struct unit *punit);
> > 
> > What are the semantics of this? When is punit used, when x,y?
> 
> (x,y) give the starting point of the warmap
> punit is used to deduce the move type characteristics like
> 1. Sea / land
> 2. Igter /normal
> 3. Number of move-points.
> punit doesn't have to be at (x,y).
> punit == NULL  means "city"-type warmap will be generated (it is not
> necessary to have a city at (x,y)).

What do you think about changing this to:

void cost_map_generate(struct move_cost_map *p,
                       int x, int y, struct unit_type *ptype, int move_left, 
                       int fuel,struct player *owner);
?

> This was explained in the comment before generate_warmap in my patch.
> 
> > 
> > > void cost_map_destroy(struct move_cost_map *p);
> > > 
> > > /* access functions */
> > > int cost_map_sea_cost(struct move_cost_map *p, int x,int y);
> > > int cost_map_land_cost(struct move_cost_map *p, int x,int y);
> > > 
> > > 1. All these methods can eventually go to a new file.
> > > 2. Old argument list of generate_warmap(pcity, punit) is unacceptable
> > > as it does not cover whole range of warmap's use, forcing the direct
> > > use of really_generate_warmap.
> > > 3. We can remove "vector" field from move_cost_map to save space and
> > > make a new struct path_map for use by methods like find_the 
> > > shortest_paths and air_can_move_between.
> > > 
> > > To make reviewing easier I can split the patch into parts:
> > > 1. Introduction of "access functions". 
> > 
> > In the header file only or do you want to change all "warmap" uses?
> 
> Change all warmap uses.  I can split it into ("introduction of access
> function" and "changing all warmap uses" parts but that is a bit
> artificial IMO).

I don't like this. You change every access and than will touch these
lines a second time if you change them to use a non-global
cost_map. This is unnecessary IMHO. Only change the accesses which you
transform to the non-global case.

> > > Making default warmap local.
> > 
> > > 2. Introduction of allocation/destruction methods.
> > 
> > This is ok.
> > 
> > > 3. Change generate_warmap -> cost_map_generate
> > 
> > Have you changed the signature of generate_warmap at this time?
> 
> yes (if you mean prototype)

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Any sufficiently advanced technology is indistinguishable from magic."
    -- Arthur C. Clarke