[Freeciv-Dev] Re: [PATCH] directional code cleanup: patch #1
[Top] [All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
Raimar Falke wrote:
> On Wed, Aug 15, 2001 at 11:58:09AM -0400, Jason Dorje Short wrote:
> > Raimar Falke wrote:
> > > On Wed, Aug 15, 2001 at 03:28:26AM -0400, Jason Dorje Short wrote:
> > > > Trent Piepho wrote:
> > > > > Since these are macros, convention is to give them names in all caps.
> > > >
> > > > They are macros, but act just like functions (and could be made so in
> > > > the future).
> > >
> > > I would like to second Trent here. There are macros and this property
> > > should be visible by the name. You may also define both: a macro and a
> > > function.
> >
> > How come this convention isn't held by, say, the iteration macros?
>
> The code isn't perfect. If you want to do it... At least for new code
> this schema can be adopted.
OK...if this is convention, I will certainly follow it. That the
iteration macros (and others IIRC) don't follow it may indicate that not
everyone agrees with the convention, though.
> > Sorry to be argumentative here, but this seems counter-intuitive to me -
> > and also makes the code look uglier IMO, which you cite as a reason for
> > another change.
> >
> > As for the names being too long, DIR_REVERSE seems to be much less
> > confusing than DIR_REV.
>
> An extra comment would be helpfull. You may also say what makes a
> cardinal direction so special.
Done.
> Can you change
> char* dir_get_name(int dir);
> to
> const char* dir_get_name(int dir);
Yes - that is clearly the correct way to do it.
Third (possibly final) patch attached.
jason Index: client/goto.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/goto.c,v
retrieving revision 1.16
diff -u -r1.16 goto.c
--- client/goto.c 2001/08/14 11:04:19 1.16
+++ client/goto.c 2001/08/15 17:22:56
@@ -306,7 +306,6 @@
static void create_goto_map(struct unit *punit, int src_x, int src_y,
enum goto_move_restriction restriction)
{
- static const char *d[] = { "NW", "N", "NE", "W", "E", "SW", "S", "SE" };
int x, y, x1, y1, dir;
struct tile *psrctile, *pdesttile;
enum unit_move_type move_type = unit_types[punit->type].move_type;
@@ -325,8 +324,8 @@
/* Try to move to all tiles adjacent to x,y. The coordinats of the tile we
try to move to are x1,y1 */
for (dir = 0; dir < 8; dir++) {
- /* if the direction is N, S, E or W d[dir][1] is the /0 character... */
- if ((restriction == GOTO_MOVE_CARDINAL_ONLY) && d[dir][1]) continue;
+ if ((restriction == GOTO_MOVE_CARDINAL_ONLY) && !DIR_IS_CARDINAL(dir))
+ continue;
x1 = x + DIR_DX[dir];
y1 = y + DIR_DY[dir];
@@ -385,10 +384,10 @@
goto_map.move_cost[x1][y1] = total_cost;
if (add_to_queue)
add_to_mapqueue(total_cost, x1, y1);
- goto_map.vector[x1][y1] = 128>>dir;
+ goto_map.vector[x1][y1] = 1 << DIR_REVERSE(dir);
freelog(LOG_DEBUG,
"Candidate: %s from (%d, %d) to (%d, %d), cost %d",
- d[dir], x, y, x1, y1, total_cost);
+ dir_get_name(dir), x, y, x1, y1, total_cost);
}
break;
@@ -417,10 +416,10 @@
goto_map.move_cost[x1][y1] = total_cost;
if (add_to_queue)
add_to_mapqueue(total_cost, x1, y1);
- goto_map.vector[x1][y1] = 128>>dir;
+ goto_map.vector[x1][y1] = 1 << DIR_REVERSE(dir);
freelog(LOG_DEBUG,
"Candidate: %s from (%d, %d) to (%d, %d), cost %d",
- d[dir], x, y, x1, y1, total_cost);
+ dir_get_name(dir), x, y, x1, y1, total_cost);
}
break;
@@ -443,10 +442,10 @@
goto_map.move_cost[x1][y1] = total_cost;
if (add_to_queue)
add_to_mapqueue(total_cost, x1, y1);
- goto_map.vector[x1][y1] = 128>>dir;
+ goto_map.vector[x1][y1] = 1 << DIR_REVERSE(dir);
freelog(LOG_DEBUG,
"Candidate: %s from (%d, %d) to (%d, %d), cost %d",
- d[dir], x, y, x1, y1, total_cost);
+ dir_get_name(dir), x, y, x1, y1, total_cost);
}
break;
@@ -644,7 +643,7 @@
if (dir >= 4) {
x = x1;
y = y1;
- dir = 7 - dir;
+ dir = DIR_REVERSE(dir);
}
return goto_map.drawn[x] + y*4 + dir;
Index: client/gui-gtk/mapview.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/mapview.c,v
retrieving revision 1.87
diff -u -r1.87 mapview.c
--- client/gui-gtk/mapview.c 2001/08/13 21:17:53 1.87
+++ client/gui-gtk/mapview.c 2001/08/15 17:22:59
@@ -2012,8 +2012,8 @@
put_line(map_canvas->window, src_x, src_y, dir);
}
if (tile_visible_mapcanvas(dest_x, dest_y)) {
- put_line(map_canvas_store, dest_x, dest_y, 7-dir);
- put_line(map_canvas->window, dest_x, dest_y, 7-dir);
+ put_line(map_canvas_store, dest_x, dest_y, DIR_REVERSE(dir));
+ put_line(map_canvas->window, dest_x, dest_y, DIR_REVERSE(dir));
}
increment_drawn(src_x, src_y, dir);
@@ -2062,14 +2062,15 @@
normalize_map_pos(&dest_x, &dest_y);
refresh_tile_mapcanvas(dest_x, dest_y, 1);
if (NORMAL_TILE_WIDTH%2 == 0 || NORMAL_TILE_HEIGHT%2 == 0) {
- if (dir == 2) { /* Since the tle doesn't have a middle we draw an extra
pixel
- on the adjacent tile when drawing in this direction. */
+ if (dir == DIR8_NORTHEAST) {
+ /* Since the tile doesn't have a middle we draw an extra pixel
+ on the adjacent tile when drawing in this direction. */
dest_x = src_x + 1;
dest_y = src_y;
assert(is_real_tile(dest_x, dest_y));
normalize_map_pos(&dest_x, &dest_y);
refresh_tile_mapcanvas(dest_x, dest_y, 1);
- } else if (dir == 5) { /* the same */
+ } else if (dir == DIR8_SOUTHWEST) { /* the same */
dest_x = src_x;
dest_y = src_y + 1;
assert(is_real_tile(dest_x, dest_y));
Index: client/gui-mui/mapclass.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-mui/mapclass.c,v
retrieving revision 1.57
diff -u -r1.57 mapclass.c
--- client/gui-mui/mapclass.c 2001/08/15 16:02:21 1.57
+++ client/gui-mui/mapclass.c 2001/08/15 17:23:02
@@ -1286,8 +1286,10 @@
put_line(_rp(o), _mleft(o),_mtop(o),src_x, src_y, dir);
}
if (tile_visible_mapcanvas(dest_x, dest_y)) {
- put_line(data->map_layer->rp, 0,0,dest_x, dest_y, 7-dir);
- put_line(_rp(o), _mleft(o),_mtop(o),dest_x, dest_y, 7-dir);
+ put_line(data->map_layer->rp, 0, 0,
+ dest_x, dest_y, DIR_REVERSE(dir));
+ put_line(_rp(o), _mleft(o),_mtop(o),
+ dest_x, dest_y, DIR_REVERSE(dir));
}
increment_drawn(src_x, src_y, dir);
}
@@ -1339,14 +1341,15 @@
normalize_map_pos(&dest_x, &dest_y);
refresh_tile_mapcanvas(dest_x, dest_y, 1); /* !! */
if (NORMAL_TILE_WIDTH%2 == 0 || NORMAL_TILE_HEIGHT%2 == 0) {
- if (dir == 2) { /* Since the tile doesn't have a middle we draw
an extra pixel
- on the adjacent tile when drawing in this
direction. */
+ if (dir == DIR8_NORTHEAST) {
+ /* Since the tile doesn't have a middle we draw an extra pixel
+ on the adjacent tile when drawing in this direction. */
dest_x = src_x + 1;
dest_y = src_y;
assert(is_real_tile(dest_x, dest_y));
normalize_map_pos(&dest_x, &dest_y);
refresh_tile_mapcanvas(dest_x, dest_y, 1); /* !! */
- } else if (dir == 5) { /* the same */
+ } else if (dir == DIR8_SOUTHWEST) { /* the same */
dest_x = src_x;
dest_y = src_y + 1;
assert(is_real_tile(dest_x, dest_y));
Index: client/gui-xaw/mapview.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-xaw/mapview.c,v
retrieving revision 1.73
diff -u -r1.73 mapview.c
--- client/gui-xaw/mapview.c 2001/08/14 07:21:57 1.73
+++ client/gui-xaw/mapview.c 2001/08/15 17:23:03
@@ -1329,8 +1329,8 @@
}
if (tile_visible_mapcanvas(dest_x, dest_y)) {
- put_line(map_canvas_store, dest_x, dest_y, 7-dir);
- put_line(XtWindow(map_canvas), dest_x, dest_y, 7-dir);
+ put_line(map_canvas_store, dest_x, dest_y, DIR_REVERSE(dir));
+ put_line(XtWindow(map_canvas), dest_x, dest_y, DIR_REVERSE(dir));
}
increment_drawn(src_x, src_y, dir);
@@ -1359,14 +1359,15 @@
normalize_map_pos(&dest_x, &dest_y);
refresh_tile_mapcanvas(dest_x, dest_y, 1);
if (NORMAL_TILE_WIDTH%2 == 0 || NORMAL_TILE_HEIGHT%2 == 0) {
- if (dir == 2) { /* Since the tle doesn't have a middle we draw an extra
pixel
- on the adjacent tile when drawing in this direction. */
+ if (dir == DIR8_NORTHEAST) {
+ /* Since the tile doesn't have a middle we draw an extra pixel
+ on the adjacent tile when drawing in this direction. */
dest_x = src_x + 1;
dest_y = src_y;
assert(is_real_tile(dest_x, dest_y));
normalize_map_pos(&dest_x, &dest_y);
refresh_tile_mapcanvas(dest_x, dest_y, 1);
- } else if (dir == 5) { /* the same */
+ } else if (dir == DIR8_SOUTHWEST) { /* the same */
dest_x = src_x;
dest_y = src_y + 1;
assert(is_real_tile(dest_x, dest_y));
Index: common/map.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.c,v
retrieving revision 1.79
diff -u -r1.79 map.c
--- common/map.c 2001/08/14 14:31:19 1.79
+++ common/map.c 2001/08/15 17:23:04
@@ -1058,8 +1058,8 @@
tile0->move_cost[dir] = tile_move_cost_ai(tile0, tile1, x, y,
x1, y1, maxcost);
/* reverse: not at all obfuscated now --dwp */
- tile1->move_cost[7 - dir] = tile_move_cost_ai(tile1, tile0, x1, y1,
- x, y, maxcost);
+ tile1->move_cost[DIR_REVERSE(dir)] =
+ tile_move_cost_ai(tile1, tile0, x1, y1, x, y, maxcost);
} else {
/* trying to move off the screen. */
tile0->move_cost[dir] = maxcost;
@@ -1333,4 +1333,15 @@
assert(is_real_tile(*x, *y));
normalize_map_pos(x, y);
+}
+
+/**************************************************************************
+Return the debugging name of the direction.
+**************************************************************************/
+const char* dir_get_name(int dir)
+{
+ static const char *names[8] = { "NW", "N", "NE", "W",
+ "E", "SW", "S", "SE" };
+ assert(dir >= 0 && dir < 8);
+ return names[dir];
}
Index: common/map.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.h,v
retrieving revision 1.85
diff -u -r1.85 map.h
--- common/map.h 2001/08/14 14:31:19 1.85
+++ common/map.h 2001/08/15 17:23:05
@@ -429,6 +429,29 @@
|5|6|7|
-------
*/
+enum {
+ /* FIXME: DIR8 is used to avoid conflict with another enum */
+ DIR8_NORTHWEST = 0,
+ DIR8_NORTH = 1,
+ DIR8_NORTHEAST = 2,
+ DIR8_WEST = 3,
+ DIR8_EAST = 4,
+ DIR8_SOUTHWEST = 5,
+ DIR8_SOUTH = 6,
+ DIR8_SOUTHEAST = 7
+};
+
+/* return the reverse of the direction */
+#define DIR_REVERSE(dir) (7 - (dir))
+
+/* is the direction "cardinal"? Cardinal directions
+ * (also called cartesian) are the four main ones */
+#define DIR_IS_CARDINAL(dir) \
+ ((dir) == DIR8_NORTH || (dir) == DIR8_EAST || \
+ (dir) == DIR8_WEST || (dir) == DIR8_SOUTH)
+
+const char* dir_get_name(int dir);
+
extern const int DIR_DX[8];
extern const int DIR_DY[8];
Index: server/gotohand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/gotohand.c,v
retrieving revision 1.102
diff -u -r1.102 gotohand.c
--- server/gotohand.c 2001/08/14 11:04:22 1.102
+++ server/gotohand.c 2001/08/15 17:23:07
@@ -329,7 +329,7 @@
/* else c = ptile->move_cost[k];
This led to a bad bug where a unit in a swamp was considered too far
away */
else { /* we have a city */
- int tmp = map_get_tile(x1, y1)->move_cost[7-dir]; /* The move in the
opposite direction*/
+ int tmp = map_get_tile(x1, y1)->move_cost[DIR_REVERSE(dir)];
move_cost = (ptile->move_cost[dir] + tmp +
(ptile->move_cost[dir] > tmp ? 1 : 0))/2;
}
@@ -624,7 +624,6 @@
enum goto_move_restriction restriction)
{
struct player *pplayer = unit_owner(punit);
- static const char *d[] = { "NW", "N", "NE", "W", "E", "SW", "S", "SE" };
int igter, x, y, x1, y1, dir;
int orig_x, orig_y;
struct tile *psrctile, *pdesttile;
@@ -678,8 +677,8 @@
/* Try to move to all tiles adjacent to x,y. The coordinats of the tile we
try to move to are x1,y1 */
for (dir = 0; dir < 8; dir++) {
- /* if the direction is N, S, E or W d[dir][1] is the /0 character... */
- if ((restriction == GOTO_MOVE_CARDINAL_ONLY) && d[dir][1]) continue;
+ if ((restriction == GOTO_MOVE_CARDINAL_ONLY) && !DIR_IS_CARDINAL(dir))
+ continue;
x1 = x + DIR_DX[dir];
y1 = y + DIR_DY[dir];
@@ -755,15 +754,15 @@
if (warmap.cost[x1][y1] > total_cost) {
warmap.cost[x1][y1] = total_cost;
add_to_mapqueue(total_cost, x1, y1);
- local_vector[x1][y1] = 128>>dir;
+ local_vector[x1][y1] = 1 << DIR_REVERSE(dir);
freelog(LOG_DEBUG,
"Candidate: %s from (%d, %d) to (%d, %d), cost %d",
- d[dir], x, y, x1, y1, total_cost);
+ dir_get_name(dir), x, y, x1, y1, total_cost);
} else if (warmap.cost[x1][y1] == total_cost) {
- local_vector[x1][y1] |= 128>>dir;
+ local_vector[x1][y1] |= 1 << DIR_REVERSE(dir);
freelog(LOG_DEBUG,
"Co-Candidate: %s from (%d, %d) to (%d, %d), cost %d",
- d[dir], x, y, x1, y1, total_cost);
+ dir_get_name(dir), x, y, x1, y1, total_cost);
}
}
break;
@@ -822,15 +821,15 @@
if (warmap.seacost[x1][y1] > total_cost) {
warmap.seacost[x1][y1] = total_cost;
add_to_mapqueue(total_cost, x1, y1);
- local_vector[x1][y1] = 128>>dir;
+ local_vector[x1][y1] = 1 << DIR_REVERSE(dir);
freelog(LOG_DEBUG,
"Candidate: %s from (%d, %d) to (%d, %d), cost %d",
- d[dir], x, y, x1, y1, total_cost);
+ dir_get_name(dir), x, y, x1, y1, total_cost);
} else if (warmap.seacost[x1][y1] == total_cost) {
- local_vector[x1][y1] |= 128>>dir;
+ local_vector[x1][y1] |= 1 << DIR_REVERSE(dir);
freelog(LOG_DEBUG,
"Co-Candidate: %s from (%d, %d) to (%d, %d), cost %d",
- d[dir], x, y, x1, y1, total_cost);
+ dir_get_name(dir), x, y, x1, y1, total_cost);
}
}
break;
@@ -865,15 +864,15 @@
if (warmap.cost[x1][y1] > total_cost) {
warmap.cost[x1][y1] = total_cost;
add_to_mapqueue(total_cost, x1, y1);
- local_vector[x1][y1] = 128>>dir;
+ local_vector[x1][y1] = 1 << DIR_REVERSE(dir);
freelog(LOG_DEBUG,
"Candidate: %s from (%d, %d) to (%d, %d), cost %d",
- d[dir], x, y, x1, y1, total_cost);
+ dir_get_name(dir), x, y, x1, y1, total_cost);
} else if (warmap.cost[x1][y1] == total_cost) {
- local_vector[x1][y1] |= 128>>dir;
+ local_vector[x1][y1] |= 1 << DIR_REVERSE(dir);
freelog(LOG_DEBUG,
"Co-Candidate: %s from (%d, %d) to (%d, %d), cost %d",
- d[dir], x, y, x1, y1, total_cost);
+ dir_get_name(dir), x, y, x1, y1, total_cost);
}
}
break;
@@ -912,7 +911,7 @@
break;
for (dir = 0; dir < 8; dir++) {
- if ((restriction == GOTO_MOVE_CARDINAL_ONLY) && d[dir][1])
+ if ((restriction == GOTO_MOVE_CARDINAL_ONLY) && !DIR_IS_CARDINAL(dir))
continue;
if (local_vector[x][y] & (1<<dir)) {
@@ -923,10 +922,10 @@
move_cost = (move_type == SEA_MOVING) ? warmap.seacost[x1][y1] :
warmap.cost[x1][y1];
add_to_mapqueue(MAXCOST-1 - move_cost, x1, y1);
- warmap.vector[x1][y1] |= 128>>dir; /* Mark it on the warmap */
+ warmap.vector[x1][y1] |= 1 << DIR_REVERSE(dir); /* Mark it on the
warmap */
local_vector[x][y] -= 1<<dir; /* avoid repetition */
freelog(LOG_DEBUG, "PATH-SEGMENT: %s from (%d, %d) to (%d, %d)",
- d[7-dir], x1, y1, x, y);
+ dir_get_name(DIR_REVERSE(dir)), x1, y1, x, y);
}
}
}
@@ -950,7 +949,6 @@
const int dest_x, const int dest_y)
{
int k, d[8], x, y, n, a, best = 0, d0, d1, h0, h1, u, c;
- const char *dir[] = { "NW", "N", "NE", "W", "E", "SW", "S", "SE" };
struct tile *ptile, *adjtile;
int nearland;
struct city *pcity;
@@ -965,7 +963,7 @@
from going round in little circles to move across desirable squares */
for (k = 0; k < 8; k++) {
if (warmap.vector[punit->x][punit->y]&(1<<k)
- && !(restriction == GOTO_MOVE_CARDINAL_ONLY && dir[k][1])) {
+ && !(restriction == GOTO_MOVE_CARDINAL_ONLY && !DIR_IS_CARDINAL(k))) {
x = map_adjust_x(punit->x + DIR_DX[k]);
y = map_adjust_y(punit->y + DIR_DY[k]);
if (x == dest_x && y == dest_y)
@@ -974,7 +972,9 @@
}
for (k = 0; k < 8; k++) {
- if ((restriction == GOTO_MOVE_CARDINAL_ONLY) && dir[k][1]) continue;
+ if ((restriction == GOTO_MOVE_CARDINAL_ONLY) && !DIR_IS_CARDINAL(k))
+ continue;
+
if (!(warmap.vector[punit->x][punit->y]&(1<<k))) d[k] = 0;
else {
if (is_ground_unit(punit))
@@ -1066,7 +1066,7 @@
do {
do {
k = myrand(8);
- } while ((restriction == GOTO_MOVE_CARDINAL_ONLY) && dir[k][1]);
+ } while ((restriction == GOTO_MOVE_CARDINAL_ONLY) && !DIR_IS_CARDINAL(k));
} while (d[k] < best);
return(k);
}
@@ -1128,7 +1128,6 @@
{
struct player *pplayer = unit_owner(punit);
int x, y, dir;
- static const char *d[] = { "NW", "N", "NE", "W", "E", "SW", "S", "SE" };
int unit_id, dest_x, dest_y, waypoint_x, waypoint_y;
struct unit *penemy = NULL;
@@ -1188,7 +1187,7 @@
return;
}
- freelog(LOG_DEBUG, "Going %s", d[dir]);
+ freelog(LOG_DEBUG, "Going %s", dir_get_name(dir));
x = map_adjust_x(punit->x + DIR_DX[dir]);
y = punit->y + DIR_DY[dir]; /* no need to adjust this */
penemy = is_enemy_unit_tile(map_get_tile(x, y), punit->owner);
- [Freeciv-Dev] [PATCH] directional code cleanup: patch #1, Jason Dorje Short, 2001/08/15
- [Freeciv-Dev] Re: [PATCH] directional code cleanup: patch #1, Trent Piepho, 2001/08/15
- [Freeciv-Dev] Re: [PATCH] directional code cleanup: patch #1, Jason Dorje Short, 2001/08/15
- [Freeciv-Dev] Re: [PATCH] directional code cleanup: patch #1, Raimar Falke, 2001/08/15
- [Freeciv-Dev] Re: [PATCH] directional code cleanup: patch #1, Jason Dorje Short, 2001/08/15
- [Freeciv-Dev] Re: [PATCH] directional code cleanup: patch #1, Raimar Falke, 2001/08/15
- [Freeciv-Dev] Re: [PATCH] directional code cleanup: patch #1,
Jason Dorje Short <=
- [Freeciv-Dev] Re: [PATCH] directional code cleanup: patch #1, Trent Piepho, 2001/08/15
- [Freeciv-Dev] Re: [PATCH] directional code cleanup: patch #1, Raimar Falke, 2001/08/15
- [Freeciv-Dev] Re: [PATCH] directional code cleanup: patch #1, Jason Dorje Short, 2001/08/15
- [Freeciv-Dev] Re: [PATCH] directional code cleanup: patch #1, Raimar Falke, 2001/08/15
- [Freeciv-Dev] Re: [PATCH] directional code cleanup: patch #1, Gaute B Strokkenes, 2001/08/15
- [Freeciv-Dev] Re: [PATCH] directional code cleanup: patch #1, Gregory Berkolaiko, 2001/08/15
- [Freeciv-Dev] Re: [PATCH] directional code cleanup: patch #1, Raimar Falke, 2001/08/15
- [Freeciv-Dev] Re: [PATCH] directional code cleanup: patch #1, Trent Piepho, 2001/08/15
- [Freeciv-Dev] Re: [PATCH] directional code cleanup: patch #1, Gregory Berkolaiko, 2001/08/15
[Freeciv-Dev] Re: [PATCH] directional code cleanup: patch #1, Gregory Berkolaiko, 2001/08/15
|
|
