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

Update binding flags, and pick the correct httprequest class #62

Closed
wants to merge 2 commits into from

Conversation

Tatsujinichi
Copy link

@Tatsujinichi Tatsujinichi commented Aug 9, 2017

For Issue
BindingFlags and Invoke Member Unity 2017.1 #61

not tested on earlier versions of unity,
works on 2017.1.0f3

@Tatsujinichi Tatsujinichi changed the title Update binding flags Update binding flags, and pick the correct httprequest class Aug 17, 2017
@Tatsujinichi
Copy link
Author

@nullorvoid can you review this

@nullorvoid
Copy link
Contributor

Logic looks fine, no problem with this, but it will need to be tested on 5.5.3 before I will merge it. Would you mind doing this? I can do this, but at the earliest will be Tuesday next week

@@ -131,8 +132,8 @@ public void SetupModules(List<string> moduleNames, Action<Exception> cb)
{
if (moduleName == t.Name)
{
BindingFlags staticProperty = BindingFlags.Static | BindingFlags.GetProperty;
BindingFlags publicMethod = BindingFlags.Public | BindingFlags.InvokeMethod;
BindingFlags staticProperty = BindingFlags.InvokeMethod | BindingFlags.GetField | BindingFlags.GetProperty;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Provided these flag changes work on all devices, LGTM 👍

@AlmirKadric
Copy link
Collaborator

@Tatsujinichi why some of the lines showing as changed although they're not
Line endings?

Other than above comments LGMT 👍

@Tatsujinichi
Copy link
Author

I assume my editor was converting spaces to tabs, but I could be wrong.

@AlmirKadric
Copy link
Collaborator

@Tatsujinichi
Both lines in the diff have tabs, its why i'm assuming line endings. Not really a big deal, more a nuisance and it could even be the line endings are wrong in the original code. If you could just check what it is so we know

@Tatsujinichi
Copy link
Author

Tatsujinichi commented Aug 17, 2017

Yes, it was line endings.
Before: CRFL ( 0D 0A )
before
After: LF ( 0A )
after

I believe the LF is the correct ending.

@AlmirKadric
Copy link
Collaborator

👍

@Tatsujinichi
Copy link
Author

Actually I am not sure the unity version of httprequest is working at all. I've switched back to the .net version.

@nullorvoid
Copy link
Contributor

We should probably move it from WWW to UnityWebRequest which is much newer and probably better supported.

@Tatsujinichi
Copy link
Author

closing this as #66 should accomplish the same thing

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.

3 participants