Opened 9 years ago

Closed 9 years ago

Last modified 13 months ago

#4925 closed defect (fixed)

error() sometimes doesn't show up but ScummVM terminates

Reported by: m-kiewitz Owned by: sev-
Priority: normal Component: Engine: SCI
Keywords: Cc:
Game:

Description

Hi,

we talked about this yesterday on IRC. Sometimes at least for SCI engine, an error() doesn't make the debug console popup, but instead ScummVM terminates.
This is caused by if (debugger && !debugger->isAttached()) in static void defaultErrorHandler(const char *msg) (engines/engine.cpp)

It seems debugger is never detached.
Just put an error() inside reg_t kGraphSaveBox(EngineState *s, int argc, reg_t *argv) of sci engine (sci/engine/kgraphics.cpp), then run almost any game, enter debugger, exit debugger and wait for the error() to occur. At least here ScummVM exits immediately instead of showing the error().

Thank you

Regards
M. Kiewitz

Ticket imported from: #3030449. Ticket imported from: bugs/4925.

Change History (7)

comment:1 by fingolfin, 9 years ago

This is a bug in the SCI console, I think. It does not detach itself -- basically, any Cmd_FOO(argc, argv) function which wants to *exit* the debugger (as opposed to just step over a single instruction) should not simply "return false", but also set _detach_now = true;. An easy way to achieve that is to replace "return false;" by "return Cmd_Exit(0,0);".
We should probably add docs on that.

comment:2 by m-kiewitz, 9 years ago

Owner: changed from sev- to m-kiewitz

comment:3 by m-kiewitz, 9 years ago

thx for the help, max
will fix this

comment:4 by fingolfin, 9 years ago

No problem.

To be clear, maybe I shouldn't have called it a "bug in the SCI console", but rather under-documented behavior of the GUI debugger/console that very easily leads to bugs in actual console implementations" ;).
So, ideally, we'd also improve the docs for this in GUI::Debugger and add a method to it dedicated to "exiting" the debugger vs. "temporarily resuming code execution". Calling Cmd_Exit like that is maybe not so nice, and I think _detach_now should ideally be made private (some engines, like Kyra and SCUMM, already use it and set it to true, but that would be better done by an appropriate method).

comment:5 by sev-, 9 years ago

As I see now, all of these were addressed. I.e. SCI engine fixed, and API for debugger updated.

comment:6 by sev-, 9 years ago

Owner: changed from m-kiewitz to sev-
Resolution: fixed
Status: newclosed

comment:7 by digitall, 13 months ago

Component: Engine: SCI
Note: See TracTickets for help on using tickets.