SCI: QFG4 v1.0 Thief Guild Color Puzzle crash due to ScummVM script patch

[yacobean27]

In Quest for Glory 4 v1.0. In the Thief Guild there is a Barrel. Once you solve all the Thief guild puzzle you can move that Barrel and a Color puzzle comes up. Once you solve the the puzzle the game will crash with Error:[qfg4 340 call 4b5 @ 04ba]: lookupSelector: Attempt to send to non-object or invalid script. Address 0000:0000! What should happen is the Wall should open up and the Guild Master should appear. This only happens in ScummVM (PC). The issue doesn't happen in DosBox. I will attach a save file right before the puzzle. Click on the Barrel. The puzzle is
Blue Blue Yellow
Green Green Green
Red Green Red

[sluicebox]

Regression from script patch: 603115dfca61446439973689b15f713e7de96edf for bug #9894.

This is a very large script patch written by a contributor six years ago. The crash is coming from within a script function that the patch invents. Unfortunately, no one on the SCI team knows anything about this, and I cannot overstate that this patch is mammoth and terrifying. Presumably it works in other versions, and there's a 1.0 incompatibility, but wow this will take a lot of time to figure out what's going on here, and it does not look fun. Don't hold your breath!

Thank you for reporting this. 1.0 can crash itself just fine on its own, it doesn't need our help.

[yacobean27]

I was hoping it was going to be something easy. If it's going to be a pain to fix, I'll close the case. There's no reason to spend all that time fixing it for only a few people that play 1.0. Thanks Sluicebox for all you do!

Replying to sluicebox:

Regression from script patch: 603115dfca61446439973689b15f713e7de96edf for bug #9894.

This is a very large script patch written by a contributor six years ago. The crash is coming from within a script function that the patch invents. Unfortunately, no one on the SCI team knows anything about this, and I cannot overstate that this patch is mammoth and terrifying. Presumably it works in other versions, and there's a 1.0 incompatibility, but wow this will take a lot of time to figure out what's going on here, and it does not look fun. Don't hold your breath!

Thank you for reporting this. 1.0 can crash itself just fine on its own, it doesn't need our help.

[eriktorbjorn]

I assume "1.0" refers to the floppy version? My CD version also calls itself "1.0", which is confusing. Not to mention that the Quest For Glory Anthology and the Quest For Glory Collection Series aren't quite identical, though the difference most likely isn't visible from within the game. And the GOG version includes "one of NewRisingSun's script level timer bugs patches", just to confuse things even further...

[sluicebox]

Oh this is a serious bug. Broken script patches are unacceptable!

The crash is due to a call instruction with a hard-coded offset that is based on an unenforced assumption about the script's layout and offsets.

I don't think this patch is fixable. Even if we tighten up the signature on the incompatible patch so that it doesn't match against floppy 1.0, there are three separate patches, plus a variant, and some of them would still match and then we'd have an unexpected frankenstein that I'm sure will break in other ways.

I think I could replace this patch with a completely different set that would work on all versions and fail gracefully if only partially applied. It would take a lot of work and not be particularly fun. But when I look closer at this script bug, I'm not sure this behavior should have been altered at all.

The Thieves Guild has a secret passage that doesn't allow you to immediately walk out of it when you open it. If you re-enter the room then the passage allows you to walk out of it. That is certainly a bug, but there are two ways of interpreting it:

1. Sierra accidentally forgot to enable passage-walking when opening the passage door (revealing a giant insect blocking the passage)
2. Sierra accidentally enabled passage-walking on room initialization *even though there's a giant insect blocking the passage*

The original bug reporter and the patch author assumed the first interpretation, but I think the second is more likely: that the passage was supposed to be a reward for curing the Chief, by clearing the obstacle of a giant insect. That is consistent with sCureThief being the only script that changes the room obstacles to clear the passage; that takes multiple deliberate lines. And QFG4 is full of room initialization bugs that test flags wrong and set the wrong state. I think the bug is that you could walk out the passage at all while a giant insect blocks it.

We can't be certain. I checked the hintbook, it didn't even mention the passage. Given that this is a minor inconsistency and original behavior, and that fixing it incorrectly took 300+ intense lines in script_patches.cpp that obscured a crash bug, and that an alternate patch would be about as complex, and that the end result could be spreading unintended behavior instead of curbing it, I'm leaning towards just deleting the patch to fix the crash and keep original behavior.

[sluicebox <22204938+sluicebox@…>]

In 0663e356:

SCI32: Remove QFG4 Thieves' Guild script patch

Fixes bug #15776