[Freeciv-Dev] Re: patches

[Jason Dorje Short <jshort@xxxxxxxxxxxxx>]

"Ross W. Wetmore" wrote:
> 
> Better.
> 
> But note that the rotational system allows you to code the
> DIR_IS_CARDINAL test into the loop parameters so you only
> iterate over the right ones in the loop code as well.
> 
> All the iterate macros are the same tight loop code just with
> different start/stop/increment values as loop parameters.
> 
> And there is a sleazy trick that lets you do this even with
> the DIR_DX system. Define a mapping array from one to the
> other, Do the loop over DIR_DX2 indicies, and do a lookup
> aka transformation whenever you need a "dir" value for DIR_DX
> i.e. _itr_dir = DX2toDX[_loop_dir].

Ross,

I entirely agree that a better system is possible.  But I do think an
intermediate step such as the patch I proposed is not a bad idea; at a
minimum it reduces the amount of code we have to deal with in a later
change.


Raimar and others,

Trent proposed such a macro a month or two ago.  I would change it so
that it is schema-independent.  It would be #defined as something like
this:

#define adjc_all_iterate(center_x, center_y, 
                         itr_x, itr_y, itr_dir, card)
{
  int itr_dir, itr_x, itr_y;
  for (itr_dir=0;
       itr_dir<8;
       itr_dir += 1 + card ? DIR_NEXT_CARD(itr_dir+1) : 0) {
    itr_x = center_x + DIR_DX[itr_dir];
    itr_y = center_y + DIR_DY[itr_dir];
    if (!normalize_map_pos(&itr_x, &itr_y))
      continue;

DIR_NEXT_CARD returns the difference to the next cardinal direction from
the one given; in the case of a rotational system it would just be "1"
whereas for the current system it would be much more complicated.

A macro like this allows us to write all iterators as wrappers to this
one, saving dozens of lines of code and making things simpler overall
(IMO, of course).  It should also be able to retain the efficiency of
the current code (if we use a rotational system).

#define adjc_iterate(center_x, center_y, itr_x, itr_y)
  adjc_dir_iterate(center_x, center_y, itr_x, itr_y, _itr_dir)

#define adjc_dir_iterate(center_x, center_y, itr_x, itr_y, itr_dir)
  adjc_all_iterate(center_x, center_y, itr_x, itr_y, itr_dir, 0)

#define adjc_cart_iterate(center_x, center_y, itr_x, itr_y)
  adjc_all_iterate(center_x, center_y, itr_x, itr_y, _itr_dir, 1)

And it could also be used in places where the cartesian nature of the
iteration loop is a variable (there are 4-6 such loops IIRC).

Irrelevancies like backslashes, internal macro variables, and a
time-saving check for border tiles have been left out of this example
code, of course.

jason