Replacing Obsolete WWW with UnityWebRequest - Githubissues

shamsdev / davinci

An esay-to-use image downloading and caching library for Unity

MIT License

290 stars 52 forks source link

Replacing Obsolete WWW with UnityWebRequest #20

Closed khmaies5 closed 3 years ago

khmaies5 commented 3 years ago

Unity is going to replace WWW with UnityWebRequest, they are advising users to use UnityWebRequest in the docs Obsolete: WWW has been replaced with UnityWebRequest.

khmaies5 commented 3 years ago

Www is supported until unity 2017 starting from 2018 and up you need to use UnityWebRequest. -this merge will break this asset in unity 2017 and older. You can say in docs that this asset requires Unity 2018 and up or i can add a condition in the code to check the version of Unity and use the proper API depending on that

Techie-Pi commented 3 years ago

We could create an option to use WWW instead on UnityWebRequest, something like .legacyWww(true). But defaulting to UnityWebRequest

khmaies5 commented 3 years ago

i can update the code like this


#if UNITY_2018_3_OR_NEWER
    private UnityWebRequest www;
#else
    private WWW www;
#endif

Techie-Pi commented 3 years ago

That's a better option, great

Techie-Pi commented 3 years ago

Great! There's nothing left, right?

khmaies5 commented 3 years ago

yep, that's all

khmaies5 commented 3 years ago

@shamsdev I only changed the lines that needed to be changed, the other changes for some reason are automatically detected by git despite I didn't touch them, maybe its file encoding thing since I use windows vs code.

Techie-Pi commented 3 years ago

Same for #13

khmaies5 commented 3 years ago

@Techie-Pi i must pull the new changes in the main repo and push again ?

Techie-Pi commented 3 years ago

The simpler option is to fetch upstream when #23 is merged and use something like eclint to fix your local file, that could end with one of the following possibilities:

the unnecesary changes are canceled by upstream and only the relevant ones are left
in this PR everything stays similar to right now with the unnecesary changes, in that case, maybe an exception could be made, and the pull request is merged

Same for #13 if I'm not wrong

Techie-Pi commented 3 years ago

An alternative could be to close this PR and add the changes again when #23 is merged, without the unnecesary changes already commited

Techie-Pi commented 3 years ago

~~@khmaies5 I'm going to create soon a PR on your fork fixing the problems, stay tuned~~

I think we're going to need to close this PR and pull the changes on a new branch on @khmaies5 fork. Same for #13

khmaies5 commented 3 years ago

I will close this pull request and wait for your changes and then re submit my changes

Techie-Pi commented 3 years ago

Great, I'll ask you to remove the Update-to-UnityWebRequest branch on khmaies5/davinci and create it again. I'll create a PR with the code changes once the new branch is created on your side

Techie-Pi commented 3 years ago

I saw you deleted the branch, now you could fetch upstream and I'll take care of everything else. Thank you so much and sorry for all the inconveniences 😅

khmaies5 commented 3 years ago

@Techie-Pi Not problem dude there is not inconvenience, this is a great asset and its great that is not abandoned. I created a new pull request #25

Techie-Pi commented 3 years ago

Thank you so much!! Really, thank you 😉