Hierarchical rules

src/main/java/com/team766/framework3/Rule.java

@@ -57,28 +57,36 @@ enum Cancellation {
         CANCEL_NEWLY_ACTION,
     }
 
+    private RuleGroupBase container;
     private final String name;
-    private final BooleanSupplier predicate;
+    private BooleanSupplier predicate;
     private final Map<TriggerType, Supplier<Procedure>> triggerProcedures =
             Maps.newEnumMap(TriggerType.class);
     private final Map<TriggerType, Set<Mechanism>> triggerReservations =
             Maps.newEnumMap(TriggerType.class);
-    private final Cancellation cancellationOnFinish;
+    private Cancellation cancellationOnFinish;
 
     private TriggerType currentTriggerType = TriggerType.NONE;
     private boolean sealed = false;
 
-    /* package */ Rule(
-            String name,
-            BooleanSupplier predicate,
-            RulePersistence rulePersistence,
-            Supplier<Procedure> onTriggeringProcedure) {
+    /* package */ Rule(RuleGroupBase container, String name, BooleanSupplier predicate) {
         if (predicate == null) {
             throw new IllegalArgumentException("Rule predicate has not been set.");
         }
 
-        if (onTriggeringProcedure == null) {
-            throw new IllegalArgumentException("On-triggering Procedure is not defined.");
+        this.container = container;
+        this.name = name;
+        this.predicate = predicate;
+    }
+
+    public Rule withOnTriggeringProcedure(
+            RulePersistence rulePersistence, Supplier<Procedure> onTriggeringProcedure) {
+        if (sealed) {
+            throw new IllegalStateException(
+                    "Cannot modify rules once they've been evaluated in the RuleEngine");
+        }
+        if (triggerProcedures.containsKey(TriggerType.NEWLY)) {
+            throw new IllegalStateException("This trigger already has an OnTriggering action");
         }
 
         final Supplier<Procedure> newlyTriggeringProcedure =
@@ -125,13 +133,22 @@ enum Cancellation {
                     }
                 };
 
-        this.name = name;
-        this.predicate = predicate;
-        if (newlyTriggeringProcedure != null) {
-            triggerProcedures.put(TriggerType.NEWLY, newlyTriggeringProcedure);
-            triggerReservations.put(
-                    TriggerType.NEWLY, getReservationsForProcedure(newlyTriggeringProcedure));
-        }
+        triggerProcedures.put(TriggerType.NEWLY, newlyTriggeringProcedure);
+        triggerReservations.put(
+                TriggerType.NEWLY, getReservationsForProcedure(newlyTriggeringProcedure));
+
+        return this;
+    }
+
+    public Rule withOnTriggeringProcedure(
+            RulePersistence rulePersistence, Set<Mechanism> reservations, Runnable action) {
+        return withOnTriggeringProcedure(
+                rulePersistence, () -> new FunctionalInstantProcedure(reservations, action));
+    }
+
+    public Rule withOnTriggeringProcedure(
+            RulePersistence rulePersistence, Mechanism reservation, Runnable action) {
+        return withOnTriggeringProcedure(rulePersistence, Set.of(reservation), action);
     }
 
     /** Specify a creator for the Procedure that should be run when this rule was triggering before and is no longer triggering. */
@@ -140,6 +157,10 @@ public Rule withFinishedTriggeringProcedure(Supplier<Procedure> action) {
             throw new IllegalStateException(
                     "Cannot modify rules once they've been evaluated in the RuleEngine");
         }
+        if (triggerProcedures.containsKey(TriggerType.FINISHED)) {
+            throw new IllegalStateException(
+                    "This trigger already has an FinishedTriggering action");
+        }
 
         triggerProcedures.put(TriggerType.FINISHED, action);
         triggerReservations.put(TriggerType.FINISHED, getReservationsForProcedure(action));
@@ -151,6 +172,55 @@ public Rule withFinishedTriggeringProcedure(Set<Mechanism> reservations, Runnabl
                 () -> new FunctionalInstantProcedure(reservations, action));
     }
 
