[Scummvm-devel] consoles, warnings, errors and so forth

[Max Horn]

Hi fuzzie,

glad you wrote this email! I was thinking about writing one on the subject, too, and you saved me a lot of work :).


Am 01.06.2011 um 12:37 schrieb A. Milburn:

> Hi all,
> 
> We semi-recently had a discussion/argument about disabling the Windows
> console by default. I am somewhat wary of returning to the topic, since
> it ended up being somewhat of a flamewar, but..

I don't think there is still heat left in that, but we'll see. My impression was that people were mostly agreeing on what to do, except for Kirben. Then again, maybe I misremember, and of course I am also highly biased.

Alas, let's turn to more fruitful points.

> 
> The (accidental) disabling of the Windows console in recent builds

OOps, gotta interrupt you here for another important point: While I propose to disable the console on windows by default, the plan was to do this after we implemented various precautions (as discussed below). It being off now is really an accident, one that I would like to see fixed ASAP until we have a proper solution in place.

If somebody here can tell me why exactly the console is off and how to re-enable it, please let me know. Even better, just implement the fix. 


> reminded
> me about it, since it's more difficult for users to provide needed info
> now -- somewhat because there's no code to *enable* the console, but since
> this is also a problem on Linux and etc, and there's no way to magically
> enable a console at runtime there (and distros are unlikely to provide a
> 'with console' shortcut), it brought me to thinking about it again.
> 
> It's also important to me that I be able to ask users which warnings they
> saw from scummvm when they have problems - although not necessarily in
> real-time.

Sure, I think we all agree on that.

> 
> So I'd really like to discuss again the idea of removing the need for
> an external console at all.
> 
> My immediate thoughts about *solutions* are:
> 
> * Can we fix the Advanced Detector so that it calls logMessage for unknown
>  games etc? Perhaps we need to add a kInfo type for these messages?

I originally suggested an kInfo type for that, but I think it was rejected back then, forgot why exactly... I think one of the reasons was that we don't want to put non-warning non-error user info into the logs exclusively. Anyway, sure, in principle, we could do this.

>  (It would also be nice to find a way for a fallback detector to do the
>  same, at the moment I have one for Living Books sitting in my tree
>  because I don't really want to add it with printf()s.)

Yeah, in-tree we have three places in engines/ that still use printf, all of them are related to (failed / fuzzy) game detection. The other two are in SCUMM and AGI. 

We could try to add some code that allows to unify these three use cases (and maybe yours in LB) in a good way. One way: Just add a function somewhere that does the logging and optional GUI reporting. Another idea is to remove this output completely from the detectors. Instead, augment the data they return to contain information about the (failed) detection. Then, the code which called the detectors could (a) log it, in a uniform manner, and (b) display suitable GUI dialogs to the user containing the information in an appropriate form.

> 
> * Could we furthermore add a GUIWarningMessage function which displays a
>  dialog for such cases? And fix both that and GUIErrorMessage to
>  also call logMessage and print to the console?

We can do that, but I feel it is not enough. What would this dialog display? I see two basic options (with tons of variations):
a) detailed information about the issue (e.g. including the MD5s),
b) a pointer to the log file.
(of course, they can be combined, too)

Both have their issue. a) is problematic because with mass add, there could be hundreds of unknown MD5s; we don't want to pop up one dialog for each; we can't fit all this into a single dialog (well, we could add a scrollable text viewer widget, I guess). Also, without a way to copy& paste information, it seems even less useful.
Of course the latter could be mitigated by offering a "Copy to Clipboard" button, as was discussed previously. 

Approach b) seems better to me, at least on desktop machines. Show something like this:
 "While adding new games, ScummVM noticed some file that look like currently unknown game variants
  (King's Quest 4, Monkey Island and 5 more).
  Please report them to the ScummVM team, by sending us your log file found at
   C:\Path\To\LogFile.txt
 "
Ideally with an "Open log file" button (on desktop machines) that opens the log file in a text editor.


Some other options (for desktop machines, at least) we discussed including an "Report by email" button (which opens the user's email client and pre-fills an email with the MD5 info), but that is problematic for various reasons. Another is to automatically submit this via a web form, but this is easily abused, so no good either.


> 
> * Does it make sense to move the printf() functionality out of all of these
>  callers and into logMessage anyway? I'm thinking of error, warning, etc.
>  .. if we do that, can we stop warning() from being included in an #ifdef
>  DISABLE_TEXT_CONSOLE and do that work in the relevant backend?

We already removed most printf's by using OSystem::logMessage (as you recognized ;). But there are a few remaining use cases, esp. in base (and of course the ones in engines), which do not seem suitable for logMessage in its current form. E.g. the command line usage instructions. I gu. We never finished deciding how to handle the remaining printf's. I'd be very happy to proceed with this project, but except for LordHoto implementing the Logger functionalit, it seemed that there was little interest for this whole project, so I kinda gave up on it. (It doesn't really help to discuss these things with yourself alone).

So, yeah, in my dream world, there would be *no* (f)printf left outside of backends/ (and *maybe* base, for command line interactions, where it kinds seems silly to insist on doing without printf).

> 
> * Could we make logMessage also add lines to the in-game debug consoles, at
>  least in the case of warnings, so that users can simply look there?

We could do that, sure. Although right now there is no console in the launcher, and you can't open the console if an error dialog is up. Also, there is no copy & paste in the console, and it is overall rather limited. Yet esp. on handheld devices, this might be quite helpful.


>  And perhaps have a uniform keystroke for opening the console?

Many games seem to support Ctrl-D, but not all. A uniform keystroke (as with the GMM) would be a good thing. You got my support ;)

> Maybe even a little fade in/out warning icon or similar? (Okay, maybe that last one
>  is crazy).

I don't really understand what you meant here, so I really can't say how crazy it is :)

> In any case, it would be neat to have a backlog of warnings
>  before an error, given the errors *do* end up in the in-game debug consoles.

Aye.


> * If we add something like a GUIWarningMessage, could we also look into
>  adding a simple button on supported OSes (Windows, Mac, Linux, etc) which
>  offers to open a text editor with the contents of the log file?

That would be nice. And easily doable on Mac, probably on Windows, not so sure on Linux. Alas, this would be encapsulated in an OSystem API, and I think it would be good to have this, at least everywhere we can implement it.


> It is
>  not very discoverable otherwise, and seems like a much more immediate
>  solution than the web browser form submission discussed on-list, but still
>  more usable than Max's proposal to provide instructions for finding it.

For Unices in general, I see no alternative to providing the path to the log in the dialog / providing instructions to find it, sadly. But for Windows & Mac, an "open log" button would clearly superior. Let's do it ;).


> These are perhaps not very well thought out (although I would really appreciate
> a nice way to get warnings on the in-game debug consoles) but hopefully all of
> them would be relatively easy to implement.. but since most of it is fairly
> invasive and people might not be happy with it, I thought I'd better bring it
> up for discussion here rather than just complaining on IRC!

Thanks for bringing this up. And if you know want to work on some of this, or coerce some people to work on it... ;).


Cheers,
Max