[Scummvm-git-logs] scummvm master -> e25ac0bc55d88bb8761450e460ba8b4cd0674145

antoniou79 a.antoniou79 at gmail.com
Wed Oct 7 12:26:43 UTC 2020


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

Summary:
465807f165 JANITORIAL: Better vertical align of methos and members
e25ac0bc55 ANDROID: Correctly init config keys and reflect values on GUI


Commit: 465807f165a2d0a4f3f8fb73686ce52db677bbaa
    https://github.com/scummvm/scummvm/commit/465807f165a2d0a4f3f8fb73686ce52db677bbaa
Author: antoniou (a.antoniou79 at gmail.com)
Date: 2020-10-07T15:26:31+03:00

Commit Message:
JANITORIAL: Better vertical align of methos and members

Changed paths:
    common/config-manager.h


diff --git a/common/config-manager.h b/common/config-manager.h
index 185535d5e0..e632f23eba 100644
--- a/common/config-manager.h
+++ b/common/config-manager.h
@@ -58,37 +58,37 @@ public:
 
 	class Domain {
 	private:
-		StringMap _entries;
-		StringMap _keyValueComments;
-		String _domainComment;
+		StringMap     _entries;
+		StringMap     _keyValueComments;
+		String        _domainComment;
 
 	public:
 		typedef StringMap::const_iterator const_iterator;
 		const_iterator begin() const { return _entries.begin(); }
 		const_iterator end()   const { return _entries.end(); }
 
-		bool empty() const { return _entries.empty(); }
+		bool           empty() const { return _entries.empty(); }
 
-		bool contains(const String &key) const { return _entries.contains(key); }
+		bool           contains(const String &key) const { return _entries.contains(key); }
 
-		String &operator[](const String &key) { return _entries[key]; }
-		const String &operator[](const String &key) const { return _entries[key]; }
+		String        &operator[](const String &key) { return _entries[key]; }
+		const String  &operator[](const String &key) const { return _entries[key]; }
 
-		void setVal(const String &key, const String &value) { _entries.setVal(key, value); }
+		void           setVal(const String &key, const String &value) { _entries.setVal(key, value); }
 
-		String &getVal(const String &key) { return _entries.getVal(key); }
-		const String &getVal(const String &key) const { return _entries.getVal(key); }
+		String        &getVal(const String &key) { return _entries.getVal(key); }
+		const String  &getVal(const String &key) const { return _entries.getVal(key); }
 
-		void clear() { _entries.clear(); }
+		void           clear() { _entries.clear(); }
 
-		void erase(const String &key) { _entries.erase(key); }
+		void           erase(const String &key) { _entries.erase(key); }
 
-		void setDomainComment(const String &comment);
-		const String &getDomainComment() const;
+		void           setDomainComment(const String &comment);
+		const String  &getDomainComment() const;
 
-		void setKVComment(const String &key, const String &comment);
-		const String &getKVComment(const String &key) const;
-		bool hasKVComment(const String &key) const;
+		void           setKVComment(const String &key, const String &comment);
+		const String  &getKVComment(const String &key) const;
+		bool           hasKVComment(const String &key) const;
 	};
 
 	typedef HashMap<String, Domain, IgnoreCase_Hash, IgnoreCase_EqualTo> DomainMap;
@@ -107,16 +107,16 @@ public:
 	static char const *const kCloudDomain;
 #endif
 
-	void				loadDefaultConfigFile(); /*!< Load the default configuration file. */
-	void				loadConfigFile(const String &filename); /*!< Load a specific configuration file. */
+	void                     loadDefaultConfigFile(); /*!< Load the default configuration file. */
+	void                     loadConfigFile(const String &filename); /*!< Load a specific configuration file. */
 
 	/**
 	 * Retrieve the config domain with the given name.
 	 * @param domName Name of the domain to retrieve.
 	 * @return Pointer to the domain, or 0 if the domain does not exist.
 	 */
