[Freeciv-Dev] Re: PATCH: remove map_adjust_[xy] invocations from server (PR#989)

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Fri, Oct 05, 2001 at 01:43:57AM -0700, jdorje@xxxxxxxxxxxxxxxxxxxxx wrote:
> Here is a correction to the previous patch.
> 
> Changes include:
> 
> - The client code was cut out; that wasn't supposed to be there anyway.
> 
> - A redundante is_real_tile check was removed.  In this case I have now
> simply replaced a logging-of-error with a simple assertion.  It's
> cleaner but slightly less safe in non-debug mode.
> 
> Unless I'm badly mistaken this should be ready for inclusion.
> 

>  /**************************************************************************
> +Returns TRUE iff the map position is normal.  "Normal" here means that it
> +is both a real/valid coordinate set and that the coordinates are in their
> +canonical/proper form.  In plain English: the coordinates must be on the
> +map.
> +**************************************************************************/
> +int is_normal_map_pos(int x, int y)
> +{
> +  return 0 <= y && y < map.ysize && 0 <= x && x < map.xsize;

This should be expresed in a way of normalize_map_pos. Or the other
way round. We may avoid the function call if is_normal_map_pos is a
macro. And with the RANGE_CHECK_0 from Ross there would also be no
double evaluation.

>  /**************************************************************************
> -  smooth_map should be viewed  as a  corrosion function on the map, it levels
> -  out the differences in the heightmap.
> +  smooth_map should be viewed  as a corrosion function on the map, it
> +  levels out the differences in the heightmap.
>  **************************************************************************/
>  static void smooth_map(void)
>  {
> -  int mx,my,px,py;
> -  int a;
> -  
> +  /* This overwrites itself as it runs, creating an unpredictable
> +     feedback that depends on the ordering of whole_map_iterate.  Is
> +     this desired?  --JDS */

Mhhh since this obvious wrong: either iterate randomly or save the
value in another array.

> +  assert(is_real_tile(x, y));
> +  normalize_map_pos(&x, &y);

is_real_tile is unnecessary.

> +  assert(is_real_tile(x, y));
> +  normalize_map_pos(&x, &y);

Same.

> +  assert(is_real_tile(x, y));
> +  normalize_map_pos(&x, &y); /* Should be handled by caller.  --JDS */

Same.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "I was dead ... but I'm better now."
    -- Capitain Sheridan in Babylon 5