[Freeciv-Dev] Re: (PR#13624) AI caught with pants down by overzealous assert

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=13624 >

Per I. Mathisen wrote:

> The thing is, this is an easy AI, and it expects that it can change
> government at any time, instantly, but I removed that possibility. It
> optimistically keeps trying anyway, but since someone added an assert
> against trying a revolution in the middle of a revolution, it crashes.

There are a lot of possible cases here and the asserts are intended to 
restrict the behavior to only those which are sane.  However behavior 
was changed recently so that now on the turn when you finish a 
revolution, you can revolt again immediately.  I had a feeling this 
could be buggy.  You can reproduce this in a single-player game: change 
government to republic, then on the turn when the revolution finishes 
change it back to despotism.

I believe this patch fixes this behavior.

-jason

? sset.diff
Index: server/plrhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/plrhand.c,v
retrieving revision 1.405
diff -p -u -r1.405 plrhand.c
--- server/plrhand.c    6 Aug 2005 18:15:38 -0000       1.405
+++ server/plrhand.c    8 Aug 2005 14:54:12 -0000
@@ -294,10 +294,14 @@ static void start_revolution(struct play
 
   /* Set revolution_finishes value. */
   if (pplayer->revolution_finishes > 0) {
-    /* Player already has an active revolution. */
-    assert(pplayer->revolution_finishes > game.info.turn);
-    assert(pplayer->government == game.info.government_when_anarchy);
-    return;
+    /* Player already has an active revolution.  Note that on the turn
+     * the revolution finishes the government may be changed out of
+     * anarchy but the revolution counter is not reset until the end
+     * of the turn. */
+    assert(pplayer->revolution_finishes >= game.info.turn);
+    assert(pplayer->revolution_finishes == game.info.turn
+          || pplayer->government == game.info.government_when_anarchy);
+    turns = pplayer->revolution_finishes - game.info.turn;
   } else if ((pplayer->ai.control && !ai_handicap(pplayer, H_REVOLUTION))
             || get_player_bonus(pplayer, EFT_NO_ANARCHY)) {
     /* AI players without the H_REVOLUTION handicap can skip anarchy */
@@ -316,15 +320,17 @@ static void start_revolution(struct play
          "Revofin %d (%d).",
          pplayer->name, get_government_name(pplayer->target_government),
          pplayer->revolution_finishes, game.info.turn);
-  notify_player_ex(pplayer, NULL, E_REVOLT_START,
-                  /* TRANS: this is a message event so don't make it
-                   * too long. */
-                  PL_("The %s have incited a revolt! "
-                      "%d turn of anarchy will ensue!",
-                      "The %s have incited a revolt! "
-                      "%d turns of anarchy will ensue!",
-                      turns),
-                  get_nation_name_plural(pplayer->nation), turns);
+  if (turns > 0) {
+    notify_player_ex(pplayer, NULL, E_REVOLT_START,
+                    /* TRANS: this is a message event so don't make it
+                     * too long. */
+                    PL_("The %s have incited a revolt! "
+                        "%d turn of anarchy will ensue!",
+                        "The %s have incited a revolt! "
+                        "%d turns of anarchy will ensue!",
+                        turns),
+                    get_nation_name_plural(pplayer->nation), turns);
+  }
   gamelog(GAMELOG_REVOLT, pplayer);
 
   /* Now see if the revolution is instantaneous. */
@@ -359,19 +365,8 @@ void handle_player_change_government(str
          get_government_name(pplayer->government),
          pplayer->revolution_finishes, game.info.turn);
 
-  if (pplayer->government == game.info.government_when_anarchy) {
-    /* Already having a revolution. */
-    assert(pplayer->revolution_finishes >= 0);
-    if (pplayer->revolution_finishes <= game.info.turn
-       && government != game.info.government_when_anarchy) {
-      /* The revolution was already over.  Now we should enter the new
-       * government immediately. */
-      finish_revolution(pplayer);
-    }
-  } else {
-    /* No revolution: start one. */
-    start_revolution(pplayer);
-  }
+  /* Start or continue a revolution. */
+  start_revolution(pplayer);
 
   freelog(LOG_DEBUG,
          "Government change complete for %s.  Target government is %s; "