Search Unity

  1. Megacity Metro Demo now available. Download now.
    Dismiss Notice
  2. Unity support for visionOS is now available. Learn more in our blog post.
    Dismiss Notice

Unity: In 2017.3.0, why revert WWWAudioExtensions change made in 5.6.0?

Discussion in 'Immediate Mode GUI (IMGUI)' started by SonicBloomEric, Mar 27, 2018.

  1. SonicBloomEric

    SonicBloomEric

    Joined:
    Sep 11, 2014
    Posts:
    1,085
    Back in Unity 5.6.0, the WWW.GetAudioClip() function was reimplemented as an Extension method: it was moved to a WWWAudioExtensions static class.

    This worked fine, but caused a headache for folks who compile C# to DLL - the relevant IL changed and, due to the nature of Extension Methods, tracking down the changed method was a real pain.

    The change made sense, however: Unity is moving to a modular design and removing those functions from the WWW class would remove the dependency on Audio. Great.

    However, this appears to have been reversed in Unity 2017.3.0.

    Why the change? Are we no longer going to see the audio system fully broken out as a separate module from the WWW class? Is this a temporary change? (I hope not...)
     
  2. Aurimas-Cernius

    Aurimas-Cernius

    Unity Technologies

    Joined:
    Jul 31, 2013
    Posts:
    3,722
    This is an unfortunate result from out side. Apologies for that.

    WWW was hit by modularisation effort in bad ways. We began splitting it to reduce required dependencies and then a difference future for it was determined.
    UnityWebRequest is the indented replacement for WWW. It is split up to few modules so that only required dependencies are pulled in.
    WWW was reimplemented since 2017.1 to be a thin wrapper on top of UnityWebRequest and exists as a single module with all possible dependencies. We are going to deprecate it, but no exact plan for that exist yet (because it's widely used), so currently we simply keep it as is and don't intend to update it's API in any way, all new features go to UnityWebRequest only and we recommend to switch to it.

    So, if you are writing a custom .dll, I recommend using UnityWebRequest and if you want to support multiple Unity versions ranging from 5.6 to present, I recommend you don't use the static helpers like UnityWebRequest.GetAudioClip() (which got replaced by UnityWebRequestMultimedia.GetAudioClip()), instead manually create UnityWebRequest and relevant download handler, that part of API remains stable before and after module split up.
     
    SonicBloomEric likes this.
  3. SonicBloomEric

    SonicBloomEric

    Joined:
    Sep 11, 2014
    Posts:
    1,085
    As a user, I would like to request that you do not revert changes like this in the future. If there was a good reason to revert the WWWAudioExtensions version rather than simply deprecating it when WWW was also deprecated, I would very much appreciate it.

    Flipping back and forth seemingly without reason causes code maintenance issues for users. Asset Store publishers who ship DLLs rather than source code are one such set of users: each time a change like this is made, an entirely new version of the asset must be built and manually uploaded with [some seriously awful] tools. I add this last comment so that you understand that it isn't just "more builds" that need to be managed, but an entire, unsupported [by Unity, at least] pipeline.

    At the very least, API changes like this should be in the release notes - I did not find any mention of this in any of the pertinent release notes.

    For others who may read this, please understand that @Aurimas-Cernius is referring to using a UnityWebRequest object instance with an attached DownloadHandlerAudioClip object instance to control data interpretation. Both of those appear to be forward compatible from Unity 5.4.0 to latest (2018.1 beta at time of writing).
     
    Last edited: Apr 3, 2018
  4. AdrianoVerona_Unity

    AdrianoVerona_Unity

    Unity Technologies

    Joined:
    Apr 11, 2013
    Posts:
    317
    Hi

    First I am really sorry for the trouble we caused.

    I can't answer the why because I don't know.

    What I can say is that we are committed to a stable API and that we are always trying to improve our processes to avoid introducing API breaking changes (some times it is unavoidable though). I am not sure if this case was unavoidable (I think it was) or not.

    Keep in mind that sometimes it may happen that we overlook some change. In this case, please, keep doing what you have been doing so far and tell us :)

    Adriano
     
    SonicBloomEric likes this.
  5. SonicBloomEric

    SonicBloomEric

    Joined:
    Sep 11, 2014
    Posts:
    1,085
    Hi @AdrianoVerona_Unity! Thanks for the response! With respect to this:

    To be clear, I wish to applaud the efforts of the team to build a stable API while maintaining a professional development process. I recognize the difficulty faced when needing to change how an API works, where an API lives, etc.

    The problem I have here is that API changes occurred and they were not communicated. When WWW.GetAudioClip was removed in 5.6.0, there was no mention in the release notes. What's more, the API Updater was not able to catch this case when compiled into a DLL. This change was then reversed in 2017.3.0, also with no mention in the release notes. Similarly, the API Updater was not able to catch this case [when compiled into a DLL]. Both of these changes resulted in lost hours, scrambling to figure out what broke and when (it is exceedingly difficult to sift through the many versions of Unity to find the location of the breakage).

    Could you explain why you think that this case was unavoidable? How did returning the Extension Method version of an API to its normal class version change anything for consumers? Could not the Extension Method version simply be deprecated alongside the WWW class when that happens?
     
    Last edited: Apr 3, 2018
  6. AdrianoVerona_Unity

    AdrianoVerona_Unity

    Unity Technologies

    Joined:
    Apr 11, 2013
    Posts:
    317
    HI

    Indeed, that was bad.

    APIUpdater should be able to update from WWWAudioExtensions.GetAudioClip() -> WWW.GetAudioClip() (in both scripts / assemblies). I'll take a look.

    I think I did not expressed myself correctly. When I said



    If the goal was to break the dependency from WWW to Audio simply marking the methods in WWW as obsolete would not work (but I am not sure that that was the reason for the change).

    I (personally) apologize ; I know how frustrating / time consuming it is to chase this type of issues.

    As I said in the other message, we do strive to get our API as stable as possible and make our users life easier, not harder.

    Adriano
     
    SonicBloomEric likes this.
  7. SonicBloomEric

    SonicBloomEric

    Joined:
    Sep 11, 2014
    Posts:
    1,085
    Thanks for the response @AdrianoVerona_Unity. It's very much appreciated.

    Some followups:

    Cool. From what I can tell, this was not caught in versions up-to-and-including Unity 2018.1 betas. Certainly not the 2017.3 line.

    I agree. My complaints about the initial change (moving the Audio APIs out of WWW in 5.6.0) were the following:
    1. API change was not communicated.
    2. API Updater failed to catch this case.
    I did not have a problem with the reason for the move (to remove the audio dependency) - I wholeheartedly support the initiative.

    My complaints about this new change (moving the Audio APIs back into WWW in 2017.3.0) are the following:
    1. API change was not communicated.
    2. API Updater failed to catch this case.
    3. No one seems able to explain why the change happened.
    To this date, the only explanation I've heard (having talked to other Unity devs via other channels as well) is a relatively muddy explanation from @Aurimas-Cernius. If I understand him correctly, the flow of decisions went like this:
    1. [5.6.0] Break up WWW to reduce dependencies.
    2. [2017.1.0] Reimplement WWW as a wrapper over UnityWebRequest.
    3. [2017.3.0] Consolidate WWW into a single module, reversing the changes implemented in 5.6.0.
    It is #3 that makes absolutely no sense to me. Reconsolidate WWW so that it's easier to deprecate? How hard is it to deprecate WWW and a few extension modules all at the same time? What is wrong with leaving the WWW modules as-is and simply start the deprecation process by annotating the various modules?

    Here is why I'm harping on this: if no one at Unity can reasonably describe why this change occurred, then no one at Unity will learn from this mistake going forward. The "mistake" in question can either be the lack of communication, the needless API breakage, or both. Regardless, it is disheartening as a user to see that such a substantial change appears to lack a decent rationalization.

    For a change of this magnitude (consolidation of modules in a codebase), I would expect a decent commit log describing the situation. Does such a log not exist?
     
  8. AdrianoVerona_Unity

    AdrianoVerona_Unity

    Unity Technologies

    Joined:
    Apr 11, 2013
    Posts:
    317
    Lack of communication for sure was a mistake.

    I can assure you we learn with our mistakes; sometimes they repeat again because we are humans :) but we try to improve every time it happens.

    Adriano
     
  9. SonicBloomEric

    SonicBloomEric

    Joined:
    Sep 11, 2014
    Posts:
    1,085
    Yes, yes. But can you articulate why the revert happened in 2017.3? Specifically, why was the decision made to reconsolidate WWW into a single, monolithic module?

    I should point out that "an engineer simply thought it was better organized" is a completely fine response. In such a case, we would clearly see the "mistake" and understand that perhaps Unity as a company would be better aware of the issues this can cause going forward. But if it was some other reason, then the lesson would be different...
     
  10. Aurimas-Cernius

    Aurimas-Cernius

    Unity Technologies

    Joined:
    Jul 31, 2013
    Posts:
    3,722
    It was never truly split. Even though the API itself was split, the reimplemented WWW was in a single module. The rever happened to clean up our code internally.
    Unfortunately, we made the same mistake twice: modifications to API were fine with regard to scripts, but not to precompiled assembles. I feel bad of this mess we've made...
     
    SonicBloomEric likes this.
  11. SonicBloomEric

    SonicBloomEric

    Joined:
    Sep 11, 2014
    Posts:
    1,085
    Can you explain what you mean by "module" here? Am I correct in understanding that this would be a "managed DLL"?

    That clarification is important because you could have avoided the API breakage by simply adjusting the WWW extensions into their own separate DLLs (extension methods do not have to be in the same DLL as the class they extend). Unless I'm missing something...

    Here is the answer to the why. This was simply a case of "code cleanup" without fully understanding repercussions.

    Hopefully the team will have a better understanding of API change implications. Specifically that Extension Method-related changes are not seamless for users and the same care must be taken when changing them as with any other API adjustment in the future.

    This seems like a good learning opportunity for the team, rather than just the handful of engineers who happened to trip over the edge case.
     
  12. Aurimas-Cernius

    Aurimas-Cernius

    Unity Technologies

    Joined:
    Jul 31, 2013
    Posts:
    3,722
    A "module" here means a collection of things that can be removed entirely if not used and a given platform setup allows that. That includes both managed and native code as well as some extra resource files etc. Whether everything can actually be removed depends on platform, i.e. all native code might be linked to single dynamic library, so only managed parts would be removed then. But the idea is that module should designed so it is fully removable.

    As to this particular case, the goal was to have Audio as a module. Since WWW was still in core and quite tightly integrated into it, the audio related part of it was split up. After WWW was reimplemented as a wrapper on top of UnityWebRequest, it became a module itself. It was intentionally made a single module with a bunch of dependencies to be an additional discouragement to use it (UnityWebRequest is split up to multiple modules).
     
  13. SonicBloomEric

    SonicBloomEric

    Joined:
    Sep 11, 2014
    Posts:
    1,085
    It sounds like you guys have an issue with communication, then. Where do you message the following:
    1. What Modules are included in builds?
    2. What goes into each individual Module?
    My understanding is that much/most of this is automated. But for someone to be discouraged they have to have a clear way of understanding both #1 and #2 above. Users need to know what modules are being included and why.

    Another question: how are modules scoped by the system? Is there a module definition file somewhere that allows you to specify which native libs, managed resources, and assemblies are included when a user references something within that set? Or is the module system automated based on the internal dependencies of module-element?