[Freeciv-Dev] Re: shared vision again

[Thue <thue@xxxxxxx>]

On Sun, 27 Aug 2000 14:39:26 David Pfitzner wrote:
> On Wed, 23 Aug 2000 Thue <thue@xxxxxxxxxxxxxxxxxxxxxxxxxx> wrote:
> > 
> > Now you can in the diplomatic dialog agree to a shared vision clause. As
long
> > as the clause is in effect all map info will be passed along. one clause
only
> > gives vision one way.
> 
> > The effect is transitive; if p1 gives info to p2 and p2 to p3, p1 in
effect
> > also gives to p3.
> 
> I'm a bit uncomfortable about having it transitive, but OTOH I can't
> think of a good way to do it otherwise without getting too complicated.

Why would it be bad?
Anyway, I think that part of the code turned out very well :). (ok, so there is
three nested players_iterate inside a do {} while, but it is a very intuitive
nested loop :))

> > +void diplomacy_dialog_vision_callback(GtkWidget *w, gpointer data)
> > +{
> 
> > +  if (!has_capability("shared_vision", aconnection.capability)) {
> > +    append_output_window("clause not added as the server does not"
> > +                    "have the capability");
> > +    return;
> > +  }
> 
> If I understand this, shouldn't the button be disabled (or omitted)
> instead, so the callback is never called and this check is not needed?

Done. I also disabled the buttons when the shared vision was already given.
I left the check, it doesn't hurt.

> > +  players_vision_command=gtk_accelbutton_new(_("_Redraw vision"), accel);
> 
> This label should say instead "Withdraw vision" (Xaw client too).

Done.

> > +static void create_vision_dependensies(void)
> 
> Spelling: dependencies

Ups

> > +void give_shared_vision(struct player *pfrom, struct player *pto)
> > +{
> > +  int save_vision[MAX_NUM_PLAYERS+MAX_NUM_BARBARIANS];
> > +  assert(pfrom != pto);
> > +  if (pfrom->gives_shared_vision & (1<<pto->player_no)) {
> > +    freelog(LOG_ERROR, "tried to give shared vision you already have
given.");
> > +    return;
> > +  }
> 
> For messages like above, I assume you are using these for debugging,
> but wording could be more general to detect strange clients in 
> general case, eg ("%s tried to give shared vision to %s - already given",
> pfrom->name, pto->name).  (Also, could the client trick the server 
> into triggering that assert?  Bad if so.  See also some diplomatic 
> treaty server-checking patches by Andy Black and Marko Lindqvist?)

Done

> > +    freelog(LOG_ERROR, "tried removing shared vision you don't have.");
> 
> (Ditto.)

(Ditto.)

> -- David

Thanks!
The patch also removes a few additional minor issues. And updates to CVS, which
seemed to have moved to much that not much of the patch applied without
failures or at least offsets. (even though it has only been 4 days!)

-Thue

shared_vision.diff.gz
Description: GNU Zip compressed data