Re: [Freeciv-Dev] (PR#40215) Number of citizens not equal to city size

2008-04-28 Thread Marko Lindqvist

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\"[

Re: [Freeciv-Dev] wrong NEWS linked to from wiki front page

2008-04-28 Thread Daniel Markstedt
FIXED.

Thanks for reporting. =)

Best,
 ~Daniel

On 4/29/08, John Keller <[EMAIL PROTECTED]> wrote:
> Just thought I'd let someone know: the release announcement from 21
>  April on the wiki front page links to NEWS-2.1.3 instead of NEWS-2.1.4.
>
>  I'm not sure if this list is the best place to post that, but I figured
>  I'd give it a try...
>
>  - John
>
>  ___
>  Freeciv-dev mailing list
>  Freeciv-dev@gna.org
>  https://mail.gna.org/listinfo/freeciv-dev
>

___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


[Freeciv-Dev] wrong NEWS linked to from wiki front page

2008-04-28 Thread John Keller
Just thought I'd let someone know: the release announcement from 21
April on the wiki front page links to NEWS-2.1.3 instead of NEWS-2.1.4.

I'm not sure if this list is the best place to post that, but I figured
I'd give it a try...

- John

___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


[Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work

2008-04-28 Thread Madeline Book

http://bugs.freeciv.org/Ticket/Display.html?id=40184 >

> [engla - Mon Apr 28 16:15:54 2008]:
> 
> 2008/4/28 Madeline Book <[EMAIL PROTECTED]>:
> >
> >  http://bugs.freeciv.org/Ticket/Display.html?id=40184 >
> >
> >  The usual status update, gentlemen.
> >
> >  I will have more time to work on this this week since
> >  the stuff I wanted to do for longturn is done now. ;)
> >
> 
> You've made the Global observer work again, I noticed! Would it work
> in a networked game too? I haven't looked into the details in the
> changes. If so, could we break out that part and commit it as soon as
> possible? AFAIK Marko would be happy to have the global observer back.
> And I would too.

See PR#40139. Since I needed global observer to work for testing
editor features, I had need to apply the patch posted there to
my local editor branch. Similarly with PR#40124.


--
料理が全然上手じゃないという噂を聞いた。


___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


Re: [Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work

2008-04-28 Thread Ulrik Sverdrup

http://bugs.freeciv.org/Ticket/Display.html?id=40184 >

2008/4/28 Madeline Book <[EMAIL PROTECTED]>:
>
>  http://bugs.freeciv.org/Ticket/Display.html?id=40184 >
>
>  The usual status update, gentlemen.
>
>  I will have more time to work on this this week since
>  the stuff I wanted to do for longturn is done now. ;)
>

You've made the Global observer work again, I noticed! Would it work
in a networked game too? I haven't looked into the details in the
changes. If so, could we break out that part and commit it as soon as
possible? AFAIK Marko would be happy to have the global observer back.
And I would too.

ulrik



___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


Re: [Freeciv-Dev] (PR#40216) Re: [patch] proposed menu restructuring

2008-04-28 Thread John Keller
Hi, Ulrik. Sorry about replying on-list, you'll see why below.

Ulrik Sverdrup wrote:
> Forwarding the patches too, as they seemed to be missing.
> 
> John, this is a very good proposition. I'll try to look at it.

Cool, thanks! I'm hoping to make it a first of many...

> And -- you can create tickes. In fact you just created
> http://bugs.freeciv.org/Ticket/Display.html?id=40217
> with some misdirected message.

Yeah, I realized that just after I used "reply-all". I'd meant to
replace the [EMAIL PROTECTED] address with the list address, as I did for this
email.

> The important thing to reply to a ticket is the subject line. check
> this subject line. It has to contain a reference to the ticket
> (PR#40216).

I understand, and I've tried that before. This is something that goes
back some time (early 2006, at least). My account's permissions got
messed up at some point, and no one has been able to fix it since then.

When I try to reply via email, I get a message back from RT with the
subject line "Message not recorded" and "Permission Denied" in the body.
In fact, I tried replying to that spurious ticket I created yesterday,
and got this very reply.

The Web interface is no different, in that I can't add comments. I
apparently don't have the right permissions - the "Update Type" pulldown
only lists "Comments (Not sent to requestors)". That allows me to
comment, but the list never gets notified - which makes it almost
impossible to interact with anyone.

It's frustrating since all this means that I can open tickets, but I
can't reply to them - or to other people's tickets. Here's a link to a
thread from February from my last time asking about this:
  https://mail.gna.org/public/freeciv-dev/2008-02/msg00122.html

Hopefully, the kinks in my RT account can be fixed by someone. I've got
a bunch of other ideas that I'm hoping to work on. :-)

Cheers,

- John

___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


Re: [Freeciv-Dev] [patch] proposed menu restructuring

2008-04-28 Thread John Keller
Daniel Markstedt wrote:
> On 4/27/08, John Keller <[EMAIL PROTECTED]> wrote:
>> [...]
>>
>>
>>  I'd be interested in people's reactions to these patches. Hopefully they
>>  (or something like them) could eventually make their way into Freeciv.
>>
>>  - John
>>
> 
> Just one quick comment: You're working with 2.1, aren't you? The menu
> layout has changed somewhat in 2.2 and I think you'd be interesting in
> taking a look.
> 
> Best,
>  ~Daniel

Yup! Already there. :-) I actually based this patch on trunk. I've been
following both trunk and the latest development version (so right now,
2.2) for the past couple of years, playing full games in them when time
allows.

My patches were based on my experiences since the 1.x days, but my
comments in my email were more specifically referring to trunk and 2.2.

Cheers,

- John

___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev