SharedMaterial rant

unity_sxF4kWUhnH3bzw · March 30, 2018, 7:21pm

I gave myself a headache a few weeks ago trying to modify a gameObject’s material and thought I had figured out the issue, but I ran into the same problem today.

When you try to modify an object’s material and run a test against it, the editor completely forbids you from doing so. This is because it might cause… MEMORY LEAKS IN THE EDITOR !!! (GASP) :( and shortly thereafter, the end of all life as we know it. It’s definitely impossible to either have Unity clean this up or give the user a WARNING instead of an error and allow them to clean it up manually.

And since it’s explicitly forbidden, we should issue an error message that sounds like a warning and suggests that it ISN’T explicitly forbidden:

[Error] Instantiating material due to calling renderer.material during edit mode. This will leak materials into the scene. You most likely want to use renderer.sharedMaterial instead.

How is it not bad practice to expose a variable to changes through scripting in a game context, but not in the editor context? How am I to test the functionality in the game if I can’t run tests in the editor? Why is material even exposed if it’s so dangerous to the editor? Is there any legitimate case for modifying material?

Oh, right, I’m supposed to use sharedMaterial. Well that sounds good, but here’s the problem: I don’t want to use a material which is shared between multiple objects. I want to create a new instance of this object and refer to its material, and not to the material on other instances of the object.

… what? SharedMaterial ISN’T shared between multiple objects, or even multiple instances of the same object? It actually refers to a single new material on a new object, and will not be shared unless I explicitly pass it around? Shouldn’t it then be called SHAREABLE material? Don’t almost all objects have the property of potentially being assigned to multiple objects by reference, and if so does that mean we have to rename EVERYTHING IN UNITY to have the prefix “shared”?

If this thing can do everything that I want ‘material’ to do, why not just call this thing ‘material’ and delete the old ‘material’ which is somehow mysteriously impossible to clean up after in the editor context? I guess naming objects based on their intrinsic qualities, and not qualities which they might possess if and only if new qualities are explicitly added, is passé.

All of this has the feel of being a messy, patchy, poorly considered solution to some deep engineering problem that I don’t have knowledge of. Quite possibly I am a complete idiot. I find it hard to believe that this can’t be done better, or at LEAST that ‘sharedMaterial’ can’t be given a better name.

All I want to do is get the specific material that actually exists on an object. I don’t want to risk altering random materials on other objects because I have to address a material that’s shared between multiple objects. If the object doesn’t exist and needs to be instantiated for a test, surely Unity should instantiate the entire object INCLUDING A NEW MATERIAL and then delete it after the test, again INCLUDING THE INSTANTIATED MATERIAL so it doesn’t leak. I don’t understand why this is difficult.

lordofduct · March 30, 2018, 8:17pm

For starters.

I agree, the naming of material and sharedMaterial is really weird and confusing. And is really the result of the engine history where features were added on in post, and the existing properties had to remain the same, and have some sort of familiar operation to remain compatible… while adding on the new feature.

This combined with Unity’s early designs of the engine being very ‘novice’ friendly results in bizarre naming and system choices. For example… the idea of accessing a property causing the object to behave different is a bizarre choice to me. I’d rather think it’d be a better design that there was a function that transitions the state of the object.

That being… if you access ‘material’, it causes Renderer to duplicate the material and start using this clone instead. Rather I think it’d be better to have a function like ‘Material MakeMaterialUnique()’ that would do this. And then the material proeprty just returns which ever material it has… shared or otherwise.

But alas, I guess they rather desired that the name of the property to some how reflect the intent. But end up with a weird situation. Where ‘material’ is always this object’s material and only this object’s if accessed. And ‘sharedMaterial’ is this object’s material shared or otherwise.

Weird semantic situation IMO.

…

With that said.

I find some of what you said to be counter to my experience.

For starters, I want to ask what version of Unity you use, as Unity may have changed things that I’m unaware of.

Next, something you said later in your post:

Wait, what?

‘sharedMaterial’ is shared between multiple objects. Unless you’ve accessed material (which clones and makes the material unique to that object). If you never access the ‘material’ property, the material used is the one that is shared by all instances.

Also this error:

When does this happen?

I don’t get this when I access material in both 5.6.3, and 2017.3

I just do this simple code:

    private void Start()
    {
        var m = this.GetComponent<Renderer>().material;
        m.color = Color.blue;
    }

Is there something else you’re doing to copy out material?

methos5k · March 31, 2018, 12:24am

I think you get that error/warning if you run in edit mode.

lordofduct · March 31, 2018, 1:44am

Is this a mode I’m unaware of?

I assumed they meant when running in the editor. Like that play button at the top.

Is there another edit mode?

Like during an ‘editor script’ or something? Hrmm… I’ma test that.

[edit]
Didn’t test that yet, just wanted to add on that I can see the error/warning appearing then. Because duplicating and editing a material at edit time would be weird usually. There wouldn’t be a related asset, so any changes weren’t getting saved. So there’s no real reason to do it. Unless you wanted to create an editor time animation of some sort… which could explain why I never ran into it since I don’t have a need to modify materials in an editor script that aren’t an asset.

methos5k · March 31, 2018, 2:18am

Ya, I’m pretty sure I came across it when someone’s code I was looking at/tested had ExecuteInEditMode.

unity_sxF4kWUhnH3bzw · April 5, 2018, 7:22pm

This is in Unity 2017.3.1f1, in a testing context. If I have a script that does:

    public void SetOpacity(float opacity)
    {
        var newColor = gameObject.GetComponent<Renderer>().material.color;
        newColor.a = opacity;
        gameObject.GetComponent<Renderer>().material.color = newColor;
    }

And a test that does:

    [Test]
    [Category("Unit_Tests")]
    public void SetOpacity_SetsOpacityCorrectly()
    {
        testSubject.SetOpacity(.5);

        float actualOpacity = testSubject.GetComponent<Renderer>().material.color.a;
        Assert.AreEqual(0.5f, actualOpacity);
    }

When I try to run this in the Test Runner, the editor will scream bloody murder that I’m leaking generated materials into the scene. Which shouldn’t even matter at all, because the worst case scenario is that I just kill my editor if it gets slow - as you noted, this only affects “Edit mode”, whatever that is. I should also be allowed to do manual cleanup myself. This should be a warning, not an error.

The error suggests using sharedMaterial, but I don’t understand what that is, why it’s called “shared”, if it’s actually shared or not… if indeed it’s the thing I want to manipulate in my case. Modifying material directly sounds like exactly what I want to do. It just causes problems in this one specific case for the editor.

Indeed, modifying material works as expected in the scene. Therefore my answer to this problem has been… just don’t write any tests for the behavior!

You can’t even pass it a new material to ensure that one won’t be created. It won’t even check to see it one already exists - it just blindly instantiates another and then cries about how it can’t clean up its own mess. You don’t even have to do anything with it - it’s simply impossible to even make a reference to an object’s material while tests are running. This code in a test also produces the same error at the last line:

            Material material = new Material(Shader.Find("Diffuse"));
            testSubject.GetComponent<Renderer>().material = material;
            Debug.Log("material = " + frame.GetComponent<Renderer>().material);