From 743fc2f69d00a825acae038aa368c97ee249dcea Mon Sep 17 00:00:00 2001 From: Marko Lindqvist Date: Thu, 8 Apr 2021 17:15:30 +0300 Subject: [PATCH 03/44] 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 682d55de49..e7c7716af3 100644 --- a/common/city.h +++ b/common/city.h @@ -291,6 +291,12 @@ enum city_updates { CU_POPUP_DIALOG = 1 << 2 }; +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 */ @@ -412,7 +418,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 de7549c4e3..905f00a8fd 100644 --- a/server/citytools.c +++ b/server/citytools.c @@ -145,7 +145,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); } @@ -164,7 +165,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; + } } /************************************************************************//** @@ -2134,7 +2137,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; @@ -2143,7 +2146,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) { @@ -2286,6 +2298,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); } @@ -2293,9 +2310,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); @@ -2321,7 +2341,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) { @@ -2367,6 +2387,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 0856af0fc2..9d60166fbf 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 7d56e029b5..b0befe9c90 100644 --- a/server/cityturn.c +++ b/server/cityturn.c @@ -356,24 +356,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. */ @@ -448,6 +453,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 02c7467cf4..691a38658e 100644 --- a/server/sanitycheck.c +++ b/server/sanitycheck.c @@ -371,7 +371,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