[Scummvm-git-logs] scummvm master -> 1af7fe8b9616c5ade0af00b7720db0b4967471ff

[csnover]

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

Summary:
11ee0f90ac SCI: Warn more loudly about uninitialised parameter reads
774713564d SCI32: Add script patch for Shivers room 35170
1af7fe8b96 SCI32: Remove no-longer-necessary Phant1 VMD sync hack


Commit: 11ee0f90ac67a3ee0a36506c35e35af6a11f7936
    https://github.com/scummvm/scummvm/commit/11ee0f90ac67a3ee0a36506c35e35af6a11f7936
Author: Colin Snover (github.com at zetafleet.com)
Date: 2016-11-19T19:05:38-06:00

Commit Message:
SCI: Warn more loudly about uninitialised parameter reads

Silently returning zero values can cause games to break. e.g.
Shivers 1 room 35170 has a script bug where vJoystick::handleEvent
makes a super call which causes doVerb to be called a second time
with no arguments. In the original game this happened to work
because the value already on the stack happened to be 1. In ScummVM
this silently (unless VM debug messages were enabled) failed
because the uninitialised read value was forced to 0.

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



diff --git a/engines/sci/engine/vm.cpp b/engines/sci/engine/vm.cpp
index a6d37d0..ac15aeb 100644
--- a/engines/sci/engine/vm.cpp
+++ b/engines/sci/engine/vm.cpp
@@ -142,11 +142,13 @@ static reg_t read_var(EngineState *s, int type, int index) {
 				s->variables[type][index] = make_reg(0, solution.value);
 				break;
 			}
-			case VAR_PARAM:
+			case VAR_PARAM: {
 				// Out-of-bounds read for a parameter that goes onto stack and hits an uninitialized temp
 				//  We return 0 currently in that case
-				debugC(kDebugLevelVM, "[VM] Read for a parameter goes out-of-bounds, onto the stack and gets uninitialized temp");
+				const SciCallOrigin origin = s->getCurrentCallOrigin();
+				warning("Uninitialized read for parameter %d from %s", index, origin.toString().c_str());
 				return NULL_REG;
+			}
 			default:
 				break;
 			}


Commit: 774713564d212b42804b896cb2a03d3e5e248384
    https://github.com/scummvm/scummvm/commit/774713564d212b42804b896cb2a03d3e5e248384
Author: Colin Snover (github.com at zetafleet.com)
Date: 2016-11-19T19:06:04-06:00

Commit Message:
SCI32: Add script patch for Shivers room 35170

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 5f3370b..dbc351d 100644
--- a/engines/sci/engine/script_patches.cpp
+++ b/engines/sci/engine/script_patches.cpp
@@ -4553,6 +4553,40 @@ static const SciScriptPatcherEntry sq5Signatures[] = {
 
 #ifdef ENABLE_SCI32
 #pragma mark -
+#pragma mark Shivers
+
+// In room 35170, there is a CCTV control station with a joystick that must be
+// clicked and dragged to pan the camera. In order to enable dragging, on
+// mousedown, the vJoystick::handleEvent method calls vJoystick::doVerb(1),
+// which enables the drag functionality of the joystick. However,
+// vJoystick::handleEvent then makes a super call to ShiversProp::handleEvent,
+// which calls vJoystick::doVerb(). This second call, which fails to pass an
+// argument, causes an uninitialized read off the stack for the first parameter.
+// In SSCI, this happens to work because the uninitialized value on the stack
+// happens to be 1. Disabling the super call avoids the bad doVerb call without
+// any apparent ill effect.
+static const uint16 shiversSignatureJoystickFix[] = {
+	SIG_MAGICDWORD,
+	0x38, SIG_UINT16(0xa5),    // pushi handleEvent
+	0x78,                      // push1
+	0x8f, 0x01,                // lsp 1
+	0x59, 0x02,                // &rest 2
+	0x57, 0x7f, SIG_UINT16(6), // super ShiversProp[7f], 6
+	SIG_END
+};
+
+static const uint16 shiversPatchJoystickFix[] = {
+	0x48,                      // ret
+	PATCH_END
+};
+
+//          script, description,                                      signature                        patch
+static const SciScriptPatcherEntry shiversSignatures[] = {
+	{  true, 35170, "fix CCTV joystick interaction",               1, shiversSignatureJoystickFix,     shiversPatchJoystickFix },
+	SCI_SIGNATUREENTRY_TERMINATOR
+};
+
+#pragma mark -
 #pragma mark Space Quest 6
 
 // After the explosion in the Quarters of Deepship 86, the game tries to perform
@@ -5119,6 +5153,9 @@ void ScriptPatcher::processScript(uint16 scriptNr, byte *scriptData, const uint3
 	case GID_QFG4:
 		signatureTable = qfg4Signatures;
 		break;
+	case GID_SHIVERS:
+		signatureTable = shiversSignatures;
+		break;
 #endif
 	case GID_SQ1:
 		signatureTable = sq1vgaSignatures;


Commit: 1af7fe8b9616c5ade0af00b7720db0b4967471ff
    https://github.com/scummvm/scummvm/commit/1af7fe8b9616c5ade0af00b7720db0b4967471ff
Author: Colin Snover (github.com at zetafleet.com)
Date: 2016-11-19T19:06:04-06:00

Commit Message:
SCI32: Remove no-longer-necessary Phant1 VMD sync hack

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



diff --git a/engines/sci/graphics/video32.cpp b/engines/sci/graphics/video32.cpp
index bf0c990..6eae4cc 100644
--- a/engines/sci/graphics/video32.cpp
+++ b/engines/sci/graphics/video32.cpp
@@ -737,14 +737,6 @@ VMDPlayer::EventFlags VMDPlayer::playUntilEvent(const EventFlags flags) {
 
 		g_sci->_gfxFrameout->addScreenItem(*_screenItem);
 
-		// HACK: When VMD playback is allowed to yield back to the VM, this
-		// causes the VMD decoder to freak out for some reason and video output
-		// starts jittering horribly. This problem does not happen if audio sync
-		// is disabled, but this causes some audible clicking between frames
-		// of audio (and, presumably, will cause some AV sync problems). Still,
-		// that's better than really bad jitter.
-		_decoder->setAudioSync(!(flags & kEventFlagYieldToVM));
-
 		_decoder->start();
 	}