From 6586ecc7a861815cfa00208392455a0b329e394b Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 22 Nov 2019 14:41:10 -0500 Subject: [PATCH 1/7] InputCommon/FunctionExpression: include std::min/std::max are used within this translation unit, so it needs to be included to prevent potential compilation failures. --- .../Core/InputCommon/ControlReference/FunctionExpression.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp b/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp index 18b98c5e19..f1cdc690a3 100644 --- a/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp +++ b/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp @@ -2,11 +2,12 @@ // Licensed under GPLv2+ // Refer to the license.txt file included. +#include "InputCommon/ControlReference/FunctionExpression.h" + +#include #include #include -#include "InputCommon/ControlReference/FunctionExpression.h" - namespace ciface { namespace ExpressionParser From cb8fbe872e86705136bd1fa5f50b2bccfbb9b50a Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 22 Nov 2019 14:44:33 -0500 Subject: [PATCH 2/7] InputCommon/FunctionExpression: Collapse namespaces Since we target C++17, we can collapse the namespaces into a single declaration specifier. --- .../InputCommon/ControlReference/FunctionExpression.cpp | 7 ++----- .../Core/InputCommon/ControlReference/FunctionExpression.h | 7 ++----- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp b/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp index f1cdc690a3..8bfd57402d 100644 --- a/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp +++ b/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp @@ -8,9 +8,7 @@ #include #include -namespace ciface -{ -namespace ExpressionParser +namespace ciface::ExpressionParser { constexpr int LOOP_MAX_REPS = 10000; constexpr ControlState CONDITION_THRESHOLD = 0.5; @@ -523,5 +521,4 @@ void FunctionExpression::SetValue(ControlState) { } -} // namespace ExpressionParser -} // namespace ciface +} // namespace ciface::ExpressionParser diff --git a/Source/Core/InputCommon/ControlReference/FunctionExpression.h b/Source/Core/InputCommon/ControlReference/FunctionExpression.h index f5332a0d25..fb0b2a7da0 100644 --- a/Source/Core/InputCommon/ControlReference/FunctionExpression.h +++ b/Source/Core/InputCommon/ControlReference/FunctionExpression.h @@ -12,9 +12,7 @@ #include "InputCommon/ControlReference/ExpressionParser.h" #include "InputCommon/ControlReference/FunctionExpression.h" -namespace ciface -{ -namespace ExpressionParser +namespace ciface::ExpressionParser { class FunctionExpression : public Expression { @@ -51,5 +49,4 @@ private: std::unique_ptr MakeFunctionExpression(std::string name); -} // namespace ExpressionParser -} // namespace ciface +} // namespace ciface::ExpressionParser From 64bc6f53fd22fabedeef56016080d5996a144fdb Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 22 Nov 2019 14:48:10 -0500 Subject: [PATCH 3/7] InputCommon/FunctionExpression: Remove cyclical include This header was including itself, which is likely not intended. --- Source/Core/InputCommon/ControlReference/FunctionExpression.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Core/InputCommon/ControlReference/FunctionExpression.h b/Source/Core/InputCommon/ControlReference/FunctionExpression.h index fb0b2a7da0..44d9092ce9 100644 --- a/Source/Core/InputCommon/ControlReference/FunctionExpression.h +++ b/Source/Core/InputCommon/ControlReference/FunctionExpression.h @@ -9,8 +9,8 @@ #include #include +#include "Common/CommonTypes.h" #include "InputCommon/ControlReference/ExpressionParser.h" -#include "InputCommon/ControlReference/FunctionExpression.h" namespace ciface::ExpressionParser { From ddf8abf507c40eb5e038297a87d2881a1d33d05c Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 22 Nov 2019 14:50:37 -0500 Subject: [PATCH 4/7] InputCommon/FunctionExpression: Remove unused LOOP_MAX_REPS constant This isn't used anywhere in the translation unit, so we can remove it. --- Source/Core/InputCommon/ControlReference/FunctionExpression.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp b/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp index 8bfd57402d..ae3f14097e 100644 --- a/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp +++ b/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp @@ -10,7 +10,6 @@ namespace ciface::ExpressionParser { -constexpr int LOOP_MAX_REPS = 10000; constexpr ControlState CONDITION_THRESHOLD = 0.5; using Clock = std::chrono::steady_clock; From 10fea99d8049d3224f736bcfaa1cf981fa0d14ec Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 22 Nov 2019 14:51:57 -0500 Subject: [PATCH 5/7] InputCommon/FunctionExpression: Make MakeFunctionExpression() take a std::string_view There's nothing within this function that requires a copy of the string to be made, so we can make use of a non-owning view --- .../Core/InputCommon/ControlReference/FunctionExpression.cpp | 2 +- Source/Core/InputCommon/ControlReference/FunctionExpression.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp b/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp index ae3f14097e..abc6eece02 100644 --- a/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp +++ b/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp @@ -447,7 +447,7 @@ private: mutable Clock::time_point m_release_time = Clock::now(); }; -std::unique_ptr MakeFunctionExpression(std::string name) +std::unique_ptr MakeFunctionExpression(std::string_view name) { if ("not" == name) return std::make_unique(); diff --git a/Source/Core/InputCommon/ControlReference/FunctionExpression.h b/Source/Core/InputCommon/ControlReference/FunctionExpression.h index 44d9092ce9..e247d9a623 100644 --- a/Source/Core/InputCommon/ControlReference/FunctionExpression.h +++ b/Source/Core/InputCommon/ControlReference/FunctionExpression.h @@ -6,6 +6,7 @@ #include #include +#include #include #include @@ -47,6 +48,6 @@ private: std::vector> m_args; }; -std::unique_ptr MakeFunctionExpression(std::string name); +std::unique_ptr MakeFunctionExpression(std::string_view name); } // namespace ciface::ExpressionParser From 1f6077922bc4f21803be7dd4435819ce19bddd54 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 22 Nov 2019 14:55:33 -0500 Subject: [PATCH 6/7] InputCommon/FunctionExpression: Remove unnecessary 'else' in MakeFunctionExpression() Given all conditional bodies only contain a return, the use of else here isn't necessary. This has the benefit of consistently vertically aligning the names. --- .../ControlReference/FunctionExpression.cpp | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp b/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp index abc6eece02..6463a85101 100644 --- a/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp +++ b/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp @@ -451,30 +451,30 @@ std::unique_ptr MakeFunctionExpression(std::string_view name { if ("not" == name) return std::make_unique(); - else if ("if" == name) + if ("if" == name) return std::make_unique(); - else if ("sin" == name) + if ("sin" == name) return std::make_unique(); - else if ("timer" == name) + if ("timer" == name) return std::make_unique(); - else if ("toggle" == name) + if ("toggle" == name) return std::make_unique(); - else if ("minus" == name) + if ("minus" == name) return std::make_unique(); - else if ("deadzone" == name) + if ("deadzone" == name) return std::make_unique(); - else if ("smooth" == name) + if ("smooth" == name) return std::make_unique(); - else if ("hold" == name) + if ("hold" == name) return std::make_unique(); - else if ("tap" == name) + if ("tap" == name) return std::make_unique(); - else if ("relative" == name) + if ("relative" == name) return std::make_unique(); - else if ("pulse" == name) + if ("pulse" == name) return std::make_unique(); - else - return nullptr; + + return nullptr; } int FunctionExpression::CountNumControls() const From 814fd165af56289401124989806201a4d110bc08 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 22 Nov 2019 15:07:16 -0500 Subject: [PATCH 7/7] InputCommon/FunctionExpression: Use Yoda conditions, we do not The general convention in the codebase is to compare the non-constant value/string with the constant value/string, not the other way around. --- .../ControlReference/FunctionExpression.cpp | 50 +++++++++---------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp b/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp index 6463a85101..75f78526e6 100644 --- a/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp +++ b/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp @@ -23,7 +23,7 @@ private: ValidateArguments(const std::vector>& args) override { // Optional 2nd argument for clearing state: - if (1 == args.size() || 2 == args.size()) + if (args.size() == 1 || args.size() == 2) return ArgumentsAreValid{}; else return ExpectedArguments{"toggle_state_input, [clear_state_input]"}; @@ -62,7 +62,7 @@ private: ArgumentValidation ValidateArguments(const std::vector>& args) override { - if (1 == args.size()) + if (args.size() == 1) return ArgumentsAreValid{}; else return ExpectedArguments{"expression"}; @@ -79,7 +79,7 @@ private: ArgumentValidation ValidateArguments(const std::vector>& args) override { - if (1 == args.size()) + if (args.size() == 1) return ArgumentsAreValid{}; else return ExpectedArguments{"expression"}; @@ -95,7 +95,7 @@ private: ArgumentValidation ValidateArguments(const std::vector>& args) override { - if (1 == args.size()) + if (args.size() == 1) return ArgumentsAreValid{}; else return ExpectedArguments{"seconds"}; @@ -138,7 +138,7 @@ private: ArgumentValidation ValidateArguments(const std::vector>& args) override { - if (3 == args.size()) + if (args.size() == 3) return ArgumentsAreValid{}; else return ExpectedArguments{"condition, true_expression, false_expression"}; @@ -158,7 +158,7 @@ private: ArgumentValidation ValidateArguments(const std::vector>& args) override { - if (1 == args.size()) + if (args.size() == 1) return ArgumentsAreValid{}; else return ExpectedArguments{"expression"}; @@ -177,7 +177,7 @@ class DeadzoneExpression : public FunctionExpression ArgumentValidation ValidateArguments(const std::vector>& args) override { - if (2 == args.size()) + if (args.size() == 2) return ArgumentsAreValid{}; else return ExpectedArguments{"input, amount"}; @@ -198,7 +198,7 @@ class SmoothExpression : public FunctionExpression ArgumentValidation ValidateArguments(const std::vector>& args) override { - if (2 == args.size() || 3 == args.size()) + if (args.size() == 2 || args.size() == 3) return ArgumentsAreValid{}; else return ExpectedArguments{"input, seconds_up, seconds_down = seconds_up"}; @@ -213,7 +213,7 @@ class SmoothExpression : public FunctionExpression const ControlState desired_value = GetArg(0).GetValue(); const ControlState smooth_up = GetArg(1).GetValue(); - const ControlState smooth_down = (3 == GetArgCount() ? GetArg(2).GetValue() : smooth_up); + const ControlState smooth_down = GetArgCount() == 3 ? GetArg(2).GetValue() : smooth_up; const ControlState smooth = (desired_value < m_value) ? smooth_down : smooth_up; const ControlState max_move = std::chrono::duration_cast(elapsed).count() / smooth; @@ -242,7 +242,7 @@ class HoldExpression : public FunctionExpression ArgumentValidation ValidateArguments(const std::vector>& args) override { - if (2 == args.size()) + if (args.size() == 2) return ArgumentsAreValid{}; else return ExpectedArguments{"input, seconds"}; @@ -281,7 +281,7 @@ class TapExpression : public FunctionExpression ArgumentValidation ValidateArguments(const std::vector>& args) override { - if (2 == args.size() || 3 == args.size()) + if (args.size() == 2 || args.size() == 3) return ArgumentsAreValid{}; else return ExpectedArguments{"input, seconds, taps = 2"}; @@ -298,7 +298,7 @@ class TapExpression : public FunctionExpression const bool is_time_up = elapsed > seconds; - const u32 desired_taps = (3 == GetArgCount()) ? u32(GetArg(2).GetValue() + 0.5) : 2; + const u32 desired_taps = GetArgCount() == 3 ? u32(GetArg(2).GetValue() + 0.5) : 2; if (input < CONDITION_THRESHOLD) { @@ -400,7 +400,7 @@ class PulseExpression : public FunctionExpression ArgumentValidation ValidateArguments(const std::vector>& args) override { - if (2 == args.size()) + if (args.size() == 2) return ArgumentsAreValid{}; else return ExpectedArguments{"input, seconds"}; @@ -449,29 +449,29 @@ private: std::unique_ptr MakeFunctionExpression(std::string_view name) { - if ("not" == name) + if (name == "not") return std::make_unique(); - if ("if" == name) + if (name == "if") return std::make_unique(); - if ("sin" == name) + if (name == "sin") return std::make_unique(); - if ("timer" == name) + if (name == "timer") return std::make_unique(); - if ("toggle" == name) + if (name == "toggle") return std::make_unique(); - if ("minus" == name) + if (name == "minus") return std::make_unique(); - if ("deadzone" == name) + if (name == "deadzone") return std::make_unique(); - if ("smooth" == name) + if (name == "smooth") return std::make_unique(); - if ("hold" == name) + if (name == "hold") return std::make_unique(); - if ("tap" == name) + if (name == "tap") return std::make_unique(); - if ("relative" == name) + if (name == "relative") return std::make_unique(); - if ("pulse" == name) + if (name == "pulse") return std::make_unique(); return nullptr;