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

@@ -214,23 +214,27 @@ bool AnimExpression::parseExpression(const QString& str, QString::const_iterator
             return false;
         }
     } else {
-        qCCritical(animation) << "Error parsing expression, unexpected symbol";
+        unconsumeToken(token);
-        return false;
+        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:
+        case OpCode::And: evalAnd(map, stack); break;
-            switch (opCode.type) {
+        case OpCode::Or: evalOr(map, stack); break;
-            case OpCode::Not:
+        case OpCode::GreaterThan: evalGreaterThan(map, stack); break;
-                evalNot(map, stack);
+        case OpCode::GreaterThanEqual: evalGreaterThanEqual(map, stack); break;
-                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 {
+#define POP_BOOL(NAME)                           \
-    bool lhs = stack.top().coerceBool(map);
+    const OpCode& NAME##_temp = stack.top();     \
-    stack.pop();
+    bool NAME = NAME##_temp.coerceBool(map);     \
-    stack.push(OpCode {!lhs});
+    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;
+    }
 }

libraries/animation/src/AnimExpression.h

@@ -71,15 +71,12 @@ protected:
             LessThanEqual,
             Equal,
             NotEqual,
-            LeftParen,
-            RightParen,
             Not,
-            Minus,
+            Subtract,
-            Plus,
+            Add,
             Multiply,
             Modulus,
-            UnaryPlus,
+            Minus
-            UnaryMinus
         };
         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

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();

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.)

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();
+    e = AnimExpression("!!f");
-    vars.set("x", false);
+    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);
+        auto opCode = e.evaluate(vars);
-    QVERIFY(opCode.type == AnimExpression::OpCode::Bool);
+        QVERIFY(opCode.type == AnimExpression::OpCode::Bool);
-    QVERIFY(opCode.coerceBool(vars) == true);
+        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);
+    }
 }