-	Domain *			getDomain(const String &domName);
-	const Domain *		getDomain(const String &domName) const; /*!< @overload */
+	Domain                  *getDomain(const String &domName);
+	const Domain            *getDomain(const String &domName) const; /*!< @overload */
 
 
     /**
@@ -126,16 +126,16 @@ public:
 	 * @{
 	 */
 
-	bool				hasKey(const String &key) const;
-	const String &		get(const String &key) const;
-	void				set(const String &key, const String &value);
+	bool                     hasKey(const String &key) const;
+	const String            &get(const String &key) const;
+	void                     set(const String &key, const String &value);
     /** @} */
 	
 	/**
 	 * Update a configuration entry for the active domain and flush
 	 * the configuration file to disk if the value changed.
 	 */
-	void				setAndFlush(const String &key, const Common::String &value);
+	void                     setAndFlush(const String &key, const Common::String &value);
 
 #if 1
     /**
@@ -148,11 +148,11 @@ public:
 	 * @{
 	 */
 
-	bool				hasKey(const String &key, const String &domName) const;
-	const String &		get(const String &key, const String &domName) const;
-	void				set(const String &key, const String &value, const String &domName);
+	bool                     hasKey(const String &key, const String &domName) const;
+	const String            &get(const String &key, const String &domName) const;
+	void                     set(const String &key, const String &value, const String &domName);
 
-	void				removeKey(const String &key, const String &domName);
+	void                     removeKey(const String &key, const String &domName);
 	/** @} */
 #endif
 
