[Freeciv-Dev] (PR#13429) Move player_research to team

["Mateusz Stefek" <mstefek@xxxxxxxxx>]

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

> [jdorje - Sat Jul 09 16:36:44 2005]:
> 
> 1.  Why does team.h need to include player.h?

You are right. It only needs tech.h

> 2.  get_player_research should return NULL if the player has no team.
> Callers must be audited for teamless players or NULL return values.

Why can't we assume that each players has a team?
An autogame with barbarians and civil war seems to work.

 
> -jason
Updated patch attached. With assertion in get_player_research()

--
mateusz

? civgame.log
? civscore.log
? regression
? server/civgame-1000.sav.gz
? server/civgame-1500.sav.gz
? server/civgame-2000.sav.gz
? server/civgame-2500.sav.gz
? server/civgame-3000.sav.gz
? server/civgame-3500.sav.gz
Index: client/packhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/packhand.c,v
retrieving revision 1.528
diff -u -r1.528 packhand.c
--- client/packhand.c   9 Jul 2005 17:46:07 -0000       1.528
+++ client/packhand.c   9 Jul 2005 18:21:35 -0000
@@ -1416,7 +1416,7 @@
   bool poptechup, new_tech = FALSE;
   char msg[MAX_LEN_MSG];
   struct player *pplayer = &game.players[pinfo->playerno];
-  struct player_research* research = get_player_research(pplayer);
+  struct player_research* research;
 
   sz_strlcpy(pplayer->name, pinfo->name);
 
@@ -1470,6 +1470,8 @@
    * only send the player info out at appropriate times - e.g., while the
    * game is running. */
   new_tech = read_player_info_techs(pplayer, pinfo->inventions);
+  
+  research = get_player_research(pplayer);
 
   poptechup = (research->researching != pinfo->researching
                || research->tech_goal != pinfo->tech_goal);
Index: common/game.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/game.c,v
retrieving revision 1.226
diff -u -r1.226 game.c
--- common/game.c       9 Jul 2005 17:46:08 -0000       1.226
+++ common/game.c       9 Jul 2005 18:21:40 -0000
@@ -331,7 +331,6 @@
 ***************************************************************/
 void game_free(void)
 {
-  clean_players_research();
   game_remove_all_players();
   map_free();
   idex_free();
@@ -466,9 +465,6 @@
   city_list_free(pplayer->cities);
   pplayer->cities = NULL;
 
-  free(pplayer->research);
-  pplayer->research = NULL;
-
   if (is_barbarian(pplayer)) game.info.nbarbarians--;
 }
 
Index: common/player.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/player.c,v
retrieving revision 1.187
diff -u -r1.187 player.c
--- common/player.c     4 Jul 2005 17:48:37 -0000       1.187
+++ common/player.c     9 Jul 2005 18:21:44 -0000
@@ -79,16 +79,6 @@
 }
 
 /***************************************************************
- Fill the structure with some sane values
-***************************************************************/
-void player_research_init(struct player_research* research)
-{
-  memset(research, 0, sizeof(struct player_research));
-  research->tech_goal = A_UNSET;
-  research->changed_from = -1;
-}
-
-/***************************************************************
   In the server you must use server_player_init.  Note that
   this function is matched by game_remove_player() in game.c,
   there is no corresponding player_free() in this file.
@@ -133,8 +123,7 @@
   plr->economic.tax=PLAYER_DEFAULT_TAX_RATE;
   plr->economic.science=PLAYER_DEFAULT_SCIENCE_RATE;
   plr->economic.luxury=PLAYER_DEFAULT_LUXURY_RATE;
-  plr->research = fc_malloc(sizeof(struct player_research));
-  player_research_init(plr->research);
+
   player_limit_to_government_rates(plr);
   spaceship_init(&plr->spaceship);
 
@@ -725,51 +714,11 @@
 }
 
 /****************************************************************************
-  Merges research of two players. This is used by teams
-****************************************************************************/
-void merge_players_research(struct player* p1, struct player* p2)
-{
-  struct player_research* old_research;
-  if (p1->research == p2->research) {
-    return;
-  }
-  old_research = p1->research;
-  players_iterate(aplayer) {
-    if (aplayer->research == old_research) {
-      aplayer->research = p2->research;
-    }
-  } players_iterate_end;
-  free(old_research);
-}
-
-/****************************************************************************
-****************************************************************************/
-void clean_players_research()
-{
-  players_iterate(aplayer) {
-    if (aplayer->research) {
-      players_iterate(bplayer) {
-        if (aplayer == bplayer || aplayer->research != bplayer->research) {
-         continue;
-       }
-       bplayer->research = NULL;
-      } players_iterate_end;
-      free(aplayer->research);
-      aplayer->research = NULL;
-    }
-  } players_iterate_end;
-  
-  players_iterate(aplayer) {
-    aplayer->research = fc_malloc(sizeof (struct player_research));
-    player_research_init(aplayer->research);
-  } players_iterate_end;
-}
-
-/****************************************************************************
   Returns player_research struct of the given player. Note that team
   members share research
 ****************************************************************************/
