[Freeciv-Dev] Re: PATCH: map iteration (PR#1018)
[Top] [All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
jdorje@xxxxxxxxxxxxxxxxxxxxx wrote:
>
> There are a lot of places in the code that have loops like
>
> for (y=0; y<map.ysize; y++) {
> for (x=0; x<map.xsize; x++) {
> ...
> }
> ...
> }
>
> These loops will generally not work under a general topology, since they
> are intended to loop over only normal, real positions and for many
> topologies that will not happen.
>
> The easy solution is to do an is_normal_map_pos check, as in:
>
> for (y=0; y<map.ysize; y++) {
> for (x=0; x<map.xsize; x++) {
> if (is_normal_map_pos(x, y)) {
> ...
> } else {
> <possibly some extra handling>
> }
> }
> ...
> }
>
> The cleanest example of this is in whole_map_iterate, where the
> is_normal_map_pos check can just be added directly.
So, now back to this patch...
Questions:
1. Ross, do you see the necessity of these changes yet?
2. Are there any more loops that can be replaced by iterator macros?
Such replacement is IMO the best solution possible. But, I don't see
how to convert any more of them.
3. Is the debugging function print_landarea_map() in game.c (part of
the LAND_AREA_DEBUG group) necessary? If no consensus can be reached, I
suppose it must be left in.
4. Is regular_map_pos_is_normal good, or should we stick to
is_normal_map_pos? Again, regular_map_pos_is_normal provides identical
functionality but assumes the map position it is given is "regular",
i.e. within (0..map.xsize-1,0..map.ysize-1). This makes it much faster.
5. For loops that cannot use any existing iterator macros, do we want
to create new macros y_map_iterate and yx_map_iterate? I am
indifferent.
6. I have not touched any mapgen stuff, although some of the loops
there could easily use whole_map_iterate. Is this desired?
7. I have also not touched Overview_FillBuffer in gui-mui. I don't
know what the correct fix there might be.
Once these issues are resolved, we should be able to think about closing
this.
jason? rc
? topology-current
? old
? topology
? tilespec_patch
? corecleanup_08.ReadMe
? civserver-regular
Index: common/game.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/game.c,v
retrieving revision 1.112
diff -u -r1.112 game.c
--- common/game.c 2001/10/26 07:33:23 1.112
+++ common/game.c 2001/10/30 13:15:02
@@ -104,12 +104,32 @@
return (((when >= 0) && (when < sizeof (list))) ? list[when] : '?');
}
+/* Writes the map_char_expr expression for each position on the map.
+ map_char_expr is provided with the variables x,y to evaluate the
+ position. The 'type' arguemnt is used for formatting by printf;
+ for instance it should be "%c" for characters. */
+#define WRITE_MAP_DATA(type, map_char_expr) \
+{ \
+ int x, y; \
+ for (x=0; x<map.xsize; x++) { \
+ printf("%d", x % 10); \
+ } \
+ putchar('\n'); \
+ for (y=0; y<map.ysize; y++) { \
+ printf("%d ", y % 10); \
+ for (x=0; x<map.xsize; x++) { \
+ printf(type, regular_map_pos_is_normal(x, y) ? (map_char_expr) : ' '); \
+ } \
+ printf(" %d\n", y % 10); \
+ } \
+}
+
/**************************************************************************
...
**************************************************************************/
static void print_landarea_map(struct claim_map *pcmap, int turn)
{
- int x, y, p;
+ int p;
if (turn == 0) {
putchar ('\n');
@@ -122,78 +142,22 @@
for (p = 0; p < game.nplayers; p++)
{
printf (".know (%d)\n ", p);
- for (x = 0; x < map.xsize; x++)
- {
- printf ("%d", x % 10);
- }
- putchar ('\n');
- for (y = 0; y < map.ysize; y++)
- {
- printf ("%d ", y % 10);
- for (x = 0; x < map.xsize; x++)
- {
- printf ("%c",
- (pcmap->claims[map_inx(x,y)].know & (1u << p)) ?
- 'X' :
- '-');
- }
- printf (" %d\n", y % 10);
- }
+ WRITE_MAP_DATA("%c", pcmap->claims[map_inx(x,y)].know & (1u << p) ?
+ 'X' : '-');
printf (".cities (%d)\n ", p);
- for (x = 0; x < map.xsize; x++)
- {
- printf ("%d", x % 10);
- }
- putchar ('\n');
- for (y = 0; y < map.ysize; y++)
- {
- printf ("%d ", y % 10);
- for (x = 0; x < map.xsize; x++)
- {
- printf ("%c",
- (pcmap->claims[map_inx(x,y)].cities & (1u << p)) ?
- 'O' :
- '-');
- }
- printf (" %d\n", y % 10);
- }
+ WRITE_MAP_DATA("%c", pcmap->claims[map_inx(x,y)].cities & (1u << p) ?
+ 'O' : '-');
}
}
printf ("Turn %d (%c)...\n", turn, when_char (turn));
printf (".whom\n ");
- for (x = 0; x < map.xsize; x++)
- {
- printf ("%d", x % 10);
- }
- putchar ('\n');
- for (y = 0; y < map.ysize; y++)
- {
- printf ("%d ", y % 10);
- for (x = 0; x < map.xsize; x++)
- {
- printf ("%X", pcmap->claims[map_inx(x,y)].whom);
- }
- printf (" %d\n", y % 10);
- }
+ WRITE_MAP_DATA("%X", pcmap->claims[map_inx(x,y)].whom);
printf (".when\n ");
- for (x = 0; x < map.xsize; x++)
- {
- printf ("%d", x % 10);
- }
- putchar ('\n');
- for (y = 0; y < map.ysize; y++)
- {
- printf ("%d ", y % 10);
- for (x = 0; x < map.xsize; x++)
- {
- printf ("%c", when_char (pcmap->claims[map_inx(x,y)].when));
- }
- printf (" %d\n", y % 10);
- }
+ WRITE_MAP_DATA("%c", when_char(pcmap->claims[map_inx(x,y)].when));
}
#endif
Index: common/map.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.h,v
retrieving revision 1.101
diff -u -r1.101 map.h
--- common/map.h 2001/10/30 10:59:20 1.101
+++ common/map.h 2001/10/30 13:15:02
@@ -274,6 +274,11 @@
enum known_type tile_is_known(int x, int y);
int is_real_tile(int x, int y);
int is_normal_map_pos(int x, int y);
+/* Determines whether the position is normal given that it's in
+ the range 0<=x<map.xsize and 0<=y<map.ysize. It's primarily a
+ faster version of is_normal_map_pos since such checks are pretty
+ common. */
+#define regular_map_pos_is_normal(x, y) (1)
/*
* A "border position" is any one that has adjacent positions that are
* not normal/proper.
@@ -473,9 +478,11 @@
{ \
int WMI_x_itr, WMI_y_itr; \
for (WMI_y_itr = 0; WMI_y_itr < map.ysize; WMI_y_itr++) \
- for (WMI_x_itr = 0; WMI_x_itr < map.xsize; WMI_x_itr++)
+ for (WMI_x_itr = 0; WMI_x_itr < map.xsize; WMI_x_itr++) \
+ if (regular_map_pos_is_normal(WMI_x_itr, WMI_y_itr)) {
#define whole_map_iterate_end \
+ } \
}
/*
Index: server/barbarian.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/barbarian.c,v
retrieving revision 1.36
diff -u -r1.36 barbarian.c
--- server/barbarian.c 2001/10/30 10:59:20 1.36
+++ server/barbarian.c 2001/10/30 13:15:03
@@ -81,7 +81,7 @@
{
int newplayer = game.nplayers;
struct player *barbarians;
- int i, j, k;
+ int i;
for( i = 0; i < game.nplayers; i++ ) {
barbarians = &game.players[i];
@@ -93,9 +93,9 @@
pick_ai_player_name( game.nation_count-1, barbarians->name);
sz_strlcpy(barbarians->username, barbarians->name);
/* I need to make them to forget the map, I think */
- for( j = 0; j < map.xsize; j++ )
- for( k = 0; k < map.ysize; k++ )
- map_clear_known( j, k, barbarians);
+ whole_map_iterate(x, y) {
+ map_clear_known(x, y, barbarians);
+ } whole_map_iterate_end;
}
barbarians->economic.gold += 100; /* New leader, new money */
return barbarians;
Index: server/gamelog.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/gamelog.c,v
retrieving revision 1.14
diff -u -r1.14 gamelog.c
--- server/gamelog.c 2001/01/13 13:43:38 1.14
+++ server/gamelog.c 2001/10/30 13:15:03
@@ -91,7 +91,11 @@
for (y=0;y<map.ysize;y++) {
for (x=0;x<map.xsize;x++) {
+ if (regular_map_pos_is_normal(x, y)) {
hline[x] = (map_get_terrain(x,y)==T_OCEAN) ? ' ' : '.';
+ } else {
+ hline[x] = '#';
+ }
}
gamelog(GAMELOG_MAP,"%s",hline);
}
Index: server/maphand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/maphand.c,v
retrieving revision 1.88
diff -u -r1.88 maphand.c
--- server/maphand.c 2001/10/30 10:59:21 1.88
+++ server/maphand.c 2001/10/30 13:15:04
@@ -275,7 +275,7 @@
**************************************************************************/
void send_all_known_tiles(struct conn_list *dest)
{
- int y, x;
+ int tiles_sent;
if (dest==NULL) dest = &game.game_connections;
@@ -285,10 +285,19 @@
}
} conn_list_iterate_end;
- /* send whole map row by row to each player to balance the
+ /* send whole map piece by piece to each player to balance the
load of the send buffers better */
- for (y=0; y<map.ysize; y++) {
- conn_list_do_buffer(dest);
+ tiles_sent = 0;
+ conn_list_do_buffer(dest);
+
+ whole_map_iterate(x, y) {
+ tiles_sent++;
+ if (tiles_sent % map.xsize) {
+ conn_list_do_unbuffer(dest);
+ flush_packets();
+ conn_list_do_buffer(dest);
+ }
+
conn_list_iterate(*dest, pconn) {
struct player *pplayer = pconn->player;
@@ -297,21 +306,18 @@
}
if (pplayer) {
- for (x=0; x<map.xsize; x++) {
- if (map_get_known(x, y, pplayer)) {
+ if (map_get_known(x, y, pplayer)) {
send_NODRAW_tiles(pplayer, &pconn->self, x, y, 0);
send_tile_info_always(pplayer, &pconn->self, x, y);
- }
}
} else {
- for (x=0; x<map.xsize; x++) {
- send_tile_info_always(pplayer, &pconn->self, x, y);
- }
+ send_tile_info_always(pplayer, &pconn->self, x, y);
}
} conn_list_iterate_end;
- conn_list_do_unbuffer(dest);
- flush_packets();
- }
+ } whole_map_iterate_end;
+
+ conn_list_do_unbuffer(dest);
+ flush_packets();
}
/**************************************************************************
Index: server/savegame.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/savegame.c,v
retrieving revision 1.37
diff -u -r1.37 savegame.c
--- server/savegame.c 2001/10/26 07:33:24 1.37
+++ server/savegame.c 2001/10/30 13:15:05
@@ -68,7 +68,7 @@
\
for (y = 0; y < map.ysize; y++) { \
for (x = 0; x < map.xsize; x++) { \
- if (is_normal_map_pos(x, y)) { \
+ if (regular_map_pos_is_normal(x, y)) { \
line[x] = get_xy_char; \
if(!isprint(line[x] & 0x7f)) { \
freelog(LOG_FATAL, "Trying to write invalid" \
@@ -127,7 +127,7 @@
} \
for(x = 0; x < map.xsize; x++) { \
char ch = line[x]; \
- if (is_normal_map_pos(x, y)) { \
+ if (regular_map_pos_is_normal(x, y)) { \
set_xy_char; \
} else { \
assert(ch == '#'); \
- [Freeciv-Dev] Re: PATCH: map iteration (PR#1018),
jdorje <=
|
|
