ACS improvements needed for menus

* Added user.sysop, user.cosysop meta values.
* Implemented previously documented user.regnum
* Added variables true, false with values as you expect.
* Implemented the new user values in AcsExpr

bbs/acs.cpp

@@ -39,7 +39,9 @@ namespace wwiv::bbs {
 bool check_acs(const std::string& expression, acs_debug_t debug) {
   Eval eval(expression);
 
-  eval.add("user", std::make_unique<UserValueProvider>(a()->user(), a()->sess().effective_sl()));
+  auto esl = a()->sess().effective_sl();
+  auto eslrec = a()->effective_slrec();
+  eval.add("user", std::make_unique<UserValueProvider>(a()->user(), esl, eslrec));
 
   const auto result = eval.eval();
 
@@ -57,7 +59,9 @@ bool check_acs(const std::string& expression, acs_debug_t debug) {
 bool validate_acs(const std::string& expression, acs_debug_t debug) {
   Eval eval(expression);
 
-  eval.add("user", std::make_unique<UserValueProvider>(a()->user(), a()->sess().effective_sl()));
+  auto esl = a()->sess().effective_sl();
+  auto eslrec = a()->effective_slrec();
+  eval.add("user", std::make_unique<UserValueProvider>(a()->user(), esl, eslrec));
 
   try {
     eval.eval_throws();

core/parser/ast.cpp

@@ -207,6 +207,7 @@ static std::unique_ptr<BinaryOperatorNode> createBinaryOperator(const Token& tok
 }
 
 bool Ast::reduce(std::stack<std::unique_ptr<AstNode>>& stack) {
+  VLOG(2) << "Ast::reduce. Stack Size: " << stack.size();
   if (stack.size() < 3) {
     throw parse_error(fmt::format("Can't reduce stack with < 3 elements."));
   }
@@ -249,7 +250,7 @@ bool Ast::reduce(std::stack<std::unique_ptr<AstNode>>& stack) {
 std::unique_ptr<AstNode> Ast::parseExpression(std::vector<Token>::iterator& it,
                                               const std::vector<Token>::iterator& end) {
   std::stack<std::unique_ptr<AstNode>> stack;
-  for (; it != end; it++) {
+  for (; it != end; ++it) {
     switch (it->type) { 
     case TokenType::rparen: {
       // If we have a right parens, we are no longer in an expression.
@@ -364,7 +365,7 @@ std::unique_ptr<AstNode> Ast::parseGroup(std::vector<Token>::iterator& it,
 
 /** 
  * True if we need to reduce the stack.  We only allow logical_operations to trigger
- * reduce if we are betwen expressions (this happens at partse, not at parseExpression
+ * reduce if we are between expressions (this happens at parse, not at parseExpression
  */
 bool Ast::need_reduce(const std::stack<std::unique_ptr<AstNode>>& stack, bool allow_logical) {
   if (stack.empty()) {

core/parser/ast.h

@@ -71,14 +71,14 @@ class Expression : public AstNode {
   //Expression() : AstNode(AstType::EXPR), id_(++expression_id) {}
   Expression(std::unique_ptr<Expression>&& left, Operator op, std::unique_ptr<Expression>&& right);
 
-  Expression* left() const { return left_.get(); }
-  Expression* right() const { return right_.get(); }
-  Operator op() const noexcept { return op_; }
-  int id() const noexcept { return id_; }
-  virtual std::string ToString() const override;
-  virtual std::string ToString(bool include_children) const;
-  virtual std::string ToString(bool include_children, int indent) const;
-  virtual void accept(AstVisitor* visitor) override;
+  [[nodiscard]] Expression* left() const { return left_.get(); }
+  [[nodiscard]] Expression* right() const { return right_.get(); }
+  [[nodiscard]] Operator op() const noexcept { return op_; }
+  [[nodiscard]] int id() const noexcept { return id_; }
+  [[nodiscard]] std::string ToString() const override;
+  [[nodiscard]] virtual std::string ToString(bool include_children) const;
+  [[nodiscard]] virtual std::string ToString(bool include_children, int indent) const;
+  void accept(AstVisitor* visitor) override;
 
   // Used when constructing these.
   // TODO(rushfan): Move into constructor and construct these fully.

core_test/parser/ast_test.cpp

@@ -115,15 +115,15 @@ TEST_F(AstTest, Expr_Parens) {
   auto* root = ast.root();
   VLOG(1) << root->ToString();
 
-  auto expr = dynamic_cast<Expression*>(root);
+  auto* expr = dynamic_cast<Expression*>(root);
   CHECK_NOTNULL(expr);
   EXPECT_TRUE(HasOp(expr, Operator::logical_or)) << root->ToString();
   ASSERT_NE(nullptr, expr);
 
-  auto left = dynamic_cast<Expression*>(expr->left());
+  auto* left = dynamic_cast<Expression*>(expr->left());
   VLOG(1) << "Left: " << left->ToString();
   EXPECT_TRUE(HasExpression(left, "user.sl", Operator::gt, "200"));
-  auto right = dynamic_cast<Expression*>(expr->right());
+  auto* right = dynamic_cast<Expression*>(expr->right());
   VLOG(1) << "Right: " << right->ToString();
   EXPECT_TRUE(HasExpression(right, "user.ar", Operator::eq, "A"));
 }

sdk/acs/eval.cpp

@@ -62,7 +62,29 @@ std::optional<Value> Eval::to_value(Factor* n) {
   throw eval_error(fmt::format("Error finding factor for object: '{}'.", to_string(*n)));
 }
 
-Eval::Eval(std::string expression) : expression_(std::move(expression)) {}
+/** Shorthand to create an optional Value */
+template <typename T> static std::optional<Value> val(T&& v) {
+  return std::make_optional<Value>(std::forward<T>(v));
+}
+
+class DefaultValueProvider : public ValueProvider {
+public:
+  DefaultValueProvider() : ValueProvider("") {}
+  [[nodiscard]] std::optional<Value> value(const std::string& name) override {
+    if (name == "true") {
+      return val(true);
+    }
+    if (name == "false") {
+      return val(false);
+    }
+    return std::nullopt;
+  }
+
+};
+
+Eval::Eval(std::string expression) : expression_(std::move(expression)) {
+  add("", std::make_unique<DefaultValueProvider>());  
+}
 
 
 void Eval::visit(Expression* n) { 

sdk/acs/expr.cpp

@@ -58,7 +58,15 @@ std::string AcsExpr::get() {
   }
 
   if (regnum_) {
-    expr.emplace_back("user.regnum == 'true'");
+    expr.emplace_back("user.regnum == true");
+  }
+
+  if (sysop_) {
+    expr.emplace_back("user.sysop == true");
+  }
+
+  if (cosysop_) {
+    expr.emplace_back("user.cosysop == true");
   }
 
   bool first{true};

sdk/acs/expr.h

@@ -82,8 +82,19 @@ class AcsExpr final {
     }
     return *this;
   }
-  AcsExpr& regnum() {
-    regnum_ = true;
+
+  AcsExpr& regnum(bool b) {
+    regnum_ = b;
+    return *this;
+  }
+
+  AcsExpr& sysop(bool b) {
+    sysop_ = b;
+    return *this;
+  }
+
+  AcsExpr& cosysop(bool b) {
+    cosysop_ = b;
     return *this;
   }
 
@@ -102,6 +113,8 @@ class AcsExpr final {
   int min_dsl_{0};
   int max_dsl_{255};
   bool regnum_{false};
+  bool sysop_{false};
+  bool cosysop_{false};
 };
 
 }

sdk/acs/uservalueprovider.cpp

@@ -55,6 +55,15 @@ std::optional<Value> UserValueProvider::value(const std::string& name) {
   if (iequals(name, "name")) {
     return val(user_->GetName());
   }
+  if (iequals(name, "regnum")) {
+    return val(user_->GetWWIVRegNumber() != 0);
+  }
+  if (iequals(name, "sysop")) {
+    return val(user_->GetSl() == 255);
+  }
+  if (iequals(name, "cosysop")) {
+    return val((sl_.ability & ability_cosysop) != 0);
+  }
   throw eval_error(fmt::format("No user attribute named 'user.{}' exists.", name));
 }
 

sdk/acs/uservalueprovider.h

@@ -35,13 +35,14 @@ class UserValueProvider final : public ValueProvider {
    * Constructs a new ValueProvider.  'user' must remain valid for 
    * the duration of this instance lifetime.
    */
-  explicit UserValueProvider(User* user, int effective_sl)
-  : ValueProvider("user"), user_(user), effective_sl_(effective_sl) {}
+  UserValueProvider(User* user, int effective_sl, slrec sl)
+  : ValueProvider("user"), user_(user), effective_sl_(effective_sl), sl_(sl) {}
   [[nodiscard]] std::optional<Value> value(const std::string& name) override;
 
 private:
   User* user_;
   int effective_sl_;
+  slrec sl_;
 };
 
 }

sdk/menus/menu.cpp

@@ -132,7 +132,9 @@ std::optional<Menu56> Create56MenuFrom43(const Menu430& m4) {
   h.title = oh.szMenuTitle;
   {
     acs::AcsExpr ae;
-    h.acs = ae.min_dsl(oh.nMinDSL).dar_int(oh.uDAR).min_sl(oh.nMinSL).ar_int(oh.uAR).get();
+    ae.min_dsl(oh.nMinDSL).dar_int(oh.uDAR).min_sl(oh.nMinSL).ar_int(oh.uAR);
+    ae.sysop(oh.nSysop).cosysop(oh.nCoSysop);
+    h.acs = ae.get();
   }
   h.password = oh.szPassWord;
 
@@ -149,6 +151,7 @@ std::optional<Menu56> Create56MenuFrom43(const Menu430& m4) {
       // TODO(rushfan): Add support for co-sysop, sysop, restrict into ACS.
       acs::AcsExpr ae;
       ae.min_sl(o.nMinSL).max_sl(o.iMaxSL).min_dsl(o.nMinDSL).max_dsl(o.iMaxDSL).ar_int(o.uAR).dar_int(o.uDAR);
+      ae.sysop(o.nSysop).cosysop(o.nCoSysop);
       i.acs = ae.get();
     }
 

sdk_test/acs/acs_test.cpp

@@ -35,15 +35,16 @@ using namespace wwiv::sdk::acs;
 
 class AcsTest : public ::testing::Test {
 public:
-  AcsTest() {}
+  AcsTest() = default;
 
   void createEval(const std::string& expr) { 
     eval = std::make_unique<Eval>(expr);
-    eval->add("user", std::make_unique<UserValueProvider>(&user_, user_.GetSl()));
+    eval->add("user", std::make_unique<UserValueProvider>(&user_, user_.GetSl(), sl_));
 
   }
   std::unique_ptr<Eval> eval;
-  wwiv::sdk::User user_;
+  wwiv::sdk::User user_{};
+  slrec sl_{};
 };
 
 TEST_F(AcsTest, SL_GT_Pass) {
@@ -127,4 +128,56 @@ TEST_F(AcsTest, BadExpression) {
   createEval("foo == ~ foo");
   EXPECT_FALSE(eval->eval());
   LOG(INFO) << wwiv::strings::JoinStrings(eval->debug_info(), "\r\n");
-}
+}
+
+TEST_F(AcsTest, Sysop_Pass) {
+  user_.SetSl(255);
+  createEval("user.sysop == \"true\"");
+  EXPECT_TRUE(eval->eval());
+}
+
+TEST_F(AcsTest, Sysop_Pass_Literal) {
+  user_.SetSl(255);
+  createEval("user.sysop == true");
+  EXPECT_TRUE(eval->eval());
+}
+
+TEST_F(AcsTest, Sysop_Fail) {
+  user_.SetSl(200);
+  createEval("user.sysop == true");
+  EXPECT_FALSE(eval->eval());
+}
+
+TEST_F(AcsTest, Sysop_Pass_Negated) {
+  user_.SetSl(200);
+  createEval("user.sysop == false");
+  EXPECT_TRUE(eval->eval());
+}
+
+TEST_F(AcsTest, Regnum_Pass) {
+  user_.SetSl(12);
+  user_.SetWWIVRegNumber(12345);
+  createEval("user.regnum == true");
+  EXPECT_TRUE(eval->eval());
+}
+
+TEST_F(AcsTest, Regnum_Fail) {
+  user_.SetSl(12);
+  user_.SetWWIVRegNumber(0);
+  createEval("user.regnum == true");
+  EXPECT_FALSE(eval->eval());
+}
+
+TEST_F(AcsTest, CoSysop_Pass) {
+  user_.SetSl(200);
+  sl_.ability |= ability_cosysop;
+  createEval("user.cosysop == true");
+  EXPECT_TRUE(eval->eval());
+}
+
+TEST_F(AcsTest, CoSysop_Fail) {
+  user_.SetSl(200);
+  sl_.ability &= ~ability_cosysop;
+  createEval("user.cosysop == true");
+  EXPECT_FALSE(eval->eval());
+}

sdk_test/acs/expr_test.cpp

@@ -68,9 +68,9 @@ TEST_F(AcsExprTest, DSL_Min_Max) {
 
 TEST_F(AcsExprTest, DSL_Min_255) { EXPECT_EQ(ae.min_dsl(255).get(), "user.dsl >= 255"); }
 
-TEST_F(AcsExprTest, RegNum) { EXPECT_EQ(ae.regnum().get(), "user.regnum == 'true'"); }
+TEST_F(AcsExprTest, RegNum) { EXPECT_EQ(ae.regnum(true).get(), "user.regnum == true"); }
 
 TEST_F(AcsExprTest, MultipleConditions) {
-  EXPECT_EQ(ae.min_dsl(18).max_dsl(21).dar('A').regnum().get(),
-            "user.dar == 'A' && user.dsl >= 18 && user.dsl <= 21 && user.regnum == 'true'");
+  EXPECT_EQ(ae.min_dsl(18).max_dsl(21).dar('A').regnum(true).get(),
+            "user.dar == 'A' && user.dsl >= 18 && user.dsl <= 21 && user.regnum == true");
 }

wwivutil/acs/acs.cpp

@@ -66,7 +66,9 @@ int AcsCommand::Execute() {
   }
 
   Eval eval(expr);
-  eval.add("user", std::make_unique<UserValueProvider>(&user, user.GetSl()));
+  const auto sl = user.GetSl();
+  const auto& slr = config()->config()->sl(sl);
+  eval.add("user", std::make_unique<UserValueProvider>(&user, sl, slr));
 
   bool result = eval.eval();
   std::cout << "Evaluate: '" << expr << "' ";