Re: [gnugo-devel] Preparatory patch

[Inge Wallin]

Gunnar wrote:
> Inge wrote:
> > But for the rest of the move reasons, in fact for the overwhelming
> > majority, the is_threat parameter makes perfect sense and is, in my
> > opinion, no nuisance at all.  If we continue down your path we will
> > eventually end up with a
> > ATTACK_WITH_TWO_STEP_APPROACH_MOVE_GOOD_KO_THREAT, which is much
> > worse.
> 
> You should make a distinction between the internal handling of move
> reasons and the external interface to the rest of the engine. E.g.
> this change
> 
> -         add_owl_attack_move(move, pos, dragon[pos].owl_attack_code);
> +         add_owl_attack_move(move, pos, dragon[pos].owl_attack_code, FALSE);
> 
> adds a parameter that is more distracting than anything else. In
> contrast the attack_code parameter to this function is very natural.

Somehow we have to tell the move evaluator whether the move is a
threat or works outright.  We now have two competing ways of doing
this:
1. Named move reasons with every combination covered in the name.
2. Parameters ('code' and now 'is_threat').

I think that the parameter way of doing it is easier since it is done
the same way everywhere.  When you have seen 3 calls to
add_some_move() and checked the last parameter, from then on you know
that it tells wheter it is a threat or not.  No confusion, just old
familiar stuff.

> > I state that the current scheme makes it more or less impossible to
> > create a general machinery that handles threats and combinations of
> > threats.  My scheme is a remedy of this.  If you or somebody else can
> > come up with something better, then good.
> 
> This is exactly why I want to know how you want that machinery to
> work.

As I said in my last mail: I will write a real description in the form
of a texinfo chapter for the documentation.


> Inge wrote:
> > I think it buys us something.  A call like
> > 
> >   add_attack_move(pos, worm, WIN, TRUE);
> > 
> > says more about what is happening than
> > 
> >   add_attack_move(pos, worm, WIN, 1);
> > 
> 
> No, it doesn't. Those two are virtually identical and a lot worse for
> readability than

I simply don't understand how you can say that.  What do the rest of
you think?

> Those cases are not comparable. (0, BAD_KO, GOOD_KO, WIN) carry
> information that (0, 1, 2, 3) don't. Using TRUE instead of 1 does not
> add any information since 1 being true is fundamental in C.

Yes, but when you see a 1, you expect it to be a numerical value.
Using TRUE tells the reader that it is a boolean value that is
intended.  For the caller it is a difference, for the callee it is
not.  But as I wrote previously, this is not a point that I want to
fight very hard for.

> Maybe. It would at least allow us to keep the construction
> 
> switch (...) {
>   case ATTACK_MOVE:
>     [big block 1]
>     break;
>   case ATTACK_THREAT_MOVE:
>     [big block 2]
>     break;
>   ...
> }
> 
> which is more readable than the construction after Inge's patch:
> 
> switch (...) {
>   case ATTACK_MOVE:
>     if (is_threat) { 
>       [big block 2]
>       break;
>     }
>     /* Not a threat if we reach this point. */
>     [big block 1]
>     break;
>   ...
> }

Yes.  I am not really satisfied with this, but it was a way to get
the actually pretty complex patch to work with minimum effort.  I will
integrate the contents of the "if (is_threat)" branch into the rest of
the code.

        -Inge