SCI: Some renaming and added several TODOs concerning reverb

svn-id: r54461

6 files changed, 26 insertions, 23 deletions

diff --git a/engines/sci/engine/savegame.cpp b/engines/sci/engine/savegame.cpp
index 17c9b81fce..0df6515483 100644
--- a/engines/sci/engine/savegame.cpp
+++ b/engines/sci/engine/savegame.cpp
@@ -557,7 +557,7 @@ void SciMusic::saveLoadWithSerializer(Common::Serializer &s) {
 
 		soundSetSoundOn(_soundOn);
 		soundSetMasterVolume(masterVolume);
-		setReverb(reverb);
+		setGlobalReverb(reverb);
 	}
 
 	if (s.isSaving())
diff --git a/engines/sci/sound/drivers/mididriver.h b/engines/sci/sound/drivers/mididriver.h
index c9cde09494..710bd98c97 100644
--- a/engines/sci/sound/drivers/mididriver.h
+++ b/engines/sci/sound/drivers/mididriver.h
@@ -111,7 +111,9 @@ public:
 		return _driver ? _driver->property(MIDI_PROP_MASTER_VOLUME, 0xffff) : 0;
 	}
 
+	// Returns the current reverb
 	byte getReverb() const { return _reverb; }
+	// Sets the current reverb, used mainly in MT-32
 	virtual void setReverb(byte reverb) { _reverb = reverb; }
 
 	virtual void playSwitch(bool play) {
diff --git a/engines/sci/sound/midiparser_sci.cpp b/engines/sci/sound/midiparser_sci.cpp
index 398917764c..ed6026932a 100644
--- a/engines/sci/sound/midiparser_sci.cpp
+++ b/engines/sci/sound/midiparser_sci.cpp
@@ -501,6 +501,8 @@ void MidiParser_SCI::parseNextEvent(EventInfo &info) {
 			// Also, sci/sound/iterator/iterator.cpp, function BaseSongIterator::parseMidiCommand()
 			switch (info.basic.param1) {
 			case kSetReverb:
+				// TODO: This should be the song's reverb, and we need to check it against
+				// the global one
 				if (info.basic.param2 != 127)	// 127: SCI invalid, ignore
 					((MidiPlayer *)_driver)->setReverb(info.basic.param2);
 				break;
diff --git a/engines/sci/sound/music.cpp b/engines/sci/sound/music.cpp
index d92669de39..443f743ee6 100644
--- a/engines/sci/sound/music.cpp
+++ b/engines/sci/sound/music.cpp
@@ -225,17 +225,19 @@ MusicEntry *SciMusic::getActiveSci0MusicSlot() {
 	return highestPrioritySlot;
 }
 
-void SciMusic::setReverb(byte reverb) {
+void SciMusic::setGlobalReverb(byte reverb) {
 	Common::StackLock lock(_mutex);
-	if (reverb != 127)	// 127: SCI invalid, ignore
+	if (reverb != 127) {
+		// Set global reverb normally
+		// TODO: Set global music reverb
+		// TODO: Only set reverb when the reverb of the active song is 127
 		_pMidiDrv->setReverb(reverb);
-
-	// SSCI stored a separate reverb value per song
-	// We don't, currently, as the current functionality
-	// works without an additional variable
+	} else {
+		// TODO: Set reverb of the active song
+	}
 }
 
-byte SciMusic::getReverb() {
+byte SciMusic::getCurrentReverb() {
 	Common::StackLock lock(_mutex);
 	return _pMidiDrv->getReverb();
 }
@@ -666,6 +668,7 @@ MusicEntry::MusicEntry() {
 	loop = 0;
 	volume = MUSIC_VOLUME_DEFAULT;
 	hold = -1;
+	reverb = -1;
 
 	pauseCounter = 0;
 	sampleLoopCounter = 0;
diff --git a/engines/sci/sound/music.h b/engines/sci/sound/music.h
index 71f923367d..c29a8456b5 100644
--- a/engines/sci/sound/music.h
+++ b/engines/sci/sound/music.h
@@ -76,6 +76,7 @@ public:
 	uint16 loop;
 	int16 volume;
 	int16 hold;
+	int16 reverb;
 
 	int16 pauseCounter;
 	uint sampleLoopCounter;
@@ -187,8 +188,8 @@ public:
 	void sendMidiCommand(uint32 cmd);
 	void sendMidiCommand(MusicEntry *pSnd, uint32 cmd);
 
-	void setReverb(byte reverb);
-	byte getReverb();
+	void setGlobalReverb(byte reverb);
+	byte getCurrentReverb();
 
 	virtual void saveLoadWithSerializer(Common::Serializer &ser);
 
diff --git a/engines/sci/sound/soundcmd.cpp b/engines/sci/sound/soundcmd.cpp
index 644e2a43d7..3327fcfcfe 100644
--- a/engines/sci/sound/soundcmd.cpp
+++ b/engines/sci/sound/soundcmd.cpp
@@ -492,21 +492,16 @@ reg_t SoundCommandParser::kDoSoundSendMidi(int argc, reg_t *argv, reg_t acc) {
 }
 
 reg_t SoundCommandParser::kDoSoundGlobalReverb(int argc, reg_t *argv, reg_t acc) {
-	if (argc == 0)
-		return make_reg(0, _music->getReverb());
+	byte prevReverb = _music->getCurrentReverb();
+	byte reverb = argv[0].toUint16() & 0xF;
 
-	debugC(2, kDebugLevelSound, "doSoundGlobalReverb: %d", argv[0].toUint16() & 0xF);
-
-	// This is a bit different than SSCI, but the end result should be the same.
-	// SSCI checks the first entry in the playlist for its current reverb value.
-	// If it's 127 (invalid), it sets the reverb parameter of this call to the
-	// driver, otherwise it doesn't, to preserve the song's reverb. When we modify
-	// the global reverb, we send its value to the soundcard, if it isn't 127 (invalid).
-	// This assumes that the currently set reverb in the driver will be the reverb
-	// value sent from the last song, which should be the one at the top of the list.
-	_music->setReverb(argv[0].toUint16() & 0xF);
+	if (argc == 1) {
+		debugC(2, kDebugLevelSound, "doSoundGlobalReverb: %d", argv[0].toUint16() & 0xF);
+		if (reverb <= 10)
+			_music->setGlobalReverb(reverb);
+	}
 
-	return acc;
+	return make_reg(0, prevReverb);
 }
 
 reg_t SoundCommandParser::kDoSoundSetHold(int argc, reg_t *argv, reg_t acc) {