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: Fri, 25 Oct 2002 22:54:32 +0200

On Thu, Oct 24, 2002 at 11:09:35PM +0000, Per I. Mathisen wrote:
> On Thu, 24 Oct 2002, Raimar Falke wrote:
> > Noise.
> 
> Well, it fixes stuff. It shouldn't really be there, but it makes the code
> better and it takes effort to remove.

Than make a separate patch which is applied before or after it.

> > > +Quickly register team info received from server in client.
> >
> > Quickly? This assumes that there is also a slow way?!
> 
> Yes:
> 
> > > +/***************************************************************
> > > +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)
> 
> I _could_ use this also in the client, but that would be seriously slow.

I don't see a problem if you change:

+  if (has_capability("team", aconnection.capability)
+      && pinfo->team != TEAM_NONE) {
+    team_register(pplayer, pinfo->team, pinfo->team_name);
+  } else {
+    pplayer->team = TEAM_NONE;
+  }

to

+  if (has_capability("team", aconnection.capability)
+      && pinfo->team != TEAM_NONE) {
     if(pinfo->team != pplayer->team) {
       team_add_player(pplayer, pinfo->team_name);
     }
+  } else {
+    pplayer->team = TEAM_NONE;
+  }

So you can remove team_register.

> > 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.
> 
> I did? The client part is written so that it works no matter how and when
> you change it in the server. The server part would work fine from this
> part of the API's perspective - the problem lies higher up in the API
> hierarchy (changing alliances etc).
> 
> > > --- 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?
> 
> The low-level API is written so that team changing is possible, although
> this is unintentional - I do not think teams should change after pregame.

It is ok if you are this flexible. But in this case the flexibility
comes with a price (team name in every player_info). And as you stated
you don't use this flexibility in the upper layers.

> > > +if (str != NULL || strlen(str) > 0) {
> >
> > More strict checking. " " for example shouldn't be allowed.
> 
> Good point. But it isn't just " " but also "  " and maybe other invisible
> characters as well? Is there a convenient way to test this?

common/shared:get_sane_name

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "The BeOS takes the best features from the major operating systems. 
  It's got the power and flexibility of Unix, the interface and ease 
  of use of the MacOS, and Minesweeper from Windows."


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