Change the in-memory storage of options to a sorted vector

build/libaegisub/libaegisub.vcxproj

@@ -56,7 +56,6 @@
   <!-- Source files -->
   <ItemGroup>
     <ClInclude Include="$(SrcDir)common\charset_6937.h" />
-    <ClInclude Include="$(SrcDir)common\option_visit.h" />
     <ClInclude Include="$(SrcDir)common\parser.h" />
     <ClInclude Include="$(SrcDir)config.h" />
     <ClInclude Include="$(SrcDir)include\libaegisub\access.h" />
@@ -141,7 +140,6 @@
     <ClCompile Include="$(SrcDir)common\log.cpp" />
     <ClCompile Include="$(SrcDir)common\mru.cpp" />
     <ClCompile Include="$(SrcDir)common\option.cpp" />
-    <ClCompile Include="$(SrcDir)common\option_visit.cpp" />
     <ClCompile Include="$(SrcDir)common\parser.cpp" />
     <ClCompile Include="$(SrcDir)common\path.cpp" />
     <ClCompile Include="$(SrcDir)common\thesaurus.cpp" />

build/libaegisub/libaegisub.vcxproj.filters

@@ -32,9 +32,6 @@
     <ClInclude Include="$(SrcDir)common\charset_6937.h">
       <Filter>Header Files</Filter>
     </ClInclude>
-    <ClInclude Include="$(SrcDir)common\option_visit.h">
-      <Filter>Header Files</Filter>
-    </ClInclude>
     <ClInclude Include="$(SrcDir)config.h">
       <Filter>Header Files</Filter>
     </ClInclude>
@@ -235,9 +232,6 @@
     <ClCompile Include="$(SrcDir)common\option.cpp">
       <Filter>Source Files\Common</Filter>
     </ClCompile>
-    <ClCompile Include="$(SrcDir)common\option_visit.cpp">
-      <Filter>Source Files\Common</Filter>
-    </ClCompile>
     <ClCompile Include="$(SrcDir)common\thesaurus.cpp">
       <Filter>Source Files\Common</Filter>
     </ClCompile>

libaegisub/Makefile

@@ -26,7 +26,6 @@ aegisub_OBJ := \
 	$(d)common/log.o \
 	$(d)common/mru.o \
 	$(d)common/option.o \
-	$(d)common/option_visit.o \
 	$(d)common/path.o \
 	$(d)common/thesaurus.o \
 	$(d)common/util.o \

libaegisub/common/option.cpp

@@ -21,45 +21,165 @@
 #include "libaegisub/cajun/reader.h"
 #include "libaegisub/cajun/writer.h"
 #include "libaegisub/cajun/elements.h"
+#include "libaegisub/cajun/visitor.h"
 
 #include "libaegisub/exception.h"
 #include "libaegisub/fs.h"
 #include "libaegisub/io.h"
 #include "libaegisub/log.h"
 #include "libaegisub/option_value.h"
+#include "libaegisub/make_unique.h"
 
