> > Open issues:
> > - No visual feed-back to the user that the engine is scoring. Opening a
> > small dialog box would be nice, as it may take some seconds.
>
> I will delay this patch until after 0.1.5 anyway. You have time to
> improve ;)
Good. I haven't worked with Gtk too much yet, so I wanted to report in
before I get lost in it for too long.
>
> > - If Quarry is used to have two gtp clients play against each other,
> > only black is asked to score.
>
> This should be fixed, yes.
Proposed behaviour:
If there are two gtp clients, have them both score. If they agree,
that's the final score, no user interaction needed (not even possible?).
If they disagree, inform user about which stones/strings are disputed
and have him decide.
Agree?
> > - The score the gtp client calculates should be queried and compared to
> > what Quarry determines. Yell if they disagree.
>
> This is less important. Quarry's count function is certainly not perfect
> now. If the engines agree with each other we should be satisfied. So,
> while this should be implemented in future, there is no need to do it now.
>
> > - Seki?
>
> Seki is a general problem. It is of high priority, but I think it is OK
> to be postponed for a while.
Okay, two more points I will eventually put in a TODO.
> > The missing feed-back while scoring might be a show-stopper, but before
> > starting on that, I wanted to ask if the approach in general is ok.
>
> Yes, it should be implemented. `GtkProgressDialog' should be used (and
> improved if needed).
Okay, I'll take a look at it.
> > +static int
> > +parse_final_status_list(GtpClient *client, int successful,
> > + GtpClientUserCallbackData *callback_data)
> > +{
> > + StringListItem *this_item;
> > + int idx = 0;
> > +
> > + GtpClientFinalStatusListCallback final_status_list_callback
> > + = (GtpClientFinalStatusListCallback) callback_data->response_callback;
> > +
> > + BoardPositionList *stones
> > + = board_position_list_new_empty(string_list_count(&client->response));
>
> I believe output of `final_status_list' can come with several stones
> per line. Unless I missed something, your code expects only one stone
> per line. This must be fixed. The easiest way is probably to allocate
> large `PositionList' from the start.
Ah, after reading the GTP specification on final_status_list the n'th
time, I finally got it. In each line, there is a list of stones
belonging to one string. Right now, engine_has_scored marks entire
strings - that's why it works, but it's admittedly not very nice and
I'll change it.
>
> >
> > +typedef enum {
> > + GTP_ALIVE,
> > + GTP_DEAD,
> > + GTP_SEKI
> > +} GtpStoneStatus;
> >
>
> Such hunks are bad because they alter the number of empty lines between
> declarations. If there are two empty lines between non-closely related
> declarations, please keep them.
Oops, yes, I usually do.
> In general, don't alter `gtk-goban-window.c' a lot yet. It will be
> difficult to merge with my current patch. I hope to submit it tonight.
I will maybe rework parse_final_status_list but wait with displaying of
a progress dialog until you are done.
> Overall your patch seems good, keep working :)
Thanks. I will as time allows.
/Martin
