merge

3 files changed, 122 insertions, 64 deletions

diff --git a/indra/llxml/llcontrol.cpp b/indra/llxml/llcontrol.cpp
index 36ee22e73e..90ed279b29 100755
--- a/indra/llxml/llcontrol.cpp
+++ b/indra/llxml/llcontrol.cpp
@@ -132,14 +132,14 @@ bool LLControlVariable::llsd_compare(const LLSD& a, const LLSD & b)
 
 LLControlVariable::LLControlVariable(const std::string& name, eControlType type,
 							 LLSD initial, const std::string& comment,
-							 bool persist, bool hidefromsettingseditor)
+							 ePersist persist, bool hidefromsettingseditor)
 	: mName(name),
 	  mComment(comment),
 	  mType(type),
 	  mPersist(persist),
 	  mHideFromSettingsEditor(hidefromsettingseditor)
 {
-	if (mPersist && mComment.empty())
+	if ((persist != PERSIST_NO) && mComment.empty())
 	{
 		LL_ERRS() << "Must supply a comment for control " << mName << LL_ENDL;
 	}
@@ -260,7 +260,7 @@ void LLControlVariable::setDefaultValue(const LLSD& value)
 	}
 }
 
-void LLControlVariable::setPersist(bool state)
+void LLControlVariable::setPersist(ePersist state)
 {
 	mPersist = state;
 }
@@ -292,10 +292,29 @@ void LLControlVariable::resetToDefault(bool fire_signal)
 	}
 }
 
-bool LLControlVariable::isSaveValueDefault()
-{ 
-    return (mValues.size() ==  1) 
-        || ((mValues.size() > 1) && llsd_compare(mValues[1], mValues[0]));
+bool LLControlVariable::shouldSave(bool nondefault_only)
+{
+	// This method is used to decide whether we should save a given
+	// variable. Two of the three values of mPersist are easy.
+	if (mPersist == PERSIST_NO)
+		return false;
+
+	if (mPersist == PERSIST_ALWAYS)
+		return true;
+
+	// PERSIST_NONDFT
+	// If caller doesn't need us to filter, just save.
+	if (! nondefault_only)
+		return true;
+
+	// PERSIST_NONDFT: caller only wants us to save this variable if its value
+	// differs from default.
+	if (isDefault())                // never been altered
+		return false;
+
+	// We've set at least one other value: compare it to default. Save only if
+	// they differ.
+	return ! llsd_compare(getSaveValue(), getDefault());
 }
 
 LLSD LLControlVariable::getSaveValue() const
@@ -355,12 +374,12 @@ std::string LLControlGroup::typeEnumToString(eControlType typeenum)
 	return mTypeString[typeenum];
 }
 
