loodakrawa / SpriterDotNet

A pure C# Spriter implementation
zlib License
220 stars 75 forks source link

SpriterObjectPool bug #56

Closed AlexVK79 closed 8 years ago

AlexVK79 commented 8 years ago

There must be some bug with SpriterObjectPool. With PoolingEnabled==true I get the null pointer exception:

SpriterSpatial SpriterDotNet.SpriterProcessor:Interpolate (SpriterSpatial, SpriterSpatial, Single, Int32)+0x12 at C:\ALL_PROJECTS\PvPGame\PVP_GAME\Assets\SpriterDotNet\SpriterDotNet\SpriterProcessor.cs:446   C#

Void SpriterDotNet.SpriterProcessor:UpdateFrameData (FrameData, SpriterAnimation, SpriterAnimation, Single, Single)+0xd6 at C:\ALL_PROJECTS\PvPGame\PVP_GAME\Assets\SpriterDotNet\SpriterDotNet\SpriterProcessor.cs:49 C# FrameData SpriterDotNet.AnimationDataProvider.DefaultAnimationDataProvider:GetFrameData (Single, Single, Single, SpriterAnimation, SpriterAnimation)+0x32 at C:\ALL_PROJECTS\PvPGame\PVP_GAME\Assets\SpriterDotNet\SpriterDotNet\AnimationDataProvider\DefaultAnimationDataProvider.cs:23 C# Void SpriterDotNet.SpriterAnimator2:Animate (Single)+0x1f at C:\ALL_PROJECTS\PvPGame\PVP_GAME\Assets\SpriterDotNet\SpriterDotNet\SpriterAnimator.cs:237 C# Void SpriterDotNetUnity.UnitySpriterAnimator:Animate (Single)+0x17 at C:\ALL_PROJECTS\PvPGame\PVP_GAME\Assets\SpriterDotNet\UnitySpriterAnimator.cs:44 C# Void SpriterDotNet.SpriterAnimator2:Step (Single)+0x1a3 at C:\ALL_PROJECTS\PvPGame\PVP_GAME\Assets\SpriterDotNet\SpriterDotNet\SpriterAnimator.cs:229 C# Void SpriterDotNetUnity.SpriterDotNetBehaviour:Update ()+0x39 at C:\ALL_PROJECTS\PvPGame\PVP_GAME\Assets\SpriterDotNet\SpriterDotNetBehaviour.cs:72 C#

With PoolingEnabled==false everything works just fine. The bug occurs in specific combination of several different Spriter animations on screen. Each animation plays just fine alone, but when 2 or more are playing the bug occurs (I guess because SpriterObjectPool is static and is therefore shared across multiple Spriter animations).

loodakrawa commented 8 years ago

That seems really strange - could you please debug this and see what exactly is null? Because at that line (446) - everything that could be null is already accessed earlier in the method. With anything null it should blow up at line 443

AlexVK79 commented 8 years ago

Both - SpriterSpatial a, SpriterSpatial b are null. Debugging it further... it seems that both

SpriterSpatial[] boneInfosA = GetBoneInfos(firstKeyA, first, adjustedTimeFirst); SpriterSpatial[] boneInfosB = GetBoneInfos(secondKeyA, second, adjustedTimeSecond);

in

public static void SpriterProcessor::UpdateFrameData(FrameData frameData, SpriterAnimation first, SpriterAnimation second, float targetTime, float factor)

have null elements at the end (array size is 12 in our case but only first 10 values are not null).

AlexVK79 commented 8 years ago

OK, the fix is to replace

boneInfos = SpriterObjectPool.GetArray<SpriterSpatial>(boneInfosA.Length);
for (int i = 0; i < boneInfosA.Length; ++i)

with smth. like:

int boneInfosA_Length = firstKeyA.BoneRefs.Length;
boneInfos = SpriterObjectPool.GetArray<SpriterSpatial>(boneInfosA_Length);
for (int i = 0; i < boneInfosA_Length; ++i)

in public static void UpdateFrameData(FrameData frameData, SpriterAnimation first, SpriterAnimation second, float targetTime, float factor)

The problem is that SpriterObjectPool may return array whose Length is bigger than requested and this will cause exceptions. I am not sure if there are more similar bugs in the code (need to be checked), but this fix seems to solve all exceptions for us.

loodakrawa commented 8 years ago

Good catch. I'll fix it ASAP - the current pool implementation was a temporary solution anyway so I'll do it properly this time taking this into account and always returning arrays of the correct size. Also I'll avoid having a static pool if possible.

AlexVK79 commented 8 years ago

Static pool would probably be a good memory saver (imagine 100+ spriter units on screen) as long as the implementation is bug free.

loodakrawa commented 8 years ago

Fixed in v1.5.0