From 199e5cb4f7a4aea5832ac35431d1049f51ccf58b Mon Sep 17 00:00:00 2001
From: Max Horn
Date: Fri, 14 Apr 2006 18:51:42 +0000
Subject: 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
---
 engines/scumm/dialogs.cpp | 40 ++++++++++++++++++++++++++++++++++------
 1 file 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;
-- 
cgit v1.2.3