-#include "option_visit.h"
+#include <boost/algorithm/string/predicate.hpp>
-
 #include <boost/interprocess/streams/bufferstream.hpp>
 #include <cassert>
 #include <memory>
 
 namespace {
-	/// @brief Write an option to a json object
+using namespace agi;
-	/// @param[out] obj  Parent object
+
-	/// @param[in] path  Path option should be stored in.
+DEFINE_EXCEPTION(OptionJsonValueError, Exception);
-	/// @param[in] value Value to write.
+
-	void put_option(json::Object &obj, const std::string &path, const json::UnknownElement &value) {
+class ConfigVisitor final : public json::ConstVisitor {
-		std::string::size_type pos = path.find('/');
+	std::vector<std::unique_ptr<OptionValue>> values;
-		// Not having a '/' denotes it is a leaf.
+
-		if (pos == std::string::npos) {
+	/// Option name prefix to add to read names
-			assert(obj.find(path) == obj.end());
+	std::string name;
-			obj[path] = value;
+
-		}
+	/// Log errors rather than throwing them, for when loading user config files
+	/// (as a bad user config file shouldn't make the program fail to start)
+	bool ignore_errors;
+
+	void Error(const char *message) {
+		if (ignore_errors)
+			LOG_E("option/load/config_visitor") << "Error loading option from user configuration: " << message;
 		else
-			put_option(obj[path.substr(0, pos)], path.substr(pos + 1), value);
+			throw OptionJsonValueError(message);
 	}
 
-	template<class T>
+	template<class OptionValueType>
-	void put_array(json::Object &obj, const std::string &path, const char *element_key, std::vector<T> const& value) {
+	void ReadArray(json::Array const& src, std::string const& array_type) {
-		json::Array array;
+		typename OptionValueType::value_type arr;
-		for (T const& elem : value) {
+		arr.reserve(src.size());
-			array.push_back(json::Object());
+
-			static_cast<json::Object&>(array.back())[element_key] = (json::UnknownElement)elem;
+		for (json::Object const& obj : src) {
+			if (obj.size() != 1)
+				return Error("Invalid array member");
+			if (obj.begin()->first != array_type)
+				return Error("Attempt to insert value into array of wrong type");
+
+			arr.push_back((typename OptionValueType::value_type::value_type)(obj.begin()->second));
 		}
 
-		put_option(obj, path, array);
+		values.push_back(agi::make_unique<OptionValueType>(name, std::move(arr)));
 	}
+
+	void Visit(const json::Object& object) {
+		auto old_name = name;
+		for (auto const& obj : object) {
+			name = old_name + (name.empty() ? "" : "/") + obj.first;
+			obj.second.Accept(*this);
+		}
+		name = old_name;
+	}
+
+	void Visit(const json::Array& array) {
+		if (array.empty())
+			return Error("Cannot infer the type of an empty array");
+
+		json::Object const& front = array.front();
+		if (front.size() != 1)
+			return Error("Invalid array member");
+
+		auto const& array_type = front.begin()->first;
+		if (array_type == "string")
+			ReadArray<OptionValueListString>(array, array_type);
+		else if (array_type == "int")
+			ReadArray<OptionValueListInt>(array, array_type);
+		else if (array_type == "double")
+			ReadArray<OptionValueListDouble>(array, array_type);
+		else if (array_type == "bool")
+			ReadArray<OptionValueListBool>(array, array_type);
+		else if (array_type == "color")
+			ReadArray<OptionValueListColor>(array, array_type);
+		else
+			Error("Array type not handled");
+	}
+
+	void Visit(const json::Integer& number) {
+		values.push_back(agi::make_unique<OptionValueInt>(name, number));
+	}
+
+	void Visit(const json::Double& number) {
+		values.push_back(agi::make_unique<OptionValueDouble>(name, number));
+	}
+
+	void Visit(const json::String& string) {
+		size_t size = string.size();
+		if ((size == 4 && string[0] == '#') ||
+			(size == 7 && string[0] == '#') ||
+			(size >= 10 && boost::starts_with(string, "rgb(")) ||
+			((size == 9 || size == 10) && boost::starts_with(string, "&H")))
+		{
+			values.push_back(agi::make_unique<OptionValueColor>(name, string));
+		} else {
+			values.push_back(agi::make_unique<OptionValueString>(name, string));
+		}
+	}
+
+	void Visit(const json::Boolean& boolean) {
+		values.push_back(agi::make_unique<OptionValueBool>(name, boolean));
+	}
+
+	void Visit(const json::Null& null) {
+		Error("Attempt to read null value");
+	}
+
+public:
+	ConfigVisitor(bool ignore_errors) : ignore_errors(ignore_errors) { }
+	std::vector<std::unique_ptr<OptionValue>> Values() { return std::move(values); }
+};
+
+/// @brief Write an option to a json object
+/// @param[out] obj  Parent object
+/// @param[in] path  Path option should be stored in.
+/// @param[in] value Value to write.
+void put_option(json::Object &obj, const std::string &path, const json::UnknownElement &value) {
+	std::string::size_type pos = path.find('/');
+	// Not having a '/' denotes it is a leaf.
+	if (pos == std::string::npos) {
+		assert(obj.find(path) == obj.end());
+		obj[path] = value;
+	}
+	else
+		put_option(obj[path.substr(0, pos)], path.substr(pos + 1), value);
+}
+
+template<class T>
+void put_array(json::Object &obj, const std::string &path, const char *element_key, std::vector<T> const& value) {
+	json::Array array;
+	for (T const& elem : value) {
+		array.push_back(json::Object());
+		static_cast<json::Object&>(array.back())[element_key] = (json::UnknownElement)elem;
+	}
+
+	put_option(obj, path, array);
+}
+
+struct option_name_cmp {
+	bool operator()(std::unique_ptr<OptionValue> const& a, std::unique_ptr<OptionValue> const& b) const {
+		return a->GetName() < b->GetName();
+	}
+
+	bool operator()(std::unique_ptr<OptionValue> const& a, std::string const& b) const {
+		return a->GetName() < b;
+	}
+
+	bool operator()(std::unique_ptr<OptionValue> const& a, const char *b) const {
+		return a->GetName() < b;
+	}
+};
+
 }
 
 namespace agi {
@@ -78,19 +198,13 @@ Options::~Options() {
 		Flush();
 }
 
-void Options::ConfigNext(std::istream& stream) {
-	LoadConfig(stream);
-}
-
 void Options::ConfigUser() {
 	try {
-		std::unique_ptr<std::istream> stream(io::Open(config_file));
+		LoadConfig(*io::Open(config_file), true);
-		LoadConfig(*stream, true);
 	}
 	catch (fs::FileNotFound const&) {
 		return;
 	}
-	/// @todo Handle other errors such as parsing and notifying the user.
 }
 
 void Options::LoadConfig(std::istream& stream, bool ignore_errors) {
@@ -100,67 +214,104 @@ void Options::LoadConfig(std::istream& stream, bool ignore_errors) {
 		json::Reader::Read(config_root, stream);
 	} catch (json::Reader::ParseException& e) {
 		LOG_E("option/load") << "json::ParseException: " << e.what() << ", Line/offset: " << e.m_locTokenBegin.m_nLine + 1 << '/' << e.m_locTokenBegin.m_nLineOffset + 1;
+		return;
 	} catch (json::Exception& e) {
-		/// @todo Do something better here, maybe print the exact error
 		LOG_E("option/load") << "json::Exception: " << e.what();
+		return;
 	}
 
-	ConfigVisitor config_visitor(values, "", ignore_errors, !ignore_errors);
+	ConfigVisitor config_visitor(ignore_errors);
 	config_root.Accept(config_visitor);
+
+	auto new_values = config_visitor.Values();
+	if (new_values.empty()) return;
+
+	sort(begin(new_values), end(new_values), option_name_cmp());
+
+	if (values.empty()) {
+		values = std::move(new_values);
+		return;
+	}
+
+	auto src_it = begin(new_values), src_end = end(new_values);
+	auto dst_it = begin(values), dst_end = end(values);
+
+	while (src_it != src_end && dst_it != dst_end) {
+		int cmp = (*src_it)->GetName().compare((*dst_it)->GetName());
+		if (cmp < 0) // Option doesn't exist in defaults so skip
+			++src_it;
+		else if (cmp > 0)
+			++dst_it;
+		else {
+			if (ignore_errors) {
+				try {
+					(*dst_it)->Set((*src_it).get());
+				}
+				catch (agi::InternalError const& e) {
+					LOG_E("option/load/config_visitor") << "Error loading option from user configuration: " << e.GetMessage();
+				}
+			}
+			else {
+				*dst_it = std::move(*src_it);
+			}
+			++src_it;
+			++dst_it;
+		}
+	}
 }
 
-OptionValue* Options::Get(const std::string &name) {
+OptionValue *Options::Get(const char *name) {
-	auto index = values.find(name);
+	auto index = lower_bound(begin(values), end(values), name, option_name_cmp());
-	if (index != values.end())
+	if (index != end(values) && (*index)->GetName() == name)
-		return index->second.get();
+		return index->get();
 
 	LOG_E("option/get") << "agi::Options::Get Option not found: (" << name << ")";
-	throw agi::InternalError("Option value not found: " + name);
+	throw agi::InternalError("Option value not found: " + std::string(name));
 }
 
 void Options::Flush() const {
 	json::Object obj_out;
 
 	for (auto const& ov : values) {
-		switch (ov.second->GetType()) {
+		switch (ov->GetType()) {
 			case OptionType::String:
-				put_option(obj_out, ov.first, ov.second->GetString());
+				put_option(obj_out, ov->GetName(), ov->GetString());
 				break;
 
 			case OptionType::Int:
-				put_option(obj_out, ov.first, ov.second->GetInt());
+				put_option(obj_out, ov->GetName(), ov->GetInt());
 				break;
 
 			case OptionType::Double:
-				put_option(obj_out, ov.first, ov.second->GetDouble());
+				put_option(obj_out, ov->GetName(), ov->GetDouble());
 				break;
 
 			case OptionType::Color:
-				put_option(obj_out, ov.first, ov.second->GetColor().GetRgbFormatted());
+				put_option(obj_out, ov->GetName(), ov->GetColor().GetRgbFormatted());
 				break;
 
 			case OptionType::Bool:
-				put_option(obj_out, ov.first, ov.second->GetBool());
+				put_option(obj_out, ov->GetName(), ov->GetBool());
 				break;
 
 			case OptionType::ListString:
-				put_array(obj_out, ov.first, "string", ov.second->GetListString());
+				put_array(obj_out, ov->GetName(), "string", ov->GetListString());
 				break;
 
 			case OptionType::ListInt:
-				put_array(obj_out, ov.first, "int", ov.second->GetListInt());
+				put_array(obj_out, ov->GetName(), "int", ov->GetListInt());
 				break;
 
 			case OptionType::ListDouble:
-				put_array(obj_out, ov.first, "double", ov.second->GetListDouble());
+				put_array(obj_out, ov->GetName(), "double", ov->GetListDouble());
 				break;
 
 			case OptionType::ListColor:
-				put_array(obj_out, ov.first, "color", ov.second->GetListColor());
+				put_array(obj_out, ov->GetName(), "color", ov->GetListColor());
 				break;
 
 			case OptionType::ListBool:
-				put_array(obj_out, ov.first, "bool", ov.second->GetListBool());
+				put_array(obj_out, ov->GetName(), "bool", ov->GetListBool());
 				break;
 		}
 	}

libaegisub/common/option_visit.cpp

@@ -1,158 +0,0 @@
-// Copyright (c) 2010, Amar Takhar <verm@aegisub.org>
-//
-// Permission to use, copy, modify, and distribute this software for any
-// purpose with or without fee is hereby granted, provided that the above
-// copyright notice and this permission notice appear in all copies.
-//
-// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
-// WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
-// MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
-// ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
-// WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
-// ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
-// OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
-
-/// @file option_visit.cpp
-/// @brief Cajun JSON visitor to load config values.
-/// @see option_visit.h
-/// @ingroup libaegisub
-
-#include "option_visit.h"
-
-#include <cassert>
-
-#include <libaegisub/log.h>
-#include <libaegisub/option_value.h>
-#include <libaegisub/make_unique.h>
-
-#include <boost/algorithm/string/predicate.hpp>
-
-namespace agi {
-
-ConfigVisitor::ConfigVisitor(OptionValueMap &val, const std::string &member_name, bool ignore_errors, bool replace)
-: values(val)
-, name(member_name)
-, ignore_errors(ignore_errors)
-, replace(replace)
-{
-}
-
-void ConfigVisitor::Error(const char *message) {
-	if (ignore_errors)
-		LOG_E("option/load/config_visitor") << "Error loading option from user configuration: " << message;
-	else
-		throw OptionJsonValueError(message);
-}
-
-void ConfigVisitor::Visit(const json::Object& object) {
-	if (!name.empty())
-		name += "/";
-
-	for (auto const& obj : object) {
-		ConfigVisitor config_visitor(values, name + obj.first, ignore_errors, replace);
-		obj.second.Accept(config_visitor);
-	}
-}
-
-template<class OptionValueType>
-std::unique_ptr<OptionValue> ConfigVisitor::ReadArray(json::Array const& src, std::string const& array_type) {
-	typename OptionValueType::value_type arr;
-	arr.reserve(src.size());
-
-	for (json::Object const& obj : src) {
-		if (obj.size() != 1) {
-			Error("Invalid array member");
-			return nullptr;
-		}
-		if (obj.begin()->first != array_type) {
-			Error("Attempt to insert value into array of wrong type");
-			return nullptr;
-		}
-
-		arr.push_back((typename OptionValueType::value_type::value_type)(obj.begin()->second));
-	}
-
-	return agi::make_unique<OptionValueType>(name, std::move(arr));
-}
-
-void ConfigVisitor::Visit(const json::Array& array) {
-	if (array.empty()) {
-		Error("Cannot infer the type of an empty array");
-		return;
-	}
-
-	json::Object const& front = array.front();
-	if (front.size() != 1) {
-		Error("Invalid array member");
-		return;
-	}
-
-	const std::string& array_type = front.begin()->first;
-
-	if (array_type == "string")
-		AddOptionValue(ReadArray<OptionValueListString>(array, array_type));
-	else if (array_type == "int")
-		AddOptionValue(ReadArray<OptionValueListInt>(array, array_type));
-	else if (array_type == "double")
-		AddOptionValue(ReadArray<OptionValueListDouble>(array, array_type));
-	else if (array_type == "bool")
-		AddOptionValue(ReadArray<OptionValueListBool>(array, array_type));
-	else if (array_type == "color")
-		AddOptionValue(ReadArray<OptionValueListColor>(array, array_type));
-	else
-		Error("Array type not handled");
-}
-
-void ConfigVisitor::Visit(const json::Integer& number) {
-	AddOptionValue(agi::make_unique<OptionValueInt>(name, number));
-}
-
-void ConfigVisitor::Visit(const json::Double& number) {
-	AddOptionValue(agi::make_unique<OptionValueDouble>(name, number));
-}
-
-void ConfigVisitor::Visit(const json::String& string) {
-	size_t size = string.size();
-	if ((size == 4 && string[0] == '#') ||
-		(size == 7 && string[0] == '#') ||
-		(size >= 10 && boost::starts_with(string, "rgb(")) ||
-		((size == 9 || size == 10) && boost::starts_with(string, "&H")))
-	{
-		AddOptionValue(agi::make_unique<OptionValueColor>(name, string));
-	} else {
-		AddOptionValue(agi::make_unique<OptionValueString>(name, string));
-	}
-}
-
-void ConfigVisitor::Visit(const json::Boolean& boolean) {
-	AddOptionValue(agi::make_unique<OptionValueBool>(name, boolean));
-}
-
-void ConfigVisitor::Visit(const json::Null& null) {
-	Error("Attempt to read null value");
-}
-
-void ConfigVisitor::AddOptionValue(std::unique_ptr<OptionValue>&& opt) {
-	if (!opt) {
-		assert(ignore_errors);
-		return;
-	}
-
-	auto it = values.find(name);
-	if (it == values.end())
-		values[name] = std::move(opt);
-	else if (replace)
-		it->second = std::move(opt);
-	else {
-		try {
-			values[name]->Set(opt.get());
-		}
-		catch (agi::InternalError const& e) {
-			if (ignore_errors)
-				LOG_E("option/load/config_visitor") << "Error loading option from user configuration: " << e.GetMessage();
-			else
-				throw;
-		}
-	}
-}
-} // namespace agi

libaegisub/common/option_visit.h

@@ -1,62 +0,0 @@
-// Copyright (c) 2010, Amar Takhar <verm@aegisub.org>
-//
-// Permission to use, copy, modify, and distribute this software for any
-// purpose with or without fee is hereby granted, provided that the above
-// copyright notice and this permission notice appear in all copies.
-//
-// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
-// WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
-// MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
-// ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
-// WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
-// ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
-// OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
-
-/// @file option_visit.h
-/// @brief Cajun JSON visitor to load config values.
-/// @see option_visit.cpp
-/// @ingroup libaegisub
-
-#include "libaegisub/option.h"
-#include "libaegisub/cajun/elements.h"
-#include "libaegisub/cajun/visitor.h"
-
-#include "libaegisub/exception.h"
-
-#include <memory>
-
-namespace agi {
-
-DEFINE_EXCEPTION(OptionJsonValueError, Exception);
-
-class ConfigVisitor final : public json::ConstVisitor {
-	/// Option map being populated
-	OptionValueMap &values;
-	/// Option name prefix to add to read names
-	std::string name;
-	/// Log errors rather than throwing them, for when loading user config files
-	/// (as a bad user config file shouldn't make the program fail to start)
-	bool ignore_errors;
-	/// Replace existing options rather than changing their value, so that the
-	/// default value is changed to the new one
-	bool replace;
-
-	void Error(const char *message);
-
-	template<class OptionValueType>
-	std::unique_ptr<OptionValue> ReadArray(json::Array const& src, std::string const& array_type);
-
-	void AddOptionValue(std::unique_ptr<OptionValue>&& opt);
-public:
-	ConfigVisitor(OptionValueMap &val, const std::string &member_name, bool ignore_errors = false, bool replace = false);
-
-	void Visit(const json::Array& array);
-	void Visit(const json::Object& object);
-	void Visit(const json::Integer& number);
-	void Visit(const json::Double& number);
-	void Visit(const json::String& string);
-	void Visit(const json::Boolean& boolean);
-	void Visit(const json::Null& null);
-};
-
-} // namespace agi

libaegisub/include/libaegisub/option.h

@@ -22,6 +22,7 @@
 #include <iosfwd>
 #include <map>
 #include <memory>
+#include <vector>
 
 #include <libaegisub/fs_fwd.h>
 
@@ -33,8 +34,6 @@ namespace json {
 namespace agi {
 class OptionValue;
 
-using OptionValueMap = std::map<std::string, std::unique_ptr<OptionValue>>;
-
 class Options {
 public:
 	/// Options class settings.
@@ -44,8 +43,7 @@ public:
 	};
 
 private:
-	/// Internal OptionValueMap
+	std::vector<std::unique_ptr<OptionValue>> values;
-	OptionValueMap values;
 
 	/// User config (file that will be written to disk)
 	const agi::fs::path config_file;
@@ -74,14 +72,15 @@ public:
 	/// @brief Get an option by name.
 	/// @param name Option to get.
 	/// Get an option value object by name throw an internal exception if the option is not found.
-	OptionValue* Get(const std::string &name);
+	OptionValue *Get(const char *name);
+	OptionValue *Get(std::string const& name) { return Get(name.c_str()); }
 
 	/// @brief Next configuration file to load.
 	/// @param[in] src Stream to load from.
 	/// Load next config which will supersede any values from previous configs
 	/// can be called as many times as required, but only after ConfigDefault() and
 	/// before ConfigUser()
-	void ConfigNext(std::istream &stream);
+	void ConfigNext(std::istream &stream) { LoadConfig(stream); }
 
 	/// @brief Set user config file.
 	/// Set the user configuration file and read options from it, closes all

libaegisub/include/libaegisub/option_value.h

@@ -90,7 +90,7 @@ protected:
 public:
 	virtual ~OptionValue() = default;
 
-	std::string GetName() const { return name; }
+	std::string const& GetName() const { return name; }
 	virtual OptionType GetType() const = 0;
 	virtual bool IsDefault() const = 0;
 	virtual void Reset() = 0;

tests/tests/option.cpp

@@ -120,6 +120,16 @@ TEST_F(lagi_option, heterogeneous_arrays_rejected) {
 	EXPECT_THROW(agi::Options("", "{ \"key\" : [ { \"bool\" : true }, { \"double\" : 1.0 } ] }", agi::Options::FLUSH_SKIP), agi::Exception);
 }
 
+TEST_F(lagi_option, set_works) {
+	agi::Options opt("", all_types, agi::Options::FLUSH_SKIP);
+
+	ASSERT_NO_THROW(opt.Get("Integer")->SetInt(1000));
+	EXPECT_EQ(1000, opt.Get("Integer")->GetInt());
+
+	ASSERT_NO_THROW(opt.Get("String")->SetString("Hello"));
+	EXPECT_EQ("Hello", opt.Get("String")->GetString());
+}
+
 TEST_F(lagi_option, flush_roundtrip) {
 	agi::fs::Remove("data/options/tmp");