[Freeciv-Dev] Re: Cleaning up gotohand.c (PR#1068)

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Tue, Dec 04, 2001 at 05:08:56PM +0000, Gregory Berkolaiko wrote:
>  --- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote: 
> > On Tue, Dec 04, 2001 at 04:00:05PM +0000, Gregory Berkolaiko wrote:
> > > 
> > > I would call it much ado about nothing, since it doesn't facilitate
> > the
> > > use of warmap and doesn't speed anything up.  And it is MUCH work to
> > > trace calls to warmap throughout the code and understand which warmap
> > > they are actually refering to.
> > 
> > But the separation of parts which can be seperated _is_ a long term
> > for me. And the goto code is a good example for a separable part of
> > code. It is IMHO a misdesign that the references of warmap are
> > sprinkled all over the code.
> 
> Hey, what do you mean by "separation"?

Encapsulation. Or in another way: no global variables. The goto code
is a good example: consider the global warmap is gone. Now all access
is done through functions/macros. These can be either of the form:
"get_cost_of_field(x,y)" (which would be a 1:1 mapping to the current
direct access) or to something like
"get_path_into_struct(start_x,start_y,end_x,end_y, various unit
attributes (like type, current, fuel, owner), struct path *result)"
This struct contains the path to take and the costs at the various
steps. This is what I want to achieve. And what I implemented (and if
I would have more time I would really be pushing this (into the CVS)).

> I thought you want to remove seacost field from warmap and do everything
> on one map (well, I agree it's more unification than separation)...

Separation between parts of the code. Each part (AI, server, gotohand
in this example) have a clean interface which is exposed. And a global
warmap variable isn't a clean interface as may already known.

> > > BTW, can it be done automatically, I mean whenever warmap.cost is
> > used,
> > > print a log message with the file:line info?
> > 
> > 1) A good interface with query functions/macros would avoid such
> > question in the first place.
> > 
> > 2) gdb has AFAIK a feature to catch access of a memory region.
> 
> I tried to do it once.  It dumps core
> 
> > 3) grep is your friend.
> 
> No I want runtime info.  You see warmap is generated once and then used
> all over and then once again generated and many times used...  I want to
> know which access corresponds to which generation.

You can do a search-and-replace and replace
"warmap\.(sea)?cost[.*?][.*?]" with "get_warmap(\1,\2,\3)".

> > > There is one step on the road to tightening interface, that I think
> > is
> > > worth taking.  There are 2 functions, really_generate_warmap and
> > > generate_warmap (which is a wrapper for really_generate_warmap, doing
> > > some initialisations).  Unfortunately, due to objective reasons (see
> > > below), really_generate_warmap is also called directly from the code,
> > > making it difficult to understand which initialisation should go
> > where. 
> > > One should probably unify it.
> > > 
> > > The "exceptional" warmap are generated when AI wants to know 
> > 
> > > "where unit A will be able to go when it comes to city X?"
> > 
> > Can you rephrase this?
> 
> generate_warmap tell you answer to the question
> "where unit A can go from where it is now"?
> 
> if you want an answer to the question
> "where unit A can go from city C which is over there"
> you'd have to access _really_generate_warmap directly.
> 
> Example:
> http://www.freeciv.org/lxr/source/server/settlers.c?v=cvs#L960
> 
> There punit wants to know where ferryboat can take him by sea from mycity
> (ferryboat is somewhere far away).
> 
> Two lines below punit want to know where it can go by land:  another
> place where land- and sea- warmaps must coexist.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Like the ad says, at 300 dpi you can tell she's wearing a
  swimsuit. At 600 dpi you can tell it's wet. At 1200 dpi you
  can tell it's painted on. I suppose at 2400 dpi you can tell
  if the paint is giving her a rash."
    -- Joshua R. Poulson