@@ -161,69 +161,69 @@ public:
 	 * @{
 	 */
 
-	int					getInt(const String &key, const String &domName = String()) const; /*!< Get integer value. */
-	bool				getBool(const String &key, const String &domName = String()) const; /*!< Get Boolean value. */
-	void				setInt(const String &key, int value, const String &domName = String()); /*!< Set integer value. */
-	void				setBool(const String &key, bool value, const String &domName = String()); /*!< Set integer value. */
+	int                      getInt(const String &key, const String &domName = String()) const; /*!< Get integer value. */
+	bool                     getBool(const String &key, const String &domName = String()) const; /*!< Get Boolean value. */
+	void                     setInt(const String &key, int value, const String &domName = String()); /*!< Set integer value. */
+	void                     setBool(const String &key, bool value, const String &domName = String()); /*!< Set integer value. */
 
 
-	void				registerDefault(const String &key, const String &value);
-	void				registerDefault(const String &key, const char *value);
-	void				registerDefault(const String &key, int value);
-	void				registerDefault(const String &key, bool value);
+	void                     registerDefault(const String &key, const String &value);
+	void                     registerDefault(const String &key, const char *value);
+	void                     registerDefault(const String &key, int value);
+	void                     registerDefault(const String &key, bool value);
 
-	void				flushToDisk(); /*!< Flush configuration to disk. */
+	void                     flushToDisk(); /*!< Flush configuration to disk. */
 
-	void				setActiveDomain(const String &domName); /*!< Set the given domain as active. */
-	Domain *			getActiveDomain() { return _activeDomain; } /*!< Get the active domain. */
-	const Domain *		getActiveDomain() const { return _activeDomain; } /*!< @overload */
-	const String &		getActiveDomainName() const { return _activeDomainName; } /*!< Get the name of the active domain. */
+	void                     setActiveDomain(const String &domName); /*!< Set the given domain as active. */
+	Domain                  *getActiveDomain() { return _activeDomain; } /*!< Get the active domain. */
+	const Domain            *getActiveDomain() const { return _activeDomain; } /*!< @overload */
+	const String            &getActiveDomainName() const { return _activeDomainName; } /*!< Get the name of the active domain. */
 
-	void				addGameDomain(const String &domName); /*!< Add a new game domain. */
-	void				removeGameDomain(const String &domName); /*!< Remove a game domain. */
-	void				renameGameDomain(const String &oldName, const String &newName); /*!< Rename a game domain. */
+	void                     addGameDomain(const String &domName); /*!< Add a new game domain. */
+	void                     removeGameDomain(const String &domName); /*!< Remove a game domain. */
+	void                     renameGameDomain(const String &oldName, const String &newName); /*!< Rename a game domain. */
 
-	void				addMiscDomain(const String &domName); /*!< Add a miscellaneous domain. */
-	void				removeMiscDomain(const String &domName); /*!< Remove a miscellaneous domain. */
-	void				renameMiscDomain(const String &oldName, const String &newName); /*!< Rename a miscellaneous domain. */
+	void                     addMiscDomain(const String &domName); /*!< Add a miscellaneous domain. */
+	void                     removeMiscDomain(const String &domName); /*!< Remove a miscellaneous domain. */
+	void                     renameMiscDomain(const String &oldName, const String &newName); /*!< Rename a miscellaneous domain. */
 
-	bool				hasGameDomain(const String &domName) const;
-	bool				hasMiscDomain(const String &domName) const;
+	bool                     hasGameDomain(const String &domName) const;
+	bool                     hasMiscDomain(const String &domName) const;
 
-	const DomainMap &	getGameDomains() const { return _gameDomains; }
-	DomainMap::iterator beginGameDomains() { return _gameDomains.begin(); }
-	DomainMap::iterator endGameDomains() { return _gameDomains.end(); }
+	const DomainMap         &getGameDomains() const { return _gameDomains; }
+	DomainMap::iterator      beginGameDomains() { return _gameDomains.begin(); }
+	DomainMap::iterator      endGameDomains() { return _gameDomains.end(); }
 
-	static void			defragment(); // move in memory to reduce fragmentation
-	void 				copyFrom(ConfigManager &source);
+	static void              defragment(); // move in memory to reduce fragmentation
+	void                     copyFrom(ConfigManager &source);
 	/** @} */
 private:
 	friend class Singleton<SingletonBaseType>;
 	ConfigManager();
 
-	void			loadFromStream(SeekableReadStream &stream);
-	void			addDomain(const String &domainName, const Domain &domain);
-	void			writeDomain(WriteStream &stream, const String &name, const Domain &domain);
-	void			renameDomain(const String &oldName, const String &newName, DomainMap &map);
+	void                     loadFromStream(SeekableReadStream &stream);
+	void                     addDomain(const String &domainName, const Domain &domain);
+	void                     writeDomain(WriteStream &stream, const String &name, const Domain &domain);
+	void                     renameDomain(const String &oldName, const String &newName, DomainMap &map);
 
-	Domain			_transientDomain;
-	DomainMap		_gameDomains;
-	DomainMap		_miscDomains; // Any other domains
-	Domain			_appDomain;
-	Domain			_defaultsDomain;
+	Domain                  _transientDomain;
+	DomainMap               _gameDomains;
+	DomainMap               _miscDomains; // Any other domains
+	Domain                  _appDomain;
+	Domain                  _defaultsDomain;
 
-	Domain			_keymapperDomain;
+	Domain                  _keymapperDomain;
 
 #ifdef USE_CLOUD
-	Domain			_cloudDomain;
+	Domain                  _cloudDomain;
 #endif
 
-	Array<String>	_domainSaveOrder;
+	Array<String>           _domainSaveOrder;
 
-	String			_activeDomainName;
-	Domain *		_activeDomain;
+	String                  _activeDomainName;
+	Domain                 *_activeDomain;
 
-	String			_filename;
+	String                  _filename;
 };
 
 /** @} */
@@ -231,6 +231,6 @@ private:
 } // End of namespace Common
 
 /** Shortcut for accessing the configuration manager. */
-#define ConfMan		Common::ConfigManager::instance()
+#define ConfMan                  Common::ConfigManager::instance()
 
 #endif


Commit: e25ac0bc55d88bb8761450e460ba8b4cd0674145
    https://github.com/scummvm/scummvm/commit/e25ac0bc55d88bb8761450e460ba8b4cd0674145
Author: antoniou (a.antoniou79 at gmail.com)
Date: 2020-10-07T15:26:31+03:00

Commit Message:
ANDROID: Correctly init config keys and reflect values on GUI

