<URL: http://bugs.freeciv.org/Ticket/Display.html?id=40092 >
In PR#40072, the second reported error has Teotitlan shown with the wrong
owner. This patch does not fix the underlying issue, but is a step toward
making the savegame playable.
The savegame.c:2800 code tries to fix inconsistent games. This leads to
reality_check_city() and update_dumb_city(). I've straightened the logic in
update_dumb_city(), and added a bit more error checking. Found a bug in
PR#39895, missing one crucial line: update_vision_site_from_city().
Comparing the current code with 2.1 and 2.0, and walk-through of PR#13718
changes, found a possible improvement through waiting to update the national
borders (a few lines), matching older code in 2.0.
Other minor cleanup.
Index: server/citytools.c
===================================================================
--- server/citytools.c (revision 14391)
+++ server/citytools.c (working copy)
@@ -186,10 +186,9 @@
terrain_type_iterate(pterrain) {
/* Now we do the same for every available terrain. */
- goodness
- = is_terrain_near_tile(ptile, pterrain)
- ? nc->terrain[terrain_index(pterrain)]
- : -nc->terrain[terrain_index(pterrain)];
+ goodness = is_terrain_near_tile(ptile, pterrain)
+ ? nc->terrain[terrain_index(pterrain)]
+ : -nc->terrain[terrain_index(pterrain)];
if (goodness > 0) {
priority /= mult_factor;
} else if (goodness < 0) {
@@ -998,12 +997,11 @@
vision_reveal_tiles(pcity->server.vision, game.info.city_reveal_tiles);
city_refresh_vision(pcity);
- tile_set_city(ptile, pcity);
+ tile_set_city(ptile, pcity); /* partly redundant to server_set_tile_city() */
city_list_prepend(pplayer->cities, pcity);
/* Claim the ground we stand on */
map_claim_ownership(ptile, pplayer, ptile);
- map_claim_border(ptile, pplayer);
if (terrain_control.may_road) {
tile_set_special(ptile, S_ROAD);
@@ -1031,7 +1029,7 @@
}
}
- /* Place a worker at the city center; this is the free-worked tile.
+ /* Place a worker at the is_city_center() is_free_worked_tile().
* This must be done before the city refresh (below) so that the city
* is in a sane state. */
/* Ugly detail here is that if another city is currently working
@@ -1042,6 +1040,10 @@
* ptile->worked does not point to it, it will give tile up. */
server_set_tile_city(pcity, CITY_MAP_SIZE/2, CITY_MAP_SIZE/2, C_TILE_WORKER);
+ /* Update the national borders. This affects the citymap tile status,
+ * so must be done after the above and before arranging workers. */
+ map_claim_border(ptile, pplayer);
+
/* Refresh the city. First a city refresh is done (this shouldn't
* send any packets to the client because the city has no supported units)
* then rearrange the workers. Note that auto_arrange_workers does its
@@ -1172,9 +1174,9 @@
/* idex_unregister_city() is called in game_remove_city() below */
- /* identity_number_release(pcity->id) is *NOT* done! The cities may still be
- alive in the client, or in the player map. The number of removed
- cities is small, so the loss is acceptable.
+ /* identity_number_release(pcity->id) is *NOT* done! The cities may
+ still be alive in the client, or in the player map. The number of
+ removed cities is small, so the loss is acceptable.
*/
old_vision = pcity->server.vision;
@@ -1730,10 +1732,11 @@
bool update_dumb_city(struct player *pplayer, struct city *pcity)
{
bv_imprs improvements;
- struct vision_site *pdcity = map_get_player_city(pcity->tile, pplayer);
+ struct tile *ptile = city_tile(pcity);
+ struct vision_site *pdcity = map_get_player_city(ptile, pplayer);
/* pcity->occupied isn't used at the server, so we go straight to the
* unit list to check the occupied status. */
- bool occupied = (unit_list_size(pcity->tile->units) > 0);
+ bool occupied = (unit_list_size(ptile->units) > 0);
bool walls = city_got_citywalls(pcity);
bool happy = city_happy(pcity);
bool unhappy = city_unhappy(pcity);
@@ -1747,27 +1750,32 @@
} improvement_iterate_end;
if (NULL == pdcity) {
+ map_get_player_tile(ptile, pplayer)->site =
pdcity = create_vision_site_from_city(pcity);
- map_get_player_tile(pcity->tile, pplayer)->site = pdcity;
- } else if (pdcity->identity == pcity->id
- && vision_owner(pdcity) == city_owner(pcity)
- && pdcity->size == pcity->size
- && 0 == strcmp(pdcity->name, city_name(pcity))
- && pdcity->occupied == occupied
+ } else if (pdcity->location != ptile) {
+ freelog(LOG_ERROR, "Trying to update bad city (wrong location)"
+ " at %i,%i for player %s",
+ TILE_XY(pcity->tile),
+ player_name(pplayer));
+ pdcity->location = ptile; /* ?? */
+ } else if (pdcity->identity != pcity->id) {
+ freelog(LOG_ERROR, "Trying to update old city (wrong identity)"
+ " at %i,%i for player %s",
+ TILE_XY(pcity->tile),
+ player_name(pplayer));
+ pdcity->identity = pcity->id; /* ?? */
+ } else if (pdcity->occupied == occupied
&& pdcity->walls == walls
&& pdcity->happy == happy
&& pdcity->unhappy == unhappy
- && BV_ARE_EQUAL(pdcity->improvements, improvements)) {
+ && BV_ARE_EQUAL(pdcity->improvements, improvements)
+ && pdcity->size == pcity->size
+ && vision_owner(pdcity) == city_owner(pcity)
+ && 0 == strcmp(pdcity->name, city_name(pcity))) {
return FALSE;
}
- if (pdcity->identity != pcity->id) {
- freelog(LOG_ERROR, "Trying to update old city (wrong ID)"
- " at %i,%i for player %s",
- TILE_XY(pcity->tile),
- player_name(pplayer));
- pdcity->identity = pcity->id; /* ?? */
- }
+ update_vision_site_from_city(pdcity, pcity);
pdcity->occupied = occupied;
pdcity->walls = walls;
pdcity->happy = happy;
@@ -1782,12 +1790,14 @@
**************************************************************************/
void reality_check_city(struct player *pplayer,struct tile *ptile)
{
- struct city *pcity = tile_city(ptile);
- struct player_tile *playtile = map_get_player_tile(ptile, pplayer);
struct vision_site *pdcity = map_get_player_city(ptile, pplayer);
if (pdcity) {
+ struct city *pcity = tile_city(ptile);
+
if (!pcity || pcity->id != pdcity->identity) {
+ struct player_tile *playtile = map_get_player_tile(ptile, pplayer);
+
dlsend_packet_city_remove(pplayer->connections, pdcity->identity);
playtile->site = NULL;
free(pdcity);
Index: server/maphand.c
===================================================================
--- server/maphand.c (revision 14391)
+++ server/maphand.c (working copy)
@@ -1742,6 +1742,26 @@
#endif
/*************************************************************************
+ Change ownership of a single tile.
+
+ This implementation is somewhat inefficient. By design, it's not
+ called often. Does not send updates to client.
+*************************************************************************/
+static void map_change_ownership(struct tile *ptile, struct player *play)
+{
+#ifdef CHANGE_CITY_SITE
+ struct vision_site *psite = map_get_player_site(ptile, play);
+
+ assert(NULL != psite);
+ update_city_tile_status_map(tile_city(psite->location), ptile);
+#else
+ city_list_iterate(play->cities, pcity) {
+ update_city_tile_status_map(pcity, ptile);
+ } city_list_iterate_end;
+#endif
+}
+
+/*************************************************************************
Claim ownership of a single tile.
*************************************************************************/
void map_claim_ownership(struct tile *ptile, struct player *powner,
@@ -1761,7 +1781,7 @@
}
}
- if (NULL != powner /* assume && NULL != psource */) {
+ if (NULL != powner && NULL != psource) {
struct city *pcity = tile_city(ptile);
struct player_tile *playtile = map_get_player_tile(psource, powner);
@@ -1769,37 +1789,35 @@
if (ptile != psource) {
map_get_player_tile(ptile, powner)->site = playtile->site;
} else if (NULL != pcity) {
- playtile->site = update_vision_site_from_city(playtile->site, pcity);
+ update_vision_site_from_city(playtile->site, pcity);
} else {
/* has new owner */
playtile->site->owner = powner;
}
} else {
assert(ptile == psource);
+ assert(NULL != pcity); /* FIXME: temporary IDENTITY_NUMBER_ZERO */
if (NULL != pcity) {
playtile->site = create_vision_site_from_city(pcity);
} else {
- playtile->site = create_vision_site(-1/*FIXME*/, psource, powner);
+ playtile->site = create_vision_site(IDENTITY_NUMBER_ZERO, psource,
powner);
}
}
+ } else {
+ assert(NULL == powner && NULL == psource);
}
tile_set_owner(ptile, powner);
send_tile_info(NULL, ptile, FALSE);
- /* This implementation is somewhat inefficient. By design, it's not
- * called often. Does not send updates to client.
- */
- if (NULL != ploser && ploser != powner) {
- city_list_iterate(ploser->cities, pcity) {
- update_city_tile_status_map(pcity, ptile);
- } city_list_iterate_end;
+ if (ploser != powner) {
+ if (NULL != ploser) {
+ map_change_ownership(ptile, ploser);
+ }
+ if (NULL != powner) {
+ map_change_ownership(ptile, powner);
+ }
}
- if (NULL != powner && ploser != powner) {
- city_list_iterate(powner->cities, pcity) {
- update_city_tile_status_map(pcity, ptile);
- } city_list_iterate_end;
- }
}
/*************************************************************************
Index: common/vision.c
===================================================================
--- common/vision.c (revision 14391)
+++ common/vision.c (working copy)
@@ -99,8 +99,8 @@
****************************************************************************/
struct vision_site *create_vision_site_from_city(const struct city *pcity)
{
- struct vision_site *psite = create_vision_site(pcity->id, pcity->tile,
- city_owner(pcity));
+ struct vision_site *psite =
+ create_vision_site(pcity->id, city_tile(pcity), city_owner(pcity));
psite->size = pcity->size;
sz_strlcpy(psite->name, city_name(pcity));
@@ -110,12 +110,12 @@
/****************************************************************************
Returns the basic structure filled with current elements.
****************************************************************************/
-struct vision_site *update_vision_site_from_city(struct vision_site *psite,
- const struct city *pcity)
+void update_vision_site_from_city(struct vision_site *psite,
+ const struct city *pcity)
{
- psite->identity = pcity->id;
- psite->owner = pcity->owner;
+ /* should be same identity and location */
+ psite->owner = city_owner(pcity);
+
psite->size = pcity->size;
sz_strlcpy(psite->name, city_name(pcity));
- return psite;
}
Index: common/vision.h
===================================================================
--- common/vision.h (revision 14391)
+++ common/vision.h (working copy)
@@ -113,12 +113,12 @@
struct tile *location; /* Cannot be NULL */
struct player *owner; /* May be NULL, always check! */
- int identity; /* city/unit >= 100 */
+ int identity; /* city > IDENTITY_NUMBER_ZERO
*/
+ unsigned short size;
bool occupied;
bool walls;
bool happy;
bool unhappy;
- unsigned short size;
bv_imprs improvements;
};
@@ -128,8 +128,8 @@
struct vision_site *create_vision_site(int identity, struct tile *location,
struct player *owner);
struct vision_site *create_vision_site_from_city(const struct city *pcity);
-struct vision_site *update_vision_site_from_city(struct vision_site *psite,
- const struct city *pcity);
+void update_vision_site_from_city(struct vision_site *psite,
+ const struct city *pcity);
/* get 'struct site_list' and related functions: */
#define SPECLIST_TAG site
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
