[Scummvm-tracker] [ScummVM :: Bugs] #15776: SCI: QFG4 v1.0 Thief Guild Color Puzzle crash due to ScummVM script patch

[ScummVM :: Bugs]

#15776: SCI: QFG4 v1.0 Thief Guild Color Puzzle crash due to ScummVM script patch
---------------------------------+--------------------------------
Reporter:  yacobean27            |       Owner:  yacobean27
    Type:  defect                |      Status:  new
Priority:  normal                |   Component:  Engine: SCI
 Version:                        |  Resolution:
Keywords:  QFG 1.0; Thief Guild  |        Game:  Quest for Glory 4
---------------------------------+--------------------------------
Changes (by sluicebox):

 * resolution:  outdated =>
 * status:  closed => new

Comment:

 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 it 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 sCureChief 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.
-- 
Ticket URL: <https://bugs.scummvm.org/ticket/15776#comment:6>
ScummVM :: Bugs <https://bugs.scummvm.org>
ScummVM