[gnugo-devel] RE: nando_3_9.4b

[Portela Fernand]

Arend wrote:

> A few comments:
> * Please use "acode" for attack codes and "dcode" for defend,
> reversing this (in owl_confirm_safety) is really confusing...

Ooops. I guess I wasn't even thinking when I did this change.

> (Also I tend to confuse LOSE and LOSS, so maybe we stick with 0 for the
> former?)

It confused me too, so I removed any LOSE I could find in the code. Still
some left somewhere ?

> * I think owl_attack returning GAIN with kworm == NO_MOVE should be
> considered a bug. E.g. the move valuation shouldn't have to work around
> this.

Ok. I will try and see what happens.

> Should be GAIN or LOSS in the comment I suppose. And the latter for
> owl_defense_code, or course.

Ack.

> > +     if (w != MAX_GOAL_WORMS)
> > +       if (LOSS > savecode) {
> Shouldn't this be "GAIN > savecode"?

Right. Here, it still worked as supposed to, but this...

> > +       else if (LOSS == savecode) {

never worked as expected. Thanks for pointing it out.

> > +   else if (dcode == LOSS)
> > +     saveworm = wid;
> 
> I think this last "else if" should be moved before the "if (captured > 2"
> etc. stuff, and should also make similar comparisons (check whether worm
> "wid" is bigger than saveworm, in we case already captured s.th. in
> another variation, etc.). E.g. if we have captured s.th., that is not in
> the list, _and_ dcode is LOSS, then savecode will be updated to GAIN by
> UPDATE_SAVED_KO_RESULT below, but saveworm is not set.

Indeed. 
Sharp eyes you have :)

> (...)I think the second case is not necessarry. If the defense code is
> LOSS, then attack code will probably have been "WIN", and the dragon is
> critical.

In this patch, I believe it is considered alive (see make_dragons() around
line 360). Possibly something wrong there then...

> However, it seems to me you miss the case where the attack
> code on our dragon is GAIN and defense code WIN.

Right again. 

> I think this should be a standard OWL_DEFEND_MOVE, with a special case
> treatment in estimate_territorial_value().

Agreed.


/nando