Re: [gnugo-devel] tactics code cleanup

[Gunnar Farneback]

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.

> 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.

> As a start, a patch to correct this (and testing what this does) would
> be very welcome.

The only relevant correction would be to change WIN to 1, but that has
no actual effect.

/Gunnar