[Freeciv-Dev] Re: [PATCH] aiunit.c ai_military_findvictim() cleanup (PR#1264)

[Petr Baudis <pasky@xxxxxxxxxxx>]

Dear diary, on Sun, Feb 17, 2002 at 10:53:11PM CET, I got a letter, where
Gregory Berkolaiko <gberkolaiko@xxxxxxxxxxx> told me, that...
>  --- Petr Baudis <pasky@xxxxxxxxxxx> wrote: 
> > > > > Is there no better way to say if a boat has passangers.  If the
> > > > > answer is "no" then you should write it, and separate the code below
> > > > > into it.
> > > > 
> > > > Hmm.. after all, shouldn't we just exit when we're a ferryboat,
> > > > independently on our charge? Why to waste them on the enemies? (if
> > > > answer is "yes, we
> > > 
> > > Why not?  What if somebody set transporter capacity of a battleship to 3?
> > 
> > Perfectly valid point. Let's have a looook... <hours are passing ;>...
> > Well, how are represented the passengers of a ferry? The actual code
> > appears as buggy to me, as it produces wrong results when there're more
> > boats at one tile, one loaded and one empty (and we're the empty one).
> > [NOT FIXED]
> 
> sure, it is buggy.  maybe you can write a better one?

I was asking just for a hint how passengers are represented in struct unit
(ai.passenger can contain only ONE unit_id) - I'm now too tired to run more
research in this area. Ok, I'll try tommorow if I'll get a moment.

> > > > > in general, not only barbarians should pillage.
> > > > 
> > > > Is it used anywhere else?
> > > 
> > > could be but I don't think so.  but it shouldn't stop you IMO
> > 
> > I don't see why some other code should be considering pillage when we're
> > doing
> 
> well, think about this: will a unit ever do pillage??  here we are
> considering moving to a tile (x1,y1) to do pillage.  what happens when we
> arrive there?  do you think it will remember what it wanted on this tile?

Pillage seems to be default activity for barbarians. See ai_military_findjob().

> > > I think we should make many small functions with good names, like
> > > consider_defending_town, consider_pillaging, consider_hut_poaching and
> > > has_passangers.
> > > 
> > > Even if these functions are only used once, they will be easier to
> > > maintain, they will encourage re-use in other parts of AI and they will
> > > shorten long routines.
..snip..
> > It's not a bad thing, but I don't think it's worthwhile doing now.
> 
> oh well, our opinions differ.

If you will later introduce a patch doing this, I won't probably burn it,
though ;).

> > > But this is my philosophy which might be different from the mainstream.
> > > 
> > > Also, how about writing local macro SET_BEST(new_best, new_x, new_y) ?
> > 
> > Good idea, done.
> > 
> > > Or, even better, change *dest_x and *dest_y in some centralised place,
> > > not every time you change best.  Yes, that's it.
> > 
> > How would you imagine that..? Saving old best value and comparing it? That
> > seems like ugly kludge to me.
> 
> well SET_BEST seems more of a kludge to me.
> making a new variable (local to iterate loop) 
> int new_best=0;
> 
> and adding before iterate_end
> if (new_best > best) {
>   best = new_best;
>   dest=...
> }
> 
> is not a kludge at all.  Raimar?

It's also ok for me. I'll let Raimar decide which one to use.

> > > Somehow I like it more.  Don't really know why.
> > 
> > Let's see now :).
> 
> Mmmm, not much better.  Let's see why.
> 
> I know I suggested it, but it looks ugly :((
> > +#define SET_BEST(new_best) \
> > +  do { best = new_best; *dest_x = x1; *dest_y = y1; } while (0)

Above.

> > +      /* Horsemen in city refused to attack phalanx just outside that was
> > +       * bodyguarding catapult; thus, we get the best attacker on the tile
> > as
> > +       * well, for the case when there are multiple different units on one
> > +       * title. Thus we force punit to attack a stack of units if they are
> > +       * endangering punit seriously, even if they aren't that weak. */
> > +      struct unit *patt = get_attacker(punit, x1, y1);
> > +
> > +      if (!can_unit_attack_tile(punit, x1, y1))
> > +        continue;
> > +
> > +      /* If we are in the city, let's deeply consider defending it. See
> > patt's
> > +       * comment for details about the big unequality. */
> 
> You call that explanation????

I do. I really can't imagine what better you want. Can you please give me an
example?

> This is just a statement of intent.  Explanation would explain _why_ these
> particular values are compared!

Imagine multiple units on one tile as one big unit. Then its defense factor can
be taken as defense factor of best defense unit there, and it attack factor as
attack factor of best attack unit there.

> If you don't know, just say it (not in the code though ;).  I don't know.

I think it's relatively straightforward when you stare at it sufficiently
enough.  Could be that I stared from a wrong angle, though.

> But I have strong suspicion that this inequality is _wrong_ :
> 
> get_total_defense_power(pdef, punit) * get_total_defense_power(punit, pdef) 
> >= 
> get_total_attack_power(patt, punit) * get_total_attack_power(punit, pdef)
> 
> It should have get_total_defense_power(patt, punit), not (pdef,punit).

Nope. BEST DEFENSE unit on the tile defends.

> > +      if (map_get_city(x, y)
> > +          && get_total_defense_power(pdef, punit) *
> > +             get_total_defense_power(punit, pdef) >= /* didn't like > --
> > Syela */
> > +             get_total_attack_power(patt, punit) *
> > +             get_total_attack_power(punit, pdef)
> > +          && unit_list_size(&(map_get_tile(punit->x, punit->y)->units)) < 2
> > +          && get_total_attack_power(patt, punit) > 0) {
> > +        freelog(LOG_DEBUG, "%s defending %s from %s's %s",
> > +                unit_type(punit)->name,
> > +                map_get_city(x, y)->name,
> > +                unit_owner(pdef)->name, unit_type(pdef)->name);
> > +        
> 
> ugh, my head is killing me
> time to go to sleep...

Good nite and sweet patt/pdef dreams.. ;)

-- 

                                Petr "Pasky" Baudis

* elinks maintainer                * IPv6 guy (XS26 co-coordinator)
* IRCnet operator                  * FreeCiv AI hacker
.
No one can feel as helpless as the owner of a sick goldfish.
.
Public PGP key && geekcode && homepage: http://pasky.ji.cz/~pasky/