From 656594c994ee1e4accbc2056c78f3687f1a65a80 Mon Sep 17 00:00:00 2001 From: Marko Lindqvist Date: Thu, 8 Apr 2021 17:53:43 +0300 Subject: [PATCH 04/23] Wait until city is in sane state before sending info packets Don't send city info packets to client in the middle of the city processing when city might be in inconsistent state. See osdn #41851 Signed-off-by: Marko Lindqvist --- common/city.h | 8 +++++++- server/citytools.c | 34 ++++++++++++++++++++++++++++------ server/citytools.h | 1 + server/cityturn.c | 15 ++++++++++++--- server/sanitycheck.c | 2 +- 5 files changed, 49 insertions(+), 11 deletions(-) diff --git a/common/city.h b/common/city.h index b600452b90..5a202f672c 100644 --- a/common/city.h +++ b/common/city.h @@ -299,6 +299,12 @@ enum city_build_result { CB_NO_MIN_DIST }; +enum city_needs_arrange { + CNA_NOT = 0, + CNA_NORMAL, + CNA_BROADCAST_PENDING +}; + struct tile_cache; /* defined and only used within city.c */ struct adv_city; /* defined in ./server/advisors/infracache.h */ @@ -405,7 +411,7 @@ struct city { /* If set, workers need to be arranged when the city is unfrozen. * Set inside auto_arrange_workers() and city_freeze_workers_queue(). */ - bool needs_arrange; + enum city_needs_arrange needs_arrange; /* If set, city needs to be refreshed at a later time. * Set inside city_refresh() and city_refresh_queue_add(). */ diff --git a/server/citytools.c b/server/citytools.c index 0054ae96b1..6dae87e96c 100644 --- a/server/citytools.c +++ b/server/citytools.c @@ -141,7 +141,8 @@ void city_thaw_workers(struct city *pcity) { pcity->server.workers_frozen--; fc_assert(pcity->server.workers_frozen >= 0); - if (pcity->server.workers_frozen == 0 && pcity->server.needs_arrange) { + if (pcity->server.workers_frozen == 0 + && pcity->server.needs_arrange != CNA_NOT) { city_refresh(pcity); /* Citizen count sanity */ auto_arrange_workers(pcity); } @@ -160,7 +161,9 @@ void city_freeze_workers_queue(struct city *pcity) city_list_prepend(arrange_workers_queue, pcity); city_freeze_workers(pcity); - pcity->server.needs_arrange = TRUE; + if (pcity->server.needs_arrange == CNA_NOT) { + pcity->server.needs_arrange = CNA_NORMAL; + } } /**************************************************************************** @@ -2099,7 +2102,7 @@ void refresh_dumb_city(struct city *pcity) (Split off from send_city_info_at_tile() because that was getting too difficult for me to understand... --dwp) **************************************************************************/ -static void broadcast_city_info(struct city *pcity) +void broadcast_city_info(struct city *pcity) { struct packet_city_info packet; struct packet_web_city_info_addition web_packet; @@ -2108,7 +2111,16 @@ static void broadcast_city_info(struct city *pcity) struct traderoute_packet_list *routes = traderoute_packet_list_new(); /* Send to everyone who can see the city. */ - package_city(pcity, &packet, &web_packet, routes, FALSE); + if (pcity->server.needs_arrange == CNA_NOT) { + /* Send city only when it's in sane state. + * If it's not, it will be sent to everyone once + * rearrangement has been finished. */ + package_city(pcity, &packet, &web_packet, routes, FALSE); + } else { + pcity->server.needs_arrange = CNA_BROADCAST_PENDING; + + return; + } players_iterate(pplayer) { if (can_player_see_city_internals(pplayer, pcity)) { if (!send_city_suppressed || pplayer != powner) { @@ -2251,6 +2263,11 @@ void send_city_info_at_tile(struct player *pviewer, struct conn_list *dest, if (!pcity) { pcity = tile_city(ptile); } + if (pcity->server.needs_arrange != CNA_NOT) { + pcity->server.needs_arrange = CNA_BROADCAST_PENDING; + + return; + } if (pcity) { powner = city_owner(pcity); } @@ -2258,9 +2275,12 @@ void send_city_info_at_tile(struct player *pviewer, struct conn_list *dest, /* send info to owner */ /* This case implies powner non-NULL which means pcity non-NULL */ if (!send_city_suppressed) { + /* Wait that city has been rearranged, if it's currently + * not in sane state. */ + routes = traderoute_packet_list_new(); - /* send all info to the owner */ + /* Send all info to the owner */ update_dumb_city(powner, pcity); package_city(pcity, &packet, &web_packet, routes, FALSE); lsend_packet_city_info(dest, &packet, FALSE); @@ -2286,7 +2306,7 @@ void send_city_info_at_tile(struct player *pviewer, struct conn_list *dest, if (pcity) { routes = traderoute_packet_list_new(); - package_city(pcity, &packet, &web_packet, routes, FALSE); /* should be dumb_city info? */ + package_city(pcity, &packet, &web_packet, routes, FALSE); /* should be dumb_city info? */ lsend_packet_city_info(dest, &packet, FALSE); web_lsend_packet(city_info_addition, dest, &web_packet, FALSE); traderoute_packet_list_iterate(routes, route_packet) { @@ -2332,6 +2352,8 @@ void package_city(struct city *pcity, struct packet_city_info *packet, int i; int ppl = 0; + fc_assert(!pcity->server.needs_arrange); + packet->id = pcity->id; packet->owner = player_number(city_owner(pcity)); packet->tile = tile_index(city_tile(pcity)); diff --git a/server/citytools.h b/server/citytools.h index e96cb34cf9..3abbf3e04b 100644 --- a/server/citytools.h +++ b/server/citytools.h @@ -50,6 +50,7 @@ void send_city_info_at_tile(struct player *pviewer, struct conn_list *dest, struct city *pcity, struct tile *ptile); void send_all_known_cities(struct conn_list *dest); void send_player_cities(struct player *pplayer); +void broadcast_city_info(struct city *pcity); void package_city(struct city *pcity, struct packet_city_info *packet, struct packet_web_city_info_addition *web_packet, struct traderoute_packet_list *routes, diff --git a/server/cityturn.c b/server/cityturn.c index 66e7b537c1..20fa02f6cf 100644 --- a/server/cityturn.c +++ b/server/cityturn.c @@ -308,24 +308,29 @@ void auto_arrange_workers(struct city *pcity) { struct cm_parameter cmp; struct cm_result *cmr; + bool broadcast_needed; /* See comment in freeze_workers(): we can't rearrange while * workers are frozen (i.e. multiple updates need to be done). */ if (pcity->server.workers_frozen > 0) { - pcity->server.needs_arrange = TRUE; + if (pcity->server.needs_arrange == CNA_NOT) { + pcity->server.needs_arrange = CNA_NORMAL; + } return; } TIMING_LOG(AIT_CITIZEN_ARRANGE, TIMER_START); + broadcast_needed = (pcity->server.needs_arrange == CNA_BROADCAST_PENDING); + /* Freeze the workers and make sure all the tiles around the city * are up to date. Then thaw, but hackishly make sure that thaw * doesn't call us recursively, which would waste time. */ city_freeze_workers(pcity); - pcity->server.needs_arrange = FALSE; + pcity->server.needs_arrange = CNA_NOT; city_map_update_all(pcity); - pcity->server.needs_arrange = FALSE; + pcity->server.needs_arrange = CNA_NOT; city_thaw_workers(pcity); /* Now start actually rearranging. */ @@ -423,6 +428,10 @@ void auto_arrange_workers(struct city *pcity) } sanity_check_city(pcity); + if (broadcast_needed) { + broadcast_city_info(pcity); + } + cm_result_destroy(cmr); TIMING_LOG(AIT_CITIZEN_ARRANGE, TIMER_STOP); } diff --git a/server/sanitycheck.c b/server/sanitycheck.c index 86d7251164..810f2e059e 100644 --- a/server/sanitycheck.c +++ b/server/sanitycheck.c @@ -369,7 +369,7 @@ static void check_city_feelings(const struct city *pcity, const char *file, * savegame despite the fact that city workers_frozen level of the city * is above zero -> can't limit sanitycheck callpoints by that. Instead * we check even more relevant needs_arrange. */ - SANITY_CITY(pcity, !pcity->server.needs_arrange); + SANITY_CITY(pcity, pcity->server.needs_arrange == CNA_NOT); SANITY_CITY(pcity, city_size_get(pcity) - spe == sum); } -- 2.30.2