Opened 2 years ago

Closed 7 months ago

#10241 closed defect (fixed)

SCI: SQ4: Vohaul's taunt to Roger bug

Reported by: EricOakford Owned by: sluicebox
Priority: normal Component: Engine: SCI
Keywords: has-pull-request Cc:
Game: Space Quest 4

Description

ScummVM: 1.10.0git5038-gd9dfca2fcb
Game: Space Quest IV Windows CD 1.0
Platform: Win7-64

When Roger plugs the PocketPal into an outlet for a second time, Vohaul appears on the screen, and taunts Roger by showing Roger Jr. in the beam. The problem is, if either text-only or both text and speech is selected, the game just briefly shows the associated text, then exits the screen. However, if speech-only is selected, the scene plays out just fine.

Attachments (1)

sq4-cd-win.032 (31.4 KB ) - added by EricOakford 2 years ago.
Savegame of Roger about to plug in the PocketPal a second time

Download all attachments as: .zip

Change History (10)

by EricOakford, 2 years ago

Attachment: sq4-cd-win.032 added

Savegame of Roger about to plug in the PocketPal a second time

comment:1 by m-kiewitz, 2 years ago

Are you able to check, if this also happens when using the original interpreter?

comment:2 by m-kiewitz, 2 years ago

Really weird, I checked using original interpreter and even with really high cycles it keeps them on screen for way longer. Will have to dig in.

comment:3 by m-kiewitz, 2 years ago

Hmmm, it seems it's sync'd with music and the music resource (851d for example) creates a ::cue basically immediately.

May be caused by our current inaccurate MIDI parser code, which uses a ScummVM class to do it and it's not working exactly how the original interpreter did it.

This one also is a special script, which will need some adjustment in any case to do DUAL mode properly. vohaulScript::changeState, starting at state 1 in script 545.

comment:4 by m-kiewitz, 2 years ago

I just checked the music resource.
It's simply silence for a specific amount of seconds.
It makes no real sense to do it that way. Sierra could have simply used ::seconds or ::ticks instead.

It's surely the Midi parser code, but as I said - I will have to make it work for Dual mode anyway by using a script patch. I can simply fix this too and do it properly, that should even save quite a few bytes.

comment:5 by EricOakford, 2 years ago

The bug does happen in the original Windows interpreter if Text is selected.

comment:6 by sluicebox, 7 months ago

I ran into this a while ago, yikes is it intense. I have a patch that fixes this, though it needs some final polish and loads more testing, but oh my gosh it's more a script rewrite than a patch.

It looks to me like when they were converting scripts for the CD version they got to this one and said "i'm not touching this mess". Instead it's a "if speech then new-code else run-the-old-code-as-is", which is unusual in this game. The old text-mode code not only has empty midi tracks for timing but also lots of leading space characters for placing text on screen. I think if this script were less fragile they would have just upgraded it to use the new FaceTalker class, like they did with the other scripts such as zondraTalker in room 320, in which case it would automatically work in dual mode. (btw I love dual mode )

Despite all that, the midi timing is working correctly in ScummVM, not that we want to use it.

The real obstacle to fixing this is that the audio and text for Vohaul's second and third messages are out of sync. The audio for the second message contains *both* messages, and there is simply no audio resource for the third message. Ain't nothin' easy.

My patch rewrites the script to use tVOHAUL, a FaceTalker, in all modes, and passes the (many) text formatting parameters it needs to draw the text in the same way that the room's text-mode code does. This automatically gets us the right behavior in all three modes. To handle the broken 2nd/3rd messages it sets a timer the length of the first half of the 2nd message's audio, and if that fires before the user dismisses the message it hot swaps the on-screen text with the third message if in text or dual mode, a feature not supported by FaceTalker. It also works around FaceTalker's message-delay calculation coming up with a value that's too short in text-mode since it's based on string length and this broken message only has half the message in its text.

Anyway, this one almost broke me, and it still might.

comment:7 by sluicebox, 7 months ago

Keywords: has-pull-request added
Owner: set to sluicebox

https://github.com/scummvm/scummvm/pull/1569

This fixes text+speech mode. I wasn't able to reproduce the text-only mode timing problems but if they haven't been fixed yet this will fix them too as it removes the MIDI timing.

comment:8 by Filippos Karapetis <bluegr@…>, 7 months ago

In 3dc0c3ec:

SCI: Fix SQ4CD Vohaul pocketpal text+speech mode

Fixes a scene which is incompatible with our text+speech mode, bug #10241

comment:9 by bluegr, 7 months ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.