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

[Jason Dorje Short <jshort@xxxxxxxxxxxxx>]

Raimar Falke wrote:
> 
> 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.

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

Ross's patch has a greater scope than what you and I are considering;
however it handles things differently than I would.

As to the general issues:

DIR_REVERSE has been replaced by 7-dir because Ross plans to change all
of the server code to use the rotational directional system, while
temporarily leaving the GUI using the vertical system (with magic
numbers).  I believe the better solution is to have all code continue to
use the vertical system (with no magic numbers) until all dependency
issues are resolved; however, I believe some of his applied patches have
already changed some DIR_REVERSE usages back to be 7-dir (for instance
in client/gui-*/mapview.c).  I feel strongly that this is not the
correct way to handle the problem; it makes fixing the GUI later even
harder.

I will state again that non-real map positions should not be wrapped. 
Imagine the situation of a hexagon that wraps along the sides only; for
this simple topology many non-real tiles will have no canonical wrapping
position:
<-     x     ->
<-   x x x   ->
<- x x x x x ->
<- x x x x x ->
<- x x x x x ->
<-   x x x   ->
<-     x     ->
(Note that this is different from Trent's proposed "sphere" topology.)

I believe he also introduces another function to determine the nearest
normal position to any non-real map position.  Such a function will be
necessary if the GUI is to continue it's current behavior (which is not
strictly necessary IMO).  Until this function is in use (or the GUI
behavior is changed) there will be a need for map_adjust_[xy].  Also,
the current GUI method of wrapping will not work cleanly with
normalize_map_pos.

I agree with you that is_normal_map_pos is a needed function (although I
believe there may still be disagreement about the name.  Gaute?). 
Attached is another patch which implements (but does not yet use) this
function.

In short, I agree with your "plan", Raimar.  However, I don't think any
agreement on what the final directional system should be is really
needed just yet - we can all agree that code should be written
independently of the directional system, and that should be the
immediate goal.  Note that some of the patches you've applied have taken
an opposite approach to this plan.

jason

Index: common/map.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.c,v
retrieving revision 1.82
diff -u -r1.82 map.c
--- common/map.c        2001/08/26 09:28:16     1.82
+++ common/map.c        2001/08/28 17:58:00
@@ -1298,6 +1298,17 @@
   return 0 <= y && y < map.ysize;
 }
 
+/***************************************************************
+Returns TRUE iff (x, y) is a "normal" map position.
+Normal[ized] map positions are defined to be canonically wrapped
+real tiles.
+***************************************************************/
+int is_normal_map_pos(int x, int y)
+{
+  return 0 <= x && x < map.xsize &&
+         0 <= y && y < map.ysize;
+}
+
 /**************************************************************************
 Normalizes the map position. Returns TRUE if it is valid, false otherwise.
 **************************************************************************/
Index: common/map.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.h,v
retrieving revision 1.87
diff -u -r1.87 map.h
--- common/map.h        2001/08/26 09:28:17     1.87
+++ common/map.h        2001/08/28 17:58:00
@@ -215,6 +215,7 @@
 enum known_type tile_is_known(int x, int y);
 int check_coords(int *x, int *y);
 int is_real_tile(int x, int y);
+int is_normal_map_pos(int x, int y);
 int normalize_map_pos(int *x, int *y);
 void rand_neighbour(int x0, int y0, int *x, int *y);