[Freeciv-Dev] [PATCH] directional code cleanup: patch #1
[Top] [All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
In the interests of having small patches (whether that be a good or a
bad thing), I offer the following patch. It is the first in a series of
cleanups for the directional code/iterator macros, not quite redundant
with the "core code" cleanups. After this is integrated (or before if
they're independent), I'll send others along.
This patch removes the magic numbers and expressions associated with the
DIR_D[XY] macros, collecting all such implementation-dependent code into
map.[ch].
Additions to map.[ch]:
- macros dir_reverse(dir) and dir_is_cardinal(dir) are created
- an enumeration is created for the 8 directions
- the directional names used for logging ("N") are defined globally
Changes throughout the code:
- (7-dir) is replaced by dir_reverse(dir)
- 128>>dir is replaced by 1<<dir_reverse(dir)
- d[dir][1] is replaced by !dir_is_cardinal(dir)
- "2" is replaced by DIR8_NORTHEAST, etc.
I hope that I've found all of the "magic" bits of code here, but I can't
be sure.
Advantages:
- Lower code complexity.
- Much easier to change the directional system.
Disadvantages:
- Slightly more code overall.
- Insignificant performance hit.
The chance of bugs should be low.
The patch is made against current CVS (Aug. 15th).
jason shortIndex: 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 04:50:53
@@ -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_NAMES[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_NAMES[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_NAMES[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 04:50:56
@@ -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.56
diff -u -r1.56 mapclass.c
--- client/gui-mui/mapclass.c 2001/08/05 14:44:58 1.56
+++ client/gui-mui/mapclass.c 2001/08/15 04:50:58
@@ -1283,8 +1283,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);
}
@@ -1336,14 +1338,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 04:51:00
@@ -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 04:51:01
@@ -44,6 +44,7 @@
/* used to compute neighboring tiles */
const int DIR_DX[8] = { -1, 0, 1, -1, 1, -1, 0, 1 };
const int DIR_DY[8] = { -1, -1, -1, 0, 0, 1, 1, 1 };
+const char *DIR_NAMES[8] = { "NW", "N", "NE", "W", "E", "SW", "S", "SE" };
/* like DIR_DX[] and DIR_DY[], only cartesian */
const int CAR_DIR_DX[4] = {1, 0, -1, 0};
@@ -1058,8 +1059,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;
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 04:51:01
@@ -429,8 +429,26 @@
|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
+};
+
+#define dir_reverse(dir) (7 - (dir))
+#define dir_is_cardinal(dir) \
+ ((dir) == DIR8_NORTH || (dir) == DIR8_EAST || \
+ (dir) == DIR8_WEST || (dir) == DIR8_SOUTH)
+
extern const int DIR_DX[8];
extern const int DIR_DY[8];
+extern const char *DIR_NAMES[8]; /* short names, used for debugging */
/* like DIR_DX[] and DIR_DY[], only cartesian */
extern const int CAR_DIR_DX[4];
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 04:51:04
@@ -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;
@@ -679,7 +678,8 @@
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 +755,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_NAMES[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_NAMES[dir], x, y, x1, y1, total_cost);
}
}
break;
@@ -822,15 +822,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_NAMES[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_NAMES[dir], x, y, x1, y1, total_cost);
}
}
break;
@@ -865,15 +865,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_NAMES[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_NAMES[dir], x, y, x1, y1, total_cost);
}
}
break;
@@ -912,7 +912,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 +923,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_NAMES[dir_reverse(dir)], x1, y1, x, y);
}
}
}
@@ -950,7 +950,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 +964,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 +973,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 +1067,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 +1129,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 +1188,7 @@
return;
}
- freelog(LOG_DEBUG, "Going %s", d[dir]);
+ freelog(LOG_DEBUG, "Going %s", DIR_NAMES[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 <=
- [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, 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, 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
|
|