-struct player_research *get_player_research(const struct player *p1)
+struct player_research *get_player_research(const struct player *plr)
 {
-  return p1->research;
+  assert(plr && plr->team);
+  return &(plr->team->research);
 }
Index: common/player.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/player.h,v
retrieving revision 1.158
diff -u -r1.158 player.h
--- common/player.h     7 Jul 2005 08:34:38 -0000       1.158
+++ common/player.h     9 Jul 2005 18:21:44 -0000
@@ -68,55 +68,6 @@
   int luxury;
 };
 
-struct player_research {
-  /* The number of techs and future techs the player has
-   * researched/acquired. */
-  int techs_researched, future_tech;
-
-  /* Invention being researched in. Valid values for researching are:
-   *  - any existing tech but not A_NONE or
-   *  - A_FUTURE.
-   * In addition A_NOINFO is allowed at the client for enemies.
-   *
-   * bulbs_researched tracks how many bulbs have been accumulated toward
-   * this research target. */
-  Tech_type_id researching;        
-  int bulbs_researched;
-
-  /* If the player changes his research target in a turn, he loses some or
-   * all of the bulbs he's accumulated toward that target.  We save the
-   * original info from the start of the turn so that if he changes back
-   * he will get the bulbs back. */
-  Tech_type_id changed_from;
-  int bulbs_researched_before;
-
-  /* If the player completed a research this turn, this value is turned on
-   * and changing targets may be done without penalty. */
-  bool got_tech;
-
-  struct {
-    /* One of TECH_UNKNOWN, TECH_KNOWN or TECH_REACHABLE. */
-    enum tech_state state;
-
-    /* 
-     * required_techs, num_required_techs and bulbs_required are
-     * cached values. Updated from build_required_techs (which is
-     * called by update_research).
-     */
-    tech_vector required_techs;
-    int num_required_techs, bulbs_required;
-  } inventions[A_LAST];
-
-  /* Tech goal (similar to worklists; when one tech is researched the next
-   * tech toward the goal will be chosen).  May be A_NONE. */
-  Tech_type_id tech_goal;
-
-  /*
-   * Cached values. Updated by update_research.
-   */
-  int num_known_tech_with_flag[TF_LAST];
-};
-
 struct player_score {
   int happy;
   int content;
@@ -225,7 +176,7 @@
   struct city_list *cities;
   struct player_score score;
   struct player_economic economic;
-  struct player_research* research;
+
   int bulbs_last_turn;    /* # bulbs researched last turn only */
   struct player_spaceship spaceship;
   struct player_ai ai;
@@ -247,7 +198,6 @@
   bv_debug debug;
 };
 
-void player_research_init(struct player_research* research);
 void player_init(struct player *plr);
 struct player *find_player_by_name(const char *name);
 struct player *find_player_by_name_prefix(const char *name,
@@ -320,8 +270,6 @@
 
 struct player_research *get_player_research(const struct player *p1);
 
-void merge_players_research(struct player* p1, struct player* p2);
-void clean_players_research(void);
 #define players_iterate(PI_player)                                            \
 {                                                                             \
   struct player *PI_player;                                                   \
Index: common/team.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/team.c,v
retrieving revision 1.5
diff -u -r1.5 team.c
--- common/team.c       24 Jun 2005 04:34:03 -0000      1.5
+++ common/team.c       9 Jul 2005 18:21:44 -0000
@@ -187,5 +187,6 @@
     sz_strlcpy(teams[i].name, names[i]);
 
     teams[i].players = 0;
+    player_research_init(&(teams[i].research));
   }
 }
Index: common/team.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/team.h,v
retrieving revision 1.2
diff -u -r1.2 team.h
--- common/team.h       21 May 2005 19:40:24 -0000      1.2
+++ common/team.h       9 Jul 2005 18:21:44 -0000
@@ -16,11 +16,15 @@
 
 #include "fc_types.h"
 
+#include "tech.h"
+
 #define MAX_NUM_TEAMS (MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS)
 
 struct team {
   Team_type_id index;
   char name[MAX_LEN_NAME];
+  
+  struct player_research research;
 
   int players; /* # of players on the team */
 };
Index: common/tech.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/tech.c,v
retrieving revision 1.98
diff -u -r1.98 tech.c
--- common/tech.c       8 Jul 2005 03:31:18 -0000       1.98
+++ common/tech.c       9 Jul 2005 18:21:48 -0000
@@ -678,3 +678,13 @@
     tech_free(i);
   }
 }
