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

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Sat, Dec 15, 2001 at 02:56:57PM +0000, Gregory Berkolaiko wrote:
>  --- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote: 
> >
> > For a transition time I think a parallel interface would be good. This
> > allows you to change one function at a time to use a local warmap and
> > also reduce the patch size.
> > 
> > struct move_cost_map *cost_map_create(struct city *pcity, struct unit
> > *punit);
> > void cost_map_destory(struct move_cost_map *p);
> > 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);
> 
> 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?

> 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?

> 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?


> Then, possibly:
> 1. Introduction of struct path_map.  Conversion of find_path/direction to
> use path_map.

> 2. Relocation of the cost_map code into separate file.

I'm not sure about this. Currently I think that extra functions (which
work on a separate struct) in gotohand.[ch] are ok.

> > You can exchange "cost_map_" with something different. And if you use
> > "warmap" you have to explain what "warmap" means ;) The first patch
> > has to do the changes in gotohand.c (please don't introduce extra
> > cache rules, in the long run we want to avoid the global warmap). And
> 
> I did not introduce any new cache rules, I just made the old ones more
> explicit and documented.

> Caching seems to be useful as about 50% of the calls to generate_warmap
> get a recycled warmap (usually city's).  But if I turn the caching off,
> the run time doesn't seem to be affected which I don't know how to
> explain.

Than make no caching and we will find out what to cache when is
caching useful by profiling.

> > than a series of patches which each centered around a
> > generate_warmap() call. Something like this.
> 
> Yes, then we will try to localise each warmap.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
  "With a PC, I always felt limited by the software available.
   On Unix, I am limited by my knowledge."
    -- Peter J. Schoenster <pschon@xxxxxxxxxxxxxxxxx>