[Freeciv-Dev] Re: (PR#2413) ai diplomacy patch

["Jason Short via RT" <rt@xxxxxxxxxxxxxx>]

Per I. Mathisen via RT wrote:
> This is a new version of the AI diplomacy patch. Thanks a lot to Jordi for
> finding bugs and thus encouraging me to work on this patch again :-)

> +/* FIXME: these should be set by difficulty and civworld */
> +enum diplomacy_type {
> +  DIP_NONE=0,           /* No attitude */
> +  DIP_PACIFIST=1,       /* War desire -50% if positive */
> +  DIP_MILITARIST=2,     /* War desire +50% if positive */
> +  DIP_TRADER=4,         /* Worth of our offer -25%, trade cities */
> +  DIP_GREEDY=8,         /* Worth of our offer +50% */
> +  DIP_XENOPHOBE=16,     /* Only ever agrees to ceasefire and tech */
> +  DIP_TRUSTWORTHY=32,   /* Does not break treaties unless violated */
> +  DIP_BACKSTABBER=64,   /* Happily agrees to anything and then... */
> +  DIP_MEGALOMANIAC=128, /* Does not share victory */
> +};

Style issues...

>  struct ai_data {
> +  /* AI diplomacy */
> +  struct {
> +    enum winning_strategy strategy;
> +    int timer; /* pursue our goals with some stubbornness */
> +    struct {
> +      int war_desire; /* desire for war or peace */
> +      int war_fear;   /* fear of this player's military might */
> +      int spam;       /* timer to avoid spamming a player with chat */
> +    } other[MAX_NUM_PLAYERS];
> +    struct player *target;    /* concentrate on this player */
> +    int mil_strength;         /* Estimate of our military strength */
> +    int personality;          /* Personality traits */
> +  } diplomacy;

What is timer?  Is this measured in turns, or seconds?

> Index: client/gui-gtk/plrdlg.c
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/plrdlg.c,v
> retrieving revision 1.41
> diff -u -r1.41 plrdlg.c
> --- client/gui-gtk/plrdlg.c   2002/11/14 09:14:54     1.41
> +++ client/gui-gtk/plrdlg.c   2002/11/25 22:49:50
> @@ -488,10 +488,11 @@
>    if (pplayer->is_alive 
>        && pplayer != game.player_ptr
>        && player_has_embassy(game.player_ptr, pplayer)) {
> -    if (pplayer->is_connected)
> +    if (pplayer->is_connected || pplayer->ai.control) {
>        gtk_widget_set_sensitive(players_meet_command, TRUE);
> -    else
> +    } else {
>        gtk_widget_set_sensitive(players_meet_command, FALSE);
> +    }
>      gtk_widget_set_sensitive(players_int_command, TRUE);
>      return;
>    }

IMO a new function, can_meet_with_player (or similar), should exist. 
Then this just becomes
   gtk_widget_set_sensitive(players_meet_command,
                            can_meet_with_player());
and something similar works for other GUIs.  This change can be made 
soon (before the main patch), so that future issues like the gui-gtk-2.0 
one don't crop up.

And shouldn't this information check game.ai_diplomacy?  Is that 
information available to the client?

Finally, what about MUI?

> Index: common/game.h
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/common/game.h,v
> retrieving revision 1.116
> diff -u -r1.116 game.h
> --- common/game.h     2002/11/11 10:00:47     1.116
> +++ common/game.h     2002/11/25 22:49:51
> @@ -80,6 +80,7 @@
>    int researchcost; /* Multiplier on cost of new research */
>    int diplcost, freecost, conquercost;
>    int diplchance;
> +  bool diplomacy;
>    int cityfactor;
>    int citymindist;
>    int civilwarsize;

IMO ai_diplomacy is a better name.  But why would anyone want to disable 
this?  Just because it might be buggy at first?

>  /***************************************************************
> +returns true iff players are allied or at peace
> +***************************************************************/
> +bool pplayers_in_peace(const struct player *pplayer,
> +                 const struct player *pplayer2)
> +{
> +  enum diplstate_type ds = pplayer_get_diplstate(pplayer, pplayer2)->type;
> +  if (pplayer == pplayer2)
> +    return TRUE;
> +  if (is_barbarian(pplayer) || is_barbarian(pplayer2))
> +    return FALSE;
> +  return (ds == DS_ALLIANCE || ds == DS_PEACE);
> +}

More style issues - and this could be written better, I think.

I have no comments yet on the bulk of the code in advdiplomacy.c.  But I 
will :-).

jason