Re: [gnugo-devel] sgf output - first edition

[Gunnar Farneback]

Paul wrote:
>   - default setting: output all debug info available - is it ok?

I prefer no debug info as the default. 

>   - it outputs all debug information when using "save" from
>     play_ascii, maybe should not

An explicit save from within play_ascii should use the same level of
debug info as what is saved by -o.

>   - doesn't work good if file contains variations (e.g. undo is used).
>     should it be fixed? (it works, but some debug information looks
>     inconsistent)

Don't bother with this case. The sgf format can't handle this well
anyway. We do want undo to give a game record with variations but it
doesn't matter if the debug info looks funny in that case.

> if there are no objections, i'll rewrite gmp.c

I'm pretty sure you don't need to touch gmp.c. It's play_gmp.c you
want to look into.

> and sgfdecide.c using new output format as well.

As far as I know everything in sgfdecide.c is fine in this respect.
You may want to take a look at play_solo.c and play_test.c though.

> --- sgf/sgfnode.c       11 Apr 2002 19:52:06 -0000      1.14
> +++ sgf/sgfnode.c       1 Sep 2002 15:50:06 -0000
> @@ -43,6 +43,9 @@
>  # endif
>  #endif
>  
> +#include "gnugo.h"
> +#include "liberty.h"
> +
>  #include "sgftree.h"
>  #include "gg_utils.h"

This is a no, no. The code in the sgf directory should be an
independent library with no dependencies back to the engine. The code
which needs to use engine internals should stay in sgffile.c.

> +static void
> +restore_property(SGFProperty *prop)
> +{
> +  if(prop) {
> +    restore_property(prop->next);
> +    prop->name&=~0x20;
> +  }
> +}

What's the purpose of this?

> -  fprintf(sgfout, ")\n");
> +  /* FIXME: uncomment this if we ever return to sgffile.c
> +     commented to prevent it from overwriting file outted with outputsgf */
> +  /* fprintf(sgfout, ")\n"); */

Don't comment out code by actually enclosing it in a comment. Instead
use the 

#if 0
  ....
#endif

construction. The reasons why this is better are that:

* C comments don't nest. Preprocessor directives do.
* Comments interfere with the indentation of the disabled code and
  makes it harder to read.
* Syntax highlighting (e.g. color schemes) tends to get fooled by
  comments.
* It's much easier to turn the code on and off by switching between
  "#if 1" and "#if 0" than by removing and adding comments. Also
  "#else" can be used to good effect.

Occasionally it can be better to disable code with the

  if (0) {
    ...
  }

or

  if (0 && (...))

constructions. See existing code for examples.

/Gunnar