Skip to content
This repository was archived by the owner on Dec 5, 2024. It is now read-only.

Fixing #351 #395

Merged
merged 4 commits into from
Nov 3, 2017
Merged

Fixing #351 #395

merged 4 commits into from
Nov 3, 2017

Conversation

GalHorowitz
Copy link
Contributor

Applying shiena's code fixes #351.

Description of the Change

the problem is that ImageConversion got moved out to a separate DLL, which means using Texture2D as a DLL location base no longer works. Worse, it got moved to its own assembly - literally a DLL which only contains this class, which means there is no type we can use to get at its location. So, sigh, we'll need to add yet another codepath to the StreamExtensions code to reference the UnityEngine.ImageConversionModule.dll dll and get the ImageConversion type.

Apparently, you don't need to, and that's all you have to do.

Benefits

Solving #351

Applying shiena's code fixed this issue.
@StanleyGoldman
Copy link
Contributor

Hey @GalHorowitz thanks for participating and trying to fix this issue.
We can definitely use the help!

Did you test this in other versions of unity yet?
I have to go download the new version of Unity now...

@GalHorowitz
Copy link
Contributor Author

This was tested in the latest Unity beta. I will be able to test Unity 5.x later on today.

@GalHorowitz
Copy link
Contributor Author

I just realized Unity 2017.2 is no longer in Beta, it is the official release. 2017.3 is the Beta. Should I still verify Unity 5.x?

@StanleyGoldman
Copy link
Contributor

Hey @GalHorowitz,
Currently we support Unity 5.4 to 2017.1
I will be testing in a few different versions of Unity, but if you could check at least one for us that would be great. If not that's okay too, I'll let you know what I find when i get a chance..

@GalHorowitz
Copy link
Contributor Author

I tested it on the following versions:

  • 2017.2.0b5
  • 2017.1.0f3
  • 5.6.4f1

I did not encounter any issues in either.

@GalHorowitz
Copy link
Contributor Author

@StanleyGoldman Is there any other version I should test?

@StanleyGoldman
Copy link
Contributor

Hey @GalHorowitz

No it's okay, you've done great so far.
I haven't gotten a chance to do my tests yet, I will today.

@StanleyGoldman
Copy link
Contributor

StanleyGoldman commented Nov 1, 2017

Okay, I'm actually testing now...
Checking the following versions..

  • Unity 5.4.5f1 (64-bit)
  • Unity 5.5.3f1 (64-bit)
  • Unity 5.6.0f3 (64-bit)
  • Unity 2017.1.0f3 (64-bit)
  • Unity 2017.1.0f3 .Net 4.5 Runtime (64-bit)
  • Unity 2017.2.0f3 (64-bit)
  • Unity 2017.2.0f3 .Net 4.5 Runtime (64-bit)

@StanleyGoldman
Copy link
Contributor

Yea, this checks out...

Unity is eventually going to remove the full UnityEngine DLL so this
hopefully ensures we never have to worry about this codepath again
@shana
Copy link
Member

shana commented Nov 3, 2017

I've added some extra logic to ensure that when Unity eventually gets rid of the full UnityEngine.dll, we still work correctly and don't have to worry about this again.

@shana shana merged commit 6321788 into github-for-unity:master Nov 3, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NullReferenceException: Object reference not set to an instance of an object
3 participants