[Freeciv-Dev] Re: [Patch] for to players_iterate

[Gregory Berkolaiko <gberkolaiko@xxxxxxxxxxx>]

Had a carefull look at it.
Seems absolutely fine, but we should wait for Tony to explain the aplayer
bussiness.  As for the rest, the only real change is use of
num_human_ai_players, isn't it.

Best,
G.

 --- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote: 
> On Wed, Feb 20, 2002 at 11:41:53AM +0000, Gregory Berkolaiko wrote:
> >  --- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote: > 
> > > The attached patch replaces loops like
> > > 
> > >   for(i=0; i<game.nplayers; i++)
> > > 
> > > with players_iterate. 90% are straightforward. Can somebody look at
> > > the change of ai_manage_diplomat? Especially what 
> > > 
> > >    if (!ctarget && aplayer) {
> > > 
> > > should mean? The loop in handle_unit_enter_city and one in
> > 
> > The use of aplayer seems severely wrong to me.
> > Tony wrote it, ask him, but I think you are doing the right thing.
> > 
> > 
> > As for the other issues, could you split your patch please (into
> > stratightforward and structural changes).
> > I don't want to spend hours trying to find the few real changes in the sea
> of
> > straightforward ones.
> 
> Here are the real changes.
> 
>       Raimar
> 
> -- 
>  email: rf13@xxxxxxxxxxxxxxxxx
>  "USENET is *not* the non-clickable part of WWW!"
> > ? diff
> ? test.c
> ? players_iterate1.diff
> Index: ai/aiunit.c
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/ai/aiunit.c,v
> retrieving revision 1.175
> diff -u -r1.175 aiunit.c
> --- ai/aiunit.c       2002/02/21 09:44:50     1.175
> +++ ai/aiunit.c       2002/02/21 10:25:33
> @@ -2170,7 +2170,6 @@
>   */
>  static void ai_manage_diplomat(struct player *pplayer, struct unit
> *pdiplomat)
>  {
> -  int i;
>    bool handicap, has_emb;
>    int continent, dist, rmd, oic, did;
>    struct packet_unit_request req;
> @@ -2178,7 +2177,6 @@
>    struct city *pcity, *ctarget;
>    struct tile *ptile;
>    struct unit *ptres;
> -  struct player *aplayer = NULL;
>  
>    if (pdiplomat->activity != ACTIVITY_IDLE)
>      handle_unit_activity_request(pdiplomat, ACTIVITY_IDLE);
> @@ -2232,8 +2230,7 @@
>      dist=MAX(map.xsize, map.ysize);
>      continent=map_get_continent(pdiplomat->x, pdiplomat->y);
>      handicap = ai_handicap(pplayer, H_TARGETS);
> -    for( i = 0; i < game.nplayers; i++) {
> -      aplayer = &game.players[i];
> +    players_iterate(aplayer) {
>        if (aplayer == pplayer) continue;
>        /* sneaky way of avoiding foul diplomat capture  -AJS */
>        has_emb=player_has_embassy(pplayer, aplayer) || pdiplomat->foul;
> @@ -2254,10 +2251,14 @@
>         }
>       }
>        city_list_iterate_end;
> -    }
> -    if (!ctarget && aplayer) {
> +    } players_iterate_end;
> +
> +    assert(game.nplayers > 0);
> +    assert(pplayer == &game.players[pplayer->player_no]);
> +
> +    if (!ctarget) {
>        /* No enemy cities are useful.  Check our own. -AJS */
> -      city_list_iterate(aplayer->cities, acy)
> +      city_list_iterate(pplayer->cities, acy)
>       if (continent != map_get_continent(acy->x, acy->y)) continue;
>       if (!count_diplomats_on_tile(acy->x, acy->y)) {
>         ctarget=acy;
> Index: server/citytools.c
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/server/citytools.c,v
> retrieving revision 1.165
> diff -u -r1.165 citytools.c
> --- server/citytools.c        2002/02/21 09:44:52     1.165
> +++ server/citytools.c        2002/02/21 10:25:35
> @@ -1184,7 +1184,6 @@
>  **************************************************************************/
>  void handle_unit_enter_city(struct unit *punit, struct city *pcity)
>  {
> -  int i, n;
>    bool do_civil_war = FALSE;
>    int coins;
>    struct player *pplayer = unit_owner(punit);
> @@ -1206,16 +1205,13 @@
>      spaceship_lost(cplayer);
>    }
>    
> -  if(city_got_building(pcity, B_PALACE) 
> -     && city_list_size(&cplayer->cities) >= game.civilwarsize 
> -     && game.nplayers < game.nation_count
> -     && game.civilwarsize < GAME_MAX_CIVILWARSIZE) {
> -    n = 0;
> -    for( i = 0; i < game.nplayers; i++ )
> -      if(!is_barbarian(&game.players[i]))
> -     n++;
> -    if(n < MAX_NUM_PLAYERS && civil_war_triggered(cplayer))
> -      do_civil_war = TRUE;
> +  if (city_got_building(pcity, B_PALACE)
> +      && city_list_size(&cplayer->cities) >= game.civilwarsize
> +      && game.nplayers < game.nation_count
> +      && game.civilwarsize < GAME_MAX_CIVILWARSIZE
> +      && get_num_human_and_ai_players() < MAX_NUM_PLAYERS
> +      && civil_war_triggered(cplayer)) {
> +    do_civil_war = TRUE;
>    }
>  
>    /* 
> Index: server/meta.c
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/server/meta.c,v
> retrieving revision 1.47
> diff -u -r1.47 meta.c
> --- server/meta.c     2002/02/21 09:44:53     1.47
> +++ server/meta.c     2002/02/21 10:25:35
> @@ -274,7 +274,7 @@
>  {
>    static struct timer *time_since_last_send = NULL;
>    char desc[4096], info[4096];
> -  int num_nonbarbarians = 0;
> +  int num_nonbarbarians = get_num_human_and_ai_players();
>  
>    if (reset_timer && time_since_last_send)
>    {
> @@ -292,12 +292,6 @@
>    if (!time_since_last_send) {
>      time_since_last_send = new_timer(TIMER_USER, TIMER_ACTIVE);
>    }
> -
> -  players_iterate(pplayer) {
> -    if (!is_barbarian(pplayer)) {
> -      ++num_nonbarbarians;
> -    }
> -  } players_iterate_end;
>  
>    /* build description block */
>    desc[0]='\0';
> Index: server/srv_main.c
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/server/srv_main.c,v
> retrieving revision 1.65
> diff -u -r1.65 srv_main.c
> --- server/srv_main.c 2002/02/21 09:44:54     1.65
> +++ server/srv_main.c 2002/02/21 10:25:36
> @@ -238,17 +238,16 @@
>  **************************************************************************/
>  static void do_apollo_program(void)
>  {
> -  int cityid;
> -  struct player *pplayer;
> -  struct city *pcity;
> -  if ((cityid=game.global_wonders[B_APOLLO]) && 
> -      (pcity=find_city_by_id(cityid))) {
> -    pplayer=city_owner(pcity);
> +  struct city *pcity = find_city_wonder(B_APOLLO);
> +
> +  if (pcity) {
> +    struct player *pplayer = city_owner(pcity);
> +
>      if (game.civstyle == 1) {
>        players_iterate(other_player) {
> -     city_list_iterate(other_player->cities, pcity)
> +     city_list_iterate(other_player->cities, pcity) {
>         show_area(pplayer, pcity->x, pcity->y, 0);
> -     city_list_iterate_end;
> +     } city_list_iterate_end;
>        } players_iterate_end;
>      } else {
>        map_know_all(pplayer);
> @@ -263,12 +262,13 @@
>  **************************************************************************/
>  static void marco_polo_make_contact(void)
>  {
> -  int cityid  = game.global_wonders[B_MARCO];
> -  int o;
> -  struct city *pcity;
> -  if (cityid && (pcity = find_city_by_id(cityid)))
> -    for (o = 0; o < game.nplayers; o++)
> -      make_contact(city_owner(pcity), get_player(o), pcity->x, pcity->y);
> +  struct city *pcity = find_city_wonder(B_MARCO);
> +
> +  if (pcity) {
> +    players_iterate(pplayer) {
> +      make_contact(city_owner(pcity), pplayer, pcity->x, pcity->y);
> +    } players_iterate_end;
> +  }
>  }
>  
>  /**************************************************************************
> @@ -311,38 +311,38 @@
>  **************************************************************************/
>  static void update_diplomatics(void)
>  {
> -  int p, p2;
> +  players_iterate(player1) {
> +    players_iterate(player2) {
> +      struct player_diplstate *pdiplstate =
> +       &player1->diplstates[player2->player_no];
>  
> -  for(p = 0; p < game.nplayers; p++) {
> -    for(p2 = 0; p2 < game.nplayers; p2++) {
> -      game.players[p].diplstates[p2].has_reason_to_cancel =
> -     MAX(game.players[p].diplstates[p2].has_reason_to_cancel - 1, 0);
> +      pdiplstate->has_reason_to_cancel =
> +       MAX(pdiplstate->has_reason_to_cancel - 1, 0);
>  
> -      if(game.players[p].diplstates[p2].type == DS_CEASEFIRE) {
> -     switch(--game.players[p].diplstates[p2].turns_left) {
> +      if(pdiplstate->type == DS_CEASEFIRE) {
> +     switch(--pdiplstate->turns_left) {
>       case 1:
> -       notify_player(&game.players[p],
> +       notify_player(player1,
>                       _("Game: Concerned citizens point "
> -                       "out that the cease-fire with %s will run out soon."),
> -                     game.players[p2].name);
> -       break;
> -     case -1:
> -       notify_player(&game.players[p],
> -                     _("Game: The cease-fire with %s has "
> -                       "run out. You are now neutral towards the %s."),
> -                     game.players[p2].name,
> -                     get_nation_name_plural(game.players[p2].nation));
> -       game.players[p].diplstates[p2].type = DS_NEUTRAL;
> -       check_city_workers(&game.players[p]);
> -       check_city_workers(&game.players[p2]);
> -       break;
> -     }
> -      }
> -      game.players[p].reputation =
> -     MIN(game.players[p].reputation + GAME_REPUTATION_INCR,
> -         GAME_MAX_REPUTATION);
> -    }
> -  }
> +                       "out that the cease-fire with %s will run out soon."),
> +                     player2->name);
> +       break;
> +     case -1:
> +       notify_player(player1,
> +                     _("Game: The cease-fire with %s has "
> +                       "run out. You are now neutral towards the %s."),
> +                     player2->name,
> +                     get_nation_name_plural(player2->nation));
> +       pdiplstate->type = DS_NEUTRAL;
> +       check_city_workers(player1);
> +       check_city_workers(player2);
> +       break;
> +     }
> +        }
> +      player1->reputation = MIN(player1->reputation + GAME_REPUTATION_INCR,
> +                             GAME_MAX_REPUTATION);
> +    } players_iterate_end;
> +  } players_iterate_end;
>  }
>  
>  /**************************************************************************
> @@ -1860,22 +1860,27 @@
>     * in generate_ai_players() later
>     */
>    server_state = RUN_GAME_STATE;
> -  for(i=0; i<game.nplayers; i++) {
> -    if (game.players[i].nation == MAX_NUM_NATIONS &&
> !game.players[i].ai.control) {
> -      send_select_nation(&game.players[i]);
> +  players_iterate(pplayer) {
> +    if (pplayer->nation == MAX_NUM_NATIONS && !pplayer->ai.control) {
> +      send_select_nation(pplayer);
>        server_state = SELECT_RACES_STATE;
>      }
> -  }
> +  } players_iterate_end;
>  
>    while(server_state==SELECT_RACES_STATE) {
> +    bool flag = FALSE;
> +
>      sniff_packets();
> -    for(i=0; i<game.nplayers; i++) {
> -      if (game.players[i].nation == MAX_NUM_NATIONS &&
> !game.players[i].ai.control) {
> +
> +    players_iterate(pplayer) {
> +      if (pplayer->nation == MAX_NUM_NATIONS && !pplayer->ai.control) {
> +     flag = TRUE;
>       break;
>        }
> -    }
> -    if(i==game.nplayers) {
> -      if(i>0) {
> +    } players_iterate_end;
> +
> +    if(!flag) {
> +      if (game.nplayers > 0) {
>       server_state=RUN_GAME_STATE;
>        } else {
>       con_write(C_COMMENT,
>  

__________________________________________________
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