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

[Max Horn]

Am 01.06.2011 um 15:11 schrieb A. Milburn:

> Hi Max,
> 
> Thanks for the reply!
> 
> On Wed, Jun 01, 2011 at 02:42:50PM +0200, Max Horn wrote:
>> 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. 
> 
> LordHoto's comment on the forum was "Because the sdl-config script of the
> prebuilt SDL development files for MinGW does pass -mwindows to the linker".
> 
>>> 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.
> 
> It isn't clear to me from the "Many of the current warnings (i.e. loading
> related, missing code or features) in source code, can provide useful
> feedback to users" section of the console discussion on the wiki whether
> this is generally agreed on or not, so I thought I would be clear about it.

Well, I don't see anybody debating there that it is important that users *can* see the warnings, if they need to / are asked to. What people could not agree on is whether this is an argument that the windows console must be visible at all times. Indeed, some people (MD5 and me at least) thought that for most users, most of the time, seeing those warnings is not that helpful, because we generate far too many not very important warnings.

I also still think that any truly important warning should be shown to the user properly by a dialog... But in all this, I see nobody arguing that we don't need to provide any way for the user to find out about warnings :). Indeed, we added the logging code specifically to ensure warnings can be read, if necessary.



> 
>> 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.
> 
> Right, I was definitely thinking of it just on top of whatever else. But if
> it's not rejected, it would be a quick fix to allow people to look in the
> logs for the md5s.

Sure. If nobody objects soon, feel free to go ahead and implement it!


> 
>> 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.
> 
> That would be very nice. One annoying thing about the LB detector right now
> is that it sometimes duplicates the output of the Advanced Detector's own
> md5 mismatch code - because it seems to make much more sense for LB to try
> and do a generic fallback which matches all LB games, rather than a
> ADFileBasedFallback with a huge list of known filenames as well as the
> fallback detection for anything which isn't known, but maybe that's silly..

What you say sounds quite sensible to me. For some games, the ADFileBasedFallback code may make a lot of sense, but it certainly is not a perfect solution for everything. Indeed, I have contemplated of untangling it a bit further from the rest of the advanced detector code, to make it even more "optional".


> 
>> 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
> 
> I would personally prefer the option to have both, at least when not
> mass-adding,

Well, even for a single game, the information we currently printf can be 10 lines or more, with MD5s for multiple files. Hrm.

> but I can't say I mind too much - but you'd presumably want the
> plain md5 view for non-mass-add so that people could take photographs or
> write down things from their non-desktop devices?

The "photograph screen of non-desktop devices" is a good point, hadn't thought of that. Still I don't think the average user will appreciate seeing all this MD5 information, even for a single game... (but maybe I am totally mistaken here). Maybe we can not show it by default, but have a "show details" button that opens another dialog with more information, maybe even scrollable? Does that sounds acceptable, or is it now too much of a "compromise trying to please everybody" solution?


> I don't know how annoying
> it is for people to have to install and run ScummVM on a desktop box with
> the same data. Maybe it's a non-existant problem.

Maybe... we really would need testers for all this, it's easy to design solution in theory, but how well they work in practice is another question.

Still, we gotta do *something*. And it shouldn't be hard to implement multiple different methods, then produce one binary for each, and then let users try them.

I.e. one version shows a dialog like the one I suggested; one also has a button with "show details", and one shows the details right away in the dialog. Well, maybe only the first N lines. or if somebody implements a scrollable text field, we could show 'em all. Taking the console GUI code as a start for that might be one possibility.



>> 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.
> 
> Mostly I was thinking of people e.g. being able to take photographs when an
> error pops up..

Yeah, good point.

> this could be replaced with (for example) an error dialog
> with a scrollable text widget with the contents of the log? But I suppose
> that the console would still be very helpful, and we could always *add*
> trivial copy/paste if we add clipboard code to the backends anyway?

Sure, having copy&paste support on the console at least would be swell; pasting text seems easy enough. It's the "copying" part I am a bit scared off ;). Implementing full blown text selection sounds like adding heaps of code. But maybe we could have a mode that copies the whole text of a given dialog / copies the full content of the console (or maybe only the active line, or whatever).

Alas, I think the copy&paste code is somewhat orthogonal to the rest; I would prefer to worry about that later, if ever (or let somebody else worry about it).

> 
> (If some errors are caused by being out-of-memory then perhaps this isn't
> a good solution. I don't know what typical errors from all the engines /
> platforms are like.)

Memory errors of course also occur. Well, we can't solve every problem, and not right away, but if we can improve things for 98% of all cases, that's already pretty good ;). No need to wait for a perfect solution, either.

> 
>> 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 ;)
> 
> I seem to remember that Ctrl-D conflicted with a built-in game keystroke of
> some kind. And I don't know if it works on handheld devices?

Handheld devices have a big problem with that anyway. Just like with all games that use keystrokes to activate thing. This is a *major* problem, and one of the most important things in ScummVM development is still implementing a way to fully address it. We do have a "keymapper" in our code base, but I don't think anybody works on it, and it's only marginally useful in its current form, I feel: It requires to much manual user input, and has too little automatic powers. What we really could need is a way for engines to specify for each game (variant) what inputs it needs, and for what (roughly, at least). This then can be used to make a sensible default mapping (being able to edit this default with the keymapper of course would be a desirable extra feature). Note that EVENT_MAINMENU and EVENT_RTL are kind of attempts to bypass this shortcoming in my eyes.

Alas: This is now taking us too far afield, we could fill several threads with just discussing this stuff ;).

Back to your more immediate desire: We could of course add EVENT_DEBUG and allow backends to choose a suitable way (if any) to tigger it (e.g. Ctrl-F6 on desktops ?). I am not sure this feature is that important that we'd want to do this, but if nobody objects / proposes a better way, I would have no qualms with implementing it that way, at least for the time being. *If* we ever come up with a better way to assign such global controls, we can still get rid of EVENT_DEBUG again.

But I am kind of expecting some people to shout out now ;).

> 
>> 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.
> 
>> 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 ;).
> 
> I was thinking we could just name the log files with ".txt" and then call
> xdg-open, which I hope is present on most Linux machines, or Notepad on
> Windows, or TextEdit on Mac, or whatever the equivalent is on other OSes.

Sure, why not. So, we'd add a new OSystem API for that? Like
  virtual void OSystem::openLogFileInEditor() {}
or so...



Cheers,
Max