D5693: First attempt to port remaining dbo* statements to qCDebug().

[Ian Wadham]

Hi Johan and thanks for your reply and for all the porting work you are doing,

Sorry to have been a bit slow getting back to you, but I have had a very busy
last few days.

On 05/05/2017, at 4:06 AM, Johan Ouwerkerk wrote:

> On Wed, May 3, 2017 at 10:03 PM, Luigi Toscano <luigi.toscano at tiscali.it> wrote:
>> Ian Wadham ha scritto:
>>> 
>>> Re the current issue, it might be unwise to replace all occurrences of
>>> "dboN" with calls to qCDebug(), unless the "KSudokuLog" parameter
>>> exactly reproduces the behaviour of debug levels and macros defined
>>> in file "generator/debug.h".  If log messages appear as "all or none",
>>> you will either be flooded with (possibly millions of) messages from
>>> the KSudoku solver or receive no messages at all.  Neither of those
>>> alternatives would be any use if a bug appeared in the main solver ---
>>> and the "debugging on" alternative would be a serious drag on resources,
>>> because that solver is invoked multiple times when generating a puzzle.
>>> 
>>> Also, if Johan goes ahead, lines like the following would need attention:
>>>    if (dbgLevel >= 2) {
>>>        this->print (boardValues);
>>>    }
>>> 
>>> It might be easiest just to redefine the "debug.h" file in terms of qCDebug().
> 
> This is somewhat difficult.
> With the structured logging approach, messages should be filtered out
> at runtime instead of compile time based on user-configurable
> settings.

Wait till you come to the "author's debugging tools" in KGoldrunner… ;-)
If enabled at compile time, they can be operated by keystroke at runtime.
Conventional logging to stderr (or console) is simply too slow.  It can
change the timing and make the bug you are looking for disappear.

> Hardcoded 'debug levels' run counter to that idea.
> 
>> 
>>> Similar remarks apply in games where I used the "debug.h" approach to
>>> debug details of animation frames (Kubrick? KGoldrunner?).
>> 
>> I disagree that using different systems for logging at the same time
>> (printf/fprintf/qDebug) only adds more complexity. On the other side, it is
>> possible to select different levels by defining different logging categories,
>> and this is something that we should consider for the logs of the solver. I
>> rechecked also the logging messages ported from kDebug/qDebug/dbe (sometimes
>> the same log was sent through qDebug and dbe) and I think that other levels
>> are not required there, but that's open for discussion.
>> 
>> --
>> Luigi
> 
> To avoid flooding the log, multiple logging categories are certainly a
> good idea.
> We could use a KSudokuDebugLog like category with a default filter
> that lets no messages through.
> Then we could log to that category at info (qCInfo()), warning
> (qCWarning()) and debug (qCDebug()) levels depending on the log
> message.

I don't think you should be spending a lot of time on this, as opposed to the
many other porting tasks remaining in KDE Games.  The "dboN" code was
written as "throwaway code"  to help write, test and debug the first KSudoku solver
I wrote (as in ksudokuboard.cpp).  Ordinarily I would delete such code before
release, but I played safe and left it in, just in case there was a serious bug I had
missed.

In the event, the only complaint (on bugs.kde.org) was that the KSudoku solver's
KMessageBox messages sometimes say that it had to guess when a human does
not, because he/she has other logic tricks to apply that are not included in the
solver's code.  Maybe those messages should be re-worded…

So I should have deleted or commented out the logging code years ago…

Maybe the best thing would be to leave in the level 1 debug messages as
qCDebug() info messages and comment out or delete all the others.  Then,
if someone does get an invalid puzzle generated, there will be some hope
of finding the iteration at which the algorithm failed and then re-enabling the
higher-level logs (more and more detail) to elaborate that.

OTOH, puzzles are generated in release versions by taking a random seed
from the system clock, so there is little hope of reproducing an error unless it
affects some specific type of puzzle and not others.

> Perhaps it is best to move this discussion to Phabricator:
> https://phabricator.kde.org/D5693 & https://phabricator.kde.org/T5980
> That way we keep the discussion and ongoing revisions to the code in
> the same place.

Sorry, no.  See my other email, in reply to Albert's comment about that.

Cheers, Ian W.