From: <geo...@us...> - 2012-10-22 05:55:51
|
Revision: 5327 http://freeorion.svn.sourceforge.net/freeorion/revision/?rev=5327&view=rev Author: geoffthemedio Date: 2012-10-22 05:55:45 +0000 (Mon, 22 Oct 2012) Log Message: ----------- Attempt to fix longstanding issue (bug 2986732 - std::string values in config.xml erroneously saved as "1") by adding explicit flag parameter to Options and checking that before assuming an XML entry with no text is a flag (rather than a blank string, which was then given the text "1" which lead to "1" as text for strings in-game). Not sure how to actually test this, though. Modified Paths: -------------- trunk/FreeOrion/util/OptionsDB.cpp trunk/FreeOrion/util/OptionsDB.h Modified: trunk/FreeOrion/util/OptionsDB.cpp =================================================================== --- trunk/FreeOrion/util/OptionsDB.cpp 2012-10-22 02:31:00 UTC (rev 5326) +++ trunk/FreeOrion/util/OptionsDB.cpp 2012-10-22 05:55:45 UTC (rev 5327) @@ -73,7 +73,7 @@ OptionsDB::Option::Option(char short_name_, const std::string& name_, const boost::any& value_, const boost::any& default_value_, const std::string& description_, - const ValidatorBase* validator_, bool storable_) : + const ValidatorBase* validator_, bool storable_, bool flag_) : name(name_), short_name(short_name_), value(value_), @@ -81,6 +81,7 @@ description(description_), validator(validator_), storable(storable_), + flag(flag_), option_changed_sig_ptr(new boost::signal<void ()>()) { if (short_name_) @@ -88,27 +89,24 @@ } void OptionsDB::Option::SetFromString(const std::string& str) { - if (validator) { // non-flag + if (!flag) value = validator->Validate(str); - } else { // flag + else value = boost::lexical_cast<bool>(str); - } } std::string OptionsDB::Option::ValueToString() const { - if (validator) { // non-flag + if (!flag) return validator->String(value); - } else { // flag + else return boost::lexical_cast<std::string>(boost::any_cast<bool>(value)); - } } std::string OptionsDB::Option::DefaultValueToString() const { - if (validator) { // non-flag + if (!flag) return validator->String(default_value); - } else { // flag + else return boost::lexical_cast<std::string>(boost::any_cast<bool>(default_value)); - } } @@ -130,11 +128,10 @@ if (it == m_options.end()) throw std::runtime_error("Attempted to validate unknown option \"" + name + "\"."); - if (it->second.validator) { // non-flag + if (it->second.validator) it->second.validator->Validate(value); - } else { // flag + else if (it->second.flag) boost::lexical_cast<bool>(value); - } } std::string OptionsDB::GetValueString(const std::string& option_name) const { @@ -257,9 +254,9 @@ } XMLElement temp(name); - if (it->second.validator) { // non-flag + if (it->second.validator) { temp.SetText(it->second.ValueToString()); - } else { // flag + } else if (it->second.flag) { if (!boost::any_cast<bool>(it->second.value)) continue; } @@ -303,7 +300,7 @@ if (option.value.empty()) throw std::runtime_error("The value member of option \"--" + option.name + "\" is undefined."); - if (option.validator) { // non-flag + if (!option.flag) { // non-flag try { // ensure a parameter exists... if (i + 1 >= static_cast<unsigned int>(args.size())) @@ -352,16 +349,14 @@ if (option.value.empty()) throw std::runtime_error("The value member of option \"--" + option.name + "\" is undefined."); - if (option.validator) { // non-flag + if (!option.flag) { if (j < single_char_options.size() - 1) throw std::runtime_error(std::string("Option \"-") + single_char_options[j] + "\" was given with no parameter."); else option.SetFromString(args[++i]); - } else { // flag + } else { option.value = true; } - - //option_changed = true; } } } @@ -369,18 +364,18 @@ } void OptionsDB::SetFromXML(const XMLDoc& doc) { - for (int i = 0; i < doc.root_node.NumChildren(); ++i) { + for (int i = 0; i < doc.root_node.NumChildren(); ++i) SetFromXMLRecursive(doc.root_node.Child(i), ""); - } } void OptionsDB::SetFromXMLRecursive(const XMLElement& elem, const std::string& section_name) { std::string option_name = section_name + (section_name == "" ? "" : ".") + elem.Tag(); - // flags have no text or children; their presence at all indicates a value of true - std::string option_value = elem.NumChildren() || elem.Text() != "" ? elem.Text() : "1"; + if (elem.NumChildren()) { + for (int i = 0; i < elem.NumChildren(); ++i) + SetFromXMLRecursive(elem.Child(i), option_name); - if (option_value != "") { + } else { std::map<std::string, Option>::iterator it = m_options.find(option_name); if (it == m_options.end()) { @@ -389,19 +384,19 @@ } Option& option = it->second; - if (option.value.empty()) { - Logger().errorStream() << "The value member of option \"" << option.name << "\" in config.xml is undefined."; - return; - } + //if (!option.flag && option.value.empty()) { + // Logger().errorStream() << "The value member of option \"" << option.name << "\" in config.xml is undefined."; + // return; + //} - try { - option.SetFromString(option_value); - } catch (const std::exception& e) { - Logger().errorStream() << "OptionsDB::SetFromXMLRecursive() : while processing config.xml the following exception was caught when attemptimg to set option \"" << option_name << "\": " << e.what(); - return; + if (option.flag) { + option.value = true; + } else { + try { + option.SetFromString(elem.Text()); + } catch (const std::exception& e) { + Logger().errorStream() << "OptionsDB::SetFromXMLRecursive() : while processing config.xml the following exception was caught when attemptimg to set option \"" << option_name << "\": " << e.what(); + } } } - - for (int i = 0; i < elem.NumChildren(); ++i) - SetFromXMLRecursive(elem.Child(i), option_name); } Modified: trunk/FreeOrion/util/OptionsDB.h =================================================================== --- trunk/FreeOrion/util/OptionsDB.h 2012-10-22 02:31:00 UTC (rev 5326) +++ trunk/FreeOrion/util/OptionsDB.h 2012-10-22 05:55:45 UTC (rev 5327) @@ -170,7 +170,7 @@ { if (m_options.find(name) != m_options.end()) throw std::runtime_error("OptionsDB::Add<>() : Option " + name + " was specified twice."); - m_options[name] = Option(static_cast<char>(0), name, default_value, default_value, description, validator.Clone(), storable); + m_options[name] = Option(static_cast<char>(0), name, default_value, default_value, description, validator.Clone(), storable, false); OptionAddedSignal(name); } @@ -182,7 +182,7 @@ { if (m_options.find(name) != m_options.end()) throw std::runtime_error("OptionsDB::Add<>() : Option " + name + " was specified twice."); - m_options[name] = Option(short_name, name, default_value, default_value, description, validator.Clone(), storable); + m_options[name] = Option(short_name, name, default_value, default_value, description, validator.Clone(), storable, false); OptionAddedSignal(name); } @@ -195,7 +195,7 @@ if (m_options.find(name) != m_options.end()) throw std::runtime_error("OptionsDB::AddFlag<>() : Option " + name + " was specified twice."); m_options[name] = Option(static_cast<char>(0), name, false, boost::lexical_cast<std::string>(false), - description, 0, storable); + description, 0, storable, true); OptionAddedSignal(name); } @@ -208,7 +208,7 @@ if (m_options.find(name) != m_options.end()) throw std::runtime_error("OptionsDB::AddFlag<>() : Option " + name + " was specified twice."); m_options[name] = Option(short_name, name, false, boost::lexical_cast<std::string>(false), - description, 0, storable); + description, 0, storable, true); OptionAddedSignal(name); } @@ -241,7 +241,7 @@ Option(); Option(char short_name_, const std::string& name_, const boost::any& value_, const boost::any& default_value_, const std::string& description_, - const ValidatorBase *validator_, bool storable_); + const ValidatorBase *validator_, bool storable_, bool flag_); void SetFromString(const std::string& str); std::string ValueToString() const; @@ -255,6 +255,7 @@ boost::shared_ptr<const ValidatorBase> validator; ///< a validator for the option. Flags have no validators; lexical_cast boolean conversions are done for them. bool storable; ///< whether this option can be stored in an XML config file for use across multiple runs + bool flag; mutable boost::shared_ptr<boost::signal<void ()> > option_changed_sig_ptr; |