[Scummvm-git-logs] scummvm master -> 08b9c26752174a247d5aa06da7b498c818def1a7

[sluicebox]

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

Summary:
08b9c26752 SCI: Fix kSetSignalLoop handling in early SCI0 and KQ4


Commit: 08b9c26752174a247d5aa06da7b498c818def1a7
    https://github.com/scummvm/scummvm/commit/08b9c26752174a247d5aa06da7b498c818def1a7
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2023-07-12T08:15:20-07:00

Commit Message:
SCI: Fix kSetSignalLoop handling in early SCI0 and KQ4

Fixes KQ4 introduction not automatically ending

Thanks to @athrxx for locating the correct behavior in disassembly
Thanks to @eriktorbjorn for reporting this

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 7e29c4a5949..6c3495b5a81 100644
--- a/engines/sci/sound/midiparser_sci.cpp
+++ b/engines/sci/sound/midiparser_sci.cpp
@@ -717,56 +717,64 @@ bool MidiParser_SCI::processEvent(const EventInfo &info, bool fireEvents) {
 	switch (info.command()) {
 	case 0xC:
 		if (info.channel() == 0xF) {// SCI special case
-			if (info.basic.param1 != kSetSignalLoop) {
-				// At least in kq5/french&mac the first scene in the intro has
-				// a song that sets signal to 4 immediately on tick 0. Signal
-				// isn't set at that point by sierra sci and it would cause the
-				// castle daventry text to get immediately removed, so we
-				// currently filter it. Sierra SCI ignores them as well at that
-				// time. However, this filtering should only be performed for
-				// SCI1 and newer games. Signalling is done differently in SCI0
-				// though, so ignoring these signals in SCI0 games will result
-				// in glitches (e.g. the intro of LB1 Amiga gets stuck - bug
-				// #5693). Refer to MusicEntry::setSignal() in sound/music.cpp.
-				// FIXME: SSCI doesn't start playing at the very beginning
-				// of the stream, but at a fixed location a few commands later.
-				// That is probably why this signal isn't triggered
-				// immediately there.
-				bool skipSignal = false;
-				if (_soundVersion >= SCI_VERSION_1_EARLY) {
-					if (!_position._playTick) {
-						skipSignal = true;
-						switch (g_sci->getGameId()) {
-						case GID_ECOQUEST2:
-							// In Eco Quest 2 room 530 - gonzales is supposed to dance
-							// WORKAROUND: we need to signal in this case on tick 0
-							// this whole issue is complicated and can only be properly fixed by
-							// changing the whole parser to a per-channel parser. SSCI seems to
-							// start each channel at offset 13 (may be 10 for us) and only
-							// starting at offset 0 when the music loops to the initial position.
-							if (g_sci->getEngineState()->currentRoomNumber() == 530)
-								skipSignal = false;
-							break;
+			if (info.basic.param1 == kSetSignalLoop) {
+				_loopTick = _position._playTick;
+				// kSetSignalLoop (127) is not passed on to scripts, except in SCI_VERSION_0_EARLY.
+				// We also pass it to all versions of KQ4 because the scripts expect this. Sierra didn't
+				// update them when they changed the driver behavior. Introduction script 222 waits
+				// on signal 127 in sound 106 to start the game, causing later versions to wait forever.
+				// Now the introduction correctly ends when the music does in all versions.
+				if (_soundVersion > SCI_VERSION_0_EARLY && g_sci->getGameId() != GID_KQ4) {
+					return true;
+				}
+			}
+
+			// At least in kq5/french&mac the first scene in the intro has
+			// a song that sets signal to 4 immediately on tick 0. Signal
+			// isn't set at that point by sierra sci and it would cause the
+			// castle daventry text to get immediately removed, so we
+			// currently filter it. Sierra SCI ignores them as well at that
+			// time. However, this filtering should only be performed for
+			// SCI1 and newer games. Signaling is done differently in SCI0
+			// though, so ignoring these signals in SCI0 games will result
+			// in glitches (e.g. the intro of LB1 Amiga gets stuck - bug
+			// #5693). Refer to MusicEntry::setSignal() in sound/music.cpp.
+			// FIXME: SSCI doesn't start playing at the very beginning
+			// of the stream, but at a fixed location a few commands later.
+			// That is probably why this signal isn't triggered
+			// immediately there.
+			bool skipSignal = false;
+			if (_soundVersion >= SCI_VERSION_1_EARLY) {
+				if (!_position._playTick) {
+					skipSignal = true;
+					switch (g_sci->getGameId()) {
+					case GID_ECOQUEST2:
+						// In Eco Quest 2 room 530 - gonzales is supposed to dance
+						// WORKAROUND: we need to signal in this case on tick 0
+						// this whole issue is complicated and can only be properly fixed by
+						// changing the whole parser to a per-channel parser. SSCI seems to
+						// start each channel at offset 13 (may be 10 for us) and only
+						// starting at offset 0 when the music loops to the initial position.
+						if (g_sci->getEngineState()->currentRoomNumber() == 530)
+							skipSignal = false;
+						break;
 #ifdef ENABLE_SCI32
-						case GID_KQ7:
-							if (g_sci->getEngineState()->currentRoomNumber() == 6050) {
-								skipSignal = false;
-							}
-							break;
-#endif
-						default:
-							break;
+					case GID_KQ7:
+						if (g_sci->getEngineState()->currentRoomNumber() == 6050) {
+							skipSignal = false;
 						}
+						break;
+#endif
+					default:
+						break;
 					}
 				}
-				if (!skipSignal) {
-					if (!_jumpingToTick) {
-						_pSnd->setSignal(info.basic.param1);
-						debugC(4, kDebugLevelSound, "signal %04x", info.basic.param1);
-					}
+			}
+			if (!skipSignal) {
+				if (!_jumpingToTick) {
+					_pSnd->setSignal(info.basic.param1);
+					debugC(4, kDebugLevelSound, "signal %04x", info.basic.param1);
 				}
-			} else {
-				_loopTick = _position._playTick;
 			}
 
 			// Done with this event.