[Scummvm-git-logs] scummvm master -> 28a06656af8e67219055cc8bbee57fa49dc2de61

csnover csnover at users.noreply.github.com
Sun Apr 23 20:10:48 CEST 2017


This automated email contains information about 50 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
c88a43b615 SCI32: Reset ScreenItem cel type when updating from a VM object
10a4e5d591 SCI32: Fix missing break statement
205f8c4a59 SCI32: Fix call to kFileIOIsValidDirectory in RAMA
94524633d7 SCI32: Put Lighthouse restart game in restart save slot
2212b82794 SCI32: Add workarounds for Lighthouse
f711edc876 SCI32: Add workarounds for RAMA
168774c3c6 SCI32: Add kPlayVMD subop 27 (SetPlane)
7b3c9f4a07 SCI32: Add Lighthouse workaround
b8f0224b62 SCI32: Remove unused SegManager from DuckPlayer
09fea5f108 SCI32: Fix bad draw rectangle size in Duck video player
4d9b019b18 SCI32: Activate SCI3 draw/erase list calculation algorithm
88f020c7d4 SCI32: Fix out-of-bounds reads of truncated uncompressed cels
8aed6759e4 SCI32: Remove incomplete SCI3 detection warning
c9342beca9 SCI: Remove unused commented code
ca507e2b0b SCI: Fix missing music in LSL7
42406cb11a SCI32: Undummy AvoidPath for SCI3
866419fa71 SCI: Implement fallback detection for SCI3
7997451384 SCI32: Remove TODO from relocateOffsetSci3
2573fe8c96 SCI32: Fix slow fade of Phant2 interface bars
b5ecbff39b SCI32: Fix crash in Phant2 when clicking Help in the control panel
a867fb70dd SCI32: Fix audio playback with monitored channel in SCI3
5948813169 SCI32: Fix crash when writing word to VIRTUALFILE_HANDLE_SCI32SAVE
e06d4a71fb SCI32: Fix missing main menu in LSL7
14e4ea6c85 SCI32: Add workarounds for LSL7
f58019b680 SCI32: Remove spinloops from Phant2
b3ecc54a7a SCI: Always search for .CSC script patches
d24f5537be SCI32: Implement SCI3 Script::syncStringHeap
d0a143fa87 SCI32: Fix locals offset in SCI3
5c5e485ff0 SCI32: Improve debugging output of object metadata in SCI3
8299460917 SCI32: Hook up mustSetViewVisible for SCI3
90c6c0580e SCI: Convert Object to use Common::Array for SCI3
6b95528b49 SCI32: Fix bad relocations of SCI3 objects
0676e640db SCI: Fix bad offsets in disassembly for SCI3 lofsa/lofss
61fba94139 SCI: Remove dead code in Script_Offset disassembler
0394fd44e8 SCI: Fix whitespace errors
cb50682b15 SCI: Improve documentation of Object class
2906ca9947 SCI: Replace mostly-unused flags property with a single boolean
eadf5d818f SCI: Fix support for 32-bit SCI3 script offsets
fc02b34215 SCI: Deduplicate Object::locateVarSelector code
a799cb3462 SCI: Fix SCI3 exports
f3db412d6f SCI32: Serialize Robots in SCI3
36cb241b8e SCI32: Remove unused code branch
eb9965274d SCI32: Fix race conditions in Audio32
8bdfb78895 SCI32: Add debugger command to list digital audio samples
1962b1bb6d SCI32: Replace magic numbers in SCI3 selector init
3d4fb4ccb4 SCI32: Fix mustSetViewVisible for SCI3
a2d7851e4d SCI32: Improve disassembly output of SCI3 property opcodes
6f95b1a440 SCI32: Fix missing mustSetViewVisible data in cloned objects
d53f3f6095 SCI32: Exclude SCI3 code from compilation when SCI32 is disabled
28a06656af SCI: Improve error messages in Script::validateExportFunc


Commit: c88a43b6156ad65d18bf2bc5ac7b8966eb616b18
    https://github.com/scummvm/scummvm/commit/c88a43b6156ad65d18bf2bc5ac7b8966eb616b18
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Reset ScreenItem cel type when updating from a VM object

In Phant2, when going to email on Curtis's office computer, a
screen item that contained a bitmap on the last frame was updated
to contain a view on the next frame. This crashed the engine when
it tried to reuse the old disposed bitmap instead of the new view
because the cel type was never changed from kCelTypeMem to
kCelTypeView.

Changed paths:
    engines/sci/graphics/screen_item32.cpp


diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp
index 77ff9a3..d184485 100644
--- a/engines/sci/graphics/screen_item32.cpp
+++ b/engines/sci/graphics/screen_item32.cpp
@@ -212,15 +212,13 @@ void ScreenItem::setFromObject(SegManager *segMan, const reg_t object, const boo
 		}
 	}
 
