feat: update Tactic structure (#18)

* Feature: turn GetFirstEnabledActions into GetAction

* Feature: add params to constructors

* Fix: move ThreadSafeRandom into namespace

* Fix: swap parameters in test

* Fix: apply suggestions from code review

Co-authored-by: Jens Steenmetz <35835627+JensSteenmetz@users.noreply.github.com>

* Fix: prevent index out of range exception

* Feature: add test

* Fix: scope of variable

---------

Co-authored-by: Jens Steenmetz <35835627+JensSteenmetz@users.noreply.github.com>

Aplib.Core/Tactics/AnyOfTactic.cs

@@ -17,7 +17,7 @@ public class AnyOfTactic : Tactic
         /// Initializes a new instance of the <see cref="AnyOfTactic"/> class with the specified sub-tactics.
         /// </summary>
         /// <param name="subTactics">The list of sub-tactics.</param>
-        public AnyOfTactic(List<Tactic> subTactics)
+        public AnyOfTactic(params Tactic[] subTactics)
         {
             SubTactics = new();
 
@@ -32,19 +32,25 @@ public AnyOfTactic(List<Tactic> subTactics)
         /// </summary>
         /// <param name="subTactics">The list of sub-tactics.</param>
         /// <param name="guard">The guard condition.</param>
-        public AnyOfTactic(List<Tactic> subTactics, Func<bool> guard) : this(subTactics) => Guard = guard;
+        public AnyOfTactic(Func<bool> guard, params Tactic[] subTactics) : this(subTactics) => Guard = guard;
 
         /// <inheritdoc/>
-        public override List<PrimitiveTactic> GetFirstEnabledActions()
+        public override Action? GetAction()
         {
-            List<PrimitiveTactic> primitiveTactics = new();
+            List<Action> actions = new();
 
             foreach (Tactic subTactic in SubTactics)
             {
-                primitiveTactics.AddRange(subTactic.GetFirstEnabledActions());
+                Action? action = subTactic.GetAction();
+
+                if (action is not null)
+                    actions.Add(action);
             }
 
-            return primitiveTactics;
+            if (actions.Count == 0)
+                return null;
+
+            return actions[ThreadSafeRandom.Next(actions.Count)];
         }
     }
 }

Aplib.Core/Tactics/FirstOfTactic.cs

@@ -12,7 +12,7 @@ public class FirstOfTactic : AnyOfTactic
         /// Initializes a new instance of the <see cref="FirstOfTactic"/> class with the specified sub-tactics.
         /// </summary>
         /// <param name="subTactics">The list of sub-tactics.</param>
-        public FirstOfTactic(List<Tactic> subTactics) : base(subTactics)
+        public FirstOfTactic(params Tactic[] subTactics) : base(subTactics)
         {
         }
 
@@ -21,22 +21,22 @@ public FirstOfTactic(List<Tactic> subTactics) : base(subTactics)
         /// </summary>
         /// <param name="subTactics">The list of sub-tactics.</param>
         /// <param name="guard">The guard condition.</param>
-        public FirstOfTactic(List<Tactic> subTactics, Func<bool> guard) : base(subTactics, guard)
+        public FirstOfTactic(Func<bool> guard, params Tactic[] subTactics) : base(guard, subTactics)
         {
         }
 
         /// <inheritdoc/>
-        public override List<PrimitiveTactic> GetFirstEnabledActions()
+        public override Action? GetAction()
         {
             foreach (Tactic subTactic in SubTactics)
             {
-                List<PrimitiveTactic> firstOfTactics = subTactic.GetFirstEnabledActions();
+                Action? action = subTactic.GetAction();
 
-                if (firstOfTactics.Count > 0)
-                    return firstOfTactics;
+                if (action is not null)
+                    return action;
             }
 
-            return new();
+            return null;
         }
     }
 }

Aplib.Core/Tactics/PrimitiveTactic.cs

