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

[Petr Baudis <pasky@xxxxxxxxxxx>]

Dear diary, on Sun, Mar 10, 2002 at 06:32:26PM CET, I got a letter,
where Markus Linnala <maage@xxxxxxxxx> told me, that...
> 
> Part 2 of assess_danger cleanup. This is on top of '[PATCH} try to cleanup 
> assess_danger (PR#1321)'.
> 
> Move common calculations to function.
> 
> -/* yes, I know cloning all this code is bad form.  I don't really
> -want to write a funct that takes nine ints by reference. -- Syela */
> 
> Now we need only 6 ints by reference.
> 
> diff -ur -X freeciv/diff_ignore 
> freeciv-cleanup-assess-danger-1/ai/advmilitary.c 
> freeciv-cleanup-assess-danger-2/ai/advmilitary.c
> --- freeciv-cleanup-assess-danger-1/ai/advmilitary.c  Sun Mar 10 19:08:28 2002
> +++ freeciv-cleanup-assess-danger-2/ai/advmilitary.c  Sun Mar 10 19:28:02 2002
> @@ -220,13 +220,83 @@
>      assess_danger(pcity);
>    city_list_iterate_end;
>  }
> -       
> +
> +static void make_virtual_unit(struct unit *virtualunit, 
> +                           int owner, 
> +                           int x,
> +                           int y,
> +                           Unit_Type_id type,
> +                           bool veteran,
> +                           int hp)

Uh-oh. I can't say I want to see this in the code :). Please reformat this
somewhat sanely.

> +{
> +/* this memset didn't work because of syntax that
> +the intrepid David Pfitzner discovered was in error. -- Syela */
> +  memset(virtualunit, 0, sizeof(struct unit));
> +  virtualunit->owner = owner;
> +  virtualunit->x = x;
> +  virtualunit->y = y;
> +  virtualunit->type = type;
> +  virtualunit->veteran = veteran;
> +  virtualunit->hp = hp;
> +}
> +
> +static void assess_danger_unit_helper(struct player *pplayer, struct city 
> *pcity, struct unit *punit, 
> +                                   int boatid, int boatdist, int boatspeed, 
> +                                   bool def_against_diplomat, bool pikemen,
> +                                   int *badmojo, int *danger, int 
> *danger_wall, int *danger_coastal, 
> +                                   int *danger_sam, int *danger_sdi)

You said 6? I see 14. This is not the way. Really. You need to get at least
figure out some better name for the function and stock the danger values into a
structure.

I would also want boat info in special structure, but that's for future and for
another patch.

By the way, I would stop now generating more patches and wait for people to
read the existing ones and proccess them. And obviously listen to the
objections and fix the patches appropriatelly ;). Otherwise you'll end like
Ross, with his huge corecleanup patch, minimal benefit for mainstream freeciv
and nearly lost hours of coding (please don't react to this - I don't want
flame about that, okay, probably I'm wrong, yes I think his objections are very
inspiring and valuable etc etc ;)).

-- 

                                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/