Fix blackouts feature error and add new unit tests

Assets/Scripts/Lights.cs

@@ -3,16 +3,18 @@
 using System.Collections.Generic;
 
 /// <summary>
-/// Facilitates the management and retrieval of Fade components associated with a collection of GameObjects, black screens.
+/// Script for handling the lights in the environment (blackouts).
 /// </summary>
 namespace Lights
 {
     public class InfiniteEnumerator : IEnumerator<int>
     {
-        private int _initialValue;
-        private int _currentValue;
+        private readonly int _initialValue = 0;
+        private int _currentValue = 0;
 
-        public InfiniteEnumerator(int initialValue = 0)
+        public InfiniteEnumerator() { }
+
+        public InfiniteEnumerator(int initialValue)
         {
             _initialValue = initialValue;
             _currentValue = 0;
@@ -29,43 +31,67 @@ public void Reset()
             _currentValue = 0;
         }
 
-        public int Current => _currentValue;
+        public int Current
+        {
+            get { return _currentValue; }
+        }
 
-        object IEnumerator.Current => Current;
+        object IEnumerator.Current
+        {
+            get { return Current; }
+        }
 
-        public void Dispose() { }
+        void IDisposable.Dispose() { }
     }
-
     public class LightsSwitch
     {
-        private int _episodeLength;
+        private readonly int _episodeLength = 0;
         private bool _lightStatus = true;
-        private List<int> _blackouts;
-        private IEnumerator<int> _blackoutsEnum;
+        private readonly List<int> _blackouts = new List<int>();
+        private readonly IEnumerator<int> _blackoutsEnum;
         private int _nextFrameSwitch = -1;
 
-        public LightsSwitch(int episodeLength = 0, List<int> blackouts = null)
+        public LightsSwitch()
+        {
+            _blackoutsEnum = _blackouts.GetEnumerator();
+        }
+
+        public LightsSwitch(int episodeLength, List<int> blackouts)
         {
             if (episodeLength < 0)
-                throw new ArgumentException("Episode length cannot be negative.");
+            {
+                throw new ArgumentException("Episode length (timeLimit) cannot be negative.", nameof(episodeLength));
+            }
 
             _episodeLength = episodeLength;
-            _blackouts = blackouts ?? new List<int>();
+            _blackouts = blackouts ?? throw new ArgumentNullException(nameof(blackouts));
 
-            if (_blackouts.Count > 0)
+            if (_blackouts.Count > 1)
             {
-                foreach (var blackout in _blackouts)
+                for (int i = 1; i < _blackouts.Count; i++)
                 {
-                    if (blackout < 0 || blackout >= _episodeLength)
-                        throw new ArgumentException("Blackout interval is invalid.");
+                    if (_blackouts[i] <= _blackouts[i - 1])
+                    {
+                        throw new ArgumentException("Invalid blackout sequence: values must be in strictly increasing order.", nameof(blackouts));
+                    }
                 }
+            }
 
-                _blackouts.Sort();
+            if (_blackouts.Count > 0)
+            {
+                if (_blackouts[_blackouts.Count - 1] > _episodeLength)
+                {
+                    throw new ArgumentException("Blackout time cannot exceed the episode length (timeLimit).", nameof(blackouts));
+                }
 
-                _blackoutsEnum =
-                    _blackouts[0] < 0
-                        ? new InfiniteEnumerator(-_blackouts[0])
-                        : _blackouts.GetEnumerator();
+                if (_blackouts[0] < 0)
+                {
+                    _blackoutsEnum = new InfiniteEnumerator(-_blackouts[0]);
+                }
+                else
+                {
+                    _blackoutsEnum = _blackouts.GetEnumerator();
+                }
             }
             else
             {
@@ -77,22 +103,45 @@ public LightsSwitch(int episodeLength = 0, List<int> blackouts = null)
 
         public void Reset()
         {
-            _lightStatus = true;
-            _blackoutsEnum.Reset();
-            _nextFrameSwitch = _blackoutsEnum.MoveNext() ? _blackoutsEnum.Current : -1;
+            try
+            {
+                _lightStatus = true;
+                _blackoutsEnum.Reset();
+                if (_blackoutsEnum.MoveNext())
+                {
+                    _nextFrameSwitch = _blackoutsEnum.Current;
+                }
+                else
+                {
+                    _nextFrameSwitch = -1;
+                }
+            }
+            catch (Exception ex)
+            {
+                Console.WriteLine($"Error resetting LightsSwitch (blackouts): {ex.Message}");
+                throw;
+            }
         }
 
         public bool LightStatus(int step, int agentDecisionInterval)
         {
-            if (step < 0 || agentDecisionInterval <= 0)
-                throw new ArgumentException("Step and agent decision interval must be positive.");
-
-            if (step == _nextFrameSwitch * agentDecisionInterval)
+            try
+            {
+                if (step == _nextFrameSwitch * agentDecisionInterval)
+                {
+                    _lightStatus = !_lightStatus;
+                    if (_blackoutsEnum.MoveNext())
+                    {
+                        _nextFrameSwitch = _blackoutsEnum.Current;
+                    }
+                }
+                return _lightStatus;
+            }
+            catch (Exception ex)
             {
-                _lightStatus = !_lightStatus;
-                _nextFrameSwitch = _blackoutsEnum.MoveNext() ? _blackoutsEnum.Current : -1;
+                Console.WriteLine($"Error in LightStatus (blackouts): {ex.Message}");
+                return _lightStatus;
             }
-            return _lightStatus;
         }
     }
 }

Assets/Tests/EditMode/LightSwitchTests.cs

@@ -9,15 +9,17 @@
 public class LightsSwitchTests
 {
     [Test]
-    public void IncrementValueOnMoveNext()
+    public void InfiniteEnumerator_IncrementValueOnMoveNext()
     {
-        int initialValue = 3;
-        var enumerator = new InfiniteEnumerator(initialValue);
-
+        var enumerator = new InfiniteEnumerator(3);
         enumerator.MoveNext();
-        int currentValue = enumerator.Current;
+        Assert.AreEqual(3, enumerator.Current);
+    }
 
-        Assert.AreEqual(initialValue, currentValue);
+    [Test]
+    public void LightsSwitch_ThrowExceptionForNegativeEpisodeLength()
+    {
+        Assert.Throws<ArgumentException>(() => new LightsSwitch(-10, new List<int> { 1, 2, 3 }));
     }
 
     [Test]
@@ -34,61 +36,87 @@ public void ResetEnumeratorToZero()
     }
 
     [Test]
-    public void KeepIncrementingOnMultipleMoveNext()
+    public void InfiniteEnumerator_ResetToZero()
     {
-        int initialValue = 2;
-        var enumerator = new InfiniteEnumerator(initialValue);
-
-        enumerator.MoveNext();
-        Assert.AreEqual(2, enumerator.Current);
-
+        var enumerator = new InfiniteEnumerator(5);
         enumerator.MoveNext();
-        Assert.AreEqual(4, enumerator.Current);
+        enumerator.Reset();
+        Assert.AreEqual(0, enumerator.Current);
+    }
 
-        enumerator.MoveNext();
-        Assert.AreEqual(6, enumerator.Current);
+    [Test]
+    public void LightsSwitch_InitializeWithValidParameters()
+    {
+        var lightsSwitch = new LightsSwitch(10, new List<int> { 1, 2, 3 });
+        Assert.DoesNotThrow(() => lightsSwitch.Reset());
     }
 
     [Test]
-    public void ThrowExceptionForNegativeEpisodeLength()
+    public void LightsSwitch_HandleInfiniteBlackouts()
     {
-        Assert.Throws<ArgumentException>(() => new LightsSwitch(-1, new List<int> { 1 }));
+        var lightsSwitch = new LightsSwitch(10, new List<int> { -2 });
+        Assert.IsTrue(lightsSwitch.LightStatus(0, 1));
+        Assert.IsFalse(lightsSwitch.LightStatus(2, 1));
+        Assert.IsTrue(lightsSwitch.LightStatus(4, 1));
     }
 
     [Test]
-    public void ThrowExceptionForInvalidBlackoutInterval()
+    public void LightsSwitch_ResetBehavior()
     {
-        Assert.Throws<ArgumentException>(() => new LightsSwitch(5, new List<int> { -1, 6 }));
+        var lightsSwitch = new LightsSwitch(10, new List<int> { 1, 3 });
+        lightsSwitch.LightStatus(1, 1);
+        lightsSwitch.Reset();
+        Assert.IsTrue(lightsSwitch.LightStatus(0, 1));
     }
 
     [Test]
-    public void ReturnTrueIfNoBlackouts()
+    public void LightsSwitch_HandleEmptyBlackoutList()
     {
         var lightsSwitch = new LightsSwitch(10, new List<int>());
-
-        bool lightStatus = lightsSwitch.LightStatus(0, 1);
-
-        Assert.IsTrue(lightStatus);
+        Assert.IsTrue(lightsSwitch.LightStatus(0, 1));
+        Assert.IsTrue(lightsSwitch.LightStatus(5, 1));
     }
 
     [Test]
-    public void HandleMultipleBlackoutsProperly()
+    public void LightsSwitch_HandleDifferentAgentDecisionIntervals()
     {
-        var blackouts = new List<int> { 1, 2, 3 };
-        var lightsSwitch = new LightsSwitch(10, blackouts);
+        var lightsSwitch = new LightsSwitch(20, new List<int> { 2, 4 });
+        Assert.IsTrue(lightsSwitch.LightStatus(0, 2));
+        Assert.IsFalse(lightsSwitch.LightStatus(4, 2));
+        Assert.IsTrue(lightsSwitch.LightStatus(8, 2));
+    }
 
-        Assert.IsTrue(lightsSwitch.LightStatus(0, 1));
-        Assert.IsFalse(lightsSwitch.LightStatus(1, 1));
-        Assert.IsTrue(lightsSwitch.LightStatus(2, 1));
-        Assert.IsFalse(lightsSwitch.LightStatus(3, 1));
+    [Test]
+    public void LightsSwitch_ThrowExceptionForInvalidBlackoutSequence()
+    {
+        Assert.Throws<ArgumentException>(() => new LightsSwitch(20, new List<int> { 10, 1 }));
     }
 
     [Test]
-    public void ThrowExceptionForInvalidStepOrAgentDecisionInterval()
+    public void LightsSwitch_HandleInfiniteBlackoutsWithNegativeNumber()
     {
-        var lightsSwitch = new LightsSwitch(10, new List<int> { 1 });
+        /* 
+         * The blackout list contains a single negative number, which means that the blackout will go repeat infinitely on the blackoutInterval frame.
+         * The light should be OFF at step 0, ON at step 1, OFF at step 2, and so on.
+         */
+        const int episodeLength = 100;
+        const int blackoutInterval = 20;
+        const int agentDecisionInterval = 1;
+
+        var blackoutList = new List<int> { -blackoutInterval };
+        var lightsSwitch = new LightsSwitch(episodeLength, blackoutList);
+
+        /* Initial light status checks, then follows with the blackout sequences */
+        Assert.IsTrue(lightsSwitch.LightStatus(0, agentDecisionInterval), "Light should be ON at step 0");
+        Assert.IsTrue(lightsSwitch.LightStatus(19, agentDecisionInterval), "Light should be ON before blackout at step 19");
+
+        Assert.IsFalse(lightsSwitch.LightStatus(20, agentDecisionInterval), "Light should be OFF during blackout at step 20");
+        Assert.IsFalse(lightsSwitch.LightStatus(39, agentDecisionInterval), "Light should be OFF during blackout at step 39");
+
+        Assert.IsTrue(lightsSwitch.LightStatus(40, agentDecisionInterval), "Light should be ON after blackout at step 40");
+        Assert.IsTrue(lightsSwitch.LightStatus(59, agentDecisionInterval), "Light should be ON before next blackout at step 59");
 
-        Assert.Throws<ArgumentException>(() => lightsSwitch.LightStatus(-1, 1));
-        Assert.Throws<ArgumentException>(() => lightsSwitch.LightStatus(1, 0));
+        Assert.IsFalse(lightsSwitch.LightStatus(60, agentDecisionInterval), "Light should be OFF during blackout at step 60");
+        Assert.IsFalse(lightsSwitch.LightStatus(79, agentDecisionInterval), "Light should be OFF during blackout at step 79");
     }
-}
+}