-BOOL LLControlGroup::declareControl(const std::string& name, eControlType type, const LLSD initial_val, const std::string& comment, BOOL persist, BOOL hidefromsettingseditor)
+LLControlVariable* LLControlGroup::declareControl(const std::string& name, eControlType type, const LLSD initial_val, const std::string& comment, LLControlVariable::ePersist persist, BOOL hidefromsettingseditor)
 {
 	LLControlVariable* existing_control = getControl(name);
 	if (existing_control)
  	{
-		if (persist && existing_control->isType(type))
+		if ((persist != LLControlVariable::PERSIST_NO) && existing_control->isType(type))
 		{
 			if (!existing_control->llsd_compare(existing_control->getDefault(), initial_val))
 			{
@@ -374,66 +393,66 @@ BOOL LLControlGroup::declareControl(const std::string& name, eControlType type,
 		{
 			LL_WARNS() << "Control named " << name << " already exists, ignoring new declaration." << LL_ENDL;
 		}
- 		return TRUE;
+ 		return existing_control;
 	}
 
 	// if not, create the control and add it to the name table
 	LLControlVariable* control = new LLControlVariable(name, type, initial_val, comment, persist, hidefromsettingseditor);
 	mNameTable[name] = control;	
-	return TRUE;
+	return control;
 }
 
-BOOL LLControlGroup::declareU32(const std::string& name, const U32 initial_val, const std::string& comment, BOOL persist)
+LLControlVariable* LLControlGroup::declareU32(const std::string& name, const U32 initial_val, const std::string& comment, LLControlVariable::ePersist persist)
 {
 	return declareControl(name, TYPE_U32, (LLSD::Integer) initial_val, comment, persist);
 }
 
-BOOL LLControlGroup::declareS32(const std::string& name, const S32 initial_val, const std::string& comment, BOOL persist)
+LLControlVariable* LLControlGroup::declareS32(const std::string& name, const S32 initial_val, const std::string& comment, LLControlVariable::ePersist persist)
 {
 	return declareControl(name, TYPE_S32, initial_val, comment, persist);
 }
 
-BOOL LLControlGroup::declareF32(const std::string& name, const F32 initial_val, const std::string& comment, BOOL persist)
+LLControlVariable* LLControlGroup::declareF32(const std::string& name, const F32 initial_val, const std::string& comment, LLControlVariable::ePersist persist)
 {
 	return declareControl(name, TYPE_F32, initial_val, comment, persist);
 }
 
-BOOL LLControlGroup::declareBOOL(const std::string& name, const BOOL initial_val, const std::string& comment, BOOL persist)
+LLControlVariable* LLControlGroup::declareBOOL(const std::string& name, const BOOL initial_val, const std::string& comment, LLControlVariable::ePersist persist)
 {
 	return declareControl(name, TYPE_BOOLEAN, initial_val, comment, persist);
 }
 
-BOOL LLControlGroup::declareString(const std::string& name, const std::string& initial_val, const std::string& comment, BOOL persist)
+LLControlVariable* LLControlGroup::declareString(const std::string& name, const std::string& initial_val, const std::string& comment, LLControlVariable::ePersist persist)
 {
 	return declareControl(name, TYPE_STRING, initial_val, comment, persist);
 }
 
-BOOL LLControlGroup::declareVec3(const std::string& name, const LLVector3 &initial_val, const std::string& comment, BOOL persist)
+LLControlVariable* LLControlGroup::declareVec3(const std::string& name, const LLVector3 &initial_val, const std::string& comment, LLControlVariable::ePersist persist)
 {
 	return declareControl(name, TYPE_VEC3, initial_val.getValue(), comment, persist);
 }
 
-BOOL LLControlGroup::declareVec3d(const std::string& name, const LLVector3d &initial_val, const std::string& comment, BOOL persist)
+LLControlVariable* LLControlGroup::declareVec3d(const std::string& name, const LLVector3d &initial_val, const std::string& comment, LLControlVariable::ePersist persist)
 {
 	return declareControl(name, TYPE_VEC3D, initial_val.getValue(), comment, persist);
 }
 
-BOOL LLControlGroup::declareRect(const std::string& name, const LLRect &initial_val, const std::string& comment, BOOL persist)
+LLControlVariable* LLControlGroup::declareRect(const std::string& name, const LLRect &initial_val, const std::string& comment, LLControlVariable::ePersist persist)
 {
 	return declareControl(name, TYPE_RECT, initial_val.getValue(), comment, persist);
 }
 
-BOOL LLControlGroup::declareColor4(const std::string& name, const LLColor4 &initial_val, const std::string& comment, BOOL persist )
+LLControlVariable* LLControlGroup::declareColor4(const std::string& name, const LLColor4 &initial_val, const std::string& comment, LLControlVariable::ePersist persist )
 {
 	return declareControl(name, TYPE_COL4, initial_val.getValue(), comment, persist);
 }
 
-BOOL LLControlGroup::declareColor3(const std::string& name, const LLColor3 &initial_val, const std::string& comment, BOOL persist )
+LLControlVariable* LLControlGroup::declareColor3(const std::string& name, const LLColor3 &initial_val, const std::string& comment, LLControlVariable::ePersist persist )
 {
 	return declareControl(name, TYPE_COL3, initial_val.getValue(), comment, persist);
 }
 
-BOOL LLControlGroup::declareLLSD(const std::string& name, const LLSD &initial_val, const std::string& comment, BOOL persist )
+LLControlVariable* LLControlGroup::declareLLSD(const std::string& name, const LLSD &initial_val, const std::string& comment, LLControlVariable::ePersist persist )
 {
 	return declareControl(name, TYPE_LLSD, initial_val, comment, persist);
 }
@@ -664,11 +683,11 @@ U32 LLControlGroup::loadFromFileLegacy(const std::string& filename, BOOL require
 			switch(declare_as)
 			{
 			case TYPE_COL4:
-				declareColor4(name, LLColor4::white, LLStringUtil::null, NO_PERSIST);
+				declareColor4(name, LLColor4::white, LLStringUtil::null, LLControlVariable::PERSIST_NO);
 				break;
 			case TYPE_STRING:
 			default:
-				declareString(name, LLStringUtil::null, LLStringUtil::null, NO_PERSIST);
+				declareString(name, LLStringUtil::null, LLStringUtil::null, LLControlVariable::PERSIST_NO);
 				break;
 			}
 		}
@@ -805,21 +824,12 @@ U32 LLControlGroup::saveToFile(const std::string& filename, BOOL nondefault_only
 		{
 			LL_WARNS() << "Tried to save invalid control: " << iter->first << LL_ENDL;
 		}
-
-		if( control && control->isPersisted() )
+		else if( control->shouldSave(nondefault_only) )
 		{
-			if (!(nondefault_only && (control->isSaveValueDefault())))
-			{
-				settings[iter->first]["Type"] = typeEnumToString(control->type());
-				settings[iter->first]["Comment"] = control->getComment();
-				settings[iter->first]["Value"] = control->getSaveValue();
-				++num_saved;
-			}
-			else
-			{
-				// Debug spam
-				// LL_INFOS() << "Skipping " << control->getName() << LL_ENDL;
-			}
+			settings[iter->first]["Type"] = typeEnumToString(control->type());
+			settings[iter->first]["Comment"] = control->getComment();
+			settings[iter->first]["Value"] = control->getSaveValue();
+			++num_saved;
 		}
 	}
 	llofstream file;
@@ -862,13 +872,14 @@ U32 LLControlGroup::loadFromFile(const std::string& filename, bool set_default_v
 	
 	for(LLSD::map_const_iterator itr = settings.beginMap(); itr != settings.endMap(); ++itr)
 	{
-		bool persist = true;
+		LLControlVariable::ePersist persist = LLControlVariable::PERSIST_NONDFT;
 		std::string const & name = itr->first;
 		LLSD const & control_map = itr->second;
 		
 		if(control_map.has("Persist")) 
 		{
-			persist = control_map["Persist"].asInteger();
+			persist = control_map["Persist"].asInteger()?
+					  LLControlVariable::PERSIST_NONDFT : LLControlVariable::PERSIST_NO;
 		}
 		
 		// Sometimes we want to use the settings system to provide cheap persistence, but we
@@ -887,6 +898,8 @@ U32 LLControlGroup::loadFromFile(const std::string& filename, bool set_default_v
 		LLControlVariable* existing_control = getControl(name);
 		if(existing_control)
 		{
+			// set_default_values is true when we're loading the initial,
+			// immutable files from app_settings, e.g. settings.xml.
 			if(set_default_values)
 			{
 				// Override all previously set properties of this control.
@@ -908,6 +921,9 @@ U32 LLControlGroup::loadFromFile(const std::string& filename, bool set_default_v
 			}
 			else if(existing_control->isPersisted())
 			{
+				// save_values is specifically false for (e.g.)
+				// SessionSettingsFile and UserSessionSettingsFile -- in other
+				// words, for a file that's supposed to be transient.
 				existing_control->setValue(control_map["Value"], save_values);
 			}
 			// *NOTE: If not persisted and not setting defaults, 
@@ -915,6 +931,39 @@ U32 LLControlGroup::loadFromFile(const std::string& filename, bool set_default_v
 		}
 		else
 		{
+			// We've never seen this control before. Either we're loading up
+			// the initial set of default settings files (set_default_values)
+			// -- or we're loading user settings last saved by a viewer that
+			// supports a superset of the variables we know.
+			// CHOP-962: if we're loading an unrecognized user setting, make
+			// sure we save it later. If you try an experimental viewer, tweak
+			// a new setting, briefly revert to an old viewer, then return to
+			// the new one, we don't want the old viewer to discard the
+			// setting you changed.
+			if (! set_default_values)
+			{
+				// Using PERSIST_ALWAYS insists that saveToFile() (which calls
+				// LLControlVariable::shouldSave()) must save this control
+				// variable regardless of its value. We can safely set this
+				// LLControlVariable persistent because the 'persistent' flag
+				// is not itself persisted!
+				persist = LLControlVariable::PERSIST_ALWAYS;
+				// We want to mention unrecognized user settings variables
+				// (e.g. from a newer version of the viewer) in the log. But
+				// we also arrive here for Boolean variables generated by
+				// the notifications subsystem when the user checks "Don't
+				// show me this again." These aren't declared in settings.xml;
+				// they're actually named for the notification they suppress.
+				// We don't want to mention those. Apologies, this is a bit of
+				// a hack: we happen to know that user settings go into an
+				// LLControlGroup whose name is "Global".
+				if (getKey() == "Global")
+				{
+					LL_INFOS("LLControlGroup") << "preserving unrecognized " << getKey()
+											   << " settings variable " << name << LL_ENDL;
+				}
+			}
+
 			declareControl(name, 
 						   typeStringToEnum(control_map["Type"].asString()), 
 						   control_map["Value"], 
@@ -923,7 +972,7 @@ U32 LLControlGroup::loadFromFile(const std::string& filename, bool set_default_v
 						   hidefromsettingseditor
 						   );
 		}
-		
+
 		++validitems;
 	}
 
diff --git a/indra/llxml/llcontrol.h b/indra/llxml/llcontrol.h
index fd2b2fbccd..6c780ecbcc 100755
--- a/indra/llxml/llcontrol.h
+++ b/indra/llxml/llcontrol.h
@@ -70,8 +70,6 @@ class LLVector3d;
 class LLColor4;
 class LLColor3;
 
-const BOOL NO_PERSIST = FALSE;
-
 typedef enum e_control_type
 {
 	TYPE_U32 = 0,
@@ -98,21 +96,28 @@ public:
 	typedef boost::signals2::signal<bool(LLControlVariable* control, const LLSD&), boost_boolean_combiner> validate_signal_t;
 	typedef boost::signals2::signal<void(LLControlVariable* control, const LLSD&, const LLSD&)> commit_signal_t;
 
+	enum ePersist
+	{
+		PERSIST_NO,                 // don't save this var
+		PERSIST_NONDFT,             // save this var if differs from default
+		PERSIST_ALWAYS              // save this var even if has default value
+	};
+
 private:
 	std::string		mName;
 	std::string		mComment;
 	eControlType	mType;
-	bool			mPersist;
+	ePersist		mPersist;
 	bool			mHideFromSettingsEditor;
 	std::vector<LLSD> mValues;
-	
+
 	commit_signal_t mCommitSignal;
 	validate_signal_t mValidateSignal;
 	
 public:
 	LLControlVariable(const std::string& name, eControlType type,
 					  LLSD initial, const std::string& comment,
-					  bool persist = true, bool hidefromsettingseditor = false);
+					  ePersist persist = PERSIST_NONDFT, bool hidefromsettingseditor = false);
 
 	virtual ~LLControlVariable();
 	
@@ -129,8 +134,8 @@ public:
 	validate_signal_t* getValidateSignal() { return &mValidateSignal; }
 
 	bool isDefault() { return (mValues.size() == 1); }
-	bool isSaveValueDefault();
-	bool isPersisted() { return mPersist; }
+	bool shouldSave(bool nondefault_only);
+	bool isPersisted() { return mPersist != PERSIST_NO; }
 	bool isHiddenFromSettingsEditor() { return mHideFromSettingsEditor; }
 	LLSD get()			const	{ return getValue(); }
 	LLSD getValue()		const	{ return mValues.back(); }
@@ -140,7 +145,7 @@ public:
 	void set(const LLSD& val)	{ setValue(val); }
 	void setValue(const LLSD& value, bool saved_value = TRUE);
 	void setDefaultValue(const LLSD& value);
-	void setPersist(bool state);
+	void setPersist(ePersist);
 	void setHiddenFromSettingsEditor(bool hide);
 	void setComment(const std::string& comment);
 
@@ -205,19 +210,19 @@ public:
 		virtual void apply(const std::string& name, LLControlVariable* control) = 0;
 	};
 	void applyToAll(ApplyFunctor* func);
-	
-	BOOL declareControl(const std::string& name, eControlType type, const LLSD initial_val, const std::string& comment, BOOL persist, BOOL hidefromsettingseditor = FALSE);
-	BOOL declareU32(const std::string& name, U32 initial_val, const std::string& comment, BOOL persist = TRUE);
-	BOOL declareS32(const std::string& name, S32 initial_val, const std::string& comment, BOOL persist = TRUE);
-	BOOL declareF32(const std::string& name, F32 initial_val, const std::string& comment, BOOL persist = TRUE);
-	BOOL declareBOOL(const std::string& name, BOOL initial_val, const std::string& comment, BOOL persist = TRUE);
-	BOOL declareString(const std::string& name, const std::string &initial_val, const std::string& comment, BOOL persist = TRUE);
-	BOOL declareVec3(const std::string& name, const LLVector3 &initial_val,const std::string& comment,  BOOL persist = TRUE);
-	BOOL declareVec3d(const std::string& name, const LLVector3d &initial_val, const std::string& comment, BOOL persist = TRUE);
-	BOOL declareRect(const std::string& name, const LLRect &initial_val, const std::string& comment, BOOL persist = TRUE);
-	BOOL declareColor4(const std::string& name, const LLColor4 &initial_val, const std::string& comment, BOOL persist = TRUE);
-	BOOL declareColor3(const std::string& name, const LLColor3 &initial_val, const std::string& comment, BOOL persist = TRUE);
-	BOOL declareLLSD(const std::string& name, const LLSD &initial_val, const std::string& comment, BOOL persist = TRUE);
+
+	LLControlVariable* declareControl(const std::string& name, eControlType type, const LLSD initial_val, const std::string& comment, LLControlVariable::ePersist persist, BOOL hidefromsettingseditor = FALSE);
+	LLControlVariable* declareU32(const std::string& name, U32 initial_val, const std::string& comment, LLControlVariable::ePersist persist = LLControlVariable::PERSIST_NONDFT);
+	LLControlVariable* declareS32(const std::string& name, S32 initial_val, const std::string& comment, LLControlVariable::ePersist persist = LLControlVariable::PERSIST_NONDFT);
+	LLControlVariable* declareF32(const std::string& name, F32 initial_val, const std::string& comment, LLControlVariable::ePersist persist = LLControlVariable::PERSIST_NONDFT);
+	LLControlVariable* declareBOOL(const std::string& name, BOOL initial_val, const std::string& comment, LLControlVariable::ePersist persist = LLControlVariable::PERSIST_NONDFT);
+	LLControlVariable* declareString(const std::string& name, const std::string &initial_val, const std::string& comment, LLControlVariable::ePersist persist = LLControlVariable::PERSIST_NONDFT);
+	LLControlVariable* declareVec3(const std::string& name, const LLVector3 &initial_val,const std::string& comment,  LLControlVariable::ePersist persist = LLControlVariable::PERSIST_NONDFT);
+	LLControlVariable* declareVec3d(const std::string& name, const LLVector3d &initial_val, const std::string& comment, LLControlVariable::ePersist persist = LLControlVariable::PERSIST_NONDFT);
+	LLControlVariable* declareRect(const std::string& name, const LLRect &initial_val, const std::string& comment, LLControlVariable::ePersist persist = LLControlVariable::PERSIST_NONDFT);
+	LLControlVariable* declareColor4(const std::string& name, const LLColor4 &initial_val, const std::string& comment, LLControlVariable::ePersist persist = LLControlVariable::PERSIST_NONDFT);
+	LLControlVariable* declareColor3(const std::string& name, const LLColor3 &initial_val, const std::string& comment, LLControlVariable::ePersist persist = LLControlVariable::PERSIST_NONDFT);
+	LLControlVariable* declareLLSD(const std::string& name, const LLSD &initial_val, const std::string& comment, LLControlVariable::ePersist persist = LLControlVariable::PERSIST_NONDFT);
 
 	std::string getString(const std::string& name);
 	std::string getText(const std::string& name);
@@ -366,7 +371,7 @@ private:
 		init_value = convert_to_llsd(default_value);
 		if(type < TYPE_COUNT)
 		{
-			group.declareControl(name, type, init_value, comment, FALSE);
+			group.declareControl(name, type, init_value, comment, LLControlVariable::PERSIST_NO);
 			return true;
 		}
 		return false;
diff --git a/indra/llxml/tests/llcontrol_test.cpp b/indra/llxml/tests/llcontrol_test.cpp
index ede81956ec..c273773c9b 100755
--- a/indra/llxml/tests/llcontrol_test.cpp
+++ b/indra/llxml/tests/llcontrol_test.cpp
@@ -128,7 +128,11 @@ namespace tut
 	template<> template<>
 	void control_group_t::test<3>()
 	{
-		int results = mCG->loadFromFile(mTestConfigFile.c_str());
+		// Pass default_values = true. This tells loadFromFile() we're loading
+		// a default settings file that declares variables, rather than a user
+		// settings file. When loadFromFile() encounters an unrecognized user
+		// settings variable, it forcibly preserves it (CHOP-962).
+		int results = mCG->loadFromFile(mTestConfigFile.c_str(), true);
 		LLControlVariable* control = mCG->getControl("TestSetting");
 		LLSD new_value = 13;
 		control->setValue(new_value, FALSE);