On Tue, 2005-12-20 at 23:05 +0100, Jérémy Rosen wrote:
> thanks a lot.... you could try to make the thing a little bit more
> organized by adding the [victory] inside the [start] and rename [start]
> into [game] or something like that...

You're absolutely right; I was still thinking of it in terms of an
append-only log.  Will flush brain now.  Should be easy to fix.

> I can't comment on the actual content, but what is really important is
> the framework, adding data to the reports afterward would be easy...
> 
> > 
> > Yeah, it didn't really fit as a member of anything else.  The behaviour
> > has changed a little in that each new load/start creates a new file, but
> > the filename numbers order them so I think that's OK.
> 
> why ? can't you append to an existing file ?

Yes, I could, but I want the network thread to be completely
independent.  My plan was, on creating a new upload_log object, fire off
a thread saying "send every file up to and not including the one I just
created".  The network thread deletes files behind itself, and then
exits when nothing left to do.  That's why the code looks a bit weird at
the moment.

> btw, you open out_ at object creation time, but only use it again at
> destruction time. for diabled log, you should check it at destruction
> time rather than creation time anyway (I guess it's a global option, so
> you might want to check it directly instead of passing it as a parameter)

There are two factors here: (1) is logging enabled (which we check at
the last possible moment, in the destructor), and (2) do we want to log
this game (which depends on whether it's a replay, multiplayer, etc).

Rather than using a constructor parameter, I really should create a base
logging class, and two subclasses: one to discard logging, and one to
actually do logging.

> why not create/write/close it all at destruction time, this way you
> would not have to kee cout_ as a member... (OTOH if we want to have
> progressive logging, keep it that way)
> 
> I think you could find a better file name than simply a number.... IIRC
> there is a standard function to create a unique file name in a
> directory... (not sure if you can use it because of win32, though)

Again, these are part of the plan to fire-and-forget the upload network
thread.  I really hate threads, so am determined that it not have to
communicate back to the parent.

> seriously, though, this patch is good enough for inclusion as it is, but
> since I won't commit it before 1.1.0 is out, let's use the time to make
> it even better :)

Sure!  Thanks very much for your comments, I'll post the next one soon.

> Hope this helped

Absolutely; double thanks as reading other people's code is never fun.

Cheers!
Rusty.
-- 
 ccontrol: http://ozlabs.org/~rusty/ccontrol


Reply via email to