[Freeciv-Dev] Re: civserver segfault with new research system (PR#1221)

[Gregory Berkolaiko <gberkolaiko@xxxxxxxxxxx>]

 --- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote: 
> 
> And here is the patch. Please test since I added another assert since

Oy-voy-voy!  Just look what happens if tech_leakage = 2 and you have an
embassy with one player who happens to possess some technologies.  The
cost for all these technologies will be 0!!!

>   case 2:
>     {
>       int players = 0, players_with_tech = 0;
> 
>       /* Find out how many players have tech */
>       players_iterate(other) {
>         if (!player_has_embassy(pplayer, other)) {
>           continue;
>         }
> 
>         players++;
>         if (get_invention(other, tech) == TECH_KNOWN) {
>           players_with_tech++;
>         }
>       } players_iterate_end;
>  
>       if (players > 0) {
>         /* The player has an embassy with at least one other player */
>         cost = ((players - players_with_tech) * cost) / players;
>       }
>     }
>     break;

Well, actually not 0 but 1, but it's the same thing.
You probably want to initialise players to 1 
(and then remove if(players > 0) condition).

But even so, leakage mode number 2 makes little sense.
Much more sensible IMO is to have

int players = get_num_human_and_ai_players();
cost *= (players - players_with_tech_and_embassy) / players;

That is the only difference from mode 1 is that only those with whom we
have embassy count towards players_with_tech.


> we have to be sure that no barbarian contributes to
> game.global_advances.

Cannot test, but it makes sense.

Best, 
G.


> 
>       Raimar
> 
> -- 
>  email: rf13@xxxxxxxxxxxxxxxxx
>  "The two rules for success in life are:
>   1) Never tell them everything you know."
> > ? rc
> ? diff
> ? whole_diff
> ? msgfmt1.diff
> ? tech_info1.diff
> ? no_tech_loss1.diff
> ? local_warmap131201.diff
> ? queue
> ? unit_move_turns1.diff
> Index: common/tech.c
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/common/tech.c,v
> retrieving revision 1.32
> diff -u -r1.32 tech.c
> --- common/tech.c     2002/01/11 13:50:49     1.32
> +++ common/tech.c     2002/01/14 09:17:05
> @@ -53,6 +53,8 @@
>  void set_invention(struct player *pplayer, Tech_Type_id tech,
>                  enum tech_state value)
>  {
> +  assert(!is_barbarian(pplayer));
> +
>    if (pplayer->research.inventions[tech].state == value) {
>      return;
>    }
> @@ -344,31 +346,36 @@
> 
>
**************************************************************************/
>  int base_total_bulbs_required(struct player *pplayer, Tech_Type_id
> tech)
>  {
> -  int cost;
> -
> -  assert(!is_barbarian(pplayer));
> +  int cost, tech_cost_style = game.rgame.tech_cost_style;
>  
>    if (get_invention(pplayer, tech) == TECH_KNOWN) {
>      return 0;
>    }
> +
> +  if (tech > game.num_tech_types) {
> +    /* Future techs use style 0 */
> +    tech_cost_style = 0;
> +  }
> +
> +  if (tech_cost_style == 2 && advances[tech].preset_cost == -1) {
> +    /* No preset, using style 1 */
> +    tech_cost_style = 1;
> +  }
>  
> -  if (tech > game.num_tech_types || game.rgame.tech_cost_style == 0) {
> -    /* Future tech or cost_style 0 */
> +  switch (tech_cost_style) {
> +  case 0:
>      cost = pplayer->research.techs_researched * game.researchcost;
> -  } else if (game.rgame.tech_cost_style == 1
> -          || (game.rgame.tech_cost_style == 2
> -              && advances[tech].preset_cost == -1)) {
> -    /* cost_style 1 or cost_style 2 with unset value */
> +    break;
> +  case 1:
>      cost = advances[tech].num_reqs * game.researchcost;
> -  } else if (game.rgame.tech_cost_style == 2) {
> -    /* cost_style 2 with set value */
> -    cost =
> -     advances[tech].preset_cost * game.researchcost /
> +    break;
> +  case 2:
> +    cost = (advances[tech].preset_cost * game.researchcost) /
>       GAME_DEFAULT_RESEARCHCOST;
> -  } else {
> -    /* other */
> -    freelog(LOG_ERROR, "Invalid tech_cost_style %d",
> -         game.rgame.tech_cost_style);
> +    break;
> +  default:
> +    freelog(LOG_ERROR, "Invalid tech_cost_style %d %d",
> +         game.rgame.tech_cost_style, tech_cost_style);
>      assert(0);
>      exit(1);
>    }
> @@ -378,28 +385,47 @@
>      cost *= 2;
>    }
>  
> -  if (game.rgame.tech_leakage == 0) {
> +  switch (game.rgame.tech_leakage) {
> +  case 0:
>      /* no change */
> -  } else if (game.rgame.tech_leakage == 1) {
> -    int players = get_num_human_and_ai_players();
> -    cost = ((players - game.global_advances[tech]) * cost) / players;
> -  } else if (game.rgame.tech_leakage == 2) {
> -    int players = 0, players_with_tech = 0;
> -
> -    /* Find out how many players have tech */
> -    players_iterate(other) {
> -      if (!player_has_embassy(pplayer, other)) {
> -     continue;
> +    break;
> +
> +  case 1:
> +    {
> +      int players = get_num_human_and_ai_players();
> +      if (players > 0) {
> +     /* Every non barbarian-only game */
> +     assert(players >= game.global_advances[tech]);
> +     cost = ((players - game.global_advances[tech]) * cost) / players;
>        }
> +    }
> +    break;
>  
> -      players++;
> -      if (get_invention(other, tech) == TECH_KNOWN) {
> -     players_with_tech++;
> +  case 2:
> +    {
> +      int players = 0, players_with_tech = 0;
> +
> +      /* Find out how many players have tech */
> +      players_iterate(other) {
> +     if (!player_has_embassy(pplayer, other)) {
> +       continue;
> +     }
> +
> +     players++;
> +     if (get_invention(other, tech) == TECH_KNOWN) {
> +       players_with_tech++;
> +     }
>        }
> -    } players_iterate_end;
> +      players_iterate_end;
>  
> -    cost = ((players - players_with_tech) * cost) / players;
> -  } else {
> +      if (players > 0) {
> +     /* The player has an embassy with at least one other player */
> +     cost = ((players - players_with_tech) * cost) / players;
> +      }
> +    }
> +    break;
> +
> +  default:
>      freelog(LOG_ERROR, "Invalid tech_leakage %d",
> game.rgame.tech_leakage);
>      assert(0);
>      exit(1);
>  

__________________________________________________
Do You Yahoo!?
Everything you'll ever need on one web page
from News and Sport to Email and Music Charts
http://uk.my.yahoo.com