bendmorris / spinehaxe

Spine runtime for Haxe 3. Based on current AS3/libgdx runtimes.
Other
64 stars 28 forks source link

* Bugfix: restored operability for flash target after openfl 4.4.1 release; #25

Closed mepsoid closed 7 years ago

mepsoid commented 7 years ago

Please apply some minor fixes for issue #24

bendmorris commented 7 years ago

I was actually in the middle of a runtime update, and I believe the issue is now resolved. Could you try again with the latest master? I've tested with OpenFL 3, 4 and NME on both flash and native.

mepsoid commented 7 years ago

It doesn't work again: C:/HaxeToolkit/haxe/lib/spinehaxe/git/spinehaxe/animation/TrackEntry.hx:83: characters 2-43 : Type parameter must be a class or enum instance (found Dynamic) and 11 same errors more. It's because generic method ArrayUtils.clearArray awaits concrete class, not type definition like Function = Dynamic that is abstract one.

I have one useful code snippet from stackoverflow for fast array clearing, it can be rewritten for changing array size:

public static inline function clear<T>(aArray:Array<T>) {
    #if (cpp || php)
    aArray.splice(0, aArray.length);
    #else
    untyped aArray.length = 0;
    #end
}
mepsoid commented 7 years ago

BTW isn't it better to create method clear() in Listeners class instead of clumsy access control:

@:allow(spinehaxe.animation.AnimationState)
var _listeners:Array<Function> = new Array();

with helper method calls in TrackEntry and AnimationState classes:

ArrayUtils.clearArray(onStart._listeners);
ArrayUtils.clearArray(onInterrupt._listeners);
ArrayUtils.clearArray(onEnd._listeners);
ArrayUtils.clearArray(onDispose._listeners);
ArrayUtils.clearArray(onComplete._listeners);
ArrayUtils.clearArray(onEvent._listeners);
bendmorris commented 7 years ago

Thanks, I'm surprised I didn't run into this in my testing. Can you provide details about how you're running when you see this error? Platform, library versions and Haxe version would be helpful.

I have one useful code snippet from stackoverflow for fast array clearing

I don't want to use splice here - splice returns an array, and thus allocates one, so it can increase GC activity. (see: https://github.com/HaxeFoundation/haxe/issues/4238)

BTW isn't it better to create method clear() in Listeners class instead of clumsy access control:

This is a port of an AS3 library, which uses the pattern of internal variables in many places. To make updating easier I'd rather stay close to the source than add my own helper methods. I think that's valid feedback for the original repo though (https://github.com/EsotericSoftware/spine-runtimes)

bendmorris commented 7 years ago

It looks like this issue was fixed in Haxe 3.3.0-rc1: https://github.com/HaxeFoundation/haxe/issues/4273 For now I'll just remove the @:generic, the difference should be minor if any.

mepsoid commented 7 years ago

I have updated my haxe compiler from 3.2.0 to 3.4.0 and now I have 2 issues more. The minor one: C:/HaxeToolkit/haxe/lib/spinehaxe/git/spinehaxe/IkConstraint.hx:223: characters 8-10 : Local variable a1 used without being initialized and C:/HaxeToolkit/haxe/lib/spinehaxe/git/spinehaxe/IkConstraint.hx:229: characters 9-11 : Local variable a2 used without being initialized It's because locals a1 and a2 are not initialized in line 146 of class IkConstraint and further code have statements when a1 and a2 are not setup before usage.

The major issue is: [Fault] exception, information=Attachment type not implemented: skinnedmesh in readAttachment method of the class SkeletonJson. I've seen that this type is absent in AttachmentType enumerator. Is it obsolete or something? Previous version of spine library works well with this data.

mepsoid commented 7 years ago

And I'd like to suggest splitting of Listener to two classes -- one for onEvent handling with two arguments and another for other types of events handling with one argument. Methods like function add(listener:Function=Dynamic) doesn't say anything about listener signature that may simply cause runtime errors in advance. I am very pleased of haxe ability to describe signature of functions argument like TrackEntry->Event->Void and so on.

bendmorris commented 7 years ago

Variable initialization issue is fixed. For skinnedmesh, yes, it is no longer supported as of Spine 3.2: https://github.com/EsotericSoftware/spine-runtimes/issues/661. You can re-export with a more recent version of Spine to update your JSON.

bendmorris commented 7 years ago

Took your suggestion on listener classes - there is now a parameterized BaseListeners with Listeners and EventListeners child classes, so the compiler will check for correct types on add/invoke.