Property list changes are only notified when it did change in Curve

[Rindbee]

Object::notify_property_list_changed() should only be called when the structure of the property has actually changed.

The structure of the property tree has not changed if the size of the array has not changed, and the only possibility is that the element value has changed.

Fix #74865.

[MewPurPur]

It's nice that it fixes the most visible symptom of my issue, but the curve editor will still be reconstructed when adding or removing points, which isn't expected behavior.

For example in a WIP commit where I implemented a snap button in the curve editor, every time I add/remove a point, the snap button would get untoggled. Makes the whole thing dysfunctional. This helps with the re-construction issue but it's not a full fix IMO

[Rindbee]

but the curve editor will still be reconstructed when adding or removing points, which isn't expected behavior.

Because when adding or removing array elements, the structure of the property tree does change. See Curve::_get_property_list().

Perhaps when the size of Array changes, we can try to use a new signal to specifically notify this change. Make these array properties behave like a sub-inspector (same as EditorPropertyResource) in the inspector.

[Rindbee]

And EditorInspector::update_tree() is not only a redraw process, but also a rebuild process. I think it is more appropriate to rename it to EditorInspector::rebuild_tree(). One reason the editor is not smooth is that it is called too often. Usually for the whole tree.

[anvilfolk]

Did a very quick look - the thought process and changes do seem to make sense. I like the discussion around making a sub-editor just for the points though, since that would allow changing points without resetting the rest of the editor. I think that would help @MewPurPur 's future efforts to add snapping!

Any chance that makes sense to add in this PR, or is that a future PR?

[YuriSizov]

I don't see why we would need to redraw the entire tree when the structure of an array or a dictionary property changes. It's all handled by a single editor property control (from the POV of the inspector), we don't really need to update the entire tree.

[anvilfolk]

@YuriSizov so the issue is that the points of the curve are individually exported as properties. The curve doesn't export the array of points:

godot/scene/resources/curve.cpp

Lines 1263 to 1281 in c0a48a3

	void Curve2D::_get_property_list(List<PropertyInfo> *p_list) const {
	for (int i = 0; i < points.size(); i++) {
	PropertyInfo pi = PropertyInfo(Variant::VECTOR2, vformat("point_%d/position", i));
	pi.usage &= ~PROPERTY_USAGE_STORAGE;
	p_list->push_back(pi);
	if (i != 0) {
	pi = PropertyInfo(Variant::VECTOR2, vformat("point_%d/in", i));
	pi.usage &= ~PROPERTY_USAGE_STORAGE;
	p_list->push_back(pi);
	}
	if (i != points.size() - 1) {
	pi = PropertyInfo(Variant::VECTOR2, vformat("point_%d/out", i));
	pi.usage &= ~PROPERTY_USAGE_STORAGE;
	p_list->push_back(pi);
	}
	}
	}

Whereas, for instance, GradientTexture2D directly exports the underlying packed arrays. This means that the property list doesn't change, and therefore the inspector tree isn't rebuilt (to display the "new" properties). For Curve, since the property list changed, the inspector is going to rebuild the tree in order to display all the properties.

Unfortunately, I believe it would be a compatibility-breaking change to directly export the underlying point & tangent arrays directly.

I think maybe the way to handle this is to follow up on the idea of having a subinspector just for the points, and that's what gets updated when the property list changes, leaving the rest untouched (particularly, the visual curve editor itself), which is at the origin of this bug. But I also don't fully understand UI and whether this is possible :)

[Rindbee]

The array here refers to using ADD_ARRAY/ADD_ARRAY_COUNT and _get_property_list() to organize the properties of the object into an array (like an array of structures).

[YuriSizov]

Well, I guess it makes sense to make this change for now to speed things up a bit. Looks like a proper solution would require bigger changes to the way we work with the property list in the inspector.

[YuriSizov]

Thanks!