[Freeciv-Dev] bug/fix: assess_danger_player() (PR#216)

[David Pfitzner <dwp@xxxxxxxxxxxxxx>]

I was hacking some code clean-up, and doing regression testing
to try not to break things, when I had a regression-test
failure which turned out not to be my fault.  Instead, I think
it was due to existing code using uninitialized data.

In particular, occurred when loading a savegame, starting, and 
changing a player to AI, then hitting "end turn".  I traced
regression differences to pcity->ai.wallvalue taking strange 
values for that player for some cities, affecting related AI
unit calculations.  

Turns out that pcity->ai.wallvalue is set by ai/advmilitary.c:
assess_danger(),  which (following torturous AI call sequences...)
occurs in server loop via end_turn(), which occurs after 
ai_start_turn() (aka "Misleading name for manage_units").
Ie, ai.wallvalue was being used before being initialized.
Which is presumably why assess_danger() is called for all AI 
cities before entering the server loop when reloading. 

This patch adds a small func assess_danger_player(), and
calls it when a player is toggled to AI.  I also added
calls to this when a civil war happens, since presumably
things change alot then too...

-- David

diff -u -r --exclude-from exclude freeciv-cvs/ai/advmilitary.c 
freeciv-mod/ai/advmilitary.c
--- freeciv-cvs/ai/advmilitary.c        Sat Aug 21 12:45:18 1999
+++ freeciv-mod/ai/advmilitary.c        Wed Dec 29 22:39:43 1999
@@ -187,6 +187,21 @@
   return(dist);
 }
 
+/********************************************************************** 
+  Call assess_danger() for all cities owned by pplayer.
+  This is necessary to initialize ... <some ai data>
+  before ... <some ai calculations> ...
+***********************************************************************/
+void assess_danger_player(struct player *pplayer)
+{
+  city_list_iterate(pplayer->cities, pcity)
+    assess_danger(pcity);
+  city_list_iterate_end;
+}
+         
+/********************************************************************** 
+...
+***********************************************************************/
 int assess_danger(struct city *pcity)
 {
   int i, danger = 0, v, dist, con, m;
diff -u -r --exclude-from exclude freeciv-cvs/ai/advmilitary.h 
freeciv-mod/ai/advmilitary.h
--- freeciv-cvs/ai/advmilitary.h        Sat Jun 12 17:40:01 1999
+++ freeciv-mod/ai/advmilitary.h        Wed Dec 29 22:39:53 1999
@@ -22,6 +22,7 @@
                                  struct ai_choice *choice);
 void  military_advisor_choose_build(struct player *pplayer, struct city *pcity,
                                    struct ai_choice *choice);
+void assess_danger_player(struct player *pplayer);
 int assess_danger(struct city *pcity);
 void establish_city_distances(struct player *pplayer, struct city *pcity);
 int assess_defense_quadratic(struct city *pcity);
diff -u -r --exclude-from exclude freeciv-cvs/server/citytools.c 
freeciv-mod/server/citytools.c
--- freeciv-cvs/server/citytools.c      Tue Dec 28 23:31:17 1999
+++ freeciv-mod/server/citytools.c      Wed Dec 29 23:00:16 1999
@@ -43,6 +43,8 @@
 #include "unithand.h"
 #include "unittools.h"
 
+#include "advmilitary.h"       /* assess_danger_player() */
+
 #include "citytools.h"
 
 /****************************************************************
@@ -789,6 +791,15 @@
 
   game.nplayers++;
   game.max_players = game.nplayers;
+  
+  /* Not sure if this is necessary, but might be a good idea
+     to avoid doing some ai calculations with bogus data:
+  */
+  assess_danger_player(cplayer);
+  if (pplayer->ai.control) {
+    assess_danger_player(pplayer);
+  }
+                   
   return cplayer;
 }
 
diff -u -r --exclude-from exclude freeciv-cvs/server/civserver.c 
freeciv-mod/server/civserver.c
--- freeciv-cvs/server/civserver.c      Tue Dec 28 23:23:35 1999
+++ freeciv-mod/server/civserver.c      Wed Dec 29 22:41:09 1999
@@ -429,12 +429,11 @@
 
   if (!game.is_new_game) {
     for (i=0;i<game.nplayers;i++) {
-      civ_score(&game.players[i]);  /* if we don't, the AI gets really 
confused */
-      if (game.players[i].ai.control) {
-       set_ai_level_direct(&game.players[i], game.players[i].ai.skill_level);
-        city_list_iterate(game.players[i].cities, pcity)
-          assess_danger(pcity); /* a slowdown, but a necessary one */
-        city_list_iterate_end;
+      struct player *pplayer = get_player(i);
+      civ_score(pplayer);      /* if we don't, the AI gets really confused */
+      if (pplayer->ai.control) {
+       set_ai_level_direct(pplayer, pplayer->ai.skill_level);
+       assess_danger_player(pplayer); /* a slowdown, but a necessary one */
       }
     }
   }
diff -u -r --exclude-from exclude freeciv-cvs/server/stdinhand.c 
freeciv-mod/server/stdinhand.c
--- freeciv-cvs/server/stdinhand.c      Wed Dec 29 22:08:11 1999
+++ freeciv-mod/server/stdinhand.c      Wed Dec 29 23:01:38 1999
@@ -40,6 +40,8 @@
 #include "sernet.h"
 #include "meta.h"
 
+#include "advmilitary.h"       /* assess_danger_player() */
+
 #include "stdinhand.h"
 
 #define MAX_LEN_CMD MAX_LEN_PACKET
@@ -1070,6 +1072,9 @@
        level could have been set as AI, then toggled, then saved,
        then reloaded. */ 
     set_ai_level(caller, pplayer->name, pplayer->ai.skill_level);
+    /* The following is sometimes necessary to avoid using
+       uninitialized data... */
+    assess_danger_player(pplayer);
   } else {
     notify_player(0, _("Game: %s is now human."), pplayer->name);
     cmd_reply(CMD_AITOGGLE, caller, C_OK,