Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unity 5 Compatibility fix #13

Merged
merged 3 commits into from Feb 25, 2015
Merged

Unity 5 Compatibility fix #13

merged 3 commits into from Feb 25, 2015

Conversation

alaa-eddine
Copy link

managed to get it working with Unity 5 :)
Here is the fixed version.

I used #Unity_5 conditional compilation switch so the old code is kept as is for older version.

@mythgarr
Copy link
Owner

I'm no longer actively maintaining the Spriter2Unity project and haven't done much to keep current on Unity changes in general, but I'm sure many users will appreciate your changes.
I can't verify whether or not your changes work, but it appears that the line
{{BindingFlags bindingFlags}}
shows up twice in AnimationCurveUtils.cs. It looks like this method is public as of Unity 5, so that line is only needed for older versions.

@alaa-eddine
Copy link
Author

Sorry for this mistake :)
I fixed it in the last commit

mythgarr added a commit that referenced this pull request Feb 25, 2015
@mythgarr mythgarr merged commit 23b956f into mythgarr:master Feb 25, 2015
@AlexanderYoshi
Copy link

I wanted to verity that his changes work correctly, but that line 192 in AnimationCurveUtils.cs is not needed.

        //Use reflection to get the internal method
        BindingFlags bindingFlags = BindingFlags.Static | BindingFlags.NonPublic;

@alaa-eddine
Copy link
Author

The line is needed for Unity < 5.

I'll post another PR

@Dharengo
Copy link

Are you sure it is? The line doesn't actually do anything. The variable actually is never used anywhere.

@AlexanderYoshi
Copy link

He's correct - the 4.3 version uses it as a parameter.

@alaa-eddine
Copy link
Author

just made a PR to fix it :)

@Dharengo
Copy link

Then why isn't it in side a directive?

@Dharengo
Copy link

Ah I've seen your fix. I've tried to merge your fork into mine but apparently I ran into some kind of conflict. Idk how to resolve conflicts so I just changed my fork manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants