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

[Gregory Berkolaiko <gberkolaiko@xxxxxxxxxxx>]

 --- 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);
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".  Making default warmap local.
2. Introduction of allocation/destruction methods.
3. Change generate_warmap -> cost_map_generate

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.

> 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 a series of patches which each centered around a
> generate_warmap() call. Something like this.

Yes, then we will try to localise each warmap.

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