Complete.Org: Mailing Lists: Archives: freeciv-dev: October 2002:
[Freeciv-Dev] Re: teamslite9
Home

[Freeciv-Dev] Re: teamslite9

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: "Per I. Mathisen" <per@xxxxxxxxxxx>
Cc: freeciv-dev@xxxxxxxxxxx
Subject: [Freeciv-Dev] Re: teamslite9
From: Raimar Falke <rf13@xxxxxxxxxxxxxxxxx>
Date: Thu, 24 Oct 2002 21:29:11 +0200

On Thu, Oct 24, 2002 at 01:24:26PM +0000, Per I. Mathisen wrote:
> Here is another verison of the teams patch. Changes:
>   - a player's team is now shown in player dialog (gtk1 only)
>   - more style fixes
>   - API changes/renames
>   - changed gamelog so that ranking info for civserver is produced again
>   - added description of what teams are in "help team"
> 
> Please comment and playtest. I need people who know how to port the player
> dialog change to other GUIs.

> @@ -369,7 +403,6 @@
>  {
>    ai_manage_cities(pplayer);
>    /* manage cities will establish our tech_wants. */
> -  /* if I were upgrading units, which I'm not, I would do it here -- Syela 
> */ 

Noise.

> +/***************************************************************
> +  Returns the id of a team given its name, or -1 if not found

s/-1/TEAM_NONE/

> +***************************************************************/
> +Team_Type_id team_find_by_name(const char *name)
> +{

> +  int i;
> +
> +  assert(name != NULL);
> +  for(i = 0; i < game.team_count; i++) {
> +     if(mystrcasecmp(name, team_get_name(i)) == 0) {
> +     return i;
> +     }
> +  }

Use the iterate.

> +/***************************************************************
> +  Returns pointer to a team given its id
> +***************************************************************/

> +struct team_type *team_get_by_idx(Team_Type_id team)

Even while get_nation_by_idx does it so it should be
team_get_by_id. It is an ID and not an index. Even if it is internally
an index.

> +/***************************************************************
> +  Count living members of given team
> +***************************************************************/
> +int team_count_members_alive(Team_Type_id id)
> +{
> +  struct team_type *team = team_get_by_idx(id);
> +  int count = 0;
> +
> +  if (team == NULL) {
> +    return 0;
> +  }
> +  assert(team->team_no < game.team_count
> +         && team->team_no != TEAM_NONE);
> +  players_iterate(pplayer) {
> +    if ((pplayer->is_alive) && (pplayer->team == team->team_no)) {
> +      count++;
> +    }
> +  } players_iterate_end;
> +  return count;

This _can_ be written as:

int count = 0;

if(team!=NULL)
{
...
...
}

return count;

i.e. only one exit.

> +/***************************************************************

> +  Quickly register team info received from server in client.

Quickly? This assumes that there is also a slow way?!

> +/***************************************************************
> +  Set a player to a team. Removes previous team affiliation,
> +  creates a new team if it does not exist.
> +***************************************************************/

> +void team_add_player(struct player *plr, const char *name)

Maybe s/name/team_name/.

The canonical variable name is "pplayer" and not "plr".

> +{
> +  Team_Type_id team_id, i;
> +
> +  assert((plr != NULL) && (name != NULL));
> +
> +  /* find or create team */
> +  team_id = team_find_by_name(name);
> +  if (team_id == TEAM_NONE) {

> +    /* see if we have another team available */
> +    for (i = 0; i < MAX_NUM_TEAMS; i++) {

Iterate. Oops you can't use it here.

> +      if (teams[i].team_no == TEAM_NONE) {
> +        team_id = i;
> +        break;
> +      }
> +    }
> +    /* check if too many teams */

> +    if (team_id == TEAM_NONE) {
> +      assert(FALSE);
> +    }

Whats wrong with assert(team_id!=TEAM_NONE)?

> +    /* add another team */

> +    teams[team_id].team_no = team_id;

Please choose no or id. I'm for id.

> +/***************************************************************
> +  Removes a player from a team, and removes the team if empty of
> +  players
> +***************************************************************/
> +void team_remove_player(struct player *plr)
> +{
> +  Team_Type_id i;

> +  int count=0;

Missing whitespace.

> +  assert((plr != NULL) && (plr->team < MAX_NUM_TEAMS));

Check that this is only called in pregame. Since you stated above that
the client trusts that the team association doesn't change during a
game.

> +  if (plr->team == TEAM_NONE) {
> +    return;
> +  }
> +  /* anyone else using my team? */
> +  for (i = 0; i < MAX_NUM_TEAMS; i++) {

> +    if ((teams[i].team_no == plr->team)
> +        && (plr->team != i)) {
> +      count++;
> +    }

Here and in some other places you use extensively parentheses. This is
not bad per se. But usually (not sure about this, I don't have a
statistics handy) we omit them.

Another point: count can be bool and after it is set you can break the
loop (this is also for some other loops).

> +  /* no other team members left? remove team */
> +  if (count == 0) {
> +    teams[plr->team].team_no = TEAM_NONE;
> +    game.team_count--;
> +  }
> +  plr->team = TEAM_NONE;
> +}

> +/***************************************************************
> +  Initializes team structure
> +***************************************************************/
> +void team_init()
> +{
> +  Team_Type_id i;
> +
> +  for (i = 0; i < MAX_NUM_TEAMS; i++) {
> +    /* mark as unused */
> +    teams[i].team_no = TEAM_NONE;
> +  }

assert(TEAM_NONE<0 || TEAM_NONE>=MAX_NUM_TEAMS);

> @@ -857,7 +861,6 @@
>    SEND_PACKET_END;
>  }
>  
> -

noise.


> retrieving revision 1.126
> diff -u -r1.126 packets.h
> --- common/packets.h  2002/10/19 04:02:07     1.126
> +++ common/packets.h  2002/10/24 13:21:30
> @@ -460,6 +460,8 @@
>    int playerno;
>    char name[MAX_LEN_NAME];
>    bool is_male;
> +  int team;

> +  char team_name[MAX_LEN_NAME];

Why? Base question: is it possible that the team of a player changes
or not?

>    int government;
>    int embassy;
>    int city_style;
> Index: common/player.c
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/common/player.c,v
> retrieving revision 1.100
> diff -u -r1.100 player.c
> --- common/player.c   2002/09/23 22:47:10     1.100
> +++ common/player.c   2002/10/24 13:21:30
> @@ -71,6 +71,7 @@
>    plr->is_male = TRUE;
>    plr->government=game.default_government;
>    plr->nation=MAX_NUM_NATIONS;
> +  plr->team = TEAM_NONE;
>    plr->capital = FALSE;
>    unit_list_init(&plr->units);
>    city_list_init(&plr->cities);
> Index: common/player.h
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/common/player.h,v
> retrieving revision 1.82
> diff -u -r1.82 player.h
> --- common/player.h   2002/09/23 22:47:10     1.82
> +++ common/player.h   2002/10/24 13:21:30
> @@ -160,6 +160,7 @@
>    bool is_male;
>    int government;
>    Nation_Type_id nation;
> +  Team_Type_id team;
>    bool turn_done;
>    int nturns_idle;
>    bool is_alive;
> Index: server/diplhand.c
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/server/diplhand.c,v
> retrieving revision 1.63
> diff -u -r1.63 diplhand.c
> --- server/diplhand.c 2002/09/22 19:21:30     1.63
> +++ server/diplhand.c 2002/10/24 13:21:30
> @@ -393,7 +393,6 @@
>    }
>  }
>  
> -

Noise.

> Index: server/gamelog.c
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/server/gamelog.c,v
> retrieving revision 1.20
> diff -u -r1.20 gamelog.c
> --- server/gamelog.c  2002/09/29 18:33:12     1.20
> +++ server/gamelog.c  2002/10/24 13:21:30
> @@ -67,16 +67,20 @@
>    
>    va_start(args, message);
>    my_vsnprintf(buf, sizeof(buf), message, args);
> -  if (level==GAMELOG_MAP){
> -    if (buf[0] == '(')  /* KLUGE!! FIXME: remove when we fix the gamelog 
> format --jjm */

> +  if (level==GAMELOG_MAP) {

Missing whitespace.

> @@ -85,7 +89,6 @@
>    fclose(fs);
>  }
>  
> -

Noise.

> +  /* average game scores for teams */
> +  team_iterate(team) {
> +    int numplayers = 0;
> +    int count = 0;

Looks like count should be renamed to num_total_players and numplayers
to num_players_in_team.

> Index: server/plrhand.c
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/server/plrhand.c,v
> retrieving revision 1.246
> diff -u -r1.246 plrhand.c
> --- server/plrhand.c  2002/10/09 14:10:17     1.246
> +++ server/plrhand.c  2002/10/24 13:21:30
> @@ -797,7 +797,7 @@
>    struct player *pplayer2;
>    int reppenalty = 0;
>    bool has_senate;
> -    
> +
>    if (other_player < 0 || other_player >= game.nplayers) {
>      return;
>    }

Noise.

> Index: server/savegame.c
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/server/savegame.c,v
> retrieving revision 1.92
> diff -u -r1.92 savegame.c
> --- server/savegame.c 2002/09/27 12:32:47     1.92
> +++ server/savegame.c 2002/10/24 13:21:30
> @@ -577,6 +577,15 @@
>    sz_strlcpy(plr->username,
>            secfile_lookup_str_default(file, "", "player%d.username", plrno));
>    plr->nation=secfile_lookup_int(file, "player%d.race", plrno);
> +  /* not all players have teams */
> +  if (section_file_lookup(file, "player%d.team", plrno)) {
> +    char tmp[MAX_LEN_NAME];

Missing empty line.

> +  /* quit if all players are allied to each other */
> +  all_allied = TRUE;
> +  players_iterate(pplayer) {
> +    players_iterate(aplayer) {
> +      if (!pplayers_allied(pplayer, aplayer)) {
> +        all_allied = FALSE;

Break out early.

> +      }
> +    } players_iterate_end;
> +  } players_iterate_end;
> +  if (all_allied) {
> +    notify_conn(&game.est_connections, _("Game ended in allied victory"));
> +    gamelog(GAMELOG_NORMAL, _("Game ended in allied victory"));
> +    gamelog(GAMELOG_TEAM, "ALLIEDVICTORY");
> +    return TRUE;
> +  }
> +
> +  return FALSE;
>  }

>  /**************************************************************************
> @@ -533,14 +584,14 @@
>  **************************************************************************/
>  void start_game(void)
>  {
> -  if(server_state!=PRE_GAME_STATE) {
> +  if(server_state != PRE_GAME_STATE) {
>      con_puts(C_SYNTAX, _("The game is already running."));
>      return;
>    }
>  
>    con_puts(C_OK, _("Starting game."));
>  
> -  server_state=SELECT_RACES_STATE; /* loaded ??? */
> +  server_state = SELECT_RACES_STATE; /* loaded ??? */
>    force_end_of_sniff = TRUE;
>  }
>  

Noise.

> @@ -1976,6 +2027,20 @@
>      if(map.num_start_positions==0) {
>        create_start_positions();
>      }
> +  }
> +
> +  /* Set up alliances based on team selections */
> +  if (game.is_new_game && game.team_count > 0) {
> +   players_iterate(pplayer) {
> +     players_iterate(pdest) {
> +      if ((pplayer->team == pdest->team)
> +          && (pplayer->player_no != pdest->player_no)) {
> +        pplayer->diplstates[pdest->player_no].type = DS_ALLIANCE;
> +        give_shared_vision(pplayer, pdest);
> +        pplayer->embassy |= (1 << pdest->player_no);
> +      }
> +    } players_iterate_end;
> +   } players_iterate_end;
>    }
>  
>    initialize_move_costs(); /* this may be the wrong place to do this */
> Index: server/stdinhand.c
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/server/stdinhand.c,v
> retrieving revision 1.257
> diff -u -r1.257 stdinhand.c
> --- server/stdinhand.c        2002/10/16 18:40:54     1.257
> +++ server/stdinhand.c        2002/10/24 13:21:31
> @@ -946,6 +946,7 @@
>    
>    /* mostly non-harmful: */
>    CMD_SET,
> +  CMD_TEAM,
>    CMD_FIX,
>    CMD_UNFIX,
>    CMD_RULESETDIR,
> @@ -1072,6 +1073,14 @@
>     N_("set <option-name> <value>"),
>     N_("Set server option."), NULL
>    },
> +  {"team",   ALLOW_CTRL,
> +   N_("team <player> [team]"),
> +   N_("Change, add or remove a player's team affiliation."),
> +   N_("Sets a player as member of a team. If no team specified, the "
> +      "player is set teamless. Use \"\" if names contain whitespace. "
> +      "A team is a group of players that start out allied and fight "
> +      "together to achieve team victory.")

"start out allied, with shared vision and an embassy with each
other. Also they get each tech research by a team member at the end of
the turn."

> @@ -1730,7 +1739,7 @@
>  {
>    struct player *pplayer;
>    PlayerNameStatus PNameStatus;
> -   
> +
>    if (server_state!=PRE_GAME_STATE)
>    {
>      cmd_reply(CMD_CREATE, caller, C_SYNTAX,

Noise.

> @@ -2658,6 +2667,55 @@
>  /******************************************************************
>    ...
>  ******************************************************************/
> +static void team_command(struct connection *caller, char *str) 
> +{
> +  struct player *pplayer;
> +  enum m_pre_result match_result;
> +  char buf[MAX_LEN_CONSOLE_LINE];
> +  char *arg[2];
> +  int ntokens = 0;
> +

> +  if (server_state!=PRE_GAME_STATE) {

Whitespace.

> +    cmd_reply(CMD_TEAM, caller, C_SYNTAX,
> +              _("Cannot change teams once game has begun."));
> +    return;
> +  }
> +
> +  if (str != NULL || strlen(str) > 0) {

More strict checking. " " for example shouldn't be allowed.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
  "Windows is the one true OS. MS invented the GUI. MS invented 
   the 32 bit OS. MS is open and standard. MS loves you. We have 
   always been at war with Oceana."


[Prev in Thread] Current Thread [Next in Thread]