SCI: GK1: Gabriel's mouth doesn't move during final CD scene

[sluicebox]

Gabriel's mouth doesn't move during the final scene in CD versions on ScummVM but it does on Sierra's interpreter. Grace's mouth moves on ScummVM but she's facing away so there's not much to sync. Console logs:

WARNING: Sync::start: failed to find resource sync36.670(1, 0, 1, 1)!
WARNING: Sync::start: failed to find resource sync36.670(1, 0, 1, 2)!
WARNING: Sync::start: failed to find resource sync36.670(1, 0, 1, 2)!
...

Floppy works but it's not trying to sync against speech.

To reproduce just use either debugger to warp to room 670.

SCI Companion also can't find lip syncs for those messages. If they're not there then maybe Sierra's interpreter falls back on unsynchronized mouth movements?

[m-kiewitz]

This is really weird.
I guess we should check if the mouth movement is random/pseudo-random or lip-sync'd.
This may even be a special resource file case (we had something like that multiple times already, one game had certain resources multiple times inside the resource file and we took the wrong one).

At least SCI16 (up to 1.1) did this fall back inside the scripts, so it should do the same for us. There may also be simply some inaccuracy in our VM, which causes the fallback not happening.

I guess a proper video of the original interpreter behavior on this would be quite nice to have.

For King's Quest 6 some data is actually missing. In KQ6 case there are 2 types of sync data. One the regular SCI one, and then special hi-res data. I thought about recreating those, when one of them is missing. Gets really irritating at the end of the game. This of course is not the case for GK1.

[m-kiewitz]

Have you tried SCI Viewer as well?
I'm almost always using this one for checking. But even SCI Viewer also has some issues with certain resources.

[sluicebox]

This is my first time dealing with the base36 resource names or sync data so I'm going slow but here's what I've found so far...

There are 8 sync36 resources for this scene but they all exist as patch files in the AUD dir on the CD. There are none in the resource volume files for this module. SCI Viewer can see the resources in the patch files and I probably just didn't have the patch files in the right place for SCI Companion to find or maybe it doesn't know that patch file name format.

The sync36 resources are only for Gabriel's messages, which makes sense since Grace is facing away, though the side of her mouth does animate. One exception: there's no sync resource for Gabriel's first message, sequence 2, which is just "Yes". Sierra's interpreter doesn't animate his mouth for this line. So there is indeed one missing resource from the game and we can rule out Sierra's interpreter falling back on default animation.

So ScummVM isn't finding Gabriel's sync data that is there and not animating him and it's logging warnings about Grace's sync resource not being there, which there shouldn't be, but animating her anyway.

Another wrinkle, the alternate ending where Gabriel dies has Mosely in his place. There are no sync resources for his lines, though it seems like there should be, but his mouth is still animating in ScummvM and Sierra's interpreter. ScummVM logs that the sync resources are missing for this too.

[sluicebox]

To test the Mosely sequence set flag 407 before warping to room 670

[m-kiewitz]

Can you please copy the files into the main gabriel knight directory (the one with the resource files). If they are still not found, something about patch file decoding is going wrong.

[sluicebox]

It's a bug in the name matching.

ResourceManager::readResourcePatchesBase36() is looking for all sync36 resources which start with the "#" character: SearchMan.listMatchingMembers(files, "#???????.???")

The problem is that this ends up in Common::matchString() making the comparison, which treats "#" as a wildcard to mean any digit, and so it returns false and the sync36 patch files never get loaded.

case '#':
	if (!isDigit(*str))
		return false;
	pat++;
	str++;
	break;

[m-kiewitz]

Yikes, thanks for checking that.
That may be responsible for some other problems too.

[m-kiewitz]

I see, it seems this got broken in 2016 probably.
06641f29a7bdcda280b0291f215193f055c83969

This extra functionality was added back then. I don't see some escape for this :/

[m-kiewitz]

Maybe we could check all files, that are using the full 8+3 bytes as name and then check locally.

Wjp, what do you think about it?
Adding an escape would also be possible, for example adding "\" as prefix for escaping.

[sluicebox]

​https://github.com/scummvm/scummvm/pull/1521

[Filippos Karapetis <bluegr@…>]

In 08ae526:

SCI: Fix sync36 patch files not being loaded

Fixes GK1 bug #10788