From c03e52bebbfbd5f18cdc3dfd6732f170162c9c3e Mon Sep 17 00:00:00 2001 From: sluicebox Date: Sun, 10 Feb 2019 19:31:54 -0800 Subject: SCI: Fix ECO1CD Mosaic puzzle crash, bug #10884 Fixes a bug in the original that crashes the interpreter --- engines/sci/engine/script_patches.cpp | 76 +++++++++++++++++++++++++++++++---- 1 file changed, 69 insertions(+), 7 deletions(-) (limited to 'engines/sci') diff --git a/engines/sci/engine/script_patches.cpp b/engines/sci/engine/script_patches.cpp index 56c5a4acec..afc8cf9554 100644 --- a/engines/sci/engine/script_patches.cpp +++ b/engines/sci/engine/script_patches.cpp @@ -121,6 +121,8 @@ static const char *const selectorNameTable[] = { "ignoreActors", // Laura Bow 1 Colonel's Bequest "at", // Longbow, QFG4 "owner", // Longbow + "delete", // EcoQuest 1 + "signal", // EcoQuest 1, GK1 #ifdef ENABLE_SCI32 "newWith", // SCI2 array script "scrollSelections", // GK2 @@ -133,7 +135,6 @@ static const char *const selectorNameTable[] = { "get", // Torin, GK1 "newRoom", // GK1 "normalize", // GK1 - "signal", // GK1 "set", // Torin "clear", // Torin "masterVolume", // SCI2 master volume reset @@ -209,7 +210,9 @@ enum ScriptPatcherSelectors { SELECTOR_setLoop, SELECTOR_ignoreActors, SELECTOR_at, - SELECTOR_owner + SELECTOR_owner, + SELECTOR_delete, + SELECTOR_signal #ifdef ENABLE_SCI32 , SELECTOR_newWith, @@ -223,7 +226,6 @@ enum ScriptPatcherSelectors { SELECTOR_get, SELECTOR_newRoom, SELECTOR_normalize, - SELECTOR_signal, SELECTOR_set, SELECTOR_clear, SELECTOR_masterVolume, @@ -608,11 +610,71 @@ static const uint16 ecoquest1PatchProphecyScroll[] = { PATCH_END }; -// script, description, signature patch +// The temple has a complex script bug in the CD version which can crash the +// interpreter when solving the mosaic puzzle after loading a game that was +// saved during the puzzle. The bug causes invalid memory access which locks up +// Sierra's interpreter and can cause ours to fail an assertion. +// +// Room 140 has three insets and a conch shell in the middle. This room's script +// was significantly changed in the CD version and transition animations were +// added. Due to these changes the shell no longer renders beneath the insets +// and so Sierra added code to hide the shell while they're displayed. +// Unfortunately this code is incorrect and leaves the game in a state that's +// unsafe to save. The shell is removed from the cast when showing an inset and +// then shell:init is called when hiding. This leaves shell:underBits pointing +// to hunk memory while temporarily not a member of the cast. Hunk memory isn't +// persisted in saved games but underBits' values are. SCI games handle this in +// Game:replay by clearing the underBits of cast members when restoring. Saving +// while the puzzle is displayed causes shell:underBits' stale hunk value to +// survive restoring. Solving the puzzle adds the shell back to the cast via +// init followed by a call to kAnimate that accesses the potentially stale +// shell:underBits. If the hunk segment id upon restoring in ScummVM is the +// same as when saved then this out of bounds access will fail an assertion. +// +// We fix this by fully disposing the shell when showing an inset so that its +// resources are cleaned up and it's safe to save the game. In order to do this +// without changing the animation effect we set shell's disposal flag and then +// immediately call shell:delete. This is equivalent to shell:dispose but +// prevents hiding the shell before the transition animation takes place. +// +// Applies to: PC CD +// Responsible methods: MosaicWall:doVerb, localproc_2ab6 in script 140 +// Fixes bug #10884 +static const uint16 ecoquest1SignatureMosaicPuzzleFix[] = { + 0x36, // push [ conchShell:owner ] + 0x34, SIG_UINT16(0x008c), // ldi 008c [ room number ] + 0x1a, // eq? [ is conchShell owned by room 140? ] + 0x30, SIG_UINT16(0x000b), // bnt 000b [ no shell to hide ] + SIG_MAGICDWORD, + 0x39, SIG_SELECTOR8(delete), // pushi delete + 0x78, // push1 + 0x72, SIG_UINT16(0x056a), // lofsa shell + 0x36, // push + 0x81, 0x05, // lag 05 + 0x4a, 0x06, // send 06 [ cast delete: shell ] + SIG_END +}; + +static const uint16 ecoquest1PatchMosaicPuzzleFix[] = { + 0x89, 0x0b, // lsg 0b [ current room number, saves 2 bytes ] + 0x1a, // eq? [ is conchShell owned by room 140? ] + 0x31, 0x0e, // bnt 0e [ no shell to hide, save a byte ] + 0x39, PATCH_SELECTOR8(signal), // pushi signal + 0x78, // push1 + 0x38, PATCH_UINT16(0xc014), // pushi c014 [ kSignalDisposeMe | shell:signal ] + 0x39, PATCH_SELECTOR8(delete), // pushi delete + 0x76, // push0 + 0x72, PATCH_UINT16(0x056a), // lofsa shell + 0x4a, 0x0a, // send 0a [ shell signal: c014, delete ] + PATCH_END +}; + +// script, description, signature patch static const SciScriptPatcherEntry ecoquest1Signatures[] = { - { true, 160, "CD: give superfluous oily shell", 1, ecoquest1SignatureGiveOilyShell, ecoquest1PatchGiveOilyShell }, - { true, 660, "CD: bad messagebox and freeze", 1, ecoquest1SignatureStayAndHelp, ecoquest1PatchStayAndHelp }, - { true, 816, "CD: prophecy scroll", 1, ecoquest1SignatureProphecyScroll, ecoquest1PatchProphecyScroll }, + { true, 140, "CD: mosaic puzzle fix", 2, ecoquest1SignatureMosaicPuzzleFix, ecoquest1PatchMosaicPuzzleFix }, + { true, 160, "CD: give superfluous oily shell", 1, ecoquest1SignatureGiveOilyShell, ecoquest1PatchGiveOilyShell }, + { true, 660, "CD: bad messagebox and freeze", 1, ecoquest1SignatureStayAndHelp, ecoquest1PatchStayAndHelp }, + { true, 816, "CD: prophecy scroll", 1, ecoquest1SignatureProphecyScroll, ecoquest1PatchProphecyScroll }, SCI_SIGNATUREENTRY_TERMINATOR }; -- cgit v1.2.3