Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#4964 closed defect (fixed)

KQ5CD: ScummVM freezes in dark forest

Reported by: SF/kuroshiro Owned by: bluegr
Priority: normal Component: Engine: SCI
Keywords: script Cc:
Game: King's Quest 5

Description

Game: King's Quest 5 (CD/Dos/English)
ScummVM Version: 1.2.0SVN 51294
OS: Windows 7 x64

In the dark forest, if you go to the witch's hut before killing her, ScummVM will freeze up and have to be closed. It doesn't crash -- just locks up, and then the console spits about a bunch of kCan(t)BeHere - invalid rect warnings.

I'm pretty certain it is caused when the witch is supposed to appear on that screen.

Ticket imported from: #3034714. Ticket imported from: bugs/4964.

Attachments (1)

kq5.003 (59.2 KB ) - added by SF/kuroshiro 9 years ago.
walk left to see the bug

Download all attachments as: .zip

Change History (10)

by SF/kuroshiro, 9 years ago

Attachment: kq5.003 added

walk left to see the bug

comment:1 by sev-, 9 years ago

Component: Engine: SCI
Game: King's Quest 5

comment:2 by SF/kurufinwe, 9 years ago

I encountered the same problem with the floppy version (DOS/English), so this bug is not limited to the CD version.

comment:3 by wjp, 9 years ago

For what it's worth, it hangs in witch::findPosn()

comment:4 by digitall, 9 years ago

Still occuring as of today with CD version on:
ScummVM 1.3.0git3524-gb1055a3-dirty (Mar 2 2011 19:31:35)
Features compiled in: Vorbis FLAC MP3 ALSA SEQ TiMidity RGB zLib FluidSynth Theora
on Linux x86_32.

comment:5 by bluegr, 9 years ago

This occurs because script 200 calls its superclass (via op_super), which in turn calls script 994, which after calling 999, it deletes the calling script 200 via kDisposeScript.

Hacking kDisposeScript to ignore script 200 gets the game in an unwinnable situation: when entering the room where the witch is supposed to appear and kill the player (room 22 I think), the witch never appears, the game doesn't freeze, but Graham won't enter her house, and she's nowhere to be found to be given the bottle. Thus the player is effectively stuck.

This might be related to what we're doing in kDisposeScript:
int script = argv[0].offset;

SegmentId id = s->_segMan->getScriptSegment(script);
Script *scr = s->_segMan->getScriptIfLoaded(id);
if (scr) {
if (s->_executionStack.back().addr.pc.segment != id)
scr->setLockers(1);
}

Effectively, we're forcing a script to be unloaded unless it's the calling script. Disabling this code makes the game enter the aforementioned unwinnable situation.

The same code seems to be responsible for the crash in Hoyle 3 (bug #3038837), which is currently avoided with a hack inside SegManager::uninstantiateScriptSci0(). So, perhaps the correct solution here is to perform additional steps when forcibly unloading a script, in order to take care of the other scripts that locked the one requested to be unloaded.

comment:6 by wjp, 9 years ago

Some random observations about things that affect this:

Put a breakpoint on witch::cantBeHere. Walk west. The first time this breakpoint triggers after the room changes:

If you set witch::blocks to 0000:0000, the hang disappears.

Or, if you set witchCage::bottom and witchCage::right both to 150 (so that witch::br* is contained inside witchCage::*, the hang disappears. (But the witch's position is different than above.)

---

I'm not sure what the logic in this function/room is intended to accomplish. It seems to conclude the witch is in an invalid position if she's not inside witchCage, but witchCage is (0,0,0,0) at that point. I have no idea what the expected behaviour here is, and if there's anything wrong at this point.

comment:7 by wjp, 9 years ago

I looked at the scripts involved together with waltervn a bit last night.

It seems the intended behaviour is to make sure the witch appears inside the witchCage.
However, the witchCage object is weird. It appears to have 12 variables in the raw data rather than the 8 it should have given its base class. The 4 'extra' variables appear to be valid dimensions for a rectangle.

I'm not yet sure if we're handling this object wrong, and/or if the witchCage object somehow never gets added to witch::blocks in the original due to some failed sanity check somewhere.

I did already verify that the object appears as-is in SSCI's memory around the point where we hang, so both the (0,0,0,0) and the 4 extra variables are there.

comment:8 by bluegr, 9 years ago

Fixed (with a script patch) in rev 2a37ed3. Thanks to wjp
[18:07] <CIA-92> for his help and work on this

comment:9 by bluegr, 9 years ago

Keywords: script added
Owner: set to bluegr
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.