Some cleanup of the SCUMM ConfigDialog; also added a big FIXME comment explaining what the dialog does badly, and how that could be fixed (anybody feeling bored, feel free to implement the solution I outline there)

svn-id: r21890

1 files changed, 34 insertions, 6 deletions

diff --git a/engines/scumm/dialogs.cpp b/engines/scumm/dialogs.cpp
index 6c87844b8c..333e702944 100644
--- a/engines/scumm/dialogs.cpp
+++ b/engines/scumm/dialogs.cpp
@@ -642,6 +642,31 @@ enum {
 	kKeysCmd = 'KEYS'
 };
 
+// FIXME: We use the empty string as domain name here. This tells the
+// ConfigManager to use the 'default' domain for all its actions. We do that
+// to get as close as possible to editing the 'active' settings.
+//
+// However, that requires bad & evil hacks in the ConfigManager code,
+// and even then still doesn't work quite correctly.
+// For example, if the transient domain contains 'false' for the 'fullscreen'
+// flag, but the user used a hotkey to switch to windowed mode, then the dialog
+// will display the wrong value anyway.
+//
+// Proposed solution consisting of multiple steps:
+// 1) Add special code to the open() code that reads out everything stored
+//    in the transient domain that is controlled by this dialog, and updates
+//    the dialog accordingly.
+// 2) Even more code is added to query the backend for current settings, like
+//    the fullscreen mode flag etc., and also updates the dialog accordingly.
+// 3) The domain being edited is set to the active game domain.
+// 4) If the dialog is closed with the "OK" button, then we remove everything
+//    stored in the transient domain (or at least everything corresponding to
+//    switches in this dialog.
+//    If OTOH the dialog is closed with "Cancel" we do no such thing.
+//
+// These changes will achieve two things at once: Allow us to get rid of using
+//  "" as value for the domain, and in fact provide a somewhat better user 
+// experience at the same time.
 ConfigDialog::ConfigDialog(ScummEngine *scumm)
 	: GUI::OptionsDialog("", "scummconfig"), _vm(scumm) {
 
@@ -687,20 +712,23 @@ void ConfigDialog::open() {
 	GUI_OptionsDialog::open();
 
 	// update checkboxes, too
-	_subtitlesCheckbox->setState(ConfMan.getBool("subtitles"));
-	_speechCheckbox->setState(!ConfMan.getBool("speech_mute"));
+	_subtitlesCheckbox->setState(ConfMan.getBool("subtitles", _domain));
+	_speechCheckbox->setState(!ConfMan.getBool("speech_mute", _domain));
 }
 
 void ConfigDialog::close() {
 	if (getResult()) {
 		// Subtitles
 		ConfMan.set("subtitles", _subtitlesCheckbox->getState(), _domain);
-		ConfMan.set("speech_mute", !_speechCheckbox->getState(), _domain);
+
 		// Sync with current setting
-		if (ConfMan.getBool("speech_mute"))
+		if (!_speechCheckbox->getState()) {
+			ConfMan.set("speech_mute", true, _domain);
 			_vm->_voiceMode = 2;
-		else
-			_vm->_voiceMode = ConfMan.getBool("subtitles");
+		} else {
+			ConfMan.set("speech_mute", false, _domain);
+			_vm->_voiceMode = _subtitlesCheckbox->getState();
+		}
 
 		if (_vm->_game.version >= 7)
 			_vm->VAR(_vm->VAR_VOICE_MODE) = _vm->_voiceMode;