@@ -9,9 +9,9 @@ namespace Aplib.Core.Tactics
     public class PrimitiveTactic : Tactic
     {
         /// <summary>
-        /// Gets or sets the action of the primitive tactic.
+        /// Gets the action of the primitive tactic.
         /// </summary>
-        public readonly Action Action;
+        protected readonly Action Action;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="PrimitiveTactic"/> class with the specified action.
@@ -27,7 +27,7 @@ public class PrimitiveTactic : Tactic
         public PrimitiveTactic(Action action, Func<bool> guard) : base(guard) => Action = action;
 
         /// <inheritdoc/>
-        public override List<PrimitiveTactic> GetFirstEnabledActions() => IsActionable() ? new() { this } : new();
+        public override Action? GetAction() => IsActionable() ? Action : null;
 
         /// <inheritdoc/>
         public override bool IsActionable() => base.IsActionable() && Action.IsActionable();

Aplib.Core/Tactics/Tactic.cs

@@ -1,5 +1,4 @@
 using System;
-using System.Collections.Generic;
 
 namespace Aplib.Core.Tactics
 {
@@ -27,10 +26,10 @@ protected Tactic()
         protected Tactic(Func<bool> guard) => Guard = guard;
 
         /// <summary>
-        /// Gets the first enabled primitive actions.
+        /// Gets the enabled action.
         /// </summary>
-        /// <returns>A list of primitive tactics that are enabled.</returns>
-        public abstract List<PrimitiveTactic> GetFirstEnabledActions();
+        /// <returns>An action that is enabled.</returns>
+        public abstract Action? GetAction();
 
         /// <summary>
         /// Determines whether the tactic is actionable.

Aplib.Core/ThreadSafeRandom.cs

@@ -0,0 +1,34 @@
+using System;
+
+namespace Aplib.Core
+{
+    internal static class ThreadSafeRandom
+    {
+        [ThreadStatic]
+        private static Random? _local;
+        private static readonly Random _global = new();
+
+        private static Random _instance
+        {
+            get
+            {
+                if (_local is null)
+                {
+                    int seed;
+                    lock (_global)
+                    {
+                        seed = _global.Next();
+                    }
+
+                    _local = new Random(seed);
+                }
+
+                return _local;
+            }
+        }
+
+        public static int Next() => _instance.Next();
+
+        public static int Next(int maxValue) => _instance.Next(maxValue);
+    }
+}

Aplib.Tests/Core/Tactics/TacticTests.cs

@@ -1,10 +1,13 @@
 using Aplib.Core.Tactics;
+using FluentAssertions;
 using Action = Aplib.Core.Action;
 
 namespace Aplib.Tests.Core.Tactics;
 public class TacticTests
 {
-    private readonly Action _emptyAction = new(() => { });
+    private readonly Action _emptyAction = new(effect: () => { });
+    private static string _result = "abc";
+    private readonly Action _filledAction = new(effect: () => _result = "def");
 
     private static bool TrueGuard() => true;
 
@@ -16,18 +19,18 @@ public class TacticTests
     /// Then the result should be the first subtactic.
     /// </summary>
     [Fact]
-    public void GetFirstEnabledActions_WhenTacticTypeIsFirstOf_ReturnsEnabledPrimitiveTactics()
+    public void GetAction_WhenTacticTypeIsFirstOf_ReturnsEnabledPrimitiveTactics()
     {
         // Arrange
         PrimitiveTactic tactic1 = new(_emptyAction);
-        PrimitiveTactic tactic2 = new(_emptyAction);
+        PrimitiveTactic tactic2 = new(_filledAction);
         FirstOfTactic parentTactic = new([tactic1, tactic2]);
 
         // Act
-        List<PrimitiveTactic> enabledActions = parentTactic.GetFirstEnabledActions();
+        Action? enabledAction = parentTactic.GetAction();
 
         // Assert
-        Assert.Contains(tactic1, enabledActions);
+        Assert.Equal(_emptyAction, enabledAction);
     }
 
     /// <summary>
@@ -36,18 +39,18 @@ public void GetFirstEnabledActions_WhenTacticTypeIsFirstOf_ReturnsEnabledPrimiti
     /// Then the result should be the first subtactic.
     /// </summary>
     [Fact]
-    public void GetFirstEnabledActions_WhenTacticTypeIsFirstOfAndGuardEnabled_ReturnsEnabledPrimitiveTactics()
+    public void GetAction_WhenTacticTypeIsFirstOfAndGuardEnabled_ReturnsEnabledPrimitiveTactics()
     {
         // Arrange
         PrimitiveTactic tactic1 = new(_emptyAction);
-        PrimitiveTactic tactic2 = new(_emptyAction);
-        FirstOfTactic parentTactic = new([tactic1, tactic2], TrueGuard);
+        PrimitiveTactic tactic2 = new(_filledAction);
+        FirstOfTactic parentTactic = new(TrueGuard, [tactic1, tactic2]);
 
         // Act
-        List<PrimitiveTactic> enabledActions = parentTactic.GetFirstEnabledActions();
+        Action? enabledAction = parentTactic.GetAction();
 
         // Assert
-        Assert.Contains(tactic1, enabledActions);
+        Assert.Equal(_emptyAction, enabledAction);
     }
 
     /// <summary>
@@ -56,19 +59,18 @@ public void GetFirstEnabledActions_WhenTacticTypeIsFirstOfAndGuardEnabled_Return
     /// Then the result should contain all the subtactics.
     /// </summary>
     [Fact]
-    public void GetFirstEnabledActions_WhenTacticTypeIsAnyOf_ReturnsEnabledPrimitiveTactics()
+    public void GetAction_WhenTacticTypeIsAnyOf_ReturnsEnabledPrimitiveTactics()
     {
         // Arrange
         PrimitiveTactic tactic1 = new(_emptyAction);
         PrimitiveTactic tactic2 = new(_emptyAction);
         AnyOfTactic parentTactic = new([tactic1, tactic2]);
 
         // Act
-        List<PrimitiveTactic> enabledActions = parentTactic.GetFirstEnabledActions();
+        Action? enabledAction = parentTactic.GetAction();
 
         // Assert
-        Assert.Contains(tactic1, enabledActions);
-        Assert.Contains(tactic2, enabledActions);
+        Assert.Equal(_emptyAction, enabledAction);
     }
 
     /// <summary>
@@ -77,16 +79,16 @@ public void GetFirstEnabledActions_WhenTacticTypeIsAnyOf_ReturnsEnabledPrimitive
     /// Then the result should contain the primitive tactic.
     /// </summary>
     [Fact]
-    public void GetFirstEnabledActions_WhenTacticTypeIsPrimitiveAndActionIsActionable_ReturnsEnabledPrimitiveTactic()
+    public void GetAction_WhenTacticTypeIsPrimitiveAndActionIsActionable_ReturnsEnabledPrimitiveTactic()
     {
         // Arrange
         PrimitiveTactic tactic = new(_emptyAction, TrueGuard);
 
         // Act
-        List<PrimitiveTactic> enabledActions = tactic.GetFirstEnabledActions();
+        Action? enabledAction = tactic.GetAction();
 
         // Assert
-        Assert.Contains(tactic, enabledActions);
+        Assert.Equal(_emptyAction, enabledAction);
     }
 
     /// <summary>
@@ -95,16 +97,34 @@ public void GetFirstEnabledActions_WhenTacticTypeIsPrimitiveAndActionIsActionabl
     /// Then the result should be an empty list.
     /// </summary>
     [Fact]
-    public void GetFirstEnabledActions_WhenTacticTypeIsPrimitiveAndActionIsNotActionable_ReturnsEmptyList()
+    public void GetAction_WhenTacticTypeIsPrimitiveAndActionIsNotActionable_ReturnsEmptyList()
     {
         // Arrange
         PrimitiveTactic tactic = new(_emptyAction, FalseGuard);
 
         // Act
-        List<PrimitiveTactic> enabledActions = tactic.GetFirstEnabledActions();
+        Action? enabledAction = tactic.GetAction();
+
+        // Assert
+        Assert.Null(enabledAction);
+    }
+
+    /// <summary>
+    /// Given a tactic with a guard that returns true and an action,
+    /// When calling the Execute method,
+    /// Then _result should be "def".
+    /// </summary>
+    [Fact]
+    public void Execute_WhenGuardReturnsTrue_ActionIsExecuted()
+    {
+        // Arrange
+        PrimitiveTactic tactic = new(_filledAction, TrueGuard);
+
+        // Act
+        tactic.GetAction()!.Execute();
 
         // Assert
-        Assert.Empty(enabledActions);
+        Assert.Equal("def", _result);
     }
 
     /// <summary>
@@ -116,7 +136,7 @@ public void GetFirstEnabledActions_WhenTacticTypeIsPrimitiveAndActionIsNotAction
     public void IsActionable_WhenGuardReturnsTrue_ReturnsTrue()
     {
         // Arrange
-        PrimitiveTactic tactic = new(_emptyAction, TrueGuard);
+        PrimitiveTactic tactic = new(_filledAction, TrueGuard);
 
         // Act
         bool isActionable = tactic.IsActionable();