"seal" rules after ruleengine using them is run() the first time.

this prevents accidental modification of rules after the containing ruleengine starts getting used.

also switch to a LinkedHashMap for storing rules and tweak some of the methods used in unit tests.

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

@@ -57,6 +57,7 @@ enum TriggerType {
             Maps.newEnumMap(TriggerType.class);
 
     private TriggerType currentTriggerType = TriggerType.NONE;
+    private boolean sealed = false;
 
     /* package */ Rule(
             String name, BooleanSupplier predicate, Supplier<Procedure> newlyTriggeringProcedure) {
@@ -79,6 +80,11 @@ enum TriggerType {
 
     /** Specify a creator for the Procedure that should be run when this rule was triggering before and is no longer triggering. */
     public Rule withFinishedTriggeringProcedure(Supplier<Procedure> action) {
+        if (sealed) {
+            throw new IllegalStateException(
+                    "Cannot modify rules once they've been evaluated in the RuleEngine");
+        }
+
         triggerProcedures.put(TriggerType.FINISHED, action);
         triggerReservations.put(TriggerType.FINISHED, getReservationsForProcedure(action));
         return this;
@@ -107,6 +113,10 @@ public String getName() {
         return currentTriggerType;
     }
 
+    /* package */ void seal() {
+        sealed = true;
+    }
+
     /* package */ void reset() {
         currentTriggerType = TriggerType.NONE;
     }

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

@@ -10,8 +10,7 @@
 import edu.wpi.first.wpilibj2.command.CommandScheduler;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.LinkedList;
-import java.util.List;
+import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.BooleanSupplier;
@@ -32,9 +31,10 @@ public class RuleEngine implements LoggingBase {
 
     private static record RuleAction(Rule rule, Rule.TriggerType triggerType) {}
 
-    private final List<Rule> rules = new LinkedList<>();
+    private final LinkedHashMap<String, Rule> rules = new LinkedHashMap<>();
     private final Map<Rule, Integer> rulePriorities = new HashMap<>();
     private BiMap<Command, RuleAction> ruleMap = HashBiMap.create();
+    private boolean sealed = false;
 
     protected RuleEngine() {}
 
@@ -45,7 +45,7 @@ public Category getLoggerCategory() {
 
     protected Rule addRule(String name, BooleanSupplier condition, Supplier<Procedure> action) {
         Rule rule = new Rule(name, condition, action);
-        rules.add(rule);
+        rules.put(name, rule);
         int priority = rulePriorities.size();
         rulePriorities.put(rule, priority);
         return rule;
@@ -58,12 +58,13 @@ protected Rule addRule(
     }
 
     @VisibleForTesting
-    /* package */ Map<String, Rule> getRuleNameMap() {
-        Map<String, Rule> namedRules = new HashMap<>();
-        for (Rule rule : rules) {
-            namedRules.put(rule.getName(), rule);
-        }
-        return namedRules;
+    /* package */ int size() {
+        return rules.size();
+    }
+
+    @VisibleForTesting
+    /* package */ Rule getRuleByName(String name) {
+        return rules.get(name);
     }
 
     @VisibleForTesting
@@ -82,15 +83,26 @@ protected Rule getRuleForTriggeredProcedure(Command command) {
         return (ruleAction == null) ? null : ruleAction.rule;
     }
 
+    private void sealRules() {
+        for (Rule rule : rules.values()) {
+            rule.seal();
+        }
+    }
+
     public final void run() {
+        if (!sealed) {
+            sealRules();
+            sealed = true;
+        }
+
         Set<Mechanism<?>> mechanismsToUse = new HashSet<>();
 
         // TODO(MF3): when creating a Procedure, check that the reservations are the same as
         // what the Rule pre-computed.
 
         // evaluate each rule
         ruleLoop:
-        for (Rule rule : rules) {
+        for (Rule rule : rules.values()) {
             try {
                 rule.evaluate();
 

src/test/java/com/team766/framework3/RuleEngineTest.java

@@ -3,12 +3,12 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import com.team766.TestCase3;
 import edu.wpi.first.wpilibj2.command.Command;
 import edu.wpi.first.wpilibj2.command.CommandScheduler;
-import java.util.Map;
 import java.util.Set;
 import java.util.function.BooleanSupplier;
 import org.junit.jupiter.api.Test;
@@ -58,6 +58,34 @@ public boolean getAsBoolean() {
     private final FakeMechanism2 fm2 = new FakeMechanism2();
     private final FakeMechanism3 fm3 = new FakeMechanism3();
 
+    @Test
+    public void testSeal() {
+        // test that we can modify rules before we call run
+        RuleEngine rulesOne =
+                new RuleEngine() {
+                    {
+                        addRule("rule1_1", () -> true, () -> Procedure.NO_OP)
+                                .withFinishedTriggeringProcedure(() -> Procedure.NO_OP);
+                    }
+                };
+        rulesOne.run();
+
+        // test that
+        RuleEngine rulesTwo =
+                new RuleEngine() {
+                    {
+                        addRule("rule2_1", () -> true, () -> Procedure.NO_OP);
+                    }
+                };
+        rulesTwo.run();
+
+        assertThrows(
+                IllegalStateException.class,
+                () ->
+                        rulesTwo.getRuleByName("rule2_1")
+                                .withFinishedTriggeringProcedure(() -> Procedure.NO_OP));
+    }
+
     @Test
     public void testAddRuleAndGetPriority() {
         // simply test that rules we add are added - and at the expected priority
@@ -77,12 +105,11 @@ public void testAddRuleAndGetPriority() {
                     }
                 };
 
-        Map<String, Rule> namedRules = myRules.getRuleNameMap();
         // make sure we have 2 rules
-        assertEquals(2, namedRules.size());
+        assertEquals(2, myRules.size());
         // with priorities based on insertion order, starting at 0
-        assertEquals(0, myRules.getPriorityForRule(namedRules.get("fm1_p0")));
-        assertEquals(1, myRules.getPriorityForRule(namedRules.get("fm1_p1")));
+        assertEquals(0, myRules.getPriorityForRule(myRules.getRuleByName("fm1_p0")));
+        assertEquals(1, myRules.getPriorityForRule(myRules.getRuleByName("fm1_p1")));
     }
 
     @Test