From 431a108c351431345d9aab4144ef55f80de84ed3 Mon Sep 17 00:00:00 2001
From: "Anthony J. Thibault" <ajt@highfidelity.io>
Date: Wed, 4 Nov 2015 20:13:17 -0800
Subject: [PATCH] Bugfixes to expression for !!x expressions
Added stub eval methods. only boolean not, boolean and, boolean or and unary minus
are implemented.
---
libraries/animation/src/AnimExpression.cpp | 186 +++++++++++++++++++--
libraries/animation/src/AnimExpression.h | 25 ++-
libraries/animation/src/AnimVariant.cpp | 2 +
libraries/animation/src/AnimVariant.h | 11 ++
tests/animation/src/AnimTests.cpp | 87 +++++++++-
5 files changed, 280 insertions(+), 31 deletions(-)
diff --git a/libraries/animation/src/AnimExpression.cpp b/libraries/animation/src/AnimExpression.cpp
index 8807262028..a03925f8f9 100644
--- a/libraries/animation/src/AnimExpression.cpp
+++ b/libraries/animation/src/AnimExpression.cpp
@@ -214,23 +214,27 @@ bool AnimExpression::parseExpression(const QString& str, QString::const_iterator
return false;
}
} else {
- qCCritical(animation) << "Error parsing expression, unexpected symbol";
- return false;
+ unconsumeToken(token);
+ if (parseUnaryExpression(str, iter)) {
+ return true;
+ } else {
+ qCCritical(animation) << "Error parsing expression";
+ return false;
+ }
}
}
bool AnimExpression::parseUnaryExpression(const QString& str, QString::const_iterator& iter) {
auto token = consumeToken(str, iter);
- if (token.type == Token::Plus) {
+ if (token.type == Token::Plus) { // unary plus is a no op.
if (parseExpression(str, iter)) {
- _opCodes.push_back(OpCode {OpCode::UnaryPlus});
return true;
} else {
return false;
}
} else if (token.type == Token::Minus) {
if (parseExpression(str, iter)) {
- _opCodes.push_back(OpCode {OpCode::UnaryMinus});
+ _opCodes.push_back(OpCode {OpCode::Minus});
return true;
} else {
return false;
@@ -255,21 +259,173 @@ AnimExpression::OpCode AnimExpression::evaluate(const AnimVariantMap& map) const
case OpCode::Identifier:
case OpCode::Int:
case OpCode::Float:
+ case OpCode::Bool:
stack.push(opCode);
break;
- default:
- switch (opCode.type) {
- case OpCode::Not:
- evalNot(map, stack);
- break;
- }
+ case OpCode::And: evalAnd(map, stack); break;
+ case OpCode::Or: evalOr(map, stack); break;
+ case OpCode::GreaterThan: evalGreaterThan(map, stack); break;
+ case OpCode::GreaterThanEqual: evalGreaterThanEqual(map, stack); break;
+ case OpCode::LessThan: evalLessThan(map, stack); break;
+ case OpCode::LessThanEqual: evalLessThanEqual(map, stack); break;
+ case OpCode::Equal: evalEqual(map, stack); break;
+ case OpCode::NotEqual: evalNotEqual(map, stack); break;
+ case OpCode::Not: evalNot(map, stack); break;
+ case OpCode::Subtract: evalSubtract(map, stack); break;
+ case OpCode::Add: evalAdd(map, stack); break;
+ case OpCode::Multiply: evalMultiply(map, stack); break;
+ case OpCode::Modulus: evalModulus(map, stack); break;
+ case OpCode::Minus: evalMinus(map, stack); break;
}
}
return stack.top();
}
-void AnimExpression::evalNot(const AnimVariantMap& map, std::stack<OpCode>& stack) const {
- bool lhs = stack.top().coerceBool(map);
- stack.pop();
- stack.push(OpCode {!lhs});
+#define POP_BOOL(NAME) \
+ const OpCode& NAME##_temp = stack.top(); \
+ bool NAME = NAME##_temp.coerceBool(map); \
+ stack.pop()
+
+#define PUSH(EXPR) \
+ stack.push(OpCode {(EXPR)})
+
+void AnimExpression::evalAnd(const AnimVariantMap& map, std::stack<OpCode>& stack) const {
+ POP_BOOL(lhs);
+ POP_BOOL(rhs);
+ PUSH(lhs && rhs);
+}
+
+void AnimExpression::evalOr(const AnimVariantMap& map, std::stack<OpCode>& stack) const {
+ POP_BOOL(lhs);
+ POP_BOOL(rhs);
+ PUSH(lhs || rhs);
+}
+
+void AnimExpression::evalGreaterThan(const AnimVariantMap& map, std::stack<OpCode>& stack) const {
+ OpCode lhs = stack.top(); stack.pop();
+ OpCode rhs = stack.top(); stack.pop();
+
+ // TODO:
+ PUSH(false);
+}
+
+void AnimExpression::evalGreaterThanEqual(const AnimVariantMap& map, std::stack<OpCode>& stack) const {
+ OpCode lhs = stack.top(); stack.pop();
+ OpCode rhs = stack.top(); stack.pop();
+
+ // TODO:
+ PUSH(false);
+}
+
+void AnimExpression::evalLessThan(const AnimVariantMap& map, std::stack<OpCode>& stack) const {
+ OpCode lhs = stack.top(); stack.pop();
+ OpCode rhs = stack.top(); stack.pop();
+
+ // TODO:
+ PUSH(false);
+}
+
+void AnimExpression::evalLessThanEqual(const AnimVariantMap& map, std::stack<OpCode>& stack) const {
+ OpCode lhs = stack.top(); stack.pop();
+ OpCode rhs = stack.top(); stack.pop();
+
+ // TODO:
+ PUSH(false);
+}
+
+void AnimExpression::evalEqual(const AnimVariantMap& map, std::stack<OpCode>& stack) const {
+ OpCode lhs = stack.top(); stack.pop();
+ OpCode rhs = stack.top(); stack.pop();
+
+ // TODO:
+ PUSH(false);
+}
+
+void AnimExpression::evalNotEqual(const AnimVariantMap& map, std::stack<OpCode>& stack) const {
+ OpCode lhs = stack.top(); stack.pop();
+ OpCode rhs = stack.top(); stack.pop();
+
+ // TODO:
+ PUSH(false);
+}
+
+void AnimExpression::evalNot(const AnimVariantMap& map, std::stack<OpCode>& stack) const {
+ POP_BOOL(rhs);
+ PUSH(!rhs);
+}
+
+void AnimExpression::evalSubtract(const AnimVariantMap& map, std::stack<OpCode>& stack) const {
+ OpCode lhs = stack.top(); stack.pop();
+ OpCode rhs = stack.top(); stack.pop();
+
+ // TODO:
+ PUSH(0.0f);
+}
+
+void AnimExpression::evalAdd(const AnimVariantMap& map, std::stack<OpCode>& stack) const {
+ OpCode lhs = stack.top(); stack.pop();
+ OpCode rhs = stack.top(); stack.pop();
+
+ // TODO:
+ PUSH(0.0f);
+}
+
+void AnimExpression::evalMultiply(const AnimVariantMap& map, std::stack<OpCode>& stack) const {
+ OpCode lhs = stack.top(); stack.pop();
+ OpCode rhs = stack.top(); stack.pop();
+
+ // TODO:
+ PUSH(0.0f);
+}
+
+void AnimExpression::evalModulus(const AnimVariantMap& map, std::stack<OpCode>& stack) const {
+ OpCode lhs = stack.top(); stack.pop();
+ OpCode rhs = stack.top(); stack.pop();
+
+ // TODO:
+ PUSH((int)0);
+}
+
+void AnimExpression::evalMinus(const AnimVariantMap& map, std::stack<OpCode>& stack) const {
+ OpCode rhs = stack.top(); stack.pop();
+
+ switch (rhs.type) {
+ case OpCode::Identifier: {
+ const AnimVariant& var = map.get(rhs.strVal);
+ switch (var.getType()) {
+ case AnimVariant::Type::Bool:
+ qCWarning(animation) << "AnimExpression: type missmatch for unary minus, expected a number not a bool";
+ // interpret this as boolean not.
+ PUSH(!var.getBool());
+ break;
+ case AnimVariant::Type::Int:
+ PUSH(-var.getInt());
+ break;
+ case AnimVariant::Type::Float:
+ PUSH(-var.getFloat());
+ break;
+ default:
+ // TODO: Vec3, Quat are unsupported
+ assert(false);
+ PUSH(false);
+ break;
+ }
+ }
+ case OpCode::Int:
+ PUSH(-rhs.intVal);
+ break;
+ case OpCode::Float:
+ PUSH(-rhs.floatVal);
+ break;
+ case OpCode::Bool:
+ qCWarning(animation) << "AnimExpression: type missmatch for unary minus, expected a number not a bool";
+ // interpret this as boolean not.
+ PUSH(!rhs.coerceBool(map));
+ break;
+ default:
+ qCCritical(animation) << "AnimExpression: ERRROR for unary minus, expected a number, type = " << rhs.type;
+ assert(false);
+ PUSH(false);
+ break;
+ }
}
diff --git a/libraries/animation/src/AnimExpression.h b/libraries/animation/src/AnimExpression.h
index 8d216ca412..afada44e86 100644
--- a/libraries/animation/src/AnimExpression.h
+++ b/libraries/animation/src/AnimExpression.h
@@ -71,15 +71,12 @@ protected:
LessThanEqual,
Equal,
NotEqual,
- LeftParen,
- RightParen,
Not,
- Minus,
- Plus,
+ Subtract,
+ Add,
Multiply,
Modulus,
- UnaryPlus,
- UnaryMinus
+ Minus
};
OpCode(Type type) : type {type} {}
OpCode(const QStringRef& strRef) : type {Type::Identifier}, strVal {strRef.toString()} {}
@@ -118,12 +115,24 @@ protected:
bool parseUnaryExpression(const QString& str, QString::const_iterator& iter);
OpCode evaluate(const AnimVariantMap& map) const;
+ void evalAnd(const AnimVariantMap& map, std::stack<OpCode>& stack) const;
+ void evalOr(const AnimVariantMap& map, std::stack<OpCode>& stack) const;
+ void evalGreaterThan(const AnimVariantMap& map, std::stack<OpCode>& stack) const;
+ void evalGreaterThanEqual(const AnimVariantMap& map, std::stack<OpCode>& stack) const;
+ void evalLessThan(const AnimVariantMap& map, std::stack<OpCode>& stack) const;
+ void evalLessThanEqual(const AnimVariantMap& map, std::stack<OpCode>& stack) const;
+ void evalEqual(const AnimVariantMap& map, std::stack<OpCode>& stack) const;
+ void evalNotEqual(const AnimVariantMap& map, std::stack<OpCode>& stack) const;
void evalNot(const AnimVariantMap& map, std::stack<OpCode>& stack) const;
+ void evalSubtract(const AnimVariantMap& map, std::stack<OpCode>& stack) const;
+ void evalAdd(const AnimVariantMap& map, std::stack<OpCode>& stack) const;
+ void evalMultiply(const AnimVariantMap& map, std::stack<OpCode>& stack) const;
+ void evalModulus(const AnimVariantMap& map, std::stack<OpCode>& stack) const;
+ void evalMinus(const AnimVariantMap& map, std::stack<OpCode>& stack) const;
QString _expression;
- mutable std::stack<Token> _tokenStack;
+ mutable std::stack<Token> _tokenStack; // TODO: remove, only needed during parsing
std::vector<OpCode> _opCodes;
-
};
#endif
diff --git a/libraries/animation/src/AnimVariant.cpp b/libraries/animation/src/AnimVariant.cpp
index 234e9cef09..cae6dce23d 100644
--- a/libraries/animation/src/AnimVariant.cpp
+++ b/libraries/animation/src/AnimVariant.cpp
@@ -15,6 +15,8 @@
#include <RegisteredMetaTypes.h>
#include "AnimVariant.h" // which has AnimVariant/AnimVariantMap
+const AnimVariant AnimVariant::FALSE = AnimVariant();
+
QScriptValue AnimVariantMap::animVariantMapToScriptValue(QScriptEngine* engine, const QStringList& names, bool useNames) const {
if (QThread::currentThread() != engine->thread()) {
qCWarning(animation) << "Cannot create Javacript object from non-script thread" << QThread::currentThread();
diff --git a/libraries/animation/src/AnimVariant.h b/libraries/animation/src/AnimVariant.h
index ff7794a16a..b15b25f4e0 100644
--- a/libraries/animation/src/AnimVariant.h
+++ b/libraries/animation/src/AnimVariant.h
@@ -34,6 +34,8 @@ public:
NumTypes
};
+ static const AnimVariant FALSE;
+
AnimVariant() : _type(Type::Bool) { memset(&_val, 0, sizeof(_val)); }
AnimVariant(bool value) : _type(Type::Bool) { _val.boolVal = value; }
AnimVariant(int value) : _type(Type::Int) { _val.intVal = value; }
@@ -186,6 +188,15 @@ public:
void clearMap() { _map.clear(); }
bool hasKey(const QString& key) const { return _map.find(key) != _map.end(); }
+ const AnimVariant& get(const QString& key) const {
+ auto iter = _map.find(key);
+ if (iter != _map.end()) {
+ return iter->second;
+ } else {
+ return AnimVariant::FALSE;
+ }
+ }
+
// Answer a Plain Old Javascript Object (for the given engine) all of our values set as properties.
QScriptValue animVariantMapToScriptValue(QScriptEngine* engine, const QStringList& names, bool useNames) const;
// Side-effect us with the value of object's own properties. (No inherited properties.)
diff --git a/tests/animation/src/AnimTests.cpp b/tests/animation/src/AnimTests.cpp
index 94caed66f5..58fd3a1d35 100644
--- a/tests/animation/src/AnimTests.cpp
+++ b/tests/animation/src/AnimTests.cpp
@@ -357,20 +357,91 @@ void AnimTests::testExpressionTokenizer() {
}
void AnimTests::testExpressionParser() {
- QString str = "(!x)";
- AnimExpression e(str);
+
+ auto vars = AnimVariantMap();
+ vars.set("f", false);
+ vars.set("t", true);
+ vars.set("ten", (int)10);
+ vars.set("twenty", (int)20);
+ vars.set("five", (float)5.0f);
+ vars.set("forty", (float)40.0f);
+
+ AnimExpression e("(!f)");
QVERIFY(e._opCodes.size() == 2);
if (e._opCodes.size() == 2) {
QVERIFY(e._opCodes[0].type == AnimExpression::OpCode::Identifier);
- QVERIFY(e._opCodes[0].strVal == "x");
+ QVERIFY(e._opCodes[0].strVal == "f");
QVERIFY(e._opCodes[1].type == AnimExpression::OpCode::Not);
+
+ auto opCode = e.evaluate(vars);
+ QVERIFY(opCode.type == AnimExpression::OpCode::Bool);
+ QVERIFY((opCode.intVal != 0) == true);
}
- auto vars = AnimVariantMap();
- vars.set("x", false);
+ e = AnimExpression("!!f");
+ QVERIFY(e._opCodes.size() == 3);
+ if (e._opCodes.size() == 3) {
+ QVERIFY(e._opCodes[0].type == AnimExpression::OpCode::Identifier);
+ QVERIFY(e._opCodes[0].strVal == "f");
+ QVERIFY(e._opCodes[1].type == AnimExpression::OpCode::Not);
+ QVERIFY(e._opCodes[2].type == AnimExpression::OpCode::Not);
- auto opCode = e.evaluate(vars);
- QVERIFY(opCode.type == AnimExpression::OpCode::Bool);
- QVERIFY(opCode.coerceBool(vars) == true);
+ auto opCode = e.evaluate(vars);
+ QVERIFY(opCode.type == AnimExpression::OpCode::Bool);
+ QVERIFY((opCode.intVal != 0) == false);
+ }
+
+ e = AnimExpression("!!(!f)");
+ QVERIFY(e._opCodes.size() == 4);
+ if (e._opCodes.size() == 4) {
+ QVERIFY(e._opCodes[0].type == AnimExpression::OpCode::Identifier);
+ QVERIFY(e._opCodes[0].strVal == "f");
+ QVERIFY(e._opCodes[1].type == AnimExpression::OpCode::Not);
+ QVERIFY(e._opCodes[2].type == AnimExpression::OpCode::Not);
+ QVERIFY(e._opCodes[3].type == AnimExpression::OpCode::Not);
+
+ auto opCode = e.evaluate(vars);
+ QVERIFY(opCode.type == AnimExpression::OpCode::Bool);
+ QVERIFY((opCode.intVal != 0) == true);
+ }
+/*
+ e = AnimExpression("f || !f");
+ QVERIFY(e._opCodes.size() == 4);
+ if (e._opCodes.size() == 4) {
+ QVERIFY(e._opCodes[0].type == AnimExpression::OpCode::Identifier);
+ QVERIFY(e._opCodes[0].strVal == "f");
+ QVERIFY(e._opCodes[1].type == AnimExpression::OpCode::Identifier);
+ QVERIFY(e._opCodes[1].strVal == "f");
+ QVERIFY(e._opCodes[2].type == AnimExpression::OpCode::Not);
+ QVERIFY(e._opCodes[3].type == AnimExpression::OpCode::Or);
+
+ auto opCode = e.evaluate(vars);
+ QVERIFY(opCode.type == AnimExpression::OpCode::Bool);
+ QVERIFY((opCode.intVal != 0) == true);
+ }
+*/
+ e = AnimExpression("-10");
+ QVERIFY(e._opCodes.size() == 2);
+ if (e._opCodes.size() == 1) {
+ QVERIFY(e._opCodes[0].type == AnimExpression::OpCode::Int);
+ QVERIFY(e._opCodes[0].intVal == 10);
+ QVERIFY(e._opCodes[1].type == AnimExpression::OpCode::Minus);
+
+ auto opCode = e.evaluate(vars);
+ QVERIFY(opCode.type == AnimExpression::OpCode::Int);
+ QVERIFY(opCode.intVal == 10);
+ }
+
+ e = AnimExpression("-ten");
+ QVERIFY(e._opCodes.size() == 2);
+ if (e._opCodes.size() == 1) {
+ QVERIFY(e._opCodes[0].type == AnimExpression::OpCode::Identifier);
+ QVERIFY(e._opCodes[0].strVal == "ten");
+ QVERIFY(e._opCodes[1].type == AnimExpression::OpCode::Minus);
+
+ auto opCode = e.evaluate(vars);
+ QVERIFY(opCode.type == AnimExpression::OpCode::Int);
+ QVERIFY(opCode.intVal == 10);
+ }
}