Materials do not fire changed signal when any of its properties change

[Kubulambula]

Godot version

4.0.beta7

System information

Irelevant

Issue description

Any Material Node Resource does not emit the changed signal when its properties change (Properties of base Resources still emit the signal)

Steps to reproduce

Copy-paste this script to any node

@tool
extends Node


@export var x: Resource:
	set(value):
		print("set")
		x = value
		x.changed.connect(foo)


func foo():
	print("changed")

Try different Resources and change their properties and watch when the changed signal is emmited.
When testing any Material derivate, any of its properties changing did not emitted the signal.

Minimal reproduction project

See above

[Kubulambula]

I did some digging and there is no emit_changed() function call is made from Material except when changing shaders. Is this by design? Shouldn't the signal be fired every time any property of the Resource is changed?

[clayjohn]

The docs for changed and emit_changed() appear to indicate that change will be emitted for any change in a user facing property.

That being said, from working with the codebase for awhile, that definitely is not the case in practice. Many property setters do not have emit_changed() in them. I have always thought this was for performance reasons. In other words, only where the engine requires the changed signal will a property.

it looks like the docs were updated relatively recently by Mickeon in #67072. @Mickeon Do you have any additional insight as to whether emit_changed should be called from every property of a resource-inheriting class?

[Mickeon]

I have always thought this was for performance reasons. In other words, only where the engine requires the changed signal will a property.

This is probably it, to be honest. Hence my highlighting on the "usually" part of it. I am not sure if having them emit "changed" would be a performance burden, or if they normally should but they aren't as a result of a bug, right now.

Any Material node

But wait, Material is not a Node, it is a Resource.

[Kubulambula]

But wait, Material is not a Node, it is a Resource.

Ooops. I meant Resource

[Kubulambula]

The docs for changed and emit_changed() appear to indicate that change will be emitted for any change in a user facing property.

That's why I expect to receive the signal on any change.
I am using a @tool script to scale and distort a Mesh based on the Material's albedo_texture and other stuff in the editor so that artists in my team can have easier life.

If this is how it is with other Resources, there should be a better consistency on when the signal is or isn't emmited.

[outobugi]

I encountered this problem as well, but ended up using "property_list_changed" signal.
I'm making a terrain editor which uses individual ORM Materials and had to know when those change so I can update the texture arrays.

Edit: As expected "property_list_changed" doesn't fire on value changes, but only on texture changes. I need to know those changes too.

[clayjohn]

We discussed this on rocketchat with a few core contributors and the consensus is that the documentation should be changed to no longer say that changed will be emitted when any user-facing property is changed. The current state is that changed is emitted on a case-by-case basis in setters where users have requested it and shown a need for it.

The documentation for emit changed should be changed from:

Emits the [changed](https://docs.godotengine.org/en/latest/classes/class_resource.html#class-resource-signal-changed) signal. This method is called automatically for built-in resources.

to

Emits the [changed](https://docs.godotengine.org/en/latest/classes/class_resource.html#class-resource-signal-changed) signal. This method is called automatically for some built-in resources.

Then we should strive to note in the documentation for which properties changed will be emitted.

[aXu-AP]

Going through all members of all resources will be a feat in itself, however if somebody wants to do it, we should have a reusable text to add in appropriate places. How about:

Setting [member x] will emit [signal Resource.changed] signal.

I also wrote a small tool script to test if changing certain properties will trigger changed.

@tool
extends Node

@export var resource : Resource:
	set(value):
		resource = value
		resource.changed.connect(func(): print("Resource changed"))

EDIT: I'm not sure why I didn't notice that op already supplied very similiar code snippet 😅

This may be automated further by iterating through property list and setting values, however I'm not sure how to handle all types in reasonable manner.

[Mickeon]

I think it would be more reasonable to note when changed is NOT automatically emitted, instead.

[aXu-AP]

Material does really seem to be the odd one here, so far in my sporadical testing most resources do emit changed. There's some quirky ones as well, like SpriteFrames, which emits the signal when editing frames, but doesn't when adding/removing a whole animation.

[josefalanga]

I'm taking the update for the docs project.

Then we should strive to note in the documentation for which properties changed will be emitted.

Just for clarification, this details should be added in the Material class docs, right? Not in the Resource class docs.

[vvvvvvitor]

Is this still open? I was thinking of fixing this as my first contribution to the project.

[AThousandShips]

It is not, sorry been a PR open and just linking it

[vvvvvvitor]

It is not, sorry been a PR open and just linking it

Ah, it's fine, I'll just look for another issue!