Search Unity

Is it necessary to destroy the instance material manaully in Unity 3.5?

Discussion in 'Scripting' started by Irwin, Aug 10, 2012.

  1. Irwin

    Irwin

    Joined:
    Aug 10, 2012
    Posts:
    2
    There are a lot of text objects in my game and I need to change the font color of some of them at some times. So i have to change the renderer.material.color instead of renderer.sharedMaterial.color.
    My question is since i change the material property which means there creates some instance copies of the shared material, do i need to destroy the instance materials manually when i destroy the text objects?
     
  2. Eric5h5

    Eric5h5

    Volunteer Moderator Moderator

    Joined:
    Jul 19, 2006
    Posts:
    32,401
    Yes.

    --Eric
     
  3. Irwin

    Irwin

    Joined:
    Aug 10, 2012
    Posts:
    2
    Can't i just destroy the game object and let unity help me to destroy the instance materials used by the game object? I cannot image a reason why unity does not do this.
     
  4. ImogenPoot

    ImogenPoot

    Joined:
    Jul 2, 2012
    Posts:
    214
    This is one of the greatest misconceptions I see with Unity. They have basically rendered void a lot of natural patterns you are used to as a NET developer, including C# semantics. Resources HAVE to be released automatically, violating this concept basically reduces NET back to C, since managing resources is one of the greatest source of errors and one of the most annoying sort of bugs to look for.

    And the argument that automatic resource management would cause some "stutter" in the game is just a no-brainer. There is nothing that would prevent you from adding the resource explicitly to some sort of Dispose-queue, where you can release them, or pool them as you wish. But the default behaviour MUST be automatic release...

    Okay they did it that way, but then they should set up a huge Documentation section where they explain exactly, what Resources are NOT automatically released and more importantly, how they consider it to be best practice to release/manage them. But then again, citing Bjarne Stroustrup: "Most people don't read manuals, programmers don't read manuals and compilers certainly don't read manuals..."
     
    Last edited: Aug 10, 2012
  5. Eric5h5

    Eric5h5

    Volunteer Moderator Moderator

    Joined:
    Jul 19, 2006
    Posts:
    32,401
    Well, Unity's making things both easier on you and harder. For most people, when they change, say, the color of an object's material, they expect just that object to change color. So Unity helpfully makes a new instance of the material and thus appears to behave the way you expect. The problem is that if you don't know this, then material instances continuously build up unless you manage them yourself. In many cases this isn't actually a problem, because you're not doing it every frame or anything, and you load a new level (in which case this stuff is auto-cleaned-up) before it becomes an issue. But if you deviate from that pattern (e.g., I typically use just one scene for the entire game), then you have to be aware of this and act accordingly. It's not really that big a deal once you know what's going on, but indeed it's not really documented very well (or at all).

    --Eric
     
    Lorrak and deus0 like this.
  6. Dreamora

    Dreamora

    Joined:
    Apr 5, 2008
    Posts:
    26,601
    Actually over time unity will clean them, you can alternatively force clean up such instances with Resources.UnloadUnusedAssets (); which is unitys counterpart to System.GC.Collect ();


    Yes Unity violates basically every single .NET design principles and good practices (the worst is the enforcement of dump polling just cause UnityScript is utter crap that should be canned cause while having gotten Generics it didn't need, still hasn't gotten event support which it needed since Unity 1 for proper .NET practices / OO dev practices), but its important to keep in mind that Unity is also no .NET application. Its a C / C++ application with Mono (not MS .NET) as a scripting layer that wraps around C objects, as such the GC CAN not manage these objects as it never owned them to start with
     
  7. ImogenPoot

    ImogenPoot

    Joined:
    Jul 2, 2012
    Posts:
    214
    Yes with the difference that you now are forcing the game to stutter. You should be able to pass a time limit to this function, say 5 milliseconds or something, so that it returns without being visible to the user.

    Well, say the C-API has a function AddReference() and DecrementReference(). Then you can certainly pass lifetime to NET by just doing like:

    Code (csharp):
    1. class NativeObject : IDisposable
    2. {
    3. IntPtr m_Handle;
    4. public NativeObject(IntPtr handle) { m_Handle = handle; API.AddReference(m_Handle); }
    5. ~NativeObject() { Dispose(); }
    6. public void Dispose() { API.DecrementReference(m_Handle); m_Handle = IntPtr.Zero }
    7. }
     
  8. gfoot

    gfoot

    Joined:
    Jan 5, 2011
    Posts:
    550
    But this is what IDisposable, and particularly finalizers, are for. At the very least, a bit of reference-counting wouldn't hurt.

    The thing that winds me up is that half the time these leaks come from property accessors, where even reading the property causes the cloning to take place. That is evil.
     
  9. Stephan-B

    Stephan-B

    Joined:
    Feb 23, 2011
    Posts:
    2,269
    Would be nice if the cloning happened when you first change some property as opposed to when you first read it. All subsequent reads would then point to the instance.
     
  10. Noisecrime

    Noisecrime

    Joined:
    Apr 7, 2010
    Posts:
    2,054
    Wow, this is news to me.

    If Unity creates something I expect it to clean up after all references to it (ie. the gameObject) are destroyed. If it doesn't, I'm now left wondering exactly what has suddenly become my responsibility to clean up, a horrible situation to be placed in.

    I did notice reading caused clones and thought that was very weird, why would reading requiring a clone instance to be generated? Makes no sense to me.
     
  11. gfoot

    gfoot

    Joined:
    Jan 5, 2011
    Posts:
    550
    If you read the mesh, then you might be about to modify it, and Unity wants to prevent you from modifying a mesh that's potentially shared with other MeshFilters.

    Ultimately if you know the rules then you can arrange to make the right Destroy calls yourself, but obviously it would be better if the API didn't suffer this problem. For example, 'mesh' could return a new IReadOnlyMesh interface that only exposes the readability of the various data arrays within the mesh but does not allow assignments.

    Also, refcounting and automatic destruction of unreferenced objects would be handy. You can pull some tricks with weak references to get the garbage collector to help here as well.

    As dreamora said (thanks!), Resource.UnloadUnusedAssets() does work to clear up these unreferenced objects. It sounds like it's an ineffficient process though.

    For reference, here's some leaky code:

    Code (csharp):
    1.  
    2. public void Update()
    3. {
    4.     _meshFilter.sharedMesh = new Mesh();
    5.     var mesh = _meshFilter.mesh;
    6.     Destroy(_meshFilter.sharedMesh);
    7. }
    8.  
    Without the middle line it doesn't leak. Reading the 'mesh' property causes the current mesh to get cloned, and then both 'mesh' and 'sharedMesh' point to the clone. The original is unreferenced, at least by this MeshFilter.

    It's more noticeable in this obvious, easy to write, but leaky and incorrect, case:

    Code (csharp):
    1.  
    2. public void Update()
    3. {
    4.     if (NeedNewMesh())
    5.     {
    6.         Destroy(_meshFilter.mesh);
    7.         _meshFilter.mesh = GenerateNewMesh();
    8.     }
    9. }
    10.  
    The fix is to just use 'sharedMesh' and make sure you never ever reference 'mesh'. You can also remember the generated meshes separately, but still, if you use 'mesh' ever, either read or write, then it generates a clone which you need to explicitly destroy as well as the original mesh which you remembered from earlier on.
     
    DuncanMJA likes this.