<URL: http://bugs.freeciv.org/Ticket/Display.html?id=39636 >
Assert fails when updating unit causes it to have smaller vision range.
Consistency between vision layers is being checked in wrong place -
when we are in the middle of the process updating them one by one.
Attached patch removes assert after update of single layer, and places
it after all the layers have been iterated (similar iteration is in
several places)
Is there another bug in the fact that V_INVIS layer has been greater?
Shouldn't it be constant to all units (revealing only adjacent tiles)?
- ML
diff -Nurd -X.diff_ignore freeciv/server/citytools.c freeciv/server/citytools.c
--- freeciv/server/citytools.c 2007-08-13 22:23:44.000000000 +0300
+++ freeciv/server/citytools.c 2007-08-30 17:30:49.000000000 +0300
@@ -769,7 +769,7 @@
int old_trade_routes[NUM_TRADEROUTES];
bv_imprs had_small_wonders;
char old_city_name[MAX_LEN_NAME];
- struct vision *old_vision;
+ struct vision *old_vision, *new_vision;
assert(pgiver != ptaker);
@@ -800,12 +800,15 @@
give_citymap_from_player_to_player(pcity, pgiver, ptaker);
old_vision = pcity->server.vision;
- pcity->server.vision = vision_new(ptaker, pcity->tile, FALSE);
+ new_vision = vision_new(ptaker, pcity->tile, FALSE);
+ pcity->server.vision = new_vision;
vision_layer_iterate(v) {
- vision_change_sight(pcity->server.vision, v,
+ vision_change_sight(new_vision, v,
vision_get_sight(old_vision, v));
} vision_layer_iterate_end;
+ ASSERT_VISION(new_vision);
+
sz_strlcpy(old_city_name, pcity->name);
if (game.info.allowed_city_names == 1
&& city_list_find_name(ptaker->cities, pcity->name)) {
@@ -2165,4 +2168,6 @@
vision_change_sight(pcity->server.vision, V_MAIN, radius_sq);
vision_change_sight(pcity->server.vision, V_INVIS, 2);
+
+ ASSERT_VISION(pcity->server.vision);
}
diff -Nurd -X.diff_ignore freeciv/server/maphand.c freeciv/server/maphand.c
--- freeciv/server/maphand.c 2007-07-04 14:04:17.000000000 +0300
+++ freeciv/server/maphand.c 2007-08-30 17:12:27.000000000 +0300
@@ -1700,15 +1700,7 @@
}
/* Vision structure - see documentation in maphand.h */
-struct vision {
- /* These values cannot be changed after initialization. */
- struct player *player;
- struct tile *tile;
- bool can_reveal_tiles;
- /* The radius of the vision source. */
- int radius_sq[V_COUNT];
-};
/****************************************************************************
Create a new vision source.
@@ -1753,8 +1745,6 @@
vision->radius_sq[vlayer], radius_sq,
vision->can_reveal_tiles, vlayer);
vision->radius_sq[vlayer] = radius_sq;
-
- assert(vision->radius_sq[V_MAIN] >= vision->radius_sq[V_INVIS]);
}
/****************************************************************************
diff -Nurd -X.diff_ignore freeciv/server/maphand.h freeciv/server/maphand.h
--- freeciv/server/maphand.h 2007-06-28 20:19:22.000000000 +0300
+++ freeciv/server/maphand.h 2007-08-30 17:12:26.000000000 +0300
@@ -149,7 +149,17 @@
vision sources are active. The same process applies when transferring
a unit or city between players, etc.
****************************************************************************/
-struct vision;
+
+struct vision {
+ /* These values cannot be changed after initialization. */
+ struct player *player;
+ struct tile *tile;
+ bool can_reveal_tiles;
+
+ /* The radius of the vision source. */
+ int radius_sq[V_COUNT];
+};
+
struct vision *vision_new(struct player *pplayer, struct tile *ptile,
bool can_reveal_tiles);
int vision_get_sight(const struct vision *vision, enum vision_layer vlayer);
@@ -158,4 +168,9 @@
void vision_clear_sight(struct vision *vision);
void vision_free(struct vision *vision);
+#define ASSERT_VISION(v) \
+ do { \
+ assert((v)->radius_sq[V_MAIN] >= (v)->radius_sq[V_INVIS]); \
+ } while(FALSE);
+
#endif /* FC__MAPHAND_H */
diff -Nurd -X.diff_ignore freeciv/server/unittools.c freeciv/server/unittools.c
--- freeciv/server/unittools.c 2007-08-06 16:42:12.000000000 +0300
+++ freeciv/server/unittools.c 2007-08-30 17:05:46.000000000 +0300
@@ -2723,6 +2723,7 @@
struct city *pcity;
struct unit *ptransporter = NULL;
struct vision *old_vision = punit->server.vision;
+ struct vision *new_vision;
conn_list_do_buffer(pplayer->connections);
@@ -2743,13 +2744,16 @@
/* Insert them again. */
unit_list_iterate(cargo_units, pcargo) {
struct vision *old_vision = pcargo->server.vision;
+ struct vision *new_vision = vision_new(pcargo->owner, pdesttile, TRUE);
- pcargo->server.vision = vision_new(pcargo->owner, pdesttile, TRUE);
+ pcargo->server.vision = new_vision;
vision_layer_iterate(v) {
- vision_change_sight(pcargo->server.vision, v,
+ vision_change_sight(new_vision, v,
get_unit_vision_at(pcargo, pdesttile, v));
} vision_layer_iterate_end;
+ ASSERT_VISION(new_vision);
+
/* Silently free orders since they won't be applicable anymore. */
free_unit_orders(pcargo);
@@ -2775,12 +2779,15 @@
move */
/* Enhance vision if unit steps into a fortress */
- punit->server.vision = vision_new(punit->owner, pdesttile, TRUE);
+ new_vision = vision_new(punit->owner, pdesttile, TRUE);
+ punit->server.vision = new_vision;
vision_layer_iterate(v) {
- vision_change_sight(punit->server.vision, v,
+ vision_change_sight(new_vision, v,
get_unit_vision_at(punit, pdesttile, v));
} vision_layer_iterate_end;
+ ASSERT_VISION(new_vision);
+
/* Claim ownership of fortress? */
if (tile_has_special(pdesttile, S_FORTRESS)
&& (!pdesttile->owner || pplayers_at_war(pdesttile->owner, pplayer))) {
@@ -3260,12 +3267,16 @@
****************************************************************************/
void unit_refresh_vision(struct unit *punit)
{
+ struct vision *uvision = punit->server.vision;
+
vision_layer_iterate(v) {
/* This requires two calls to get_unit_vision_at...it could be
* optimized. */
- vision_change_sight(punit->server.vision, v,
+ vision_change_sight(uvision, v,
get_unit_vision_at(punit, punit->tile, v));
} vision_layer_iterate_end;
+
+ ASSERT_VISION(uvision);
}
/****************************************************************************
diff -Nurd -X.diff_ignore freeciv/server/citytools.c freeciv/server/citytools.c
--- freeciv/server/citytools.c 2007-08-14 02:36:43.000000000 +0300
+++ freeciv/server/citytools.c 2007-08-30 17:17:28.000000000 +0300
@@ -772,7 +772,7 @@
int old_trade_routes[NUM_TRADEROUTES];
bv_imprs had_small_wonders;
char old_city_name[MAX_LEN_NAME];
- struct vision *old_vision;
+ struct vision *old_vision, *new_vision;
assert(pgiver != ptaker);
@@ -807,13 +807,16 @@
give_citymap_from_player_to_player(pcity, pgiver, ptaker);
old_vision = pcity->server.vision;
- pcity->server.vision = vision_new(ptaker, pcity->tile);
- vision_reveal_tiles(pcity->server.vision, game.info.city_reveal_tiles);
+ new_vision = vision_new(ptaker, pcity->tile);
+ pcity->server.vision = new_vision;
+ vision_reveal_tiles(new_vision, game.info.city_reveal_tiles);
vision_layer_iterate(v) {
- vision_change_sight(pcity->server.vision, v,
+ vision_change_sight(new_vision, v,
vision_get_sight(old_vision, v));
} vision_layer_iterate_end;
+ ASSERT_VISION(new_vision);
+
sz_strlcpy(old_city_name, pcity->name);
if (game.info.allowed_city_names == 1
&& city_list_find_name(ptaker->cities, pcity->name)) {
@@ -2179,4 +2182,6 @@
vision_change_sight(pcity->server.vision, V_MAIN, radius_sq);
vision_change_sight(pcity->server.vision, V_INVIS, 2);
+
+ ASSERT_VISION(pcity->server.vision);
}
diff -Nurd -X.diff_ignore freeciv/server/maphand.c freeciv/server/maphand.c
--- freeciv/server/maphand.c 2007-08-04 18:36:24.000000000 +0300
+++ freeciv/server/maphand.c 2007-08-30 17:15:50.000000000 +0300
@@ -1855,15 +1855,7 @@
}
/* Vision structure - see documentation in maphand.h */
-struct vision {
- /* These values cannot be changed after initialization. */
- struct player *player;
- struct tile *tile;
- bool can_reveal_tiles;
- /* The radius of the vision source. */
- int radius_sq[V_COUNT];
-};
/****************************************************************************
Create a new vision source.
@@ -1921,8 +1913,6 @@
vision->radius_sq[vlayer], radius_sq,
vision->can_reveal_tiles, vlayer);
vision->radius_sq[vlayer] = radius_sq;
-
- assert(vision->radius_sq[V_MAIN] >= vision->radius_sq[V_INVIS]);
}
/****************************************************************************
diff -Nurd -X.diff_ignore freeciv/server/maphand.h freeciv/server/maphand.h
--- freeciv/server/maphand.h 2007-08-13 20:51:02.000000000 +0300
+++ freeciv/server/maphand.h 2007-08-30 17:18:25.000000000 +0300
@@ -153,7 +153,17 @@
vision sources are active. The same process applies when transferring
a unit or city between players, etc.
****************************************************************************/
-struct vision;
+
+struct vision {
+ /* These values cannot be changed after initialization. */
+ struct player *player;
+ struct tile *tile;
+ bool can_reveal_tiles;
+
+ /* The radius of the vision source. */
+ int radius_sq[V_COUNT];
+};
+
struct vision *vision_new(struct player *pplayer, struct tile *ptile);
bool vision_reveal_tiles(struct vision *vision, bool reveal_tiles);
int vision_get_sight(const struct vision *vision, enum vision_layer vlayer);
@@ -162,4 +172,9 @@
void vision_clear_sight(struct vision *vision);
void vision_free(struct vision *vision);
+#define ASSERT_VISION(v) \
+ do { \
+ assert((v)->radius_sq[V_MAIN] >= (v)->radius_sq[V_INVIS]); \
+ } while(FALSE);
+
#endif /* FC__MAPHAND_H */
diff -Nurd -X.diff_ignore freeciv/server/unittools.c freeciv/server/unittools.c
--- freeciv/server/unittools.c 2007-08-13 20:51:02.000000000 +0300
+++ freeciv/server/unittools.c 2007-08-30 17:20:16.000000000 +0300
@@ -2654,6 +2654,7 @@
struct city *pcity;
struct unit *ptransporter = NULL;
struct vision *old_vision = punit->server.vision;
+ struct vision *new_vision;
conn_list_do_buffer(pplayer->connections);
@@ -2674,13 +2675,16 @@
/* Insert them again. */
unit_list_iterate(cargo_units, pcargo) {
struct vision *old_vision = pcargo->server.vision;
+ struct vision *new_vision = vision_new(pcargo->owner, pdesttile);
- pcargo->server.vision = vision_new(pcargo->owner, pdesttile);
+ pcargo->server.vision = new_vision;
vision_layer_iterate(v) {
- vision_change_sight(pcargo->server.vision, v,
+ vision_change_sight(new_vision, v,
get_unit_vision_at(pcargo, pdesttile, v));
} vision_layer_iterate_end;
+ ASSERT_VISION(new_vision);
+
/* Silently free orders since they won't be applicable anymore. */
free_unit_orders(pcargo);
@@ -2706,12 +2710,15 @@
move */
/* Enhance vision if unit steps into a fortress */
- punit->server.vision = vision_new(punit->owner, pdesttile);
+ new_vision = vision_new(punit->owner, pdesttile);
+ punit->server.vision = new_vision;
vision_layer_iterate(v) {
- vision_change_sight(punit->server.vision, v,
+ vision_change_sight(new_vision, v,
get_unit_vision_at(punit, pdesttile, v));
} vision_layer_iterate_end;
+ ASSERT_VISION(new_vision);
+
/* Claim ownership of fortress? */
if (tile_has_base_flag_for_unit(pdesttile, unit_type(punit),
BF_CLAIM_TERRITORY)
@@ -3199,12 +3206,16 @@
****************************************************************************/
void unit_refresh_vision(struct unit *punit)
{
+ struct vision *uvision = punit->server.vision;
+
vision_layer_iterate(v) {
/* This requires two calls to get_unit_vision_at...it could be
* optimized. */
- vision_change_sight(punit->server.vision, v,
+ vision_change_sight(uvision, v,
get_unit_vision_at(punit, punit->tile, v));
} vision_layer_iterate_end;
+
+ ASSERT_VISION(uvision);
}
/****************************************************************************
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
