Skip to content

Commit

Permalink
FlxFrame: make sorting consistent across platforms
Browse files Browse the repository at this point in the history
The behavior when Std.parseInt() fails (because of incorrect prefix or postfix input values) was inconsistent. On Neko it crashed, on Flash the sorting order changed.
closes HaxeFlixel#1926
  • Loading branch information
Gama11 authored and Aurel300 committed Apr 17, 2018
1 parent 1fbf550 commit 289720a
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 25 deletions.
2 changes: 1 addition & 1 deletion flixel/animation/FlxAnimationController.hx
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,7 @@ class FlxAnimationController implements IFlxDestroyable
var name:String = AnimFrames[0].name;
var postIndex:Int = name.indexOf(".", Prefix.length);
var postFix:String = name.substring(postIndex == -1 ? name.length : postIndex, name.length);
AnimFrames.sort(FlxFrame.sortByName.bind(_, _, Prefix.length, postFix.length));
FlxFrame.sort(AnimFrames, Prefix.length, postFix.length);

for (animFrame in AnimFrames)
{
Expand Down
22 changes: 16 additions & 6 deletions flixel/graphics/frames/FlxFrame.hx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import flixel.util.FlxColor;
import flixel.util.FlxDestroyUtil;
import flixel.util.FlxStringUtil;
import haxe.ds.Vector;
import haxe.ds.ArraySort;

/**
* Base class for all frame types
Expand All @@ -23,17 +24,26 @@ class FlxFrame implements IFlxDestroyable
private var matrix:FlxMatrix = new FlxMatrix();

/**
* Sorting function for Array<FlxFrame>#sort(),
* e.g. "tiles-001.png", "tiles-003.png", "tiles-002.png".
* Sorts an array of `FlxFrame` objects by their name, e.g.
* `["tiles-001.png", "tiles-003.png", "tiles-002.png"]`
* with `"tiles-".length == prefixLength` and `".png".length == postfixLength`.
*/
public static function sort(frames:Array<FlxFrame>, prefixLength:Int, postfixLength:Int):Void
{
ArraySort.sort(frames, sortByName.bind(_, _, prefixLength, postfixLength));
}

public static function sortByName(frame1:FlxFrame, frame2:FlxFrame, prefixLength:Int, postfixLength:Int):Int
{
var name1:String = frame1.name;
var name2:String = frame2.name;

var num1:Int = Std.parseInt(name1.substring(prefixLength, name1.length - postfixLength));
var num2:Int = Std.parseInt(name2.substring(prefixLength, name2.length - postfixLength));

var num1:Null<Int> = Std.parseInt(name1.substring(prefixLength, name1.length - postfixLength));
var num2:Null<Int> = Std.parseInt(name2.substring(prefixLength, name2.length - postfixLength));
if (num1 == null)
num1 = 0;
if (num2 == null)
num2 = 0;

return num1 - num2;
}

Expand Down
2 changes: 1 addition & 1 deletion flixel/graphics/frames/FlxTileFrames.hx
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ class FlxTileFrames extends FlxFramesCollection
var postIndex:Int = name.indexOf(".", Prefix.length);
var postFix:String = name.substring(postIndex == -1 ? name.length : postIndex, name.length);

framesToAdd.sort(FlxFrame.sortByName.bind(_, _, Prefix.length, postFix.length));
FlxFrame.sort(framesToAdd, Prefix.length, postFix.length);
return FlxTileFrames.fromFrames(framesToAdd);
}

