Re: [gnugo-devel] Preparatory patch

[Gunnar Farneback]

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.

> In fact, if the roles had been reversed, I would have rejected
> your patch where you came up with the ATTACK_MOVE_GOOD_KO and
> OWL_ATTACK_MOVE_GOOD_KO reasons since it isn't extendable with
> reasonable effort.  For instance, do you yourself want to have
> ATTACK_THREAT_GOOD_KO and associates which would be the next natural
> thing to do?

I'm perfectly aware that the ko handling in move_reasons.c won't scale
and I hesitated a lot before choosing that solution. The complexity
required for the more general solutions I considered just didn't pay
off. If someone can come up with a nice and simple scheme I'm all
ears.

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

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

add_attack_move(pos, worm, WIN);
add_attack_threat(pos, worm, WIN);

> There is a reason why we use (0, GOOD_KO, BAD_KO, WIN) instead of 
> (1, 2, 3, 4) you know.  But if you say so, I can change it.  What is
> the opinion of the rest of you, maybe especially Dan?

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.

Arend wrote:
> What about this:
> #define IS_THREAT                1
> #define ATTACK_MOVE              2
> #define DEFEND_MOVE              4
> #define CONNECT_MOVE             6
> #define CUT_MOVE                 8
> (...)
> #define ATTACK_THREAT_MOVE       (ATTACK_MOVE | IS_THREAT)
> #define DEFEND_THREAT_MOVE       (DEFEND_MOVE | IS_THREAT)
> etc.
> 
> The existing code would not need to be changed, but where necessary
> in potential new code, threat moves can be filtered out elegantly
> (e.g. the addition to my patch that I described in my last e-mail could
> implemented nicely).

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

/Gunnar