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

[Raimar Falke <rf13@xxxxxxxxxxxxxxxxx>]

On Fri, Feb 21, 2003 at 08:31:10PM +0000, Gregory Berkolaiko wrote:
> Quoting Raimar Falke <rf13@xxxxxxxxxxxxxxxxx>:
> > > + *  FORMULAE:  
> > > + *  - calculating total_MC
> > > + *  in TM_NONE
> > > + *    total_MC = sum of MC
> > > + *  in TM_CAPPED
> > > + *    total_MC = sum of MIN(MC, move_rate)
> > > + *  in TM_*_TIME
> > > + *     total_MC = ((turn + 1) * move_rate - moves_left)
> > 
> > I don't know what "in TM" means here.
> 

> TM was defined earlier and a reference to a more detailed
> explanation was given.

It looks still odd. Can't this be:

FORMULAE:  
- calculating total_MC
   * TM_NONE: total_MC = sum of MC
   * TM_CAPPED: total_MC = sum of MIN(MC, move_rate)
   * TM_*_TIME: total_MC = ((turn + 1) * move_rate - moves_left)

> > > +/* 
> > > + * Full specification of a position and time to reach it.
> > > + */
> > > +struct pf_position {
> > > +  int x, y;                      /* Coordinates of the position */
> > > +  int turn, moves_left;          /* See definitions above */
> > > +  int MC_of_next_step;           /* MC of the next step */
> > > +
> > > +  int total_MC;                  /* Total MC to reach this point */
> > > +  int total_EC;                  /* Total EC to reach this point */
> > > +
> > > +  enum direction8 dir_to_next_pos;       /* Unsed only in struct_path */
> > 
> > > +  enum direction8 dir_to_here;   /* Where did we come from */
> > 
> > This wasn't there the last time. It introduces redundancy. But it can
> > stay.

> You are mistaken, there is no redundancy.

In the path there is redundancy: we have dir_to_next_pos, dir_to_here
and (x,y). If you have (x_prev_step, y_prev_step) one of the three is
enough to calculate the others.

If you only have a single position there is obviously no redundancy.

> It should definitely stay.

> It was introduced as it was needed in the AI.

I expected this.

> > > +   */
> > > +  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.

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

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

> > > +  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.

> > > +  Alternatively,we can change the flow to
> > > +  if (ptile->terrain == T_OCEAN) {
> > > +    move_cost = PF_MAXCOST;
> > > +  } else if (terrain1 == T_OCEAN) {
> > > +    move_cost = SINGLE_MOVE;
> > > +  } else {
> > > +    move_cost = ptile->move_cost[dir];
> > > +  }
> > > +  which will achieve thesame without call-back.
> > 
> > Why didn't you done this?
> 
> For educational purposes.

??

> > > +/************************************************************ 
> > > +  Reversed LAND_MOVE cost function for a unit.
> > 
> > > +  Will be used. DO NOT REMOVE you pricks.
> > 
> > When will it be used?
> 
> After path-finding gets committed.

Obviously. But why not earlier?

> > I still think that the above two macros should be prefixed with "ai",
> > "plain", "simple", "basic" or similar since there are the most plain
> > use of PF. There is no EC handling in them. Either users of PF want
> > this.
> 
> I have no opinion on that.  The name must not exceed 77 characters in total,
> however.

There is another limit before this ;)

5.2.4.1 Translation limits 

The implementation shall be able to translate and execute at least one
program that contains at least one instance of every one of the
following limits:
...

 - 63 significant initial characters in an internal identifier or a
 macro name (each universal character name or extended source
 character is considered a single character)

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
  The trick is to keep breathing.