On Wed, 17 May 2006, Jimmy Huang wrote:
> And, thanks to Trent's insistence I find out why pl is
> NULL sometimes. I figured out the logic in the
> check_take function in decide.c was basically flawed.

It seems like you're not totally clear on the difference between local
variables and global variables.  The variable named pl in check_take() has
nothing to do with other variables of the same name in different functions.
If you have a C programming tutorial, you might want to read the bit about
local variables.

> It also explains why the bots don't like to take
> neutral planets, unless certain oddball conditions are
> met.
>
> I have summarily wiped out those oddball conditions
> (the independent planet had to be Non-Agri OR the bot
> had to have more than 4 armies???). Hopefully, this

You're wrong about that.  That code in check_take() is not checking an
independant planet, it is checking the last enemy planet it looked at in
the preceeding for loop.  (The one that looks for the closest enemy planet)

The check (!(pl->pl_flags & PLAGRI) || me->p_armies >= 5) actually makes
plenty of sense.  It is the common netrek wisdom, "don't drop armies on an
agri unless you have enough to take it."

The reason for a crash here is that when Hadley wrote the bot, there were
always enemy planets.  If you took all the enemy planets, the game was
over.  The bot would never try take a planet with only neut planets left in
the game.  Thus the case when the loop for(k=0; k< pls->num_warteamsp; k++)
never executed at all never came up, and pl was never left unset.  The bot
never had to cope with just neut planets left to take.

You are right that there is a bug in the check that surrounds the search
for neut planets.  First, it checks the last planet in the list of enemy
planets.  This list isn't in any order, so it's checking if some random
enemy planet is no agri.  Doesn't make sense.  What Hadley probably meant
to check was tpl, the target enemy planet, rather than pl.

Still, the check doesn't make sense.  What it's doing is only looking for
ind planets if the bot CAN take the closest enemy planet.  The logic is
like this:

Closest enemy planet is non-agri -> look for neut planets, take them first
Closest enemy planet is agri, but I have 5 armies -> look for neut planets,
take them first
Closest enemy planet is agri, and I don't have 5 armies -> don't try to
take neut planets

That makes no sense.  The bot would rather drop 2 on an agri than take a
neut planet.  This is the correct fix:

-   if(!(pl->pl_flags & PLAGRI) || me->p_armies >= 5){
+   if(!tpl || !(tpl->pl_flags & PLAGRI) || me->p_armies < 5){

Now the logic is:

There are no acceptable enemy planets -> look for ind ones (*)
The closest enemy planet is agri and I have five or more armies -> skip
looking for neut planets, the agri take is better
All other cases, look for nuet planets and take them before enemy planets

I think this logic is probably what Hadley intended to write, he just got
the logic backwards for the army check.