[Freeciv-Dev] Re: [PATCH] Corecleanup_07Part2 has been put in incoming

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Sun, Aug 26, 2001 at 03:17:37PM -0400, Ross W. Wetmore wrote:
> Attached is the ReadMe for second Part of the corecleanup_07 update to 
> cvs-Aug-25. 

I finally found time to look at part2.
 - corecleanup_07f isn't in unified diff
 - corecleanup_07f doesn't apply cleanly
 - there should be no *_x and *_y version just one which takes both
 coordinates
 - map_trunc isn't currently needed and should have another name
 (find_nearest_real_tile for example)
 - there should be no *wrap* methods (encapsulation of the topology)
 - (unsigned)(POS) < (unsigned)(BASE) should be expressed as a macro
 - map_inx should be map_index (or remove it completely, map_get_tile
 should handle most of the cases)
 - what is the reason for this:
-      put_line(map_canvas_store, dest_x, dest_y, DIR_REVERSE(dir));
+      put_line(map_canvas_store, dest_x, dest_y, 7-dir);

  Everybody keeps telling me the gui uses another system, but which
  one? Either make a GUI_DIR_REVERSE(dir) OR leave it and change the
  rest of the code to use the system of the gui OR tell me the reason.
 - +#define FC__CITY_C     1        /* pickup city_map_indices */
   +#include "city.h"
   This is ugly. Why?
 - -  if (city_x < 0 || city_y < 0
   -      || city_x >= CITY_MAP_SIZE
   -      || city_y >= CITY_MAP_SIZE)
   +  if (((unsigned)city_x) >= CITY_MAP_SIZE
   +   || ((unsigned)city_y) >= CITY_MAP_SIZE)
   Please leave it so or make a macro. We discussed RANGE_CHECK.
 - What is is_border2_tile? Why?

Let me state my position/plan:
 - make a is_normal(ized)_position
 - insert an "assert(is_normal(ized)_position);" into every access
 method of map.h
 - debug

 - remove map_adjust_x and map_adjust_y
 - debug

 - come to an agreement what the final direction system should be
 - migrate
 - debug

I'm won't do anything before I heard Gautes position/comments on your
patch. Jason?

> Note, this is a request for inclusion of these changes into CVS, as opposed 
> to just a RFC.

I'm sorry but in its current form it is unacceptable for me.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "We've all heard that a million monkeys banging on a million typewriters
  will eventually reproduce the entire works of Shakespeare.
  Now, thanks to the Internet, we know this is not true."