[freeciv-ai] Re: (PR#4137) Auto-Explore with new path-finding

["Gregory Berkolaiko" <Gregory.Berkolaiko@xxxxxxxxxxxx>]

Sorry, I didn't have much time lately, so didn't read your code.
<rant>
There are so many patches waiting for my review that I have no time to 
make my own :(
</rant>

On Sat, 3 May 2003, Cameron Morland wrote:

> > 2. Same place, I think
> > #define BEST_NORMAL_TILE  \
> >  (100 * MAX_NEW_TILES * DIFF_TER_SCORE +\
> >   100 * (VISION_TILES-MAX_NEW_TILES) * KNOWN_DIFF_TER_SCORE)
> 
> <Grind, grind, grind>. Right.

Great.  Now fix the indentation and add spaces around operations.

> > 6. To do the early breaking, you multiply the BEST_POSSIBLE_SCORE by
> > the
> > dist factor.  Which means you do a pow every step.  I would instead
> > calculate the furthest dist we are prepared to go each time we update
> > most_desireable.  That would be logarithm.

Cool.  Mind your spaces around operations!

> > Some comments on PF use:
> > 
> > 9. The PF should respect ZoC and avoid unknown using a proper TB-
> > callback.
> > It might also want to try to walk _past_ unknown, so a nice EC-
> > callback
> > can be used here.
> > 
> > If you are unsure of the PF and how to use it, I will do the PF-
> > related
> > changes.
> 
> I don't really understand the PF code; you're welcome to do it if you
> like. It would be best to rank paths instead of tiles; most of the
> pre-existing scoring code should probably be reusable.

Ranking paths is practically out of question, too complicated.

Comments:

1. Please fix spacing around ifs, it should be 
        if (..)
and not
        if(..)

2.
+  } else {
+    /* idle to wake the unit. */
+    handle_unit_activity_request(punit, ACTIVITY_IDLE);
+    return FALSE;
   }

is wrong and unnecessary.  Right below it, you ask whether we need this 
idle.  Yes we do, otherwise human-owned explorers won't wake up once the 
are finished exploring.  But this is a wrong place to do it.  Attached is 
a mini-patch (please do it properly) which moves the IDLING to the right 
place.

I think we should aim to remove every mention of activity from explorer 
code.  In this respect the outer if and idling in:
+       if (punit->activity == ACTIVITY_EXPLORE) {
+         if(unit_flag(punit, F_TRIREME) &&
+            (punit->moves_left != unit_move_rate(punit))) {
+           /* we're a trireme with non-full complement of movement points,
+            * so wait until next turn. */
+           return TRUE;
+         }
+         handle_unit_activity_request(punit, ACTIVITY_IDLE);
+       }
is unneccesary too, methinks.

Please fix these issues and then I'll try to sort out the path-finding.

G.