From fd14c250b8aa7b2d512194b164646ff7f658bef3 Mon Sep 17 00:00:00 2001
From: Nat Goodspeed <nat@lindenlab.com>
Date: Wed, 17 Jul 2013 11:30:09 -0400
Subject: [PATCH] CHOP-962: Preserve unrecognized user settings variables.
Change LLControlVariable::mPersist from bool to tri-state enum. Its 'true'
value used to mean "persist only if changed from default;" third state now
means "persist regardless of value." Add forcePersist() method to set that.
Replace isSaveValueDefault() method -- used only by logic to determine
whether to save the variable -- with shouldSave() method to encapsulate ALL
that logic rather than only part of it. shouldSave() recognizes
PERSIST_ALWAYS state. When loading an unrecognized control variable from a
user settings file, use forcePersist() to ensure that we later save that
variable again. Tweak one of the unit tests to adjust for new semantics.
---
indra/llxml/llcontrol.cpp | 81 ++++++++++++++++++++--------
indra/llxml/llcontrol.h | 17 ++++--
indra/llxml/tests/llcontrol_test.cpp | 6 ++-
3 files changed, 75 insertions(+), 29 deletions(-)
diff --git a/indra/llxml/llcontrol.cpp b/indra/llxml/llcontrol.cpp
index 53550d64095..9dc6292cb27 100755
--- a/indra/llxml/llcontrol.cpp
+++ b/indra/llxml/llcontrol.cpp
@@ -136,10 +136,10 @@ LLControlVariable::LLControlVariable(const std::string& name, eControlType type,
: mName(name),
mComment(comment),
mType(type),
- mPersist(persist),
+ mPersist(persist? PERSIST_YES : PERSIST_NO),
mHideFromSettingsEditor(hidefromsettingseditor)
{
- if (mPersist && mComment.empty())
+ if (persist && mComment.empty())
{
llerrs << "Must supply a comment for control " << mName << llendl;
}
@@ -262,7 +262,12 @@ void LLControlVariable::setDefaultValue(const LLSD& value)
void LLControlVariable::setPersist(bool state)
{
- mPersist = state;
+ mPersist = state? PERSIST_YES : PERSIST_NO;
+}
+
+void LLControlVariable::forcePersist()
+{
+ mPersist = PERSIST_ALWAYS;
}
void LLControlVariable::setHiddenFromSettingsEditor(bool hide)
@@ -292,10 +297,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_YES
+ // If caller doesn't need us to filter, just save.
+ if (! nondefault_only)
+ return true;
+
+ // PERSIST_YES, but 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
@@ -805,21 +829,12 @@ U32 LLControlGroup::saveToFile(const std::string& filename, BOOL nondefault_only
{
llwarns << "Tried to save invalid control: " << iter->first << llendl;
}
-
- 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
- // llinfos << "Skipping " << control->getName() << llendl;
- }
+ settings[iter->first]["Type"] = typeEnumToString(control->type());
+ settings[iter->first]["Comment"] = control->getComment();
+ settings[iter->first]["Value"] = control->getSaveValue();
+ ++num_saved;
}
}
llofstream file;
@@ -920,15 +935,35 @@ U32 LLControlGroup::loadFromFile(const std::string& filename, bool set_default_v
}
else
{
- declareControl(name,
+ // 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.
+ LLControlVariable* control = declareControl(name,
typeStringToEnum(control_map["Type"].asString()),
control_map["Value"],
control_map["Comment"].asString(),
persist,
hidefromsettingseditor
);
+ // 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 forcePersist() 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!
+ control->forcePersist();
+ LL_INFOS("LLControlGroup") << "preserving unrecognized user settings variable "
+ << name << LL_ENDL;
+ }
}
-
+
++validitems;
}
diff --git a/indra/llxml/llcontrol.h b/indra/llxml/llcontrol.h
index a7424c6780e..5d6ac33780b 100755
--- a/indra/llxml/llcontrol.h
+++ b/indra/llxml/llcontrol.h
@@ -101,13 +101,19 @@ class LLControlVariable : public LLRefCount
typedef boost::signals2::signal<void(LLControlVariable* control, const LLSD&, const LLSD&)> commit_signal_t;
private:
+ enum
+ {
+ PERSIST_NO, // don't save this var
+ PERSIST_YES, // save this var if differs from default
+ PERSIST_ALWAYS // save this var even if has default value
+ } mPersist;
+
std::string mName;
std::string mComment;
eControlType mType;
- bool mPersist;
bool mHideFromSettingsEditor;
std::vector<LLSD> mValues;
-
+
commit_signal_t mCommitSignal;
validate_signal_t mValidateSignal;
@@ -131,8 +137,8 @@ class LLControlVariable : public LLRefCount
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(); }
@@ -142,7 +148,8 @@ class LLControlVariable : public LLRefCount
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(bool state); // persist or not depending on value
+ void forcePersist(); // always persist regardless of value
void setHiddenFromSettingsEditor(bool hide);
void setComment(const std::string& comment);
diff --git a/indra/llxml/tests/llcontrol_test.cpp b/indra/llxml/tests/llcontrol_test.cpp
index ede81956ec6..c273773c9b4 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);
--
GitLab