From f7a27c5d6ad5f55b12bb0c3e4b3efe3051501453 Mon Sep 17 00:00:00 2001 From: dejabot Date: Sun, 5 Jan 2025 02:19:23 -0800 Subject: [PATCH] remove Rule.Builder and simplify RuleEngine.addRule(), letting it return a Rule that can be modified further, eg with a finishedTriggeringProcedure. --- .../java/com/team766/framework3/Rule.java | 80 +----- .../com/team766/framework3/RuleEngine.java | 13 +- .../team766/framework3/RuleEngineTest.java | 270 +++++++----------- .../java/com/team766/framework3/RuleTest.java | 38 ++- 4 files changed, 141 insertions(+), 260 deletions(-) diff --git a/src/main/java/com/team766/framework3/Rule.java b/src/main/java/com/team766/framework3/Rule.java index 5999e7f9..f79efdc4 100644 --- a/src/main/java/com/team766/framework3/Rule.java +++ b/src/main/java/com/team766/framework3/Rule.java @@ -24,8 +24,8 @@ * public class MyRules extends RuleEngine { * public MyRules() { * // add rule to spin up the shooter when the boxop presses the right trigger on the gamepad - * rules.add(Rule.create("spin up shooter", gamepad.getButton(InputConstants.XBOX_RT)). - * withNewlyTriggeringProcedure(() -> new ShooterSpin(shooter))); + * rules.add("spin up shooter", gamepad.getButton(InputConstants.XBOX_RT), + * () -> new ShooterSpin(shooter))); * ... * } * } @@ -49,55 +49,6 @@ enum TriggerType { FINISHED } - /** - * Simple Builder for {@link Rule}s. Configure Rules via this Builder; these fields will be immutable - * in the rule the Builder constructs. - * - * Instances of this Builder are created via {@link Rule#create} to simplify syntax. - */ - public static class Builder { - private final String name; - private final BooleanSupplier predicate; - private Supplier newlyTriggeringProcedure; - private Supplier finishedTriggeringProcedure; - - private Builder(String name, BooleanSupplier predicate) { - this.name = name; - this.predicate = predicate; - } - - /** Specify a creator for the Procedure that should be run when this rule starts triggering. */ - public Builder withNewlyTriggeringProcedure(Supplier action) { - this.newlyTriggeringProcedure = action; - return this; - } - - public Builder withNewlyTriggeringProcedure( - Set> reservations, Runnable action) { - this.newlyTriggeringProcedure = - () -> new FunctionalInstantProcedure(reservations, action); - return this; - } - - /** Specify a creator for the Procedure that should be run when this rule was triggering before and is no longer triggering. */ - public Builder withFinishedTriggeringProcedure(Supplier action) { - this.finishedTriggeringProcedure = action; - return this; - } - - public Builder withFinishedTriggeringProcedure( - Set> reservations, Runnable action) { - this.finishedTriggeringProcedure = - () -> new FunctionalInstantProcedure(reservations, action); - return this; - } - - // called by {@link RuleEngine#addRule}. - /* package */ Rule build() { - return new Rule(name, predicate, newlyTriggeringProcedure, finishedTriggeringProcedure); - } - } - private final String name; private final BooleanSupplier predicate; private final Map> triggerProcedures = @@ -107,15 +58,8 @@ public Builder withFinishedTriggeringProcedure( private TriggerType currentTriggerType = TriggerType.NONE; - public static Builder create(String name, BooleanSupplier predicate) { - return new Builder(name, predicate); - } - - private Rule( - String name, - BooleanSupplier predicate, - Supplier newlyTriggeringProcedure, - Supplier finishedTriggeringProcedure) { + /* package */ Rule( + String name, BooleanSupplier predicate, Supplier newlyTriggeringProcedure) { if (predicate == null) { throw new IllegalArgumentException("Rule predicate has not been set."); } @@ -131,12 +75,18 @@ private Rule( triggerReservations.put( TriggerType.NEWLY, getReservationsForProcedure(newlyTriggeringProcedure)); } + } - if (finishedTriggeringProcedure != null) { - triggerProcedures.put(TriggerType.FINISHED, finishedTriggeringProcedure); - triggerReservations.put( - TriggerType.FINISHED, getReservationsForProcedure(finishedTriggeringProcedure)); - } + /** 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 action) { + triggerProcedures.put(TriggerType.FINISHED, action); + triggerReservations.put(TriggerType.FINISHED, getReservationsForProcedure(action)); + return this; + } + + public Rule withFinishedTriggeringProcedure(Set> reservations, Runnable action) { + return withFinishedTriggeringProcedure( + () -> new FunctionalInstantProcedure(reservations, action)); } private Set> getReservationsForProcedure(Supplier supplier) { diff --git a/src/main/java/com/team766/framework3/RuleEngine.java b/src/main/java/com/team766/framework3/RuleEngine.java index 78d7ed3d..4edea0c2 100644 --- a/src/main/java/com/team766/framework3/RuleEngine.java +++ b/src/main/java/com/team766/framework3/RuleEngine.java @@ -14,6 +14,8 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.BooleanSupplier; +import java.util.function.Supplier; /** * {@link RuleEngine}s manage and process a set of {@link Rule}s. Subclasses should add rules via @@ -41,11 +43,18 @@ public Category getLoggerCategory() { return Category.RULES; } - protected void addRule(Rule.Builder builder) { - Rule rule = builder.build(); + protected Rule addRule(String name, BooleanSupplier condition, Supplier action) { + Rule rule = new Rule(name, condition, action); rules.add(rule); int priority = rulePriorities.size(); rulePriorities.put(rule, priority); + return rule; + } + + protected Rule addRule( + String name, BooleanSupplier condition, Mechanism mechanism, Runnable action) { + return addRule( + name, condition, () -> new FunctionalInstantProcedure(Set.of(mechanism), action)); } @VisibleForTesting diff --git a/src/test/java/com/team766/framework3/RuleEngineTest.java b/src/test/java/com/team766/framework3/RuleEngineTest.java index aa496c90..e6a79702 100644 --- a/src/test/java/com/team766/framework3/RuleEngineTest.java +++ b/src/test/java/com/team766/framework3/RuleEngineTest.java @@ -67,13 +67,13 @@ public void testAddRuleAndGetPriority() { new RuleEngine() { { addRule( - Rule.create("fm1_p0", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> new FakeProcedure(2, Set.of(fm1)))); + "fm1_p0", + new ScheduledPredicate(0), + () -> new FakeProcedure(2, Set.of(fm1))); addRule( - Rule.create("fm1_p1", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> new FakeProcedure(2, Set.of(fm1)))); + "fm1_p1", + new ScheduledPredicate(0), + () -> new FakeProcedure(2, Set.of(fm1))); } }; @@ -96,13 +96,13 @@ public void testRunNonConflictingRules() { new RuleEngine() { { addRule( - Rule.create("fm1", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> new FakeProcedure(2, Set.of(fm1)))); + "fm1", + new ScheduledPredicate(0), + () -> new FakeProcedure(2, Set.of(fm1))); addRule( - Rule.create("fm2", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> new FakeProcedure(2, Set.of(fm2)))); + "fm2", + new ScheduledPredicate(0), + () -> new FakeProcedure(2, Set.of(fm2))); } }; @@ -146,17 +146,13 @@ public void testFinishedProcedureBumpsNewlyProcedureForSameRule() { new RuleEngine() { { addRule( - Rule.create("fm1_p0", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p0", 1, Set.of(fm1))) - .withFinishedTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procfin_p0", - 1, - Set.of(fm1, fm2)))); + "fm1_p0", + new ScheduledPredicate(0), + () -> new FakeProcedure("fm1procnew_p0", 1, Set.of(fm1))) + .withFinishedTriggeringProcedure( + () -> + new FakeProcedure( + "fm1procfin_p0", 1, Set.of(fm1, fm2))); } }; @@ -188,28 +184,18 @@ public void testRunRulePriorities() { new RuleEngine() { { addRule( - Rule.create("fm1_p0", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1proc_p0", - 0, - Set.of(fm1, fm2)))); + "fm1_p0", + new ScheduledPredicate(0), + () -> new FakeProcedure("fm1proc_p0", 0, Set.of(fm1, fm2))); addRule( - Rule.create("fm1_p1", new PeriodicPredicate(2)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1proc_p1", - 0, - Set.of(fm1, fm3)))); + "fm1_p1", + new PeriodicPredicate(2), + () -> new FakeProcedure("fm1proc_p1", 0, Set.of(fm1, fm3))); addRule( - Rule.create("fm3_p2", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm3proc_p2", 0, Set.of(fm3)))); + "fm3_p2", + new ScheduledPredicate(0), + () -> new FakeProcedure("fm3proc_p2", 0, Set.of(fm3))); } }; @@ -250,30 +236,18 @@ public void testRunHigherPriorityRuleStillBeingRun() { new RuleEngine() { { addRule( - Rule.create("fm1_p0", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1proc_p0", - 2, - Set.of(fm1, fm2)))); + "fm1_p0", + new ScheduledPredicate(0), + () -> new FakeProcedure("fm1proc_p0", 2, Set.of(fm1, fm2))); addRule( - Rule.create("fm1_p1", new ScheduledPredicate(1)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1proc_p1", - 2, - Set.of(fm1, fm2)))); + "fm1_p1", + new ScheduledPredicate(1), + () -> new FakeProcedure("fm1proc_p1", 2, Set.of(fm1, fm2))); addRule( - Rule.create("fm1_p2", new ScheduledPredicate(3)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1proc_p2", - 2, - Set.of(fm1, fm2)))); + "fm1_p2", + new ScheduledPredicate(3), + () -> new FakeProcedure("fm1proc_p2", 2, Set.of(fm1, fm2))); } }; @@ -321,21 +295,13 @@ public void testRunLowerPriorityRuleBumped() { new RuleEngine() { { addRule( - Rule.create("fm1_p0", new ScheduledPredicate(1)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1proc_p0", - 2, - Set.of(fm1, fm2)))); + "fm1_p0", + new ScheduledPredicate(1), + () -> new FakeProcedure("fm1proc_p0", 2, Set.of(fm1, fm2))); addRule( - Rule.create("fm1_p1", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1proc_p1", - 4, - Set.of(fm1, fm2)))); + "fm1_p1", + new ScheduledPredicate(0), + () -> new FakeProcedure("fm1proc_p1", 4, Set.of(fm1, fm2))); } }; @@ -363,21 +329,15 @@ public void testRuleResetIgnoredLowerPriorityRule() { new RuleEngine() { { addRule( - Rule.create("fm1_p0", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p0", 2, Set.of(fm1)))); + "fm1_p0", + new ScheduledPredicate(0), + () -> new FakeProcedure("fm1procnew_p0", 2, Set.of(fm1))); addRule( - Rule.create("fm1_p1", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p1", 1, Set.of(fm1))) - .withFinishedTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procfin_p1", 1, Set.of(fm2)))); + "fm1_p1", + new ScheduledPredicate(0), + () -> new FakeProcedure("fm1procnew_p1", 1, Set.of(fm1))) + .withFinishedTriggeringProcedure( + () -> new FakeProcedure("fm1procfin_p1", 1, Set.of(fm2))); } }; @@ -404,21 +364,15 @@ public void testRuleResetIgnoredLowerPriorityRuleHigherPriorityRulePreviouslySch new RuleEngine() { { addRule( - Rule.create("fm1_p0", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p0", 2, Set.of(fm1)))); + "fm1_p0", + new ScheduledPredicate(0), + () -> new FakeProcedure("fm1procnew_p0", 2, Set.of(fm1))); addRule( - Rule.create("fm1_p1", new ScheduledPredicate(1)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p1", 1, Set.of(fm1))) - .withFinishedTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procfin_p1", 1, Set.of(fm2)))); + "fm1_p1", + new ScheduledPredicate(1), + () -> new FakeProcedure("fm1procnew_p1", 1, Set.of(fm1))) + .withFinishedTriggeringProcedure( + () -> new FakeProcedure("fm1procfin_p1", 1, Set.of(fm2))); } }; @@ -453,21 +407,15 @@ public void testRuleResetBumpedLowerPriorityRule() { new RuleEngine() { { addRule( - Rule.create("fm1_p0", new ScheduledPredicate(1)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p0", 2, Set.of(fm1)))); + "fm1_p0", + new ScheduledPredicate(1), + () -> new FakeProcedure("fm1procnew_p0", 2, Set.of(fm1))); addRule( - Rule.create("fm1_p1", new ScheduledPredicate(0)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p1", 2, Set.of(fm1))) - .withFinishedTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procfin_p1", 2, Set.of(fm2)))); + "fm1_p1", + new ScheduledPredicate(0), + () -> new FakeProcedure("fm1procnew_p1", 2, Set.of(fm1))) + .withFinishedTriggeringProcedure( + () -> new FakeProcedure("fm1procfin_p1", 2, Set.of(fm2))); } }; @@ -494,25 +442,17 @@ public void testLowerPriorityRuleRunsWhenProcedureFromHigherPriorityRuleFinishes new RuleEngine() { { addRule( - Rule.create("fm1_p0", new ScheduledPredicate(0, 4)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p0", 0, Set.of(fm1))) - .withFinishedTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procfin_p0", 0, Set.of(fm1)))); + "fm1_p0", + new ScheduledPredicate(0, 4), + () -> new FakeProcedure("fm1procnew_p0", 0, Set.of(fm1))) + .withFinishedTriggeringProcedure( + () -> new FakeProcedure("fm1procfin_p0", 0, Set.of(fm1))); addRule( - Rule.create("fm1_p1", new ScheduledPredicate(1)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p1", 0, Set.of(fm1))) - .withFinishedTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procfin_p1", 0, Set.of(fm1)))); + "fm1_p1", + new ScheduledPredicate(1), + () -> new FakeProcedure("fm1procnew_p1", 0, Set.of(fm1))) + .withFinishedTriggeringProcedure( + () -> new FakeProcedure("fm1procfin_p1", 0, Set.of(fm1))); } }; @@ -563,25 +503,17 @@ public void testRuleCalledAgainAfterBeingReset() { new RuleEngine() { { addRule( - Rule.create("fm1_p0", new ScheduledPredicate(1)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p0", 0, Set.of(fm1))) - .withFinishedTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procfin_p0", 0, Set.of(fm1)))); + "fm1_p0", + new ScheduledPredicate(1), + () -> new FakeProcedure("fm1procnew_p0", 0, Set.of(fm1))) + .withFinishedTriggeringProcedure( + () -> new FakeProcedure("fm1procfin_p0", 0, Set.of(fm1))); addRule( - Rule.create("fm1_p1", new ScheduledPredicate(0, 4)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p1", 1, Set.of(fm1))) - .withFinishedTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procfin_p1", 1, Set.of(fm1)))); + "fm1_p1", + new ScheduledPredicate(0, 4), + () -> new FakeProcedure("fm1procnew_p1", 1, Set.of(fm1))) + .withFinishedTriggeringProcedure( + () -> new FakeProcedure("fm1procfin_p1", 1, Set.of(fm1))); } }; @@ -633,25 +565,17 @@ public void testRuleResetPreventsFinishedForLongTrigger() { new RuleEngine() { { addRule( - Rule.create("fm1_p0", new ScheduledPredicate(1)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p0", 0, Set.of(fm1))) - .withFinishedTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procfin_p0", 0, Set.of(fm1)))); + "fm1_p0", + new ScheduledPredicate(1), + () -> new FakeProcedure("fm1procnew_p0", 0, Set.of(fm1))) + .withFinishedTriggeringProcedure( + () -> new FakeProcedure("fm1procfin_p0", 0, Set.of(fm1))); addRule( - Rule.create("fm1_p1", new ScheduledPredicate(0, 3)) - .withNewlyTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procnew_p1", 1, Set.of(fm1))) - .withFinishedTriggeringProcedure( - () -> - new FakeProcedure( - "fm1procfin_p1", 1, Set.of(fm1)))); + "fm1_p1", + new ScheduledPredicate(0, 3), + () -> new FakeProcedure("fm1procnew_p1", 1, Set.of(fm1))) + .withFinishedTriggeringProcedure( + () -> new FakeProcedure("fm1procfin_p1", 1, Set.of(fm1))); } }; diff --git a/src/test/java/com/team766/framework3/RuleTest.java b/src/test/java/com/team766/framework3/RuleTest.java index 3b7730a8..b37a5425 100644 --- a/src/test/java/com/team766/framework3/RuleTest.java +++ b/src/test/java/com/team766/framework3/RuleTest.java @@ -41,10 +41,7 @@ protected void setUp() {} @Test public void testCreate() { - Rule alwaysTrue = - Rule.create("always true", () -> true) - .withNewlyTriggeringProcedure(() -> Procedure.NO_OP) - .build(); + Rule alwaysTrue = new Rule("always true", () -> true, () -> Procedure.NO_OP); assertNotNull(alwaysTrue); assertEquals("always true", alwaysTrue.getName()); } @@ -52,10 +49,7 @@ public void testCreate() { @Test public void testEvaluate() { // start with simple test of a NONE->NEWLY->CONTINUING->CONTINUING sequence - Rule alwaysTrue = - Rule.create("always true", () -> true) - .withNewlyTriggeringProcedure(() -> Procedure.NO_OP) - .build(); + Rule alwaysTrue = new Rule("always true", () -> true, () -> Procedure.NO_OP); assertEquals(Rule.TriggerType.NONE, alwaysTrue.getCurrentTriggerType()); alwaysTrue.evaluate(); assertEquals(TriggerType.NEWLY, alwaysTrue.getCurrentTriggerType()); @@ -66,9 +60,10 @@ public void testEvaluate() { // test a full cycle: NONE->NEWLY->CONTINUING->FINISHED->NONE->NEWLY->... Rule duckDuckGooseGoose = - Rule.create("duck duck goose goose", new DuckDuckGooseGoosePredicate()) - .withNewlyTriggeringProcedure(() -> Procedure.NO_OP) - .build(); + new Rule( + "duck duck goose goose", + new DuckDuckGooseGoosePredicate(), + () -> Procedure.NO_OP); assertEquals(Rule.TriggerType.NONE, duckDuckGooseGoose.getCurrentTriggerType()); duckDuckGooseGoose.evaluate(); assertEquals(TriggerType.NEWLY, duckDuckGooseGoose.getCurrentTriggerType()); @@ -89,10 +84,11 @@ public void testGetMechanismsToReserve() { final Set> finishedMechanisms = Set.of(new FakeMechanism()); Rule duckDuckGooseGoose = - Rule.create("duck duck goose goose", new DuckDuckGooseGoosePredicate()) - .withNewlyTriggeringProcedure(newlyMechanisms, () -> {}) - .withFinishedTriggeringProcedure(finishedMechanisms, () -> {}) - .build(); + new Rule( + "duck duck goose goose", + new DuckDuckGooseGoosePredicate(), + () -> new FunctionalInstantProcedure(newlyMechanisms, () -> {})) + .withFinishedTriggeringProcedure(finishedMechanisms, () -> {}); // NONE assertEquals(Collections.emptySet(), duckDuckGooseGoose.getMechanismsToReserve()); @@ -101,12 +97,13 @@ public void testGetMechanismsToReserve() { duckDuckGooseGoose.evaluate(); assertEquals(newlyMechanisms, duckDuckGooseGoose.getMechanismsToReserve()); - // nothing between NEWLLY and FINISHED + // nothing between NEWLY and FINISHED duckDuckGooseGoose.evaluate(); assertEquals(Collections.emptySet(), duckDuckGooseGoose.getMechanismsToReserve()); // FINISHED duckDuckGooseGoose.evaluate(); + System.out.println("X: " + duckDuckGooseGoose.toString()); assertEquals(finishedMechanisms, duckDuckGooseGoose.getMechanismsToReserve()); // check NONE again @@ -121,10 +118,11 @@ public void testGetMechanismsToReserve() { @Test public void testGetProcedureToRun() { Rule duckDuckGooseGoose = - Rule.create("duck duck goose goose", new DuckDuckGooseGoosePredicate()) - .withNewlyTriggeringProcedure(() -> new TrivialProcedure("newly")) - .withFinishedTriggeringProcedure(() -> new TrivialProcedure("finished")) - .build(); + new Rule( + "duck duck goose goose", + new DuckDuckGooseGoosePredicate(), + () -> new TrivialProcedure("newly")) + .withFinishedTriggeringProcedure(() -> new TrivialProcedure("finished")); // NONE assertNull(duckDuckGooseGoose.getProcedureToRun());