[freeciv-ai] Re: definitely last version of active diplomats patch

[Raimar Falke <rf13@xxxxxxxxxxxxxxxxx>]

On Sun, Sep 08, 2002 at 03:33:56PM +0000, Per I. Mathisen wrote:
> On Fri, 6 Sep 2002, Raimar Falke wrote:
> > Can you please describe how the AI uses diplomats (which functions are
> > called in which order and do which tasks)? Can you also write down the
> > limitations and assumptions?
> 
> It calls ai_manage_diplomat(punit) for each diplomat each time the AI
> activates (which is twice per turn). Then you just follow the logic in
> that function. Note that it works differently if we are following a goto
> or not, which was set previously by another call to ai_manage_diplomat,
> and this goto is checked differently depending on our ai_role.
> 
> The ai_choose_diplomat_offensive and ai_choose_diplomat_defensive are
> called from the build code once per turn per city.

Not to me but write an entry in HACKING or README.ai

> > BTW: the patch is big. IMHO too big to really read is carefully.
> 
> That happens.
> 
> > > +  if (pplayer->player_no == acity->original) {
> > > +    incite_cost /= 2;
> > > +  }
> >
> > Why isn't this included in city_incite_cost? It should be.
> 
> It can't be, since city_incite_cost() just updates
> pcity->incite_revolt_cost, and the incite bonus is dependent on who does
> the inciting. IMHO, city_incite_cost() should be called once for every
> city at turn start, so we don't need to everywhere else before
> pcity->incite_revolt_cost is used.

So why don't we remove pcity->incite_revolt_cost and add a pplayer arg
to city_incite_cost?

> > > +  if (pplayers_at_war(pplayer, city_owner(acity))
> >
> > > +      && (find_palace(city_owner(acity)) != acity)
> >
> > Compare incite_cost against INCITE_IMPOSSIBLE_COST.
> 
> I'd rather assert that. Since we should check all possible reason why we
> can't incite here. If we missed one reason, we should fix it.

And exactly this the reason why the CMA dump core if the results
doesn't match.

> > > +    /* incite gain (FIXME: we should count wonders too but need to
> > > +       cache that somehow to avoid CPU hog -- Per) */
> >
> > What is the problem? Updating the cached value every turn is enough.
> >
> > > +    gain_incite *= SHIELD_WEIGHTING; /* cost to take city otherwise */
> >

> > ???

I understood the previous lines but not this multiplication with
SHIELD_WEIGHTING and the comment.

> The incite algorithm is ugly. I know. It is just a bunch of WAGs, but it
> works, and Greg said he'd try to make a better one.
> 
> But it is easier to experiment with new algorithms with existing code in
> cvs that has been tested and that one can compare with.
> 
> As to wonders, show me an easy to way to calculate the worth of wonders in
> enemy cities, please...
> 
> > > +{
> > > +  int dist, urgency;
> > > +  int best_dist = 30; /* any city closer than this is better than none */
> > > +  int best_urgency = 0;
> > > +  struct city *ctarget = NULL;
> > > +  int continent = map_get_continent(x, y);
> > > +
> > > +  city_list_iterate(pplayer->cities, acity) {
> >
> > Move the stuff into the loop.
> 
> I'm with Mike here. I don't care if the compiler can do tricks so that
> stuff like map_get_continent() doesn't get called on each loop - this is
> just wrong by design. If we want it to be called once, then we should call
> it outside the loop.

I'm speaking about dist and urgency.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "The Internet is really just a series of bottlenecks 
  joined by high speed networks."
    -- Sam Wilson