SCI32: Fix Phant2 movie-vs-game playback volumes

This update should give the game the default mix it receives in
Windows. If necessary, the half-volume audio bug in the DOS
interpreter can be added as an additional hack for this game, since
there are still some sub-par audio mixes that might need additional
correction (like Curtis talking to Blob when taking her out of the
cage in his apartment at the start of the game) which were also
bad in the Windows version of the game.

Fixes Trac#10165.

5 files changed, 67 insertions, 18 deletions

diff --git a/engines/sci/engine/guest_additions.cpp b/engines/sci/engine/guest_additions.cpp
index 0452cc2850..90d797aee9 100644
--- a/engines/sci/engine/guest_additions.cpp
+++ b/engines/sci/engine/guest_additions.cpp
@@ -50,12 +50,6 @@ enum {
 	kMessageTypeSpeech    = 2
 };
 
-enum {
-	kLSL6HiresUIVolumeMax  = 13,
-	kLSL6HiresSubtitleFlag = 105
-};
-
-
 GuestAdditions::GuestAdditions(EngineState *state, GameFeatures *features, Kernel *kernel) :
 	_state(state),
 	_features(features),
@@ -912,7 +906,7 @@ void GuestAdditions::syncAudioVolumeGlobalsFromScummVM() const {
 	}
 
 	case GID_PHANTASMAGORIA2: {
-		const int16 masterVolume = (ConfMan.getInt("sfx_volume") + 1) * Audio32::kMaxVolume / Audio::Mixer::kMaxMixerVolume;
+		const int16 masterVolume = (ConfMan.getInt("sfx_volume") + 1) * kPhant2VolumeMax / Audio::Mixer::kMaxMixerVolume;
 		syncPhant2VolumeFromScummVM(masterVolume);
 		syncPhant2UI(masterVolume);
 		break;
@@ -1016,12 +1010,13 @@ void GuestAdditions::syncLSL6HiresVolumeFromScummVM(const int16 musicVolume) con
 	g_sci->_soundCmd->setMasterVolume(ConfMan.getBool("mute") ? 0 : (musicVolume * MUSIC_MASTERVOLUME_MAX / kLSL6HiresUIVolumeMax));
 }
 
