Re: [Freeciv-Dev] bug in freeciv1.8.0 ?

[David Pfitzner <dwp@xxxxxxxxxxxxxx>]

Nicolas Brunel wrote:

> On Tue, 13 Apr 1999, Oleg Bulyzhin wrote:
> 
> > Hello.
> > 
> > It seems i found a bug in freeciv 1.8.0 - at least freeciv 1.7.2 behaviour 
> > is
> > different in same situation.
> > I looked through source code and here is result:
> >     a) function citizen_happy_units() (in server/cityturn.c) in freeciv
> >        version 1.8.0 was overworked since 1.7.2 and give slightly different
> >        result. (maybe it's a bugfix ?)
> >     
> >        Here is difference:
> >             Government: Democracy
> >             "Happy" wonders: Michelangelo's Chapel
> >             City with size 6 support 1 "aggro" unit
> >             When you try to convert some workers to Elvises in order to
> >             eliminate disorder you'll get:
> >                     1.7.2 -- 1 content 5 Elvises
> >                     1.8.0 -- 1 unhappy 5 Elvises
> I don't think the difference came from citizen_happy_unit.
> I'd rather think it's cause by the 2 last ligns of the diff.
> @@ -477,26 +486,25 @@
>        case G_REPUBLIC:
>    pcity->shield_surplus--;
>    this_unit->upkeep=1;
> -  if (unit_being_aggressive(this_unit)) {
> +  if (!orig_suffrage_applies && (unit_being_aggressive(this_unit)
> +              || is_field_unit(this_unit))) {

Actually, no, I don't think that is the problem.  I was able
to reproduce a case of the above difference between 1.7.2 and 
1.8.0, and after selectively applying parts of the diff between 
those versions for server/cityturn.c, I tracked down the substantive 
change.

It turns out that the difference is due to a bug in _1.7.2_,
which was fixed (possibly inadvertently) when an citizen_happy_units
was changed to be more efficient.

The relevant part of the diff is:

--- freeciv-1.7.2/server/cityturn.c     Wed Dec 16 09:07:50 1998
+++ freeciv-1.8.0/server/cityturn.c     Sun Feb 28 12:02:46 1999
@@ -195,28 +199,31 @@
 **************************************************************************/
 void citizen_happy_units(struct city *pcity, int unhap)
 {
-  if (city_got_effect(pcity, B_POLICE)) {
+  int step;         
+
+  if (improvement_variant(B_WOMENS)==0
+      && city_got_effect(pcity, B_POLICE)) {
     if (get_government(pcity->owner)==G_DEMOCRACY)
       unhap-=2;
     else
       unhap--;
   }
-  if (unhap>0 && (pcity->ppl_happy[3] + pcity->ppl_content[3])) { 
-    while (unhap> 0  && pcity->ppl_content[3]) {
-      pcity->ppl_content[3]--;
-      pcity->ppl_unhappy[3]++;
-      unhap--;
+  if (unhap>0) {                                                           
+    step=min(unhap,pcity->ppl_content[3]);                          
+    pcity->ppl_content[3]-=step;
+    pcity->ppl_unhappy[3]+=step;
+    unhap-=step;                     
+    if (unhap>0) {                       
+      step=min((unhap/2),pcity->ppl_happy[3]);    
+      pcity->ppl_happy[3]-=step;
+      pcity->ppl_unhappy[3]+=step;
+      unhap -= step * 2;                                                
+      if ((unhap > 0) && pcity->ppl_happy[3]) {                      
+       pcity->ppl_happy[3]--;                                   
+       pcity->ppl_content[3]++;                
+       unhap--;              
+      }                                                           
     }
-    while (unhap > 2 && pcity->ppl_happy[3]) {
-      pcity->ppl_happy[3]--;
-      pcity->ppl_unhappy[3]++;
-      unhap -= 2;
-    } 
-    if (unhap > 0 && pcity->ppl_happy[3]) {
-      pcity->ppl_happy[3]--;
-      pcity->ppl_content[3]++;
-      unhap--;
-    } 
   }
     /* MAKE VERY UNHAPPY CITIZENS WITH THE REST, but that is not documented */
 
---

The bug in 1.7.2 is that above, the line
-    while (unhap > 2 && pcity->ppl_happy[3]) {
should have had ">=" instead of ">".

So in 1.7.2, if there is only one non-specialist, and after
other changes that citizen is happy, an unhap of exactly 2 
doesn't make that citizen unhappy, but falls down to the next 
case and just makes them content.

In 1.8.0, in the similar case the unhap of 2 does apply
fully to the happy citizen, and makes then unhappy.

(So in this case, making too many elvii can be counter-productive;
you want to arrange a case where you have 2 happy citizens left
before applying unit unhap=2, so you end up with 1 happy, 1 unhappy,
to balance.)

So I think the only bugs in this respect for 1.8.0 are:
- Not properly documented.
- A happiness display available from the city dialog, which shows
  the happiness result at each step (like in Civ1), would be nice.

Regards,
-- David