[Freeciv-Dev] Re: (PR#2370) Path finding

[Raimar Falke <rf13@xxxxxxxxxxxxxxxxx>]

On Fri, Feb 21, 2003 at 10:25:22PM +0000, Gregory Berkolaiko wrote:
> > > > > +   */
> > > > > +  int (*get_MC) (int from_x, int from_y, enum direction8 dir,
> > > > > +              int to_x, int to_y, void *);
> > > > 
> > > > We should also supply "known".
> > > 
> > > When any user code needs it, it will be added.
> > 
> > Based on this we would have to remove this parameter also from the
> > other callbacks. It should be added here and also to is_pos_dangerous.
> 
> Definitely add to is_pos_dangerous, I am surprised it's not there yet.
> Definitely not here.  I am not having it just for uniformity's sake.  
> This is a back-bone of the whole module and +- one byte makes a difference. 
> Give me a problem that requires "known" here and cannot be solved by existing
> call-backs and then we can negotiate.

This one extra argument is one extra push for the caller. Thats it. No
other cost. gcc doesn't pop but just adjusts the %esp directly. And it
has no influence on the called function (if the function doesn't use
it). I expect that a push is fast. Very fast since you need it
everytime you call a function.

> > > > Why did you remove get_known? It is required to handicap handling.
> > > 
> > > Please explain.
> > 
> > We want to limit the user knowledge of the map via this callback.
> 
> But there are standard callbacks?  Why should we pretend it's "known" when it
> isn't and vice versa?

Hmmm. It was there since the first version of the interface. It really
looks like it came obsolete with the introduction of map_get_known2 in
version 13.

> > > > > +static int single_seamove(int x, int y, enum direction8 dir,
> > > > > +                       int x1, int y1, void *data)
> > > > > +{
> > > > > +  /* MOVE_COST_FOR_VALID_SEA_STEP means ships can move between */
> > > > > +  if (map_get_tile(x, y)->move_cost[dir]
> > > > > +      == MOVE_COST_FOR_VALID_SEA_STEP) {
> > > > > +    return SINGLE_MOVE;
> > > > > +  } else {
> > > > > +    return PF_MAXCOST;
> > > > > +  }
> > > > > +}
> > > > 
> > > > Now I see why you need PF_MAXCOST. This should be changed. get_MC
> > > > should look like this:
> > > > 
> > > >   bool (*get_MC) (int *dest_MC, int from_x, int from_y, enum direction8
> > > > dir,
> > > >                   int to_x, int to_y, void *);
> > > 
> > > This is ugly.  PF_MAXCOST is a well-defined special value and it should
> > stay.
> > > I will have no cruft in the function signature, it's big enough as it is.
> > 
> > PF_MAXCOST is ugly. Comment from others?
> 
> While waiting for comments, I can suggest an alternative: return a negative 
> number.

This is slightly better. But still not clean.

> > > > > +  Must put owner into *data.
> > > > 
> > > > What about also giving the parameters to all callbacks?
> > > 
> > > They can be supplied through *data fields if needed.
> > 
> > Yes but the above starts to make exceptions. What about leaving the
> > *_data fields and passing the param struct * into the callback instead
> > of the _data pointers. This way the callback have both the "normal"
> > fields of the parameter and also the special _data fields.
> 
> Makes good sense.  Shall we cut down number of *data fields?  If needed *data
> can point to a struct with more data.

You mean cut down to one?

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Just because you put a flag on the moon doesn't make it yours, it just
  puts a hole in the moon."