From 850a86ae002b2cea53b3e3ab45977ff941c97abd Mon Sep 17 00:00:00 2001 From: Ryan Cahoon Date: Wed, 11 Dec 2024 03:47:02 -0800 Subject: [PATCH] Hierarchical rules --- .../java/com/team766/framework3/Rule.java | 72 +++++++- .../com/team766/framework3/RuleEngine.java | 11 +- .../team766/framework3/RuleEngineTest.java | 166 ++++++++++++++++++ .../java/com/team766/framework3/RuleTest.java | 68 ++++--- 4 files changed, 279 insertions(+), 38 deletions(-) diff --git a/src/main/java/com/team766/framework3/Rule.java b/src/main/java/com/team766/framework3/Rule.java index c8cfe6b8..9510f5c9 100644 --- a/src/main/java/com/team766/framework3/Rule.java +++ b/src/main/java/com/team766/framework3/Rule.java @@ -1,7 +1,10 @@ package com.team766.framework3; import com.google.common.collect.Maps; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.function.BooleanSupplier; @@ -70,6 +73,8 @@ public static class Builder { private Supplier onTriggeringProcedure; private Cancellation cancellationOnFinish = Cancellation.DO_NOT_CANCEL; private Supplier finishedTriggeringProcedure; + private final List composedRules = new ArrayList<>(); + private final List negatedComposedRules = new ArrayList<>(); private Builder(String name, BooleanSupplier predicate) { this.name = name; @@ -157,14 +162,58 @@ public Builder withFinishedTriggeringProcedure( return this; } + /** Specify Rules which should only trigger when this Rule is also triggering. */ + public Builder whenTriggering(Rule.Builder... rules) { + composedRules.addAll(Arrays.asList(rules)); + return this; + } + + /** Specify Rules which should only trigger when this Rule is not triggering. */ + public Builder whenNotTriggering(Rule.Builder... rules) { + negatedComposedRules.addAll(Arrays.asList(rules)); + return this; + } + // called by {@link RuleEngine#addRule}. - /* package */ Rule build() { - return new Rule( - name, - predicate, - onTriggeringProcedure, - cancellationOnFinish, - finishedTriggeringProcedure); + /* package */ List build() { + return build(null); + } + + private List build(BooleanSupplier parentPredicate) { + final var rules = new ArrayList(); + + final BooleanSupplier fullPredicate = + parentPredicate == null + ? predicate + : () -> parentPredicate.getAsBoolean() && predicate.getAsBoolean(); + final var thisRule = + new Rule( + name, + fullPredicate, + onTriggeringProcedure, + cancellationOnFinish, + finishedTriggeringProcedure); + rules.add(thisRule); + + // Important! These composed predicates shouldn't invoke `predicate`. `predicate` should + // only be invoked 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. + final BooleanSupplier composedPredicate = + parentPredicate == null + ? () -> thisRule.isTriggering() + : () -> parentPredicate.getAsBoolean() && thisRule.isTriggering(); + final BooleanSupplier negativeComposedPredicate = + parentPredicate == null + ? () -> !thisRule.isTriggering() + : () -> parentPredicate.getAsBoolean() && !thisRule.isTriggering(); + for (var r : composedRules) { + rules.addAll(r.build(composedPredicate)); + } + for (var r : negatedComposedRules) { + rules.addAll(r.build(negativeComposedPredicate)); + } + return rules; } } @@ -231,6 +280,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 reset() { currentTriggerType = TriggerType.NONE; } diff --git a/src/main/java/com/team766/framework3/RuleEngine.java b/src/main/java/com/team766/framework3/RuleEngine.java index cab1c05b..56067cfe 100644 --- a/src/main/java/com/team766/framework3/RuleEngine.java +++ b/src/main/java/com/team766/framework3/RuleEngine.java @@ -41,11 +41,12 @@ public Category getLoggerCategory() { return Category.RULES; } - protected void addRule(Rule.Builder builder) { - Rule rule = builder.build(); - rules.add(rule); - int priority = rulePriorities.size(); - rulePriorities.put(rule, priority); + public void addRule(Rule.Builder builder) { + for (Rule rule : builder.build()) { + rules.add(rule); + int priority = rulePriorities.size(); + rulePriorities.put(rule, priority); + } } @VisibleForTesting diff --git a/src/test/java/com/team766/framework3/RuleEngineTest.java b/src/test/java/com/team766/framework3/RuleEngineTest.java index 0e909736..5806f7d7 100644 --- a/src/test/java/com/team766/framework3/RuleEngineTest.java +++ b/src/test/java/com/team766/framework3/RuleEngineTest.java @@ -921,4 +921,170 @@ public void testRepeatedlyPersistence() { assertNotNull(cmd2); assertTrue(cmd2.getName().endsWith("action_ends_first_proc")); } + + /** Test hierarchical Rules triggering */ + @Test + public void testRuleHierarchy() { + RuleEngine myRules = + new RuleEngine() { + { + addRule( + Rule.create("root", new ScheduledPredicate(0, 2)) + .withOnTriggeringProcedure( + ONCE_AND_HOLD, + () -> + new FakeProcedure( + "root_proc", 10, Set.of(fm1))) + .whenTriggering( + Rule.create( + "positive_combinator", + new ScheduledPredicate(1, 3)) + .withOnTriggeringProcedure( + ONCE_AND_HOLD, + () -> + new FakeProcedure( + "positive_combinator_proc", + 10, + Set.of(fm2)))) + .whenNotTriggering( + Rule.create( + "negative_combinator", + // Note: This predicate is only + // evaluated when the `root` rule is + // not triggering, so this triggers + // on frame 2, even though its + // start/end arguments say it + // triggers on frame 0. + new ScheduledPredicate(0, 1)) + .withOnTriggeringProcedure( + ONCE_AND_HOLD, + () -> + new FakeProcedure( + "negative_combinator_proc", + 10, + Set.of(fm3))))); + } + }; + + myRules.run(); + + Command cmd1 = CommandScheduler.getInstance().requiring(fm1); + assertNotNull(cmd1); + assertTrue(cmd1.getName().endsWith("root_proc")); + Command cmd2 = CommandScheduler.getInstance().requiring(fm2); + assertNull(cmd2); + Command cmd3 = CommandScheduler.getInstance().requiring(fm3); + assertNull(cmd3); + + step(); + myRules.run(); + + cmd1 = CommandScheduler.getInstance().requiring(fm1); + assertNotNull(cmd1); + assertTrue(cmd1.getName().endsWith("root_proc")); + cmd2 = CommandScheduler.getInstance().requiring(fm2); + assertNotNull(cmd2); + assertTrue(cmd2.getName().endsWith("positive_combinator_proc")); + cmd3 = CommandScheduler.getInstance().requiring(fm3); + assertNull(cmd3); + + step(); + myRules.run(); + + cmd1 = CommandScheduler.getInstance().requiring(fm1); + assertNull(cmd1); + cmd2 = CommandScheduler.getInstance().requiring(fm2); + assertNull(cmd2); + cmd3 = CommandScheduler.getInstance().requiring(fm3); + assertNotNull(cmd3); + assertTrue(cmd3.getName().endsWith("negative_combinator_proc")); + + step(); + myRules.run(); + + cmd1 = CommandScheduler.getInstance().requiring(fm1); + assertNull(cmd1); + cmd2 = CommandScheduler.getInstance().requiring(fm2); + assertNull(cmd2); + cmd3 = CommandScheduler.getInstance().requiring(fm3); + assertNull(cmd3); + } + + /** Test that the root Rule takes precedence over child rules triggering */ + @Test + public void testRuleHierarchyPriorities() { + RuleEngine myRules = + new RuleEngine() { + { + addRule( + Rule.create("root", new ScheduledPredicate(0, 2)) + .withOnTriggeringProcedure( + ONCE, + () -> + new FakeProcedure( + "root_newly_proc", 0, Set.of(fm1))) + .withFinishedTriggeringProcedure( + () -> + new FakeProcedure( + "root_finished_proc", + 0, + Set.of(fm1))) + .whenTriggering( + Rule.create( + "positive_combinator", + new ScheduledPredicate(0, 2)) + .withOnTriggeringProcedure( + ONCE_AND_HOLD, + () -> + new FakeProcedure( + "positive_combinator_proc", + 10, + Set.of(fm1)))) + .whenNotTriggering( + Rule.create( + "negative_combinator", + // Note: This predicate is only + // evaluated when the `root` rule is + // not triggering, so this triggers + // on frames 2-3, even though its + // start/end arguments say it + // triggers on frame 0-1. + new ScheduledPredicate(0, 2)) + .withOnTriggeringProcedure( + ONCE_AND_HOLD, + () -> + new FakeProcedure( + "negative_combinator_proc", + 10, + Set.of(fm1))))); + } + }; + + myRules.run(); + + Command cmd1 = CommandScheduler.getInstance().requiring(fm1); + assertNotNull(cmd1); + assertTrue(cmd1.getName().endsWith("root_newly_proc")); + + step(); + myRules.run(); + + cmd1 = CommandScheduler.getInstance().requiring(fm1); + assertNotNull(cmd1); + assertTrue(cmd1.getName().endsWith("positive_combinator_proc")); + + step(); + myRules.run(); + + cmd1 = CommandScheduler.getInstance().requiring(fm1); + assertNotNull(cmd1); + assertTrue(cmd1.getName().endsWith("root_finished_proc")); + + step(); + myRules.run(); + + cmd1 = CommandScheduler.getInstance().requiring(fm1); + assertNotNull(cmd1); + assertTrue(cmd1.getName().endsWith("negative_combinator_proc")); + } } diff --git a/src/test/java/com/team766/framework3/RuleTest.java b/src/test/java/com/team766/framework3/RuleTest.java index 0b35bb6f..795b8395 100644 --- a/src/test/java/com/team766/framework3/RuleTest.java +++ b/src/test/java/com/team766/framework3/RuleTest.java @@ -10,6 +10,7 @@ import com.team766.framework3.Rule.Cancellation; import com.team766.framework3.Rule.TriggerType; import java.util.Collections; +import java.util.List; import java.util.Set; import java.util.function.BooleanSupplier; import org.junit.jupiter.api.BeforeEach; @@ -40,15 +41,21 @@ public TrivialProcedure(String name) { public void run(Context context) {} } + private static T getSingleElement(List list) { + assertEquals(1, list.size()); + return list.get(0); + } + @BeforeEach protected void setUp() {} @Test public void testCreate() { Rule alwaysTrue = - Rule.create("always true", () -> true) - .withOnTriggeringProcedure(ONCE, () -> Procedure.NO_OP) - .build(); + getSingleElement( + Rule.create("always true", () -> true) + .withOnTriggeringProcedure(ONCE, () -> Procedure.NO_OP) + .build()); assertNotNull(alwaysTrue); assertEquals("always true", alwaysTrue.getName()); } @@ -57,9 +64,10 @@ public void testCreate() { public void testEvaluate() { // start with simple test of a NONE->NEWLY->CONTINUING->CONTINUING sequence Rule alwaysTrue = - Rule.create("always true", () -> true) - .withOnTriggeringProcedure(ONCE, () -> Procedure.NO_OP) - .build(); + getSingleElement( + Rule.create("always true", () -> true) + .withOnTriggeringProcedure(ONCE, () -> Procedure.NO_OP) + .build()); assertEquals(Rule.TriggerType.NONE, alwaysTrue.getCurrentTriggerType()); alwaysTrue.evaluate(); assertEquals(TriggerType.NEWLY, alwaysTrue.getCurrentTriggerType()); @@ -70,9 +78,10 @@ public void testEvaluate() { // test a full cycle: NONE->NEWLY->CONTINUING->FINISHED->NONE->NEWLY->... Rule duckDuckGooseGoose = - Rule.create("duck duck goose goose", new DuckDuckGooseGoosePredicate()) - .withOnTriggeringProcedure(ONCE, () -> Procedure.NO_OP) - .build(); + getSingleElement( + Rule.create("duck duck goose goose", new DuckDuckGooseGoosePredicate()) + .withOnTriggeringProcedure(ONCE, () -> Procedure.NO_OP) + .build()); assertEquals(Rule.TriggerType.NONE, duckDuckGooseGoose.getCurrentTriggerType()); duckDuckGooseGoose.evaluate(); assertEquals(TriggerType.NEWLY, duckDuckGooseGoose.getCurrentTriggerType()); @@ -89,22 +98,25 @@ public void testEvaluate() { @Test public void testGetCancellation() { Rule ruleWithOnce = - Rule.create("always true", new DuckDuckGooseGoosePredicate()) - .withOnTriggeringProcedure(ONCE, () -> Procedure.NO_OP) - .build(); + getSingleElement( + Rule.create("always true", new DuckDuckGooseGoosePredicate()) + .withOnTriggeringProcedure(ONCE, () -> Procedure.NO_OP) + .build()); assertEquals(Cancellation.DO_NOT_CANCEL, ruleWithOnce.getCancellationOnFinish()); Rule ruleWithOnceAndHold = - Rule.create("always true", new DuckDuckGooseGoosePredicate()) - .withOnTriggeringProcedure(ONCE_AND_HOLD, () -> Procedure.NO_OP) - .build(); + getSingleElement( + Rule.create("always true", new DuckDuckGooseGoosePredicate()) + .withOnTriggeringProcedure(ONCE_AND_HOLD, () -> Procedure.NO_OP) + .build()); assertEquals( Cancellation.CANCEL_NEWLY_ACTION, ruleWithOnceAndHold.getCancellationOnFinish()); Rule ruleWithRepeatedly = - Rule.create("always true", new DuckDuckGooseGoosePredicate()) - .withOnTriggeringProcedure(REPEATEDLY, () -> Procedure.NO_OP) - .build(); + getSingleElement( + Rule.create("always true", new DuckDuckGooseGoosePredicate()) + .withOnTriggeringProcedure(REPEATEDLY, () -> Procedure.NO_OP) + .build()); assertEquals( Cancellation.CANCEL_NEWLY_ACTION, ruleWithRepeatedly.getCancellationOnFinish()); } @@ -116,10 +128,11 @@ public void testGetMechanismsToReserve() { final Set> finishedMechanisms = Set.of(new FakeMechanism()); Rule duckDuckGooseGoose = - Rule.create("duck duck goose goose", new DuckDuckGooseGoosePredicate()) - .withOnTriggeringProcedure(ONCE, newlyMechanisms, () -> {}) - .withFinishedTriggeringProcedure(finishedMechanisms, () -> {}) - .build(); + getSingleElement( + Rule.create("duck duck goose goose", new DuckDuckGooseGoosePredicate()) + .withOnTriggeringProcedure(ONCE, newlyMechanisms, () -> {}) + .withFinishedTriggeringProcedure(finishedMechanisms, () -> {}) + .build()); // NONE assertEquals(Collections.emptySet(), duckDuckGooseGoose.getMechanismsToReserve()); @@ -148,10 +161,13 @@ public void testGetMechanismsToReserve() { @Test public void testGetProcedureToRun() { Rule duckDuckGooseGoose = - Rule.create("duck duck goose goose", new DuckDuckGooseGoosePredicate()) - .withOnTriggeringProcedure(ONCE, () -> new TrivialProcedure("newly")) - .withFinishedTriggeringProcedure(() -> new TrivialProcedure("finished")) - .build(); + getSingleElement( + Rule.create("duck duck goose goose", new DuckDuckGooseGoosePredicate()) + .withOnTriggeringProcedure( + ONCE, () -> new TrivialProcedure("newly")) + .withFinishedTriggeringProcedure( + () -> new TrivialProcedure("finished")) + .build()); // NONE assertNull(duckDuckGooseGoose.getProcedureToRun());