FlxAnimationController: fix off-by-one and negative modulo bugs (#3554)

* fix frame indexing and modulo behavior

fix frame indexing and modulo behavior

* clamp negative frameIndex and warn on out-of-range

Author: dtwotwo

flixel/animation/FlxAnimationController.hx

@@ -241,7 +241,7 @@ class FlxAnimationController implements IFlxDestroyable
 		var framesToAdd:Array<Int> = frames;
 		var hasInvalidFrames = false;
 		var i = framesToAdd.length;
-		while (i-- >= 0)
+		while (i-- > 0)
 		{
 			final frame = framesToAdd[i];
 			if (frame >= numFrames)
@@ -819,7 +819,17 @@ class FlxAnimationController implements IFlxDestroyable
 	{
 		if (_sprite.frames != null && numFrames > 0)
 		{
-			Frame = Frame % numFrames;
+			if (Frame < 0)
+			{
+				FlxG.log.warn('frameIndex must be a positive number, got $Frame, using 0, instead');
+				Frame = 0;
+			}
+			else if (Frame >= numFrames)
+			{
+				FlxG.log.warn('frameIndex must be less than $numFrames, got $Frame, wrapping to ${Frame % numFrames}, instead');
+				Frame = Frame % numFrames;
+			}
+
 			_sprite.frame = _sprite.frames.frames[Frame];
 			frameIndex = Frame;
 			fireCallback();

tests/unit/src/flixel/animation/FlxAnimationControllerTest.hx

@@ -278,6 +278,72 @@ class FlxAnimationControllerTest extends FlxTest
 		Assert.isTrue (sprite.animation.exists("anim4"), 'missing "anim4"');
 	}
 
+	@Test // #3554
+	function testAddAllValidFrames()
+	{
+		var bitmapData = new BitmapData(4, 1);
+		sprite.loadGraphic(bitmapData, true, 1, 1);
+		
+		sprite.animation.add("test", [0, 1, 2, 3]);
+		
+		var anim = sprite.animation.getByName("test");
+		Assert.isNotNull(anim);
+		FlxAssert.arraysEqual([0, 1, 2, 3], anim.frames);
+	}
+	
+	@Test // #3554
+	function testAddSomeInvalidFrames()
+	{
+		var bitmapData = new BitmapData(4, 1);
+		sprite.loadGraphic(bitmapData, true, 1, 1);
+		
+		sprite.animation.add("test", [0, 4, 2, 5, 3]);
+		
+		var anim = sprite.animation.getByName("test");
+		Assert.isNotNull(anim);
+		FlxAssert.arraysEqual([0, 2, 3], anim.frames);
+	}
+	
+	@Test // #3554
+	function testAddSingleValidFrame()
+	{
+		var bitmapData = new BitmapData(4, 1);
+		sprite.loadGraphic(bitmapData, true, 1, 1);
+		
+		sprite.animation.add("test", [0]);
+		
+		var anim = sprite.animation.getByName("test");
+		Assert.isNotNull(anim);
+		FlxAssert.arraysEqual([0], anim.frames);
+	}
+	
+	@Test // #3554
+	function testAddAllInvalidFrames()
+	{
+		loadSpriteSheet();
+		
+		sprite.animation.add("test", [5, 6, 7]);
+		
+		var anim = sprite.animation.getByName("test");
+		Assert.isNull(anim);
+	}
+	
+	@Test // #3554
+	function testFrameIndexNegativeClampsToZero()
+	{
+		var bitmapData = new BitmapData(4, 1);
+		sprite.loadGraphic(bitmapData, true, 1, 1);
+		
+		sprite.animation.add("test", [0, 1, 2, 3]);
+		sprite.animation.play("test");
+		
+		sprite.animation.frameIndex = -1;
+		
+		Assert.areEqual(0, sprite.animation.frameIndex);
+		sprite.animation.frameIndex = -4;
+		Assert.areEqual(0, sprite.animation.frameIndex);
+	}
+
 	function loadSpriteSheet():Void
 	{
 		var bitmapData = new BitmapData(2, 1);