Complete.Org: Mailing Lists: Archives: freeciv-dev: February 2002:
[Freeciv-Dev] Re: [PATCH] [aiunit.c-1.43] ai_military_findvictim() clean
Home

[Freeciv-Dev] Re: [PATCH] [aiunit.c-1.43] ai_military_findvictim() clean

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: rf13@xxxxxxxxxxxxxxxxxxxxxx
Cc: Raahul Kumar <raahul_da_man@xxxxxxxxx>, Gregory Berkolaiko <gberkolaiko@xxxxxxxxxxx>, Mike Kaufman <kaufman@xxxxxxxxxxxxxxxxxxxxxx>, freeciv-dev@xxxxxxxxxxx, bugs@xxxxxxxxxxxxxxxxxxx
Subject: [Freeciv-Dev] Re: [PATCH] [aiunit.c-1.43] ai_military_findvictim() cleanup (PR#1264)
From: Petr Baudis <pasky@xxxxxxxxxxx>
Date: Fri, 22 Feb 2002 21:07:11 +0100

Dear diary, on Fri, Feb 22, 2002 at 08:53:54PM CET, I got a letter,
where Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> told me, that...
> On Fri, Feb 22, 2002 at 08:37:12PM +0100, Petr Baudis wrote:
> > +  int stack_size = unit_list_size(&(map_get_tile(punit->x, 
> > punit->y)->units));
> 
> IMHO this should really be converted to a "bool only_unit_at_tile" or similar.

I don't like that. We're making the variables unneccessarily specialized, which
has IMHO minimal aspect to performace, but it's going to give us hell when
we'll try to improve this function sometimes in the future.

> > +  if (punit->unhappiness > 0) {
> > +    /* When we're causing unhappiness, we'll set best even lower, so that 
> > we
> > +     * will take even targets which we would ignore otherwise (in other 
> > words -
> > +     * we're going to commit a suicide). */
> > +    best = - 2 * MORT * TRADE_WEIGHTING;
> > +  }
> 
> > +  /* Ferryboats with passengers do not attack. -- Syela */
> > +  if (punit->ai.passenger > 0) {
> > +    return 0;
> > +  }
> 
> These two blocks can be reordered for speed.

True. Yes, sir.

> > +        /* We're only dealing with adjacent victims here. */
> > +        int move_cost = 0;
> 
> > +                    / (attack + vuln) - move_cost * SHIELD_WEIGHTING);
> 
> Can be removed.

See the TODO near there. I won't remove it now but in the patch unifying those
computations (we'll reuse Greg's one or I'll remake it, but it'll be my
top-priority after getting this patch in).

> > +          /* No need to amortize! */
> 
> Can you explain this?

We're working in one-turn horizon as we will just attack, not travel anywhere.
This is my opinion to this :). Added as a comment, sir.

Improved patch attached.

-- 

                                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/


[Prev in Thread] Current Thread [Next in Thread]