[freeciv-ai] Re: ai bodyguard code cleanup part#1 v2

[Raahul Kumar <raahul_da_man@xxxxxxxxx>]

--- "Per I. Mathisen" <per@xxxxxxxxxxx> wrote:
<snip>

> > > +  /* protect a city */
> > > +  x = acity->x;
> > > +  y = acity->y;
> >
> > This is weird. Why is bodyguard code being relied on to protect cities?
> 
> Because no other code does...? ;)

Yes, but WTF are we doing this in bodyguard code? I'd prefer it removed it and
in its own seperate section.

> > > +/* is this insanity supposed to be a sanity check? -- per */
> > > +if (aunit && unit_list_find(&map_get_tile(x, y)->units, id)
> > > +    && aunit->ai.bodyguard != BODYGUARD_NONE) {
> > >    aunit->ai.bodyguard = id;
> > > +}
> > >}
> >
> > Huh??? Explain what is happening here. I can't follow this code.
> 
> If we are a bodyguard, then aiunit is our charge. That is, if we are a
> _unit_ bodyguard. A bit above this point in the code, we try a goto. So we
> check if we are stacked with our charge. If we are, we teach our charge
> that it has a bodyguard.
> 
> Why a charge shouldn't know that it has a bodyguard before it is stacked
> with it, is beyond me, though.

Theories?
 
> > >    if (goto_is_sane(punit, dest_x, dest_y, TRUE) && punit->moves_left > 0
> > > &&
> > >       (!ferryboat ||
> > >       (real_map_distance(punit->x, punit->y, dest_x, dest_y) < 3 &&
> > > -     (punit->ai.bodyguard == 0 ||
> unit_list_find(&(map_get_tile(punit->x,
> > > +     (punit->ai.bodyguard == BODYGUARD_NONE ||
> > > unit_list_find(&(map_get_tile(punit->x,
> > >       punit->y)->units), punit->ai.bodyguard) || (dcity &&
> > >       !has_defense(dcity)))))) {
> >
> > GB, you have any comments to make on this line. I don't see why real map
> > distance is used. Since when have planes been bodyguards?
> >
> > I'm really not at all ok with these lines. It seems a wrong estimate of
> > distance.
> 
> Feel free to fix.
> 
> BTW, Greg, speaking of flying, can you post an updated version of your
> flying AI patch?
> 
> > Why unit_vulnerability? I can't remember off the top of my head what it
> does, wouldn't defencepower be good enough?
> 
> It needs to be adjusted for hp and firepower and POWER_DIVIDER.

You win. 

> > > +unit_list_iterate(pplayer->units, buddy) {
> > > +  if (buddy->ai.bodyguard == BODYGUARD_NONE /* should be !=
> > > BODYGUARD_WANTED */
> >
> > This line is interesting. Any value besides bodyguard_none means we want a
> > bodyguard.
> 
> No, it means we _wanted_ a bodyguard. We could have gotten one that died.
> Or was stolen by some other part of the code.

Bodyguards get stolen?

> > > +      || !goto_is_sane(punit, buddy->x, buddy->y, TRUE)
> > > +      || unit_type(buddy)->move_rate > unit_type(punit)->move_rate
> > > +      || unit_type(buddy)->move_type != unit_type(punit)->move_type) {
> > > +    continue;
> > > +  }
> >
> > I'm taking it the AI movement code makes faster bodyguards outpace slower
> > units.
> 
> No, it doesn't.

Per, I just pulled out some of my hair. I'm assuming that buddy unit is the
bodyguard unit. Am I in fact wrong and the buddy unit is the unit being
guarded?

You need to make it clear which unit is the bodyguard and which one is being
protected.

> > Shouldn't that be the bug that is fixed, rather than kludging the AI code
> not to use units with more move_rate as bodyguards?
> 
> You got it exactly backwards. It doesn't want to use bodyguards that are
> slower than the charge, because that would slow down the primary unit.
> Movement is too valuable to waste, I suppose. Though I think we may want
> to make some exceptions.

I don't see how to avoid this. Sometimes the defending unit(Riflemen) is so
good that we tolerate slow movement.

> > I am very much not ok with this, even though this is an existing bug. Can
> you please fix it Per? I'm thinking of Mech Inf not being used at all.
> 
> What, are you crack?? The AI uses mech inf all the time! It even uses mech
> inf as bodyguards for stinkin' riflemen! (You see, riflemen have att >
> def, so they are attack type unit while mech inf has def <= att, so it is
> a bodyguard type of unit...)

Which is very stupid. It should compare attackpower of bodyguard unit and the
normal unit. After all, a Mech inf at 1 hp is a crappy attacker compared to a
riflemen at full hp.

> > I don't see a reason why we have to stick with only one bodyguard. If the
> > square the guarded unit is a fortress or city tile we can havemultiple
> > bodyguards.
> 
> Bodyguards move around and thus become vulnerable. I really don't want
> multiple bodyguards. The very few cases where it is useful does not
> warrant the much bigger code complexity.

OK. 

> > I like everything else about this patch. A much needed one in my opinion.
> Good work Per.
> 
> Thanks, good to hear :)
> 
> Yours
> Per

Aloha,
RK.

The problem with people who worship themselves is when they get together their
religions conflict.

__________________________________________________
Do You Yahoo!?
HotJobs - Search Thousands of New Jobs
http://www.hotjobs.com