-void GuestAdditions::syncPhant2VolumeFromScummVM(const int16 musicVolume) const {
-	_state->variables[VAR_GLOBAL][kGlobalVarPhant2MasterVolume] = make_reg(0, musicVolume);
+void GuestAdditions::syncPhant2VolumeFromScummVM(const int16 masterVolume) const {
+	_state->variables[VAR_GLOBAL][kGlobalVarPhant2MasterVolume] = make_reg(0, masterVolume);
+	_state->variables[VAR_GLOBAL][kGlobalVarPhant2SecondaryVolume] = make_reg(0, masterVolume);
 
 	const reg_t soundsId = _state->variables[VAR_GLOBAL][kGlobalVarSounds];
 	if (!soundsId.isNull()) {
-		reg_t params[] = { make_reg(0, SELECTOR(setVol)), make_reg(0, musicVolume) };
+		reg_t params[] = { make_reg(0, SELECTOR(setVol)), make_reg(0, masterVolume) };
 		invokeSelector(soundsId, SELECTOR(eachElementDo), 2, params);
 	}
 }
@@ -1080,7 +1075,7 @@ void GuestAdditions::syncAudioVolumeGlobalsToScummVM(const int index, const reg_
 
 	case GID_PHANTASMAGORIA2:
 		if (index == kGlobalVarPhant2MasterVolume) {
-			const int16 masterVolume = value.toSint16() * Audio::Mixer::kMaxMixerVolume / Audio32::kMaxVolume;
+			const int16 masterVolume = value.toSint16() * Audio::Mixer::kMaxMixerVolume / kPhant2VolumeMax;
 			ConfMan.setInt("music_volume", masterVolume);
 			ConfMan.setInt("sfx_volume", masterVolume);
 			ConfMan.setInt("speech_volume", masterVolume);
@@ -1247,18 +1242,18 @@ void GuestAdditions::syncPhant1UI(const int16 oldMusicVolume, const int16 musicV
 	}
 }
 
-void GuestAdditions::syncPhant2UI(const int16 musicVolume) const {
-	const reg_t musicVolumeScript = _segMan->findObjectByName("foo2");
+void GuestAdditions::syncPhant2UI(const int16 masterVolume) const {
+	const reg_t masterVolumeScript = _segMan->findObjectByName("foo2");
 	Common::Array<reg_t> scrollBars = _segMan->findObjectsByName("P2ScrollBar");
 	for (uint i = 0; i < scrollBars.size(); ++i) {
-		if (readSelector(_segMan, scrollBars[i], SELECTOR(client)) == musicVolumeScript) {
+		if (readSelector(_segMan, scrollBars[i], SELECTOR(client)) == masterVolumeScript) {
 			// P2ScrollBar objects may exist without actually being on-screen;
 			// the easiest way to tell seems to be to look to see if it has
 			// non-null pointers to subviews. (The game will correctly set the
 			// position of the scrollbar when it first becomes visible, so this
 			// is fine.)
 			if (!readSelector(_segMan, scrollBars[i], SELECTOR(physicalBar)).isNull()) {
-				reg_t params[] = { make_reg(0, musicVolume), make_reg(0, 1) };
+				reg_t params[] = { make_reg(0, masterVolume), make_reg(0, 1) };
 				invokeSelector(scrollBars[i], SELECTOR(move), 2, params);
 				break;
 			}
diff --git a/engines/sci/engine/guest_additions.h b/engines/sci/engine/guest_additions.h
index 7c80c29aac..64bd667414 100644
--- a/engines/sci/engine/guest_additions.h
+++ b/engines/sci/engine/guest_additions.h
@@ -33,6 +33,21 @@ class Kernel;
 class Script;
 class SegManager;
 
+#ifdef ENABLE_SCI32
+enum {
+	// The in-game volumes for Phant2 use a volume range smaller than the
+	// actual master volume because movie volume needs to be controllable from
+	// the normal ScummVM launcher volume controls, but movie dialogue cannot be
+	// heard if the game audio is at the same level as movies. The game normally
+	// sets defaults so that the in-game volume is 85 and movies are 127, so we
+	// will just use 85 as the maximum volume.
+	kPhant2VolumeMax       = 85,
+
+	kLSL6HiresUIVolumeMax  = 13,
+	kLSL6HiresSubtitleFlag = 105
+};
+#endif
+
 /**
  * The GuestAdditions class hooks into the SCI virtual machine to provide
  * enhanced interactions between the ScummVM GUI and the game engine. Currently,
diff --git a/engines/sci/engine/script_patches.cpp b/engines/sci/engine/script_patches.cpp
index 443920128b..e8458480e7 100644
--- a/engines/sci/engine/script_patches.cpp
+++ b/engines/sci/engine/script_patches.cpp
@@ -26,6 +26,9 @@
 #include "sci/engine/state.h"
 #include "sci/engine/features.h"
 #include "sci/engine/script_patches.h"
+#ifdef ENABLE_SCI32
+#include "sci/engine/guest_additions.h"
+#endif
 
 #include "common/util.h"
 
@@ -3879,6 +3882,29 @@ static const uint16 phant2RatboyPatch[] = {
 	PATCH_END
 };
 
+// Phant2 has separate in-game volume controls for handling movie volume and
+// in-game volume (misleading labelled "music volume"), but really needs the
+// in-game volume to always be significantly lower than the movie volume in
+// order for dialogue in movies to be consistently audible, so patch the in-game
+// volume slider to limit it to our maximum.
+// Applies to at least: US English
+static const uint16 phant2AudioVolumeSignature[] = {
+	SIG_MAGICDWORD,
+	0x39, 0x7f,           // pushi 127 (clientMax)
+	0x39, 0x14,           // pushi 20  (clientPageSize)
+	SIG_ADDTOOFFSET(+10), // skip other init arguments
+	0x51, 0x5e,           // class P2ScrollBar
+	SIG_ADDTOOFFSET(+3),  // skip send
+	0xa3, 0x06,           // sal 6 (identifies correct slider)
+	SIG_END
+};
+
+static const uint16 phant2AudioVolumePatch[] = {
+	0x39, kPhant2VolumeMax,              // pushi (our custom volume max)
+	0x39, 0x14 * kPhant2VolumeMax / 127, // pushi (ratio of original value)
+	PATCH_END
+};
+
 // When censorship is disabled the game sticks <PROTECTED> at the end of every
 // save game name, and when it is enabled it pads the save game name with a
 // bunch of spaces. This is annoying and not helpful, so just disable all of
@@ -3945,6 +3971,7 @@ static const SciScriptPatcherEntry phantasmagoria2Signatures[] = {
 	{  true,     0, "slow interface fades",                        3, phant2SlowIFadeSignature,      phant2SlowIFadePatch },
 	{  true,     0, "bad arguments to get game version",           1, phant2GetVersionSignature,     phant2GetVersionPatch },
 	{  true,  4081, "non-responsive mouse after ratboy puzzle",    1, phant2RatboySignature,         phant2RatboyPatch },
+	{  true, 63004, "limit in-game audio volume",                  1, phant2AudioVolumeSignature,    phant2AudioVolumePatch },
 	{  true, 63016, "non-responsive mouse during music fades",     1, phant2Wait4FadeSignature,      phant2Wait4FadePatch },
 	{  true, 63019, "non-responsive mouse during computer load",   1, phant2CompSlideDoorsSignature, phant2CompSlideDoorsPatch },
 	{  true, 64990, "remove save game name mangling (1/2)",        1, phant2SaveNameSignature1,      phant2SaveNamePatch1 },
diff --git a/engines/sci/engine/vm.h b/engines/sci/engine/vm.h
index 67729bfbf9..792287ba53 100644
--- a/engines/sci/engine/vm.h
+++ b/engines/sci/engine/vm.h
@@ -149,9 +149,10 @@ enum GlobalVar {
 	kGlobalVarNewRoomNo      = 13,
 	kGlobalVarScore          = 15,
 	kGlobalVarGK2MusicVolume = 76, // 0 to 127
-	kGlobalVarFastCast       = 84, // SCI16
-	kGlobalVarMessageType    = 90,
-	kGlobalVarTextSpeed      = 94, // SCI32; 0 is fastest, 8 is slowest
+	kGlobalVarPhant2SecondaryVolume = 76, // 0 to 127
+	kGlobalVarFastCast             = 84, // SCI16
+	kGlobalVarMessageType          = 90,
+	kGlobalVarTextSpeed            = 94, // SCI32; 0 is fastest, 8 is slowest
 	kGlobalVarGK1Music1            = 102, // 0 to 127
 	kGlobalVarGK1Music2            = 103, // 0 to 127
 	kGlobalVarLSL6HiresGameFlags   = 137,
diff --git a/engines/sci/sound/audio32.cpp b/engines/sci/sound/audio32.cpp
index c3f9eb4098..ad9af4ebf6 100644
--- a/engines/sci/sound/audio32.cpp
+++ b/engines/sci/sound/audio32.cpp
@@ -359,6 +359,17 @@ int Audio32::readBuffer(Audio::st_sample_t *const buffer, const int numSamples)
 				} else if (volume > 84 && volume < kMaxVolume) {
 					volume = 63;
 				}
+			} else if (getSciVersion() == SCI_VERSION_3 && volume != kMaxVolume) {
+				// In SCI3, granularity of the non-maximum volumes is 1/32
+				volume &= ~4;
+
+				// NOTE: In the SSCI DOS interpreter, non-maximum volumes are
+				// divided by 8 which puts them in a range of [0, 16). That
+				// reduced volume range gets passed into a volume function which
+				// expects values [0, 32). So, effectively, all non-maximum
+				// volumes are half-volume in DOS in SCI3. In Windows, volumes
+				// [120, 124) are the same as 127 due to a programming bug.
+				// We do not emulate either of these incorrect behaviors.
 			}
 
 			leftVolume = rightVolume = volume * Audio::Mixer::kMaxChannelVolume / kMaxVolume;