#4772 closed defect (fixed)
Gob: ScummVM quits on pause
| Reported by: | Templier | Owned by: | lordhoto |
|---|---|---|---|
| Priority: | normal | Component: | GUI |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
When pressing CTRL+p, ScummVM exits with an error: "Could not load widget position for 'PauseDialog'"
Traced to the following code: PauseDialog::PauseDialog() : GUI::Dialog("PauseDialog") in gob.cpp:87 which will cause the error to be thrown in GuiObject::reflowLayout() because PauseDialog is not a valid dialog name.
This is working properly in 1.0, but not with the trunk.
Ticket imported from: #2954286. Ticket imported from: bugs/4772.
Attachments (1)
Change History (9)
comment:1 by , 15 years ago
comment:2 by , 15 years ago
Calling virtual functions from the constructor causes this behaviour because the vtable likely isn't yet initialized fully. (And even if it was, it would call the subclass' methods before the subclass' constructor has executed, which likely isn't desired.)
by , 15 years ago
| Attachment: | gob-construct_pause_dialog_using_coordinates_instead_of_name.patch added |
|---|
Use the other constructor to create the Pause Dialog
comment:3 by , 15 years ago
If the dialog name isn't used anywhere else (and it doesn't seem to be), we can just call the other constructor with "default" coordinates, the same way the Mohawk or scumm engines do.
The one line patch fixes the problem for me, but since I don't know much about the GUI subsystem yet, somebody with better knowledge might want to check for side effects.
comment:4 by , 15 years ago
wjp: Actually calling the virtual method is not the problem over here. It should be perfectly fine, when just GuiObject::reflowLayout is called, due to the way our widget sizes are calculated. (And it was never meant to call the subclasses reflowLayout at that point :-).
Littleboy's patch looks perfectly fine, that's how it should be done in case Gob's dialog auto-calculates it's size like SCUMM does. If that isn't the case we also have to add auto-calculation like in SCUMM...
Assigning to DrMcCoy so he can take care of that.
comment:5 by , 15 years ago
Just to explain a little more: using a "fake" dialog name is not correct, a dialog name should always match an entry in the theme file. Thus even if the Gob's code uses it's own reflowLayout to do auto calculation, it should either call the coordinates taking constructor, use a real dialog name described in the theme file or use an empty dialog name.
Though I can understand that you might want to call this "messy" ;-).
comment:7 by , 15 years ago
| Owner: | set to |
|---|---|
| Resolution: | → fixed |
| Status: | new → closed |
comment:8 by , 6 years ago
| Component: | → GUI |
|---|
That error message seems to come from the standard GuiObject::reflowLayout() function. Which strikes me as a bit odd, because it's a virtual function and the gob engine's PauseDialog class defines its own.
Apparently, when the dialog's constructor runs, and it eventually calls GuiObject's constructor, it calls reflowLayout() (that line of code is new), which is the one in GuiObject, not in PauseDialog. So apparently I either don't understand virtual functions as well as I hoped, or I'm missing something glaringly obvious.