From 8833dbe01f2ab925b1827b539db7691ae5ae3bed Mon Sep 17 00:00:00 2001 From: Contrad2 Date: Fri, 19 Jan 2024 15:58:18 +0200 Subject: [PATCH 1/5] fixing setoption behaviour. --- src/engine.cc | 44 +++++++++++++++++++++++++++++++++++---- src/utils/optionsparser.h | 4 ++-- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/src/engine.cc b/src/engine.cc index 31f8301887..378d9ce8a8 100644 --- a/src/engine.cc +++ b/src/engine.cc @@ -30,6 +30,9 @@ #include #include #include +#include +#include + #include "mcts/search.h" #include "mcts/stoppers/factory.h" @@ -83,8 +86,20 @@ MoveList StringsToMovelist(const std::vector& moves, return result; } +std::vector SplitString(const std::string& str, char delimiter) { + std::vector tokens; + std::string token; + std::istringstream tokenStream(str); + while (std::getline(tokenStream, token, delimiter)) { + tokens.push_back(token); + } + return tokens; +} + } // namespace + + EngineController::EngineController(std::unique_ptr uci_responder, const OptionsDict& options) : options_(options), @@ -359,10 +374,31 @@ void EngineLoop::CmdIsReady() { void EngineLoop::CmdSetOption(const std::string& name, const std::string& value, const std::string& context) { - options_.SetUciOption(name, value, context); - // Set the log filename for the case it was set in UCI option. - Logging::Get().SetFilename( - options_.GetOptionsDict().Get(kLogFileId)); + if (name.empty() || value.empty()) { + throw Exception("Both name and value must be provided"); + } else { + // Split the names and values by space + std::vector names = SplitString(name, ';'); + std::vector values = SplitString(value, ';'); + // Check that the number of names and values match + if (names.size() != values.size()) { + throw Exception("Mismatched number of names and values"); + } + // Set the UCI options for each name-value pair + for (size_t i = 0; i < names.size(); ++i) { + const std::string& name = names[i]; + const std::string& value = values[i]; + // Check that the name is valid + if (!options_.FindOptionByUciName(name)) { + throw Exception("Unknown option: " + name); + } + // Set the UCI option + options_.SetUciOption(name, value, context); + // Set the log filename for the case it was set in UCI option. + Logging::Get().SetFilename( + options_.GetOptionsDict().Get(kLogFileId)); + } + } } void EngineLoop::CmdUciNewGame() { engine_.NewGame(); } diff --git a/src/utils/optionsparser.h b/src/utils/optionsparser.h index 5c470c832b..8ae89e4e02 100644 --- a/src/utils/optionsparser.h +++ b/src/utils/optionsparser.h @@ -111,6 +111,8 @@ class OptionsParser { const OptionsDict& GetOptionsDict(const std::string& context = {}); // Gets the dictionary for given context which caller can modify. OptionsDict* GetMutableOptions(const std::string& context = {}); + // Returns an option based by its uci name. + Option* FindOptionByUciName(const std::string& name) const; // Gets the mutable list of default options. OptionsDict* GetMutableDefaultsOptions() { return &defaults_; } // Adds a subdictionary for a given context. @@ -123,8 +125,6 @@ class OptionsParser { void ShowHidden() const; // Returns an option based on the long flag. Option* FindOptionByLongFlag(const std::string& flag) const; - // Returns an option based by its uci name. - Option* FindOptionByUciName(const std::string& name) const; // Returns an option based by its id. Option* FindOptionById(const OptionId& id) const; From b2af2b55b6f12992550bca24d209c343b27305ec Mon Sep 17 00:00:00 2001 From: Contrad2 Date: Fri, 19 Jan 2024 19:28:03 +0200 Subject: [PATCH 2/5] Comment fix. --- src/engine.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/engine.cc b/src/engine.cc index 378d9ce8a8..a306783bfe 100644 --- a/src/engine.cc +++ b/src/engine.cc @@ -377,7 +377,7 @@ void EngineLoop::CmdSetOption(const std::string& name, const std::string& value, if (name.empty() || value.empty()) { throw Exception("Both name and value must be provided"); } else { - // Split the names and values by space + // Split the names and values by semicolon. std::vector names = SplitString(name, ';'); std::vector values = SplitString(value, ';'); // Check that the number of names and values match From fd1b815800cf08eacfa2101a00684f64d41b5487 Mon Sep 17 00:00:00 2001 From: Contrad2 Date: Sun, 21 Jan 2024 12:26:04 +0200 Subject: [PATCH 3/5] Moving changes from engine.cc to the rightful place optionsparser.cc I had the changes wrongly in the uci protocol and that was pointed out to me by borg. So Moving the relevant changes to the right place in optionsparser.cc --- src/engine.cc | 37 +++----------------------------- src/utils/optionsparser.cc | 44 ++++++++++++++++++++++++++++++++------ src/utils/optionsparser.h | 4 ++-- 3 files changed, 43 insertions(+), 42 deletions(-) diff --git a/src/engine.cc b/src/engine.cc index a306783bfe..3e6aab5660 100644 --- a/src/engine.cc +++ b/src/engine.cc @@ -30,8 +30,7 @@ #include #include #include -#include -#include + #include "mcts/search.h" @@ -85,17 +84,6 @@ MoveList StringsToMovelist(const std::vector& moves, } return result; } - -std::vector SplitString(const std::string& str, char delimiter) { - std::vector tokens; - std::string token; - std::istringstream tokenStream(str); - while (std::getline(tokenStream, token, delimiter)) { - tokens.push_back(token); - } - return tokens; -} - } // namespace @@ -372,33 +360,14 @@ void EngineLoop::CmdIsReady() { SendResponse("readyok"); } -void EngineLoop::CmdSetOption(const std::string& name, const std::string& value, +void EngineLoop::CmdSetOption(const std::string& name, + const std::string& value, const std::string& context) { - if (name.empty() || value.empty()) { - throw Exception("Both name and value must be provided"); - } else { - // Split the names and values by semicolon. - std::vector names = SplitString(name, ';'); - std::vector values = SplitString(value, ';'); - // Check that the number of names and values match - if (names.size() != values.size()) { - throw Exception("Mismatched number of names and values"); - } - // Set the UCI options for each name-value pair - for (size_t i = 0; i < names.size(); ++i) { - const std::string& name = names[i]; - const std::string& value = values[i]; - // Check that the name is valid - if (!options_.FindOptionByUciName(name)) { - throw Exception("Unknown option: " + name); - } // Set the UCI option options_.SetUciOption(name, value, context); // Set the log filename for the case it was set in UCI option. Logging::Get().SetFilename( options_.GetOptionsDict().Get(kLogFileId)); - } - } } void EngineLoop::CmdUciNewGame() { engine_.NewGame(); } diff --git a/src/utils/optionsparser.cc b/src/utils/optionsparser.cc index 64fb0c2d4d..4c21fe4912 100644 --- a/src/utils/optionsparser.cc +++ b/src/utils/optionsparser.cc @@ -30,6 +30,8 @@ #include #include #include +#include +#include #include "utils/commandline.h" #include "utils/configfile.h" @@ -64,17 +66,47 @@ std::vector OptionsParser::ListOptionsUci() const { return result; } +std::vector SplitString(const std::string& str, char delimiter) { + std::vector tokens; + std::string token; + std::istringstream tokenStream(str); + while (std::getline(tokenStream, token, delimiter)) { + tokens.push_back(token); + } + return tokens; +} + void OptionsParser::SetUciOption(const std::string& name, const std::string& value, const std::string& context) { - auto option = FindOptionByUciName(name); - if (option) { - option->SetValue(value, GetMutableOptions(context)); - return; - } - throw Exception("Unknown option: " + name); + if (name.empty() || value.empty()) { + name.empty() ? throw Exception("An option name must be provided") + : throw Exception("Provide a value for the option"); + } else { + // Split the names and values by semicolon. + std::vector names = SplitString(name, ' '); + std::vector values = SplitString(value, ' '); + // Check that the number of names and values match + if (names.size() != values.size()) { + throw Exception("Mismatched number of names and values"); + } + // Set the UCI options for each name-value pair + for (size_t i = 0; i < names.size(); ++i) { + const std::string& name = names[i]; + const std::string& value = values[i]; + auto option = FindOptionByUciName(name); + if (option) { + option->SetValue(value, GetMutableOptions(context)); + } else { + // If name is not valid throw an error. + throw Exception("Unknown option: " + name); + } + } + return; + } } + void OptionsParser::HideOption(const OptionId& id) { const auto option = FindOptionById(id); if (option) option->hidden_ = true; diff --git a/src/utils/optionsparser.h b/src/utils/optionsparser.h index 8ae89e4e02..5c470c832b 100644 --- a/src/utils/optionsparser.h +++ b/src/utils/optionsparser.h @@ -111,8 +111,6 @@ class OptionsParser { const OptionsDict& GetOptionsDict(const std::string& context = {}); // Gets the dictionary for given context which caller can modify. OptionsDict* GetMutableOptions(const std::string& context = {}); - // Returns an option based by its uci name. - Option* FindOptionByUciName(const std::string& name) const; // Gets the mutable list of default options. OptionsDict* GetMutableDefaultsOptions() { return &defaults_; } // Adds a subdictionary for a given context. @@ -125,6 +123,8 @@ class OptionsParser { void ShowHidden() const; // Returns an option based on the long flag. Option* FindOptionByLongFlag(const std::string& flag) const; + // Returns an option based by its uci name. + Option* FindOptionByUciName(const std::string& name) const; // Returns an option based by its id. Option* FindOptionById(const OptionId& id) const; From b1463a73dbb045a7c99dd45468166809e314a097 Mon Sep 17 00:00:00 2001 From: Contrad2 Date: Sun, 21 Jan 2024 12:28:13 +0200 Subject: [PATCH 4/5] Comment fix --- src/utils/optionsparser.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/optionsparser.cc b/src/utils/optionsparser.cc index 4c21fe4912..8b5fe9d80a 100644 --- a/src/utils/optionsparser.cc +++ b/src/utils/optionsparser.cc @@ -83,7 +83,7 @@ void OptionsParser::SetUciOption(const std::string& name, name.empty() ? throw Exception("An option name must be provided") : throw Exception("Provide a value for the option"); } else { - // Split the names and values by semicolon. + // Split the names and values by space. std::vector names = SplitString(name, ' '); std::vector values = SplitString(value, ' '); // Check that the number of names and values match From 53b2114d58dafa4a709734983b9cec669fd2183f Mon Sep 17 00:00:00 2001 From: Contrad2 Date: Tue, 23 Jan 2024 10:46:31 +0200 Subject: [PATCH 5/5] A few more recomendations. --- src/engine.cc | 14 +++++--------- src/utils/optionsparser.cc | 17 ++--------------- 2 files changed, 7 insertions(+), 24 deletions(-) diff --git a/src/engine.cc b/src/engine.cc index 3e6aab5660..423206640e 100644 --- a/src/engine.cc +++ b/src/engine.cc @@ -31,8 +31,6 @@ #include #include - - #include "mcts/search.h" #include "mcts/stoppers/factory.h" #include "utils/commandline.h" @@ -84,9 +82,8 @@ MoveList StringsToMovelist(const std::vector& moves, } return result; } -} // namespace - +} // namespace EngineController::EngineController(std::unique_ptr uci_responder, const OptionsDict& options) @@ -363,11 +360,10 @@ void EngineLoop::CmdIsReady() { void EngineLoop::CmdSetOption(const std::string& name, const std::string& value, const std::string& context) { - // Set the UCI option - options_.SetUciOption(name, value, context); - // Set the log filename for the case it was set in UCI option. - Logging::Get().SetFilename( - options_.GetOptionsDict().Get(kLogFileId)); + options_.SetUciOption(name, value, context); + // Set the log filename for the case it was set in UCI option. + Logging::Get().SetFilename( + options_.GetOptionsDict().Get(kLogFileId)); } void EngineLoop::CmdUciNewGame() { engine_.NewGame(); } diff --git a/src/utils/optionsparser.cc b/src/utils/optionsparser.cc index 8b5fe9d80a..bc099c7234 100644 --- a/src/utils/optionsparser.cc +++ b/src/utils/optionsparser.cc @@ -79,21 +79,9 @@ std::vector SplitString(const std::string& str, char delimiter) { void OptionsParser::SetUciOption(const std::string& name, const std::string& value, const std::string& context) { - if (name.empty() || value.empty()) { - name.empty() ? throw Exception("An option name must be provided") - : throw Exception("Provide a value for the option"); + if (name.empty()) { + throw Exception("An option name must be provided"); } else { - // Split the names and values by space. - std::vector names = SplitString(name, ' '); - std::vector values = SplitString(value, ' '); - // Check that the number of names and values match - if (names.size() != values.size()) { - throw Exception("Mismatched number of names and values"); - } - // Set the UCI options for each name-value pair - for (size_t i = 0; i < names.size(); ++i) { - const std::string& name = names[i]; - const std::string& value = values[i]; auto option = FindOptionByUciName(name); if (option) { option->SetValue(value, GetMutableOptions(context)); @@ -101,7 +89,6 @@ void OptionsParser::SetUciOption(const std::string& name, // If name is not valid throw an error. throw Exception("Unknown option: " + name); } - } return; } }