[Freeciv-Dev] Re: [PATCH] city_map_size fix and idea

["Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>]

You are correct, there needs to be a better way for the longterm.

Currently, tile_is_known() normalizes, but if this is removed the 
code below breaks. And the rule is supposed to be you always call
functions with normalized coordinates.

The fix actually makes sure that it *can* be normalized. Unwrappable
coordinates cannot, and break here, which was the reason for the core.

The iterator is cartesian_adjacent_iterate_dir() but the "dir" flavours
are still controversial, so aren't yet available.

But I wish you wouldn't pop out the switch hack every time though. 
Functional code flow as oppposed to data driven techniques is going
to proliferate a lot of stuff like this throughout the code that
if you ever *do* need to change is going to be another core cleanup. :-)

 ... and besides it hurts me everytime I see it!

Cheers, 
RossW
=====

At 04:26 PM 01/08/22 -0700, Trent Piepho wrote:
>On Wed, 22 Aug 2001, Ross W. Wetmore wrote:
>> +    tileno = INDEX_NSEW(
>> +      (is_real_tile(abs_x0, abs_y0-1)
>> +        && (tile_is_known(abs_x0, abs_y0-1)==TILE_UNKNOWN)),
>> +      (is_real_tile(abs_x0, abs_y0+1)
>> +        && (tile_is_known(abs_x0, abs_y0+1)==TILE_UNKNOWN)),
>> +      (is_real_tile(abs_x0+1, abs_y0)
>> +        && (tile_is_known(abs_x0+1, abs_y0)==TILE_UNKNOWN)),
>> +      (is_real_tile(abs_x0-1, abs_y0)
>> +        && (tile_is_known(abs_x0-1, abs_y0)==TILE_UNKNOWN)));
>
>It looks like your change doesn't normalize the coordinates.  Isn't there
some
>sort of cardinal map iterator function that could be used here? 
>
>Something sort of like this...
>
>  int x,y,dir, n=0,s=0,e=0,w=0;
>
>  /* Find which of the cardinal directions are unknown */
>  adj_card_dir_iterate(abs_x0, abs_y0, x, y, dir) {
>    if(tile_is_known(x,y)==TILE_UNKNOWN) {
>      switch(dir) {
>        case DIR8_NORTH:  n=1; break;
>        case DIR8_SOUTH:  s=1; break;
>        case DIR8_EAST:   e=1; break;
>        case DIR8_WEST:   w=1; break;
>      }
>    }
>  } adj_card_dir_iterate_end;
>  tileno = INDEX_NSEW(n,s,e,w);
>
>It make look a little more complex, but I think once you make the previous
way
>normalize coordinates, it will be simpler.  Also keep in mind that the
iterate
>macro is smart, and if the center tile isn't on the map boundary, it avoids
>the check for is_real_tile() and doesn't normalize the coordinates.
>
>I don't know if adj_card_dir_iterate exists, but it should!  If it doesn't..
>
>adj_dir_iterate(...) {
>  if(!IS_CARDINAL(dir)) continue;
>
>Does the same thing, but is less efficient, since the iterator macro will
>still generate and normalize the non-cardinal points we aren't interested in.
>I'm sure you remember, that if the directions use the DIR_DX2 order that
>a cardinal iterate can be done by incrementing dir by 2 each iteration.