Also comments added about how this process works. Fixes bug #11743

Changed paths:
    backends/platform/android/android.cpp


diff --git a/backends/platform/android/android.cpp b/backends/platform/android/android.cpp
index c773bc4fb3..07f7cfeefc 100644
--- a/backends/platform/android/android.cpp
+++ b/backends/platform/android/android.cpp
@@ -302,25 +302,97 @@ void *OSystem_Android::audioThreadFunc(void *arg) {
 	return 0;
 }
 
+//
+// When launching ScummVM (from ScummVMActivity) order of business is as follows:
+// 1. scummvm_main() (base/main.cpp)
+// 1.1. call system.initBackend() (from scummvm_main() (base/main.cpp))
+//       According to comments in main.cpp:
+//         "Init the backend. Must take place after all config data (including the command line params) was read."
+// 1.2. call setupGraphics(system); (from scummvm_main() (base/main.cpp))
+// 1.3. call launcherDialog() (from scummvm_main() (base/main.cpp))
+//         Upon calling launcherDialog() the transient domain configuration options are cleared!
+//       According to comments in main.cpp:
+//         "Those that affect the graphics mode and the others (like bootparam etc.) should not blindly be passed to the first game launched from the launcher."
 void OSystem_Android::initBackend() {
 	ENTER();
 
 	_main_thread = pthread_self();
 
+	// Warning: ConfMan.registerDefault() can be used for a Session of ScummVM
+	//          but:
+	//              1. The values will NOT persist to storage
+	//                 ie. they won't get saved to scummvm.ini
+	//              2. The values will NOT be reflected on the GUI
+	//                 and they cannot be recovered after exiting scummvm and re-launching
+	//          Also, if after a ConfMan.registerDefault(), we subsequently use ConfMan.hasKey()
+	//          here or anywhere else in ScummVM, it WILL NOT return true.
+	//			As noted in ConfigManager::hasKey() implementation: (common/config_manager.cpp)
+	//			// Search the domains in the following order:
+	//              // 1) the transient domain,
+	//              // 2) the active game domain (if any),
+	//              // 3) the application domain.
+	//         -->  // The defaults domain is explicitly *not* checked. <--
+	//
+	// So for at least some of these keys,
+	// we need to additionally check with hasKey() if they are persisted
+	// and set them explicitly that way.
+	// TODO Maybe the registerDefault only has meaning for "savepath"
+	//      and similar key/values retrieved from "Command Line"
+	//      so that they won't get "nuked"
+	//      and maintained for the duration ScummVM app session (until we exit the app)
 	ConfMan.registerDefault("fullscreen", true);
 	ConfMan.registerDefault("aspect_ratio", true);
+	ConfMan.registerDefault("filtering", false);
 	ConfMan.registerDefault("touchpad_mouse_mode", true);
 	ConfMan.registerDefault("onscreen_control", true);
+	ConfMan.registerDefault("autosave_period", 0);
+
+	// explicitly set this, since fullscreen cannot be changed from GUI
+	// and for Android it should be persisted (and ConfMan.hasKey("fullscreen") check should return true for it)
+	// Also in Options::dialogBuild() (gui/options.cpp), since Android does not have kFeatureFullscreenMode (see hasFeature() below)
+	//      the state of the checkbox in the GUI is set to true (and disabled)
+	ConfMan.setBool("fullscreen", true);
+
+	// Aspect ratio can be changed from the GUI.
+	// However we set it explicitly here (in addition to the registerDefault command above)
+	//         if it's not already set in the persistent config file
+	if (!ConfMan.hasKey("aspect_ratio")) {
+		ConfMan.setBool("aspect_ratio", true);
+	}
+
+	if (!ConfMan.hasKey("filtering")) {
+		ConfMan.setBool("filtering", false);
+	}
+
+	// Note: About the stretch mode setting
+	//       If not explicitly set in the config file
+	//       the default used by OSystem::setStretchMode() (common/system.h)
+	//       is the one returned by getDefaultStretchMode() (backends/graphics/opengl-graphics.cpp)
+	//       which currently is STRETCH_FIT
+
+	if (!ConfMan.hasKey("touchpad_mouse_mode")) {
+		ConfMan.setBool("touchpad_mouse_mode", true);
+	}
+	_touchpad_mode = ConfMan.getBool("touchpad_mouse_mode");
+
+	if (!ConfMan.hasKey("onscreen_control")) {
+		ConfMan.setBool("onscreen_control", true);
+	}
+	JNI::showKeyboardControl(ConfMan.getBool("onscreen_control"));
+
+	if (!ConfMan.hasKey("autosave_period")) {
+		ConfMan.setInt("autosave_period", 0);
+	}
+
 	// The swap_menu_and_back is a deprecated configuration key
 	// It is no longer relevant, after introducing the keymapper functionality
 	// since the behaviour of the menu and back buttons is now handled by the keymapper.
 	// We now ignore it completely
 
-	ConfMan.registerDefault("autosave_period", 0);
 	ConfMan.setBool("FM_high_quality", false);
 	ConfMan.setBool("FM_medium_quality", true);
 
-	// we need a relaxed delay for the remapping timeout since handling touch interface and virtual keyboard can be slow
+	// We need a relaxed delay for the remapping timeout since handling touch interface and virtual keyboard can be slow
 	// and especially in some occasions when we need to pull down (hide) the keyboard and map a system key (like the AC_Back) button.
 	// 8 seconds should be enough
 	ConfMan.registerDefault("remap_timeout_delay_ms", 8000);
@@ -329,31 +401,22 @@ void OSystem_Android::initBackend() {
 	}
 
 	if (!ConfMan.hasKey("browser_lastpath")) {
-		// TODO remove the debug message eventually
-		LOGD("Setting Browser Lastpath to root");
 		ConfMan.set("browser_lastpath", "/");
 	}
 
-	if (ConfMan.hasKey("touchpad_mouse_mode")) {
-		_touchpad_mode = ConfMan.getBool("touchpad_mouse_mode");
-	} else {
-		ConfMan.setBool("touchpad_mouse_mode", true);
-		_touchpad_mode = true;
-	}
-
-	if (ConfMan.hasKey("onscreen_control"))
-		JNI::showKeyboardControl(ConfMan.getBool("onscreen_control"));
-	else
-		ConfMan.setBool("onscreen_control", true);
-
 	// BUG: "transient" ConfMan settings get nuked by the options
 	// screen. Passing the savepath in this way makes it stick
-	// (via ConfMan.registerDefault)
+	// (via ConfMan.registerDefault() which is called from DefaultSaveFileManager constructor (backends/saves/default/default-saves.cpp))
 	// Note: The aforementioned bug is probably the one reported here:
 	//  https://bugs.scummvm.org/ticket/3712
 	//  and maybe here:
 	//  https://bugs.scummvm.org/ticket/7389
-	// TODO is this right to save full path?
+	// However, we do NOT set the savepath key explicitly for ConfMan
+	//          and thus the savepath will only be persisted as "default" config
+	//          for the rest of the app session (until exit).
+	//          It will NOT be reflected on the GUI, if it's not set explicitly by the user there
+	// TODO Why do we need it not shown on the GUI though?
+	//      Btw, this is a ScummVM thing, the "defaults" do not show they values on our GUI)
 	_savefileManager = new DefaultSaveFileManager(ConfMan.get("savepath"));
 	// TODO remove the debug message eventually
 	LOGD("Setting DefaultSaveFileManager path to: %s", ConfMan.get("savepath").c_str());
@@ -389,6 +452,8 @@ void OSystem_Android::initBackend() {
 }
 
 bool OSystem_Android::hasFeature(Feature f) {
+	if (f == kFeatureFullscreenMode)
+		return false;
 	if (f == kFeatureVirtualKeyboard ||
 			f == kFeatureOpenUrl ||
 			f == kFeatureTouchpadMode ||




More information about the Scummvm-git-logs mailing list