Expand Down
9 changes: 4 additions & 5 deletions tests/unit/src/flixel/graphics/frames/FlxAtlasFramesTest.hx
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,17 @@ class FlxAtlasFramesTest extends FlxTest
@Test
function testTexturePackerJson()
{
var bmd:BitmapData = new BitmapData(1, 1);
var bmd = new BitmapData(1, 1);
var arrJson = '{"frames":[{"filename":"alien.png","frame":{"x":2,"y":2,"w":46,"h":16},"rotated":false,"trimmed":true,"spriteSourceSize":{"x":1,"y":0,"w":46,"h":16},"sourceSize":{"w":48,"h":16},"pivot":{"x":0.5,"y":0.5}},{"filename":"medium.png","frame":{"x":2,"y":20,"w":32,"h":32},"rotated":false,"trimmed":false,"spriteSourceSize":{"x":0,"y":0,"w":32,"h":32},"sourceSize":{"w":32,"h":32},"pivot":{"x":0.5,"y":0.5}},{"filename":"ship.png","frame":{"x":36,"y":38,"w":12,"h":8},"rotated":true,"trimmed":false,"spriteSourceSize":{"x":0,"y":0,"w":12,"h":8},"sourceSize":{"w":12,"h":8},"pivot":{"x":0.5,"y":0.5}},{"filename":"small.png","frame":{"x":36,"y":20,"w":16,"h":16},"rotated":false,"trimmed":false,"spriteSourceSize":{"x":0,"y":0,"w":16,"h":16},"sourceSize":{"w":16,"h":16},"pivot":{"x":0.5,"y":0.5}}]}';
var hashJson = '{"frames":{"alien.png":{"frame":{"x":2,"y":2,"w":46,"h":16},"rotated":false,"trimmed":true,"spriteSourceSize":{"x":1,"y":0,"w":46,"h":16},"sourceSize":{"w":48,"h":16},"pivot":{"x":0.5,"y":0.5}},"medium.png":{"frame":{"x":2,"y":20,"w":32,"h":32},"rotated":false,"trimmed":false,"spriteSourceSize":{"x":0,"y":0,"w":32,"h":32},"sourceSize":{"w":32,"h":32},"pivot":{"x":0.5,"y":0.5}},"ship.png":{"frame":{"x":36,"y":38,"w":12,"h":8},"rotated":true,"trimmed":false,"spriteSourceSize":{"x":0,"y":0,"w":12,"h":8},"sourceSize":{"w":12,"h":8},"pivot":{"x":0.5,"y":0.5}},"small.png":{"frame":{"x":36,"y":20,"w":16,"h":16},"rotated":false,"trimmed":false,"spriteSourceSize":{"x":0,"y":0,"w":16,"h":16},"sourceSize":{"w":16,"h":16},"pivot":{"x":0.5,"y":0.5}}}}';
var atlasArray:FlxAtlasFrames = FlxAtlasFrames.fromTexturePackerJson(bmd, arrJson);
var atlasHash:FlxAtlasFrames = FlxAtlasFrames.fromTexturePackerJson(bmd, hashJson);
var atlasArray = FlxAtlasFrames.fromTexturePackerJson(bmd, arrJson);
var atlasHash = FlxAtlasFrames.fromTexturePackerJson(bmd, hashJson);

Assert.areEqual(atlasArray.numFrames, atlasHash.numFrames);

var hashArr:FlxFrame;
for (frameArr in atlasArray.frames)
{
hashArr = atlasHash.framesHash.get(frameArr.name);
var hashArr = atlasHash.framesHash.get(frameArr.name);
Assert.areEqual(frameArr.name, hashArr.name);
Assert.isTrue(frameArr.sourceSize.equals(hashArr.sourceSize));
}
Expand Down
37 changes: 25 additions & 12 deletions tests/unit/src/flixel/graphics/frames/FlxFrameTest.hx
Original file line number Diff line number Diff line change
@@ -1,25 +1,20 @@
package flixel.graphics.frames;

import massive.munit.Assert;

@:access(flixel.graphics.frames.FlxFrame.new)
class FlxFrameTest extends FlxTest
{
@Test
@:access(flixel.graphics.frames.FlxFrame.new)
function testSortByName()
function testSort()
{
var indices = [3, 5, 8, 1, 6];
var frames:Array<FlxFrame> = [];
var prefix = "tiles-";
var postfix = ".png";

for (index in indices)
{
var frame = new FlxFrame(null);
frame.name = prefix + "00" + index + postfix;
frames.push(frame);
}

frames.sort(FlxFrame.sortByName.bind(_, _, prefix.length, postfix.length));

var frames = [for (i in indices) createFrame(prefix + "00" + i + postfix)];
FlxFrame.sort(frames, prefix.length, postfix.length);

var resultingIndices:Array<Null<Int>> = [];
for (frame in frames)
{
Expand All @@ -29,4 +24,22 @@ class FlxFrameTest extends FlxTest
}
FlxAssert.arraysEqual([1, 3, 5, 6, 8], resultingIndices);
}

@Test // #1926
function testSortNoPrefix()
{
var length = 5;
var frames:Array<FlxFrame> = [for (i in 0...length) createFrame("split/" + i)];
FlxFrame.sort(frames, 0, 0);

for (i in 0...length)
Assert.areEqual("split/" + i, frames[i].name);
}

function createFrame(name:String):FlxFrame
{
var frame = new FlxFrame(null);
frame.name = name;
return frame;
}
}

0 comments on commit 289720a

Please sign in to comment.