[Freeciv-Dev] Re: [PATCH] try to cleanup assess_danger (PR#1321)

[Petr Baudis <pasky@xxxxxxxxxxx>]

Dear diary, on Tue, Mar 12, 2002 at 11:33:34AM CET, I got a letter, where
Raahul Kumar <raahul_da_man@xxxxxxxxx> told me, that...
> 
> --- Markus Linnala <maage@xxxxxxxxx> wrote:
> > 
> > Change variable names to what they mean.
> > 
> 
> I have some problems with thse patches, and the sequel to this patch. I agree
> with everything Petr already said on these patches, and believe the
> maintainers should carefully contrast what Petr has done to this patch.

Just please don't get me wrong. I think this patch helps, and is step in the
right direction. It doesn't clean up anything by far, and I'm going to clean
this up even more in the future, but it's question if it's blocker for this
patch - as I can't tell you "ok, I'm going to send you the patch for it next
week, then we'll do some brainstorming in following days and I assure I'll be
available for making changes". Especially as I hadn't yet time to finish the
proccess_*_want() patch.

> > diff -ur -X freeciv/diff_ignore freeciv/ai/advmilitary.c
> > freeciv-cleanup-assess-danger-1/ai/advmilitary.c
> > --- freeciv/ai/advmilitary.c        Wed Mar  6 12:05:09 2002
> > +++ freeciv-cleanup-assess-danger-1/ai/advmilitary.c        Sun Mar 10 
> > 19:08:28 2002
> > @@ -228,15 +228,14 @@
> >  {
> >    int i, danger = 0, v, dist, m;
> >    Unit_Type_id utype;
> > -  int danger2 = 0; /* linear for walls */
> > -  int danger3 = 0; /* linear for coastal */
> > -  int danger4 = 0; /* linear for SAM */
> > -  int danger5 = 0; /* linear for SDI */
> > +  int danger_wall = 0; /* linear for walls */
> > +  int danger_coastal = 0; /* linear for coastal */
> > +  int danger_sam = 0; /* linear for SAM */
> > +  int danger_sdi = 0; /* linear for SDI */
> 
> I like the name change from danger[1,2,3,4]. This seems to already be in 
> Petr's
> CVS though.

See above.

> >    int badmojo = 0;
> 
> No explanation of what this is.

Probably out of scope of this patch. It doesn't aim to be complete cleanup,
contrary to many mine patches. .oO(dainty ;))))

> [EVIL CODE]
> 
> >    unit_list_iterate(map_get_tile(pcity->x, pcity->y)->units, punit)
> >      if (unit_flag(punit, F_PIKEMEN)) pikemen = TRUE;
> 
> > +    if (unit_flag(punit, F_DIPLOMAT)) def_against_diplomat = TRUE;
> >    unit_list_iterate_end;
> >  
> >    players_iterate(aplayer) {
> > @@ -286,7 +286,7 @@
> >            igwall = unit_really_ignores_citywalls(funit);
> >            if ((is_ground_unit(funit) && v != 0) ||
> >                (is_ground_units_transport(funit))) {
> > -            if (dist <= m * 3) urgency++;
> > +            if (dist <= m * 3) pcity->ai.urgency++;
> >              if (dist <= m) pcity->ai.grave_danger++;
> 
> This seems like a candidate for the combat calcs. 

Especially candidate for YOU and your SINGLE_MOVE, isn't it? >:)

> >            v *= v;
> >  
> > -          if (!igwall) danger2 += v * m / dist;
> > -          else if (is_sailing_unit(funit)) danger3 += v * m / dist;
> > -          else if (is_air_unit(funit) && !unit_flag(funit, F_NUCLEAR))
> > danger4 += v * m / dist;
> > -          if (unit_flag(funit, F_MISSILE)) danger5 += v * m / MAX(m, dist);
> > +          if (!igwall) danger_wall += v * m / dist;
> > +          else if (is_sailing_unit(funit)) danger_coastal += v * m / dist;
> > +          else if (is_air_unit(funit) && !unit_flag(funit, F_NUCLEAR))
> > danger_sam += v * m / dist;
> > +          if (unit_flag(funit, F_MISSILE)) danger_sdi += v * m / MAX(m,
> > dist);
> >            if (!unit_flag(funit, F_NUCLEAR)) { /* only SDI helps against
> > NUCLEAR */
> >              v = dangerfunct(v, m, dist);
> >              danger += v;
> 
> [/EVIL CODE]
> 
> This entire block should be replaced by a calculation from the combat calcs.
> I really do not like this. My reason for voting against this patch is based
> mostly on this block of code. 

It's not cleaning up anything there specifically. The main question here is, if
we want small patches walking into right direction, or big patches jumping to
the target.. It's kinda philosophical question however.

-- 

                                Petr "Pasky" Baudis

* elinks maintainer                * IPv6 guy (XS26 co-coordinator)
* IRCnet operator                  * FreeCiv AI hacker
.
"If you have acquired knowledge, what do you lack?
    If you lack knowledge, what have you acquired?"
Lev. R. 1:6
.
Public PGP key && geekcode && homepage: http://pasky.ji.cz/~pasky/