+    public Rule withFinishedTriggeringProcedure(Mechanism reservation, Runnable action) {
+        return withFinishedTriggeringProcedure(Set.of(reservation), action);
+    }
+
+    /** Specify Rules which should only trigger when this Rule is also triggering. */
+    public Rule whenTriggering(RuleGroup rules) {
+        if (sealed) {
+            throw new IllegalStateException(
+                    "Cannot modify rules once they've been evaluated in the RuleEngine");
+        }
+        rules.mergeInto(container, this, true);
+        return this;
+    }
+
+    /** Specify Rules which should only trigger when this Rule is not triggering. */
+    public Rule whenNotTriggering(RuleGroup rules) {
+        if (sealed) {
+            throw new IllegalStateException(
+                    "Cannot modify rules once they've been evaluated in the RuleEngine");
+        }
+        rules.mergeInto(container, this, false);
+        return this;
+    }
+
+    /* package */ void attachTo(RuleGroupBase container, Rule parent, boolean triggerValue) {
+        if (sealed) {
+            throw new IllegalStateException(
+                    "Cannot modify rules once they've been evaluated in the RuleEngine");
+        }
+        this.container = container;
+        if (parent != null) {
+            final var previousPredicate = this.predicate;
+            this.predicate =
+                    triggerValue
+                            // Important! These composed predicates shouldn't invoke the parent's
+                            // `predicate`. Each Rule's `predicate` should be invoked only once
+                            // per call to RuleEngine.run(), so having all rules in the hierarchy
+                            // call it would not work as expected. Instead, we have the child rules
+                            // query the triggering state of the parent rule.
+                            // Also Important! The order of these conditions matters: we want the
+                            // user's predicate to be invoked only when this rule is active
+                            // (i.e. when its parent condition is satisfied), so we put the user's
+                            // predicate second, so it gets short-circuited when the rule is not
+                            // active.
+                            ? () -> parent.isTriggering() && previousPredicate.getAsBoolean()
+                            : () -> !parent.isTriggering() && previousPredicate.getAsBoolean();
+        }
+    }
+
     private static Set<Mechanism> getReservationsForProcedure(Supplier<Procedure> supplier) {
         if (supplier != null) {
             Procedure procedure = supplier.get();
@@ -169,6 +239,15 @@ public String getName() {
         return currentTriggerType;
     }
 
+    /* package */ boolean isTriggering() {
+        return switch (currentTriggerType) {
+            case NEWLY -> true;
+            case CONTINUING -> true;
+            case FINISHED -> false;
+            case NONE -> false;
+        };
+    }
+
     /* package */ void seal() {
         sealed = true;
     }

src/main/java/com/team766/framework3/RuleEngine.java

@@ -1,11 +1,8 @@
 package com.team766.framework3;
 
-import static com.team766.framework3.RulePersistence.ONCE_AND_HOLD;
-
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.BiMap;
 import com.google.common.collect.HashBiMap;
-import com.team766.logging.Category;
 import com.team766.logging.LoggerExceptionUtils;
 import com.team766.logging.Severity;
 import edu.wpi.first.wpilibj2.command.Command;
@@ -15,8 +12,6 @@
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Set;
-import java.util.function.BooleanSupplier;
-import java.util.function.Consumer;
 import java.util.function.Supplier;
 
 /**
@@ -30,7 +25,7 @@
  * For a {@link Rule} to trigger, its predicate must be satisfied -- and, the {@link Mechanism}s the corresponding {@link Procedure} would reserve
  * must not be in use or about to be in use from a higher priority {@link Rule}.
  */
-public class RuleEngine implements StatusesMixin, LoggingBase {
+public class RuleEngine extends RuleGroupBase {
 
     private static record RuleAction(Rule rule, Rule.TriggerType triggerType) {}
 
@@ -42,91 +37,14 @@ private static record RuleAction(Rule rule, Rule.TriggerType triggerType) {}
     protected RuleEngine() {}
 
     @Override
-    public Category getLoggerCategory() {
-        return Category.RULES;
-    }
-
-    protected Rule addRule(
-            String name,
-            BooleanSupplier condition,
-            RulePersistence rulePersistence,
-            Supplier<Procedure> action) {
-        Rule rule = new Rule(name, condition, rulePersistence, action);
-        rules.put(name, rule);
+    protected void addRule(Rule rule) {
+        if (sealed) {
+            throw new IllegalStateException(
+                    "Cannot add rules after the RuleEngine has started running");
+        }
+        rules.put(rule.getName(), rule);
         int priority = rulePriorities.size();
         rulePriorities.put(rule, priority);
-        return rule;
-    }
-
-    protected Rule addRule(String name, BooleanSupplier condition, Supplier<Procedure> action) {
-        return addRule(name, condition, ONCE_AND_HOLD, action);
-    }
-
-    protected Rule addRule(
-            String name,
-            BooleanSupplier condition,
-            RulePersistence rulePersistence,
-            Set<Mechanism> mechanisms,
-            Consumer<Context> action) {
-        return addRule(
-                name,
-                condition,
-                rulePersistence,
-                () -> new FunctionalProcedure(mechanisms, action));
-    }
-
-    protected Rule addRule(
-            String name,
-            BooleanSupplier condition,
-            Set<Mechanism> mechanisms,
-            Consumer<Context> action) {
-        return addRule(name, condition, ONCE_AND_HOLD, mechanisms, action);
-    }
-
-    protected Rule addRule(
-            String name,
-            BooleanSupplier condition,
-            RulePersistence rulePersistence,
-            Mechanism mechanism,
-            Consumer<Context> action) {
-        return addRule(name, condition, rulePersistence, Set.of(mechanism), action);
-    }
-
-    protected Rule addRule(
-            String name, BooleanSupplier condition, Mechanism mechanism, Consumer<Context> action) {
-        return addRule(name, condition, ONCE_AND_HOLD, mechanism, action);
-    }
-
-    protected Rule addRule(
-            String name,
-            BooleanSupplier condition,
-            RulePersistence rulePersistence,
-            Set<Mechanism> mechanisms,
-            Runnable action) {
-        return addRule(
-                name,
-                condition,
-                rulePersistence,
-                () -> new FunctionalInstantProcedure(mechanisms, action));
-    }
-
-    protected Rule addRule(
-            String name, BooleanSupplier condition, Set<Mechanism> mechanisms, Runnable action) {
-        return addRule(name, condition, ONCE_AND_HOLD, mechanisms, action);
-    }
-
-    protected Rule addRule(
-            String name,
-            BooleanSupplier condition,
-            RulePersistence rulePersistence,
-            Mechanism mechanism,
-            Runnable action) {
-        return addRule(name, condition, rulePersistence, Set.of(mechanism), action);
-    }
-
-    protected Rule addRule(
-            String name, BooleanSupplier condition, Mechanism mechanism, Runnable action) {
-        return addRule(name, condition, ONCE_AND_HOLD, mechanism, action);
     }
 
     @VisibleForTesting

src/main/java/com/team766/framework3/RuleGroup.java

@@ -0,0 +1,21 @@
+package com.team766.framework3;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class RuleGroup extends RuleGroupBase {
+    private final List<Rule> rules = new ArrayList<>();
+
+    @Override
+    /* package */ void addRule(Rule rule) {
+        rules.add(rule);
+    }
+
+    /* package */ void mergeInto(RuleGroupBase container, Rule parent, boolean triggerValue) {
+        for (var rule : rules) {
+            rule.attachTo(container, parent, triggerValue);
+            container.addRule(rule);
+        }
+        rules.clear();
+    }
+}