URL: http://bugs.freeciv.org/Ticket/Display.html?id=40215
2008/4/27 Marko Lindqvist:
Observing autogame player:
handle_city_info() 3 citizens not equal 2 city size in Nagasaki.
For debugging the problem I added check to the server side to get
breakpoint when it is about to send such inconsistent packet. Turned
out that these inconsistent packets are usually sent as part of
transfer_city(), but problem itself is not (necessarily) there. Cities
are already inconsistent before they are transfered. I tried also
adding sanity check for this, but I had to take it off simply because
it was failing constantly and producing far too much warnings. It sees
that cities are in inconsistent state in server side most of the time,
but usually their consistency is restored before sending them to
client (so client side warning has not been failing constantly)
Attached patch fails to fix the bug (at least all of it), but makes
future debugging easier:
- Adds server side consistency check before sending city packet to
client. If it fails, error message is printed and city consistency
restored with refresh_city()
- Adds sanity check function against this problem. This function is
not called anywhere in the current code as it causes too much warnings
spam
- Fixes two of the more obvious cases where city gets inconsistent
state. This requires calling refresh_city() resulting in inefficient
looking code. Some optimization rearrangements are probably needed
later - once we've got the actual bug fixed
- ML
diff -Nurd -X.diff_ignore freeciv/server/citytools.c freeciv/server/citytools.c
--- freeciv/server/citytools.c 2008-04-05 01:48:20.0 +0300
+++ freeciv/server/citytools.c 2008-04-29 02:19:24.0 +0300
@@ -1053,7 +1053,10 @@
if (NULL != pwork) {
/* was previously worked by another city */
+/* Turn citizen into specialist. */
pwork-specialists[DEFAULT_SPECIALIST]++;
+/* One less citizen. Update feelings counters */
+city_refresh(pwork);
pwork-server.synced = FALSE;
city_freeze_workers_queue(pwork);
}
@@ -1691,6 +1694,7 @@
bool dipl_invest)
{
int i;
+ int ppl = 0;
packet-id=pcity-id;
packet-owner = player_number(city_owner(pcity));
@@ -1704,13 +1708,43 @@
packet-ppl_content[i] = pcity-feel[CITIZEN_CONTENT][i];
packet-ppl_unhappy[i] = pcity-feel[CITIZEN_UNHAPPY][i];
packet-ppl_angry[i] = pcity-feel[CITIZEN_ANGRY][i];
+if (i == 0) {
+ ppl += packet-ppl_happy[i];
+ ppl += packet-ppl_content[i];
+ ppl += packet-ppl_unhappy[i];
+ ppl += packet-ppl_angry[i];
+}
}
/* The number of data in specialists[] array */
packet-specialists_size = specialist_count();
specialist_type_iterate(sp) {
packet-specialists[sp] = pcity-specialists[sp];
+ppl += packet-specialists[sp];
} specialist_type_iterate_end;
+ if (packet-size != ppl) {
+static bool recursion = FALSE;
+
+if (recursion) {
+ /* Recursion didn't help. Do not enter infinite recursive loop.
+ * Package city as it is. */
+ freelog(LOG_ERROR, Failed to fix inconsistent city size.);
+ recursion = FALSE;
+} else {
+ freelog(LOG_ERROR, City size %d, citizen count %d for %s,
+ packet-size, ppl, city_name(pcity));
+ /* Try to fix */
+ city_refresh(pcity);
+
+ /* And repackage */
+ recursion = TRUE;
+ package_city(pcity, packet, dipl_invest);
+ recursion = FALSE;
+
+ return;
+}
+ }
+
for (i = 0; i NUM_TRADEROUTES; i++) {
packet-trade[i]=pcity-trade[i];
packet-trade_value[i]=pcity-trade_value[i];
diff -Nurd -X.diff_ignore freeciv/server/cityturn.c freeciv/server/cityturn.c
--- freeciv/server/cityturn.c 2008-03-12 21:57:04.0 +0200
+++ freeciv/server/cityturn.c 2008-04-29 03:08:47.0 +0300
@@ -481,6 +481,7 @@
**/
bool city_reduce_size(struct city *pcity, int pop_loss)
{
+ int loss_remain;
int i;
if (pop_loss == 0) {
@@ -499,24 +500,28 @@
}
/* First try to kill off the specialists */
- i = pop_loss - city_reduce_specialists(pcity, pop_loss);
+ loss_remain = pop_loss - city_reduce_specialists(pcity, pop_loss);
- if (0 i) {
+ /* Update number of people in each feelings category.
+ * This must be after new pcity-size and specialists counts
+ * have been set, and before any auto_arrange_workers() */
+ city_refresh(pcity);
+
+ if (loss_remain 0) {
/* Take it out on workers */
-i -= city_reduce_workers(pcity, i);
+loss_remain -= city_reduce_workers(pcity, loss_remain);
/* Then rearrange workers */
auto_arrange_workers(pcity);
sync_cities();
} else {
-city_refresh(pcity);
send_city_info(city_owner(pcity), pcity);
}
- if (0 != i) {
+ if (0 != loss_remain) {
freelog(LOG_FATAL, city_reduce_size()
has remaining %d of %d for \%s\[%d],
-i, pop_loss,
+
