From c9ad5430d03d8c7d1be4dd7f1cc8b948bc4076d8 Mon Sep 17 00:00:00 2001 From: Jordan Woyak Date: Thu, 7 Nov 2024 11:51:11 -0600 Subject: [PATCH] InputCommon: Fix input expression assignment operator behavior. --- .../ControlReference/ExpressionParser.cpp | 105 +++++++++++------- .../ControlReference/ExpressionParser.h | 5 +- .../ControlReference/FunctionExpression.cpp | 99 +++++++---------- .../ControlReference/FunctionExpression.h | 1 - 4 files changed, 113 insertions(+), 97 deletions(-) diff --git a/Source/Core/InputCommon/ControlReference/ExpressionParser.cpp b/Source/Core/InputCommon/ControlReference/ExpressionParser.cpp index 0db51cfe2a..35cadf8cdb 100644 --- a/Source/Core/InputCommon/ControlReference/ExpressionParser.cpp +++ b/Source/Core/InputCommon/ControlReference/ExpressionParser.cpp @@ -4,9 +4,9 @@ #include "InputCommon/ControlReference/ExpressionParser.h" #include +#include #include #include -#include #include #include #include @@ -14,7 +14,6 @@ #include #include -#include "Common/Assert.h" #include "Common/MsgHandler.h" #include "Common/StringUtil.h" @@ -252,12 +251,17 @@ ParseStatus Lexer::Tokenize(std::vector& tokens) return ParseStatus::Successful; } +Expression* Expression::GetLValue() +{ + return this; +} + class ControlExpression : public Expression { public: explicit ControlExpression(ControlQualifier qualifier) : m_qualifier(std::move(qualifier)) {} - ControlState GetValue() const override + ControlState GetValue() override { if (s_hotkey_suppressions.IsSuppressed(m_input)) return 0; @@ -352,55 +356,67 @@ public: { } - ControlState GetValue() const override + ControlState GetValue() override { + if (op == TOK_ASSIGN || op == TOK_COMMA) + { + return GetLValue()->GetValue(); + } + + // Strict evaluation order of lhs,rhs in case of side effects. + const ControlState lhs_value = lhs->GetValue(); + const ControlState rhs_value = rhs->GetValue(); + switch (op) { case TOK_AND: - return std::min(lhs->GetValue(), rhs->GetValue()); + return std::min(lhs_value, rhs_value); case TOK_OR: - return std::max(lhs->GetValue(), rhs->GetValue()); + return std::max(lhs_value, rhs_value); case TOK_ADD: - return lhs->GetValue() + rhs->GetValue(); + return lhs_value + rhs_value; case TOK_SUB: - return lhs->GetValue() - rhs->GetValue(); + return lhs_value - rhs_value; case TOK_MUL: - return lhs->GetValue() * rhs->GetValue(); + return lhs_value * rhs_value; case TOK_DIV: { - const ControlState result = lhs->GetValue() / rhs->GetValue(); + const ControlState result = lhs_value / rhs_value; return std::isinf(result) ? 0.0 : result; } case TOK_MOD: { - const ControlState result = std::fmod(lhs->GetValue(), rhs->GetValue()); + const ControlState result = std::fmod(lhs_value, rhs_value); return std::isnan(result) ? 0.0 : result; } + case TOK_LTHAN: + return lhs_value < rhs_value; + case TOK_GTHAN: + return lhs_value > rhs_value; + case TOK_XOR: + return std::max(std::min(1 - lhs_value, rhs_value), std::min(lhs_value, 1 - rhs_value)); + default: + assert(false); + return 0; + } + } + + Expression* GetLValue() override + { + switch (op) + { case TOK_ASSIGN: { - // Use this carefully as it's extremely powerful and can end up in unforeseen situations - lhs->SetValue(rhs->GetValue()); - return lhs->GetValue(); + Expression* const lvalue = lhs->GetLValue(); + const ControlState rvalue = rhs->GetValue(); + lvalue->SetValue(rvalue); + return lvalue; } - case TOK_LTHAN: - return lhs->GetValue() < rhs->GetValue(); - case TOK_GTHAN: - return lhs->GetValue() > rhs->GetValue(); case TOK_COMMA: - { - // Eval and discard lhs: lhs->GetValue(); - return rhs->GetValue(); - } - case TOK_XOR: - { - const auto lval = lhs->GetValue(); - const auto rval = rhs->GetValue(); - return std::max(std::min(1 - lval, rval), std::min(lval, 1 - rval)); - } + return rhs->GetLValue(); default: - ASSERT(false); - return 0; + return this; } } @@ -448,7 +464,7 @@ class LiteralReal : public LiteralExpression public: explicit LiteralReal(ControlState value) : m_value(value) {} - ControlState GetValue() const override { return m_value; } + ControlState GetValue() override { return m_value; } std::string GetName() const override { return ValueToString(m_value); } @@ -470,7 +486,7 @@ class VariableExpression : public Expression public: explicit VariableExpression(std::string name) : m_name(std::move(name)) {} - ControlState GetValue() const override { return m_variable_ptr ? *m_variable_ptr : 0; } + ControlState GetValue() override { return m_variable_ptr ? *m_variable_ptr : 0; } void SetValue(ControlState value) override { @@ -500,7 +516,7 @@ public: m_modifiers.pop_back(); } - ControlState GetValue() const override + ControlState GetValue() override { // True if we have no modifiers const bool modifiers_pressed = std::ranges::all_of( @@ -561,7 +577,7 @@ public: } private: - void EnableSuppression(bool force = false) const + void EnableSuppression(bool force = false) { if (!m_suppressor || force) m_suppressor = s_hotkey_suppressions.MakeSuppressor(&m_modifiers, &m_final_input); @@ -569,8 +585,8 @@ private: HotkeySuppressions::Modifiers m_modifiers; std::unique_ptr m_final_input; - mutable HotkeySuppressions::Suppressor m_suppressor; - mutable bool m_is_blocked = false; + HotkeySuppressions::Suppressor m_suppressor; + bool m_is_blocked = false; }; // This class proxies all methods to its either left-hand child if it has bound controls, or its @@ -586,7 +602,7 @@ public: { } - ControlState GetValue() const override { return GetActiveChild()->GetValue(); } + ControlState GetValue() override { return GetActiveChild()->GetValue(); } void SetValue(ControlState value) override { GetActiveChild()->SetValue(value); } int CountNumControls() const override { return GetActiveChild()->CountNumControls(); } @@ -880,6 +896,18 @@ private: } } + static bool IsRTLBinaryOp(TokenType type) { return type == TOK_ASSIGN; } + + static bool IsBinaryOpWithPrecedence(Token tok, int precedence) + { + if (!tok.IsBinaryOperator()) + return false; + + const int tok_precedence = OperatorPrecedence(tok.type); + return (tok_precedence < precedence) || + (IsRTLBinaryOp(tok.type) && tok_precedence <= precedence); + } + ParseResult ParseInfixOperations(int precedence = OperatorPrecedence()) { ParseResult lhs = ParseAtom(Chew()); @@ -889,11 +917,10 @@ private: std::unique_ptr expr = std::move(lhs.expr); - // TODO: handle LTR/RTL associativity? while (true) { const Token op = Peek(); - if (op.IsBinaryOperator() && OperatorPrecedence(op.type) < precedence) + if (IsBinaryOpWithPrecedence(op, precedence)) { Chew(); ParseResult rhs = ParseInfixOperations(OperatorPrecedence(op.type)); diff --git a/Source/Core/InputCommon/ControlReference/ExpressionParser.h b/Source/Core/InputCommon/ControlReference/ExpressionParser.h index af24900d52..a7dfff9feb 100644 --- a/Source/Core/InputCommon/ControlReference/ExpressionParser.h +++ b/Source/Core/InputCommon/ControlReference/ExpressionParser.h @@ -170,10 +170,13 @@ class Expression { public: virtual ~Expression() = default; - virtual ControlState GetValue() const = 0; + virtual ControlState GetValue() = 0; virtual void SetValue(ControlState state) = 0; virtual int CountNumControls() const = 0; virtual void UpdateReferences(ControlEnvironment& finder) = 0; + + // Perform any side effects and return Expression to be SetValue'd. + virtual Expression* GetLValue(); }; class ParseResult diff --git a/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp b/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp index 15d8c72c0c..f6a528dd26 100644 --- a/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp +++ b/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp @@ -26,7 +26,7 @@ private: return ExpectedArguments{"toggle_state_input, [clear_state_input]"}; } - ControlState GetValue() const override + ControlState GetValue() override { const ControlState inner_value = GetArg(0).GetValue(); @@ -48,8 +48,8 @@ private: return m_state; } - mutable bool m_released{}; - mutable bool m_state{}; + bool m_released{}; + bool m_state{}; }; // usage: not(expression) @@ -65,7 +65,7 @@ private: return ExpectedArguments{"expression"}; } - ControlState GetValue() const override { return 1.0 - GetArg(0).GetValue(); } + ControlState GetValue() override { return 1.0 - GetArg(0).GetValue(); } void SetValue(ControlState value) override { GetArg(0).SetValue(1.0 - value); } }; @@ -82,7 +82,7 @@ private: return ExpectedArguments{"expression"}; } - ControlState GetValue() const override { return std::abs(GetArg(0).GetValue()); } + ControlState GetValue() override { return std::abs(GetArg(0).GetValue()); } }; // usage: sin(expression) @@ -98,7 +98,7 @@ private: return ExpectedArguments{"expression"}; } - ControlState GetValue() const override { return std::sin(GetArg(0).GetValue()); } + ControlState GetValue() override { return std::sin(GetArg(0).GetValue()); } }; // usage: cos(expression) @@ -114,7 +114,7 @@ private: return ExpectedArguments{"expression"}; } - ControlState GetValue() const override { return std::cos(GetArg(0).GetValue()); } + ControlState GetValue() override { return std::cos(GetArg(0).GetValue()); } }; // usage: tan(expression) @@ -130,7 +130,7 @@ private: return ExpectedArguments{"expression"}; } - ControlState GetValue() const override { return std::tan(GetArg(0).GetValue()); } + ControlState GetValue() override { return std::tan(GetArg(0).GetValue()); } }; // usage: asin(expression) @@ -146,7 +146,7 @@ private: return ExpectedArguments{"expression"}; } - ControlState GetValue() const override { return std::asin(GetArg(0).GetValue()); } + ControlState GetValue() override { return std::asin(GetArg(0).GetValue()); } }; // usage: acos(expression) @@ -162,7 +162,7 @@ private: return ExpectedArguments{"expression"}; } - ControlState GetValue() const override { return std::acos(GetArg(0).GetValue()); } + ControlState GetValue() override { return std::acos(GetArg(0).GetValue()); } }; // usage: atan(expression) @@ -178,7 +178,7 @@ private: return ExpectedArguments{"expression"}; } - ControlState GetValue() const override { return std::atan(GetArg(0).GetValue()); } + ControlState GetValue() override { return std::atan(GetArg(0).GetValue()); } }; // usage: atan2(y, x) @@ -194,7 +194,7 @@ private: return ExpectedArguments{"y, x"}; } - ControlState GetValue() const override + ControlState GetValue() override { return std::atan2(GetArg(0).GetValue(), GetArg(1).GetValue()); } @@ -213,7 +213,7 @@ private: return ExpectedArguments{"expression"}; } - ControlState GetValue() const override { return std::sqrt(GetArg(0).GetValue()); } + ControlState GetValue() override { return std::sqrt(GetArg(0).GetValue()); } }; // usage: pow(base, exponent) @@ -229,10 +229,7 @@ private: return ExpectedArguments{"base, exponent"}; } - ControlState GetValue() const override - { - return std::pow(GetArg(0).GetValue(), GetArg(1).GetValue()); - } + ControlState GetValue() override { return std::pow(GetArg(0).GetValue(), GetArg(1).GetValue()); } }; // usage: min(a, b) @@ -248,10 +245,7 @@ private: return ExpectedArguments{"a, b"}; } - ControlState GetValue() const override - { - return std::min(GetArg(0).GetValue(), GetArg(1).GetValue()); - } + ControlState GetValue() override { return std::min(GetArg(0).GetValue(), GetArg(1).GetValue()); } }; // usage: max(a, b) @@ -267,10 +261,7 @@ private: return ExpectedArguments{"a, b"}; } - ControlState GetValue() const override - { - return std::max(GetArg(0).GetValue(), GetArg(1).GetValue()); - } + ControlState GetValue() override { return std::max(GetArg(0).GetValue(), GetArg(1).GetValue()); } }; // usage: clamp(value, min, max) @@ -286,7 +277,7 @@ private: return ExpectedArguments{"value, min, max"}; } - ControlState GetValue() const override + ControlState GetValue() override { return std::clamp(GetArg(0).GetValue(), GetArg(1).GetValue(), GetArg(2).GetValue()); } @@ -305,7 +296,7 @@ private: return ExpectedArguments{"seconds"}; } - ControlState GetValue() const override + ControlState GetValue() override { const auto now = Clock::now(); const auto elapsed = now - m_start_time; @@ -332,7 +323,7 @@ private: } private: - mutable Clock::time_point m_start_time = Clock::now(); + Clock::time_point m_start_time = Clock::now(); }; // usage: if(condition, true_expression, false_expression) @@ -348,10 +339,11 @@ private: return ExpectedArguments{"condition, true_expression, false_expression"}; } - ControlState GetValue() const override + ControlState GetValue() override { return GetLValue()->GetValue(); } + + Expression* GetLValue() override { - return (GetArg(0).GetValue() > CONDITION_THRESHOLD) ? GetArg(1).GetValue() : - GetArg(2).GetValue(); + return (GetArg(0).GetValue() > CONDITION_THRESHOLD) ? &GetArg(1) : &GetArg(2); } }; @@ -368,7 +360,7 @@ private: return ExpectedArguments{"expression"}; } - ControlState GetValue() const override + ControlState GetValue() override { // Subtraction for clarity: return 0.0 - GetArg(0).GetValue(); @@ -388,7 +380,7 @@ private: return ExpectedArguments{"expression"}; } - ControlState GetValue() const override { return GetArg(0).GetValue(); } + ControlState GetValue() override { return GetArg(0).GetValue(); } }; // usage: deadzone(input, amount) @@ -403,7 +395,7 @@ class DeadzoneExpression : public FunctionExpression return ExpectedArguments{"input, amount"}; } - ControlState GetValue() const override + ControlState GetValue() override { const ControlState val = GetArg(0).GetValue(); const ControlState deadzone = GetArg(1).GetValue(); @@ -424,7 +416,7 @@ class SmoothExpression : public FunctionExpression return ExpectedArguments{"input, seconds_up, seconds_down = seconds_up"}; } - ControlState GetValue() const override + ControlState GetValue() override { const auto now = Clock::now(); const auto elapsed = now - m_last_update; @@ -452,8 +444,8 @@ class SmoothExpression : public FunctionExpression } private: - mutable ControlState m_value = 0.0; - mutable Clock::time_point m_last_update = Clock::now(); + ControlState m_value = 0.0; + Clock::time_point m_last_update = Clock::now(); }; // usage: hold(input, seconds) @@ -468,7 +460,7 @@ class HoldExpression : public FunctionExpression return ExpectedArguments{"input, seconds"}; } - ControlState GetValue() const override + ControlState GetValue() override { const auto now = Clock::now(); @@ -491,8 +483,8 @@ class HoldExpression : public FunctionExpression } private: - mutable bool m_state = false; - mutable Clock::time_point m_start_time = Clock::now(); + bool m_state = false; + Clock::time_point m_start_time = Clock::now(); }; // usage: tap(input, seconds, taps=2) @@ -507,7 +499,7 @@ class TapExpression : public FunctionExpression return ExpectedArguments{"input, seconds, taps = 2"}; } - ControlState GetValue() const override + ControlState GetValue() override { const auto now = Clock::now(); @@ -549,9 +541,9 @@ class TapExpression : public FunctionExpression } private: - mutable bool m_released = true; - mutable u32 m_taps = 0; - mutable Clock::time_point m_start_time = Clock::now(); + bool m_released = true; + u32 m_taps = 0; + Clock::time_point m_start_time = Clock::now(); }; // usage: relative(input, speed, [max_abs_value, [shared_state]]) @@ -567,7 +559,7 @@ class RelativeExpression : public FunctionExpression return ExpectedArguments{"input, speed, [max_abs_value, [shared_state]]"}; } - ControlState GetValue() const override + ControlState GetValue() override { // There is a lot of funky math in this function but it allows for a variety of uses: // @@ -610,8 +602,8 @@ class RelativeExpression : public FunctionExpression } private: - mutable ControlState m_state = 0.0; - mutable Clock::time_point m_last_update = Clock::now(); + ControlState m_state = 0.0; + Clock::time_point m_last_update = Clock::now(); }; // usage: pulse(input, seconds) @@ -626,7 +618,7 @@ class PulseExpression : public FunctionExpression return ExpectedArguments{"input, seconds"}; } - ControlState GetValue() const override + ControlState GetValue() override { const auto now = Clock::now(); @@ -662,9 +654,9 @@ class PulseExpression : public FunctionExpression } private: - mutable bool m_released = false; - mutable bool m_state = false; - mutable Clock::time_point m_release_time = Clock::now(); + bool m_released = false; + bool m_state = false; + Clock::time_point m_release_time = Clock::now(); }; std::unique_ptr MakeFunctionExpression(std::string_view name) @@ -752,11 +744,6 @@ Expression& FunctionExpression::GetArg(u32 number) return *m_args[number]; } -const Expression& FunctionExpression::GetArg(u32 number) const -{ - return *m_args[number]; -} - u32 FunctionExpression::GetArgCount() const { return u32(m_args.size()); diff --git a/Source/Core/InputCommon/ControlReference/FunctionExpression.h b/Source/Core/InputCommon/ControlReference/FunctionExpression.h index 3ba58759cd..be6e2c19ff 100644 --- a/Source/Core/InputCommon/ControlReference/FunctionExpression.h +++ b/Source/Core/InputCommon/ControlReference/FunctionExpression.h @@ -42,7 +42,6 @@ protected: ValidateArguments(const std::vector>& args) = 0; Expression& GetArg(u32 number); - const Expression& GetArg(u32 number) const; u32 GetArgCount() const; private: