[Freeciv-Dev] Re: [PATCH] [1.4] cleanup of proccess_*_want() (PR#1295)

[Petr Baudis <pasky@xxxxxxxxxxx>]

Dear diary, on Sun, Apr 07, 2002 at 07:28:39PM CEST, I got a letter,
where Gregory Berkolaiko <Gregory.Berkolaiko@xxxxxxxxxxxx> told me, that...
> On Sun, 7 Apr 2002, Petr Baudis wrote:
> 
> > > +         * protect them (?). */
> > > +        if (walls && move_type == LAND_MOVING) {
> > > +          desire *= pcity->ai.wallvalue;
> > > +          /* TODO: More use of POWER_FACTOR ! */
> > > +          desire /= POWER_FACTOR;
> > > +        }
> > > 
> > > Well, it's better to build defenders which will go well with existing 
> > > structures, isn't it?  I think there is nothing to fix here.
> > 
> > IMHO we should rather increase desire for city walls when building land
> > defenders.. but maybe it wouldn't work so well in the grand scheme of 
> > things..
> 
> Err, don't try to confuse me.

But it's fun.. ;p

> We _already_ have citywalls => if we build a land he will benefit from it, 
> and a ship won't, so we have to take that into account, right?

I told you it won't work ;))).

> > > The below comment is nonsense.  The clause does not apply to battleships.
> > > 
> > > +  /* I was getting four-figure desire for battleships otherwise. -- 
> > > Syela */
> > > +  if (!walls && unit_types[best_unit_type].move_type == LAND_MOVING) {
> > >      best *= pcity->ai.wallvalue;
> > > +    best /= POWER_FACTOR;
> > > +  }
> > 
> > Well, maybe this way battleships won't get larger desire than land units.. I
> > can't understand it fully as well, but I've no idea where else I should 
> > place
> > that comment.. Or should I remove it completely?
> 
> Well, nobody seems to understand this comment => it's not serving it's 
> purpose (to comment the code).  Remove it methinks

I'm willing to replace it with some meaningful explanation of the sense of this
block (I just don't get it; was never good in city walls affairs, you know; I'd
probably rather increase want for _city walls_ *here*).

> > > +static void process_attacker_want(struct player *pplayer, struct city 
> > > *pcity,
> > > +                                  int value, Unit_Type_id 
> > > victim_unit_type,
> > > +                                  bool veteran, int x, int y, bool unhap,
> > > +                                  int *best_value, int *best_choice,
> > > +                                  int boatx, int boaty, int boatspeed,
> > > +                                  int needferry)
> > > +{ 

> > > +      /* TODO: Case for B_AIRPORT. -- Raahul */
> > > +      bool will_be_veteran = (move_type == LAND_MOVING ||
> > > +                              player_knows_improvement_tech(pplayer, 
> > > B_PORT));

> > > I sincerely hope this function will never be trusted to evaluate the need 
> > > for bombers.  I appreciate Raahul remembering me ;)  but I think we can 
> > > safely remove the comment.
> > 
> > Why it shouldn't evaluate need for bombers? I'd like much more cleaning the
> > current evaluation routines so that we can add support for flying units 
> > there
> > nicely than gluing the air evaluation routines to the face of the current 
> > eval
> > routines, thus only dramatically increasing the current mess in evaluation
> > routines.
> 
> This is an idiotic routine called from another messy and idiotic routine 
> which in turn is called from yet another even more messy and idiotic 
> routine.  All this and should be done in three functions 
> ai_eval_land_offensive
> ai_eval_sea_offensive
> ai_eval_air_offensive  
> (which could share some common functions) nicely and clearly.  The above 
> unit types are quite different and trying to unite their evaluation brings 
> the code into the state it is now: AWFUL.

Totally agreed. But then I would welcome if you would first do
ai_eval_(land|sea)_offensive() and THEN introduce ai_eval_air_offensive(). What
will happen when you'll suddenly disappear as Syela did? ;p

> Sorry for shouting...  I just spent an hour trying to trace evaluation of 
> city walls...

I understand you fully :).

> > I'm not sure yet if I should first clean assess_danger() or f_s_t_k().
> 
> they are both worthy targets

:^) I'll choose accordingly to my mood ;).

-- 
 
                                Petr "Pasky" Baudis
 
* ELinks maintainer                * IPv6 guy (XS26 co-coordinator)
* IRCnet operator                  * FreeCiv AI hacker
.
Teamwork is essential -- it allows you to blame someone else.
.
Public PGP key && geekcode && homepage: http://pasky.ji.cz/~pasky/