Re: [gnugo-devel] tactics code cleanup

[Arend Bayer]

Gunnar wrote:

> Arend wrote:
> > > +      /* Check if the two liberties are located like the figure above. */
> > > +      if (alib != SW(blib)
> > > +          && alib != NW(blib)
> > > +          && alib != NE(blib)
> > > +          && alib != SE(blib))
> > > +        continue;
> > > +
> > > +      ai = I(alib);
> > > +      aj = J(alib);
> > > +      bi = I(blib);
> > > +      bj = J(blib);
> > > +      /* Which of the two corner points should we use? One of them is
> > > +       * always occupied by the string at (str), the other one is either
> > > +       * free or occupied by something else.
> > > +       */
> > As an aside, this comment seems wrong to me (both before and after your
> > patch).
>
> Why? The two liberties are (ai, aj) and (bi, bj). The two corner
> points referred to in the comment are (ai, bj) and (aj, bi), used in
> the code fragment below the comment:
>
>   if (BOARD(bi, aj) == EMPTY)
>     *move = POS(bi, aj);
>   else if (BOARD(ai, bj) == EMPTY)
>     *move = POS(ai, bj);
>   else
>     return 0;
>
> Looks fine to me.

E.g.:

|OOOO
|XXXO
|X.bO
|Xa..
+----

I don't think I can quickly come up with a situation where it matters
that the comment is wrong, and I am not sure there is one.

> > edge_close_backfill() seems to have returned WIN without even checking
> > whether the string can be defended. So either I am completely
> > misunderstanding this function, or this is a gross bug.
>
> You misunderstand the function. It has been a move generator all
> along, just that the caller did the actual adding to the list of
> moves. There was no reason for it to return WIN, though. Presumably
> this happened by mistake when we did the first conversion to symbolic
> WIN values.
Yes that confused me, as Evan already pointed out.

Arend