+
+/***************************************************************
+ Fill the structure with some sane values
+***************************************************************/
+void player_research_init(struct player_research* research)
+{
+  memset(research, 0, sizeof(struct player_research));
+  research->tech_goal = A_UNSET;
+  research->changed_from = -1;
+}
Index: common/tech.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/tech.h,v
retrieving revision 1.57
diff -u -r1.57 tech.h
--- common/tech.h       8 Jul 2005 03:31:18 -0000       1.57
+++ common/tech.h       9 Jul 2005 18:21:48 -0000
@@ -109,6 +109,57 @@
 
 BV_DEFINE(tech_vector, A_LAST);
 
+struct player_research {
+  /* The number of techs and future techs the player has
+   * researched/acquired. */
+  int techs_researched, future_tech;
+
+  /* Invention being researched in. Valid values for researching are:
+   *  - any existing tech but not A_NONE or
+   *  - A_FUTURE.
+   * In addition A_NOINFO is allowed at the client for enemies.
+   *
+   * bulbs_researched tracks how many bulbs have been accumulated toward
+   * this research target. */
+  Tech_type_id researching;        
+  int bulbs_researched;
+
+  /* If the player changes his research target in a turn, he loses some or
+   * all of the bulbs he's accumulated toward that target.  We save the
+   * original info from the start of the turn so that if he changes back
+   * he will get the bulbs back. */
+  Tech_type_id changed_from;
+  int bulbs_researched_before;
+
+  /* If the player completed a research this turn, this value is turned on
+   * and changing targets may be done without penalty. */
+  bool got_tech;
+
+  struct {
+    /* One of TECH_UNKNOWN, TECH_KNOWN or TECH_REACHABLE. */
+    enum tech_state state;
+
+    /* 
+     * required_techs, num_required_techs and bulbs_required are
+     * cached values. Updated from build_required_techs (which is
+     * called by update_research).
+     */
+    tech_vector required_techs;
+    int num_required_techs, bulbs_required;
+  } inventions[A_LAST];
+
+  /* Tech goal (similar to worklists; when one tech is researched the next
+   * tech toward the goal will be chosen).  May be A_NONE. */
+  Tech_type_id tech_goal;
+
+  /*
+   * Cached values. Updated by update_research.
+   */
+  int num_known_tech_with_flag[TF_LAST];
+};
+
+void player_research_init(struct player_research* research);
+
 enum tech_state get_invention(const struct player *pplayer,
                              Tech_type_id tech);
 void set_invention(struct player *pplayer, Tech_type_id tech,
Index: server/savegame.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/savegame.c,v
retrieving revision 1.260
diff -u -r1.260 savegame.c
--- server/savegame.c   9 Jul 2005 17:46:09 -0000       1.260
+++ server/savegame.c   9 Jul 2005 18:22:16 -0000
@@ -1738,7 +1738,17 @@
   int id;
   int target_no;
   struct team *pteam;
-  struct player_research *research = get_player_research(plr);
+  struct player_research *research;
+
+  /* not all players have teams */
+  id = secfile_lookup_int_default(file, -1, "player%d.team_no", plrno);
+  pteam = team_get_by_id(id);
+  if (pteam) {
+    /* Players with no team will be assigned to an empty team later. */
+    team_add_player(plr, pteam);
+  }
+
+  research = get_player_research(plr);
 
   server_player_init(plr, TRUE, FALSE);
   ai = ai_data_get(plr);
@@ -1791,14 +1801,6 @@
   /* Add techs from game and nation, but ignore game.info.tech. */
   init_tech(plr, 0);
 
-  /* not all players have teams */
-  id = secfile_lookup_int_default(file, -1, "player%d.team_no", plrno);
-  pteam = team_get_by_id(id);
-  if (pteam) {
-    /* Players with no team will be assigned to an empty team later. */
-    team_add_player(plr, pteam);
-  }
-
   if (is_barbarian(plr)) {
     plr->nation = game.control.nation_count - 1;
   }
Index: server/srv_main.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/srv_main.c,v
retrieving revision 1.276
diff -u -r1.276 srv_main.c
--- server/srv_main.c   29 Jun 2005 02:17:53 -0000      1.276
+++ server/srv_main.c   9 Jul 2005 18:22:23 -0000
@@ -1909,14 +1909,6 @@
    } players_iterate_end;
   }
   
-  players_iterate(pplayer) {
-    players_iterate(pdest) {
-      if (players_on_same_team(pplayer, pdest)
-          && pplayer->player_no != pdest->player_no) {
-       merge_players_research(pplayer, pdest);
-      }
-    } players_iterate_end;
-  } players_iterate_end;
   /* tell the gamelog about the players */
   players_iterate(pplayer) {
     gamelog(GAMELOG_PLAYER, pplayer);