From ee838f56f7492f3a00927657e8dc3402c752cbd3 Mon Sep 17 00:00:00 2001 From: Marco Costalba Date: Sat, 17 Mar 2012 10:29:33 +0100 Subject: [PATCH] Fix UCI 'button' options When a button fires UCIOption::operator=() is called and from there the on_change() function. Now it happens that in case of a button the on_change() function resets option's value to "false" triggering again UCIOption::operator=() that calls again on_change() and so on in an endless loop that is experienced by the user as an application hang. Rework the button logic to fix the issue and also be more clear about how button works. Reported by several people working with Scid and tracked down to the "Clear Hash" UCI button by Steven Atkinson. Bug recently introduced by 2ef5b4066e649. No functional change. Signed-off-by: Marco Costalba --- src/ucioption.cpp | 18 ++++++++++++------ src/ucioption.h | 6 +++--- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/ucioption.cpp b/src/ucioption.cpp index 51987d03..75eb2053 100644 --- a/src/ucioption.cpp +++ b/src/ucioption.cpp @@ -33,10 +33,10 @@ OptionsMap Options; // Global object namespace { /// 'On change' actions, triggered by an option's value change -void on_eval(UCIOption&) { Eval::init(); } -void on_threads(UCIOption&) { Threads.read_uci_options(); } -void on_hash_size(UCIOption& o) { TT.set_size(o); } -void on_clear_hash(UCIOption& o) { TT.clear(); o = false; } // UCI button +void on_eval(const UCIOption&) { Eval::init(); } +void on_threads(const UCIOption&) { Threads.read_uci_options(); } +void on_hash_size(const UCIOption& o) { TT.set_size(o); } +void on_clear_hash(const UCIOption&) { TT.clear(); } /// Our case insensitive less() function as required by UCI protocol bool ci_less(char c1, char c2) { return tolower(c1) < tolower(c2); } @@ -74,7 +74,7 @@ OptionsMap::OptionsMap() { o["Threads"] = UCIOption(cpus, 1, MAX_THREADS); o["Use Sleeping Threads"] = UCIOption(true, on_threads); o["Hash"] = UCIOption(32, 4, 8192, on_hash_size); - o["Clear Hash"] = UCIOption(false, on_clear_hash); + o["Clear Hash"] = UCIOption(on_clear_hash); o["Ponder"] = UCIOption(true); o["OwnBook"] = UCIOption(true); o["MultiPV"] = UCIOption(1, 1, 500); @@ -121,6 +121,9 @@ UCIOption::UCIOption(const char* v, Fn* f) : type("string"), min(0), max(0), idx UCIOption::UCIOption(bool v, Fn* f) : type("check"), min(0), max(0), idx(Options.size()), on_change(f) { defaultValue = currentValue = (v ? "true" : "false"); } +UCIOption::UCIOption(Fn* f) : type("button"), min(0), max(0), idx(Options.size()), on_change(f) +{ defaultValue = currentValue = "false"; } + UCIOption::UCIOption(int v, int minv, int maxv, Fn* f) : type("spin"), min(minv), max(maxv), idx(Options.size()), on_change(f) { std::ostringstream ss; ss << v; defaultValue = currentValue = ss.str(); } @@ -134,12 +137,15 @@ void UCIOption::operator=(const string& v) { assert(!type.empty()); if ( !v.empty() - && (type == "check") == (v == "true" || v == "false") + && (type == "check" || type == "button") == (v == "true" || v == "false") && (type != "spin" || (atoi(v.c_str()) >= min && atoi(v.c_str()) <= max))) { currentValue = v; if (on_change) (*on_change)(*this); + + if (type == "button") + currentValue = "false"; } } diff --git a/src/ucioption.h b/src/ucioption.h index cde83231..7af54739 100644 --- a/src/ucioption.h +++ b/src/ucioption.h @@ -30,12 +30,12 @@ struct OptionsMap; /// UCIOption class implements an option as defined by UCI protocol class UCIOption { - typedef void (Fn)(UCIOption&); + typedef void (Fn)(const UCIOption&); public: - UCIOption() {} // Required by std::map::operator[] - UCIOption(const char* v, Fn* = NULL); + UCIOption(Fn* = NULL); UCIOption(bool v, Fn* = NULL); + UCIOption(const char* v, Fn* = NULL); UCIOption(int v, int min, int max, Fn* = NULL); void operator=(const std::string& v); -- 2.39.2