[freeciv-ai] Re: ai bodyguard code cleanup part#1

[Gregory Berkolaiko <Gregory.Berkolaiko@xxxxxxxxxxxx>]

On Sat, 10 Aug 2002, Per I. Mathisen wrote:

> This patch does stylistic cleanup and introduces the new (but here unused)
> function ai_unit_new_role(), which should add an incremental number of
> much needed sanity checks to unit task assignment in the AI code.
> 
> Cleanups:
>  - new enum BODYGUARD_* instead of magic numbers -1, 0
>  - documented several functions
>  - cleaned up style in look_for_charge and ai_military_bodyguard

I can't say I love your style.  Also I cannot say that you changed all var 
names that are crying to be changed (like i in ai_military_bodyguard).  
I also suppose you could write more comments.

But even in its present form it is a step forward.  There is one thing I 
would want you to do though:

In look_for_charge,
+    unit_list_iterate(pplayer->units, body) {
+      /* if buddy already has a bodyguard, ignore it */
+      if (body->ai.charge == buddy->id) { def = 0; }
+    } unit_list_iterate_end;

First of all, I would write it as:
unit_list_iterate(pplayer->units, body) {
  if (body->ai.charge == buddy->id) {
    /* it appears buddy already has a bodyguard, 
     * so it's not a good charge */
    def = 0;
  }
} unit_list_iterate_end;
but that's a style issue and you can ignore it.

More importantly, there is a check before that,
  if (buddy->ai.bodyguard == 0) continue;
which means that if we got to "def = 0;", buddy has a bodyguard but is not 
aware of it.  This is bad: we have a doubly linked pair and one of the 
links is broken.  If you are seriously working on the bodyguards, you have 
to catch all such occurences and fix them or explain them.


> There are no autogame changes. Please proofread.
> 
> More bodyguard patches will be coming, since I want to see if I can give
> our active diplomats bodyguards.

Ha, now I see why your diplomats were not getting the bodyguards: they are 
too fast!!

G.