<URL: http://bugs.freeciv.org/Ticket/Display.html?id=40086 >
Define FREE_WORKED_TILES for counting is_free_worked_tile(), currently
only 1! Should generalize in rulesets?
Fix real_sanity_check_city() to prevent removing city workers for any
is_free_worked_tile(). Should consolidate with other functions!
Rewrite SANITY_* macros to standardize freelog() output format.
Don't assert() in game_remove_player(), use freelog() instead.
Committed trunk revision 14388.
Committed S2_2 revision 14389.
Index: server/sanitycheck.c
===================================================================
--- server/sanitycheck.c (revision 14387)
+++ server/sanitycheck.c (working copy)
@@ -39,37 +39,50 @@
#ifdef SANITY_CHECKING
-#define SANITY_CHECK(x)
\
+#define SANITY_(_s) \
+ freelog(LOG_ERROR, "Failed %s:%d at %s:%d! " _s, \
+ __FILE__,__LINE__, file, line
+
+#define SANITY_CHECK(check) \
do { \
- if (!(x)) {
\
- freelog(LOG_ERROR, "Failed %s:%d (%s:%d): %s", \
- __FILE__,__LINE__, file, line, #x); \
+ if (!(check)) { \
+ SANITY_("!(%s)"),
\
+ #check); \
} \
} while(0)
-#define SANITY_TILE(ptile, check) \
+#define SANITY_CITY(_city, check) \
do { \
if (!(check)) { \
- struct city *pcity = tile_city(ptile); \
- freelog(LOG_ERROR, "Failed %s:%d (%s:%d) at %s (%d,%d): %s", \
- __FILE__,__LINE__, file, line, \
- NULL != pcity ? city_name(pcity) \
- : terrain_rule_name(tile_terrain(ptile)), \
- TILE_XY(ptile), #check); \
+ SANITY_("(%4d,%4d) !(%s) in \"%s\"[%d]"),
\
+ TILE_XY((_city)->tile), \
+ #check, \
+ city_name(_city), \
+ (_city)->size); \
} \
} while(0)
-#define SANITY_CITY(pcity, check) \
+#define SANITY_TERRAIN(_tile, check) \
do { \
if (!(check)) { \
- freelog(LOG_ERROR, "Failed %s:%d (%s:%d) in %s[%d](%d,%d): %s", \
- __FILE__,__LINE__, file, line, \
- city_name(pcity), (pcity)->size, \
- TILE_XY((pcity)->tile), #check); \
+ SANITY_("(%4d,%4d) !(%s) at \"%s\""), \
+ TILE_XY(_tile), \
+ #check, \
+ terrain_rule_name(tile_terrain(_tile))); \
} \
} while(0)
+#define SANITY_TILE(_tile, check) \
+ do { \
+ struct city *_tile##_city = tile_city(_tile); \
+ if (NULL != _tile##_city) {
\
+ SANITY_CITY(_tile##_city, check);
\
+ } else { \
+ SANITY_TERRAIN(_tile, check); \
+ } \
+ } while(0)
+
/**************************************************************************
Sanity checking on map (tile) specials.
**************************************************************************/
@@ -168,6 +181,7 @@
CHECK_NATIVE_POS(ptile->nat_x, ptile->nat_y);
if (NULL != pcity) {
+ SANITY_TILE(ptile, same_pos(pcity->tile, ptile));
SANITY_TILE(ptile, tile_owner(ptile) != NULL);
}
if (tile_owner(ptile) != NULL) {
@@ -196,10 +210,6 @@
} adjc_iterate_end;
}
- if (pcity) {
- SANITY_TILE(ptile, same_pos(pcity->tile, ptile));
- }
-
unit_list_iterate(ptile->units, punit) {
SANITY_TILE(ptile, same_pos(punit->tile, ptile));
@@ -221,6 +231,7 @@
**************************************************************************/
void real_sanity_check_city(struct city *pcity, const char *file, int line)
{
+ int citizens, delta;
int workers = 0;
struct player *pplayer = city_owner(pcity);
@@ -246,74 +257,92 @@
/* Note that cities may be found on land or water. */
city_map_iterate(x, y) {
- struct tile *ptile;
+ struct tile *ptile = city_map_to_map(pcity, x, y);
- if ((ptile = city_map_to_map(pcity, x, y))) {
+ if (NULL != ptile) {
struct player *owner = tile_owner(ptile);
- switch (get_worker_city(pcity, x, y)) {
+ /* bypass get_worker_city() check is_valid_city_coords() */
+ switch (pcity->city_map[x][y]) {
case C_TILE_EMPTY:
if (ptile->worked) {
- freelog(LOG_ERROR, "Tile at %s->%d,%d%s marked as "
- "empty but worked by %s!",
- city_name(pcity),
+ SANITY_("(%4d,%4d) marked as empty, "
+ "but worked by \"%s\"[%d]! "
+ "\"%s\"[%d]%s"),
TILE_XY(ptile),
- is_city_center(x, y) ? " (city center)" : "",
- city_name(ptile->worked));
+ city_name(ptile->worked), ptile->worked->size,
+ city_name(pcity), pcity->size,
+ is_city_center(x, y) ? "{city center}" : "");
}
if (is_enemy_unit_tile(ptile, pplayer)) {
- freelog(LOG_ERROR, "Tile at %s->%d,%d%s marked as "
- "empty but occupied by an enemy unit!",
- city_name(pcity), TILE_XY(ptile),
- is_city_center(x, y) ? " (city center)" : "");
+ SANITY_("(%4d,%4d) marked as empty, "
+ "but occupied by an enemy unit! "
+ "\"%s\"[%d]%s"),
+ TILE_XY(ptile),
+ city_name(pcity), pcity->size,
+ is_city_center(x, y) ? "{city center}" : "");
}
if (game.info.borders > 0 && owner && owner != city_owner(pcity)) {
- freelog(LOG_ERROR, "Tile at %s->%d,%d%s marked as "
- "empty but in enemy territory!",
- city_name(pcity), TILE_XY(ptile),
- is_city_center(x, y) ? " (city center)" : "");
+ SANITY_("(%4d,%4d) marked as empty, "
+ "but in enemy territory! "
+ "\"%s\"[%d]%s"),
+ TILE_XY(ptile),
+ city_name(pcity), pcity->size,
+ is_city_center(x, y) ? "{city center}" : "");
}
if (!city_can_work_tile(pcity, x, y)) {
/* Complete check. */
- freelog(LOG_ERROR, "Tile at %s->%d,%d%s marked as "
- "empty but is unavailable!",
- city_name(pcity), TILE_XY(ptile),
- is_city_center(x, y) ? " (city center)" : "");
+ SANITY_("(%4d,%4d) marked as empty, "
+ "but is unavailable! "
+ "\"%s\"[%d]%s"),
+ TILE_XY(ptile),
+ city_name(pcity), pcity->size,
+ is_city_center(x, y) ? "{city center}" : "");
}
break;
case C_TILE_WORKER:
- if ((ptile)->worked != pcity) {
- freelog(LOG_ERROR, "Tile at %s->%d,%d%s marked as "
- "worked but main map disagrees!",
- city_name(pcity), TILE_XY(ptile),
- is_city_center(x, y) ? " (city center)" : "");
+ if (ptile->worked != pcity) {
+ SANITY_("(%4d,%4d) marked as worked, "
+ "but main map disagrees! "
+ "\"%s\"[%d]%s"),
+ TILE_XY(ptile),
+ city_name(pcity), pcity->size,
+ is_city_center(x, y) ? "{city center}" : "");
}
if (is_enemy_unit_tile(ptile, pplayer)) {
- freelog(LOG_ERROR, "Tile at %s->%d,%d%s marked as "
- "worked but occupied by an enemy unit!",
- city_name(pcity), TILE_XY(ptile),
- is_city_center(x, y) ? " (city center)" : "");
+ SANITY_("(%4d,%4d) marked as worked, "
+ "but occupied by an enemy unit! "
+ "\"%s\"[%d]%s"),
+ TILE_XY(ptile),
+ city_name(pcity), pcity->size,
+ is_city_center(x, y) ? "{city center}" : "");
}
if (game.info.borders > 0 && owner && owner != city_owner(pcity)) {
- freelog(LOG_ERROR, "Tile at %s->%d,%d%s marked as "
- "worked but in enemy territory!",
- city_name(pcity), TILE_XY(ptile),
- is_city_center(x, y) ? " (city center)" : "");
+ SANITY_("(%4d,%4d) marked as worked, "
+ "but in enemy territory! "
+ "\"%s\"[%d]%s"),
+ TILE_XY(ptile),
+ city_name(pcity), pcity->size,
+ is_city_center(x, y) ? "{city center}" : "");
}
if (!city_can_work_tile(pcity, x, y)) {
/* Complete check. */
- freelog(LOG_ERROR, "Tile at %s->%d,%d%s marked as "
- "worked but is unavailable!",
- city_name(pcity), TILE_XY(ptile),
- is_city_center(x, y) ? " (city center)" : "");
+ SANITY_("(%4d,%4d) marked as worked, "
+ "but is unavailable! "
+ "\"%s\"[%d]%s"),
+ TILE_XY(ptile),
+ city_name(pcity), pcity->size,
+ is_city_center(x, y) ? "{city center}" : "");
}
break;
case C_TILE_UNAVAILABLE:
if (city_can_work_tile(pcity, x, y)) {
- freelog(LOG_ERROR, "Tile at %s->%d,%d%s marked as "
- "unavailable but seems to be available!",
- city_name(pcity), TILE_XY(ptile),
- is_city_center(x, y) ? " (city center)" : "");
+ SANITY_("(%4d,%4d) marked as unavailable, "
+ "but seems to be available! "
+ "\"%s\"[%d]%s"),
+ TILE_XY(ptile),
+ city_name(pcity), pcity->size,
+ is_city_center(x, y) ? "{city center}" : "");
}
break;
}
@@ -328,25 +357,33 @@
workers++;
}
} city_map_iterate_end;
- if (workers + city_specialists(pcity) != pcity->size + 1) {
- int diff = pcity->size + 1 - workers - city_specialists(pcity);
- SANITY_CITY(pcity, workers + city_specialists(pcity) == pcity->size + 1);
- if (diff > 0) {
- pcity->specialists[DEFAULT_SPECIALIST] += diff;
- } else if (diff < 0) {
+ citizens = workers + city_specialists(pcity);
+ delta = pcity->size + FREE_WORKED_TILES - citizens;
+ if (0 != delta) {
+ SANITY_("(%4d,%4d) city size not equal %d citizens, "
+ "repairing \"%s\"[%d]"),
+ TILE_XY(pcity->tile),
+ citizens - FREE_WORKED_TILES,
+ city_name(pcity), pcity->size);
+ if (delta > 0) {
+ pcity->specialists[DEFAULT_SPECIALIST] += delta;
+ } else if (delta < 0) {
specialist_type_iterate(sp) {
- int num = MIN(-diff, pcity->specialists[sp]);
+ int num = MIN(-delta, pcity->specialists[sp]);
- diff += num;
+ delta += num;
pcity->specialists[sp] -= num;
} specialist_type_iterate_end;
- if (diff < 0) {
+ if (delta < 0) {
city_map_checked_iterate(pcity->tile, city_x, city_y, ptile) {
- if (ptile->worked == pcity && diff < 0) {
+ if (delta < 0
+ && pcity == ptile->worked
+ && C_TILE_WORKER == pcity->city_map[city_x][city_y]
+ && !is_free_worked_tile(city_x, city_y)) {
server_remove_worker_city(pcity, city_x, city_y);
- diff++;
+ delta++;
}
} city_map_checked_iterate_end;
}
@@ -379,12 +416,14 @@
SANITY_TILE(ptile, is_valid);
if (pcity->city_map[city_x][city_y] != C_TILE_WORKER) {
- freelog(LOG_ERROR, "%d,%d is listed as being worked by %s "
- "on the map, but %s lists the tile %d,%d as having "
- "status %d\n",
- TILE_XY(ptile), city_name(pcity), city_name(pcity),
+ SANITY_("(%4d,%4d) marked as worked, "
+ "but its {%d,%d} has status %d in "
+ "\"%s\"[%d]%s"),
+ TILE_XY(ptile),
city_x, city_y,
- pcity->city_map[city_x][city_y]);
+ pcity->city_map[city_x][city_y],
+ city_name(pcity), pcity->size,
+ is_city_center(city_x, city_x) ? "{city center}" : "");
}
}
} whole_map_iterate_end;
@@ -398,22 +437,25 @@
unit_list_iterate(pplayer->units, punit) {
struct tile *ptile = punit->tile;
struct city *pcity;
+ struct city *phome;
struct unit *transporter = NULL, *transporter2 = NULL;
SANITY_CHECK(unit_owner(punit) == pplayer);
- if (punit->homecity != 0) {
- pcity = player_find_city_by_id(pplayer, punit->homecity);
- SANITY_CHECK(pcity != NULL);
- SANITY_CHECK(city_owner(pcity) == pplayer);
+ if (IDENTITY_NUMBER_ZERO != punit->homecity) {
+ SANITY_CHECK(phome = player_find_city_by_id(pplayer, punit->homecity));
+ if (phome) {
+ SANITY_CHECK(city_owner(phome) == pplayer);
+ }
}
if (!can_unit_continue_current_activity(punit)) {
- freelog(LOG_ERROR, "%s at %d,%d (%s) has activity %s, "
- "which it can't continue!",
+ SANITY_("(%4d,%4d) %s has activity %s, "
+ "but it can't continue at %s"),
+ TILE_XY(ptile),
unit_rule_name(punit),
- TILE_XY(ptile), tile_get_info_text(ptile, 0),
- get_activity_text(punit->activity));
+ get_activity_text(punit->activity),
+ tile_get_info_text(ptile, 0));
}
pcity = tile_city(ptile);
@@ -502,8 +544,8 @@
if (pplayer->revolution_finishes == -1) {
if (government_of_player(pplayer) == game.government_when_anarchy) {
- freelog(LOG_FATAL, "%s's government is anarchy but does not finish",
- player_name(pplayer));
+ SANITY_("%s government is anarchy, but does not finish!"),
+ nation_rule_name(nation_of_player(pplayer)));
}
SANITY_CHECK(government_of_player(pplayer) !=
game.government_when_anarchy);
} else if (pplayer->revolution_finishes > game.info.turn) {
Index: common/city.h
===================================================================
--- common/city.h (revision 14387)
+++ common/city.h (working copy)
@@ -393,9 +393,6 @@
}
-static inline bool is_city_center(int city_x, int city_y);
-static inline bool is_free_worked_tile(int city_x, int city_y);
-
/* output type functions */
const char *get_output_identifier(Output_type_id output);
@@ -617,6 +614,13 @@
} \
}
+/* === */
+
+static inline bool is_city_center(int city_x, int city_y);
+static inline bool is_free_worked_tile(int city_x, int city_y);
+#define FREE_WORKED_TILES (1)
+
+
/**************************************************************************
Return TRUE iff the given city coordinate pair is the center tile of
the citymap.
Index: common/game.c
===================================================================
--- common/game.c (revision 14387)
+++ common/game.c (working copy)
@@ -203,7 +203,7 @@
set_worker_city(pcity, x, y, C_TILE_EMPTY);
} city_map_checked_iterate_end;
city_list_unlink(city_owner(pcity)->cities, pcity);
- tile_set_city(pcity->tile, NULL);
+ tile_set_city(pcity->tile, NULL); /* redundant to set_worker_city() above */
idex_unregister_city(pcity);
destroy_city_virtual(pcity);
}
@@ -511,7 +511,6 @@
}
pplayer->attribute_block_buffer.length = 0;
-
#if 0
assert(conn_list_size(pplayer->connections) == 0);
/* FIXME: Connections that are unlinked here are left dangling. It's up to
@@ -525,7 +524,11 @@
unit_list_iterate(pplayer->units, punit) {
game_remove_unit(punit);
} unit_list_iterate_end;
- assert(unit_list_size(pplayer->units) == 0);
+ if (0 != unit_list_size(pplayer->units)) {
+ freelog(LOG_ERROR, "game_remove_player() failed to remove %d %s units",
+ unit_list_size(pplayer->units),
+ nation_rule_name(nation_of_player(pplayer)));
+ }
unit_list_unlink_all(pplayer->units);
unit_list_free(pplayer->units);
pplayer->units = NULL;
@@ -533,7 +536,11 @@
city_list_iterate(pplayer->cities, pcity) {
game_remove_city(pcity);
} city_list_iterate_end;
- assert(city_list_size(pplayer->cities) == 0);
+ if (0 != city_list_size(pplayer->cities)) {
+ freelog(LOG_ERROR, "game_remove_player() failed to remove %d %s cities",
+ city_list_size(pplayer->cities),
+ nation_rule_name(nation_of_player(pplayer)));
+ }
city_list_unlink_all(pplayer->cities);
city_list_free(pplayer->cities);
pplayer->cities = NULL;
_______________________________________________
Freeciv-dev mailing list
[email protected]
https://mail.gna.org/listinfo/freeciv-dev