-	if (updateBitmap) {
-		const reg_t bitmap = readSelector(segMan, object, SELECTOR(bitmap));
-		if (!bitmap.isNull()) {
-			_celInfo.bitmap = bitmap;
-			_celInfo.type = kCelTypeMem;
-		} else {
-			_celInfo.bitmap = NULL_REG;
-			_celInfo.type = kCelTypeView;
-		}
+	const reg_t bitmap = readSelector(segMan, object, SELECTOR(bitmap));
+	if (updateBitmap && !bitmap.isNull()) {
+		_celInfo.bitmap = bitmap;
+		_celInfo.type = kCelTypeMem;
+	} else {
+		_celInfo.bitmap = NULL_REG;
+		_celInfo.type = kCelTypeView;
 	}
 
 	if (updateCel || updateBitmap) {


Commit: 10a4e5d591e6e2af76b5b89395462475dd3b209b
    https://github.com/scummvm/scummvm/commit/10a4e5d591e6e2af76b5b89395462475dd3b209b
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Fix missing break statement

Changed paths:
    engines/sci/sound/midiparser_sci.cpp


diff --git a/engines/sci/sound/midiparser_sci.cpp b/engines/sci/sound/midiparser_sci.cpp
index 98f7480..a25ce28 100644
--- a/engines/sci/sound/midiparser_sci.cpp
+++ b/engines/sci/sound/midiparser_sci.cpp
@@ -693,6 +693,7 @@ bool MidiParser_SCI::processEvent(const EventInfo &info, bool fireEvents) {
 							if (g_sci->getEngineState()->currentRoomNumber() == 6050) {
 								skipSignal = false;
 							}
+							break;
 #endif
 						default:
 							break;


Commit: 205f8c4a59cea068d46e76a39bd32b682682d3e9
    https://github.com/scummvm/scummvm/commit/205f8c4a59cea068d46e76a39bd32b682682d3e9
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Fix call to kFileIOIsValidDirectory in RAMA

Changed paths:
    engines/sci/engine/kernel_tables.h
    engines/sci/engine/kfile.cpp


diff --git a/engines/sci/engine/kernel_tables.h b/engines/sci/engine/kernel_tables.h
index c103d2d..fd7415b 100644
--- a/engines/sci/engine/kernel_tables.h
+++ b/engines/sci/engine/kernel_tables.h
@@ -332,7 +332,7 @@ static const SciKernelMapSubEntry kFileIO_subops[] = {
 	{ SIG_SINCE_SCI21MID, 16, MAP_CALL(FileIOWriteWord),           "ii",                   NULL },
 	{ SIG_SINCE_SCI21MID, 17, "FileIOCheckFreeSpace", kCheckFreeSpace, "i(r)",             NULL },
 	{ SIG_SINCE_SCI21MID, 18, MAP_CALL(FileIOGetCWD),              "r",                    NULL },
-	{ SIG_SINCE_SCI21MID, 19, MAP_CALL(FileIOIsValidDirectory),    "r",                    NULL },
+	{ SIG_SINCE_SCI21MID, 19, MAP_CALL(FileIOIsValidDirectory),    "[ro]",                 NULL },
 #endif
 	SCI_SUBOPENTRY_TERMINATOR
 };
diff --git a/engines/sci/engine/kfile.cpp b/engines/sci/engine/kfile.cpp
index 2ef2d76a6..8429af1 100644
--- a/engines/sci/engine/kfile.cpp
+++ b/engines/sci/engine/kfile.cpp
@@ -864,9 +864,10 @@ reg_t kFileIOGetCWD(EngineState *s, int argc, reg_t *argv) {
 }
 
 reg_t kFileIOIsValidDirectory(EngineState *s, int argc, reg_t *argv) {
-	// Used in Torin's Passage and LSL7 to determine if the directory passed as
-	// a parameter (usually the save directory) is valid. We always return true
-	// here.
+	// Used in Torin's Passage, LSL7, and RAMA to determine if the directory
+	// passed as a parameter (usually the save directory) is valid. We always
+	// return true here because we do not use this directory information when
+	// saving games.
 	return TRUE_REG;
 }
 


Commit: 94524633d78b23c5e1cf5733428ec6e212db9a21
    https://github.com/scummvm/scummvm/commit/94524633d78b23c5e1cf5733428ec6e212db9a21
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Put Lighthouse restart game in restart save slot

Changed paths:
    engines/sci/engine/kfile.cpp


diff --git a/engines/sci/engine/kfile.cpp b/engines/sci/engine/kfile.cpp
index 8429af1..6f77b0a 100644
--- a/engines/sci/engine/kfile.cpp
+++ b/engines/sci/engine/kfile.cpp
@@ -1201,6 +1201,10 @@ reg_t kSaveGame32(EngineState *s, int argc, reg_t *argv) {
 		saveNo += kSaveIdShift;
 	}
 
+	if (g_sci->getGameId() == GID_LIGHTHOUSE && gameName == "rst") {
+		saveNo = kNewGameId;
+	}
+
 	// Auto-save system used by QFG4
 	if (g_sci->getGameId() == GID_QFG4) {
 		reg_t autoSaveNameId;


Commit: 2212b82794f0d5915a8e94a911aa20ece373e88f
    https://github.com/scummvm/scummvm/commit/2212b82794f0d5915a8e94a911aa20ece373e88f
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Add workarounds for Lighthouse

Changed paths:
    engines/sci/engine/workarounds.cpp


diff --git a/engines/sci/engine/workarounds.cpp b/engines/sci/engine/workarounds.cpp
index d45e798..065b889 100644
--- a/engines/sci/engine/workarounds.cpp
+++ b/engines/sci/engine/workarounds.cpp
@@ -331,6 +331,7 @@ const SciWorkaroundEntry uninitializedReadWorkarounds[] = {
 	{ GID_LAURABOW2,      -1,    90,  1,        "MuseumActor", "init",                            NULL,     6, { WORKAROUND_FAKE,   0 } }, // Random actors in museum - bug #5197
 	{ GID_LAURABOW2,     240,   240,  0,     "sSteveAnimates", "changeState",                     NULL,     0, { WORKAROUND_FAKE,   0 } }, // Steve Dorian's idle animation at the docks - bug #5028
 	{ GID_LAURABOW2,      -1,   928,  0,                 NULL, "startText",                       NULL,     0, { WORKAROUND_FAKE,   0 } }, // gets caused by Text+Audio support (see script patcher)
+	{ GID_LIGHTHOUSE,    441,    17,  0,        "StemTracker", "handleEvent",                     NULL,     0, { WORKAROUND_FAKE,   0 } }, // when operating the joystick in the puzzle to lower the bridge at the entrance to the workshop
 	{ GID_LONGBOW,        -1,     0,  0,            "Longbow", "restart",                         NULL,     0, { WORKAROUND_FAKE,   0 } }, // When canceling a restart game - bug #5244
 	{ GID_LONGBOW,        -1,   213,  0,              "clear", "handleEvent",                     NULL,     0, { WORKAROUND_FAKE,   0 } }, // When giving an answer using the druid hand sign code in any room
 	{ GID_LONGBOW,        -1,   213,  0,             "letter", "handleEvent", sig_uninitread_longbow_1,   1, { WORKAROUND_FAKE,   0 } }, // When using the druid hand sign code in any room - bug #5035
@@ -729,6 +730,7 @@ const SciWorkaroundEntry kIsObject_workarounds[] = {
 //    gameID,           room,script,lvl,          object-name, method-name,    local-call-signature, index,                workaround
 const SciWorkaroundEntry kListAt_workarounds[] = {
 	{ GID_HOYLE5,        100, 64999,  0,           "theHands", "at",                           NULL,     0, { WORKAROUND_FAKE, 0 } }, // After the first hand is dealt in Crazy Eights game in demo, an object is passed instead of a number
+	{ GID_LIGHTHOUSE,     24, 64999,  0,           "LightInv", "at",                           NULL,     0, { WORKAROUND_FAKE, 0 } }, // When taking the car keys from the table at the start of the game
 	SCI_WORKAROUNDENTRY_TERMINATOR
 };
 


Commit: f711edc8764dabc0ec4d8621e34b005b89aa096f
    https://github.com/scummvm/scummvm/commit/f711edc8764dabc0ec4d8621e34b005b89aa096f
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Add workarounds for RAMA

Changed paths:
    engines/sci/engine/workarounds.cpp


diff --git a/engines/sci/engine/workarounds.cpp b/engines/sci/engine/workarounds.cpp
index 065b889..53a65f9 100644
--- a/engines/sci/engine/workarounds.cpp
+++ b/engines/sci/engine/workarounds.cpp
@@ -388,8 +388,8 @@ const SciWorkaroundEntry uninitializedReadWorkarounds[] = {
 	{ GID_QFG4,          520, 64950,  0,             "fLake2", "handleEvent",                     NULL,     0, { WORKAROUND_FAKE,   0 } }, // CD version, at the lake, when meeting the Rusalka and attempting to leave
 	{ GID_QFG4,          780, 64964,  0,              "DPath", "init",                            NULL,     1, { WORKAROUND_FAKE,   0 } }, // CD version, walking down to the monastery basement
 	{ GID_QFG4,          800, 64950,  0,               "View", "handleEvent",                     NULL,     0, { WORKAROUND_FAKE,   0 } }, // CD version, in the room with the spider pillar, when climbing on the pillar
-	{ GID_RAMA,           -1, 64950, -1,   "InterfaceFeature", "handleEvent",                     NULL,     0, { WORKAROUND_FAKE,   0 } }, // When clicking on the main game interface
-	{ GID_RAMA,           -1, 64950, -1,               "View", "handleEvent",                     NULL,     0, { WORKAROUND_FAKE,   0 } }, // When clicking on main menu buttons
+	{ GID_RAMA,           -1, 64950, -1,                 NULL, "handleEvent",                     NULL,     0, { WORKAROUND_FAKE,   0 } }, // When clicking on the main game interface, or the main menu buttons, or mousing over things in the main game window
+	{ GID_RAMA,           -1, 64923, -1,              "Inset", "init",                            NULL,     0, { WORKAROUND_FAKE,   0 } }, // When receiving a message on the pocket computer at the start of the game
 	{ GID_SHIVERS,        -1,   952,  0,       "SoundManager", "stop",                            NULL,     2, { WORKAROUND_FAKE,   0 } }, // Just after Sierra logo
 	{ GID_SHIVERS,        -1, 64950,  0,            "Feature", "handleEvent",                     NULL,     0, { WORKAROUND_FAKE,   0 } }, // When clicking on the locked door at the beginning
 	{ GID_SHIVERS,        -1, 64950,  0,               "View", "handleEvent",                     NULL,     0, { WORKAROUND_FAKE,   0 } }, // When clicking on the gargoyle eye at the beginning


Commit: 168774c3c6bd942970b7bc35ebe7b5abdb535b5b
    https://github.com/scummvm/scummvm/commit/168774c3c6bd942970b7bc35ebe7b5abdb535b5b
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Add kPlayVMD subop 27 (SetPlane)

Used by RAMA, when playing a video at the Hub Camp computer at the
beginning of the game (room 1004).

Changed paths:
    engines/sci/engine/kernel.h
    engines/sci/engine/kernel_tables.h
    engines/sci/engine/kvideo.cpp
    engines/sci/graphics/video32.cpp
    engines/sci/graphics/video32.h


diff --git a/engines/sci/engine/kernel.h b/engines/sci/engine/kernel.h
index c648cd0..dd75f26 100644
--- a/engines/sci/engine/kernel.h
+++ b/engines/sci/engine/kernel.h
@@ -477,6 +477,7 @@ reg_t kPlayVMDPlayUntilEvent(EngineState *s, int argc, reg_t *argv);
 reg_t kPlayVMDShowCursor(EngineState *s, int argc, reg_t *argv);
 reg_t kPlayVMDSetBlackoutArea(EngineState *s, int argc, reg_t *argv);
 reg_t kPlayVMDRestrictPalette(EngineState *s, int argc, reg_t *argv);
+reg_t kPlayVMDSetPlane(EngineState *s, int argc, reg_t *argv);
 
 reg_t kShowMovie32(EngineState *s, int argc, reg_t *argv);
 reg_t kShowMovieWin(EngineState *s, int argc, reg_t *argv);
diff --git a/engines/sci/engine/kernel_tables.h b/engines/sci/engine/kernel_tables.h
index fd7415b..716a851 100644
--- a/engines/sci/engine/kernel_tables.h
+++ b/engines/sci/engine/kernel_tables.h
@@ -474,6 +474,7 @@ static const SciKernelMapSubEntry kPlayVMD_subops[] = {
 	{ SIG_SINCE_SCI21,    18, MAP_DUMMY(PlayVMDStopBlobs),         "",                     NULL },
 	{ SIG_SINCE_SCI21,    21, MAP_CALL(PlayVMDSetBlackoutArea),    "iiii",                 NULL },
 	{ SIG_SINCE_SCI21,    23, MAP_CALL(PlayVMDRestrictPalette),    "ii",                   NULL },
+	{ SIG_SCI3,           27, MAP_CALL(PlayVMDSetPlane),           "i(i)",                 NULL },
 	{ SIG_SCI3,           28, MAP_EMPTY(PlayVMDSetPreload),        "i",                    NULL },
 	SCI_SUBOPENTRY_TERMINATOR
 };
diff --git a/engines/sci/engine/kvideo.cpp b/engines/sci/engine/kvideo.cpp
index 6f93b04..9398e6d 100644
--- a/engines/sci/engine/kvideo.cpp
+++ b/engines/sci/engine/kvideo.cpp
@@ -471,6 +471,11 @@ reg_t kPlayVMDRestrictPalette(EngineState *s, int argc, reg_t *argv) {
 	return s->r_acc;
 }
 
+reg_t kPlayVMDSetPlane(EngineState *s, int argc, reg_t *argv) {
+	g_sci->_video32->getVMDPlayer().setPlane(argv[0].toSint16(), argc > 1 ? argv[1] : NULL_REG);
+	return s->r_acc;
+}
+
 reg_t kPlayDuck(EngineState *s, int argc, reg_t *argv) {
 	if (!s)
 		return make_reg(0, getSciVersion());
diff --git a/engines/sci/graphics/video32.cpp b/engines/sci/graphics/video32.cpp
index 1759e8e..44bb00b 100644
--- a/engines/sci/graphics/video32.cpp
+++ b/engines/sci/graphics/video32.cpp
@@ -919,6 +919,15 @@ void VMDPlayer::fillPalette(Palette &palette) const {
 	}
 }
 
+void VMDPlayer::setPlane(const int16 priority, const reg_t planeId) {
+	_priority = priority;
+	if (planeId != NULL_REG) {
+		_plane = g_sci->_gfxFrameout->getPlanes().findByObject(planeId);
+		assert(_plane != nullptr);
+		_planeIsOwned = false;
+	}
+}
+
 #pragma mark -
 #pragma mark VMDPlayer - Palette
 
diff --git a/engines/sci/graphics/video32.h b/engines/sci/graphics/video32.h
index 751604f..ae4767c 100644
--- a/engines/sci/graphics/video32.h
+++ b/engines/sci/graphics/video32.h
@@ -363,6 +363,11 @@ public:
 	 */
 	void ignorePalettes() { _ignorePalettes = true; }
 
+	/**
+	 * Sets the plane and plane priority used to render video.
+	 */
+	void setPlane(const int16 priority, const reg_t planeId);
+
 private:
 	/**
 	 * The location of the VMD plane, in game script


Commit: 7b3c9f4a070b515a29e5483f5f1f262b4d6fb294
    https://github.com/scummvm/scummvm/commit/7b3c9f4a070b515a29e5483f5f1f262b4d6fb294
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Add Lighthouse workaround

Changed paths:
    engines/sci/engine/workarounds.cpp


diff --git a/engines/sci/engine/workarounds.cpp b/engines/sci/engine/workarounds.cpp
index 53a65f9..941112c 100644
--- a/engines/sci/engine/workarounds.cpp
+++ b/engines/sci/engine/workarounds.cpp
@@ -331,7 +331,7 @@ const SciWorkaroundEntry uninitializedReadWorkarounds[] = {
 	{ GID_LAURABOW2,      -1,    90,  1,        "MuseumActor", "init",                            NULL,     6, { WORKAROUND_FAKE,   0 } }, // Random actors in museum - bug #5197
 	{ GID_LAURABOW2,     240,   240,  0,     "sSteveAnimates", "changeState",                     NULL,     0, { WORKAROUND_FAKE,   0 } }, // Steve Dorian's idle animation at the docks - bug #5028
 	{ GID_LAURABOW2,      -1,   928,  0,                 NULL, "startText",                       NULL,     0, { WORKAROUND_FAKE,   0 } }, // gets caused by Text+Audio support (see script patcher)
-	{ GID_LIGHTHOUSE,    441,    17,  0,        "StemTracker", "handleEvent",                     NULL,     0, { WORKAROUND_FAKE,   0 } }, // when operating the joystick in the puzzle to lower the bridge at the entrance to the workshop
+	{ GID_LIGHTHOUSE,     -1,    17,  0,                 NULL, "handleEvent",                     NULL,     0, { WORKAROUND_FAKE,   0 } }, // when operating the joystick in the puzzle to lower the bridge at the entrance to the workshop, or the joystick that moves the robotic arm in the mini-sub
 	{ GID_LONGBOW,        -1,     0,  0,            "Longbow", "restart",                         NULL,     0, { WORKAROUND_FAKE,   0 } }, // When canceling a restart game - bug #5244
 	{ GID_LONGBOW,        -1,   213,  0,              "clear", "handleEvent",                     NULL,     0, { WORKAROUND_FAKE,   0 } }, // When giving an answer using the druid hand sign code in any room
 	{ GID_LONGBOW,        -1,   213,  0,             "letter", "handleEvent", sig_uninitread_longbow_1,   1, { WORKAROUND_FAKE,   0 } }, // When using the druid hand sign code in any room - bug #5035


Commit: b8f0224b62dbfbb4d1a22f0d54c069e116486334
    https://github.com/scummvm/scummvm/commit/b8f0224b62dbfbb4d1a22f0d54c069e116486334
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Remove unused SegManager from DuckPlayer

Changed paths:
    engines/sci/graphics/video32.cpp
    engines/sci/graphics/video32.h


diff --git a/engines/sci/graphics/video32.cpp b/engines/sci/graphics/video32.cpp
index 44bb00b..a4f55ae 100644
--- a/engines/sci/graphics/video32.cpp
+++ b/engines/sci/graphics/video32.cpp
@@ -943,7 +943,6 @@ void VMDPlayer::restrictPalette(const uint8 startColor, const int16 endColor) {
 #pragma mark DuckPlayer
 
 DuckPlayer::DuckPlayer(SegManager *segMan, EventManager *eventMan) :
-	_segMan(segMan),
 	_eventMan(eventMan),
 	_decoder(new Video::AVIDecoder(Audio::Mixer::kSFXSoundType)),
 	_plane(nullptr),
diff --git a/engines/sci/graphics/video32.h b/engines/sci/graphics/video32.h
index ae4767c..f8d6cbe 100644
--- a/engines/sci/graphics/video32.h
+++ b/engines/sci/graphics/video32.h
@@ -589,7 +589,6 @@ public:
 	}
 
 private:
-	SegManager *_segMan;
 	EventManager *_eventMan;
 	Video::AVIDecoder *_decoder;
 


Commit: 09fea5f108f66083899860ae343d2ae25f8036f0
    https://github.com/scummvm/scummvm/commit/09fea5f108f66083899860ae343d2ae25f8036f0
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Fix bad draw rectangle size in Duck video player

This was causing uninitialised garbage data from the scale buffer
to be drawn to the screen.

Changed paths:
    engines/sci/graphics/video32.cpp


diff --git a/engines/sci/graphics/video32.cpp b/engines/sci/graphics/video32.cpp
index a4f55ae..0a7cd7e 100644
--- a/engines/sci/graphics/video32.cpp
+++ b/engines/sci/graphics/video32.cpp
@@ -972,9 +972,11 @@ void DuckPlayer::open(const GuiResourceId resourceId, const int displayMode, con
 	_pixelDouble = displayMode != 0;
 
 	const int16 scale = _pixelDouble ? 2 : 1;
+	// SSCI seems to incorrectly calculate the draw rect by scaling the origin
+	// in addition to the width/height for the BR point
 	_drawRect = Common::Rect(x, y,
-							 (x + _decoder->getWidth()) * scale,
-							 (y + _decoder->getHeight()) * scale);
+							 x + _decoder->getWidth() * scale,
+							 y + _decoder->getHeight() * scale);
 
 	g_sci->_gfxCursor32->hide();
 


Commit: 4d9b019b18e66f58a7cd88348dfbcd6bf6d93c79
    https://github.com/scummvm/scummvm/commit/4d9b019b18e66f58a7cd88348dfbcd6bf6d93c79
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Activate SCI3 draw/erase list calculation algorithm

This code branch, existing since at least SQ6 but apparently never
used by any SCI2/2.1 game, is in fact the algorithm used by SCI3.

This fixes (at least) doubled rendering of transparent surfaces
like the background of the inventory window and conversation
choices panel in LSL7.

Changed paths:
    engines/sci/graphics/plane32.cpp


diff --git a/engines/sci/graphics/plane32.cpp b/engines/sci/graphics/plane32.cpp
index aa8cd52..a8724de 100644
--- a/engines/sci/graphics/plane32.cpp
+++ b/engines/sci/graphics/plane32.cpp
@@ -435,8 +435,7 @@ void Plane::calcLists(Plane &visiblePlane, const PlaneList &planeList, DrawList
 	DrawList::size_type drawListSizePrimary = drawList.size();
 	const RectList::size_type eraseListCount = eraseList.size();
 
-	// TODO: Figure out which games need which rendering method
-	if (/* TODO: dword_C6288 */ false) {  // "high resolution pictures"
+	if (getSciVersion() == SCI_VERSION_3) {
 		_screenItemList.sort();
 		bool pictureDrawn = false;
 		bool screenItemDrawn = false;


Commit: 88f020c7d47b4e35a09e9cc1f2a5209acecb3a7e
    https://github.com/scummvm/scummvm/commit/88f020c7d47b4e35a09e9cc1f2a5209acecb3a7e
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Fix out-of-bounds reads of truncated uncompressed cels

This happens e.g. on the About page in LSL7 because of an
interpreter problem where bitmap handles are destroyed and then
reused without a kFrameOut call to remove old screen items from
the visible plane list before a kIsOnMe call that causes the
engine to try to read from reused bitmap handles with different
contents and dimensions.

This replaces bad memory reads on the About page in LSL7 with an
assertion failure, until the problem with the About page can be
properly addressed.

Changed paths:
    engines/sci/graphics/celobj32.cpp


diff --git a/engines/sci/graphics/celobj32.cpp b/engines/sci/graphics/celobj32.cpp
index 75f6280..df55ec9 100644
--- a/engines/sci/graphics/celobj32.cpp
+++ b/engines/sci/graphics/celobj32.cpp
@@ -263,7 +263,7 @@ int16 SCALER_Scale<FLIP, READER>::_valuesY[kCelScalerTableSize];
 struct READER_Uncompressed {
 private:
 #ifndef NDEBUG
-	const int16 _sourceHeight;
+	int16 _sourceHeight;
 #endif
 	const byte *_pixels;
 	const int16 _sourceWidth;
@@ -280,6 +280,9 @@ public:
 
 		if (numPixels < celObj._width * celObj._height) {
 			warning("%s is truncated", celObj._info.toString().c_str());
+#ifndef NDEBUG
+			_sourceHeight = numPixels / celObj._width;
+#endif
 		}
 
 		_pixels = resource.getUnsafeDataAt(pixelsOffset, numPixels);


Commit: 8aed6759e4a387981a9d35604fa33a38bf0fc057
    https://github.com/scummvm/scummvm/commit/8aed6759e4a387981a9d35604fa33a38bf0fc057
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Remove incomplete SCI3 detection warning

Map format is the same as SCI2/2.1 and volume format is detected
correctly as SCI3.

Changed paths:
    engines/sci/resource.cpp


diff --git a/engines/sci/resource.cpp b/engines/sci/resource.cpp
index d71f435..f6486ac 100644
--- a/engines/sci/resource.cpp
+++ b/engines/sci/resource.cpp
@@ -903,12 +903,6 @@ void ResourceManager::init() {
 	_mapVersion = detectMapVersion();
 	_volVersion = detectVolVersion();
 
-	// TODO/FIXME: Remove once SCI3 resource detection is finished
-	if ((_mapVersion == kResVersionSci3 || _volVersion == kResVersionSci3) && (_mapVersion != _volVersion)) {
-		warning("FIXME: Incomplete SCI3 detection: setting map and volume version to SCI3");
-		_mapVersion = _volVersion = kResVersionSci3;
-	}
-
 	if ((_volVersion == kResVersionUnknown) && (_mapVersion != kResVersionUnknown)) {
 		warning("Volume version not detected, but map version has been detected. Setting volume version to map version");
 		_volVersion = _mapVersion;


Commit: c9342beca9dedb01f2dc4fc65b23397fe946e5f9
    https://github.com/scummvm/scummvm/commit/c9342beca9dedb01f2dc4fc65b23397fe946e5f9
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI: Remove unused commented code

Changed paths:
    engines/sci/engine/script.cpp


diff --git a/engines/sci/engine/script.cpp b/engines/sci/engine/script.cpp
index 520275c..be1fa3f 100644
--- a/engines/sci/engine/script.cpp
+++ b/engines/sci/engine/script.cpp
@@ -229,7 +229,6 @@ void Script::load(int script_nr, ResourceManager *resMan, ScriptPatcher *scriptP
 
 		if (_localsOffset + _localsCount * 2 + 1 >= (int)_buf->size()) {
 			error("Locals extend beyond end of script: offset %04x, count %d vs size %d", _localsOffset, _localsCount, (int)_buf->size());
-			//_localsCount = (_bufSize - _localsOffset) >> 1;
 		}
 	}
 


Commit: ca507e2b0bb2e07539dcd22667126a77d53a8908
    https://github.com/scummvm/scummvm/commit/ca507e2b0bb2e07539dcd22667126a77d53a8908
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI: Fix missing music in LSL7

Changed paths:
    engines/sci/sci.cpp


diff --git a/engines/sci/sci.cpp b/engines/sci/sci.cpp
index a2ace9a..ff50329 100644
--- a/engines/sci/sci.cpp
+++ b/engines/sci/sci.cpp
@@ -150,6 +150,7 @@ SciEngine::SciEngine(OSystem *syst, const ADGameDescription *desc, SciGameId gam
 	SearchMan.addSubDirectoryMatching(gameDataDir, "robots");	// robot movie files
 	SearchMan.addSubDirectoryMatching(gameDataDir, "movie");	// VMD movie files
 	SearchMan.addSubDirectoryMatching(gameDataDir, "movies");	// VMD movie files
+	SearchMan.addSubDirectoryMatching(gameDataDir, "music/22s16");	// LSL7 music files
 	SearchMan.addSubDirectoryMatching(gameDataDir, "vmd");	// VMD movie files
 	SearchMan.addSubDirectoryMatching(gameDataDir, "duk");	// Duck movie files in Phantasmagoria 2
 	SearchMan.addSubDirectoryMatching(gameDataDir, "Robot Folder"); // Mac robot files


Commit: 42406cb11a775b129cfe5a913ee239f129eeae71
    https://github.com/scummvm/scummvm/commit/42406cb11a775b129cfe5a913ee239f129eeae71
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Undummy AvoidPath for SCI3

Used by LSL7.

Changed paths:
    engines/sci/engine/kernel.cpp


diff --git a/engines/sci/engine/kernel.cpp b/engines/sci/engine/kernel.cpp
index 68a09e9..8a7cc78 100644
--- a/engines/sci/engine/kernel.cpp
+++ b/engines/sci/engine/kernel.cpp
@@ -918,7 +918,6 @@ void Kernel::loadKernelNames(GameFeatures *features) {
 		_kernelNames[0x39] = "Dummy";	// ShowMovie in SCI2.1
 		_kernelNames[0x4c] = "Dummy";	// ScrollWindow in SCI2.1
 		_kernelNames[0x56] = "Dummy";	// VibrateMouse in SCI2.1 (only used in QFG4 floppy)
-		_kernelNames[0x64] = "Dummy";	// AvoidPath in SCI2.1
 		_kernelNames[0x66] = "Dummy";	// MergePoly in SCI2.1
 		_kernelNames[0x8d] = "MessageBox";	// Dummy in SCI2.1
 		_kernelNames[0x9b] = "Minimize";	// Dummy in SCI2.1


Commit: 866419fa71d398edcf087e67f7367deeadd04f14
    https://github.com/scummvm/scummvm/commit/866419fa71d398edcf087e67f7367deeadd04f14
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI: Implement fallback detection for SCI3

Changed paths:
    engines/sci/detection.cpp
    engines/sci/resource.cpp
    engines/sci/resource.h
    engines/sci/sci.cpp


diff --git a/engines/sci/detection.cpp b/engines/sci/detection.cpp
index 422bbba..cc6bb00 100644
--- a/engines/sci/detection.cpp
+++ b/engines/sci/detection.cpp
@@ -379,6 +379,15 @@ Common::String convertSierraGameId(Common::String sierraId, uint32 *gameFlags, R
 		return "qfg3";
 	}
 
+	if (sierraId == "l7")
+		return "lsl7";
+
+	if (sierraId == "p2")
+		return "phantasmagoria2";
+
+	if (sierraId == "lite")
+		return "lighthouse";
+
 	return sierraId;
 }
 
@@ -628,7 +637,7 @@ const ADGameDescription *SciMetaEngine::fallbackDetect(const FileMap &allFiles,
 		s_fallbackDesc.platform = Common::kPlatformAmiga;
 
 	// Determine the game id
-	Common::String sierraGameId = resMan.findSierraGameId();
+	Common::String sierraGameId = resMan.findSierraGameId(s_fallbackDesc.platform == Common::kPlatformMacintosh);
 
 	// If we don't have a game id, the game is not SCI
 	if (sierraGameId.empty())
diff --git a/engines/sci/resource.cpp b/engines/sci/resource.cpp
index f6486ac..c4df47d 100644
--- a/engines/sci/resource.cpp
+++ b/engines/sci/resource.cpp
@@ -30,6 +30,7 @@
 #include "common/memstream.h"
 #endif
 
+#include "sci/parser/vocabulary.h"
 #include "sci/resource.h"
 #include "sci/resource_intern.h"
 #include "sci/util.h"
@@ -2690,15 +2691,19 @@ static SciSpan<const byte>::const_iterator findSci0ExportsBlock(const SciSpan<co
 
 // This code duplicates Script::relocateOffsetSci3, but we can't use
 // that here since we can't instantiate scripts at this point.
-static int relocateOffsetSci3(const SciSpan<const byte> &buf, uint32 offset) {
+static int relocateOffsetSci3(const SciSpan<const byte> &buf, uint32 offset, const bool isBE) {
 	int relocStart = buf.getUint32LEAt(8);
 	int relocCount = buf.getUint16LEAt(18);
 	SciSpan<const byte>::const_iterator seeker = buf.cbegin() + relocStart;
 
 	for (int i = 0; i < relocCount; ++i) {
-		if (seeker.getUint32SE() == offset) {
-			// TODO: Find out what UINT16 at (seeker + 8) means
-			return buf.getUint16SEAt(offset) + (seeker + 4).getUint32SE();
+		const uint32 candidateOffset = isBE ? seeker.getUint32BE() : seeker.getUint32LE();
+		if (candidateOffset == offset) {
+			if (isBE) {
+				return buf.getUint16BEAt(offset) + (seeker + 4).getUint32BE();
+			} else {
+				return buf.getUint16LEAt(offset) + (seeker + 4).getUint32LE();
+			}
 		}
 		seeker += 10;
 	}
@@ -2706,7 +2711,7 @@ static int relocateOffsetSci3(const SciSpan<const byte> &buf, uint32 offset) {
 	return -1;
 }
 
-reg_t ResourceManager::findGameObject(bool addSci11ScriptOffset) {
+reg_t ResourceManager::findGameObject(const bool addSci11ScriptOffset, const bool isBE) {
 	Resource *script = findResource(ResourceId(kResourceTypeScript, 0), false);
 
 	if (!script)
@@ -2746,36 +2751,63 @@ reg_t ResourceManager::findGameObject(bool addSci11ScriptOffset) {
 
 		return make_reg(1, offset);
 	} else {
-		return make_reg(1, relocateOffsetSci3(*script, 22));
+		return make_reg(1, relocateOffsetSci3(*script, 22, isBE));
 	}
 }
 
-Common::String ResourceManager::findSierraGameId() {
+Common::String ResourceManager::findSierraGameId(const bool isBE) {
 	// In SCI0-SCI1, the heap is embedded in the script. In SCI1.1 - SCI2.1,
 	// it's in a separate heap resource
-	Resource *heap = 0;
-	int nameSelector = 3;
+	Resource *heap = nullptr;
+	int nameSelector = -1;
 
 	if (getSciVersion() < SCI_VERSION_1_1) {
 		heap = findResource(ResourceId(kResourceTypeScript, 0), false);
+		nameSelector = 3;
 	} else if (getSciVersion() >= SCI_VERSION_1_1 && getSciVersion() <= SCI_VERSION_2_1_LATE) {
 		heap = findResource(ResourceId(kResourceTypeHeap, 0), false);
-		nameSelector += 5;
+		nameSelector = 8;
 	} else if (getSciVersion() == SCI_VERSION_3) {
-		warning("TODO: findSierraGameId(): SCI3 equivalent");
+		heap = findResource(ResourceId(kResourceTypeScript, 0), false);
+
+		Resource *vocab = findResource(ResourceId(kResourceTypeVocab, VOCAB_RESOURCE_SELECTORS), false);
+		const uint16 numSelectors = isBE ? vocab->getUint16BEAt(0) : vocab->getUint16LEAt(0);
+		for (uint16 i = 0; i < numSelectors; ++i) {
+			uint16 selectorOffset;
+			uint16 selectorSize;
+			if (isBE) {
+				selectorOffset = vocab->getUint16BEAt((i + 1) * sizeof(uint16));
+				selectorSize = vocab->getUint16BEAt(selectorOffset);
+			} else {
+				selectorOffset = vocab->getUint16LEAt((i + 1) * sizeof(uint16));
+				selectorSize = vocab->getUint16LEAt(selectorOffset);
+			}
+
+			Common::String selectorName = Common::String((const char *)vocab->getUnsafeDataAt(selectorOffset + 2, selectorSize), selectorSize);
+			if (selectorName == "name") {
+				nameSelector = i;
+				break;
+			}
+		}
 	}
 
-	if (!heap)
+	if (!heap || nameSelector == -1)
 		return "";
 
-	int16 gameObjectOffset = findGameObject(false).getOffset();
+	int16 gameObjectOffset = findGameObject(false, isBE).getOffset();
 
 	if (!gameObjectOffset)
 		return "";
 
-	// Seek to the name selector of the first export
-	SciSpan<const byte>::const_iterator offsetPtr = heap->cbegin() + gameObjectOffset + nameSelector * 2;
-	uint16 offset = !isSci11Mac() ? offsetPtr.getUint16LE() : offsetPtr.getUint16BE();
+	int32 offset;
+	if (getSciVersion() == SCI_VERSION_3) {
+		offset = relocateOffsetSci3(*heap, gameObjectOffset + /* base selector offset */ 0x110 + nameSelector * sizeof(uint16), isBE);
+	} else {
+		// Seek to the name selector of the first export
+		SciSpan<const byte>::const_iterator offsetPtr = heap->cbegin() + gameObjectOffset + nameSelector * sizeof(uint16);
+		offset = !isSci11Mac() ? offsetPtr.getUint16LE() : offsetPtr.getUint16BE();
+	}
+
 	return heap->getStringAt(offset);
 }
 
diff --git a/engines/sci/resource.h b/engines/sci/resource.h
index 6a67bf74..85bd915 100644
--- a/engines/sci/resource.h
+++ b/engines/sci/resource.h
@@ -436,7 +436,7 @@ public:
 	/**
 	 * Finds the internal Sierra ID of the current game from script 0.
 	 */
-	Common::String findSierraGameId();
+	Common::String findSierraGameId(const bool isBE);
 
 	/**
 	 * Finds the location of the game object from script 0.
@@ -444,7 +444,7 @@ public:
 	 *        games. Needs to be false when the heap is accessed directly inside
 	 *        findSierraGameId().
 	 */
-	reg_t findGameObject(bool addSci11ScriptOffset = true);
+	reg_t findGameObject(const bool addSci11ScriptOffset, const bool isBE);
 
 	/**
 	 * Converts a map resource type to our type
diff --git a/engines/sci/sci.cpp b/engines/sci/sci.cpp
index ff50329..026c423 100644
--- a/engines/sci/sci.cpp
+++ b/engines/sci/sci.cpp
@@ -242,7 +242,7 @@ Common::Error SciEngine::run() {
 
 	// Add the after market GM patches for the specified game, if they exist
 	_resMan->addNewGMPatch(_gameId);
-	_gameObjectAddress = _resMan->findGameObject();
+	_gameObjectAddress = _resMan->findGameObject(true, isBE());
 
 	_scriptPatcher = new ScriptPatcher();
 	SegManager *segMan = new SegManager(_resMan, _scriptPatcher);


Commit: 7997451384b13f9f14091504661cd7e6180a8526
    https://github.com/scummvm/scummvm/commit/7997451384b13f9f14091504661cd7e6180a8526
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Remove TODO from relocateOffsetSci3

The field at +8 is for the MemID associated with a relocation.

Changed paths:
    engines/sci/engine/script.cpp


diff --git a/engines/sci/engine/script.cpp b/engines/sci/engine/script.cpp
index be1fa3f..86f9d8d 100644
--- a/engines/sci/engine/script.cpp
+++ b/engines/sci/engine/script.cpp
@@ -699,7 +699,6 @@ int Script::relocateOffsetSci3(uint32 offset) const {
 
 	for (int i = 0; i < relocCount; ++i) {
 		if (seeker.getUint32SEAt(0) == offset) {
-			// TODO: Find out what UINT16 at (seeker + 8) means
 			return _buf->getUint16SEAt(offset) + seeker.getUint32SEAt(4);
 		}
 		seeker += 10;


Commit: 2573fe8c96803fb7d02a77934e72343f3e64971a
    https://github.com/scummvm/scummvm/commit/2573fe8c96803fb7d02a77934e72343f3e64971a
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Fix slow fade of Phant2 interface bars

Changed paths:
    engines/sci/engine/script_patches.cpp


diff --git a/engines/sci/engine/script_patches.cpp b/engines/sci/engine/script_patches.cpp
index 608ff07..380b5b7 100644
--- a/engines/sci/engine/script_patches.cpp
+++ b/engines/sci/engine/script_patches.cpp
@@ -3229,6 +3229,36 @@ static const SciScriptPatcherEntry phantasmagoriaSignatures[] = {
 	SCI_SIGNATUREENTRY_TERMINATOR
 };
 
+#pragma mark -
+#pragma mark Phantasmagoria 2
+
+// The interface bars at the top and bottom of the screen fade in and out when
+// hovered over. This fade is performed by a script loop that calls kFrameOut
+// directly and uses global 227 as the fade delta for each frame. Global 227
+// normally contains the value 1, which means that these fades are quite slow.
+// This patch replaces the use of global 227 with an immediate value that gives
+// a reasonable fade speed.
+// Applies to at least: US English
+static const uint16 phant2SlowIFadeSignature[] = {
+	0x43, 0x21, SIG_UINT16(0), // callk FrameOut, 0
+	SIG_MAGICDWORD,
+	0x67, 0x03,                // pTos 03 (scratch)
+	0x81, 0xe3,                // lag $e3 (227)
+	SIG_END
+};
+
+static const uint16 phant2SlowIFadePatch[] = {
+	PATCH_ADDTOOFFSET(6),      // skip to lag
+	0x35, 0x05,                // ldi 5
+	PATCH_END
+};
+
+//          script, description,                                      signature                        patch
+static const SciScriptPatcherEntry phantasmagoria2Signatures[] = {
+	{  true,     0, "slow interface fades",                        3, phant2SlowIFadeSignature,     phant2SlowIFadePatch },
+	SCI_SIGNATUREENTRY_TERMINATOR
+};
+
 #endif
 
 // ===========================================================================
@@ -5679,6 +5709,10 @@ void ScriptPatcher::processScript(uint16 scriptNr, SciSpan<byte> scriptData) {
 	case GID_PHANTASMAGORIA:
 		signatureTable = phantasmagoriaSignatures;
 		break;
+
+	case GID_PHANTASMAGORIA2:
+		signatureTable = phantasmagoria2Signatures;
+		break;
 #endif
 	case GID_PQ1:
 		signatureTable = pq1vgaSignatures;


Commit: b5ecbff39b2f33748aec02ecf789037554ea34bb
    https://github.com/scummvm/scummvm/commit/b5ecbff39b2f33748aec02ecf789037554ea34bb
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Fix crash in Phant2 when clicking Help in the control panel

Changed paths:
    engines/sci/engine/kernel_tables.h


diff --git a/engines/sci/engine/kernel_tables.h b/engines/sci/engine/kernel_tables.h
index 716a851..b6fddd8 100644
--- a/engines/sci/engine/kernel_tables.h
+++ b/engines/sci/engine/kernel_tables.h
@@ -1471,7 +1471,7 @@ static const char *const sci21_default_knames[] = {
 	/*0x7e*/ "Table",
 	/*0x7f*/ "WinHelp",		// Windows only
 	/*0x80*/ "Dummy",
-	/*0x81*/ "Dummy",		// called when changing rooms in most SCI2.1 games (e.g. KQ7, GK2, MUMG deluxe, Phant1)
+	/*0x81*/ "Empty",       // called when clicking the On-line Help button in Phant2 control panel
 	/*0x82*/ "Dummy",
 	/*0x83*/ "PrintDebug",	// debug function, used by Shivers (demo and full)
 	/*0x84*/ "Dummy",


Commit: a867fb70ddc145b03b8b782401364681e8c5560c
    https://github.com/scummvm/scummvm/commit/a867fb70ddc145b03b8b782401364681e8c5560c
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Fix audio playback with monitored channel in SCI3

SCI3 changes the way that monitored channel works. In SCI2/2.1,
when a channel is monitored, it is the only audible channel. In
SCI3, monitored channels mix normally.

This is very noticeable in LSL7, where music disappears totally
during speech if the monitored channel is the only channel played
back.

Changed paths:
    engines/sci/sound/audio32.cpp
    engines/sci/sound/audio32.h


diff --git a/engines/sci/sound/audio32.cpp b/engines/sci/sound/audio32.cpp
index e9e90e4..5d008bf 100644
--- a/engines/sci/sound/audio32.cpp
+++ b/engines/sci/sound/audio32.cpp
@@ -217,6 +217,8 @@ int Audio32::readBuffer(Audio::st_sample_t *buffer, const int numSamples) {
 
 	freeUnusedChannels();
 
+	const bool playOnlyMonitoredChannel = getSciVersion() != SCI_VERSION_3 && _monitoredChannelIndex != -1;
+
 	// The caller of `readBuffer` is a rate converter,
 	// which reuses (without clearing) an intermediate
 	// buffer, so we need to zero the intermediate buffer
@@ -248,7 +250,7 @@ int Audio32::readBuffer(Audio::st_sample_t *buffer, const int numSamples) {
 		//       0 | 2  (>> 1)
 		//       1 | 4  (>> 2)
 		//       2 | 6...
-		if (_monitoredChannelIndex == -1 && _numActiveChannels > 1) {
+		if (!playOnlyMonitoredChannel && _numActiveChannels > 1) {
 			attenuationAmount = _numActiveChannels + 1;
 			attenuationStepAmount = 1;
 		} else {
@@ -299,7 +301,7 @@ int Audio32::readBuffer(Audio::st_sample_t *buffer, const int numSamples) {
 			rightVolume = channel.volume * channel.pan / 100 * Audio::Mixer::kMaxChannelVolume / kMaxVolume;
 		}
 
-		if (_monitoredChannelIndex == -1 && _attenuatedMixing) {
+		if (!playOnlyMonitoredChannel && _attenuatedMixing) {
 			leftVolume >>= attenuationAmount;
 			rightVolume >>= attenuationAmount;
 		}
@@ -326,7 +328,7 @@ int Audio32::readBuffer(Audio::st_sample_t *buffer, const int numSamples) {
 				maxSamplesWritten = _numMonitoredSamples;
 			}
 		} else if (!channel.stream->endOfStream() || channel.loop) {
-			if (_monitoredChannelIndex != -1) {
+			if (playOnlyMonitoredChannel) {
 				// Audio that is not on the monitored channel is silent
 				// when the monitored channel is active, but the stream still
 				// needs to be read in order to ensure that sound effects sync
@@ -942,14 +944,23 @@ reg_t Audio32::kernelPlay(const bool autoPlay, const int argc, const reg_t *cons
 			loop = (bool)argv[5].toSint16();
 		}
 
-		if (argc < 7 || argv[6].toSint16() < 0 || argv[6].toSint16() > Audio32::kMaxVolume) {
-			volume = Audio32::kMaxVolume;
-
-			if (argc >= 7) {
-				monitor = true;
+		if (getSciVersion() == SCI_VERSION_3) {
+			if (argc < 7) {
+				volume = Audio32::kMaxVolume;
+			} else {
+				volume = argv[6].toSint16() & Audio32::kMaxVolume;
+				monitor = argv[6].toSint16() & Audio32::kMonitorAudioFlagSci3;
 			}
 		} else {
-			volume = argv[6].toSint16();
+			if (argc < 7 || argv[6].toSint16() < 0 || argv[6].toSint16() > Audio32::kMaxVolume) {
+				volume = Audio32::kMaxVolume;
+
+				if (argc >= 7) {
+					monitor = true;
+				}
+			} else {
+				volume = argv[6].toSint16();
+			}
 		}
 	} else {
 		resourceId = ResourceId(kResourceTypeAudio, argv[0].toUint16());
@@ -960,18 +971,23 @@ reg_t Audio32::kernelPlay(const bool autoPlay, const int argc, const reg_t *cons
 			loop = (bool)argv[1].toSint16();
 		}
 
-		// TODO: SCI3 uses the 0x80 bit as a flag to
-		// indicate "priority channel", but the volume is clamped
-		// in this call to 0x7F so that flag never makes it into
-		// the audio subsystem
-		if (argc < 3 || argv[2].toSint16() < 0 || argv[2].toSint16() > Audio32::kMaxVolume) {
-			volume = Audio32::kMaxVolume;
-
-			if (argc >= 3) {
-				monitor = true;
+		if (getSciVersion() == SCI_VERSION_3) {
+			if (argc < 3) {
+				volume = Audio32::kMaxVolume;
+			} else {
+				volume = argv[2].toSint16() & Audio32::kMaxVolume;
+				monitor = argv[2].toSint16() & Audio32::kMonitorAudioFlagSci3;
 			}
 		} else {
-			volume = argv[2].toSint16();
+			if (argc < 3 || argv[2].toSint16() < 0 || argv[2].toSint16() > Audio32::kMaxVolume) {
+				volume = Audio32::kMaxVolume;
+
+				if (argc >= 3) {
+					monitor = true;
+				}
+			} else {
+				volume = argv[2].toSint16();
+			}
 		}
 
 		soundNode = argc == 4 ? argv[3] : NULL_REG;
diff --git a/engines/sci/sound/audio32.h b/engines/sci/sound/audio32.h
index 8a1d8cf..f631de2 100644
--- a/engines/sci/sound/audio32.h
+++ b/engines/sci/sound/audio32.h
@@ -164,7 +164,9 @@ public:
 		/**
 		 * The maximum channel volume.
 		 */
-		kMaxVolume = 127
+		kMaxVolume = 127,
+
+		kMonitorAudioFlagSci3 = 0x80
 	};
 
 private:


Commit: 59488131692157b607ec15df726418ac49fbbfca
    https://github.com/scummvm/scummvm/commit/59488131692157b607ec15df726418ac49fbbfca
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Fix crash when writing word to VIRTUALFILE_HANDLE_SCI32SAVE

This happens in Phant2 when trying to delete a save game from the
in-game save dialogue.

Changed paths:
    engines/sci/engine/kfile.cpp


diff --git a/engines/sci/engine/kfile.cpp b/engines/sci/engine/kfile.cpp
index 6f77b0a..732d61f 100644
--- a/engines/sci/engine/kfile.cpp
+++ b/engines/sci/engine/kfile.cpp
@@ -851,7 +851,15 @@ reg_t kFileIOReadWord(EngineState *s, int argc, reg_t *argv) {
 }
 
 reg_t kFileIOWriteWord(EngineState *s, int argc, reg_t *argv) {
-	FileHandle *f = getFileFromHandle(s, argv[0].toUint16());
+	uint16 handle = argv[0].toUint16();
+
+#ifdef ENABLE_SCI32
+	if (handle == VIRTUALFILE_HANDLE_SCI32SAVE) {
+		return s->r_acc;
+	}
+#endif
+
+	FileHandle *f = getFileFromHandle(s, handle);
 	if (f)
 		f->_out->writeUint16LE(argv[1].toUint16());
 	return s->r_acc;


Commit: e06d4a71fb34ce445297ac4df6fa23ac397c69be
    https://github.com/scummvm/scummvm/commit/e06d4a71fb34ce445297ac4df6fa23ac397c69be
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Fix missing main menu in LSL7

Changed paths:
    engines/sci/engine/kfile.cpp


diff --git a/engines/sci/engine/kfile.cpp b/engines/sci/engine/kfile.cpp
index 732d61f..e386b13 100644
--- a/engines/sci/engine/kfile.cpp
+++ b/engines/sci/engine/kfile.cpp
@@ -743,6 +743,12 @@ reg_t kFileIOExists(EngineState *s, int argc, reg_t *argv) {
 	if (isSaveCatalogue(name)) {
 		return saveCatalogueExists(name) ? TRUE_REG : NULL_REG;
 	}
+
+	// LSL7 checks to see if the autosave save exists when deciding whether to
+	// go to the main menu or not on startup
+	if (g_sci->getGameId() == GID_LSL7 && name == "autosvsg.000") {
+		return g_sci->getSaveFileManager()->listSavefiles(g_sci->getSavegameName(0)).empty() ? NULL_REG : TRUE_REG;
+	}
 #endif
 
 	// TODO: It may apparently be worth caching the existence of


Commit: 14e4ea6c85c0d3cba9568f518df1f5f05db11d44
    https://github.com/scummvm/scummvm/commit/14e4ea6c85c0d3cba9568f518df1f5f05db11d44
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Add workarounds for LSL7

Changed paths:
    engines/sci/engine/workarounds.cpp


diff --git a/engines/sci/engine/workarounds.cpp b/engines/sci/engine/workarounds.cpp
index 941112c..9ba055d 100644
--- a/engines/sci/engine/workarounds.cpp
+++ b/engines/sci/engine/workarounds.cpp
@@ -347,6 +347,8 @@ const SciWorkaroundEntry uninitializedReadWorkarounds[] = {
 	{ GID_LSL6HIRES,      -1, 64950,  1,            "Feature", "handleEvent",                     NULL,     0, { WORKAROUND_FAKE,   0 } }, // at least when entering swimming pool area
 	{ GID_LSL6HIRES,      -1, 64964,  0,              "DPath", "init",                            NULL,     1, { WORKAROUND_FAKE,   0 } }, // during the game
 	{ GID_LSL7,           -1, 64017,  0,             "oFlags", "clear",                           NULL,     0, { WORKAROUND_FAKE,   0 } }, // demo version, when it starts, and whenever the player chooses to go to the "Strip Liar's Dice" mini game
+	{ GID_LSL7,           -1, 64017,  0,        "oActorFlags", "clear",                           NULL,     0, { WORKAROUND_FAKE,   0 } }, // after an NPC walks off the left side of the screen at the Clothing Optional Pool
+	{ GID_LSL7,           -1, 64892,  0,      "oEventHandler", "killAllEventHogs",                NULL,     1, { WORKAROUND_FAKE,   0 } }, // when looking at the swordfish in the kitchen
 	{ GID_MOTHERGOOSE256, -1,     0,  0,                 "MG", "doit",                            NULL,     5, { WORKAROUND_FAKE,   0 } }, // SCI1.1: When moving the cursor all the way to the left during the game - bug #5224
 	{ GID_MOTHERGOOSE256, -1,   992,  0,             "AIPath", "init",                            NULL,     0, { WORKAROUND_FAKE,   0 } }, // Happens in the demo and full version. In the demo, it happens when walking two screens from mother goose's house to the north. In the full version, it happens in rooms 7 and 23 - bug #5269
 	{ GID_MOTHERGOOSE256, 90,    90,  0,        "introScript", "changeState",                     NULL,    65, { WORKAROUND_FAKE,   0 } }, // SCI1(CD): At the very end, after the game is completed and restarted - bug #5626


Commit: f58019b6800f9ce72549ae6ec245766adbebeae2
    https://github.com/scummvm/scummvm/commit/f58019b6800f9ce72549ae6ec245766adbebeae2
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Remove spinloops from Phant2

Changed paths:
    engines/sci/engine/script_patches.cpp


diff --git a/engines/sci/engine/script_patches.cpp b/engines/sci/engine/script_patches.cpp
index 380b5b7..7ba5f45 100644
--- a/engines/sci/engine/script_patches.cpp
+++ b/engines/sci/engine/script_patches.cpp
@@ -3253,9 +3253,55 @@ static const uint16 phant2SlowIFadePatch[] = {
 	PATCH_END
 };
 
+// The game uses a spin loop during music transitions which causes the mouse to
+// appear unresponsive during scene changes. Replace the spin loop with a call
+// to ScummVM kWait.
+// Applies to at least: US English
+// Responsible method: P2SongPlyr::wait4Fade
+static const uint16 phant2Wait4FadeSignature[] = {
+	SIG_MAGICDWORD,
+	0x76,                      // push0
+	0x43, 0x79, SIG_UINT16(0), // callk GetTime, 0
+	0xa5, 0x01,                // sat 1
+	0x78,                      // push1
+	SIG_END
+};
+
+static const uint16 phant2Wait4FadePatch[] = {
+	0x78,                                     // push1
+	0x8d, 0x00,                               // lst temp[0]
+	0x43, kScummVMWaitId, PATCH_UINT16(0x02), // callk Wait, 2
+	0x48,                                     // ret
+	PATCH_END
+};
+
+// The game uses a spin loop when navigating to and from Curtis's computer in
+// the office, which causes the mouse to appear unresponsive. Replace the spin
+// loop with a call to ScummVM kWait.
+// Applies to at least: US English
+// Responsible method: localproc 4f04
+static const uint16 phant2CompSlideDoorsSignature[] = {
+	SIG_MAGICDWORD,
+	0x35, 0x00, // ldi 0
+	0xa5, 0x00, // sat 0
+	0x8d, 0x00, // lst 0
+	0x87, 0x01, // lap 1
+	SIG_END
+};
+
+static const uint16 phant2CompSlideDoorsPatch[] = {
+	0x78,                                     // push1
+	0x8f, 0x01,                               // lsp param[1]
+	0x43, kScummVMWaitId, PATCH_UINT16(0x02), // callk Wait, 2
+	0x48,                                     // ret
+	PATCH_END
+};
+
 //          script, description,                                      signature                        patch
 static const SciScriptPatcherEntry phantasmagoria2Signatures[] = {
-	{  true,     0, "slow interface fades",                        3, phant2SlowIFadeSignature,     phant2SlowIFadePatch },
+	{  true,     0, "slow interface fades",                        3, phant2SlowIFadeSignature,      phant2SlowIFadePatch },
+	{  true, 63016, "non-responsive mouse during music fades",     1, phant2Wait4FadeSignature,      phant2Wait4FadePatch },
+	{  true, 63019, "non-responsive mouse during computer load",   1, phant2CompSlideDoorsSignature, phant2CompSlideDoorsPatch },
 	SCI_SIGNATUREENTRY_TERMINATOR
 };
 


Commit: b3ecc54a7a3023a648378771f543b060f46f7fae
    https://github.com/scummvm/scummvm/commit/b3ecc54a7a3023a648378771f543b060f46f7fae
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI: Always search for .CSC script patches

Skipping a search for .CSC scripts when any .SCR files exist does
not work with at least Phant2, because it comes with an INSTALL.SCR
file. Searching unconditionally for .CSC files should not cause any
issues since the game scripts will either be in .SCR format or in
.CSC format, not both in the same game.

Changed paths:
    engines/sci/resource.cpp


diff --git a/engines/sci/resource.cpp b/engines/sci/resource.cpp
index c4df47d..f5edda9 100644
--- a/engines/sci/resource.cpp
+++ b/engines/sci/resource.cpp
@@ -1618,9 +1618,8 @@ void ResourceManager::readResourcePatches() {
 			SearchMan.listMatchingMembers(files, "*.p32");	// Amiga SCI1 picture patches
 			SearchMan.listMatchingMembers(files, "*.p64");	// Amiga AGA SCI1 (i.e. Longbow) picture patches
 		} else if (i == kResourceTypeScript) {
-			if (files.size() == 0)
-				// SCI3 (we can't use getSciVersion() at this point)
-				SearchMan.listMatchingMembers(files, "*.csc");
+			// SCI3 (we can't use getSciVersion() at this point)
+			SearchMan.listMatchingMembers(files, "*.csc");
 		}
 
 		for (Common::ArchiveMemberList::const_iterator x = files.begin(); x != files.end(); ++x) {


Commit: d24f5537be84ac175e85ed15415c8b3c0e00674d
    https://github.com/scummvm/scummvm/commit/d24f5537be84ac175e85ed15415c8b3c0e00674d
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Implement SCI3 Script::syncStringHeap

Changed paths:
    engines/sci/engine/savegame.cpp


diff --git a/engines/sci/engine/savegame.cpp b/engines/sci/engine/savegame.cpp
index 04715d6..4c16770 100644
--- a/engines/sci/engine/savegame.cpp
+++ b/engines/sci/engine/savegame.cpp
@@ -478,7 +478,10 @@ void Script::syncStringHeap(Common::Serializer &s) {
 		const int length = _heap.size() - (buf - _heap);
 		s.syncBytes(buf.getUnsafeDataAt(0, length), length);
 	} else if (getSciVersion() == SCI_VERSION_3) {
-		warning("TODO: syncStringHeap(): Implement SCI3 variant");
+		const int stringOffset = _buf->getInt32SEAt(4);
+		const int length = _buf->getInt32SEAt(8) - stringOffset;
+		SciSpan<byte> buf = _buf->subspan<byte>(stringOffset, length);
+		s.syncBytes(buf.getUnsafeDataAt(0, length), length);
 	}
 }
 


Commit: d0a143fa87b88649aaff39b543218925e888025c
    https://github.com/scummvm/scummvm/commit/d0a143fa87b88649aaff39b543218925e888025c
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Fix locals offset in SCI3

Locals offset needs to be set even when the script has no exports.

Changed paths:
    engines/sci/engine/script.cpp


diff --git a/engines/sci/engine/script.cpp b/engines/sci/engine/script.cpp
index 86f9d8d..1672f69 100644
--- a/engines/sci/engine/script.cpp
+++ b/engines/sci/engine/script.cpp
@@ -203,12 +203,13 @@ void Script::load(int script_nr, ResourceManager *resMan, ScriptPatcher *scriptP
 		_numExports = _buf->getUint16LEAt(20);
 		if (_numExports) {
 			_exports = _buf->subspan<const uint16>(22, _numExports * sizeof(uint16));
-			// SCI3 local variables always start dword-aligned
-			if (_numExports % 2)
-				_localsOffset = 22 + _numExports * 2;
-			else
-				_localsOffset = 24 + _numExports * 2;
 		}
+
+		// SCI3 local variables always start dword-aligned
+		if (_numExports % 2)
+			_localsOffset = 22 + _numExports * sizeof(uint16);
+		else
+			_localsOffset = 24 + _numExports * sizeof(uint16);
 	}
 
 	// WORKAROUND: Increase locals, if needed (check above)


Commit: 5c5e485ff0a48b4aee279ae746941339d881914c
    https://github.com/scummvm/scummvm/commit/5c5e485ff0a48b4aee279ae746941339d881914c
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Improve debugging output of object metadata in SCI3

Changed paths:
    engines/sci/console.cpp


diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index c81d086..b8e6097 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -4733,9 +4733,18 @@ int Console::printObject(reg_t pos) {
 	debugPrintf("[%04x:%04x] %s : %3d vars, %3d methods\n", PRINT_REG(pos), s->_segMan->getObjectName(pos),
 				obj->getVarCount(), obj->getMethodCount());
 
-	if (!obj->isClass() && getSciVersion() != SCI_VERSION_3)
+	if (!obj->isClass())
 		var_container = s->_segMan->getObject(obj->getSuperClassSelector());
 	debugPrintf("  -- member variables:\n");
+
+	if (getSciVersion() == SCI_VERSION_3) {
+		debugPrintf("    (----) [---] -size- = 0000:%04x (%d)\n", obj->getVarCount(), obj->getVarCount());
+		debugPrintf("    (----) [---] -classScript- = %04x:%04x (%d)\n", PRINT_REG(obj->getClassScriptSelector()), obj->getClassScriptSelector().getOffset());
+		debugPrintf("    (----) [---] -species- = %04x:%04x (%s)\n", PRINT_REG(obj->getSpeciesSelector()), s->_segMan->getObjectName(obj->getSpeciesSelector()));
+		debugPrintf("    (----) [---] -super- = %04x:%04x (%s)\n", PRINT_REG(obj->getSuperClassSelector()), s->_segMan->getObjectName(obj->getSuperClassSelector()));
+		debugPrintf("    (----) [---] -info- = %04x:%04x (%d)\n", PRINT_REG(obj->getInfoSelector()), obj->getInfoSelector().getOffset());
+	}
+
 	for (i = 0; (uint)i < obj->getVarCount(); i++) {
 		debugPrintf("    ");
 		if (var_container && i < var_container->getVarCount()) {


Commit: 82994609177c30b69f2be155670253b63469d876
    https://github.com/scummvm/scummvm/commit/82994609177c30b69f2be155670253b63469d876
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Hook up mustSetViewVisible for SCI3

Changed paths:
    engines/sci/engine/object.cpp
    engines/sci/engine/object.h
    engines/sci/engine/selector.cpp


diff --git a/engines/sci/engine/object.cpp b/engines/sci/engine/object.cpp
index 2c1ab2a..621810d 100644
--- a/engines/sci/engine/object.cpp
+++ b/engines/sci/engine/object.cpp
@@ -24,6 +24,9 @@
 #include "sci/engine/kernel.h"
 #include "sci/engine/object.h"
 #include "sci/engine/seg_manager.h"
+#ifdef ENABLE_SCI32
+#include "sci/engine/features.h"
+#endif
 
 namespace Sci {
 
@@ -247,6 +250,28 @@ bool Object::initBaseObject(SegManager *segMan, reg_t addr, bool doInitSuperClas
 
 const int EXTRA_GROUPS = 3;
 
+#ifdef ENABLE_SCI32
+bool Object::mustSetViewVisible(const int index) const {
+	if (getSciVersion() == SCI_VERSION_3) {
+		if ((uint)index < getVarCount()) {
+			return _mustSetViewVisible[getVarSelector(index) >> 5];
+		}
+		return false;
+	} else {
+		int minIndex, maxIndex;
+		if (g_sci->_features->usesAlternateSelectors()) {
+			minIndex = 24;
+			maxIndex = 43;
+		} else {
+			minIndex = 26;
+			maxIndex = 44;
+		}
+
+		return index >= minIndex && index <= maxIndex;
+	}
+}
+#endif
+
 void Object::initSelectorsSci3(const SciSpan<const byte> &buf) {
 	const SciSpan<const byte> groupInfo = _baseObj.subspan(16);
 	const SciSpan<const byte> selectorBase = groupInfo.subspan(EXTRA_GROUPS * 32 * 2);
diff --git a/engines/sci/engine/object.h b/engines/sci/engine/object.h
index dca7e97..20b1e01 100644
--- a/engines/sci/engine/object.h
+++ b/engines/sci/engine/object.h
@@ -295,7 +295,9 @@ public:
 	bool initBaseObject(SegManager *segMan, reg_t addr, bool doInitSuperClass = true);
 	void syncBaseObject(const SciSpan<const byte> &ptr) { _baseObj = ptr; }
 
-	bool mustSetViewVisibleSci3(int selector) const { return _mustSetViewVisible[selector/32]; }
+#ifdef ENABLE_SCI32
+	bool mustSetViewVisible(int selector) const;
+#endif
 
 private:
 	void initSelectorsSci3(const SciSpan<const byte> &buf);
diff --git a/engines/sci/engine/selector.cpp b/engines/sci/engine/selector.cpp
index 5bfc73b..7f509f3 100644
--- a/engines/sci/engine/selector.cpp
+++ b/engines/sci/engine/selector.cpp
@@ -231,16 +231,7 @@ reg_t readSelector(SegManager *segMan, reg_t object, Selector selectorId) {
 
 #ifdef ENABLE_SCI32
 void updateInfoFlagViewVisible(Object *obj, int index) {
-	int minIndex, maxIndex;
-	if (g_sci->_features->usesAlternateSelectors()) {
-		minIndex = 24;
-		maxIndex = 43;
-	} else {
-		minIndex = 26;
-		maxIndex = 44;
-	}
-
-	if (index >= minIndex && index <= maxIndex && getSciVersion() >= SCI_VERSION_2) {
+	if (getSciVersion() >= SCI_VERSION_2 && obj->mustSetViewVisible(index)) {
 		obj->setInfoSelectorFlag(kInfoFlagViewVisible);
 	}
 }


Commit: 90c6c0580ec4071d22a50e4c0276ac98e69af38a
    https://github.com/scummvm/scummvm/commit/90c6c0580ec4071d22a50e4c0276ac98e69af38a
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI: Convert Object to use Common::Array for SCI3

In SCI3, index-to-selector tables no longer exist in compiled
object data (instead, the SCI3 VM uses selectors directly and
object data contains a bit map of valid selectors). In ScummVM,
the table is generated by Object::initSelectorsSci3 for
compatibility with the design of the ScummVM SCI VM. For
consistency, _baseVars is converted to use a standard container,
which works for all SCI versions.

The table for SCI3 property offsets is also changed to use a
standard container instead of manually managing the memory with
malloc/free.

Changed paths:
    engines/sci/engine/object.cpp
    engines/sci/engine/object.h
    engines/sci/engine/savegame.cpp
    engines/sci/engine/savegame.h


diff --git a/engines/sci/engine/object.cpp b/engines/sci/engine/object.cpp
index 621810d..47d5519 100644
--- a/engines/sci/engine/object.cpp
+++ b/engines/sci/engine/object.cpp
@@ -62,20 +62,42 @@ void Object::init(const SciSpan<const byte> &buf, reg_t obj_pos, bool initVariab
 	if (getSciVersion() <= SCI_VERSION_1_LATE) {
 		const SciSpan<const byte> header = buf.subspan(obj_pos.getOffset() - kOffsetHeaderSize);
 		_variables.resize(header.getUint16LEAt(kOffsetHeaderSelectorCounter));
-		_baseVars = _baseObj.subspan<const uint16>(_variables.size() * sizeof(uint16));
+
+		// Non-class objects do not have a baseVars section
+		const uint16 infoSelector = data.getUint16SEAt((_offset + 2) * sizeof(uint16));
+		if (infoSelector & kInfoFlagClass) {
+			_baseVars.reserve(_variables.size());
+			uint baseVarsOffset = _variables.size() * sizeof(uint16);
+			for (uint i = 0; i < _variables.size(); ++i) {
+				_baseVars.push_back(data.getUint16SEAt(baseVarsOffset));
+				baseVarsOffset += sizeof(uint16);
+			}
+		}
+
 		_methodCount = data.getUint16LEAt(header.getUint16LEAt(kOffsetHeaderFunctionArea) - 2);
 		for (int i = 0; i < _methodCount * 2 + 2; ++i) {
 			_baseMethod.push_back(data.getUint16SEAt(header.getUint16LEAt(kOffsetHeaderFunctionArea) + i * 2));
 		}
 	} else if (getSciVersion() >= SCI_VERSION_1_1 && getSciVersion() <= SCI_VERSION_2_1_LATE) {
 		_variables.resize(data.getUint16SEAt(2));
-		_baseVars = buf.subspan<const uint16>(data.getUint16SEAt(4), _variables.size() * sizeof(uint16));
+
+		// Non-class objects do not have a baseVars section
+		const uint16 infoSelector = data.getUint16SEAt((_offset + 2) * sizeof(uint16));
+		if (infoSelector & kInfoFlagClass) {
+			_baseVars.reserve(_variables.size());
+			uint baseVarsOffset = data.getUint16SEAt(4);
+			for (uint i = 0; i < _variables.size(); ++i) {
+				_baseVars.push_back(buf.getUint16SEAt(baseVarsOffset));
+				baseVarsOffset += sizeof(uint16);
+			}
+		}
+
 		_methodCount = buf.getUint16SEAt(data.getUint16SEAt(6));
 		for (int i = 0; i < _methodCount * 2 + 3; ++i) {
 			_baseMethod.push_back(buf.getUint16SEAt(data.getUint16SEAt(6) + i * 2));
 		}
 	} else if (getSciVersion() == SCI_VERSION_3) {
-		initSelectorsSci3(buf);
+		initSelectorsSci3(buf, initVariables);
 	}
 
 	if (initVariables) {
@@ -83,7 +105,7 @@ void Object::init(const SciSpan<const byte> &buf, reg_t obj_pos, bool initVariab
 			for (uint i = 0; i < _variables.size(); i++)
 				_variables[i] = make_reg(0, data.getUint16SEAt(i * 2));
 		} else {
-			_infoSelectorSci3 = make_reg(0, _baseObj.getUint16SEAt(10));
+			_infoSelectorSci3 = make_reg(0, data.getUint16SEAt(10));
 		}
 	}
 }
@@ -93,20 +115,20 @@ const Object *Object::getClass(SegManager *segMan) const {
 }
 
 int Object::locateVarSelector(SegManager *segMan, Selector slc) const {
-	SciSpan<const byte> buf;
+	const Common::Array<uint16> *buf;
 	uint varnum = 0;
 
 	if (getSciVersion() <= SCI_VERSION_2_1_LATE) {
 		const Object *obj = getClass(segMan);
 		varnum = getSciVersion() <= SCI_VERSION_1_LATE ? getVarCount() : obj->getVariable(1).toUint16();
-		buf = obj->_baseVars.subspan<const byte>(0);
+		buf = &obj->_baseVars;
 	} else if (getSciVersion() == SCI_VERSION_3) {
 		varnum = _variables.size();
-		buf = _baseVars.subspan<const byte>(0);
+		buf = &_baseVars;
 	}
 
 	for (uint i = 0; i < varnum; i++)
-		if (buf.getUint16SEAt(i << 1) == slc) // Found it?
+		if ((*buf)[i] == slc) // Found it?
 			return i; // report success
 
 	return -1; // Failed
@@ -117,7 +139,7 @@ bool Object::relocateSci0Sci21(SegmentId segment, int location, size_t scriptSiz
 }
 
 bool Object::relocateSci3(SegmentId segment, uint32 location, int offset, size_t scriptSize) {
-	assert(_propertyOffsetsSci3);
+	assert(_propertyOffsetsSci3.size());
 
 	for (uint i = 0; i < _variables.size(); ++i) {
 		if (location == _propertyOffsetsSci3[i]) {
@@ -147,7 +169,7 @@ int Object::propertyOffsetToId(SegManager *segMan, int propertyOffset) const {
 		if (!isClass())
 			obj = segMan->getObject(getSuperClassSelector());
 
-		return obj->_baseVars.subspan<const byte>(0).getUint16SEAt(propertyOffset);
+		return obj->_baseVars[propertyOffset >> 1];
 	}
 }
 
@@ -272,7 +294,7 @@ bool Object::mustSetViewVisible(const int index) const {
 }
 #endif
 
-void Object::initSelectorsSci3(const SciSpan<const byte> &buf) {
+void Object::initSelectorsSci3(const SciSpan<const byte> &buf, const bool initVariables) {
 	const SciSpan<const byte> groupInfo = _baseObj.subspan(16);
 	const SciSpan<const byte> selectorBase = groupInfo.subspan(EXTRA_GROUPS * 32 * 2);
 	int groups = g_sci->getKernel()->getSelectorNamesSize()/32;
@@ -315,9 +337,9 @@ void Object::initSelectorsSci3(const SciSpan<const byte> &buf) {
 	}
 
 	_variables.resize(properties);
-	uint16 *propertyIds = (uint16 *)malloc(sizeof(uint16) * properties);
+	_propertyOffsetsSci3.resize(properties);
+	_baseVars.resize(properties);
 //	uint16 *methodOffsets = (uint16 *)malloc(sizeof(uint16) * 2 * methods);
-	uint32 *propertyOffsets = (uint32 *)malloc(sizeof(uint32) * properties);
 	int propertyCounter = 0;
 	int methodCounter = 0;
 
@@ -335,18 +357,12 @@ void Object::initSelectorsSci3(const SciSpan<const byte> &buf) {
 			for (int bit = 2; bit < 32; ++bit) {
 				int value = seeker.getUint16SEAt(bit * 2);
 				if (typeMask & (1 << bit)) { // Property
-
-					// FIXME: We really shouldn't be doing endianness
-					// conversion here; instead, propertyIds should be converted
-					// to a Common::Array, like _baseMethod already is
-					// This interim solution fixes playing SCI3 PC games
-					// on Big Endian platforms
-
-					WRITE_SCI11ENDIAN_UINT16(&propertyIds[propertyCounter],
-					                         groupBaseId + bit);
-					_variables[propertyCounter] = make_reg(0, value);
+					_baseVars[propertyCounter] = groupBaseId + bit;
+					if (initVariables) {
+						_variables[propertyCounter] = make_reg(0, value);
+					}
 					uint32 propertyOffset = (seeker + bit * 2) - buf;
-					propertyOffsets[propertyCounter] = propertyOffset;
+					_propertyOffsetsSci3[propertyCounter] = propertyOffset;
 					++propertyCounter;
 				} else if (value != 0xffff) { // Method
 					_baseMethod.push_back(groupBaseId + bit);
@@ -361,12 +377,11 @@ void Object::initSelectorsSci3(const SciSpan<const byte> &buf) {
 		}
 	}
 
-	_speciesSelectorSci3 = make_reg(0, _baseObj.getUint16SEAt(4));
-	_superClassPosSci3 = make_reg(0, _baseObj.getUint16SEAt(8));
-
-	_baseVars = SciSpan<const uint16>(propertyIds, properties);
+	if (initVariables) {
+		_speciesSelectorSci3 = make_reg(0, _baseObj.getUint16SEAt(4));
+		_superClassPosSci3 = make_reg(0, _baseObj.getUint16SEAt(8));
+	}
 	_methodCount = methods;
-	_propertyOffsetsSci3 = propertyOffsets;
 	//_methodOffsetsSci3 = methodOffsets;
 }
 
diff --git a/engines/sci/engine/object.h b/engines/sci/engine/object.h
index 20b1e01..8957a61 100644
--- a/engines/sci/engine/object.h
+++ b/engines/sci/engine/object.h
@@ -73,24 +73,13 @@ enum ObjectOffsets {
 
 class Object {
 public:
-	Object() {
-		_offset = getSciVersion() < SCI_VERSION_1_1 ? 0 : 5;
-		_flags = 0;
-		_baseObj.clear();
-		_baseVars.clear();
-		_methodCount = 0;
-		_propertyOffsetsSci3 = nullptr;
-	}
-
-	~Object() {
-		if (getSciVersion() == SCI_VERSION_3) {
-			// TODO: This is super gross
-			free(const_cast<uint16 *>(_baseVars.data()));
-			_baseVars.clear();
-			free(_propertyOffsetsSci3);
-			_propertyOffsetsSci3 = nullptr;
-		}
-	}
+	Object() :
+		_offset(getSciVersion() < SCI_VERSION_1_1 ? 0 : 5),
+		_flags(0),
+		_baseObj(),
+		_baseVars(),
+		_methodCount(0),
+		_propertyOffsetsSci3() {}
 
 	Object &operator=(const Object &other) {
 		_baseObj = other._baseObj;
@@ -100,21 +89,14 @@ public:
 		_flags = other._flags;
 		_offset = other._offset;
 		_pos = other._pos;
+		_baseVars = other._baseVars;
 
 		if (getSciVersion() == SCI_VERSION_3) {
-			uint16 *baseVars = (uint16 *)malloc(other._baseVars.byteSize());
-			other._baseVars.unsafeCopyDataTo(baseVars);
-			_baseVars = SciSpan<const uint16>(baseVars, other._baseVars.size());
-
-			_propertyOffsetsSci3 = (uint32 *)malloc(sizeof(uint32) * _variables.size());
-			memcpy(_propertyOffsetsSci3, other._propertyOffsetsSci3, sizeof(uint32) * _variables.size());
-
+			_propertyOffsetsSci3 = other._propertyOffsetsSci3;
 			_superClassPosSci3 = other._superClassPosSci3;
 			_speciesSelectorSci3 = other._speciesSelectorSci3;
 			_infoSelectorSci3 = other._infoSelectorSci3;
 			_mustSetViewVisible = other._mustSetViewVisible;
-		} else {
-			_baseVars = other._baseVars;
 		}
 
 		return *this;
@@ -225,7 +207,7 @@ public:
 			error("setClassScriptSelector called for SCI3");
 	}
 
-	Selector getVarSelector(uint16 i) const { return _baseVars.getUint16SEAt(i); }
+	Selector getVarSelector(uint16 i) const { return _baseVars[i]; }
 
 	reg_t getFunction(uint16 i) const {
 		uint16 offset = (getSciVersion() < SCI_VERSION_1_1) ? _methodCount + 1 + i : i * 2 + 2;
@@ -282,7 +264,7 @@ public:
 	void cloneFromObject(const Object *obj) {
 		_baseObj = obj ? obj->_baseObj : SciSpan<const byte>();
 		_baseMethod = obj ? obj->_baseMethod : Common::Array<uint16>();
-		_baseVars = obj ? obj->_baseVars : SciSpan<const uint16>();
+		_baseVars = obj ? obj->_baseVars : Common::Array<uint16>();
 	}
 
 	bool relocateSci0Sci21(SegmentId segment, int location, size_t scriptSize);
@@ -300,12 +282,12 @@ public:
 #endif
 
 private:
-	void initSelectorsSci3(const SciSpan<const byte> &buf);
+	void initSelectorsSci3(const SciSpan<const byte> &buf, const bool initVariables);
 
 	SciSpan<const byte> _baseObj; /**< base + object offset within base */
-	SciSpan<const uint16> _baseVars; /**< Pointer to the varselector area for this object */
-	Common::Array<uint16> _baseMethod; /**< Pointer to the method selector area for this object */
-	uint32 *_propertyOffsetsSci3; /**< This is used to enable relocation of property values in SCI3 */
+	Common::Array<uint16> _baseVars; /**< The varselector area for this object */
+	Common::Array<uint16> _baseMethod; /**< The method selector area for this object */
+	Common::Array<uint32> _propertyOffsetsSci3; /**< Enables relocation of property values in SCI3 */
 
 	Common::Array<reg_t> _variables;
 	uint16 _methodCount;
diff --git a/engines/sci/engine/savegame.cpp b/engines/sci/engine/savegame.cpp
index 4c16770..a82499a 100644
--- a/engines/sci/engine/savegame.cpp
+++ b/engines/sci/engine/savegame.cpp
@@ -103,6 +103,10 @@ void syncWithSerializer(Common::Serializer &s, Node &obj) {
 	syncWithSerializer(s, obj.value);
 }
 
+void syncWithSerializer(Common::Serializer &s, bool &obj) {
+	s.syncAsByte(obj);
+}
+
 #pragma mark -
 
 // By default, sync using syncWithSerializer, which in turn can easily be overloaded.
@@ -415,6 +419,12 @@ void Object::saveLoadWithSerializer(Common::Serializer &s) {
 	s.syncAsSint32LE(_methodCount);		// that's actually a uint16
 
 	syncArray<reg_t>(s, _variables);
+	if (s.getVersion() >= 42 && getSciVersion() == SCI_VERSION_3) {
+		syncArray<bool>(s, _mustSetViewVisible);
+		syncWithSerializer(s, _superClassPosSci3);
+		syncWithSerializer(s, _speciesSelectorSci3);
+		syncWithSerializer(s, _infoSelectorSci3);
+	}
 }
 
 
diff --git a/engines/sci/engine/savegame.h b/engines/sci/engine/savegame.h
index b6a673b..53377f0 100644
--- a/engines/sci/engine/savegame.h
+++ b/engines/sci/engine/savegame.h
@@ -37,6 +37,7 @@ struct EngineState;
  *
  * Version - new/changed feature
  * =============================
+ *      42 - initial SCI3 support
  *      41 - palette support for newer SCI2.1 games; stable SCI2/2.1 save games
  *      40 - always store palvary variables
  *      39 - Accurate SCI32 arrays/strings, score metadata, avatar metadata
@@ -66,7 +67,7 @@ struct EngineState;
  */
 
 enum {
-	CURRENT_SAVEGAME_VERSION = 41,
+	CURRENT_SAVEGAME_VERSION = 42,
 	MINIMUM_SAVEGAME_VERSION = 14
 #ifdef ENABLE_SCI32
 	,


Commit: 6b95528b49e40117a309dab8cb593d140c783cf4
    https://github.com/scummvm/scummvm/commit/6b95528b49e40117a309dab8cb593d140c783cf4
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Fix bad relocations of SCI3 objects

Changed paths:
    engines/sci/engine/object.cpp
    engines/sci/engine/script.cpp


diff --git a/engines/sci/engine/object.cpp b/engines/sci/engine/object.cpp
index 47d5519..b507e36 100644
--- a/engines/sci/engine/object.cpp
+++ b/engines/sci/engine/object.cpp
@@ -140,6 +140,7 @@ bool Object::relocateSci0Sci21(SegmentId segment, int location, size_t scriptSiz
 
 bool Object::relocateSci3(SegmentId segment, uint32 location, int offset, size_t scriptSize) {
 	assert(_propertyOffsetsSci3.size());
+	assert(offset >= 0 && (uint)offset < scriptSize);
 
 	for (uint i = 0; i < _variables.size(); ++i) {
 		if (location == _propertyOffsetsSci3[i]) {
diff --git a/engines/sci/engine/script.cpp b/engines/sci/engine/script.cpp
index 1672f69..f244373 100644
--- a/engines/sci/engine/script.cpp
+++ b/engines/sci/engine/script.cpp
@@ -764,13 +764,12 @@ void Script::relocateSci0Sci21(reg_t block) {
 
 void Script::relocateSci3(reg_t block) {
 	SciSpan<const byte> relocStart = _buf->subspan(_buf->getUint32SEAt(8));
-	//int count = _bufSize - READ_SCI11ENDIAN_UINT32(_buf + 8);
+	const uint relocCount = _buf->getUint16SEAt(18);
 
 	ObjMap::iterator it;
 	for (it = _objects.begin(); it != _objects.end(); ++it) {
 		SciSpan<const byte> seeker = relocStart;
-		while (seeker.size()) {
-			// TODO: Find out what UINT16 at (seeker + 8) means
+		for (uint i = 0; i < relocCount; ++i) {
 			it->_value.relocateSci3(block.getSegment(),
 						seeker.getUint32SEAt(0),
 						seeker.getUint32SEAt(4),


Commit: 0676e640dbebbe2f49aab902454b99693b1bb560
    https://github.com/scummvm/scummvm/commit/0676e640dbebbe2f49aab902454b99693b1bb560
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI: Fix bad offsets in disassembly for SCI3 lofsa/lofss

Changed paths:
    engines/sci/engine/scriptdebug.cpp


diff --git a/engines/sci/engine/scriptdebug.cpp b/engines/sci/engine/scriptdebug.cpp
index 507fd97..52281e2 100644
--- a/engines/sci/engine/scriptdebug.cpp
+++ b/engines/sci/engine/scriptdebug.cpp
@@ -225,9 +225,12 @@ reg_t disassemble(EngineState *s, reg32_t pos, reg_t objAddr, bool printBWTag, b
 			}
 
 			if (opcode == op_lofsa || opcode == op_lofss) {
-				reg_t addr = make_reg(pos.getSegment(), findOffset(param_value, script_entity, pos.getOffset()));
+				const uint32 offset = findOffset(param_value, script_entity, retval.getOffset());
+				reg_t addr;
+				addr.setSegment(retval.getSegment());
+				addr.setOffset(offset);
 				debugN("\t%s", s->_segMan->getObjectName(addr));
-				debugN(opsize ? "[%02x]" : "[%04x]", param_value);
+				debugN(opsize ? "[%02x]" : "[%04x]", offset);
 			} else {
 				debugN(opsize ? "\t%02x" : "\t%04x", param_value);
 			}


Commit: 61fba94139c25c9c91c2f07c8282fa6aaa610a71
    https://github.com/scummvm/scummvm/commit/61fba94139c25c9c91c2f07c8282fa6aaa610a71
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI: Remove dead code in Script_Offset disassembler

Script_Offset is only ever used for lofsa/lofss opcodes.

Changed paths:
    engines/sci/engine/scriptdebug.cpp


diff --git a/engines/sci/engine/scriptdebug.cpp b/engines/sci/engine/scriptdebug.cpp
index 52281e2..6c54890 100644
--- a/engines/sci/engine/scriptdebug.cpp
+++ b/engines/sci/engine/scriptdebug.cpp
@@ -215,7 +215,9 @@ reg_t disassemble(EngineState *s, reg32_t pos, reg_t objAddr, bool printBWTag, b
 
 			break;
 
-		case Script_Offset:
+		case Script_Offset: {
+			assert(opcode == op_lofsa || opcode == op_lofss);
+
 			if (opsize) {
 				param_value = scr[retval.getOffset()];
 				retval.incOffset(1);
@@ -224,26 +226,21 @@ reg_t disassemble(EngineState *s, reg32_t pos, reg_t objAddr, bool printBWTag, b
 				retval.incOffset(2);
 			}
 
-			if (opcode == op_lofsa || opcode == op_lofss) {
-				const uint32 offset = findOffset(param_value, script_entity, retval.getOffset());
-				reg_t addr;
-				addr.setSegment(retval.getSegment());
-				addr.setOffset(offset);
-				debugN("\t%s", s->_segMan->getObjectName(addr));
-				debugN(opsize ? "[%02x]" : "[%04x]", offset);
-			} else {
-				debugN(opsize ? "\t%02x" : "\t%04x", param_value);
-			}
-
+			const uint32 offset = findOffset(param_value, script_entity, retval.getOffset());
+			reg_t addr;
+			addr.setSegment(retval.getSegment());
+			addr.setOffset(offset);
+			debugN("\t%s", s->_segMan->getObjectName(addr));
+			debugN(opsize ? "[%02x]" : "[%04x]", offset);
 			break;
+		}
 
 		case Script_SRelative:
 			if (opsize) {
 				int8 offset = (int8)scr[retval.getOffset()];
 				retval.incOffset(1);
 				debugN("\t%02x  [%04x]", 0xff & offset, kOffsetMask & (retval.getOffset() + offset));
-			}
-			else {
+			} else {
 				int16 offset = (int16)READ_SCI11ENDIAN_UINT16(&scr[retval.getOffset()]);
 				retval.incOffset(2);
 				debugN("\t%04x  [%04x]", 0xffff & offset, kOffsetMask & (retval.getOffset() + offset));


Commit: 0394fd44e8d1b4e20cbf465365fef1ad0e8bf658
    https://github.com/scummvm/scummvm/commit/0394fd44e8d1b4e20cbf465365fef1ad0e8bf658
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI: Fix whitespace errors

Changed paths:
    engines/sci/engine/script.cpp
    engines/sci/engine/scriptdebug.cpp


diff --git a/engines/sci/engine/script.cpp b/engines/sci/engine/script.cpp
index f244373..8479e43 100644
--- a/engines/sci/engine/script.cpp
+++ b/engines/sci/engine/script.cpp
@@ -696,7 +696,7 @@ static bool relocateBlock(Common::Array<reg_t> &block, int block_location, Segme
 int Script::relocateOffsetSci3(uint32 offset) const {
 	int relocStart = _buf->getUint32LEAt(8);
 	int relocCount = _buf->getUint16LEAt(18);
-	SciSpan <const byte> seeker = _buf->subspan(relocStart);
+	SciSpan<const byte> seeker = _buf->subspan(relocStart);
 
 	for (int i = 0; i < relocCount; ++i) {
 		if (seeker.getUint32SEAt(0) == offset) {
diff --git a/engines/sci/engine/scriptdebug.cpp b/engines/sci/engine/scriptdebug.cpp
index 6c54890..54f7d52 100644
--- a/engines/sci/engine/scriptdebug.cpp
+++ b/engines/sci/engine/scriptdebug.cpp
@@ -461,7 +461,7 @@ void SciEngine::scriptDebug() {
 	_console->attach();
 }
 
-void Kernel::dumpScriptObject(const SciSpan<const byte> &script, SciSpan <const byte> object) {
+void Kernel::dumpScriptObject(const SciSpan<const byte> &script, SciSpan<const byte> object) {
 	const int16 species = object.getInt16SEAt(8);
 	const int16 superclass = object.getInt16SEAt(10);
 	const int16 namepos = object.getInt16SEAt(14);


Commit: cb50682b15c308cb075b7c46ce24b499827f4260
    https://github.com/scummvm/scummvm/commit/cb50682b15c308cb075b7c46ce24b499827f4260
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI: Improve documentation of Object class

Changed paths:
    engines/sci/engine/object.h


diff --git a/engines/sci/engine/object.h b/engines/sci/engine/object.h
index 8957a61..4fd1385 100644
--- a/engines/sci/engine/object.h
+++ b/engines/sci/engine/object.h
@@ -284,15 +284,47 @@ public:
 private:
 	void initSelectorsSci3(const SciSpan<const byte> &buf, const bool initVariables);
 
-	SciSpan<const byte> _baseObj; /**< base + object offset within base */
-	Common::Array<uint16> _baseVars; /**< The varselector area for this object */
-	Common::Array<uint16> _baseMethod; /**< The method selector area for this object */
-	Common::Array<uint32> _propertyOffsetsSci3; /**< Enables relocation of property values in SCI3 */
+	/**
+	 * A pointer to the raw object data within the object's owner script.
+	 */
+	SciSpan<const byte> _baseObj;
+
+	/**
+	 * A lookup table from a property index to its corresponding selector number.
+	 */
+	Common::Array<uint16> _baseVars;
 
+	/**
+	 * A lookup table from a method index to its corresponding selector number.
+	 */
+	Common::Array<uint16> _baseMethod;
+
+	/**
+	 * A lookup table from a property index to the property's current value.
+	 */
 	Common::Array<reg_t> _variables;
+
+	/**
+	 * A lookup table from a property index to the property's original absolute
+	 * offset within the raw script data. This absolute offset is coded into the
+	 * script's relocation table, and is used to look up 32-bit values for
+	 * properties in the relocation table (which is used to break the 16-bit
+	 * barrier, since the script format still only holds 16-bit values inline).
+	 */
+	Common::Array<uint32> _propertyOffsetsSci3;
+
+	/**
+	 * The number of methods on the object.
+	 */
 	uint16 _methodCount;
 	int _flags;
+
+	/**
+	 * For SCI0 through SCI2.1, an extra index offset used when looking up
+	 * special object properties -species-, -super-, -info-, and name.
+	 */
 	uint16 _offset;
+
 	reg_t _pos; /**< Object offset within its script; for clones, this is their base */
 	reg_t _superClassPosSci3; /**< reg_t pointing to superclass for SCI3 */
 	reg_t _speciesSelectorSci3;	/**< reg_t containing species "selector" for SCI3 */


Commit: 2906ca994716d77cca73928a0c053ac5f2aadd94
    https://github.com/scummvm/scummvm/commit/2906ca994716d77cca73928a0c053ac5f2aadd94
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI: Replace mostly-unused flags property with a single boolean

There does not appear to be any reason to use a bit field instead
of a simple boolean for this one flag, since there are no other
flags that need to be set on Object like this.

Changed paths:
    engines/sci/engine/object.h
    engines/sci/engine/savegame.cpp


diff --git a/engines/sci/engine/object.h b/engines/sci/engine/object.h
index 4fd1385..1af3b7b 100644
--- a/engines/sci/engine/object.h
+++ b/engines/sci/engine/object.h
@@ -35,11 +35,6 @@ namespace Sci {
 
 class SegManager;
 
-/** Clone has been marked as 'freed' */
-enum {
-	OBJECT_FLAG_FREED = (1 << 0)
-};
-
 enum infoSelectorFlags {
 	kInfoFlagClone        = 0x0001,
 #ifdef ENABLE_SCI32
@@ -75,7 +70,7 @@ class Object {
 public:
 	Object() :
 		_offset(getSciVersion() < SCI_VERSION_1_1 ? 0 : 5),
-		_flags(0),
+		_isFreed(false),
 		_baseObj(),
 		_baseVars(),
 		_methodCount(0),
@@ -86,7 +81,7 @@ public:
 		_baseMethod = other._baseMethod;
 		_variables = other._variables;
 		_methodCount = other._methodCount;
-		_flags = other._flags;
+		_isFreed = other._isFreed;
 		_offset = other._offset;
 		_pos = other._pos;
 		_baseVars = other._baseVars;
@@ -246,8 +241,8 @@ public:
 	bool isClass() const { return (getInfoSelector().getOffset() & kInfoFlagClass); }
 	const Object *getClass(SegManager *segMan) const;
 
-	void markAsFreed() { _flags |= OBJECT_FLAG_FREED; }
-	bool isFreed() const { return _flags & OBJECT_FLAG_FREED; }
+	void markAsFreed() { _isFreed = true; }
+	bool isFreed() const { return _isFreed; }
 
 	uint getVarCount() const { return _variables.size(); }
 
@@ -317,7 +312,11 @@ private:
 	 * The number of methods on the object.
 	 */
 	uint16 _methodCount;
-	int _flags;
+
+	/**
+	 * Whether or not a clone object has been marked as 'freed'.
+	 */
+	bool _isFreed;
 
 	/**
 	 * For SCI0 through SCI2.1, an extra index offset used when looking up
diff --git a/engines/sci/engine/savegame.cpp b/engines/sci/engine/savegame.cpp
index a82499a..2306fc6 100644
--- a/engines/sci/engine/savegame.cpp
+++ b/engines/sci/engine/savegame.cpp
@@ -414,7 +414,7 @@ void LocalVariables::saveLoadWithSerializer(Common::Serializer &s) {
 }
 
 void Object::saveLoadWithSerializer(Common::Serializer &s) {
-	s.syncAsSint32LE(_flags);
+	s.syncAsSint32LE(_isFreed);
 	syncWithSerializer(s, _pos);
 	s.syncAsSint32LE(_methodCount);		// that's actually a uint16
 


Commit: eadf5d818f7dd124a8d70fde7ced8a9e5ce35c04
    https://github.com/scummvm/scummvm/commit/eadf5d818f7dd124a8d70fde7ced8a9e5ce35c04
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI: Fix support for 32-bit SCI3 script offsets

Changed paths:
    engines/sci/engine/kscripts.cpp
    engines/sci/engine/object.cpp
    engines/sci/engine/object.h
    engines/sci/engine/script.cpp
    engines/sci/engine/script.h
    engines/sci/engine/scriptdebug.cpp
    engines/sci/engine/segment.cpp
    engines/sci/engine/segment.h


diff --git a/engines/sci/engine/kscripts.cpp b/engines/sci/engine/kscripts.cpp
index a7c0d87..30a1fab 100644
--- a/engines/sci/engine/kscripts.cpp
+++ b/engines/sci/engine/kscripts.cpp
@@ -262,7 +262,10 @@ reg_t kScriptID(EngineState *s, int argc, reg_t *argv) {
 		s->variables[VAR_GLOBAL][kGlobalVarSpeed] = make_reg(0, 6);
 	}
 
-	return make_reg(scriptSeg, address);
+	reg_t addr;
+	addr.setSegment(scriptSeg);
+	addr.setOffset(address);
+	return addr;
 }
 
 reg_t kDisposeScript(EngineState *s, int argc, reg_t *argv) {
diff --git a/engines/sci/engine/object.cpp b/engines/sci/engine/object.cpp
index b507e36..177ff98 100644
--- a/engines/sci/engine/object.cpp
+++ b/engines/sci/engine/object.cpp
@@ -298,22 +298,22 @@ bool Object::mustSetViewVisible(const int index) const {
 void Object::initSelectorsSci3(const SciSpan<const byte> &buf, const bool initVariables) {
 	const SciSpan<const byte> groupInfo = _baseObj.subspan(16);
 	const SciSpan<const byte> selectorBase = groupInfo.subspan(EXTRA_GROUPS * 32 * 2);
-	int groups = g_sci->getKernel()->getSelectorNamesSize()/32;
-	int methods, properties;
 
+	int numGroups = g_sci->getKernel()->getSelectorNamesSize() / 32;
 	if (g_sci->getKernel()->getSelectorNamesSize() % 32)
-		++groups;
+		++numGroups;
 
-	_mustSetViewVisible.resize(groups);
+	_mustSetViewVisible.resize(numGroups);
 
-	methods = properties = 0;
+	int numMethods = 0;
+	int numProperties = 0;
 
 	// Selectors are divided into groups of 32, of which the first
 	// two selectors are always reserved (because their storage
 	// space is used by the typeMask).
 	// We don't know beforehand how many methods and properties
 	// there are, so we count them first.
-	for (int groupNr = 0; groupNr < groups; ++groupNr) {
+	for (int groupNr = 0; groupNr < numGroups; ++groupNr) {
 		byte groupLocation = groupInfo[groupNr];
 		const SciSpan<const byte> seeker = selectorBase.subspan(groupLocation * 32 * 2);
 
@@ -326,9 +326,9 @@ void Object::initSelectorsSci3(const SciSpan<const byte> &buf, const bool initVa
 			for (int bit = 2; bit < 32; ++bit) {
 				int value = seeker.getUint16SEAt(bit * 2);
 				if (typeMask & (1 << bit)) { // Property
-					++properties;
+					++numProperties;
 				} else if (value != 0xffff) { // Method
-					++methods;
+					++numMethods;
 				} else {
 					// Undefined selector
 				}
@@ -337,16 +337,15 @@ void Object::initSelectorsSci3(const SciSpan<const byte> &buf, const bool initVa
 			_mustSetViewVisible[groupNr] = false;
 	}
 
-	_variables.resize(properties);
-	_propertyOffsetsSci3.resize(properties);
-	_baseVars.resize(properties);
-//	uint16 *methodOffsets = (uint16 *)malloc(sizeof(uint16) * 2 * methods);
-	int propertyCounter = 0;
-	int methodCounter = 0;
+	_methodCount = numMethods;
+	_variables.resize(numProperties);
+	_baseVars.resize(numProperties);
+	_propertyOffsetsSci3.resize(numProperties);
 
 	// Go through the whole thing again to get the property values
 	// and method pointers
-	for (int groupNr = 0; groupNr < groups; ++groupNr) {
+	int propertyCounter = 0;
+	for (int groupNr = 0; groupNr < numGroups; ++groupNr) {
 		byte groupLocation = groupInfo[groupNr];
 		const SciSpan<const byte> seeker = selectorBase.subspan(groupLocation * 32 * 2);
 
@@ -367,13 +366,12 @@ void Object::initSelectorsSci3(const SciSpan<const byte> &buf, const bool initVa
 					++propertyCounter;
 				} else if (value != 0xffff) { // Method
 					_baseMethod.push_back(groupBaseId + bit);
-					_baseMethod.push_back(value + buf.getUint32SEAt(0));
-//					methodOffsets[methodCounter] = (seeker + bit * 2) - buf;
-					++methodCounter;
+					const uint32 offset = value + buf.getUint32SEAt(0);
+					assert(offset <= kOffsetMask);
+					_baseMethod.push_back(offset);
 				} else {
 					// Undefined selector
 				}
-
 			}
 		}
 	}
@@ -382,8 +380,6 @@ void Object::initSelectorsSci3(const SciSpan<const byte> &buf, const bool initVa
 		_speciesSelectorSci3 = make_reg(0, _baseObj.getUint16SEAt(4));
 		_superClassPosSci3 = make_reg(0, _baseObj.getUint16SEAt(8));
 	}
-	_methodCount = methods;
-	//_methodOffsetsSci3 = methodOffsets;
 }
 
 } // End of namespace Sci
diff --git a/engines/sci/engine/object.h b/engines/sci/engine/object.h
index 1af3b7b..992b6cd 100644
--- a/engines/sci/engine/object.h
+++ b/engines/sci/engine/object.h
@@ -208,7 +208,11 @@ public:
 		uint16 offset = (getSciVersion() < SCI_VERSION_1_1) ? _methodCount + 1 + i : i * 2 + 2;
 		if (getSciVersion() == SCI_VERSION_3)
 			offset--;
-		return make_reg(_pos.getSegment(), _baseMethod[offset]);
+
+		reg_t addr;
+		addr.setSegment(_pos.getSegment());
+		addr.setOffset(_baseMethod[offset]);
+		return addr;
 	}
 
 	Selector getFuncSelector(uint16 i) const {
@@ -258,7 +262,7 @@ public:
 
 	void cloneFromObject(const Object *obj) {
 		_baseObj = obj ? obj->_baseObj : SciSpan<const byte>();
-		_baseMethod = obj ? obj->_baseMethod : Common::Array<uint16>();
+		_baseMethod = obj ? obj->_baseMethod : Common::Array<uint32>();
 		_baseVars = obj ? obj->_baseVars : Common::Array<uint16>();
 	}
 
@@ -285,14 +289,16 @@ private:
 	SciSpan<const byte> _baseObj;
 
 	/**
-	 * A lookup table from a property index to its corresponding selector number.
+	 * A lookup table from a property index to its corresponding selector
+	 * number.
 	 */
 	Common::Array<uint16> _baseVars;
 
 	/**
 	 * A lookup table from a method index to its corresponding selector number.
+	 * In SCI3, the table contains selector + offset in pairs.
 	 */
-	Common::Array<uint16> _baseMethod;
+	Common::Array<uint32> _baseMethod;
 
 	/**
 	 * A lookup table from a property index to the property's current value.
diff --git a/engines/sci/engine/script.cpp b/engines/sci/engine/script.cpp
index 8479e43..fa17dc0 100644
--- a/engines/sci/engine/script.cpp
+++ b/engines/sci/engine/script.cpp
@@ -113,22 +113,9 @@ void Script::load(int script_nr, ResourceManager *resMan, ScriptPatcher *scriptP
 			error("Script and heap sizes combined exceed 64K. This means a fundamental "
 					"design bug was made regarding SCI1.1 and newer games.\n"
 					"Please report this error to the ScummVM team");
-	} else if (getSciVersion() == SCI_VERSION_3) {
-		// Check for scripts over 64KB. These won't work with the current 16-bit address
-		// scheme. We need an overlaying mechanism, or a mechanism to split script parts
-		// in different segments to handle these. For now, simply stop when such a script
-		// is found.
-		//
-		// Known large SCI 3 scripts are:
-		// Lighthouse: 9, 220, 270, 351, 360, 490, 760, 765, 800
-		// LSL7: 240, 511, 550
-		// Phantasmagoria 2: none (hooray!)
-		// RAMA: 70
-		//
-		// TODO: Remove this once such a mechanism is in place
-		if (script->size() > 65535)
-			warning("TODO: SCI script %d is over 64KB - it's %u bytes long. This can't "
-			      "be fully handled at the moment", script_nr, script->size());
+	} else if (getSciVersion() == SCI_VERSION_3 && script->size() > 0x3FFFF) {
+		error("Script %d size exceeds 256K (it is %u bytes).\n"
+			  "Please report this error to the ScummVM team", script_nr, script->size());
 	}
 
 	uint extraLocalsWorkaround = 0;
@@ -640,14 +627,14 @@ SciSpan<const byte> Script::getSci3ObjectsPointer() {
 	return ptr;
 }
 
-Object *Script::getObject(uint16 offset) {
+Object *Script::getObject(uint32 offset) {
 	if (_objects.contains(offset))
 		return &_objects[offset];
 	else
 		return 0;
 }
 
-const Object *Script::getObject(uint16 offset) const {
+const Object *Script::getObject(uint32 offset) const {
 	if (_objects.contains(offset))
 		return &_objects[offset];
 	else
@@ -866,7 +853,7 @@ SciSpan<const byte> Script::findBlockSCI0(ScriptObjectTypes type, bool findLastB
 
 // memory operations
 
-bool Script::isValidOffset(uint16 offset) const {
+bool Script::isValidOffset(uint32 offset) const {
 	return offset < _buf->size();
 }
 
@@ -995,7 +982,10 @@ void Script::initializeClasses(SegManager *segMan) {
 						  species, species, segMan->classTableSize(), segMan->classTableSize(), _nr);
 
 			SegmentId segmentId = segMan->getScriptSegment(_nr);
-			segMan->setClassOffset(species, make_reg(segmentId, classpos));
+			reg_t classOffset;
+			classOffset.setSegment(segmentId);
+			classOffset.setOffset(classpos);
+			segMan->setClassOffset(species, classOffset);
 		}
 
 		seeker += seeker.getUint16SEAt(2) * mult;
@@ -1182,7 +1172,7 @@ Common::Array<reg_t> Script::listObjectReferences() const {
 	return tmp;
 }
 
-bool Script::offsetIsObject(uint16 offset) const {
+bool Script::offsetIsObject(uint32 offset) const {
 	return _buf->getUint16SEAt(offset + SCRIPT_OBJECT_MAGIC_OFFSET) == SCRIPT_OBJECT_MAGIC_NUMBER;
 }
 
diff --git a/engines/sci/engine/script.h b/engines/sci/engine/script.h
index 01f3e78..2be3fd0 100644
--- a/engines/sci/engine/script.h
+++ b/engines/sci/engine/script.h
@@ -48,7 +48,7 @@ enum ScriptObjectTypes {
 	SCI_OBJ_LOCALVARS
 };
 
-typedef Common::HashMap<uint16, Object> ObjMap;
+typedef Common::HashMap<uint32, Object> ObjMap;
 
 enum ScriptOffsetEntryTypes {
 	SCI_SCR_OFFSET_TYPE_OBJECT = 0, // classes are handled by this type as well
@@ -114,7 +114,7 @@ public:
 	void syncLocalsBlock(SegManager *segMan);
 	ObjMap &getObjectMap() { return _objects; }
 	const ObjMap &getObjectMap() const { return _objects; }
-	bool offsetIsObject(uint16 offset) const;
+	bool offsetIsObject(uint32 offset) const;
 
 public:
 	Script();
@@ -123,7 +123,7 @@ public:
 	void freeScript();
 	void load(int script_nr, ResourceManager *resMan, ScriptPatcher *scriptPatcher);
 
-	virtual bool isValidOffset(uint16 offset) const;
+	virtual bool isValidOffset(uint32 offset) const;
 	virtual SegmentRef dereference(reg_t pointer);
 	virtual reg_t findCanonicAddress(SegManager *segMan, reg_t sub_addr) const;
 	virtual void freeAtAddress(SegManager *segMan, reg_t sub_addr);
@@ -140,8 +140,8 @@ public:
 
 	virtual void saveLoadWithSerializer(Common::Serializer &ser);
 
-	Object *getObject(uint16 offset);
-	const Object *getObject(uint16 offset) const;
+	Object *getObject(uint32 offset);
+	const Object *getObject(uint32 offset) const;
 
 	/**
 	 * Initializes an object within the segment manager
diff --git a/engines/sci/engine/scriptdebug.cpp b/engines/sci/engine/scriptdebug.cpp
index 54f7d52..7d186eb 100644
--- a/engines/sci/engine/scriptdebug.cpp
+++ b/engines/sci/engine/scriptdebug.cpp
@@ -71,7 +71,9 @@ const char *opcodeNames[] = {
 reg_t disassemble(EngineState *s, reg32_t pos, reg_t objAddr, bool printBWTag, bool printBytecode) {
 	SegmentObj *mobj = s->_segMan->getSegment(pos.getSegment(), SEG_TYPE_SCRIPT);
 	Script *script_entity = NULL;
-	reg_t retval = make_reg(pos.getSegment(), pos.getOffset() + 1);
+	reg_t retval;
+	retval.setSegment(pos.getSegment());
+	retval.setOffset(pos.getOffset() + 1);
 	uint16 param_value = 0xffff; // Suppress GCC warning by setting default value, chose value as invalid to getKernelName etc.
 	uint i = 0;
 	Kernel *kernel = g_sci->getKernel();
diff --git a/engines/sci/engine/segment.cpp b/engines/sci/engine/segment.cpp
index fffa7f4..2b7eba3 100644
--- a/engines/sci/engine/segment.cpp
+++ b/engines/sci/engine/segment.cpp
@@ -194,7 +194,7 @@ SegmentRef DataStack::dereference(reg_t pointer) {
 
 Common::Array<reg_t> DataStack::listAllOutgoingReferences(reg_t object) const {
 	Common::Array<reg_t> tmp;
-	for (int i = 0; i < _capacity; i++)
+	for (uint i = 0; i < _capacity; i++)
 		tmp.push_back(_entries[i]);
 
 	return tmp;
diff --git a/engines/sci/engine/segment.h b/engines/sci/engine/segment.h
index f9d151c..344977c 100644
--- a/engines/sci/engine/segment.h
+++ b/engines/sci/engine/segment.h
@@ -93,7 +93,7 @@ public:
 	 * Check whether the given offset into this memory object is valid,
 	 * i.e., suitable for passing to dereference.
 	 */
-	virtual bool isValidOffset(uint16 offset) const = 0;
+	virtual bool isValidOffset(uint32 offset) const = 0;
 
 	/**
 	 * Dereferences a raw memory pointer.
@@ -149,7 +149,7 @@ struct LocalVariables : public SegmentObj {
 public:
 	LocalVariables(): SegmentObj(SEG_TYPE_LOCALS), script_id(0) { }
 
-	virtual bool isValidOffset(uint16 offset) const {
+	virtual bool isValidOffset(uint32 offset) const {
 		return offset < _locals.size() * 2;
 	}
 	virtual SegmentRef dereference(reg_t pointer);
@@ -161,7 +161,7 @@ public:
 
 /** Data stack */
 struct DataStack : SegmentObj {
-	int _capacity; /**< Number of stack entries */
+	uint _capacity; /**< Number of stack entries */
 	reg_t *_entries;
 
 public:
@@ -171,7 +171,7 @@ public:
 		_entries = NULL;
 	}
 
-	virtual bool isValidOffset(uint16 offset) const {
+	virtual bool isValidOffset(uint32 offset) const {
 		return offset < _capacity * 2;
 	}
 	virtual SegmentRef dereference(reg_t pointer);
@@ -276,7 +276,7 @@ public:
 		}
 	}
 
-	virtual bool isValidOffset(uint16 offset) const {
+	virtual bool isValidOffset(uint32 offset) const {
 		return isValidEntry(offset);
 	}
 
@@ -380,7 +380,7 @@ struct HunkTable : public SegmentObjTable<Hunk> {
 
 // Free-style memory
 struct DynMem : public SegmentObj {
-	int _size;
+	uint _size;
 	Common::String _description;
 	byte *_buf;
 
@@ -391,7 +391,7 @@ public:
 		_buf = NULL;
 	}
 
-	virtual bool isValidOffset(uint16 offset) const {
+	virtual bool isValidOffset(uint32 offset) const {
 		return offset < _size;
 	}
 	virtual SegmentRef dereference(reg_t pointer);


Commit: fc02b34215f218d2e0a0f20990654b120f52efd0
    https://github.com/scummvm/scummvm/commit/fc02b34215f218d2e0a0f20990654b120f52efd0
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI: Deduplicate Object::locateVarSelector code

The variable count returned by Object::getVarCount is populated
by variable 1 in SCI1.1, so specially reading the variable
explicitly for that engine version is not necessary.

Changed paths:
    engines/sci/engine/object.cpp


diff --git a/engines/sci/engine/object.cpp b/engines/sci/engine/object.cpp
index 177ff98..4546e28 100644
--- a/engines/sci/engine/object.cpp
+++ b/engines/sci/engine/object.cpp
@@ -116,18 +116,16 @@ const Object *Object::getClass(SegManager *segMan) const {
 
 int Object::locateVarSelector(SegManager *segMan, Selector slc) const {
 	const Common::Array<uint16> *buf;
-	uint varnum = 0;
+	const uint varCount = getVarCount();
 
 	if (getSciVersion() <= SCI_VERSION_2_1_LATE) {
 		const Object *obj = getClass(segMan);
-		varnum = getSciVersion() <= SCI_VERSION_1_LATE ? getVarCount() : obj->getVariable(1).toUint16();
 		buf = &obj->_baseVars;
 	} else if (getSciVersion() == SCI_VERSION_3) {
-		varnum = _variables.size();
 		buf = &_baseVars;
 	}
 
-	for (uint i = 0; i < varnum; i++)
+	for (uint i = 0; i < varCount; i++)
 		if ((*buf)[i] == slc) // Found it?
 			return i; // report success
 


Commit: a799cb3462a220afcd705c74291075f98520d87c
    https://github.com/scummvm/scummvm/commit/a799cb3462a220afcd705c74291075f98520d87c
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI: Fix SCI3 exports

Export functions may be relocated above 64k in SCI3, but exports
that do not have an entry in the relocation table must be handled
the same as SCI1.1-2.1.

Changed paths:
    engines/sci/engine/guest_additions.cpp
    engines/sci/engine/kscripts.cpp
    engines/sci/engine/script.cpp
    engines/sci/engine/script.h


diff --git a/engines/sci/engine/guest_additions.cpp b/engines/sci/engine/guest_additions.cpp
index 4a3afcd..6ed2cd9 100644
--- a/engines/sci/engine/guest_additions.cpp
+++ b/engines/sci/engine/guest_additions.cpp
@@ -397,7 +397,7 @@ static const byte SRTorinPatch[] = {
 };
 
 void GuestAdditions::patchGameSaveRestoreTorin(Script &script) const {
-	const uint16 address = script.validateExportFunc(2, true);
+	const uint32 address = script.validateExportFunc(2, true);
 	byte *patchPtr = const_cast<byte *>(script.getBuf(address));
 	memcpy(patchPtr, SRTorinPatch, sizeof(SRTorinPatch));
 	if (g_sci->isBE()) {
diff --git a/engines/sci/engine/kscripts.cpp b/engines/sci/engine/kscripts.cpp
index 30a1fab..98c35fc 100644
--- a/engines/sci/engine/kscripts.cpp
+++ b/engines/sci/engine/kscripts.cpp
@@ -246,7 +246,7 @@ reg_t kScriptID(EngineState *s, int argc, reg_t *argv) {
 		return NULL_REG;
 	}
 
-	uint16 address = scr->validateExportFunc(index, true);
+	uint32 address = scr->validateExportFunc(index, true);
 
 	// Point to the heap for SCI1.1 - SCI2.1 games
 	if (getSciVersion() >= SCI_VERSION_1_1 && getSciVersion() <= SCI_VERSION_2_1_LATE)
diff --git a/engines/sci/engine/script.cpp b/engines/sci/engine/script.cpp
index fa17dc0..cd75a97 100644
--- a/engines/sci/engine/script.cpp
+++ b/engines/sci/engine/script.cpp
@@ -505,6 +505,7 @@ void Script::identifyOffsets() {
 		if (_buf->size() < 22)
 			error("Script::identifyOffsets(): script %d smaller than expected SCI3-header", _nr);
 
+		_codeOffset = _buf->getUint32LEAt(0);
 		sci3StringOffset = _buf->getUint32LEAt(4);
 		sci3RelocationOffset = _buf->getUint32LEAt(8);
 
@@ -796,23 +797,29 @@ uint32 Script::validateExportFunc(int pubfunct, bool relocSci3) {
 	if (exportsAreWide)
 		pubfunct *= 2;
 
-	uint32 offset;
+	int offset;
 
 	if (getSciVersion() != SCI_VERSION_3) {
 		offset = _exports.getUint16SEAt(pubfunct);
 	} else {
-		if (!relocSci3)
-			offset = _exports.getUint16SEAt(pubfunct) + getCodeBlockOffsetSci3();
-		else
-			offset = relocateOffsetSci3(pubfunct * 2 + 22);
+		if (!relocSci3) {
+			offset = _exports.getUint16SEAt(pubfunct) + getCodeBlockOffset();
+		} else {
+			offset = relocateOffsetSci3(pubfunct * sizeof(uint16) + /* header size */ 22);
+			// Some offsets below 0xFFFF are left as-is in the export table,
+			// e.g. Lighthouse script 64990
+			if (offset == -1) {
+				offset = _exports.getUint16SEAt(pubfunct) + getCodeBlockOffset();
+			}
+		}
 	}
 
 	// TODO: Check if this should be done for SCI1.1 games as well
 	if (getSciVersion() >= SCI_VERSION_2 && offset == 0) {
-		offset = _codeOffset;
+		offset = getCodeBlockOffset();
 	}
 
-	if (offset >= _buf->size())
+	if (offset == -1 || offset >= (int)_buf->size())
 		error("Invalid export function pointer");
 
 	return offset;
diff --git a/engines/sci/engine/script.h b/engines/sci/engine/script.h
index 2be3fd0..f63b312 100644
--- a/engines/sci/engine/script.h
+++ b/engines/sci/engine/script.h
@@ -260,9 +260,10 @@ public:
 	int relocateOffsetSci3(uint32 offset) const;
 
 	/**
-	 * Gets an offset to the beginning of the code block in a SCI3 script
+	 * Gets an offset to the beginning of the code block in a SCI1.1 or later
+	 * script
 	 */
-	int getCodeBlockOffsetSci3() { return _buf->getInt32SEAt(0); }
+	int getCodeBlockOffset() { return _codeOffset; }
 
 	/**
 	 * Get the offset array


Commit: f3db412d6f3dd90893eb90a084491b90acdddc9e
    https://github.com/scummvm/scummvm/commit/f3db412d6f3dd90893eb90a084491b90acdddc9e
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Serialize Robots in SCI3

This is necessary for at least Lighthouse, which maintains the
state of Robots across save games.

Changed paths:
    engines/sci/engine/savegame.cpp
    engines/sci/engine/savegame.h
    engines/sci/graphics/video32.h
    engines/sci/video/robot_decoder.cpp
    engines/sci/video/robot_decoder.h


diff --git a/engines/sci/engine/savegame.cpp b/engines/sci/engine/savegame.cpp
index 2306fc6..4338784 100644
--- a/engines/sci/engine/savegame.cpp
+++ b/engines/sci/engine/savegame.cpp
@@ -53,6 +53,7 @@
 #include "sci/graphics/frameout.h"
 #include "sci/graphics/palette32.h"
 #include "sci/graphics/remap32.h"
+#include "sci/graphics/video32.h"
 #endif
 
 namespace Sci {
@@ -390,6 +391,12 @@ void EngineState::saveLoadWithSerializer(Common::Serializer &s) {
 			g_sci->_gfxPorts->kernelSetPicWindow(picPortRect, picPortTop, picPortLeft, false);
 	}
 
+#ifdef ENABLE_SCI32
+	if (getSciVersion() >= SCI_VERSION_2) {
+		g_sci->_video32->beforeSaveLoadWithSerializer(s);
+	}
+#endif
+
 	_segMan->saveLoadWithSerializer(s);
 
 	g_sci->_soundCmd->syncPlayList(s);
@@ -399,6 +406,7 @@ void EngineState::saveLoadWithSerializer(Common::Serializer &s) {
 		g_sci->_gfxPalette32->saveLoadWithSerializer(s);
 		g_sci->_gfxRemap32->saveLoadWithSerializer(s);
 		g_sci->_gfxCursor32->saveLoadWithSerializer(s);
+		g_sci->_video32->saveLoadWithSerializer(s);
 	} else
 #endif
 		g_sci->_gfxPalette16->saveLoadWithSerializer(s);
@@ -972,6 +980,53 @@ void GfxCursor32::saveLoadWithSerializer(Common::Serializer &s) {
 		}
 	}
 }
+
+void Video32::beforeSaveLoadWithSerializer(Common::Serializer &s) {
+	if (getSciVersion() < SCI_VERSION_3 || s.isSaving()) {
+		return;
+	}
+
+	_robotPlayer.close();
+}
+
+void Video32::saveLoadWithSerializer(Common::Serializer &s) {
+	if (getSciVersion() < SCI_VERSION_3) {
+		return;
+	}
+
+	bool robotExists = _robotPlayer.getStatus() != RobotDecoder::kRobotStatusUninitialized;
+	s.syncAsByte(robotExists);
+	if (robotExists) {
+		GuiResourceId robotId;
+		reg_t planeId;
+		Common::Point position;
+		int16 priority, scale;
+		int frameNo;
+
+		if (s.isSaving()) {
+			robotId = _robotPlayer.getResourceId();
+			planeId = _robotPlayer.getPlaneId();
+			priority = _robotPlayer.getPriority();
+			position = _robotPlayer.getPosition();
+			scale = _robotPlayer.getScale();
+			frameNo = _robotPlayer.getFrameNo();
+		}
+
+		s.syncAsUint16LE(robotId);
+		syncWithSerializer(s, planeId);
+		s.syncAsSint16LE(priority);
+		s.syncAsSint16LE(position.x);
+		s.syncAsSint16LE(position.y);
+		s.syncAsSint16LE(scale);
+		s.syncAsSint32LE(frameNo);
+
+		if (s.isLoading()) {
+			_robotPlayer.open(robotId, planeId, priority, position.x, position.y, scale);
+			_robotPlayer.showFrame(frameNo, position.x, position.y, priority);
+		}
+	}
+}
+
 #endif
 
 void GfxPorts::saveLoadWithSerializer(Common::Serializer &s) {
diff --git a/engines/sci/engine/savegame.h b/engines/sci/engine/savegame.h
index 53377f0..6284897 100644
--- a/engines/sci/engine/savegame.h
+++ b/engines/sci/engine/savegame.h
@@ -37,7 +37,7 @@ struct EngineState;
  *
  * Version - new/changed feature
  * =============================
- *      42 - initial SCI3 support
+ *      42 - SCI3 robots and VM objects
  *      41 - palette support for newer SCI2.1 games; stable SCI2/2.1 save games
  *      40 - always store palvary variables
  *      39 - Accurate SCI32 arrays/strings, score metadata, avatar metadata
diff --git a/engines/sci/graphics/video32.h b/engines/sci/graphics/video32.h
index f8d6cbe..3200105 100644
--- a/engines/sci/graphics/video32.h
+++ b/engines/sci/graphics/video32.h
@@ -642,7 +642,7 @@ private:
  * Video32 provides facilities for playing back
  * video in SCI engine.
  */
-class Video32 {
+class Video32 : public Common::Serializable {
 public:
 	Video32(SegManager *segMan, EventManager *eventMan) :
 	_SEQPlayer(segMan),
@@ -651,6 +651,9 @@ public:
 	_robotPlayer(segMan),
 	_duckPlayer(segMan, eventMan) {}
 
+	void beforeSaveLoadWithSerializer(Common::Serializer &ser);
+	virtual void saveLoadWithSerializer(Common::Serializer &ser);
+
 	SEQPlayer &getSEQPlayer() { return _SEQPlayer; }
 	AVIPlayer &getAVIPlayer() { return _AVIPlayer; }
 	VMDPlayer &getVMDPlayer() { return _VMDPlayer; }
diff --git a/engines/sci/video/robot_decoder.cpp b/engines/sci/video/robot_decoder.cpp
index ba8a468..6a4e912 100644
--- a/engines/sci/video/robot_decoder.cpp
+++ b/engines/sci/video/robot_decoder.cpp
@@ -371,6 +371,8 @@ void RobotDecoder::initStream(const GuiResourceId robotId) {
 		error("Unable to open robot file %s", fileName.c_str());
 	}
 
+	_robotId = robotId;
+
 	const uint16 id = stream->readUint16LE();
 	if (id != 0x16) {
 		error("Invalid robot file %s", fileName.c_str());
@@ -437,17 +439,16 @@ void RobotDecoder::initAudio() {
 void RobotDecoder::initVideo(const int16 x, const int16 y, const int16 scale, const reg_t plane, const bool hasPalette, const uint16 paletteSize) {
 	_position = Common::Point(x, y);
 
-	if (scale != 128) {
-		_scaleInfo.x = scale;
-		_scaleInfo.y = scale;
-		_scaleInfo.signal = kScaleSignalManual;
-	}
+	_scaleInfo.x = scale;
+	_scaleInfo.y = scale;
+	_scaleInfo.signal = scale == 128 ? kScaleSignalNone : kScaleSignalManual;
 
 	_plane = g_sci->_gfxFrameout->getPlanes().findByObject(plane);
 	if (_plane == nullptr) {
 		error("Invalid plane %04x:%04x passed to RobotDecoder::open", PRINT_REG(plane));
 	}
 
+	_planeId = plane;
 	_minFrameRate = _frameRate - kMaxFrameRateDrift;
 	_maxFrameRate = _frameRate + kMaxFrameRateDrift;
 
@@ -585,6 +586,8 @@ void RobotDecoder::close() {
 
 	debugC(kDebugLevelVideo, "Closing robot");
 
+	_robotId = -1;
+	_planeId = NULL_REG;
 	_status = kRobotStatusUninitialized;
 	_videoSizes.clear();
 	_recordPositions.clear();
@@ -1343,6 +1346,10 @@ void RobotDecoder::expandCel(byte* target, const byte* source, const int16 celWi
 	}
 }
 
+int16 RobotDecoder::getPriority() const {
+	return _priority;
+}
+
 void RobotDecoder::setPriority(const int16 newPriority) {
 	_priority = newPriority;
 }
diff --git a/engines/sci/video/robot_decoder.h b/engines/sci/video/robot_decoder.h
index 9d8c720..aa1d9da 100644
--- a/engines/sci/video/robot_decoder.h
+++ b/engines/sci/video/robot_decoder.h
@@ -469,23 +469,25 @@ public:
 #pragma mark RobotDecoder
 
 /**
- * RobotDecoder implements the logic required
- * for Robot animations.
- *
- * @note A paused or finished RobotDecoder was
- * classified as serializable in SCI3, but the
- * save/load code would attempt to use uninitialised
- * values, so it seems that robots were not ever
- * actually able to be saved.
+ * RobotDecoder implements the logic required for Robot animations.
  */
 class RobotDecoder {
 public:
 	RobotDecoder(SegManager *segMan);
 	~RobotDecoder();
 
+	GuiResourceId getResourceId() const {
+		return _robotId;
+	}
+
 private:
 	SegManager *_segMan;
 
+	/**
+	 * The ID of the currently loaded robot.
+	 */
+	GuiResourceId _robotId;
+
 #pragma mark Constants
 public:
 	/**
@@ -1175,6 +1177,27 @@ private:
 #pragma mark Rendering
 public:
 	/**
+	 * Gets the plane used to render the robot.
+	 */
+	const reg_t getPlaneId() const {
+		return _planeId;
+	}
+
+	/**
+	 * Gets the origin of the robot.
+	 */
+	Common::Point getPosition() const {
+		return _position;
+	}
+
+	/**
+	 * Gets the scale of the robot.
+	 */
+	int16 getScale() const {
+		return _scaleInfo.x;
+	}
+
+	/**
 	 * Puts the current dimensions of the robot, in game script
 	 * coordinates, into the given rect, and returns the total
 	 * number of frames in the robot animation.
@@ -1207,6 +1230,8 @@ public:
 	 */
 	void expandCel(byte *target, const byte* source, const int16 celWidth, const int16 celHeight) const;
 
+	int16 getPriority() const;
+
 	/**
 	 * Sets the visual priority of the robot.
 	 * @see Plane::_priority
@@ -1292,6 +1317,11 @@ private:
 	DecompressorLZS _decompressor;
 
 	/**
+	 * The ID of the robot plane.
+	 */
+	reg_t _planeId;
+
+	/**
 	 * The origin of the robot animation, in screen
 	 * coordinates.
 	 */


Commit: 36cb241b8e41f2268315a847011cd143dd16d47a
    https://github.com/scummvm/scummvm/commit/36cb241b8e41f2268315a847011cd143dd16d47a
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Remove unused code branch

Changed paths:
    engines/sci/video/robot_decoder.cpp


diff --git a/engines/sci/video/robot_decoder.cpp b/engines/sci/video/robot_decoder.cpp
index 6a4e912..4d62b51 100644
--- a/engines/sci/video/robot_decoder.cpp
+++ b/engines/sci/video/robot_decoder.cpp
@@ -401,12 +401,6 @@ void RobotDecoder::initAudio() {
 
 	_audioRecordInterval = RobotAudioStream::kRobotSampleRate / _frameRate;
 
-	// TODO: Might actually be for all games newer than Lighthouse; check to
-	// see which games have this condition.
-	if (g_sci->getGameId() != GID_LIGHTHOUSE && !(_audioRecordInterval & 1)) {
-		++_audioRecordInterval;
-	}
-
 	_expectedAudioBlockSize = _audioBlockSize - kAudioBlockHeaderSize;
 	_audioBuffer = (byte *)realloc(_audioBuffer, kRobotZeroCompressSize + _expectedAudioBlockSize);
 


Commit: eb9965274d18d6bc23c976fe7e7b72747001fb8e
    https://github.com/scummvm/scummvm/commit/eb9965274d18d6bc23c976fe7e7b72747001fb8e
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Fix race conditions in Audio32

Changed paths:
    engines/sci/engine/ksound.cpp
    engines/sci/sound/audio32.cpp
    engines/sci/sound/audio32.h
    engines/sci/sound/music.cpp
    engines/sci/sound/soundcmd.cpp


diff --git a/engines/sci/engine/ksound.cpp b/engines/sci/engine/ksound.cpp
index e7cad21..ad0d836 100644
--- a/engines/sci/engine/ksound.cpp
+++ b/engines/sci/engine/ksound.cpp
@@ -356,23 +356,19 @@ reg_t kDoAudioPlay(EngineState *s, int argc, reg_t *argv) {
 }
 
 reg_t kDoAudioStop(EngineState *s, int argc, reg_t *argv) {
-	const int16 channelIndex = g_sci->_audio32->findChannelByArgs(argc, argv, 0, argc > 1 ? argv[1] : NULL_REG);
-	return make_reg(0, g_sci->_audio32->stop(channelIndex));
+	return g_sci->_audio32->kernelStop(argc, argv);
 }
 
 reg_t kDoAudioPause(EngineState *s, int argc, reg_t *argv) {
-	const int16 channelIndex = g_sci->_audio32->findChannelByArgs(argc, argv, 0, argc > 1 ? argv[1] : NULL_REG);
-	return make_reg(0, g_sci->_audio32->pause(channelIndex));
+	return g_sci->_audio32->kernelPause(argc, argv);
 }
 
 reg_t kDoAudioResume(EngineState *s, int argc, reg_t *argv) {
-	const int16 channelIndex = g_sci->_audio32->findChannelByArgs(argc, argv, 0, argc > 1 ? argv[1] : NULL_REG);
-	return make_reg(0, g_sci->_audio32->resume(channelIndex));
+	return g_sci->_audio32->kernelResume(argc, argv);
 }
 
 reg_t kDoAudioPosition(EngineState *s, int argc, reg_t *argv) {
-	const int16 channelIndex = g_sci->_audio32->findChannelByArgs(argc, argv, 0, argc > 1 ? argv[1] : NULL_REG);
-	return make_reg(0, g_sci->_audio32->getPosition(channelIndex));
+	return g_sci->_audio32->kernelPosition(argc, argv);
 }
 
 reg_t kDoAudioRate(EngineState *s, int argc, reg_t *argv) {
@@ -391,14 +387,7 @@ reg_t kDoAudioRate(EngineState *s, int argc, reg_t *argv) {
 }
 
 reg_t kDoAudioVolume(EngineState *s, int argc, reg_t *argv) {
-	const int16 volume = argc > 0 ? argv[0].toSint16() : -1;
-	const int16 channelIndex = g_sci->_audio32->findChannelByArgs(argc, argv, 1, argc > 2 ? argv[2] : NULL_REG);
-
-	if (volume != -1) {
-		g_sci->_audio32->setVolume(channelIndex, volume);
-	}
-
-	return make_reg(0, g_sci->_audio32->getVolume(channelIndex));
+	return g_sci->_audio32->kernelVolume(argc, argv);
 }
 
 reg_t kDoAudioGetCapability(EngineState *s, int argc, reg_t *argv) {
@@ -421,11 +410,7 @@ reg_t kDoAudioBitDepth(EngineState *s, int argc, reg_t *argv) {
 }
 
 reg_t kDoAudioMixing(EngineState *s, int argc, reg_t *argv) {
-	if (argc > 0) {
-		g_sci->_audio32->setAttenuatedMixing(argv[0].toUint16());
-	}
-
-	return make_reg(0, g_sci->_audio32->getAttenuatedMixing());
+	return g_sci->_audio32->kernelMixing(argc, argv);
 }
 
 reg_t kDoAudioChannels(EngineState *s, int argc, reg_t *argv) {
@@ -457,21 +442,7 @@ reg_t kDoAudioPreload(EngineState *s, int argc, reg_t *argv) {
 }
 
 reg_t kDoAudioFade(EngineState *s, int argc, reg_t *argv) {
-	if (argc < 4) {
-		return make_reg(0, 0);
-	}
-
-	// NOTE: Sierra did a nightmarish hack here, temporarily replacing
-	// the argc of the kernel arguments with 2 and then restoring it
-	// after findChannelByArgs was called.
-	const int16 channelIndex = g_sci->_audio32->findChannelByArgs(2, argv, 0, argc > 5 ? argv[5] : NULL_REG);
-
-	const int16 volume = argv[1].toSint16();
-	const int16 speed = argv[2].toSint16();
-	const int16 steps = argv[3].toSint16();
-	const bool stopAfterFade = argc > 4 ? (bool)argv[4].toUint16() : false;
-
-	return make_reg(0, g_sci->_audio32->fadeChannel(channelIndex, volume, speed, steps, stopAfterFade));
+	return g_sci->_audio32->kernelFade(argc, argv);
 }
 
 reg_t kDoAudioHasSignal(EngineState *s, int argc, reg_t *argv) {
@@ -479,11 +450,7 @@ reg_t kDoAudioHasSignal(EngineState *s, int argc, reg_t *argv) {
 }
 
 reg_t kDoAudioSetLoop(EngineState *s, int argc, reg_t *argv) {
-	const int16 channelIndex = g_sci->_audio32->findChannelByArgs(argc, argv, 0, argc == 3 ? argv[2] : NULL_REG);
-
-	const bool loop = argv[0].toSint16() != 0 && argv[0].toSint16() != 1;
-
-	g_sci->_audio32->setLoop(channelIndex, loop);
+	g_sci->_audio32->kernelLoop(argc, argv);
 	return s->r_acc;
 }
 
diff --git a/engines/sci/sound/audio32.cpp b/engines/sci/sound/audio32.cpp
index 5d008bf..df21a5f 100644
--- a/engines/sci/sound/audio32.cpp
+++ b/engines/sci/sound/audio32.cpp
@@ -875,6 +875,12 @@ int16 Audio32::stop(const int16 channelIndex) {
 	return oldNumChannels;
 }
 
+uint16 Audio32::restart(const ResourceId resourceId, const bool autoPlay, const bool loop, const int16 volume, const reg_t soundNode, const bool monitor) {
+	Common::StackLock lock(_mutex);
+	stop(resourceId, soundNode);
+	return play(kNoExistingChannel, resourceId, autoPlay, loop, volume, soundNode, monitor);
+}
+
 int16 Audio32::getPosition(const int16 channelIndex) const {
 	Common::StackLock lock(_mutex);
 	if (channelIndex == kNoExistingChannel || _numActiveChannels == 0) {
@@ -920,82 +926,6 @@ void Audio32::setLoop(const int16 channelIndex, const bool loop) {
 	channel.loop = loop;
 }
 
-reg_t Audio32::kernelPlay(const bool autoPlay, const int argc, const reg_t *const argv) {
-	if (argc == 0) {
-		return make_reg(0, _numActiveChannels);
-	}
-
-	const int16 channelIndex = findChannelByArgs(argc, argv, 0, NULL_REG);
-	ResourceId resourceId;
-	bool loop;
-	int16 volume;
-	bool monitor = false;
-	reg_t soundNode = NULL_REG;
-
-	if (argc >= 5) {
-		resourceId = ResourceId(kResourceTypeAudio36, argv[0].toUint16(), argv[1].toUint16(), argv[2].toUint16(), argv[3].toUint16(), argv[4].toUint16());
-
-		if (argc < 6 || argv[5].toSint16() == 1) {
-			loop = false;
-		} else {
-			// NOTE: Uses -1 for infinite loop. Presumably the
-			// engine was supposed to allow counter loops at one
-			// point, but ended up only using loop as a boolean.
-			loop = (bool)argv[5].toSint16();
-		}
-
-		if (getSciVersion() == SCI_VERSION_3) {
-			if (argc < 7) {
-				volume = Audio32::kMaxVolume;
-			} else {
-				volume = argv[6].toSint16() & Audio32::kMaxVolume;
-				monitor = argv[6].toSint16() & Audio32::kMonitorAudioFlagSci3;
-			}
-		} else {
-			if (argc < 7 || argv[6].toSint16() < 0 || argv[6].toSint16() > Audio32::kMaxVolume) {
-				volume = Audio32::kMaxVolume;
-
-				if (argc >= 7) {
-					monitor = true;
-				}
-			} else {
-				volume = argv[6].toSint16();
-			}
-		}
-	} else {
-		resourceId = ResourceId(kResourceTypeAudio, argv[0].toUint16());
-
-		if (argc < 2 || argv[1].toSint16() == 1) {
-			loop = false;
-		} else {
-			loop = (bool)argv[1].toSint16();
-		}
-
-		if (getSciVersion() == SCI_VERSION_3) {
-			if (argc < 3) {
-				volume = Audio32::kMaxVolume;
-			} else {
-				volume = argv[2].toSint16() & Audio32::kMaxVolume;
-				monitor = argv[2].toSint16() & Audio32::kMonitorAudioFlagSci3;
-			}
-		} else {
-			if (argc < 3 || argv[2].toSint16() < 0 || argv[2].toSint16() > Audio32::kMaxVolume) {
-				volume = Audio32::kMaxVolume;
-
-				if (argc >= 3) {
-					monitor = true;
-				}
-			} else {
-				volume = argv[2].toSint16();
-			}
-		}
-
-		soundNode = argc == 4 ? argv[3] : NULL_REG;
-	}
-
-	return make_reg(0, play(channelIndex, resourceId, autoPlay, loop, volume, soundNode, monitor));
-}
-
 #pragma mark -
 #pragma mark Effects
 
@@ -1100,4 +1030,159 @@ bool Audio32::hasSignal() const {
 	return false;
 }
 
+#pragma mark -
+#pragma mark Kernel
+
+reg_t Audio32::kernelPlay(const bool autoPlay, const int argc, const reg_t *const argv) {
+	Common::StackLock lock(_mutex);
+
+	if (argc == 0) {
+		return make_reg(0, _numActiveChannels);
+	}
+
+	const int16 channelIndex = findChannelByArgs(argc, argv, 0, NULL_REG);
+	ResourceId resourceId;
+	bool loop;
+	int16 volume;
+	bool monitor = false;
+	reg_t soundNode = NULL_REG;
+
+	if (argc >= 5) {
+		resourceId = ResourceId(kResourceTypeAudio36, argv[0].toUint16(), argv[1].toUint16(), argv[2].toUint16(), argv[3].toUint16(), argv[4].toUint16());
+
+		if (argc < 6 || argv[5].toSint16() == 1) {
+			loop = false;
+		} else {
+			// NOTE: Uses -1 for infinite loop. Presumably the
+			// engine was supposed to allow counter loops at one
+			// point, but ended up only using loop as a boolean.
+			loop = (bool)argv[5].toSint16();
+		}
+
+		if (getSciVersion() == SCI_VERSION_3) {
+			if (argc < 7) {
+				volume = Audio32::kMaxVolume;
+			} else {
+				volume = argv[6].toSint16() & Audio32::kMaxVolume;
+				monitor = argv[6].toSint16() & Audio32::kMonitorAudioFlagSci3;
+			}
+		} else {
+			if (argc < 7 || argv[6].toSint16() < 0 || argv[6].toSint16() > Audio32::kMaxVolume) {
+				volume = Audio32::kMaxVolume;
+
+				if (argc >= 7) {
+					monitor = true;
+				}
+			} else {
+				volume = argv[6].toSint16();
+			}
+		}
+	} else {
+		resourceId = ResourceId(kResourceTypeAudio, argv[0].toUint16());
+
+		if (argc < 2 || argv[1].toSint16() == 1) {
+			loop = false;
+		} else {
+			loop = (bool)argv[1].toSint16();
+		}
+
+		if (getSciVersion() == SCI_VERSION_3) {
+			if (argc < 3) {
+				volume = Audio32::kMaxVolume;
+			} else {
+				volume = argv[2].toSint16() & Audio32::kMaxVolume;
+				monitor = argv[2].toSint16() & Audio32::kMonitorAudioFlagSci3;
+			}
+		} else {
+			if (argc < 3 || argv[2].toSint16() < 0 || argv[2].toSint16() > Audio32::kMaxVolume) {
+				volume = Audio32::kMaxVolume;
+
+				if (argc >= 3) {
+					monitor = true;
+				}
+			} else {
+				volume = argv[2].toSint16();
+			}
+		}
+
+		soundNode = argc == 4 ? argv[3] : NULL_REG;
+	}
+
+	return make_reg(0, play(channelIndex, resourceId, autoPlay, loop, volume, soundNode, monitor));
+}
+
+reg_t Audio32::kernelStop(const int argc, const reg_t *const argv) {
+	Common::StackLock lock(_mutex);
+	const int16 channelIndex = findChannelByArgs(argc, argv, 0, argc > 1 ? argv[1] : NULL_REG);
+	return make_reg(0, stop(channelIndex));
+}
+
+reg_t Audio32::kernelPause(const int argc, const reg_t *const argv) {
+	Common::StackLock lock(_mutex);
+	const int16 channelIndex = findChannelByArgs(argc, argv, 0, argc > 1 ? argv[1] : NULL_REG);
+	return make_reg(0, pause(channelIndex));
+}
+
+reg_t Audio32::kernelResume(const int argc, const reg_t *const argv) {
+	Common::StackLock lock(_mutex);
+	const int16 channelIndex = findChannelByArgs(argc, argv, 0, argc > 1 ? argv[1] : NULL_REG);
+	return make_reg(0, resume(channelIndex));
+}
+
+reg_t Audio32::kernelPosition(const int argc, const reg_t *const argv) {
+	Common::StackLock lock(_mutex);
+	const int16 channelIndex = findChannelByArgs(argc, argv, 0, argc > 1 ? argv[1] : NULL_REG);
+	return make_reg(0, getPosition(channelIndex));
+}
+
+reg_t Audio32::kernelVolume(const int argc, const reg_t *const argv) {
+	Common::StackLock lock(_mutex);
+
+	const int16 volume = argc > 0 ? argv[0].toSint16() : -1;
+	const int16 channelIndex = findChannelByArgs(argc, argv, 1, argc > 2 ? argv[2] : NULL_REG);
+
+	if (volume != -1) {
+		setVolume(channelIndex, volume);
+	}
+
+	return make_reg(0, getVolume(channelIndex));
+}
+
+reg_t Audio32::kernelMixing(const int argc, const reg_t *const argv) {
+	Common::StackLock lock(_mutex);
+
+	if (argc > 0) {
+		setAttenuatedMixing(argv[0].toUint16());
+	}
+
+	return make_reg(0, getAttenuatedMixing());
+}
+
+reg_t Audio32::kernelFade(const int argc, const reg_t *const argv) {
+	if (argc < 4) {
+		return make_reg(0, 0);
+	}
+
+	Common::StackLock lock(_mutex);
+
+	// NOTE: In SSCI, this call to find the channel is hacked up; argc is
+	// set to 2 before the call, and then restored after the call.
+	const int16 channelIndex = findChannelByArgs(2, argv, 0, argc > 5 ? argv[5] : NULL_REG);
+	const int16 volume = argv[1].toSint16();
+	const int16 speed = argv[2].toSint16();
+	const int16 steps = argv[3].toSint16();
+	const bool stopAfterFade = argc > 4 ? (bool)argv[4].toUint16() : false;
+
+	return make_reg(0, fadeChannel(channelIndex, volume, speed, steps, stopAfterFade));
+}
+
+void Audio32::kernelLoop(const int argc, const reg_t *const argv) {
+	Common::StackLock lock(_mutex);
+
+	const int16 channelIndex = findChannelByArgs(argc, argv, 0, argc == 3 ? argv[2] : NULL_REG);
+	const bool loop = argv[0].toSint16() != 0 && argv[0].toSint16() != 1;
+
+	setLoop(channelIndex, loop);
+}
+
 } // End of namespace Sci
diff --git a/engines/sci/sound/audio32.h b/engines/sci/sound/audio32.h
index f631de2..c953582 100644
--- a/engines/sci/sound/audio32.h
+++ b/engines/sci/sound/audio32.h
@@ -451,6 +451,11 @@ public:
 	}
 
 	/**
+	 * Restarts playback of the given audio resource.
+	 */
+	uint16 restart(const ResourceId resourceId, const bool autoPlay, const bool loop, const int16 volume, const reg_t soundNode, const bool monitor);
+
+	/**
 	 * Returns the playback position for the given channel
 	 * number, in ticks.
 	 */
@@ -469,8 +474,6 @@ public:
 		setLoop(findChannelById(resourceId, soundNode), loop);
 	}
 
-	reg_t kernelPlay(const bool autoPlay, const int argc, const reg_t *const argv);
-
 private:
 	/**
 	 * The tick when audio was globally paused.
@@ -594,6 +597,19 @@ private:
 	 * monitoring buffer.
 	 */
 	int _numMonitoredSamples;
+
+#pragma mark -
+#pragma mark Kernel
+public:
+	reg_t kernelPlay(const bool autoPlay, const int argc, const reg_t *const argv);
+	reg_t kernelStop(const int argc, const reg_t *const argv);
+	reg_t kernelPause(const int argc, const reg_t *const argv);
+	reg_t kernelResume(const int argc, const reg_t *const argv);
+	reg_t kernelPosition(const int argc, const reg_t *const argv);
+	reg_t kernelVolume(const int argc, const reg_t *const argv);
+	reg_t kernelMixing(const int argc, const reg_t *const argv);
+	reg_t kernelFade(const int argc, const reg_t *const argv);
+	void kernelLoop(const int argc, const reg_t *const argv);
 };
 
 } // End of namespace Sci
diff --git a/engines/sci/sound/music.cpp b/engines/sci/sound/music.cpp
index dd9fbcb..ca5644d 100644
--- a/engines/sci/sound/music.cpp
+++ b/engines/sci/sound/music.cpp
@@ -484,10 +484,7 @@ void SciMusic::soundPlay(MusicEntry *pSnd) {
 	if (pSnd->isSample) {
 #ifdef ENABLE_SCI32
 		if (_soundVersion >= SCI_VERSION_2_1_EARLY) {
-			g_sci->_audio32->stop(ResourceId(kResourceTypeAudio, pSnd->resourceId), pSnd->soundObj);
-
-			g_sci->_audio32->play(kNoExistingChannel, ResourceId(kResourceTypeAudio, pSnd->resourceId), true, pSnd->loop != 0 && pSnd->loop != 1, pSnd->volume, pSnd->soundObj, false);
-
+			g_sci->_audio32->restart(ResourceId(kResourceTypeAudio, pSnd->resourceId), true, pSnd->loop != 0 && pSnd->loop != 1, pSnd->volume, pSnd->soundObj, false);
 			return;
 		} else
 #endif
diff --git a/engines/sci/sound/soundcmd.cpp b/engines/sci/sound/soundcmd.cpp
index e0cbb97..07e01fb 100644
--- a/engines/sci/sound/soundcmd.cpp
+++ b/engines/sci/sound/soundcmd.cpp
@@ -344,12 +344,10 @@ reg_t SoundCommandParser::kDoSoundPause(EngineState *s, int argc, reg_t *argv) {
 		// implementation is so different that it doesn't
 		// matter here
 		if (_soundVersion >= SCI_VERSION_2_1_EARLY && musicSlot->isSample) {
-			const int16 channelIndex = g_sci->_audio32->findChannelById(ResourceId(kResourceTypeAudio, musicSlot->resourceId), musicSlot->soundObj);
-
 			if (shouldPause) {
-				g_sci->_audio32->pause(channelIndex);
+				g_sci->_audio32->pause(ResourceId(kResourceTypeAudio, musicSlot->resourceId), musicSlot->soundObj);
 			} else {
-				g_sci->_audio32->resume(channelIndex);
+				g_sci->_audio32->resume(ResourceId(kResourceTypeAudio, musicSlot->resourceId), musicSlot->soundObj);
 			}
 		} else
 #endif
@@ -504,7 +502,7 @@ void SoundCommandParser::processUpdateCues(reg_t obj) {
 	if (musicSlot->isSample) {
 #ifdef ENABLE_SCI32
 		if (_soundVersion >= SCI_VERSION_2_1_EARLY) {
-			const int position = g_sci->_audio32->getPosition(g_sci->_audio32->findChannelById(ResourceId(kResourceTypeAudio, musicSlot->resourceId), musicSlot->soundObj));
+			const int position = g_sci->_audio32->getPosition(ResourceId(kResourceTypeAudio, musicSlot->resourceId), musicSlot->soundObj);
 
 			if (position == -1) {
 				processStopSound(musicSlot->soundObj, true);


Commit: 8bdfb7889572b680add712d461ebd868329e42b9
    https://github.com/scummvm/scummvm/commit/8bdfb7889572b680add712d461ebd868329e42b9
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Add debugger command to list digital audio samples

Changed paths:
    engines/sci/console.cpp
    engines/sci/console.h
    engines/sci/sound/audio32.cpp
    engines/sci/sound/audio32.h


diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index b8e6097..d67bd69 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -169,6 +169,7 @@ Console::Console(SciEngine *engine) : GUI::Debugger(),
 	registerCmd("sfx01_track",		WRAP_METHOD(Console, cmdSfx01Track));
 	registerCmd("show_instruments",	WRAP_METHOD(Console, cmdShowInstruments));
 	registerCmd("map_instrument",		WRAP_METHOD(Console, cmdMapInstrument));
+	registerCmd("audio_list",		WRAP_METHOD(Console, cmdAudioList));
 	// Script
 	registerCmd("addresses",			WRAP_METHOD(Console, cmdAddresses));
 	registerCmd("registers",			WRAP_METHOD(Console, cmdRegisters));
@@ -418,6 +419,7 @@ bool Console::cmdHelp(int argc, const char **argv) {
 	debugPrintf(" sfx01_track - Dumps a track of a SCI01 song\n");
 	debugPrintf(" show_instruments - Shows the instruments of a specific song, or all songs\n");
 	debugPrintf(" map_instrument - Dynamically maps an MT-32 instrument to a GM instrument\n");
+	debugPrintf(" audio_list - Lists currently active digital audio samples (SCI2.1+)\n");
 	debugPrintf("\n");
 	debugPrintf("Script:\n");
 	debugPrintf(" addresses - Provides information on how to pass addresses\n");
@@ -1318,6 +1320,21 @@ bool Console::cmdMapInstrument(int argc, const char **argv) {
 	return true;
 }
 
+bool Console::cmdAudioList(int argc, const char **argv) {
+#ifdef ENABLE_SCI32
+	if (_engine->_audio32) {
+		debugPrintf("Audio list (%d active channels):\n", _engine->_audio32->getNumActiveChannels());
+		_engine->_audio32->printAudioList(this);
+	} else {
+		debugPrintf("This SCI version does not have a software digital audio mixer\n");
+	}
+#else
+	debugPrintf("SCI32 isn't included in this compiled executable\n");
+#endif
+
+	return true;
+}
+
 bool Console::cmdSaveGame(int argc, const char **argv) {
 	if (argc != 2) {
 		debugPrintf("Saves the current game state to the hard disk\n");
diff --git a/engines/sci/console.h b/engines/sci/console.h
index 366f959..1bb86b9 100644
--- a/engines/sci/console.h
+++ b/engines/sci/console.h
@@ -127,6 +127,7 @@ private:
 	bool cmdSfx01Track(int argc, const char **argv);
 	bool cmdShowInstruments(int argc, const char **argv);
 	bool cmdMapInstrument(int argc, const char **argv);
+	bool cmdAudioList(int argc, const char **argv);
 	// Script
 	bool cmdAddresses(int argc, const char **argv);
 	bool cmdRegisters(int argc, const char **argv);
diff --git a/engines/sci/sound/audio32.cpp b/engines/sci/sound/audio32.cpp
index df21a5f..b61bbd9 100644
--- a/engines/sci/sound/audio32.cpp
+++ b/engines/sci/sound/audio32.cpp
@@ -35,6 +35,7 @@
 #include "common/textconsole.h"     // for warning
 #include "common/types.h"           // for Flag::NO
 #include "engine.h"                 // for Engine, g_engine
+#include "sci/console.h"            // for Console
 #include "sci/engine/features.h"    // for GameFeatures
 #include "sci/engine/guest_additions.h" // for GuestAdditions
 #include "sci/engine/state.h"       // for EngineState
@@ -1185,4 +1186,34 @@ void Audio32::kernelLoop(const int argc, const reg_t *const argv) {
 	setLoop(channelIndex, loop);
 }
 
+#pragma mark -
+#pragma mark Debugging
+
+void Audio32::printAudioList(Console *con) const {
+	Common::StackLock lock(_mutex);
+	for (int i = 0; i < _numActiveChannels; ++i) {
+		const AudioChannel &channel = _channels[i];
+		con->debugPrintf("  %d[%04x:%04x]: %s, started at %d, pos %d/%d, vol %d, pan %d%s%s\n",
+						 i,
+						 PRINT_REG(channel.soundNode),
+						 channel.robot ? "robot" : channel.resource->name().c_str(),
+						 channel.startedAtTick,
+						 (g_sci->getTickCount() - channel.startedAtTick) % channel.duration,
+						 channel.duration,
+						 channel.volume,
+						 channel.pan,
+						 channel.loop ? ", looping" : "",
+						 channel.pausedAtTick ? ", paused" : "");
+		if (channel.fadeStartTick) {
+			con->debugPrintf("                fade: vol %d -> %d, started at %d, pos %d/%d%s\n",
+							 channel.fadeStartVolume,
+							 channel.fadeTargetVolume,
+							 channel.fadeStartTick,
+							 (g_sci->getTickCount() - channel.fadeStartTick) % channel.duration,
+							 channel.fadeDuration,
+							 channel.stopChannelOnFade ? ", stopping" : "");
+		}
+	}
+}
+
 } // End of namespace Sci
diff --git a/engines/sci/sound/audio32.h b/engines/sci/sound/audio32.h
index c953582..f77f676 100644
--- a/engines/sci/sound/audio32.h
+++ b/engines/sci/sound/audio32.h
@@ -33,6 +33,8 @@
 #include "sci/video/robot_decoder.h" // for RobotAudioStream
 
 namespace Sci {
+class Console;
+
 #pragma mark AudioChannel
 
 /**
@@ -610,6 +612,11 @@ public:
 	reg_t kernelMixing(const int argc, const reg_t *const argv);
 	reg_t kernelFade(const int argc, const reg_t *const argv);
 	void kernelLoop(const int argc, const reg_t *const argv);
+
+#pragma mark -
+#pragma mark Debugging
+public:
+	void printAudioList(Console *con) const;
 };
 
 } // End of namespace Sci


Commit: 1962b1bb6d495365110094f200d50cc4cb303b75
    https://github.com/scummvm/scummvm/commit/1962b1bb6d495365110094f200d50cc4cb303b75
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Replace magic numbers in SCI3 selector init

Changed paths:
    engines/sci/engine/object.cpp


diff --git a/engines/sci/engine/object.cpp b/engines/sci/engine/object.cpp
index 4546e28..edd721a 100644
--- a/engines/sci/engine/object.cpp
+++ b/engines/sci/engine/object.cpp
@@ -269,8 +269,6 @@ bool Object::initBaseObject(SegManager *segMan, reg_t addr, bool doInitSuperClas
 	return false;
 }
 
-const int EXTRA_GROUPS = 3;
-
 #ifdef ENABLE_SCI32
 bool Object::mustSetViewVisible(const int index) const {
 	if (getSciVersion() == SCI_VERSION_3) {
@@ -294,11 +292,16 @@ bool Object::mustSetViewVisible(const int index) const {
 #endif
 
 void Object::initSelectorsSci3(const SciSpan<const byte> &buf, const bool initVariables) {
+	enum {
+		kExtraGroups = 3,
+		kGroupSize   = 32
+	};
+
 	const SciSpan<const byte> groupInfo = _baseObj.subspan(16);
-	const SciSpan<const byte> selectorBase = groupInfo.subspan(EXTRA_GROUPS * 32 * 2);
+	const SciSpan<const byte> selectorBase = groupInfo.subspan(kExtraGroups * kGroupSize * sizeof(uint16));
 
-	int numGroups = g_sci->getKernel()->getSelectorNamesSize() / 32;
-	if (g_sci->getKernel()->getSelectorNamesSize() % 32)
+	int numGroups = g_sci->getKernel()->getSelectorNamesSize() / kGroupSize;
+	if (g_sci->getKernel()->getSelectorNamesSize() % kGroupSize)
 		++numGroups;
 
 	_mustSetViewVisible.resize(numGroups);
@@ -313,7 +316,7 @@ void Object::initSelectorsSci3(const SciSpan<const byte> &buf, const bool initVa
 	// there are, so we count them first.
 	for (int groupNr = 0; groupNr < numGroups; ++groupNr) {
 		byte groupLocation = groupInfo[groupNr];
-		const SciSpan<const byte> seeker = selectorBase.subspan(groupLocation * 32 * 2);
+		const SciSpan<const byte> seeker = selectorBase.subspan(groupLocation * kGroupSize * sizeof(uint16));
 
 		if (groupLocation != 0)	{
 			// This object actually has selectors belonging to this group
@@ -321,8 +324,8 @@ void Object::initSelectorsSci3(const SciSpan<const byte> &buf, const bool initVa
 
 			_mustSetViewVisible[groupNr] = (typeMask & 1);
 
-			for (int bit = 2; bit < 32; ++bit) {
-				int value = seeker.getUint16SEAt(bit * 2);
+			for (int bit = 2; bit < kGroupSize; ++bit) {
+				int value = seeker.getUint16SEAt(bit * sizeof(uint16));
 				if (typeMask & (1 << bit)) { // Property
 					++numProperties;
 				} else if (value != 0xffff) { // Method
@@ -345,21 +348,21 @@ void Object::initSelectorsSci3(const SciSpan<const byte> &buf, const bool initVa
 	int propertyCounter = 0;
 	for (int groupNr = 0; groupNr < numGroups; ++groupNr) {
 		byte groupLocation = groupInfo[groupNr];
-		const SciSpan<const byte> seeker = selectorBase.subspan(groupLocation * 32 * 2);
+		const SciSpan<const byte> seeker = selectorBase.subspan(groupLocation * kGroupSize * sizeof(uint16));
 
 		if (groupLocation != 0)	{
 			// This object actually has selectors belonging to this group
 			int typeMask = seeker.getUint32SEAt(0);
-			int groupBaseId = groupNr * 32;
+			int groupBaseId = groupNr * kGroupSize;
 
-			for (int bit = 2; bit < 32; ++bit) {
-				int value = seeker.getUint16SEAt(bit * 2);
+			for (int bit = 2; bit < kGroupSize; ++bit) {
+				int value = seeker.getUint16SEAt(bit * sizeof(uint16));
 				if (typeMask & (1 << bit)) { // Property
 					_baseVars[propertyCounter] = groupBaseId + bit;
 					if (initVariables) {
 						_variables[propertyCounter] = make_reg(0, value);
 					}
-					uint32 propertyOffset = (seeker + bit * 2) - buf;
+					uint32 propertyOffset = (seeker + bit * sizeof(uint16)) - buf;
 					_propertyOffsetsSci3[propertyCounter] = propertyOffset;
 					++propertyCounter;
 				} else if (value != 0xffff) { // Method


Commit: 3d4fb4ccb4f8bed013e6be9f9c677a071e84266d
    https://github.com/scummvm/scummvm/commit/3d4fb4ccb4f8bed013e6be9f9c677a071e84266d
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Fix mustSetViewVisible for SCI3

In SCI2/2.1, variable indexes are used along with a range encoded
in the interpreter executable to determine whether an object
variable is a view-related variable. Operands to aTop, sTop, ipToa,
dpToa, ipTos, and dpTos are byte offsets into an object, which
are divided by two to get the varindex to check against the
interpreter range.

In SCI3, objects in game scripts contain groups of 32 selectors,
and each group has a flag that says whether or not the selectors
in that group are view-related. Operands to aTop, sTop, ipToa,
dpToa, ipTos, and dpTos are selectors.

Changed paths:
    engines/sci/engine/object.cpp
    engines/sci/engine/object.h
    engines/sci/engine/selector.cpp
    engines/sci/engine/selector.h
    engines/sci/engine/vm.cpp


diff --git a/engines/sci/engine/object.cpp b/engines/sci/engine/object.cpp
index edd721a..9861e02 100644
--- a/engines/sci/engine/object.cpp
+++ b/engines/sci/engine/object.cpp
@@ -270,13 +270,29 @@ bool Object::initBaseObject(SegManager *segMan, reg_t addr, bool doInitSuperClas
 }
 
 #ifdef ENABLE_SCI32
-bool Object::mustSetViewVisible(const int index) const {
+bool Object::mustSetViewVisible(int index, const bool fromPropertyOp) const {
 	if (getSciVersion() == SCI_VERSION_3) {
-		if ((uint)index < getVarCount()) {
-			return _mustSetViewVisible[getVarSelector(index) >> 5];
+		// In SCI3, visible flag lookups are based on selectors
+
+		if (!fromPropertyOp) {
+			// varindexes must be converted to selectors
+			index = getVarSelector(index);
 		}
-		return false;
+
+		if (index == -1) {
+			error("Selector %d is invalid for object %04x:%04x", index, PRINT_REG(_pos));
+		}
+
+		return _mustSetViewVisible[index >> 5];
 	} else {
+		// In SCI2, visible flag lookups are based on varindexes
+
+		if (fromPropertyOp) {
+			// property offsets must be converted to varindexes
+			assert((index % 2) == 0);
+			index >>= 1;
+		}
+
 		int minIndex, maxIndex;
 		if (g_sci->_features->usesAlternateSelectors()) {
 			minIndex = 24;
diff --git a/engines/sci/engine/object.h b/engines/sci/engine/object.h
index 992b6cd..3b81497 100644
--- a/engines/sci/engine/object.h
+++ b/engines/sci/engine/object.h
@@ -277,7 +277,7 @@ public:
 	void syncBaseObject(const SciSpan<const byte> &ptr) { _baseObj = ptr; }
 
 #ifdef ENABLE_SCI32
-	bool mustSetViewVisible(int selector) const;
+	bool mustSetViewVisible(const int index, const bool fromPropertyOp) const;
 #endif
 
 private:
diff --git a/engines/sci/engine/selector.cpp b/engines/sci/engine/selector.cpp
index 7f509f3..e28ae79 100644
--- a/engines/sci/engine/selector.cpp
+++ b/engines/sci/engine/selector.cpp
@@ -230,8 +230,8 @@ reg_t readSelector(SegManager *segMan, reg_t object, Selector selectorId) {
 }
 
 #ifdef ENABLE_SCI32
-void updateInfoFlagViewVisible(Object *obj, int index) {
-	if (getSciVersion() >= SCI_VERSION_2 && obj->mustSetViewVisible(index)) {
+void updateInfoFlagViewVisible(Object *obj, int index, bool fromPropertyOp) {
+	if (getSciVersion() >= SCI_VERSION_2 && obj->mustSetViewVisible(index, fromPropertyOp)) {
 		obj->setInfoSelectorFlag(kInfoFlagViewVisible);
 	}
 }
diff --git a/engines/sci/engine/selector.h b/engines/sci/engine/selector.h
index f2e4ec0..c6ecd9d 100644
--- a/engines/sci/engine/selector.h
+++ b/engines/sci/engine/selector.h
@@ -218,7 +218,7 @@ void invokeSelector(EngineState *s, reg_t object, int selectorId,
  * This function checks if index is in the right range, and sets the flag
  * on obj.-info- if it is.
  */
-void updateInfoFlagViewVisible(Object *obj, int index);
+void updateInfoFlagViewVisible(Object *obj, int index, bool fromPropertyOp = false);
 #endif
 
 } // End of namespace Sci
diff --git a/engines/sci/engine/vm.cpp b/engines/sci/engine/vm.cpp
index ad6234b..1df3c60 100644
--- a/engines/sci/engine/vm.cpp
+++ b/engines/sci/engine/vm.cpp
@@ -1148,7 +1148,7 @@ void run_vm(EngineState *s) {
 			// Accumulator To Property
 			validate_property(s, obj, opparams[0]) = s->r_acc;
 #ifdef ENABLE_SCI32
-			updateInfoFlagViewVisible(obj, opparams[0]>>1);
+			updateInfoFlagViewVisible(obj, opparams[0], true);
 #endif
 			break;
 
@@ -1161,7 +1161,7 @@ void run_vm(EngineState *s) {
 			// Stack To Property
 			validate_property(s, obj, opparams[0]) = POP32();
 #ifdef ENABLE_SCI32
-			updateInfoFlagViewVisible(obj, opparams[0]>>1);
+			updateInfoFlagViewVisible(obj, opparams[0], true);
 #endif
 			break;
 
@@ -1178,7 +1178,7 @@ void run_vm(EngineState *s) {
 			else
 				opProperty -= 1;
 #ifdef ENABLE_SCI32
-			updateInfoFlagViewVisible(obj, opparams[0]>>1);
+			updateInfoFlagViewVisible(obj, opparams[0], true);
 #endif
 			if (opcode == op_ipToa || opcode == op_dpToa)
 				s->r_acc = opProperty;


Commit: a2d7851e4ddb7c3858b5ee89b95daccf3dfa5531
    https://github.com/scummvm/scummvm/commit/a2d7851e4ddb7c3858b5ee89b95daccf3dfa5531
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Improve disassembly output of SCI3 property opcodes

Since SCI3 scripts use selectors instead of offsets as operands
to property-related opcodes, the disassembler can look up and
display property names everywhere (unlike SCI2.1 and earlier,
which need to know the object being operated on to look up the
correct selector for a given offset).

Changed paths:
    engines/sci/engine/scriptdebug.cpp


diff --git a/engines/sci/engine/scriptdebug.cpp b/engines/sci/engine/scriptdebug.cpp
index 7d186eb..09b38d1 100644
--- a/engines/sci/engine/scriptdebug.cpp
+++ b/engines/sci/engine/scriptdebug.cpp
@@ -196,6 +196,21 @@ reg_t disassemble(EngineState *s, reg32_t pos, reg_t objAddr, bool printBWTag, b
 				}
 
 				debugN(",");
+#ifdef ENABLE_SCI32
+			} else if (getSciVersion() == SCI_VERSION_3 && (
+				opcode == op_pToa || opcode == op_aTop ||
+				opcode == op_pTos || opcode == op_sTop ||
+				opcode == op_ipToa || opcode == op_dpToa ||
+				opcode == op_ipTos || opcode == op_dpTos)) {
+
+				const char *selectorName = "<invalid>";
+
+				if (param_value < kernel->getSelectorNamesSize()) {
+					selectorName = kernel->getSelectorName(param_value).c_str();
+				}
+
+				debugN("\t%s[%x]", selectorName, param_value);
+#endif
 			} else {
 				const char *separator = defaultSeparator;
 


Commit: 6f95b1a440c8579a5010c39ca84e2c5fcdfac682
    https://github.com/scummvm/scummvm/commit/6f95b1a440c8579a5010c39ca84e2c5fcdfac682
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Fix missing mustSetViewVisible data in cloned objects

This information comes directly from script data and is not
modified at runtime, so it does not need to be persisted in save
games, but does need to be set when reconstructing clones.

Changed paths:
    engines/sci/engine/object.h
    engines/sci/engine/savegame.cpp
    engines/sci/engine/savegame.h


diff --git a/engines/sci/engine/object.h b/engines/sci/engine/object.h
index 3b81497..75ab128 100644
--- a/engines/sci/engine/object.h
+++ b/engines/sci/engine/object.h
@@ -264,6 +264,11 @@ public:
 		_baseObj = obj ? obj->_baseObj : SciSpan<const byte>();
 		_baseMethod = obj ? obj->_baseMethod : Common::Array<uint32>();
 		_baseVars = obj ? obj->_baseVars : Common::Array<uint16>();
+#ifdef ENABLE_SCI32
+		if (getSciVersion() == SCI_VERSION_3) {
+			_mustSetViewVisible = obj ? obj->_mustSetViewVisible : Common::Array<bool>();
+		}
+#endif
 	}
 
 	bool relocateSci0Sci21(SegmentId segment, int location, size_t scriptSize);
diff --git a/engines/sci/engine/savegame.cpp b/engines/sci/engine/savegame.cpp
index 4338784..427e32d 100644
--- a/engines/sci/engine/savegame.cpp
+++ b/engines/sci/engine/savegame.cpp
@@ -104,10 +104,6 @@ void syncWithSerializer(Common::Serializer &s, Node &obj) {
 	syncWithSerializer(s, obj.value);
 }
 
-void syncWithSerializer(Common::Serializer &s, bool &obj) {
-	s.syncAsByte(obj);
-}
-
 #pragma mark -
 
 // By default, sync using syncWithSerializer, which in turn can easily be overloaded.
@@ -427,12 +423,20 @@ void Object::saveLoadWithSerializer(Common::Serializer &s) {
 	s.syncAsSint32LE(_methodCount);		// that's actually a uint16
 
 	syncArray<reg_t>(s, _variables);
+
+#ifdef ENABLE_SCI32
 	if (s.getVersion() >= 42 && getSciVersion() == SCI_VERSION_3) {
-		syncArray<bool>(s, _mustSetViewVisible);
+		// Obsolete mustSetViewVisible array
+		if (s.getVersion() == 42 && s.isLoading()) {
+			uint32 len;
+			s.syncAsUint32LE(len);
+			s.skip(len);
+		}
 		syncWithSerializer(s, _superClassPosSci3);
 		syncWithSerializer(s, _speciesSelectorSci3);
 		syncWithSerializer(s, _infoSelectorSci3);
 	}
+#endif
 }
 
 
diff --git a/engines/sci/engine/savegame.h b/engines/sci/engine/savegame.h
index 6284897..151585a 100644
--- a/engines/sci/engine/savegame.h
+++ b/engines/sci/engine/savegame.h
@@ -37,6 +37,7 @@ struct EngineState;
  *
  * Version - new/changed feature
  * =============================
+ *      43 - stop saving SCI3 mustSetViewVisible array
  *      42 - SCI3 robots and VM objects
  *      41 - palette support for newer SCI2.1 games; stable SCI2/2.1 save games
  *      40 - always store palvary variables
@@ -67,7 +68,7 @@ struct EngineState;
  */
 
 enum {
-	CURRENT_SAVEGAME_VERSION = 42,
+	CURRENT_SAVEGAME_VERSION = 43,
 	MINIMUM_SAVEGAME_VERSION = 14
 #ifdef ENABLE_SCI32
 	,


Commit: d53f3f6095975405558c2902a870ff3fa8c01a8b
    https://github.com/scummvm/scummvm/commit/d53f3f6095975405558c2902a870ff3fa8c01a8b
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI32: Exclude SCI3 code from compilation when SCI32 is disabled

Changed paths:
    engines/sci/engine/object.cpp
    engines/sci/engine/object.h
    engines/sci/engine/script.cpp
    engines/sci/engine/script.h
    engines/sci/engine/vm.cpp


diff --git a/engines/sci/engine/object.cpp b/engines/sci/engine/object.cpp
index 9861e02..2875c8b 100644
--- a/engines/sci/engine/object.cpp
+++ b/engines/sci/engine/object.cpp
@@ -96,16 +96,22 @@ void Object::init(const SciSpan<const byte> &buf, reg_t obj_pos, bool initVariab
 		for (int i = 0; i < _methodCount * 2 + 3; ++i) {
 			_baseMethod.push_back(buf.getUint16SEAt(data.getUint16SEAt(6) + i * 2));
 		}
+#ifdef ENABLE_SCI32
 	} else if (getSciVersion() == SCI_VERSION_3) {
 		initSelectorsSci3(buf, initVariables);
+#endif
 	}
 
 	if (initVariables) {
-		if (getSciVersion() <= SCI_VERSION_2_1_LATE) {
+#ifdef ENABLE_SCI32
+		if (getSciVersion() == SCI_VERSION_3) {
+			_infoSelectorSci3 = make_reg(0, data.getUint16SEAt(10));
+		} else {
+#else
+		{
+#endif
 			for (uint i = 0; i < _variables.size(); i++)
 				_variables[i] = make_reg(0, data.getUint16SEAt(i * 2));
-		} else {
-			_infoSelectorSci3 = make_reg(0, data.getUint16SEAt(10));
 		}
 	}
 }
@@ -136,6 +142,7 @@ bool Object::relocateSci0Sci21(SegmentId segment, int location, size_t scriptSiz
 	return relocateBlock(_variables, getPos().getOffset(), segment, location, scriptSize);
 }
 
+#ifdef ENABLE_SCI32
 bool Object::relocateSci3(SegmentId segment, uint32 location, int offset, size_t scriptSize) {
 	assert(_propertyOffsetsSci3.size());
 	assert(offset >= 0 && (uint)offset < scriptSize);
@@ -150,6 +157,7 @@ bool Object::relocateSci3(SegmentId segment, uint32 location, int offset, size_t
 
 	return false;
 }
+#endif
 
 int Object::propertyOffsetToId(SegManager *segMan, int propertyOffset) const {
 	int selectors = getVarCount();
@@ -305,7 +313,6 @@ bool Object::mustSetViewVisible(int index, const bool fromPropertyOp) const {
 		return index >= minIndex && index <= maxIndex;
 	}
 }
-#endif
 
 void Object::initSelectorsSci3(const SciSpan<const byte> &buf, const bool initVariables) {
 	enum {
@@ -398,5 +405,6 @@ void Object::initSelectorsSci3(const SciSpan<const byte> &buf, const bool initVa
 		_superClassPosSci3 = make_reg(0, _baseObj.getUint16SEAt(8));
 	}
 }
+#endif
 
 } // End of namespace Sci
diff --git a/engines/sci/engine/object.h b/engines/sci/engine/object.h
index 75ab128..136a831 100644
--- a/engines/sci/engine/object.h
+++ b/engines/sci/engine/object.h
@@ -73,8 +73,12 @@ public:
 		_isFreed(false),
 		_baseObj(),
 		_baseVars(),
-		_methodCount(0),
-		_propertyOffsetsSci3() {}
+		_methodCount(0)
+#ifdef ENABLE_SCI32
+		,
+		_propertyOffsetsSci3()
+#endif
+		{}
 
 	Object &operator=(const Object &other) {
 		_baseObj = other._baseObj;
@@ -86,6 +90,7 @@ public:
 		_pos = other._pos;
 		_baseVars = other._baseVars;
 
+#ifdef ENABLE_SCI32
 		if (getSciVersion() == SCI_VERSION_3) {
 			_propertyOffsetsSci3 = other._propertyOffsetsSci3;
 			_superClassPosSci3 = other._superClassPosSci3;
@@ -93,67 +98,80 @@ public:
 			_infoSelectorSci3 = other._infoSelectorSci3;
 			_mustSetViewVisible = other._mustSetViewVisible;
 		}
+#endif
 
 		return *this;
 	}
 
 	reg_t getSpeciesSelector() const {
-		if (getSciVersion() < SCI_VERSION_3)
-			return _variables[_offset];
-		else	// SCI3
+#ifdef ENABLE_SCI32
+		if (getSciVersion() == SCI_VERSION_3)
 			return _speciesSelectorSci3;
+		else
+#endif
+			return _variables[_offset];
 	}
 
 	void setSpeciesSelector(reg_t value) {
-		if (getSciVersion() < SCI_VERSION_3)
-			_variables[_offset] = value;
-		else	// SCI3
+#ifdef ENABLE_SCI32
+		if (getSciVersion() == SCI_VERSION_3)
 			_speciesSelectorSci3 = value;
+		else
+#endif
+			_variables[_offset] = value;
 	}
 
 	reg_t getSuperClassSelector() const {
-		if (getSciVersion() < SCI_VERSION_3)
-			return _variables[_offset + 1];
-		else	// SCI3
+#ifdef ENABLE_SCI32
+		if (getSciVersion() == SCI_VERSION_3)
 			return _superClassPosSci3;
+		else
+#endif
+			return _variables[_offset + 1];
 	}
 
 	void setSuperClassSelector(reg_t value) {
-		if (getSciVersion() < SCI_VERSION_3)
-			_variables[_offset + 1] = value;
-		else	// SCI3
+#ifdef ENABLE_SCI32
+		if (getSciVersion() == SCI_VERSION_3)
 			_superClassPosSci3 = value;
+		else
+#endif
+			_variables[_offset + 1] = value;
 	}
 
 	reg_t getInfoSelector() const {
-		if (getSciVersion() < SCI_VERSION_3)
-			return _variables[_offset + 2];
-		else	// SCI3
+#ifdef ENABLE_SCI32
+		if (getSciVersion() == SCI_VERSION_3)
 			return _infoSelectorSci3;
+		else
+#endif
+			return _variables[_offset + 2];
 	}
 
 	void setInfoSelector(reg_t info) {
-		if (getSciVersion() < SCI_VERSION_3)
-			_variables[_offset + 2] = info;
-		else	// SCI3
+#ifdef ENABLE_SCI32
+		if (getSciVersion() == SCI_VERSION_3)
 			_infoSelectorSci3 = info;
+		else
+#endif
+			_variables[_offset + 2] = info;
 	}
 
 #ifdef ENABLE_SCI32
 	void setInfoSelectorFlag(infoSelectorFlags flag) {
-		if (getSciVersion() < SCI_VERSION_3) {
-			_variables[_offset + 2] |= flag;
-		} else {
+		if (getSciVersion() == SCI_VERSION_3) {
 			_infoSelectorSci3 |= flag;
+		} else {
+			_variables[_offset + 2] |= flag;
 		}
 	}
 
 	// NOTE: In real engine, -info- is treated as byte size
 	void clearInfoSelectorFlag(infoSelectorFlags flag) {
-		if (getSciVersion() < SCI_VERSION_3) {
-			_variables[_offset + 2] &= ~flag;
-		} else {
+		if (getSciVersion() == SCI_VERSION_3) {
 			_infoSelectorSci3 &= ~flag;
+		} else {
+			_variables[_offset + 2] &= ~flag;
 		}
 	}
 
@@ -163,43 +181,53 @@ public:
 #endif
 
 	reg_t getNameSelector() const {
-		if (getSciVersion() < SCI_VERSION_3)
-			return _offset + 3 < (uint16)_variables.size() ? _variables[_offset + 3] : NULL_REG;
-		else	// SCI3
+#ifdef ENABLE_SCI32
+		if (getSciVersion() == SCI_VERSION_3)
 			return _variables.size() ? _variables[0] : NULL_REG;
+		else
+#endif
+			return _offset + 3 < (uint16)_variables.size() ? _variables[_offset + 3] : NULL_REG;
 	}
 
 	// No setter for the name selector
 
 	reg_t getPropDictSelector() const {
-		if (getSciVersion() < SCI_VERSION_3)
-			return _variables[2];
-		else
+#ifdef ENABLE_SCI32
+		if (getSciVersion() == SCI_VERSION_3)
 			// This should never occur, this is called from a SCI1.1 - SCI2.1 only function
 			error("getPropDictSelector called for SCI3");
+		else
+#endif
+			return _variables[2];
 	}
 
 	void setPropDictSelector(reg_t value) {
-		if (getSciVersion() < SCI_VERSION_3)
-			_variables[2] = value;
-		else
+#ifdef ENABLE_SCI32
+		if (getSciVersion() == SCI_VERSION_3)
 			// This should never occur, this is called from a SCI1.1 - SCI2.1 only function
 			error("setPropDictSelector called for SCI3");
+		else
+#endif
+			_variables[2] = value;
 	}
 
 	reg_t getClassScriptSelector() const {
-		if (getSciVersion() < SCI_VERSION_3)
-			return _variables[4];
-		else	// SCI3
+#ifdef ENABLE_SCI32
+		if (getSciVersion() == SCI_VERSION_3)
 			return make_reg(0, _baseObj.getUint16SEAt(6));
+		else
+#endif
+			return _variables[4];
 	}
 
 	void setClassScriptSelector(reg_t value) {
-		if (getSciVersion() < SCI_VERSION_3)
-			_variables[4] = value;
-		else	// SCI3
+#ifdef ENABLE_SCI32
+		if (getSciVersion() == SCI_VERSION_3)
 			// This should never occur, this is called from a SCI1.1 - SCI2.1 only function
 			error("setClassScriptSelector called for SCI3");
+		else
+#endif
+			_variables[4] = value;
 	}
 
 	Selector getVarSelector(uint16 i) const { return _baseVars[i]; }
@@ -272,7 +300,9 @@ public:
 	}
 
 	bool relocateSci0Sci21(SegmentId segment, int location, size_t scriptSize);
+#ifdef ENABLE_SCI32
 	bool relocateSci3(SegmentId segment, uint32 location, int offset, size_t scriptSize);
+#endif
 
 	int propertyOffsetToId(SegManager *segMan, int propertyOffset) const;
 
@@ -286,7 +316,9 @@ public:
 #endif
 
 private:
+#ifdef ENABLE_SCI32
 	void initSelectorsSci3(const SciSpan<const byte> &buf, const bool initVariables);
+#endif
 
 	/**
 	 * A pointer to the raw object data within the object's owner script.
@@ -310,6 +342,7 @@ private:
 	 */
 	Common::Array<reg_t> _variables;
 
+#ifdef ENABLE_SCI32
 	/**
 	 * A lookup table from a property index to the property's original absolute
 	 * offset within the raw script data. This absolute offset is coded into the
@@ -318,6 +351,7 @@ private:
 	 * barrier, since the script format still only holds 16-bit values inline).
 	 */
 	Common::Array<uint32> _propertyOffsetsSci3;
+#endif
 
 	/**
 	 * The number of methods on the object.
@@ -336,10 +370,12 @@ private:
 	uint16 _offset;
 
 	reg_t _pos; /**< Object offset within its script; for clones, this is their base */
+#ifdef ENABLE_SCI32
 	reg_t _superClassPosSci3; /**< reg_t pointing to superclass for SCI3 */
 	reg_t _speciesSelectorSci3;	/**< reg_t containing species "selector" for SCI3 */
 	reg_t _infoSelectorSci3; /**< reg_t containing info "selector" for SCI3 */
 	Common::Array<bool> _mustSetViewVisible; /** cached bit of info to make lookup fast, SCI3 only */
+#endif
 };
 
 
diff --git a/engines/sci/engine/script.cpp b/engines/sci/engine/script.cpp
index cd75a97..0a2d7e5 100644
--- a/engines/sci/engine/script.cpp
+++ b/engines/sci/engine/script.cpp
@@ -496,6 +496,7 @@ void Script::identifyOffsets() {
 			_offsetLookupStringCount++;
 		}
 
+#ifdef ENABLE_SCI32
 	} else if (getSciVersion() == SCI_VERSION_3) {
 		// SCI3
 		uint32 sci3StringOffset = 0;
@@ -607,9 +608,11 @@ void Script::identifyOffsets() {
 				}
 			}
 		}
+#endif
 	}
 }
 
+#ifdef ENABLE_SCI32
 SciSpan<const byte> Script::getSci3ObjectsPointer() {
 	SciSpan<const byte> ptr;
 
@@ -627,6 +630,7 @@ SciSpan<const byte> Script::getSci3ObjectsPointer() {
 
 	return ptr;
 }
+#endif
 
 Object *Script::getObject(uint32 offset) {
 	if (_objects.contains(offset))
@@ -681,6 +685,7 @@ static bool relocateBlock(Common::Array<reg_t> &block, int block_location, Segme
 	return true;
 }
 
+#ifdef ENABLE_SCI32
 int Script::relocateOffsetSci3(uint32 offset) const {
 	int relocStart = _buf->getUint32LEAt(8);
 	int relocCount = _buf->getUint16LEAt(18);
@@ -695,6 +700,7 @@ int Script::relocateOffsetSci3(uint32 offset) const {
 
 	return -1;
 }
+#endif
 
 bool Script::relocateLocal(SegmentId segment, int location) {
 	if (_localsBlock)
@@ -750,6 +756,7 @@ void Script::relocateSci0Sci21(reg_t block) {
 	}
 }
 
+#ifdef ENABLE_SCI32
 void Script::relocateSci3(reg_t block) {
 	SciSpan<const byte> relocStart = _buf->subspan(_buf->getUint32SEAt(8));
 	const uint relocCount = _buf->getUint16SEAt(18);
@@ -766,6 +773,7 @@ void Script::relocateSci3(reg_t block) {
 		}
 	}
 }
+#endif
 
 void Script::incrementLockers() {
 	assert(!_markedAsDeleted);
@@ -799,9 +807,8 @@ uint32 Script::validateExportFunc(int pubfunct, bool relocSci3) {
 
 	int offset;
 
-	if (getSciVersion() != SCI_VERSION_3) {
-		offset = _exports.getUint16SEAt(pubfunct);
-	} else {
+#ifdef ENABLE_SCI32
+	if (getSciVersion() == SCI_VERSION_3) {
 		if (!relocSci3) {
 			offset = _exports.getUint16SEAt(pubfunct) + getCodeBlockOffset();
 		} else {
@@ -812,7 +819,9 @@ uint32 Script::validateExportFunc(int pubfunct, bool relocSci3) {
 				offset = _exports.getUint16SEAt(pubfunct) + getCodeBlockOffset();
 			}
 		}
-	}
+	} else
+#endif
+		offset = _exports.getUint16SEAt(pubfunct);
 
 	// TODO: Check if this should be done for SCI1.1 games as well
 	if (getSciVersion() >= SCI_VERSION_2 && offset == 0) {
@@ -934,9 +943,11 @@ void Script::initializeClasses(SegManager *segMan) {
 	} else if (getSciVersion() >= SCI_VERSION_1_1 && getSciVersion() <= SCI_VERSION_2_1_LATE) {
 		seeker = _heap.subspan(4 + _heap.getUint16SEAt(2) * 2);
 		mult = 2;
+#ifdef ENABLE_SCI32
 	} else if (getSciVersion() == SCI_VERSION_3) {
 		seeker = getSci3ObjectsPointer();
 		mult = 1;
+#endif
 	}
 
 	if (!seeker)
@@ -1092,6 +1103,7 @@ void Script::initializeObjectsSci11(SegManager *segMan, SegmentId segmentId) {
 	relocateSci0Sci21(make_reg(segmentId, _heap.getUint16SEAt(0)));
 }
 
+#ifdef ENABLE_SCI32
 void Script::initializeObjectsSci3(SegManager *segMan, SegmentId segmentId) {
 	SciSpan<const byte> seeker = getSci3ObjectsPointer();
 
@@ -1110,14 +1122,17 @@ void Script::initializeObjectsSci3(SegManager *segMan, SegmentId segmentId) {
 
 	relocateSci3(make_reg(segmentId, 0));
 }
+#endif
 
 void Script::initializeObjects(SegManager *segMan, SegmentId segmentId) {
 	if (getSciVersion() <= SCI_VERSION_1_LATE)
 		initializeObjectsSci0(segMan, segmentId);
 	else if (getSciVersion() >= SCI_VERSION_1_1 && getSciVersion() <= SCI_VERSION_2_1_LATE)
 		initializeObjectsSci11(segMan, segmentId);
+#ifdef ENABLE_SCI32
 	else if (getSciVersion() == SCI_VERSION_3)
 		initializeObjectsSci3(segMan, segmentId);
+#endif
 }
 
 reg_t Script::findCanonicAddress(SegManager *segMan, reg_t addr) const {
diff --git a/engines/sci/engine/script.h b/engines/sci/engine/script.h
index f63b312..e0faa11 100644
--- a/engines/sci/engine/script.h
+++ b/engines/sci/engine/script.h
@@ -253,11 +253,13 @@ public:
 	 */
 	void syncStringHeap(Common::Serializer &ser);
 
+#ifdef ENABLE_SCI32
 	/**
 	 * Resolve a relocation in an SCI3 script
 	 * @param offset        The offset to relocate from
 	 */
 	int relocateOffsetSci3(uint32 offset) const;
+#endif
 
 	/**
 	 * Gets an offset to the beginning of the code block in a SCI1.1 or later
@@ -282,6 +284,7 @@ private:
 	 */
 	void relocateSci0Sci21(reg_t block);
 
+#ifdef ENABLE_SCI32
 	/**
 	 * Processes a relocation block within a SCI3 script
 	 *  This function is idempotent, but it must only be called after all
@@ -289,13 +292,16 @@ private:
 	 * @param obj_pos	Location (segment, offset) of the block
 	 */
 	void relocateSci3(reg_t block);
+#endif
 
 	bool relocateLocal(SegmentId segment, int location);
 
+#ifdef ENABLE_SCI32
 	/**
 	 * Gets a pointer to the beginning of the objects in a SCI3 script
 	 */
 	SciSpan<const byte> getSci3ObjectsPointer();
+#endif
 
 	/**
 	 * Initializes the script's objects (SCI0)
@@ -311,12 +317,14 @@ private:
 	 */
 	void initializeObjectsSci11(SegManager *segMan, SegmentId segmentId);
 
+#ifdef ENABLE_SCI32
 	/**
 	 * Initializes the script's objects (SCI3)
 	 * @param segMan	A reference to the segment manager
 	 * @param segmentId	The script's segment id
 	 */
 	void initializeObjectsSci3(SegManager *segMan, SegmentId segmentId);
+#endif
 
 	LocalVariables *allocLocalsSegment(SegManager *segMan);
 
diff --git a/engines/sci/engine/vm.cpp b/engines/sci/engine/vm.cpp
index 1df3c60..1d86948 100644
--- a/engines/sci/engine/vm.cpp
+++ b/engines/sci/engine/vm.cpp
@@ -572,11 +572,13 @@ uint32 findOffset(const int16 relOffset, const Script *scr, const uint32 pcOffse
 	case SCI_VERSION_1_1:
 		offset = relOffset + scr->getScriptSize();
 		break;
+#ifdef ENABLE_SCI32
 	case SCI_VERSION_3:
 		// In theory this can break if the variant with a one-byte argument is
 		// used. For now, assume it doesn't happen.
 		offset = scr->relocateOffsetSci3(pcOffset - 2);
 		break;
+#endif
 	default:
 		error("Unknown lofs type");
 	}


Commit: 28a06656af8e67219055cc8bbee57fa49dc2de61
    https://github.com/scummvm/scummvm/commit/28a06656af8e67219055cc8bbee57fa49dc2de61
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-04-23T13:07:25-05:00

Commit Message:
SCI: Improve error messages in Script::validateExportFunc

Changed paths:
    engines/sci/engine/script.cpp


diff --git a/engines/sci/engine/script.cpp b/engines/sci/engine/script.cpp
index 0a2d7e5..d35cb5b 100644
--- a/engines/sci/engine/script.cpp
+++ b/engines/sci/engine/script.cpp
@@ -798,8 +798,7 @@ uint32 Script::validateExportFunc(int pubfunct, bool relocSci3) {
 	bool exportsAreWide = (g_sci->_features->detectLofsType() == SCI_VERSION_1_MIDDLE);
 
 	if (_numExports <= (uint)pubfunct) {
-		error("validateExportFunc(): pubfunct is invalid");
-		return 0;
+		error("script.%d validateExportFunc(): pubfunct %d is invalid", _nr, pubfunct);
 	}
 
 	if (exportsAreWide)
@@ -829,7 +828,7 @@ uint32 Script::validateExportFunc(int pubfunct, bool relocSci3) {
 	}
 
 	if (offset == -1 || offset >= (int)_buf->size())
-		error("Invalid export function pointer");
+		error("Invalid export %d function pointer (%d) in script.%d", pubfunct, offset, _nr);
 
 	return offset;
 }





More information about the Scummvm-git-logs mailing list